New rule proposal: incorrect use of parenthesescan can cause unexpected behavior in code

[palaueb]

Thanks for all this awesome tool, for me, is one of the best PHP tools ever done. Great job!

Feature Request

I have seen the PHP rule on https://cloud-ci.sgs.com/sonar/coding_rules?open=php%3AS6600&rule_key=php%3AS6600 that states this In PHP, incorrect use of parentheses can cause unexpected behavior in code. Therefore, it is important to avoid the unnecessary usage of parentheses for language constructs. Using constructs with parentheses can be misleading as it will produce a syntax that looks like a normal function call. However those constructs have lower precedence in the evaluation order than some others operators, this can lead to a behavior completely different from what the function syntax would hint. Also, some of the constructs have optional parameters, while modifying the code we may remove a parameter while keeping the parentheses, resulting in invalid code.

I thing this will help refactoring to better code.

Diff

-return(true);
+return true;

-echo("hello");
+echo "hello";

-break(2);
+break 2;

-case(3);
+case 3;

-continue(3);
+continue 3;

-include($file);
+include $file;

-include_once($file);
+include_once $file;

-require($file);
+require $file;

-require_once($file);
+require_once $file;

-print("hello Human");
+print "hello Human";

-throw(new Exception('Exception message'));
+throw new Exception('Exception message');

-yield($i);
+yield $i;

-yield from(funcall());
+yield from funcall();

[samsonasik]

That seems possible with:

1. verify parentheses exists first
2. re-print the node:

$node->setAttribute(AttributeKey::ORIGINAL_NODE, null);

The caveat, this should only apply on dedicated as Expression Stmt or direct Stmt->expr, not part of other Expr itself, it means it cannot cover inside of ternary or Binary Operations, as that may cause invalid print, see separate issue that can't be covered by this tweak:

• https://github.com/rectorphp/rector/issues/8552

This can be new rule on coding-style, but seems risky if it going to be part of sets config, so if implemented, user will need to enable it, eg:

withRules([RemoveUselessParenthesesInLanguageConstructRector::class])

[samsonasik]

@palaueb after some thinking, I think it is better to use coding standard tool for it, for example, if you use ecs enable PhpCsFixer\Fixer\ControlStructure\NoUnneededControlParenthesesFixer and PhpCsFixer\Fixer\LanguageConstruct\SingleSpaceAroundConstructFixer, you will get output:

-        echo('test');
+        echo 'test';

Applied checkers:

 * PhpCsFixer\Fixer\ControlStructure\NoUnneededControlParenthesesFixer
 * PhpCsFixer\Fixer\LanguageConstruct\SingleSpaceAroundConstructFixer

[palaueb]

Lovely! thanks my friends.