Dump fixer #2218
Dump fixer #2218
Conversation
|
should not be removed either. There is some nice stuff in the |
Best name ever. 😆 |
| $funcStart = $tokens->getPrevNonWhitespace($match[0]); | ||
|
|
||
| $funcEnd = $tokens->getNextTokenOfKind($match[1], array(';')); | ||
| for ($i = $funcStart + 1; $i <= $funcEnd; ++$i) { |
There was a problem hiding this comment.
There is a shorthand for this;
$tokens->clearRange()https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/master/src/Tokenizer/Tokens.php#L1041
| */ | ||
| private function hasDump($content) | ||
| { | ||
| return false !== stripos($content, 'dump'); |
There was a problem hiding this comment.
this will make the fixing case sensitive (i.e. not remove Var_dump()),
removing this and using AbstractFunctionReferenceFixer will prevent this
|
@keradus @SpacePossum sorry for being out of this PR for some time, but now I have a little bit of it to finalize it. But first I would like to hear your opinion - do you think it is needed in core and is there a chance for it to be merged? If yes - then I will proceed with comments resolution and any suggestions/comments which may raise, otherwise let's just close it. Looking forward to hear your thoughts on it! |
|
IMO it is a very risky fixer going beyond the scope of the tool, i.e. fix code style and optimize code where possible. However the acceptable 'risky level' is of course opinionated. |
|
Yeh, this is very risky, because function calls affect the program state in PHP. Removing them could cause serious problems. |
|
Not to mention, that statements can be used as well as just expressions, so even if php was pure, there could still be assignments within the arguments, affecting the state. |
|
I'm 👎 on having this accepted as a cs fixer in here. |
|
I agree with you, that this too risky and is most likely out of scope of this awesome tool. |
|
As this issue is still high on the list when Googling for 'how to prohibit accidentally committing dump statements` - there is an awesome PHPStan plugin that allows you to define forbidden functions and constructs, and is out of the box default configured to do exactly what we want here: https://github.com/ekino/phpstan-banned-code composer require --dev ekino/phpstan-banned-code |
|
@curry684 you can even automate removing it with CommentedOutFunctionFixer and NoCommentedOutCodeFixer. |
Resolution for #2195