Conversation
jrfnl
changed the title
PHPCompatibility.FunctionDeclarations.NewTraitAbstractPrivateMethods sniffPHPCompatibility.FunctionDeclarations.NewTraitAbstractPrivateMethods sniff
40 tasks
Member
Author
|
Related to #809 |
Member
|
I believe it makes most sense to join them, but give it a new name. That way we don't have too much duplicate code (which is what all the refactoring was designed to do anway) but the name is clear. |
…sniff This sniff replaces the `PHPCompatibility.Classes.ForbiddenAbstractPrivateMethods` sniff. The new sniff addresses both the PHP 5.1 change which forbade `abstract private` methods, as well as the PHP 8.0 change where "abstract private" methods are now allowed in traits. > Traits can now define abstract private methods. Refs: * https://wiki.php.net/rfc/abstract_trait_method_validation * php/php-src@f74e30c Includes unit tests.
jrfnl
changed the title
PHPCompatibility.FunctionDeclarations.NewTraitAbstractPrivateMethods sniffPHPCompatibility.FunctionDeclarations.AbstractPrivateMethods sniff
jrfnl
force-pushed
the
php-8.0/new-trait-abstract-private-methods-sniff
branch
from
2c9833c to
350454e
Compare
Member
Author
|
@wimg PR updated - or rather replaced - with a new commit which addresses both changes in PHP in one sniff. See the updated PR description. |
wimg
approved these changes
Jul 12, 2020
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
👉 Open questions - please answer these before considering merging this PR:
NewTraitAbstractPrivateMethodssniff. Should this check be added to the existingForbiddenAbstractPrivateMethodssniff instead ? The logic in the sniffs is > 90% the same.And if so, should the existing sniff be renamed to better cover both changes ?
Currently the
ForbiddenAbstractPrivateMethodsis in theClassescategory, while I've elected to place theForbiddenAbstractPrivateMethodsin theFunctionDeclarationscategory.IMO
FunctionDeclarationsseems more logical and if we'd choose to make a change in this for the existing sniff, now would be the time considering the next release is a major.Updated PR description - this addresses the above points:
This sniff replaces the
PHPCompatibility.Classes.ForbiddenAbstractPrivateMethodssniff.The new sniff addresses both the PHP 5.1 change which forbade
abstract privatemethods, as well as the PHP 8.0 change where "abstract private" methods are now allowed in traits.Refs:
Includes unit tests.
Related to #809
Old (superseded) PR description
PHP 8.0: new
PHPCompatibility.FunctionDeclarations.NewTraitAbstractPrivateMethodssniffPHP 8.0 will allow "abstract private" methods in traits
Refs:
This new sniff detects those.
Includes unit tests.
ForbiddenAbstractPrivateMethods: bow out for abstract private methods in traits
... as this is now checked via the new
PHPCompatibility.FunctionDeclarations.NewTraitAbstractPrivateMethodssniff and would otherwise cause duplicate and - if PHP 8 is supported - incorrect errors.