Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
Be more detailed about coding standards for PHPdoc#17064
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Merged
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Merged
2 tasks
Member
fabpot left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I would also add a note that phpdocs are not part of our BC promise policy (maybe something to also add on our BC promise policy doc).
Uh oh!
There was an error while loading.Please reload this page.
| order to improve types for static analysis, the following annotations are | ||
| also allowed: | ||
| * `Generics`_, with the exception of ``@template-covariant``. |
Member
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why do we make that exception?
MemberAuthor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As far as I know, this is not (yet?) supported by PHPstan.
Member
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
phpstan supports it (otherwise, they would not havethis blog post explaining it)
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
fabpot approved these changesJul 29, 2022
Member
javiereguiluz commentedJul 29, 2022
Thank you Wouter. |
nicolas-grekas added a commit to symfony/symfony that referenced this pull requestAug 2, 2022
…PHPstan and Psalm (mdeboer, wouterj)This PR was merged into the 6.2 branch.Discussion----------Improve some PHPdocs based on existing Symfony stubs in PHPstan and Psalm| Q | A| ------------- | ---| Branch? | 6.2| Bug fix? | no| New feature? | no| Deprecations? | no (strictly spoken yes, as I guess DebugClassLoader will trigger deprecations for some now?)| Tickets | Replaces#40783| License | MIT| Doc PR |symfony/symfony-docs#17064Todo---* [x] Review the Psalm check of this PR* [x] Test the changes with DebugClassLoader in a real appDescription---This PR adds some more information to the PHPdoc of Symfony classes. By moving the information from the current stubs of static analyzers into Symfony itself: (1) we can improve the experience for all users, (2) reduce false-positives from our own Psalm check and (3) make sure they are in sync with changes done on these classes.<s>To avoid a PR that is too big to review and maintain, the scope of this PR is deliberately kept narrow:* Only PHPdoc from either [Psalm's Symfony stubs](https://github.com/psalm/psalm-plugin-symfony/tree/master/src/Stubs) or [PHPStan's Symfony stubs](https://github.com/phpstan/phpstan-symfony/tree/1.2.x/stubs) is added* The PHPdoc MUST NOT break PHPstorm* The PHPdoc MUST be supported by both PHPstan and Psalm (those are the only two static analyzers that currently ship an official Symfony plugin afaik)* The PHPdoc SHOULD NOT duplicate anything that can already be deduced from either PHP's type declarations or already existing PHPdoc.* The PHPdoc MUST document the contract and NOT be added to fit a 95% use-case or improve auto-completion.</s>EDIT: Replaced the guidelines bysymfony/symfony-docs#17064On top of this, to get this PR approved quicker, I've left out all stubs from the Form (based on the discussions in#40783) and most of [PHPStan's `Envelope` stub](https://github.com/psalm/psalm-plugin-symfony/blob/master/src/Stubs/common/Component/Messenger/Envelope.stubphp) (due to complexity of the PHPdoc).Commits-------f5a802a [DependencyInjection] Add nice exception when using non-scalar parameter as array keyfa7703b [ErrorHandler] Do not patch return types that are not a valid PHP type38ab6de Improve some PHPdocs based on existing Symfony stubs in PHPstan and Psalm
symfony-splitter pushed a commit to symfony/serializer that referenced this pull requestAug 2, 2022
…PHPstan and Psalm (mdeboer, wouterj)This PR was merged into the 6.2 branch.Discussion----------Improve some PHPdocs based on existing Symfony stubs in PHPstan and Psalm| Q | A| ------------- | ---| Branch? | 6.2| Bug fix? | no| New feature? | no| Deprecations? | no (strictly spoken yes, as I guess DebugClassLoader will trigger deprecations for some now?)| Tickets | Replaces #40783| License | MIT| Doc PR |symfony/symfony-docs#17064Todo---* [x] Review the Psalm check of this PR* [x] Test the changes with DebugClassLoader in a real appDescription---This PR adds some more information to the PHPdoc of Symfony classes. By moving the information from the current stubs of static analyzers into Symfony itself: (1) we can improve the experience for all users, (2) reduce false-positives from our own Psalm check and (3) make sure they are in sync with changes done on these classes.<s>To avoid a PR that is too big to review and maintain, the scope of this PR is deliberately kept narrow:* Only PHPdoc from either [Psalm's Symfony stubs](https://github.com/psalm/psalm-plugin-symfony/tree/master/src/Stubs) or [PHPStan's Symfony stubs](https://github.com/phpstan/phpstan-symfony/tree/1.2.x/stubs) is added* The PHPdoc MUST NOT break PHPstorm* The PHPdoc MUST be supported by both PHPstan and Psalm (those are the only two static analyzers that currently ship an official Symfony plugin afaik)* The PHPdoc SHOULD NOT duplicate anything that can already be deduced from either PHP's type declarations or already existing PHPdoc.* The PHPdoc MUST document the contract and NOT be added to fit a 95% use-case or improve auto-completion.</s>EDIT: Replaced the guidelines bysymfony/symfony-docs#17064On top of this, to get this PR approved quicker, I've left out all stubs from the Form (based on the discussions in #40783) and most of [PHPStan's `Envelope` stub](https://github.com/psalm/psalm-plugin-symfony/blob/master/src/Stubs/common/Component/Messenger/Envelope.stubphp) (due to complexity of the PHPdoc).Commits-------f5a802ab85 [DependencyInjection] Add nice exception when using non-scalar parameter as array keyfa7703ba4a [ErrorHandler] Do not patch return types that are not a valid PHP type38ab6de900 Improve some PHPdocs based on existing Symfony stubs in PHPstan and Psalm
symfony-splitter pushed a commit to symfony/error-handler that referenced this pull requestAug 2, 2022
…PHPstan and Psalm (mdeboer, wouterj)This PR was merged into the 6.2 branch.Discussion----------Improve some PHPdocs based on existing Symfony stubs in PHPstan and Psalm| Q | A| ------------- | ---| Branch? | 6.2| Bug fix? | no| New feature? | no| Deprecations? | no (strictly spoken yes, as I guess DebugClassLoader will trigger deprecations for some now?)| Tickets | Replaces #40783| License | MIT| Doc PR |symfony/symfony-docs#17064Todo---* [x] Review the Psalm check of this PR* [x] Test the changes with DebugClassLoader in a real appDescription---This PR adds some more information to the PHPdoc of Symfony classes. By moving the information from the current stubs of static analyzers into Symfony itself: (1) we can improve the experience for all users, (2) reduce false-positives from our own Psalm check and (3) make sure they are in sync with changes done on these classes.<s>To avoid a PR that is too big to review and maintain, the scope of this PR is deliberately kept narrow:* Only PHPdoc from either [Psalm's Symfony stubs](https://github.com/psalm/psalm-plugin-symfony/tree/master/src/Stubs) or [PHPStan's Symfony stubs](https://github.com/phpstan/phpstan-symfony/tree/1.2.x/stubs) is added* The PHPdoc MUST NOT break PHPstorm* The PHPdoc MUST be supported by both PHPstan and Psalm (those are the only two static analyzers that currently ship an official Symfony plugin afaik)* The PHPdoc SHOULD NOT duplicate anything that can already be deduced from either PHP's type declarations or already existing PHPdoc.* The PHPdoc MUST document the contract and NOT be added to fit a 95% use-case or improve auto-completion.</s>EDIT: Replaced the guidelines bysymfony/symfony-docs#17064On top of this, to get this PR approved quicker, I've left out all stubs from the Form (based on the discussions in #40783) and most of [PHPStan's `Envelope` stub](https://github.com/psalm/psalm-plugin-symfony/blob/master/src/Stubs/common/Component/Messenger/Envelope.stubphp) (due to complexity of the PHPdoc).Commits-------f5a802ab85 [DependencyInjection] Add nice exception when using non-scalar parameter as array keyfa7703ba4a [ErrorHandler] Do not patch return types that are not a valid PHP type38ab6de900 Improve some PHPdocs based on existing Symfony stubs in PHPstan and Psalm
symfony-splitter pushed a commit to symfony/messenger that referenced this pull requestAug 2, 2022
…PHPstan and Psalm (mdeboer, wouterj)This PR was merged into the 6.2 branch.Discussion----------Improve some PHPdocs based on existing Symfony stubs in PHPstan and Psalm| Q | A| ------------- | ---| Branch? | 6.2| Bug fix? | no| New feature? | no| Deprecations? | no (strictly spoken yes, as I guess DebugClassLoader will trigger deprecations for some now?)| Tickets | Replaces #40783| License | MIT| Doc PR |symfony/symfony-docs#17064Todo---* [x] Review the Psalm check of this PR* [x] Test the changes with DebugClassLoader in a real appDescription---This PR adds some more information to the PHPdoc of Symfony classes. By moving the information from the current stubs of static analyzers into Symfony itself: (1) we can improve the experience for all users, (2) reduce false-positives from our own Psalm check and (3) make sure they are in sync with changes done on these classes.<s>To avoid a PR that is too big to review and maintain, the scope of this PR is deliberately kept narrow:* Only PHPdoc from either [Psalm's Symfony stubs](https://github.com/psalm/psalm-plugin-symfony/tree/master/src/Stubs) or [PHPStan's Symfony stubs](https://github.com/phpstan/phpstan-symfony/tree/1.2.x/stubs) is added* The PHPdoc MUST NOT break PHPstorm* The PHPdoc MUST be supported by both PHPstan and Psalm (those are the only two static analyzers that currently ship an official Symfony plugin afaik)* The PHPdoc SHOULD NOT duplicate anything that can already be deduced from either PHP's type declarations or already existing PHPdoc.* The PHPdoc MUST document the contract and NOT be added to fit a 95% use-case or improve auto-completion.</s>EDIT: Replaced the guidelines bysymfony/symfony-docs#17064On top of this, to get this PR approved quicker, I've left out all stubs from the Form (based on the discussions in #40783) and most of [PHPStan's `Envelope` stub](https://github.com/psalm/psalm-plugin-symfony/blob/master/src/Stubs/common/Component/Messenger/Envelope.stubphp) (due to complexity of the PHPdoc).Commits-------f5a802ab85 [DependencyInjection] Add nice exception when using non-scalar parameter as array keyfa7703ba4a [ErrorHandler] Do not patch return types that are not a valid PHP type38ab6de900 Improve some PHPdocs based on existing Symfony stubs in PHPstan and Psalm
symfony-splitter pushed a commit to symfony/security-http that referenced this pull requestAug 2, 2022
…PHPstan and Psalm (mdeboer, wouterj)This PR was merged into the 6.2 branch.Discussion----------Improve some PHPdocs based on existing Symfony stubs in PHPstan and Psalm| Q | A| ------------- | ---| Branch? | 6.2| Bug fix? | no| New feature? | no| Deprecations? | no (strictly spoken yes, as I guess DebugClassLoader will trigger deprecations for some now?)| Tickets | Replaces #40783| License | MIT| Doc PR |symfony/symfony-docs#17064Todo---* [x] Review the Psalm check of this PR* [x] Test the changes with DebugClassLoader in a real appDescription---This PR adds some more information to the PHPdoc of Symfony classes. By moving the information from the current stubs of static analyzers into Symfony itself: (1) we can improve the experience for all users, (2) reduce false-positives from our own Psalm check and (3) make sure they are in sync with changes done on these classes.<s>To avoid a PR that is too big to review and maintain, the scope of this PR is deliberately kept narrow:* Only PHPdoc from either [Psalm's Symfony stubs](https://github.com/psalm/psalm-plugin-symfony/tree/master/src/Stubs) or [PHPStan's Symfony stubs](https://github.com/phpstan/phpstan-symfony/tree/1.2.x/stubs) is added* The PHPdoc MUST NOT break PHPstorm* The PHPdoc MUST be supported by both PHPstan and Psalm (those are the only two static analyzers that currently ship an official Symfony plugin afaik)* The PHPdoc SHOULD NOT duplicate anything that can already be deduced from either PHP's type declarations or already existing PHPdoc.* The PHPdoc MUST document the contract and NOT be added to fit a 95% use-case or improve auto-completion.</s>EDIT: Replaced the guidelines bysymfony/symfony-docs#17064On top of this, to get this PR approved quicker, I've left out all stubs from the Form (based on the discussions in #40783) and most of [PHPStan's `Envelope` stub](https://github.com/psalm/psalm-plugin-symfony/blob/master/src/Stubs/common/Component/Messenger/Envelope.stubphp) (due to complexity of the PHPdoc).Commits-------f5a802ab85 [DependencyInjection] Add nice exception when using non-scalar parameter as array keyfa7703ba4a [ErrorHandler] Do not patch return types that are not a valid PHP type38ab6de900 Improve some PHPdocs based on existing Symfony stubs in PHPstan and Psalm
symfony-splitter pushed a commit to symfony/console that referenced this pull requestAug 2, 2022
…PHPstan and Psalm (mdeboer, wouterj)This PR was merged into the 6.2 branch.Discussion----------Improve some PHPdocs based on existing Symfony stubs in PHPstan and Psalm| Q | A| ------------- | ---| Branch? | 6.2| Bug fix? | no| New feature? | no| Deprecations? | no (strictly spoken yes, as I guess DebugClassLoader will trigger deprecations for some now?)| Tickets | Replaces #40783| License | MIT| Doc PR |symfony/symfony-docs#17064Todo---* [x] Review the Psalm check of this PR* [x] Test the changes with DebugClassLoader in a real appDescription---This PR adds some more information to the PHPdoc of Symfony classes. By moving the information from the current stubs of static analyzers into Symfony itself: (1) we can improve the experience for all users, (2) reduce false-positives from our own Psalm check and (3) make sure they are in sync with changes done on these classes.<s>To avoid a PR that is too big to review and maintain, the scope of this PR is deliberately kept narrow:* Only PHPdoc from either [Psalm's Symfony stubs](https://github.com/psalm/psalm-plugin-symfony/tree/master/src/Stubs) or [PHPStan's Symfony stubs](https://github.com/phpstan/phpstan-symfony/tree/1.2.x/stubs) is added* The PHPdoc MUST NOT break PHPstorm* The PHPdoc MUST be supported by both PHPstan and Psalm (those are the only two static analyzers that currently ship an official Symfony plugin afaik)* The PHPdoc SHOULD NOT duplicate anything that can already be deduced from either PHP's type declarations or already existing PHPdoc.* The PHPdoc MUST document the contract and NOT be added to fit a 95% use-case or improve auto-completion.</s>EDIT: Replaced the guidelines bysymfony/symfony-docs#17064On top of this, to get this PR approved quicker, I've left out all stubs from the Form (based on the discussions in #40783) and most of [PHPStan's `Envelope` stub](https://github.com/psalm/psalm-plugin-symfony/blob/master/src/Stubs/common/Component/Messenger/Envelope.stubphp) (due to complexity of the PHPdoc).Commits-------f5a802ab85 [DependencyInjection] Add nice exception when using non-scalar parameter as array keyfa7703ba4a [ErrorHandler] Do not patch return types that are not a valid PHP type38ab6de900 Improve some PHPdocs based on existing Symfony stubs in PHPstan and Psalm
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading.Please reload this page.
A proposal for the rest of the core team to consider about our PHPdoc.