From 6959b7a4a7b7a393ca312b1ec84c9e48526691c0 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 24 Jun 2021 16:22:48 +0700 Subject: [PATCH 1/4] [DeadCode] Keep Assign expr on RemoveUnusedVariableAssignRector for impure function --- .../Fixture/on_ob_get_clean.php.inc | 49 +++++++++++++++++++ .../RemoveUnusedVariableAssignRector.php | 17 ++++++- .../SideEffect/PureFunctionDetector.php | 2 +- 3 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/on_ob_get_clean.php.inc diff --git a/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/on_ob_get_clean.php.inc b/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/on_ob_get_clean.php.inc new file mode 100644 index 00000000000..2adaca00e97 --- /dev/null +++ b/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/on_ob_get_clean.php.inc @@ -0,0 +1,49 @@ +run(); + + $result = ob_get_clean(); + + echo 'done'; + } +} + +?> +----- +run(); + + ob_get_clean(); + + echo 'done'; + } +} + +?> diff --git a/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php b/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php index cd21c4ab0ad..980d65728ce 100644 --- a/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php +++ b/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php @@ -22,6 +22,7 @@ use Rector\Core\PhpParser\Comparing\ConditionSearcher; use Rector\Core\Rector\AbstractRector; use Rector\DeadCode\NodeAnalyzer\UsedVariableNameAnalyzer; +use Rector\DeadCode\SideEffect\PureFunctionDetector; use Rector\NodeTypeResolver\Node\AttributeKey; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -35,7 +36,8 @@ public function __construct( private ReservedKeywordAnalyzer $reservedKeywordAnalyzer, private CompactFuncCallAnalyzer $compactFuncCallAnalyzer, private ConditionSearcher $conditionSearcher, - private UsedVariableNameAnalyzer $usedVariableNameAnalyzer + private UsedVariableNameAnalyzer $usedVariableNameAnalyzer, + private PureFunctionDetector $pureFunctionDetector ) { } @@ -102,6 +104,10 @@ public function refactor(Node $node): ?Node return null; } + if ($this->isImpureFunction($node->expr)) { + return $node->expr; + } + if ($node->expr instanceof MethodCall || $node->expr instanceof StaticCall) { // keep the expr, can have side effect return $node->expr; @@ -111,6 +117,15 @@ public function refactor(Node $node): ?Node return $node; } + private function isImpureFunction(Expr $expr): bool + { + if (! $expr instanceof FuncCall) { + return false; + } + + return ! $this->pureFunctionDetector->detect($expr); + } + private function shouldSkip(Assign $assign): bool { $classMethod = $assign->getAttribute(AttributeKey::METHOD_NODE); diff --git a/rules/DeadCode/SideEffect/PureFunctionDetector.php b/rules/DeadCode/SideEffect/PureFunctionDetector.php index 5ea0877a236..5b36e734cd1 100644 --- a/rules/DeadCode/SideEffect/PureFunctionDetector.php +++ b/rules/DeadCode/SideEffect/PureFunctionDetector.php @@ -30,7 +30,7 @@ final class PureFunctionDetector 'header', 'header_remove', 'http_response_code', 'setcookie', // output buffer - 'ob_start', 'ob_end_clean', 'readfile', 'printf', 'var_dump', 'phpinfo', + 'ob_start', 'ob_end_clean', 'ob_get_clean', 'readfile', 'printf', 'var_dump', 'phpinfo', 'ob_implicit_flush', 'vprintf', // mcrypt From 756f7508d8e5c871fba2b9de8553a398c399351e Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 24 Jun 2021 16:28:49 +0700 Subject: [PATCH 2/4] phpstan --- phpstan.neon | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/phpstan.neon b/phpstan.neon index 79294aa2005..85f277e987b 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -487,9 +487,17 @@ parameters: path: packages/StaticTypeMapper/PhpDocParser/UnionTypeMapper.php #17 - - message: '#Class with base "StaticTypeMapper" name is already used in "Rector\\PHPStanStaticTypeMapper\\TypeMapper\\StaticTypeMapper", "Rector\\StaticTypeMapper\\StaticTypeMapper"\. Use unique name to make classes easy to recognize#' + message: '#Class with base "UnionTypeMapper" name is already used in "Rector\\StaticTypeMapper\\PhpDocParser\\UnionTypeMapper", "Rector\\PHPStanStaticTypeMapper\\TypeMapper\\UnionTypeMapper"\. Use unique name to make classes easy to recognize#' + path: packages/PHPStanStaticTypeMapper/TypeMapper/UnionTypeMapper.php #17 + + - + message: '#Class with base "StaticTypeMapper" name is already used in "Rector\\StaticTypeMapper\\StaticTypeMapper", "Rector\\PHPStanStaticTypeMapper\\TypeMapper\\StaticTypeMapper"\. Use unique name to make classes easy to recognize#' path: packages/StaticTypeMapper/StaticTypeMapper.php #31 + - + message: '#Class with base "StaticTypeMapper" name is already used in "Rector\\StaticTypeMapper\\StaticTypeMapper", "Rector\\PHPStanStaticTypeMapper\\TypeMapper\\StaticTypeMapper"\. Use unique name to make classes easy to recognize#' + path: packages/PHPStanStaticTypeMapper/TypeMapper/StaticTypeMapper.php #21 + # false positive - '#Attribute class JetBrains\\PhpStorm\\Immutable does not exist#' From b45821d880af778e41bc5d7c7e9c2bb30ce746e1 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Thu, 24 Jun 2021 17:02:56 +0700 Subject: [PATCH 3/4] phpstan --- phpstan.neon | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 85f277e987b..79294aa2005 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -487,17 +487,9 @@ parameters: path: packages/StaticTypeMapper/PhpDocParser/UnionTypeMapper.php #17 - - message: '#Class with base "UnionTypeMapper" name is already used in "Rector\\StaticTypeMapper\\PhpDocParser\\UnionTypeMapper", "Rector\\PHPStanStaticTypeMapper\\TypeMapper\\UnionTypeMapper"\. Use unique name to make classes easy to recognize#' - path: packages/PHPStanStaticTypeMapper/TypeMapper/UnionTypeMapper.php #17 - - - - message: '#Class with base "StaticTypeMapper" name is already used in "Rector\\StaticTypeMapper\\StaticTypeMapper", "Rector\\PHPStanStaticTypeMapper\\TypeMapper\\StaticTypeMapper"\. Use unique name to make classes easy to recognize#' + message: '#Class with base "StaticTypeMapper" name is already used in "Rector\\PHPStanStaticTypeMapper\\TypeMapper\\StaticTypeMapper", "Rector\\StaticTypeMapper\\StaticTypeMapper"\. Use unique name to make classes easy to recognize#' path: packages/StaticTypeMapper/StaticTypeMapper.php #31 - - - message: '#Class with base "StaticTypeMapper" name is already used in "Rector\\StaticTypeMapper\\StaticTypeMapper", "Rector\\PHPStanStaticTypeMapper\\TypeMapper\\StaticTypeMapper"\. Use unique name to make classes easy to recognize#' - path: packages/PHPStanStaticTypeMapper/TypeMapper/StaticTypeMapper.php #21 - # false positive - '#Attribute class JetBrains\\PhpStorm\\Immutable does not exist#' From 6b45317f1be48ae00b638e11214196a454833cc6 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Fri, 25 Jun 2021 15:33:06 +0700 Subject: [PATCH 4/4] final touch: clean up --- .../Rector/Assign/RemoveUnusedVariableAssignRector.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php b/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php index 980d65728ce..2432c1c57db 100644 --- a/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php +++ b/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php @@ -104,11 +104,9 @@ public function refactor(Node $node): ?Node return null; } - if ($this->isImpureFunction($node->expr)) { - return $node->expr; - } - - if ($node->expr instanceof MethodCall || $node->expr instanceof StaticCall) { + if ($node->expr instanceof MethodCall || $node->expr instanceof StaticCall || $this->isImpureFunction( + $node->expr + )) { // keep the expr, can have side effect return $node->expr; }