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

Add throws tag #1273

Merged
merged 1 commit into from
Jan 13, 2021
Merged

Add throws tag #1273

merged 1 commit into from
Jan 13, 2021

Conversation

VincentLanglet
Copy link
Contributor

Hi @goetas

Your implementation of this interface is throwing Runtime exceptions, so it should be good to add the @throws tag to the interface ; this way the developer is aware to catch them.

@VincentLanglet
Copy link
Contributor Author

I don't think the scrutinizer error is related to my PR.

Are you interested @goetas ? :)

@goetas
Copy link
Collaborator

goetas commented Jan 13, 2021

TBH I do not know.

PHP (contrary to Java) does not force you to catch the declared exceptions, so do not see really a value in declaring them.

It would just make a lot of noise, and it we want to be strict we would have to place hundreds of @throw SomeException all over the code

@VincentLanglet
Copy link
Contributor Author

Hi

PHP (contrary to Java) does not force you to catch the declared exceptions, so do not see really a value in declaring them.

To me, it's like saying (when we were in php5)

PHP does not force you to respect @param and @return so do not see really value declaring them.

PHP does not force you, but some static analysis like phpstan/psalm can help you to report uncaught exceptions. But only if they are declared in the phpdoc, of course.

I got an error in production because I didn't catch the exception, and nothing helps me to know they were an exception to catch. This would have avoid such error.

It would just make a lot of noise

Why noise ? Phpdoc is documentation
As an example, symfony interfaces have @throws tags: https://github.com/symfony/contracts/search?q=throws

it we want to be strict we would have to place hundreds of @throw SomeException all over the code

Yes, maybe. But if so, that does mean that currently, people are using method which can throws exception without knowing it ! That's seems to be a risky thing.

People shouldn't wait to get an exception to know they have to catch something.

@goetas
Copy link
Collaborator

goetas commented Jan 13, 2021

Anyway I think that this will not hurt, so it is ok.

@goetas goetas merged commit d214941 into schmittjoh:master Jan 13, 2021
@VincentLanglet
Copy link
Contributor Author

Thanks

@VincentLanglet VincentLanglet deleted the addException branch January 13, 2021 14:08
@VincentLanglet
Copy link
Contributor Author

Does a 3.11.1 release would be possible ? @goetas :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants