From 87963a42569cb05dc6f464a18d69e95875ad824e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 14 Oct 2023 07:53:07 +0200 Subject: [PATCH 1/5] RemoveUnusedNonEmptyArrayBeforeForeachRector: skip array dim fetch --- .../Fixture/skip_property_dim_fetch.php.inc | 24 +++++++++++++++++++ ...UnusedNonEmptyArrayBeforeForeachRector.php | 4 ++++ 2 files changed, 28 insertions(+) create mode 100644 rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_property_dim_fetch.php.inc diff --git a/rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_property_dim_fetch.php.inc b/rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_property_dim_fetch.php.inc new file mode 100644 index 00000000000..72c5a88e688 --- /dev/null +++ b/rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_property_dim_fetch.php.inc @@ -0,0 +1,24 @@ +> + */ + private static $groups = []; + + public static function getFootnotes($group = self::DEFAULT_GROUP): array + { + if (! empty(self::$groups[$group])) { + foreach (self::$groups[$group] as $note) { + echo "hello"; + } + } + + return []; + } +} diff --git a/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php b/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php index 67c8aa94b18..22d2a414e4a 100644 --- a/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php +++ b/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php @@ -106,6 +106,10 @@ private function isUselessBeforeForeachCheck(If_ $if, Scope $scope): bool $foreach = $if->stmts[0]; $foreachExpr = $foreach->expr; + if ($foreachExpr instanceof Expr\ArrayDimFetch) { + return false; + } + if ($foreachExpr instanceof Variable) { $variableName = $this->nodeNameResolver->getName($foreachExpr); if (is_string($variableName) && $this->reservedKeywordAnalyzer->isNativeVariable($variableName)) { From 0edaf2da087f9ea3c01ad95f89d01e01ea702e48 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 14 Oct 2023 12:38:27 +0200 Subject: [PATCH 2/5] fix with known offset --- .../Fixture/offset_exists_dim_fetch.php.inc | 59 +++++++++++++++++++ ...UnusedNonEmptyArrayBeforeForeachRector.php | 8 ++- 2 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/offset_exists_dim_fetch.php.inc diff --git a/rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/offset_exists_dim_fetch.php.inc b/rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/offset_exists_dim_fetch.php.inc new file mode 100644 index 00000000000..0dd67d1648d --- /dev/null +++ b/rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/offset_exists_dim_fetch.php.inc @@ -0,0 +1,59 @@ +> + */ + private static $groups = []; + + public static function knownOffset(): array + { + $group = 'default'; + + self::$groups[$group] = ["foo"]; + + if (! empty(self::$groups[$group])) { + foreach (self::$groups[$group] as $group) { + echo "hello"; + } + } + + return []; + } +} + +?> +----- +> + */ + private static $groups = []; + + public static function knownOffset(): array + { + $group = 'default'; + + self::$groups[$group] = ["foo"]; + + foreach (self::$groups[$group] as $group) { + echo "hello"; + } + + return []; + } +} + +?> diff --git a/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php b/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php index 22d2a414e4a..221530dd587 100644 --- a/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php +++ b/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php @@ -106,8 +106,12 @@ private function isUselessBeforeForeachCheck(If_ $if, Scope $scope): bool $foreach = $if->stmts[0]; $foreachExpr = $foreach->expr; - if ($foreachExpr instanceof Expr\ArrayDimFetch) { - return false; + if ($foreachExpr instanceof Expr\ArrayDimFetch && $foreachExpr->dim !== null) { + $exprType = $this->nodeTypeResolver->getNativeType($foreachExpr->var); + $dimType = $this->nodeTypeResolver->getNativeType($foreachExpr->dim); + if (!$exprType->hasOffsetValueType($dimType)->yes()) { + return false; + } } if ($foreachExpr instanceof Variable) { From 8ce4f23234e9b58f0ae38ce06784e9bfdbb8acd4 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 14 Oct 2023 12:40:22 +0200 Subject: [PATCH 3/5] test phpdoc array shape --- .../skip_phpdoc_offset_dim_fetch.php.inc | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_phpdoc_offset_dim_fetch.php.inc diff --git a/rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_phpdoc_offset_dim_fetch.php.inc b/rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_phpdoc_offset_dim_fetch.php.inc new file mode 100644 index 00000000000..672da6a6121 --- /dev/null +++ b/rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_phpdoc_offset_dim_fetch.php.inc @@ -0,0 +1,32 @@ +> + */ + private static $groups = []; + + /** + * @param array{default: foo} $array + * @return array + */ + public static function knownOffset($array): array + { + $group = 'default'; + + if (! empty($array[$group])) { + foreach ($array[$group] as $groupValue) { + echo "hello"; + } + } + + return []; + } +} + +?> From af0783576e96c56be108384933b4999193a4958d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 14 Oct 2023 15:15:27 +0200 Subject: [PATCH 4/5] refactor --- ...UnusedNonEmptyArrayBeforeForeachRector.php | 45 +++++++++++-------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php b/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php index 262b272397c..bcdd6c41bed 100644 --- a/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php +++ b/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php @@ -106,24 +106,8 @@ private function isUselessBeforeForeachCheck(If_ $if, Scope $scope): bool $foreach = $if->stmts[0]; $foreachExpr = $foreach->expr; - if ($foreachExpr instanceof Expr\ArrayDimFetch && $foreachExpr->dim !== null) { - $exprType = $this->nodeTypeResolver->getNativeType($foreachExpr->var); - $dimType = $this->nodeTypeResolver->getNativeType($foreachExpr->dim); - if (!$exprType->hasOffsetValueType($dimType)->yes()) { - return false; - } - } - - if ($foreachExpr instanceof Variable) { - $variableName = $this->nodeNameResolver->getName($foreachExpr); - if (is_string($variableName) && $this->reservedKeywordAnalyzer->isNativeVariable($variableName)) { - return false; - } - - $ifType = $scope->getNativeType($foreachExpr); - if (!$ifType->isArray()->yes()) { - return false; - } + if ($this->shouldSkipForeachExpr($foreachExpr, $scope)) { + return false; } $ifCond = $if->cond; @@ -216,4 +200,29 @@ private function refactorIf(If_ $if, Scope $scope): ?Foreach_ return $stmt; } + + private function shouldSkipForeachExpr(Expr $foreachExpr, Scope $scope): bool + { + if ($foreachExpr instanceof Expr\ArrayDimFetch && $foreachExpr->dim !== null) { + $exprType = $this->nodeTypeResolver->getNativeType($foreachExpr->var); + $dimType = $this->nodeTypeResolver->getNativeType($foreachExpr->dim); + if (!$exprType->hasOffsetValueType($dimType)->yes()) { + return true; + } + } + + if ($foreachExpr instanceof Variable) { + $variableName = $this->nodeNameResolver->getName($foreachExpr); + if (is_string($variableName) && $this->reservedKeywordAnalyzer->isNativeVariable($variableName)) { + return true; + } + + $ifType = $scope->getNativeType($foreachExpr); + if (!$ifType->isArray()->yes()) { + return true; + } + } + + return false; + } } From 79af99bb52e37b4e1171c35960c2e6b61a79b8fc Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 14 Oct 2023 15:17:59 +0200 Subject: [PATCH 5/5] simplify test --- .../Fixture/skip_phpdoc_offset_dim_fetch.php.inc | 8 -------- 1 file changed, 8 deletions(-) diff --git a/rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_phpdoc_offset_dim_fetch.php.inc b/rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_phpdoc_offset_dim_fetch.php.inc index 672da6a6121..e3d1921c90a 100644 --- a/rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_phpdoc_offset_dim_fetch.php.inc +++ b/rules-tests/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector/Fixture/skip_phpdoc_offset_dim_fetch.php.inc @@ -4,16 +4,8 @@ namespace Rector\Tests\DeadCode\Rector\If_\RemoveUnusedNonEmptyArrayBeforeForeac class SkipPhpdocOffset { - public const DEFAULT_GROUP = 'default'; - - /** - * @var array> - */ - private static $groups = []; - /** * @param array{default: foo} $array - * @return array */ public static function knownOffset($array): array {