From a9b37868cf41d31fe784968c814636c2cdf8f673 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 19 Jun 2026 16:27:38 +0200 Subject: [PATCH 1/2] [php 8.0] Skip merged property in PromotedPropertyCandidateResolver --- .../Fixture/skip_narrowing_var_doc.php.inc | 24 +++++++++ .../Source/SomeCacheAdapterInterface.php | 10 ++++ .../Source/SomeCacheProvider.php | 18 +++++++ ...ertyAssignToConstructorPromotionRector.php | 50 +++++++++++++++++++ 4 files changed, 102 insertions(+) create mode 100644 rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_narrowing_var_doc.php.inc create mode 100644 rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Source/SomeCacheAdapterInterface.php create mode 100644 rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Source/SomeCacheProvider.php diff --git a/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_narrowing_var_doc.php.inc b/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_narrowing_var_doc.php.inc new file mode 100644 index 00000000000..eb3e8d12149 --- /dev/null +++ b/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_narrowing_var_doc.php.inc @@ -0,0 +1,24 @@ +cacheProvider = $cacheProvider; + } + + public function run(): object + { + return $this->cacheProvider->getCacheAdapter(); + } +} diff --git a/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Source/SomeCacheAdapterInterface.php b/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Source/SomeCacheAdapterInterface.php new file mode 100644 index 00000000000..77add8857bb --- /dev/null +++ b/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Source/SomeCacheAdapterInterface.php @@ -0,0 +1,10 @@ +shouldSkipNarrowingVarDoc($property, $param, $constructorPhpDocInfo, $paramName)) { + continue; + } + $hasChanged = true; // remove property from class @@ -388,6 +395,49 @@ private function shouldSkipPropertyOrParam(Property $property, Param $param): bo && ! $this->nodeComparator->areNodesEqual($property->type, $param->type); } + /** + * A class-typed property may carry a narrowing @var (e.g. native AdapterInterface, @var CacheProvider) used to + * type calls on the property. Promotion would drop that @var (it is not preserved as a @param here), $property so skip to + * avoid losing type information. + */ + private function shouldSkipNarrowingVarDoc( + Property $property, + Param $param, + PhpDocInfo $constructorPhpDocInfo, + string $paramName + ): bool { + if (! $property->type instanceof Node || ! $param->type instanceof Node) { + return false; + } + + // an explicit @param already carries the type onto the promoted property + if ($constructorPhpDocInfo->getParamTagValueByName($paramName) instanceof ParamTagValueNode) { + return false; + } + + $propertyPhpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($property); + $varTagValueNode = $propertyPhpDocInfo->getVarTagValueNode(); + if (! $varTagValueNode instanceof VarTagValueNode) { + return false; + } + + // a description keeps the docblock around anyway + if ($varTagValueNode->description !== '') { + return false; + } + + $varType = $this->staticTypeMapper->mapPHPStanPhpDocTypeToPHPStanType($varTagValueNode, $property); + + // only class-typed @var narrowing is lost; generics, arrays and int ranges are preserved on promotion + if (! $varType instanceof TypeWithClassName) { + return false; + } + + $paramType = $this->staticTypeMapper->mapPhpParserNodePHPStanType($param->type); + + return ! $this->typeComparator->areTypesEqual($varType, $paramType); + } + private function shouldRemoveNullFromForPromotedParamType(Property $property, Param $param): bool { if (! $property->type instanceof Node || ! $param->type instanceof Node) { From fa06820177d6d48d6d0a69beef91d53f5a44c0c1 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 19 Jun 2026 17:04:22 +0200 Subject: [PATCH 2/2] [dead-code] Fix /RemoveAlwaysTrueIfConditionRector with dynamic variable --- .../null_reset_of_plain_object.php.inc | 29 ---------------- .../Fixture/null_reset_of_user_object.php.inc | 33 +++++++++++++++++++ ...skip_null_reset_of_internal_object.php.inc | 14 ++++++++ .../skip_dynamic_variable_assign.php.inc | 23 +++++++++++++ .../RemoveUnusedVariableAssignRector.php | 21 ++++++++---- .../If_/RemoveAlwaysTrueIfConditionRector.php | 18 +++++++++- 6 files changed, 102 insertions(+), 36 deletions(-) delete mode 100644 rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/null_reset_of_plain_object.php.inc create mode 100644 rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/null_reset_of_user_object.php.inc create mode 100644 rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/skip_null_reset_of_internal_object.php.inc create mode 100644 rules-tests/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector/Fixture/skip_dynamic_variable_assign.php.inc diff --git a/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/null_reset_of_plain_object.php.inc b/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/null_reset_of_plain_object.php.inc deleted file mode 100644 index c7ce8dfabaf..00000000000 --- a/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/null_reset_of_plain_object.php.inc +++ /dev/null @@ -1,29 +0,0 @@ -name; - - $value = null; - } -} -?> ------ -name; - } -} -?> diff --git a/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/null_reset_of_user_object.php.inc b/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/null_reset_of_user_object.php.inc new file mode 100644 index 00000000000..4bc03859d7e --- /dev/null +++ b/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/null_reset_of_user_object.php.inc @@ -0,0 +1,33 @@ + +----- + diff --git a/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/skip_null_reset_of_internal_object.php.inc b/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/skip_null_reset_of_internal_object.php.inc new file mode 100644 index 00000000000..d11dbc4ee28 --- /dev/null +++ b/rules-tests/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector/Fixture/skip_null_reset_of_internal_object.php.inc @@ -0,0 +1,14 @@ +file($path); + + $finfo = null; + } +} diff --git a/rules-tests/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector/Fixture/skip_dynamic_variable_assign.php.inc b/rules-tests/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector/Fixture/skip_dynamic_variable_assign.php.inc new file mode 100644 index 00000000000..41a1dcd22ee --- /dev/null +++ b/rules-tests/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector/Fixture/skip_dynamic_variable_assign.php.inc @@ -0,0 +1,23 @@ +isNullResetOfObject($assign)) { + if ($this->isNullResetOfInternalObject($assign)) { continue; } @@ -142,16 +142,25 @@ public function refactor(Node $node): null|ClassMethod|Function_ return null; } - private function isNullResetOfObject(Assign $assign): bool + private function isNullResetOfInternalObject(Assign $assign): bool { - // resetting a native filesystem object to null releases the held file handle, - // e.g. $file = null on a SplFileObject/SplFileInfo/RecursiveDirectoryIterator + // resetting an internal PHP object to null releases the held resource/file handle, + // e.g. $file = null on a SplFileObject/RecursiveDirectoryIterator/finfo if (! $this->valueResolver->isNull($assign->expr)) { return false; } - return (new ObjectType('SplFileInfo'))->isSuperTypeOf($this->getType($assign->var)) - ->yes(); + $varType = $this->getType($assign->var); + if (! $varType instanceof ObjectType) { + return false; + } + + $classReflection = $varType->getClassReflection(); + if (! $classReflection instanceof ClassReflection) { + return false; + } + + return $classReflection->isBuiltin(); } private function isObjectWithDestructMethod(Expr $expr): bool diff --git a/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php b/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php index b17a1b2678e..37dc380a6e7 100644 --- a/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php +++ b/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php @@ -150,11 +150,16 @@ private function shouldSkipFromVariable(Expr $expr): bool /** @var Variable[] $variables */ $variables = $this->betterNodeFinder->findInstancesOf($expr, [Variable::class]); + $hasPlainVariable = false; foreach ($variables as $variable) { if ($this->exprAnalyzer->isNonTypedFromParam($variable)) { return true; } + if (! $variable->name instanceof Expr) { + $hasPlainVariable = true; + } + $type = $this->nodeTypeResolver->getNativeType($variable); if ($type instanceof IntersectionType) { foreach ($type->getTypes() as $subType) { @@ -165,7 +170,18 @@ private function shouldSkipFromVariable(Expr $expr): bool } } - return false; + // a dynamic variable assignment, e.g. ${$name} = ..., is invisible to native type inference, so the + // condition variable may be overwritten at runtime and is not safe to evaluate as always true + return $hasPlainVariable && $this->hasDynamicVariable(); + } + + private function hasDynamicVariable(): bool + { + return (bool) $this->betterNodeFinder->findFirst( + $this->getFile() + ->getNewStmts(), + static fn (Node $node): bool => $node instanceof Variable && $node->name instanceof Expr + ); } private function shouldSkipExpr(Expr $expr): bool