Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Ocramius opened this issue Oct 10, 2022 · 3 comments

Comments

@Ocramius
Copy link
Member

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
Copy link
Collaborator

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

Ocramius added a commit to Roave/BackwardCompatibilityCheck that referenced this issue Oct 10, 2022
Ocramius added a commit to Roave/BackwardCompatibilityCheck that referenced this issue Oct 10, 2022
@staabm
Copy link
Contributor

staabm commented Oct 15, 2022

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
Copy link
Member Author

@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)

uzibhalepu added a commit to uzibhalepu/BackwardCompatibilityCheck that referenced this issue Aug 6, 2024
uzibhalepu added a commit to uzibhalepu/BackwardCompatibilityCheck that referenced this issue Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants