Roave/BetterReflection

Consider making `Roave\BetterReflection\Reflection\ReflectionClass#getMethod()` non-nullable?

Ocramius opened this issue · 3 comments

Ocramius commented

We currently have:

  • Roave\BetterReflection\Reflection\ReflectionClass#getMethod(): ReflectionMethod|null
  • Roave\BetterReflection\Reflection\ReflectionClass#hasMethod(): bool

This API is kinda dumb-ish, and we kinda went back on it in 6.0.0, IMO: an exception being thrown was probably a much nicer API here.

Either we make getMethod() throw, or we remove hasMethod() here. Relying on getMethod() throwing is not so bad here, but either way could work.

kukulich commented

We should make getProperty() etc use the same pattern to be consistent.

  • 👍1
staabm commented

in case you don't want todo another major in the near future I think you could also use phpdoc to narrow the return type in the meantime.

phpstan does a similar thing with phpstan/phpstan-src@ace76ce

its a pretty recent feature in phpstan though

Ocramius commented

@staabm a new major would be OK here, IMO.

We aren't that worried about BC, on a codebase that provides full type safety (and therefore is easy yo upgrade to)