Generic/ConstructorName: bug fix x 2 #652
Open
+66
−13
Conversation
This file contains 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
…nd comments When determining whether or not a "parent" constructor was being called using a call to a PHP4-style constructor, the sniff would only look at the _next_ token after a double colon, while the next token may be whitespace or a comment, which should be ignored. Fixed now by making the sniff check for the _next non empty_ token instead. Includes tests.
…ther class When determining whether or not a "parent" constructor was being called using a call to a PHP4-style constructor, the sniff would only look at the _next_ token after a double colon, disregarding completely the class the method is being called on. This would lead to false positives for method calls to other classes, which would just happen to have a method named the same as the class being extended. This commit fixes this by verifying that the previous non-empty token before the double colon is either a hierarchy keyword or the name of the class being extended and only throwing an error if that's the case. Includes tests.
As things were, the `$startIndex` would be the `function` keyword in the function declaration, which means that the complete declaration (parameters, defaults etc) would be walked, while we only really want to look _inside_ the function body. Fixed by starting the `while` loop on the `scope_opener` instead. Additionally, while the `$startIndex` would be moved forward on each loop, it would still examine one token too much (`scope_opener` cannot be a `T_DOUBLE_COLON`, `T_STRING` after `T_DOUBLE_COLON` cannot be a `T_DOUBLE_COLON`). Also fixed now.
11 tasks
Sign up for free
to 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.
Description
Generic/ConstructorName: bug fix - allow for unconventional spacing and comments
When determining whether or not a "parent" constructor was being called using a call to a PHP4-style constructor, the sniff would only look at the next token after a double colon, while the next token may be whitespace or a comment, which should be ignored.
Fixed now by making the sniff check for the next non empty token instead.
Includes tests.
Generic/ConstructorName: bug fix - false positive on static call to other class
When determining whether or not a "parent" constructor was being called using a call to a PHP4-style constructor, the sniff would only look at the next token after a double colon, disregarding completely the class the method is being called on.
This would lead to false positives for method calls to other classes, which would just happen to have a method named the same as the class being extended.
This commit fixes this by verifying that the previous non-empty token before the double colon is either a hierarchy keyword or the name of the class being extended and only throwing an error if that's the case.
Includes tests.
Generic/ConstructorName: minor efficiency tweak
As things were, the
$startIndexwould be thefunctionkeyword in the function declaration, which means that the complete declaration (parameters, defaults etc) would be walked, while we only really want to look inside the function body.Fixed by starting the
whileloop on thescope_openerinstead.Additionally, while the
$startIndexwould be moved forward on each loop, it would still examine one token too much (scope_openercannot be aT_DOUBLE_COLON,T_STRINGafterT_DOUBLE_COLONcannot be aT_DOUBLE_COLON).Also fixed now.
Suggested changelog entry
Related issues/external references
Inspired by #649
Types of changes