Add ExprUsedAsStringNode virtual node emitted for expressions used as a string#5787
Open
phpstan-bot wants to merge 2 commits into
Open
Add ExprUsedAsStringNode virtual node emitted for expressions used as a string#5787phpstan-bot wants to merge 2 commits into
ExprUsedAsStringNode virtual node emitted for expressions used as a string#5787phpstan-bot wants to merge 2 commits into
Conversation
…as a string
- Add `PHPStan\Node\ExprUsedAsStringNode` virtual node wrapping the `Expr` whose value is used as a string.
- Add `ExprUsedAsStringVisitor` (rich parser visitor) that marks expressions used in a string context via the `isExprUsedAsString` attribute: echo/print arguments, `(string)` cast operands, string concatenation (`.` and `.=`), string interpolation/heredoc, and dynamic-name expressions (`$foo->{$s}`, `$foo->{$s}()`, `Foo::${$s}`, `Foo::{$s}()`, `Foo::{$s}`, `$$s`).
- Concatenation chains and interpolated strings are reported once for the whole expression instead of once per nested operand, by "claiming" nested concats/interpolation parts, so a rule can interpret the built string as a single unit.
- `NodeScopeResolver::processExprNode()` emits the node for any marked expression; inline HTML emits the node wrapping a synthetic `String_`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This implements the feature request for a virtual node that lets rules hook onto every place where an expression's value is used as a string, instead of hooking on
Node::classand doinginstanceofcasing forString_,Concat,InterpolatedString,AssignOp\Concat,InlineHTML, etc. (the use-case being e.g. detecting<script>tags without anonce=attribute in built/echoed HTML).A new
ExprUsedAsStringNodeis emitted for each expression whose value is used as a string. Crucially, a concatenation chain or interpolated string is reported once for the whole expression, solving the reporter's main pain point that hooking onBinaryOp\Concatfires for every concat in a long chain.Changes
src/Node/ExprUsedAsStringNode.php— new@apivirtual node wrapping the usedExpr(getExpr()).src/Parser/ExprUsedAsStringVisitor.php— new rich-parser visitor (auto-tagged viaNodeVisitor→richParserNodeVisitor) that marks expressions used as a string with theisExprUsedAsStringattribute and "claims" nested concats / interpolation parts so the whole concatenation is reported once.src/Analyser/NodeScopeResolver.php— emitsExprUsedAsStringNodeinprocessExprNode()for any marked expression, and forInlineHTMLstatements (wrapping a syntheticScalar\String_).tests/PHPStan/Node/ExprUsedAsStringRule.php,ExprUsedAsStringRuleTest.php,data/expr-used-as-string.php.Contexts covered (the "used as a string" axis):
echo/printarguments(string)cast operand.and concat-assignment.=(whole chain, once)?>/<?php$foo->{$s},$foo?->{$s},$foo->{$s}(),$foo?->{$s}(),Foo::${$s},Foo::{$s}(),Foo::{$s}(dynamic class const), and variable-variables$$s.Root cause
Not a bug but a missing extension point. Determining "this expression ends up as a string" was previously left to each rule, which had to special-case many AST node types and de-duplicate nested concatenations itself. The pattern is centralized here: the value-as-string contexts are exactly the places where PHP coerces a value to string, so they are marked at parse time and a single virtual node is emitted from the one
processExprNode()choke point. Nesting is handled by claiming child concats/interpolation parts, which makes a concatenation chain converge to a single node regardless of how the marking contexts overlap.Analogous cases
The reported example listed only output/coercion contexts; the maintainer additionally asked for
echo,print,(string)cast and dynamic-name expressions like$foo->{$s}. All of these are on the same "expression coerced to string" axis and are implemented and tested here. The "value flowing into a string-typed slot" family (passing an argument to astringparameter, assigning to astring-typed property, string literals in const/property/parameter defaults) is a distinct concept that needs target-type information unavailable at parse time, so it is intentionally not part of this node and can be added separately.Test
ExprUsedAsStringRuleTestregisters a rule onExprUsedAsStringNodeand asserts, viadata/expr-used-as-string.php, that exactly one node fires per string usage across all the contexts above — including that'<script src="' . $s . '"...',$s .= ' src="' . $s . '"'and a heredoc appended with.=each fire once (the nested concat / interpolated string does not fire a second time). The test was confirmed to fail before theNodeScopeResolverchange.Fixes phpstan/phpstan#13008