PhpdocNoEmptyReturnFixer - account for null[] #3891
Conversation
| @@ -105,7 +105,7 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens) | |||
| */ | |||
| private function fixAnnotation(DocBlock $doc, Annotation $annotation) | |||
| { | |||
| if (1 === Preg::match('/@return\s+(void|null)(?!\|)/', $doc->getLine($annotation->getStart())->getContent())) { | |||
| if (1 === Preg::match('/@return\s+(void|null)(?!\||\[)/', $doc->getLine($annotation->getStart())->getContent())) { | |||
There was a problem hiding this comment.
maybe instead of negative lookahead, let us ensure there is whitespace character after null ? or use DocBlock component here and rely on logic that is already there...
There was a problem hiding this comment.
There doesn't have to be a whitespace character, as discussed before on gitter. But you wanted me to fix that case as separate PR 😉
There was a problem hiding this comment.
still, I believe we shall switch to DocBlock, and if issue exist fix it there, for all fixers, and not here, for single fixer
dmvdbrugge
force-pushed
the
3869-phpdoc_no_empty_return-less-greedy
branch
from
db725c7
to
882ccda
Compare
| $expected = <<<'EOF' | ||
| <?php | ||
| /** | ||
| * @return null[]|string[] |
There was a problem hiding this comment.
could you add a test to make sure the change is case-insensitive, i.e. NULL[] and VOID[] get fixed as well?
There was a problem hiding this comment.
Nice catch, they didn't because Annotation::getTypes isn't normalized. Fix incoming.
dmvdbrugge
force-pushed
the
3869-phpdoc_no_empty_return-less-greedy
branch
2 times, most recently
from
99a7880
to
edf3e5e
Compare
tests/DocBlock/AnnotationTest.php
Outdated
| * | ||
| * @dataProvider provideNormalizedTypesCases | ||
| */ | ||
| public function testNormalizedTypes($input, $expected) |
There was a problem hiding this comment.
please flip the arguments
dmvdbrugge
force-pushed
the
3869-phpdoc_no_empty_return-less-greedy
branch
from
edf3e5e
to
4fe7b78
Compare
src/DocBlock/Annotation.php
Outdated
| @@ -226,6 +226,15 @@ public function setTypes(array $types) | |||
| $this->clearCache(); | |||
| } | |||
|
|
|||
| public function getNormalizedTypes() | |||
There was a problem hiding this comment.
please add a PHPDoc with a return type hint, should be @return string[] I think
src/DocBlock/Annotation.php
Outdated
| @@ -226,6 +226,15 @@ public function setTypes(array $types) | |||
| $this->clearCache(); | |||
| } | |||
|
|
|||
| public function getNormalizedTypes() | |||
| { | |||
| $normalized = array_map('strtolower', $this->getTypes()); | |||
There was a problem hiding this comment.
maybe we want this refactored to not rely on the function name, but I'll leave that to a second reviewer :)
There was a problem hiding this comment.
yes, we don't want to call functions by string with their name, as it's impossible to analyse with static code analysis
please use anonymous function as a wrapper here
There was a problem hiding this comment.
Would be good if there were a fixer for that ;)
There was a problem hiding this comment.
I did it this way because I had seen that in NativeConstantInvocationFixer. Want me to fix it there as well, or just here?
There was a problem hiding this comment.
Fixed here, let me know if you want me to fix the other one too.
- Use $annotation->getTypes() instead of Preg::match - Apply yoda - Use normalized types - Add test for new method - With test params in correct order - Add phpdoc and use anonymous function instead of string
dmvdbrugge
force-pushed
the
3869-phpdoc_no_empty_return-less-greedy
branch
from
e04702a
to
23084e9
Compare
keradus
changed the title
|
Thank you @dmvdbrugge. |
…r last annotation (dmvdbrugge) This PR was merged into the 2.15 branch. Discussion ---------- Fix handling /** and */ on the same line as the first and/or last annotation In both `Annotation::remove()` and `PhpdocNoEmptyReturnFixer` This PR is sort-of combined with #3891 Commits ------- ad23563 Fix handling `/**` and `*/` on the same line as the first and/or last annotation
This fixes #3869