diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 5a9ec882334..6b7a30856f3 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -47,8 +47,10 @@ use PHPStan\Node\IssetExpr; use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Node\PropertyAssignNode; +use PHPStan\Node\UsedAsStringNode; use PHPStan\Node\VariableAssignNode; use PHPStan\Node\VirtualNode; +use PHPStan\Parser\ExprUsedAsStringVisitor; use PHPStan\Php\PhpVersion; use PHPStan\ShouldNotHappenException; use PHPStan\TrinaryLogic; @@ -162,6 +164,10 @@ static function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $contex ); $scope = $result->getScope(); + if ($expr instanceof Assign) { + $this->emitUsedAsStringNode($nodeScopeResolver, $scope, $storage, $expr, $nodeCallback); + } + if ( $expr instanceof AssignRef && $expr->var instanceof Variable @@ -209,6 +215,111 @@ static function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $contex ); } + /** + * Emits a {@see UsedAsStringNode} for the assigned value when it lands in a + * string slot: a string-valued variable assignment or an assignment to a + * native property whose type allows a string. + * + * @param callable(Node $node, Scope $scope): void $nodeCallback + */ + private function emitUsedAsStringNode( + NodeScopeResolver $nodeScopeResolver, + MutatingScope $scope, + ExpressionResultStorage $storage, + Assign $expr, + callable $nodeCallback, + ): void + { + if (ExprUsedAsStringVisitor::isAlreadyUsedAsStringSite($expr->expr)) { + return; + } + + if (!$this->isAssignToStringSlot($scope, $expr->var, $expr->expr)) { + return; + } + + $nodeScopeResolver->callNodeCallback($nodeCallback, new UsedAsStringNode($expr->expr), $scope, $storage); + } + + /** + * Whether the assigned value lands in a string slot: a string-valued variable + * assignment, or an assignment to a native property whose declared type allows a + * string (a plain `string` or a union that contains `string`, e.g. `string|int`) + * when the assigned value can actually be coerced to a string in the current + * typing mode. A union without any `string` member (e.g. `int|float`) is not a + * string slot, and a value that cannot be coerced to a string (e.g. a + * non-`Stringable` object) does not land in the slot as a string. + */ + private function isAssignToStringSlot(MutatingScope $scope, Expr $var, Expr $assignedExpr): bool + { + if ($var instanceof Variable) { + return $scope->getType($assignedExpr)->isString()->yes(); + } + + $slotType = $this->getAssignTargetPropertyNativeType($scope, $var); + if ($slotType === null || !$this->containsString($slotType)) { + return false; + } + + // Only fire when the assigned value can actually be coerced to a string in + // the current (strict/weak) typing mode: a non-`Stringable` object assigned + // to a `string|int` property is not used as a string, while a `Stringable` + // assigned to it in weak mode is. + $coercedAssignedType = $scope->getType($assignedExpr)->toCoercedArgumentType($scope->isDeclareStrictTypes()); + + return $this->containsString($coercedAssignedType); + } + + /** + * Whether the type is a plain `string` or a union with at least one `string` + * member (e.g. `string|int`). + */ + private function containsString(Type $type): bool + { + foreach (TypeUtils::flattenTypes($type) as $innerType) { + if ($innerType->isString()->yes()) { + return true; + } + } + + return false; + } + + private function getAssignTargetPropertyNativeType(MutatingScope $scope, Expr $var): ?Type + { + if ($var instanceof PropertyFetch) { + if (!$var->name instanceof Node\Identifier) { + return null; + } + $propertyName = $var->name->toString(); + $holderType = $scope->getType($var->var); + if (!$holderType->hasInstanceProperty($propertyName)->yes()) { + return null; + } + $property = $holderType->getInstanceProperty($propertyName, $scope); + + return $property->hasNativeType() ? $property->getNativeType() : null; + } + + if ($var instanceof StaticPropertyFetch) { + if (!$var->name instanceof Node\VarLikeIdentifier) { + return null; + } + $propertyName = $var->name->toString(); + $holderType = $var->class instanceof Name + ? $scope->resolveTypeByName($var->class) + : $scope->getType($var->class); + $property = $scope->getStaticPropertyReflection($holderType, $propertyName); + if ($property === null || !$property->hasNativeType()) { + return null; + } + + return $property->getNativeType(); + } + + return null; + } + /** * @param callable(Node $node, Scope $scope): void $nodeCallback * @param Closure(MutatingScope $scope): ExpressionResult $processExprCallback diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 41fdf890721..a5d92326fb1 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -107,10 +107,12 @@ use PHPStan\Node\ReturnStatement; use PHPStan\Node\StaticMethodCallableNode; use PHPStan\Node\UnreachableStatementNode; +use PHPStan\Node\UsedAsStringNode; use PHPStan\Node\VariableAssignNode; use PHPStan\Node\VarTagChangedExpressionTypeNode; use PHPStan\Parser\ArrowFunctionArgVisitor; use PHPStan\Parser\ClosureArgVisitor; +use PHPStan\Parser\ExprUsedAsStringVisitor; use PHPStan\Parser\GotoLabelVisitor; use PHPStan\Parser\ImmediatelyInvokedClosureVisitor; use PHPStan\Parser\LineAttributesVisitor; @@ -1242,6 +1244,14 @@ public function processStmtNode( $this->callNodeCallback($nodeCallback, $prop, $scope, $storage); if ($prop->default !== null) { $this->processExprNode($stmt, $prop->default, $scope, $storage, $nodeCallback, ExpressionContext::createDeep()); + + if ( + $nativePropertyType !== null + && $nativePropertyType->isString()->yes() + && !ExprUsedAsStringVisitor::isAlreadyUsedAsStringSite($prop->default) + ) { + $this->callNodeCallback($nodeCallback, new UsedAsStringNode($prop->default), $scope, $storage); + } } if (!$scope->isInClass()) { @@ -2471,6 +2481,7 @@ public function leaveNode(Node $node): ?ExistingArrayDimFetch $impurePoints = [ new ImpurePoint($scope, $stmt, 'betweenPhpTags', 'output between PHP opening and closing tags', true), ]; + $this->callNodeCallback($nodeCallback, new UsedAsStringNode(new Node\Scalar\String_($stmt->value, $stmt->getAttributes())), $scope, $storage); } elseif ($stmt instanceof Node\Stmt\Block) { $result = $this->processStmtNodesInternal($stmt, $stmt->stmts, $scope, $storage, $nodeCallback, $context); if ($this->polluteScopeWithBlock) { @@ -2754,6 +2765,11 @@ public function processExprNode( $this->callNodeCallbackWithExpression($nodeCallback, $expr, $scope, $storage, $context); + if ($expr->getAttribute(ExprUsedAsStringVisitor::ATTRIBUTE_NAME) === true) { + $usedAsStringScope = $context->isDeep() ? $scope->exitFirstLevelStatements() : $scope; + $this->callNodeCallback($nodeCallback, new UsedAsStringNode($expr), $usedAsStringScope, $storage); + } + /** @var ExprHandler $exprHandler */ foreach ($this->container->getServicesByTag(ExprHandler::EXTENSION_TAG) as $exprHandler) { if (!$exprHandler->supports($expr)) { @@ -3269,6 +3285,35 @@ private function processParamNode( } $this->processExprNode($stmt, $param->default, $scope, $storage, $nodeCallback, ExpressionContext::createDeep()); + + $nativeParameterType = $param->type !== null + ? ParserNodeTypeToPHPStanType::resolve($param->type, $scope->isInClass() ? $scope->getClassReflection() : null) + : null; + if ( + $nativeParameterType === null + || !$nativeParameterType->isString()->yes() + || ExprUsedAsStringVisitor::isAlreadyUsedAsStringSite($param->default) + ) { + return; + } + + $this->callNodeCallback($nodeCallback, new UsedAsStringNode($param->default), $scope, $storage); + } + + /** + * Whether the native type is a string slot: a plain `string` or a union that + * contains a `string` member (e.g. `string|int`). Plain `mixed` is not a string + * slot, so arguments to untyped or PHPDoc-only `@param string` parameters do not fire. + */ + private function isStringSlotType(Type $type): bool + { + foreach (TypeUtils::flattenTypes($type) as $slotMemberType) { + if ($slotMemberType->isString()->yes()) { + return true; + } + } + + return false; } /** @@ -3708,6 +3753,17 @@ public function processArgs( $scopeToPass = $scopeToPass->enterExpressionAssign($arg->value); } $exprResult = $this->processExprNode($stmt, $arg->value, $scopeToPass, $storage, $nodeCallback, $context->enterDeep()); + // Closures and arrow functions expose their parameters as NativeParameterReflection + // (not ExtendedParameterReflection), so $parameterNativeType is null for them - their + // declared type already is the native type. + $argStringSlotType = $parameterNativeType ?? ($parameter instanceof NativeParameterReflection ? $parameter->getType() : null); + if ( + $argStringSlotType !== null + && $this->isStringSlotType($argStringSlotType) + && !ExprUsedAsStringVisitor::isAlreadyUsedAsStringSite($arg->value) + ) { + $this->callNodeCallback($nodeCallback, new UsedAsStringNode($arg->value), $scopeToPass, $storage); + } $throwPoints = array_merge($throwPoints, $exprResult->getThrowPoints()); $impurePoints = array_merge($impurePoints, $exprResult->getImpurePoints()); $isAlwaysTerminating = $isAlwaysTerminating || $exprResult->isAlwaysTerminating(); diff --git a/src/Node/UsedAsStringNode.php b/src/Node/UsedAsStringNode.php new file mode 100644 index 00000000000..87a38a852e7 --- /dev/null +++ b/src/Node/UsedAsStringNode.php @@ -0,0 +1,66 @@ +{$s}`, `$$s`, etc.), + * - the value assigned to a variable when that value is a string, + * - the value assigned to a native property whose type allows a string (a plain + * `string` or a union containing `string`, e.g. `string|int`) when the assigned + * value can be coerced to a string in the current typing mode (so a + * non-`Stringable` object assigned to such a slot does not fire, while a + * `Stringable` assigned to it in weak mode does), and the default of a native + * `string`-typed property, + * - an argument passed to a native parameter whose type allows a string (a plain + * `string` or a union containing `string`, e.g. `string|int`), including + * closures and arrow functions, and the default of a native `string`-typed + * parameter. + * + * Concatenations and interpolated strings are reported once for the whole + * expression instead of once per nested operand, so a rule can interpret the + * built string as a single unit. When the assigned value already produces its + * own node (a concatenation, interpolation or `(string)` cast), the enclosing + * assignment does not report it a second time. + * + * @api + */ +final class UsedAsStringNode extends NodeAbstract implements VirtualNode +{ + + public function __construct(private Expr $expr) + { + parent::__construct($expr->getAttributes()); + } + + public function getExpr(): Expr + { + return $this->expr; + } + + #[Override] + public function getType(): string + { + return 'PHPStan_Node_UsedAsStringNode'; + } + + /** + * @return string[] + */ + #[Override] + public function getSubNodeNames(): array + { + return []; + } + +} diff --git a/src/Parser/ExprUsedAsStringVisitor.php b/src/Parser/ExprUsedAsStringVisitor.php new file mode 100644 index 00000000000..1d68223d25f --- /dev/null +++ b/src/Parser/ExprUsedAsStringVisitor.php @@ -0,0 +1,108 @@ +exprs as $expr) { + $expr->setAttribute(self::ATTRIBUTE_NAME, true); + } + } elseif ($node instanceof Print_) { + $node->expr->setAttribute(self::ATTRIBUTE_NAME, true); + } elseif ($node instanceof Cast\String_) { + $node->expr->setAttribute(self::ATTRIBUTE_NAME, true); + } elseif ($node instanceof AssignOp\Concat) { + $node->setAttribute(self::ATTRIBUTE_NAME, true); + $node->expr->setAttribute(self::CLAIMED_ATTRIBUTE_NAME, true); + } elseif ($node instanceof Concat) { + if ($node->getAttribute(self::CLAIMED_ATTRIBUTE_NAME) !== true) { + $node->setAttribute(self::ATTRIBUTE_NAME, true); + } + $node->left->setAttribute(self::CLAIMED_ATTRIBUTE_NAME, true); + $node->right->setAttribute(self::CLAIMED_ATTRIBUTE_NAME, true); + } elseif ($node instanceof InterpolatedString) { + if ($node->getAttribute(self::CLAIMED_ATTRIBUTE_NAME) !== true) { + $node->setAttribute(self::ATTRIBUTE_NAME, true); + } + foreach ($node->parts as $part) { + if (!$part instanceof Expr) { + continue; + } + $part->setAttribute(self::CLAIMED_ATTRIBUTE_NAME, true); + } + } elseif ( + $node instanceof PropertyFetch + || $node instanceof NullsafePropertyFetch + || $node instanceof MethodCall + || $node instanceof NullsafeMethodCall + || $node instanceof StaticPropertyFetch + || $node instanceof StaticCall + || $node instanceof ClassConstFetch + ) { + if ($node->name instanceof Expr) { + $node->name->setAttribute(self::ATTRIBUTE_NAME, true); + } + } elseif ($node instanceof Variable) { + if ($node->name instanceof Expr) { + $node->name->setAttribute(self::ATTRIBUTE_NAME, true); + } + } + + return null; + } + + /** + * Whether the expression already produces its own UsedAsStringNode, so that an + * enclosing context (e.g. a string-typed assignment target) must not report it a + * second time. True for marked nodes (concatenation, interpolation, echo/print + * argument, …), the operand-emitting `(string)` cast and nested assignments. + */ + public static function isAlreadyUsedAsStringSite(Expr $expr): bool + { + return $expr->getAttribute(self::ATTRIBUTE_NAME) === true + || $expr instanceof Cast\String_ + || $expr instanceof Assign + || $expr instanceof AssignRef; + } + +} diff --git a/tests/PHPStan/Node/UsedAsStringRule.php b/tests/PHPStan/Node/UsedAsStringRule.php new file mode 100644 index 00000000000..f28db94231c --- /dev/null +++ b/tests/PHPStan/Node/UsedAsStringRule.php @@ -0,0 +1,44 @@ + + */ +class UsedAsStringRule implements Rule +{ + + private Standard $printer; + + public function __construct() + { + $this->printer = new Standard(); + } + + public function getNodeType(): string + { + return UsedAsStringNode::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $expr = $node->getExpr(); + + return [ + RuleErrorBuilder::message(sprintf( + 'Used as string: %s (%s)', + $this->printer->prettyPrintExpr($expr), + $scope->getType($expr)->describe(VerbosityLevel::precise()), + ))->identifier('tests.usedAsString')->build(), + ]; + } + +} diff --git a/tests/PHPStan/Node/UsedAsStringRuleTest.php b/tests/PHPStan/Node/UsedAsStringRuleTest.php new file mode 100644 index 00000000000..9bcbfcbebf9 --- /dev/null +++ b/tests/PHPStan/Node/UsedAsStringRuleTest.php @@ -0,0 +1,207 @@ + + */ +class UsedAsStringRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new UsedAsStringRule(); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/used-as-string.php'], [ + [ + "Used as string: 'plain' ('plain')", + 9, + ], + [ + 'Used as string: \'\' (non-falsy-string)', + 10, + ], + [ + 'Used as string: "" (non-falsy-string)', + 11, + ], + [ + "Used as string: 'printed' ('printed')", + 12, + ], + [ + "Used as string: 'a' . \$s . 'b' (non-falsy-string)", + 13, + ], + [ + 'Used as string: $s (string)', + 14, + ], + [ + 'Used as string: $s .= "appended" (non-falsy-string)', + 15, + ], + [ + 'Used as string: $s .= \' src="\' . $s . \'"\' (non-falsy-string)', + 16, + ], + [ + "Used as string: \$s . 'plain' (non-falsy-string)", + 17, + ], + [ + 'Used as string: "interp {$s} end" (non-falsy-string)', + 18, + ], + [ + "Used as string: '\n' (\"\\n\")", + 20, + ], + [ + "Used as string: '123' ('123')", + 26, + ], + [ + "Used as string: '' ('')", + 27, + ], + [ + "Used as string: \$html .= <<\nEOS (''; + echo ""; + print 'printed'; + print 'a' . $s . 'b'; + $x = (string) $s; + $s .= "appended"; + $s .= ' src="' . $s . '"'; + $t = $s . 'plain'; + $u = "interp $s end"; + ?> + + +EOS; +} + +class Holder +{ + + public string $prop = ''; + + public int $num = 1; + + public static string $staticProp = ''; + + public string|int $unionWithString; + + public int|float $unionWithoutString = 1; + + public function method(): void + { + } + + public function methodWithStringParam(string $arg): void + { + } + + public static function staticMethodWithStringParam(string $arg): void + { + } + + public const CONST_NAME = 1; + + public function assignProperties(): void + { + $this->prop = 'assigned to string property'; + $this->num = 5; + self::$staticProp = 'assigned to static string property'; + $this->unionWithString = 'assigned to union string property'; + $this->unionWithoutString = 5; + } + +} + +function dynamicNames(Holder $h, string $name): void +{ + echo $h->{$name}; + $h->{$name}(); + $$name = 1; + $x = Holder::${$name}; + Holder::{$name}(); + $y = $h::{$name}; +} + +function takesString(string $s): void +{ +} + +function takesInt(int $i): void +{ +} + +function passArguments(Holder $h, string $s): void +{ + takesString($s); + takesString('passed as string argument'); + takesInt(5); + takesInt($h->num); +} + +function passMethodArguments(Holder $h): void +{ + $h->methodWithStringParam('method string argument'); + Holder::staticMethodWithStringParam('static string argument'); +} + +function withDefaults(string $s = 'string default', int $i = 5, ?string $n = null): void +{ +} + +function closuresAndArrowFunctions(string $s): void +{ + $closure = function (string $cs = 'closure string default', int $ci = 5): void { + }; + $closure($s); + $closure('closure string argument'); + + $arrow = fn (string $as = 'arrow string default', int $ai = 5): string => $as; + $arrow($s); + $arrow('arrow string argument'); +} + +function doNowdoc(): void +{ + $html = ''; + $html .= <<<'EOS' +