Add CombineNestedDirnameFixer #3826
Conversation
gharlan
force-pushed
the
combine-dirname
branch
5 times, most recently
from
1714ae7
to
49f009a
Compare
| * @param int $index Index of `dirname` | ||
| * @param null|int $firstArgumentEndIndex Index of last token of first argument of `dirname` call | ||
| * | ||
| * @return array|bool `false` when it is not a (suppoerted) `dirname` call, an array with infos about the dirname call otherwise |
There was a problem hiding this comment.
Typo: suppoerted => supported. Also info (information) is uncountable in english and cannot have an s.
| * @param null|string $input | ||
| * | ||
| * @dataProvider provideFixCases | ||
| * @requires PHP 7.0 |
There was a problem hiding this comment.
Please add PHP <7.0 tests showing that the fixer does nothing.
| ), | ||
| ], | ||
| null, | ||
| 'Risky when dirname() is overridden.' |
There was a problem hiding this comment.
please use `dirname`
|
Should this be added to the |
|
Indeed, I would add it. |
|
@julienfalque are happy with changes? |
|
@gharlan please, add a test case: [
'<?php new dirname(dirname($path,2));',
'<?php new dirname(dirname(dirname($path)));',
],You may want to take a look at |
|
@kubawerlos good catch 👍 I will fix it. |
|
Really cool. Didnt know this $level parameter of dirname. |
README.rst
Outdated
| @@ -390,6 +390,13 @@ Choose from the list of available rules: | |||
|
|
|||
| Calling ``unset`` on multiple items should be done in one call. | |||
|
|
|||
| * **combine_dirname** [@PHP70Migration:risky, @PHP71Migration:risky] | |||
There was a problem hiding this comment.
combine dirname with... ?
combine_multiple_dirname or combine_nested_dirname ?
There was a problem hiding this comment.
+1 for combine_nested_dirname
| $this->doTest($expected, $input); | ||
| } | ||
|
|
||
| public function provideFixCases() |
There was a problem hiding this comment.
What about dirname(dirname($path, 2), 2)? Should that result in dirname($path, 4)? Can that be done as well? (I'd think so...)
There was a problem hiding this comment.
It's already supported, I will add a test case for this.
keradus
force-pushed
the
combine-dirname
branch
from
be9d90b
to
2788f07
Compare
keradus
force-pushed
the
combine-dirname
branch
from
2788f07
to
5c6f5e2
Compare
keradus
force-pushed
the
combine-dirname
branch
from
5c6f5e2
to
398f343
Compare
|
Thank you @gharlan. |
This PR was squashed before being merged into the 2.13-dev branch (closes #3826). Discussion ---------- Add CombineNestedDirnameFixer Input: ```php dirname(dirname(dirname($path))); ``` is fixed to: ```php dirname($path, 3); ``` (Requires PHP >= 7.0) Commits ------- 398f343 Add CombineNestedDirnameFixer
|
thanks @gharlan , great to see this in :) |
Input:
dirname(dirname(dirname($path)));is fixed to:
(Requires PHP >= 7.0)