diff --git a/src/Analyser/Fiber/FiberScope.php b/src/Analyser/Fiber/FiberScope.php index 36332a0296e..d10a844a0c2 100644 --- a/src/Analyser/Fiber/FiberScope.php +++ b/src/Analyser/Fiber/FiberScope.php @@ -136,6 +136,7 @@ public function filterByTruthyValue(Expr $expr): self /** @var self $scope */ $scope = parent::filterByTruthyValue($expr); $scope->truthyValueExprs = $this->truthyValueExprs; + $scope->falseyValueExprs = $this->falseyValueExprs; $scope->truthyValueExprs[] = $expr; return $scope; @@ -144,7 +145,8 @@ public function filterByTruthyValue(Expr $expr): self public function filterByFalseyValue(Expr $expr): self { /** @var self $scope */ - $scope = parent::filterByTruthyValue($expr); + $scope = parent::filterByFalseyValue($expr); + $scope->truthyValueExprs = $this->truthyValueExprs; $scope->falseyValueExprs = $this->falseyValueExprs; $scope->falseyValueExprs[] = $expr; diff --git a/src/Rules/Comparison/FunctionCallConstantConditionRule.php b/src/Rules/Comparison/FunctionCallConstantConditionRule.php index 1754bd9cdc9..98e09d13873 100644 --- a/src/Rules/Comparison/FunctionCallConstantConditionRule.php +++ b/src/Rules/Comparison/FunctionCallConstantConditionRule.php @@ -37,19 +37,26 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { $reportedMarkers = []; - foreach ($node->get(ImpossibleCheckTypeReportedCollector::class) as $fileData) { + $reportedMarkersByFile = []; + foreach ($node->get(ImpossibleCheckTypeReportedCollector::class) as $filePath => $fileData) { foreach ($fileData as $data) { $reportedMarkers[$data[0]] = true; + $reportedMarkersByFile[$filePath . "\0" . $data[0]] = true; } } $errorsByRuleTraitExprValue = []; - foreach ($node->get(FunctionCallConstantConditionCollector::class) as $fileData) { + foreach ($node->get(FunctionCallConstantConditionCollector::class) as $filePath => $fileData) { foreach ($fileData as $data) { $ruleName = $data[0]; $traitName = $data[1]; $traitKey = $traitName ?? self::NULL_TRAIT_KEY; - $exprString = $data[2]; + // A non-trait call site is per-file: the same printed condition at + // the same line in two different files must neither merge its + // deferred errors nor be suppressed by the other file's marker. + // Trait call sites keep merging across the analysed contexts - + // trait names are unique project-wide. + $exprString = $traitName === null ? $filePath . "\0" . $data[2] : $data[2]; $value = $data[3]; $valueKey = var_export($value, true); if ($data[3] === null) { @@ -68,7 +75,10 @@ public function processNode(Node $node, Scope $scope): array foreach ($ruleData as $traitKey => $traitData) { $isTrait = $traitKey !== self::NULL_TRAIT_KEY; foreach ($traitData as $exprString => $valueData) { - if (array_key_exists($exprString, $reportedMarkers)) { + // non-trait keys carry their file, so only the same file's + // marker suppresses; trait entries match markers from any + // analysed context + if (array_key_exists($exprString, $isTrait ? $reportedMarkers : $reportedMarkersByFile)) { // the ImpossibleCheckType* rule owns this call site continue; } diff --git a/src/Rules/Comparison/MatchExpressionRule.php b/src/Rules/Comparison/MatchExpressionRule.php index 4757493d62f..519afcc1d34 100644 --- a/src/Rules/Comparison/MatchExpressionRule.php +++ b/src/Rules/Comparison/MatchExpressionRule.php @@ -70,7 +70,13 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE foreach ($armConditions as $armCondition) { $armConditionScope = $armCondition->getScope(); $rawCondition = $armCondition->getCondition(); - $isTypeCheckCandidate = $this->functionCallConstantConditionHelper->isTypeCheckCandidate($rawCondition); + // Only for a match(true)-style subject is the arm comparison the + // same fact as the call's own constant truthiness - the site the + // ImpossibleCheckType* rules own. For any other subject the + // comparison ("int is never true") is an independent finding and + // must not be deduplicated away against their markers. + $isTypeCheckCandidate = ($matchConditionType->isTrue()->yes() || $matchConditionType->isFalse()->yes()) + && $this->functionCallConstantConditionHelper->isTypeCheckCandidate($rawCondition); $armConditionExpr = new Node\Expr\BinaryOp\Identical( $matchCondition, $rawCondition, diff --git a/src/Rules/PhpDoc/VarTagTypeRuleHelper.php b/src/Rules/PhpDoc/VarTagTypeRuleHelper.php index 127501aa58c..652e393db32 100644 --- a/src/Rules/PhpDoc/VarTagTypeRuleHelper.php +++ b/src/Rules/PhpDoc/VarTagTypeRuleHelper.php @@ -8,7 +8,7 @@ use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\AutowiredService; -use PHPStan\Node\Expr\TypeExpr; +use PHPStan\Node\Expr\NativeTypeExpr; use PHPStan\PhpDoc\NameScopeAlreadyBeingCreatedException; use PHPStan\PhpDoc\Tag\VarTag; use PHPStan\PhpDoc\TypeNodeResolver; @@ -78,7 +78,13 @@ public function checkVarType(Scope $scope, Node\Expr $var, Node\Expr $expr, arra $dimExpr = $arrayItem->key; } - $itemErrors = $this->checkVarType($scope, $arrayItem->value, new TypeExpr($scope->getType($expr)->getOffsetValueType($scope->getType($dimExpr))), $varTags, $assignedVariables); + // carry both flavours so the native-type check reads the native + // offset type, not the phpdoc one (mirrors the foreach key/value + // sites in WrongVariableNameInVarTagRule) + $itemErrors = $this->checkVarType($scope, $arrayItem->value, new NativeTypeExpr( + $scope->getType($expr)->getOffsetValueType($scope->getType($dimExpr)), + $scope->getNativeType($expr)->getOffsetValueType($scope->getNativeType($dimExpr)), + ), $varTags, $assignedVariables); foreach ($itemErrors as $error) { $errors[] = $error; } diff --git a/tests/PHPStan/Analyser/FiberScopeFilterByValueRule.php b/tests/PHPStan/Analyser/FiberScopeFilterByValueRule.php new file mode 100644 index 00000000000..02269631ba4 --- /dev/null +++ b/tests/PHPStan/Analyser/FiberScopeFilterByValueRule.php @@ -0,0 +1,80 @@ + + */ +class FiberScopeFilterByValueRule implements Rule +{ + + public function getNodeType(): string + { + return FuncCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$node->name instanceof Node\Name) { + return []; + } + + $functionName = $node->name->getLast(); + $args = $node->getArgs(); + + if ($functionName === 'probeFilter') { + if (count($args) < 2) { + return []; + } + $var = $args[1]->value; + if (!$var instanceof Variable || !is_string($var->name)) { + return []; + } + + $truthyType = $scope->filterByTruthyValue($args[0]->value)->getVariableType($var->name); + $falseyType = $scope->filterByFalseyValue($args[0]->value)->getVariableType($var->name); + + return [ + RuleErrorBuilder::message(sprintf( + 'truthy: %s, falsey: %s', + $truthyType->describe(VerbosityLevel::precise()), + $falseyType->describe(VerbosityLevel::precise()), + ))->identifier('tests.fiberScopeFilter')->build(), + ]; + } + + if ($functionName === 'probeChainedFilter') { + if (count($args) < 3) { + return []; + } + + $chainedType = $scope->filterByTruthyValue($args[0]->value) + ->filterByFalseyValue($args[1]->value) + ->getType($args[2]->value); + + return [ + RuleErrorBuilder::message(sprintf( + 'chained: %s', + $chainedType->describe(VerbosityLevel::precise()), + ))->identifier('tests.fiberScopeFilter')->build(), + ]; + } + + return []; + } + +} diff --git a/tests/PHPStan/Analyser/FiberScopeFilterByValueRuleTest.php b/tests/PHPStan/Analyser/FiberScopeFilterByValueRuleTest.php new file mode 100644 index 00000000000..6d4243f5740 --- /dev/null +++ b/tests/PHPStan/Analyser/FiberScopeFilterByValueRuleTest.php @@ -0,0 +1,33 @@ + + */ +class FiberScopeFilterByValueRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new FiberScopeFilterByValueRule(); + } + + public function testFilterByValue(): void + { + $this->analyse([__DIR__ . '/data/fiber-scope-filter-by-value.php'], [ + [ + 'truthy: int, falsey: null', + 15, + ], + [ + 'chained: int|int<6, max>', + 20, + ], + ]); + } + +} diff --git a/tests/PHPStan/Analyser/data/fiber-scope-filter-by-value.php b/tests/PHPStan/Analyser/data/fiber-scope-filter-by-value.php new file mode 100644 index 00000000000..916954f40cd --- /dev/null +++ b/tests/PHPStan/Analyser/data/fiber-scope-filter-by-value.php @@ -0,0 +1,21 @@ +treatPhpDocTypesAsCertain = true; + $this->analyse([ + __DIR__ . '/data/call-condition-cross-file-a.php', + __DIR__ . '/data/call-condition-cross-file-b.php', + ], [ + [ + 'If condition is always true.', + 18, + 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', + ], + [ + 'If condition is always true.', + 18, + 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', + ], + ]); + } + + public function testMarkerFromAnotherFileDoesNotSuppress(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([ + __DIR__ . '/data/call-condition-cross-file-marker-a.php', + __DIR__ . '/data/call-condition-cross-file-marker-b.php', + ], [ + [ + 'Call to function is_int() with int will always evaluate to true.', + 7, + ], + [ + 'If condition is always true.', + 7, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php index 844d145d8a2..063e1d531f0 100644 --- a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php @@ -570,4 +570,23 @@ public function testInTrait(): void ]); } + public function testMatchArmComparisonNotSuppressedByImpossibleCheck(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/match-arm-type-check-call.php'], [ + [ + 'Call to function is_int() with int will always evaluate to true.', + 8, + ], + [ + 'Match arm comparison between int and true is always false.', + 8, + ], + [ + 'Call to function is_int() with int will always evaluate to true.', + 16, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/call-condition-cross-file-a.php b/tests/PHPStan/Rules/Comparison/data/call-condition-cross-file-a.php new file mode 100644 index 00000000000..11181173993 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/call-condition-cross-file-a.php @@ -0,0 +1,20 @@ +ok()) { + } +} diff --git a/tests/PHPStan/Rules/Comparison/data/call-condition-cross-file-b.php b/tests/PHPStan/Rules/Comparison/data/call-condition-cross-file-b.php new file mode 100644 index 00000000000..19a9fd6dff3 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/call-condition-cross-file-b.php @@ -0,0 +1,20 @@ +ok()) { + } +} diff --git a/tests/PHPStan/Rules/Comparison/data/call-condition-cross-file-marker-a.php b/tests/PHPStan/Rules/Comparison/data/call-condition-cross-file-marker-a.php new file mode 100644 index 00000000000..06afe50d615 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/call-condition-cross-file-marker-a.php @@ -0,0 +1,9 @@ += 8.0 + +namespace MatchArmTypeCheckCall; + +function doFoo(int $i, int $y): string +{ + return match ($i) { + is_int($y) => 'a', + default => 'b', + }; +} + +function doBar(int $y): string +{ + return match (true) { + is_int($y) => 'a', + default => 'b', + }; +} diff --git a/tests/PHPStan/Rules/PhpDoc/WrongVariableNameInVarTagRuleTest.php b/tests/PHPStan/Rules/PhpDoc/WrongVariableNameInVarTagRuleTest.php index 6b63dd6a790..e47215d1f23 100644 --- a/tests/PHPStan/Rules/PhpDoc/WrongVariableNameInVarTagRuleTest.php +++ b/tests/PHPStan/Rules/PhpDoc/WrongVariableNameInVarTagRuleTest.php @@ -627,4 +627,15 @@ public function testNewIsAlwaysFinalClass(): void $this->analyse([__DIR__ . '/data/new-is-always-final-var-tag-type.php'], []); } + public function testDestructuringChecksAgainstNativeOffsetType(): void + { + $this->checkTypeAgainstPhpDocType = true; + $this->analyse([__DIR__ . '/data/var-tag-destructuring-native.php'], [ + [ + 'PHPDoc tag @var with type string is not subtype of type int.', + 17, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/PhpDoc/data/var-tag-destructuring-native.php b/tests/PHPStan/Rules/PhpDoc/data/var-tag-destructuring-native.php new file mode 100644 index 00000000000..aceaf0103da --- /dev/null +++ b/tests/PHPStan/Rules/PhpDoc/data/var-tag-destructuring-native.php @@ -0,0 +1,19 @@ +