Add PhpUnitMethodCasingFixer #3886
Conversation
Slamdunk
changed the title
Slamdunk
changed the title
| @@ -108,6 +108,7 @@ protected function createConfigurationDefinition() | |||
| (new FixerOptionBuilder('case', 'Whether to camel or snake case when adding the test prefix')) | |||
| ->setAllowedValues(['camel', 'snake']) | |||
| ->setDefault('camel') | |||
| ->setDeprecationMessage('Use `php_unit_method_casing` fixer instead.') | |||
There was a problem hiding this comment.
can php_unit_method_casing do same thing as this option ?
There was a problem hiding this comment.
Well, theoretically no because someone could want to transform /** @test */ function fooBar in function test_fooBar and only with this option you can do this.
But does this example make sense?
There was a problem hiding this comment.
then, if deprecated feature is not fully represented by new feature, the difference shall be mentioned as addition to "use X instead"
There was a problem hiding this comment.
Sounds right, but I have no idea what could I wrote more to explain the intentions.
If someone used to use case option for Annotation fixer, the only possibility that comes into my mind is that the developer is taking care of method casing in PHPUnit classes.
The name of the new fixer is self explanatory, as this is the exact purpose.
There was a problem hiding this comment.
well, actually, when I saw the new rule name, i thought it's about proper casing of assertsame or ExPeCtEdExCePtIoN...
| if (self::CAMEL_CASE === $this->configuration['case']) { | ||
| $newFunctionName = $functionName; | ||
| $newFunctionName = str_replace('_', ' ', $newFunctionName); | ||
| $newFunctionName = ucwords($newFunctionName); |
There was a problem hiding this comment.
ucwords takes delimiter as 2nd param, so you can put there _ and avoid remove str_replace above.
|
|
||
| /** | ||
| * @param Tokens $tokens | ||
| * @param int$index |
There was a problem hiding this comment.
Seems like you have found a bug in phpdoc_align - shouldn't this line fail style checking?
| return false; | ||
| } | ||
|
|
||
| // if the function name starts with test its a test |
| if ($this->startsWith('test', $functionName)) { | ||
| return true; | ||
| } | ||
| // If the function doesn't have test in its name, and no doc block, its not a test |
There was a problem hiding this comment.
1st its is correct, 2nd isn't -> https://data.grammarbook.com/blog/pronouns/1-grammar-error/
| */ | ||
| private function startsWith($needle, $haystack) | ||
| { | ||
| $len = strlen($needle); |
There was a problem hiding this comment.
(Personal) I'd drop $len here and put strlen directly in substr to have one-liner.
|
@kubawerlos thank you very much for the review. |
|
@Slamdunk I think no need to fix it here - maybe in separate PR it's worth to give try to initialize this fixer in |
keradus
force-pushed
the
phpunit_method_casing
branch
from
9389def
to
b87f254
Compare
|
Thank you @Slamdunk. |
This PR was squashed before being merged into the 2.13-dev branch (closes #3886). Discussion ---------- Add PhpUnitMethodCasingFixer Closes #3302 ```diff class MyTest extends \PhpUnit\FrameWork\TestCase { - public function test_my_code() {} + public function testMyCode() {} } ``` - [x] Implement base fixer - [x] Take care of `@depends` - [x] Run after `PhpUnitTestAnnotationFixer` - [x] Deprecate `PhpUnitTestAnnotationFixer` *case* config Commits ------- b87f254 Add PhpUnitMethodCasingFixer
|
this tripped https://travis-ci.org/FriendsOfPHP/PHP-CS-Fixer/jobs/419018045#L1863 |
|
i will handle it |
|
ref #3979 |
|
Thanks for your work @Slamdunk, exactly what I needed. Snake case in tests ftw ;-) |
Closes #3302
class MyTest extends \PhpUnit\FrameWork\TestCase { - public function test_my_code() {} + public function testMyCode() {} }@dependsPhpUnitTestAnnotationFixerPhpUnitTestAnnotationFixercase config