diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index a799d1a789d..5ef6418c784 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -18,6 +18,24 @@ parameters: count: 1 path: src/Analyser/ArgumentsNormalizer.php + - + rawMessage: Foreach overwrites $expr with its value variable. + identifier: foreach.valueOverwrite + count: 1 + path: src/Analyser/ConditionalExpressionHolderRecipe.php + + - + rawMessage: Foreach overwrites $exprString with its value variable. + identifier: foreach.valueOverwrite + count: 1 + path: src/Analyser/ConditionalExpressionHolderRecipe.php + + - + rawMessage: Foreach overwrites $type with its value variable. + identifier: foreach.valueOverwrite + count: 1 + path: src/Analyser/ConditionalExpressionHolderRecipe.php + - rawMessage: Casting to string something that's already string. identifier: cast.useless @@ -42,6 +60,24 @@ parameters: count: 3 path: src/Analyser/ExprHandler/Helper/ConditionalExpressionHolderHelper.php + - + rawMessage: Foreach overwrites $expr with its value variable. + identifier: foreach.valueOverwrite + count: 1 + path: src/Analyser/ExprHandler/Helper/ConditionalExpressionHolderHelper.php + + - + rawMessage: Foreach overwrites $exprString with its key variable. + identifier: foreach.keyOverwrite + count: 1 + path: src/Analyser/ExprHandler/Helper/ConditionalExpressionHolderHelper.php + + - + rawMessage: Foreach overwrites $type with its value variable. + identifier: foreach.valueOverwrite + count: 1 + path: src/Analyser/ExprHandler/Helper/ConditionalExpressionHolderHelper.php + - rawMessage: 'Doing instanceof PHPStan\Type\Constant\ConstantStringType is error-prone and deprecated. Use Type::getConstantStrings() instead.' identifier: phpstanApi.instanceofType @@ -54,6 +90,12 @@ parameters: count: 1 path: src/Analyser/ExprHandler/Helper/IncDecTypeHelper.php + - + rawMessage: Foreach overwrites $varName with its value variable. + identifier: foreach.valueOverwrite + count: 1 + path: src/Analyser/ExprHandler/MatchHandler.php + - rawMessage: Casting to string something that's already string. identifier: cast.useless @@ -96,6 +138,12 @@ parameters: count: 1 path: src/Analyser/RuleErrorTransformer.php + - + rawMessage: Foreach overwrites $exprString with its value variable. + identifier: foreach.valueOverwrite + count: 2 + path: src/Analyser/ScopeOps.php + - rawMessage: 'Parameter #2 $node of method PHPStan\BetterReflection\SourceLocator\Ast\Strategy\NodeToReflection::__invoke() expects PhpParser\Node\Expr\ArrowFunction|PhpParser\Node\Expr\Closure|PhpParser\Node\Expr\FuncCall|PhpParser\Node\Stmt\Class_|PhpParser\Node\Stmt\Const_|PhpParser\Node\Stmt\Enum_|PhpParser\Node\Stmt\Function_|PhpParser\Node\Stmt\Interface_|PhpParser\Node\Stmt\Trait_, PhpParser\Node\Stmt\ClassLike given.' identifier: argument.type @@ -126,6 +174,12 @@ parameters: count: 1 path: src/Collectors/Registry.php + - + rawMessage: Foreach overwrites $internalError with its value variable. + identifier: foreach.valueOverwrite + count: 1 + path: src/Command/AnalyseCommand.php + - rawMessage: 'Call to static method expand() of internal class Nette\DI\Helpers from outside its root namespace Nette.' identifier: staticMethod.internalClass @@ -348,6 +402,12 @@ parameters: count: 1 path: src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php + - + rawMessage: Foreach overwrites $file with its value variable. + identifier: foreach.valueOverwrite + count: 1 + path: src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php + - rawMessage: 'Parameter #2 $node of method PHPStan\BetterReflection\SourceLocator\Ast\Strategy\NodeToReflection::__invoke() expects PhpParser\Node\Expr\ArrowFunction|PhpParser\Node\Expr\Closure|PhpParser\Node\Expr\FuncCall|PhpParser\Node\Stmt\Class_|PhpParser\Node\Stmt\Const_|PhpParser\Node\Stmt\Enum_|PhpParser\Node\Stmt\Function_|PhpParser\Node\Stmt\Interface_|PhpParser\Node\Stmt\Trait_, PhpParser\Node\Expr\FuncCall|PhpParser\Node\Stmt\ClassLike|PhpParser\Node\Stmt\Const_|PhpParser\Node\Stmt\Function_ given.' identifier: argument.type @@ -480,6 +540,12 @@ parameters: count: 2 path: src/Rules/Classes/ImpossibleInstanceOfRule.php + - + rawMessage: Foreach overwrites $param with its value variable. + identifier: foreach.valueOverwrite + count: 1 + path: src/Rules/Classes/InvalidPromotedPropertiesRule.php + - rawMessage: 'Doing instanceof PHPStan\Type\ObjectType is error-prone and deprecated. Use Type::isObject() or Type::getObjectClassNames() instead.' identifier: phpstanApi.instanceofType @@ -858,6 +924,12 @@ parameters: count: 1 path: src/Type/ArrayType.php + - + rawMessage: Foreach overwrites $keyType with its value variable. + identifier: foreach.valueOverwrite + count: 1 + path: src/Type/ArrayType.php + - rawMessage: 'Doing instanceof PHPStan\Type\BooleanType is error-prone and deprecated. Use Type::isBoolean() instead.' identifier: phpstanApi.instanceofType @@ -918,6 +990,12 @@ parameters: count: 2 path: src/Type/Constant/ConstantArrayType.php + - + rawMessage: Foreach overwrites $position with its value variable. + identifier: foreach.valueOverwrite + count: 1 + path: src/Type/Constant/ConstantArrayType.php + - rawMessage: 'Doing instanceof PHPStan\Type\Constant\ConstantStringType is error-prone and deprecated. Use Type::getConstantStrings() instead.' identifier: phpstanApi.instanceofType @@ -1488,6 +1566,12 @@ parameters: count: 1 path: src/Type/Php/ArrayKeyExistsFunctionTypeSpecifyingExtension.php + - + rawMessage: Foreach overwrites $arg with its value variable. + identifier: foreach.valueOverwrite + count: 1 + path: src/Type/Php/ArrayMapFunctionReturnTypeExtension.php + - rawMessage: 'Doing instanceof PHPStan\Type\ConstantScalarType is error-prone and deprecated. Use Type::isConstantScalarValue() or Type::getConstantScalarTypes() or Type::getConstantScalarValues() instead.' identifier: phpstanApi.instanceofType @@ -1560,6 +1644,18 @@ parameters: count: 1 path: src/Type/Php/ReflectionMethodConstructorThrowTypeExtension.php + - + rawMessage: Foreach overwrites $group with its value variable. + identifier: foreach.valueOverwrite + count: 1 + path: src/Type/Regex/RegexGroupList.php + + - + rawMessage: Foreach overwrites $i with its key variable. + identifier: foreach.keyOverwrite + count: 1 + path: src/Type/Regex/RegexGroupList.php + - rawMessage: 'Doing instanceof PHPStan\Type\ObjectType is error-prone and deprecated. Use Type::isObject() or Type::getObjectClassNames() instead.' identifier: phpstanApi.instanceofType @@ -1638,6 +1734,30 @@ parameters: count: 2 path: src/Type/TypeCombinator.php + - + rawMessage: For loop initial assignment overwrites variable $i. + identifier: for.variableOverwrite + count: 2 + path: src/Type/TypeCombinator.php + + - + rawMessage: Foreach overwrites $arr with its value variable. + identifier: foreach.valueOverwrite + count: 1 + path: src/Type/TypeCombinator.php + + - + rawMessage: Foreach overwrites $idx with its key variable. + identifier: foreach.keyOverwrite + count: 1 + path: src/Type/TypeCombinator.php + + - + rawMessage: Foreach overwrites $idx with its value variable. + identifier: foreach.valueOverwrite + count: 1 + path: src/Type/TypeCombinator.php + - rawMessage: Doing instanceof PHPStan\Type\IntersectionType is error-prone and deprecated. identifier: phpstanApi.instanceofType diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 7a00057e9c4..92b2486d2d3 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -2828,10 +2828,6 @@ public function processArgs( } if ($assignByReference) { - if ($currentParameter === null) { - throw new ShouldNotHappenException(); - } - $argValue = $arg->value; if (!$argValue instanceof Variable || $argValue->name !== 'this') { $paramOutType = $this->getParameterOutExtensionsType($callLike, $calleeReflection, $currentParameter, $scope); diff --git a/src/Analyser/ScopeOps.php b/src/Analyser/ScopeOps.php index 44e6496fdc3..a450dd15baa 100644 --- a/src/Analyser/ScopeOps.php +++ b/src/Analyser/ScopeOps.php @@ -21,7 +21,9 @@ use PHPStan\TrinaryLogic; use PHPStan\Turbo\ShadowedByTurboExtension; use PHPStan\Type\ErrorType; +use PHPStan\Type\NeverType; use PHPStan\Type\Type; +use PHPStan\Type\TypeCombinator; use function array_filter; use function array_key_exists; use function array_key_first; @@ -376,10 +378,10 @@ public static function createConditionalExpressions( $newVariableTypes = $ourExpressionTypes; // When our-branch type is a subtype of their-branch type, the union - // absorbs it (merged === their). Such a variable is a poor *guard* — - // asserting its our-branch type later wouldn't reliably select this - // branch — but it remains a valid conditional *target*, so only exclude - // it from guard selection instead of dropping it entirely. + // absorbs it (merged === their). Such a variable cannot be a *guard* — + // its branch set difference below comes out empty — but it remains a + // valid conditional *target*; the flag also lets the target loop skip + // pairing these absorbed targets with constant-array guards. $guardsToExclude = []; foreach (array_keys($differingKeys) as $exprString) { if (!array_key_exists($exprString, $theirExpressionTypes)) { @@ -424,18 +426,41 @@ public static function createConditionalExpressions( continue; } - if ( - array_key_exists($exprString, $theirExpressionTypes) - && !$theirExpressionTypes[$exprString]->getCertainty()->yes() - ) { + if (!array_key_exists($exprString, $theirExpressionTypes)) { + // with no their-branch entry the merged holder keeps our type with + // lowered certainty, so no later type assertion can tell the + // branches apart + continue; + } + $theirHolder = $theirExpressionTypes[$exprString]; + if (!$theirHolder->getCertainty()->yes()) { + continue; + } + if ($holder->equalTypes($theirHolder)) { continue; } - if ($mergedExpressionTypes[$exprString]->equalTypes($holder)) { + // The set difference between the branch types is the part of our type + // the other branch cannot produce: observing it later proves this + // branch was taken, even when the full branch types overlap - so a + // representable remainder makes a sound guard where the full type + // would not (the full our-branch type may even equal the merged + // type). When the subtraction is not representable, remove() keeps + // our full type and the merged-type comparison below restores the + // long-standing behavior for such guards. + $remainder = TypeCombinator::remove($holder->getType(), $theirHolder->getType()); + if ($remainder instanceof NeverType) { + continue; + } + if ($mergedExpressionTypes[$exprString]->getType()->equals($remainder)) { + // matching this guard later would not discriminate the branches - + // the merged scope already guarantees it continue; } - $typeGuards[$exprString] = $holder; + $typeGuards[$exprString] = $remainder === $holder->getType() + ? $holder + : ExpressionTypeHolder::createYes($holder->getExpr(), $remainder); } if (count($typeGuards) === 0) { diff --git a/src/Analyser/StmtHandler/DoWhileHandler.php b/src/Analyser/StmtHandler/DoWhileHandler.php index eda9d52dc32..7b8d0ce89df 100644 --- a/src/Analyser/StmtHandler/DoWhileHandler.php +++ b/src/Analyser/StmtHandler/DoWhileHandler.php @@ -107,7 +107,7 @@ public function processStmt( $storage = $originalStorage; if ( $replayBodyRecording !== null && $replayPassStorage !== null && $replayPassResult !== null - && $prevEntryScope !== null && $bodyScope->equals($prevEntryScope) + && $bodyScope->equals($prevEntryScope) ) { // the final body walk would repeat the recorded fixpoint pass exactly // (same entry scope, deterministic walk) - adopt the pass's results diff --git a/src/Analyser/StmtHandler/ForeachHandler.php b/src/Analyser/StmtHandler/ForeachHandler.php index d80e4dccf1a..6dba6940903 100644 --- a/src/Analyser/StmtHandler/ForeachHandler.php +++ b/src/Analyser/StmtHandler/ForeachHandler.php @@ -263,7 +263,7 @@ static function () use ($condResult, $emptyArrayType): Type { if ( $replayBodyRecording !== null && $replayPassStorage !== null && $replayPassResult !== null && $replayEntryScope !== null - && $unrolledTotalKeys === null && $finalEntryScope->equals($replayEntryScope) + && $finalEntryScope->equals($replayEntryScope) ) { // the final walk would repeat the recorded fixpoint pass exactly // (same entry scope, deterministic walk) - adopt the pass's results diff --git a/src/Analyser/StmtHandler/SwitchHandler.php b/src/Analyser/StmtHandler/SwitchHandler.php index 8375c8c3de8..93e83c1a181 100644 --- a/src/Analyser/StmtHandler/SwitchHandler.php +++ b/src/Analyser/StmtHandler/SwitchHandler.php @@ -160,7 +160,7 @@ public function processStmt( $alwaysTerminating = false; } - if ($prevScope !== null && isset($branchFinalScopeResult)) { + if ($prevScope !== null) { $finalScope = $prevScope->mergeWith($finalScope); $alwaysTerminating = $alwaysTerminating && $branchFinalScopeResult->isAlwaysTerminating(); } diff --git a/src/Analyser/StmtHandler/WhileHandler.php b/src/Analyser/StmtHandler/WhileHandler.php index bf10d728520..7c76155e164 100644 --- a/src/Analyser/StmtHandler/WhileHandler.php +++ b/src/Analyser/StmtHandler/WhileHandler.php @@ -132,7 +132,7 @@ public function processStmt( $replayCondRecording !== null && $replayBodyRecording !== null && $replayPassStorage !== null && $replayPassResult !== null && $replayCondResult !== null - && $prevEntryScope !== null && $bodyScope->equals($prevEntryScope) + && $bodyScope->equals($prevEntryScope) ) { // the final walk would repeat the recorded fixpoint pass exactly // (same entry scope, deterministic walk) - adopt the pass's results diff --git a/src/Parser/RichParser.php b/src/Parser/RichParser.php index 75adc400405..89a4e91f871 100644 --- a/src/Parser/RichParser.php +++ b/src/Parser/RichParser.php @@ -360,7 +360,7 @@ private function parseIdentifiers(string $text, int $ignorePos): array } if ($openParenthesisCount > 0) { - throw new IgnoreParseException('Unexpected end, unclosed opening parenthesis', $tokenLine ?? 1); + throw new IgnoreParseException('Unexpected end, unclosed opening parenthesis', $tokenLine); } if (count($identifiers) === 0) { diff --git a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php index 4548fe93a1d..de50750ed5d 100644 --- a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php +++ b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php @@ -195,7 +195,7 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier): return null; } - [$reflectionCacheKey, $variableCacheKey] = $this->getCacheKeys($file, $identifier); // @phpstan-ignore variable.undefined + [$reflectionCacheKey, $variableCacheKey] = $this->getCacheKeys($file, $identifier); $functionReflection = $this->nodeToReflection($reflector, $fetchedFunctionNode); $this->cache->save($reflectionCacheKey, $variableCacheKey, $functionReflection->exportToCache()); diff --git a/src/Turbo/TurboExtensionEnabler.php b/src/Turbo/TurboExtensionEnabler.php index bb6b27fdd61..aa188e340c2 100644 --- a/src/Turbo/TurboExtensionEnabler.php +++ b/src/Turbo/TurboExtensionEnabler.php @@ -22,7 +22,7 @@ final class TurboExtensionEnabler * version is the short SHA of the last commit touching turbo-ext/src/, * enforced by the phar.yml turbo-version job. */ - public const EXPECTED_EXTENSION_VERSION = 'd7ef536'; + public const EXPECTED_EXTENSION_VERSION = 'a1df814'; private static bool $typeCombinatorCacheEnabled = false; diff --git a/src/Type/NeverType.php b/src/Type/NeverType.php index 37f5dfc4b61..fc1e8a0caf7 100644 --- a/src/Type/NeverType.php +++ b/src/Type/NeverType.php @@ -14,6 +14,7 @@ use PHPStan\Reflection\Type\UnresolvedPropertyPrototypeReflection; use PHPStan\ShouldNotHappenException; use PHPStan\TrinaryLogic; +use PHPStan\Turbo\ReferencedByTurboExtension; use PHPStan\Type\Enum\EnumCaseObjectType; use PHPStan\Type\Generic\TemplateType; use PHPStan\Type\Traits\NonGeneralizableTypeTrait; @@ -23,6 +24,7 @@ use PHPStan\Type\Traits\UndecidedComparisonCompoundTypeTrait; /** @api */ +#[ReferencedByTurboExtension(key: 'neverType')] class NeverType implements CompoundType { diff --git a/src/Type/Php/SubstrDynamicReturnTypeExtension.php b/src/Type/Php/SubstrDynamicReturnTypeExtension.php index 0c4f8587ab2..0a4ddba5867 100644 --- a/src/Type/Php/SubstrDynamicReturnTypeExtension.php +++ b/src/Type/Php/SubstrDynamicReturnTypeExtension.php @@ -75,32 +75,25 @@ public function getTypeFromFunctionCall( ) { $results = []; foreach ($constantStrings as $constantString) { - if ($length !== null) { - if ($functionReflection->getName() === 'mb_substr') { - $substr = mb_substr($constantString->getValue(), $offset->getValue(), $length->getValue()); - } elseif ($this->phpVersion->substrReturnFalseInsteadOfEmptyString()) { - $substr = $this->substrOrFalse($constantString->getValue(), $offset->getValue(), $length->getValue()); - } else { - $substr = substr($constantString->getValue(), $offset->getValue(), $length->getValue()); - } - } else { - if ($functionReflection->getName() === 'mb_substr') { - $substr = mb_substr($constantString->getValue(), $offset->getValue()); - } elseif ($this->phpVersion->substrReturnFalseInsteadOfEmptyString()) { - // Simulate substr call on an older PHP version if the runtime one is too new. - $substr = $this->substrOrFalse($constantString->getValue(), $offset->getValue()); - } else { - $substr = substr($constantString->getValue(), $offset->getValue()); - } + if ($functionReflection->getName() === 'mb_substr') { + $substr = $length !== null + ? mb_substr($constantString->getValue(), $offset->getValue(), $length->getValue()) + : mb_substr($constantString->getValue(), $offset->getValue()); + $results[] = new ConstantStringType($substr); + continue; } - if (is_bool($substr)) { - if ($this->phpVersion->substrReturnFalseInsteadOfEmptyString()) { - $results[] = new ConstantBooleanType($substr); - } else { - // Simulate substr call on a recent PHP version if the runtime one is too old. - $results[] = new ConstantStringType(''); - } + // substrOrFalse() detects an out-of-range offset with its own length + // check, so the result does not depend on the runtime PHP version's + // substr() semantics. false is then mapped to the analysed version's + // result: false on PHP < 8, an empty string on PHP >= 8. + $substr = $length !== null + ? $this->substrOrFalse($constantString->getValue(), $offset->getValue(), $length->getValue()) + : $this->substrOrFalse($constantString->getValue(), $offset->getValue()); + if ($substr === false) { + $results[] = $this->phpVersion->substrReturnFalseInsteadOfEmptyString() + ? new ConstantBooleanType(false) + : new ConstantStringType(''); } else { $results[] = new ConstantStringType($substr); } diff --git a/tests/PHPStan/Analyser/nsrt/bug-13833.php b/tests/PHPStan/Analyser/nsrt/bug-13833.php new file mode 100644 index 00000000000..ab2a27c72dd --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-13833.php @@ -0,0 +1,24 @@ +', $_SESSION); + assertType('mixed~null', $b); + if (!isset($_SESSION['a'])) { + echo 'this is absolutely possible'; + } +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-5051.php b/tests/PHPStan/Analyser/nsrt/bug-5051.php index a91a87716bd..3e1c4f0b2f8 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-5051.php +++ b/tests/PHPStan/Analyser/nsrt/bug-5051.php @@ -92,7 +92,7 @@ public function testWithBooleans($data): void assertType('false', $foo); } else { assertType('bool', $update); - assertType('bool', $foo); + assertType('true', $foo); } } diff --git a/tests/PHPStan/Analyser/nsrt/bug-7706.php b/tests/PHPStan/Analyser/nsrt/bug-7706.php new file mode 100644 index 00000000000..a051831393d --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-7706.php @@ -0,0 +1,25 @@ +format('Y'); + } + + return 0; + } + + protected function test(string $filter): int + { + $all = false; + if ($filter === 'all') { + $date = new \DateTime(); + $all = true; + + if (mt_rand() === 0) { // all other code expect this condition is the same as in self::working() + $all = false; + } + } + + if ($all) { + assertVariableCertainty(TrinaryLogic::createYes(), $date); + return (int) $date->format('Y'); + } + + return 0; + } +} diff --git a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php index f49bfbe3c1c..348824b2623 100644 --- a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php @@ -172,6 +172,10 @@ public function testTypesAssignedToPropertiesExpressionNames(): void 'Property PropertiesFromArrayIntoObject\Foo::$lall (int) does not accept string.', 69, ], + [ + 'Property PropertiesFromArrayIntoObject\Foo::$foo (string) does not accept float.', + 83, + ], [ 'Property PropertiesFromArrayIntoObject\Foo::$foo (string) does not accept float|int|string.', 97, diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index 397dabed083..3f79e8c5dd1 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1723,4 +1723,40 @@ public function testBug2032(): void ]); } + public function testBug13833(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->polluteScopeWithLoopInitialAssignments = false; + $this->checkMaybeUndefinedVariables = true; + $this->polluteScopeWithAlwaysIterableForeach = true; + $this->analyse([__DIR__ . '/data/bug-13833.php'], []); + } + + public function testBug9685(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->polluteScopeWithLoopInitialAssignments = false; + $this->checkMaybeUndefinedVariables = true; + $this->polluteScopeWithAlwaysIterableForeach = true; + $this->analyse([__DIR__ . '/data/bug-9685.php'], []); + } + + public function testBug7706(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->polluteScopeWithLoopInitialAssignments = false; + $this->checkMaybeUndefinedVariables = true; + $this->polluteScopeWithAlwaysIterableForeach = true; + $this->analyse([__DIR__ . '/data/bug-7706.php'], []); + } + + public function testBug8360(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->polluteScopeWithLoopInitialAssignments = false; + $this->checkMaybeUndefinedVariables = true; + $this->polluteScopeWithAlwaysIterableForeach = true; + $this->analyse([__DIR__ . '/data/bug-8360.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index 61a9e57ed0f..836549ee6b4 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -671,4 +671,11 @@ public function testBug14416(): void $this->analyse([__DIR__ . '/data/bug-14416.php'], []); } + public function testBug14421(): void + { + $this->treatPhpDocTypesAsCertain = true; + + $this->analyse([__DIR__ . '/data/bug-14421.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Variables/data/bug-13833.php b/tests/PHPStan/Rules/Variables/data/bug-13833.php new file mode 100644 index 00000000000..199dd53d22c --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-13833.php @@ -0,0 +1,16 @@ +format('Y'); + } + + return 0; + } + + protected function test(string $filter): int + { + $all = false; + if ($filter === 'all') { + $date = new \DateTime(); + $all = true; + + if (mt_rand() === 0) { // all other code expect this condition is the same as in self::working() + $all = false; + } + } + + if ($all) { + return (int) $date->format('Y'); + } + + return 0; + } +} diff --git a/turbo-ext/src/ScopeOps.cpp b/turbo-ext/src/ScopeOps.cpp index a7f19c342ae..f607a739910 100644 --- a/turbo-ext/src/ScopeOps.cpp +++ b/turbo-ext/src/ScopeOps.cpp @@ -471,17 +471,22 @@ class ScopeOps static zv::Val createConditionalExpressions(zv::TableRef conditional, zv::TableRef ours, zv::TableRef theirs, zv::TableRef merged, zv::TableRef differingKeys) { zend_class_entry *virtualNodeCe = pt_class(PT_CLASS_VIRTUAL_NODE); - if (UNEXPECTED(virtualNodeCe == NULL)) { + zend_class_entry *neverTypeCe = pt_class(PT_CLASS_NEVER_TYPE); + if (UNEXPECTED(virtualNodeCe == NULL || neverTypeCe == NULL)) { return zv::Val(); } zv::ScratchTable guardsToExclude(8); zv::ScratchTable typeGuards(8); - - /* guardsToExclude: subtype-absorbed their-branch variables are poor - * guards but stay valid conditional targets. Only the merge's differing - * keys can qualify — iterate those (in their insertion order, like the - * twin) instead of the whole holder maps. */ + /* owns the remainder-typed holders created below; the scratch table + * only borrows them */ + zv::Arr createdGuardHolders; + + /* guardsToExclude: subtype-absorbed their-branch variables cannot be + * guards (their branch set difference is empty) but stay valid + * conditional targets. Only the merge's differing keys can qualify — + * iterate those (in their insertion order, like the twin) instead of + * the whole holder maps. */ for (auto diffEntry : differingKeys) { zend_string *key = diffEntry.stringKeyOrNull(); zend_ulong idx = diffEntry.indexKey(); @@ -561,33 +566,68 @@ class ScopeOps continue; } zval *theirSlot = pt_ht_find(theirs.table(), key, idx); - if (theirSlot != NULL) { - zv::Ref theirHolder = zv::Ref(theirSlot).deref(); - if (UNEXPECTED(!pt_check_holder(theirHolder.raw()))) { - return zv::Val(); - } - if (pt_holder_certainty_value(theirHolder.asObject()) != PT_TRI_YES) { - continue; - } + if (theirSlot == NULL) { + /* with no their-branch entry the merged holder keeps our type + * with lowered certainty, so no later type assertion can tell + * the branches apart */ + continue; + } + zv::Ref theirHolder = zv::Ref(theirSlot).deref(); + if (UNEXPECTED(!pt_check_holder(theirHolder.raw()))) { + return zv::Val(); + } + if (pt_holder_certainty_value(theirHolder.asObject()) != PT_TRI_YES) { + continue; } bool equalTypes; + if (UNEXPECTED(!pt_holder_equal_types(holder.raw(), theirHolder.raw(), &equalTypes))) { + return zv::Val(); + } + if (equalTypes) { + continue; + } + + /* the branch set difference — see the twin for why an unchanged + * remainder falls back to the merged-type comparison */ + zv::Val remainder = typeCombinatorRemove(holderType(holder), holderType(theirHolder)); + if (UNEXPECTED(remainder.isUndef())) { + return zv::Val(); + } + if (remainder.ref().instanceOf(neverTypeCe)) { + continue; + } { zv::Ref mergedHolder = zv::Ref(mergedSlot).deref(); if (UNEXPECTED(!pt_check_holder(mergedHolder.raw()))) { return zv::Val(); } - if (UNEXPECTED(!pt_holder_equal_types(mergedHolder.raw(), holder.raw(), &equalTypes))) { + bool mergedEqualsRemainder = pt_types_identical_or_equal(holderType(mergedHolder), remainder.raw()); + if (UNEXPECTED(EG(exception))) { return zv::Val(); } - } - if (equalTypes) { - continue; + if (mergedEqualsRemainder) { + continue; + } } - /* borrowed entry — the scratch table has no destructor */ - zval borrowed; - ZVAL_COPY_VALUE(&borrowed, holder.raw()); - pt_ht_update(typeGuards.table(), key, idx, &borrowed); + if (Z_OBJ_P(remainder.raw()) == Z_OBJ_P(holderType(holder))) { + /* borrowed entry — the scratch table has no destructor */ + zval borrowed; + ZVAL_COPY_VALUE(&borrowed, holder.raw()); + pt_ht_update(typeGuards.table(), key, idx, &borrowed); + } else { + /* ExpressionTypeHolder::createYes($holder->expr, $remainder) — + * owned by createdGuardHolders, borrowed by the scratch table */ + zval created; + pt_holder_create(&created, zv::ObjRef(holder.asObject()).propAt(PT_ETH_PROP_EXPR).raw(), remainder.raw(), PT_TRI_YES); + zval borrowed; + ZVAL_COPY_VALUE(&borrowed, &created); + if (createdGuardHolders.isUndef()) { + createdGuardHolders = zv::Arr::create(4); + } + createdGuardHolders.push(zv::Val::adopt(created)); + pt_ht_update(typeGuards.table(), key, idx, &borrowed); + } } if (typeGuards.size() == 0) { @@ -1475,6 +1515,21 @@ class ScopeOps return !EG(exception); } + /* TypeCombinator::remove($fromType, $typeToRemove) */ + static zv::Val typeCombinatorRemove(zval *fromType, zval *typeToRemove) + { + zval retval; + if (UNEXPECTED(!pt_type_combinator_binary("remove", sizeof("remove") - 1, fromType, typeToRemove, &retval))) { + return zv::Val(); + } + zv::Val result = zv::Val::adopt(retval); + if (UNEXPECTED(!result.ref().isObject())) { + zend_throw_error(NULL, "phpstan_turbo: TypeCombinator::remove did not return an object"); + return zv::Val(); + } + return result; + } + /* $type->isSuperTypeOf($otherType)->result->value */ static bool isSuperTypeOfValue(zval *type, zval *otherType, zend_long *out) { diff --git a/turbo-ext/src/support.cpp b/turbo-ext/src/support.cpp index 6f2ff142996..8c39555ca31 100644 --- a/turbo-ext/src/support.cpp +++ b/turbo-ext/src/support.cpp @@ -49,6 +49,7 @@ static const pt_class_template pt_class_templates[PT_CLASS_COUNT] = { /* PT_CLASS_ARROW_FUNCTION */ {"arrowFunction", "PhpParser\\Node\\Expr\\ArrowFunction"}, /* PT_CLASS_TYPE */ {"type", "PHPStan\\Type\\Type"}, /* PT_CLASS_RECURSION_GUARD */ {"recursionGuard", "PHPStan\\Type\\RecursionGuard"}, + /* PT_CLASS_NEVER_TYPE */ {"neverType", "PHPStan\\Type\\NeverType"}, /* PT_CLASS_TRINARY */ {"trinaryLogic", NULL}, /* PT_CLASS_ETH */ {"expressionTypeHolder", NULL}, /* PT_CLASS_CEH */ {"conditionalExpressionHolder", NULL}, diff --git a/turbo-ext/src/support.h b/turbo-ext/src/support.h index 89b2bef8ace..9d9984fa25a 100644 --- a/turbo-ext/src/support.h +++ b/turbo-ext/src/support.h @@ -81,6 +81,7 @@ enum { PT_CLASS_ARROW_FUNCTION, PT_CLASS_TYPE, PT_CLASS_RECURSION_GUARD, + PT_CLASS_NEVER_TYPE, /* classes the extension instantiates (their PHP twins are themselves * shadowed, hence no default name): configured to the stub subclasses * so created objects satisfy the original PHPStan type hints */