Skip to content

Name the node types the arithmetic operand rules want - #337

Merged
ondrejmirtes merged 1 commit into
2.1.xfrom
rule-multiple-node-types
Sep 22, 2026
Merged

ondrejmirtes merged 1 commit into
2.1.xfrom
rule-multiple-node-types

Conversation

@ondrejmirtes

Copy link
Copy Markdown
Member

Follow-up to phpstan/phpstan-src#6540, which added MultipleNodeTypesRule.

The six OperandsInArithmetic*Rule each handle one operator in its binary form (BinaryOp\Plus) and its compound-assign form (AssignOp\Plus). Since those two share no ancestor of their own, the rules registered for Expr and rejected everything else in processNode() — which meant being called for every expression in the analysed code to do so. On phpstan-src's own analysis that was 258K calls each, 1.5M in total, essentially all of them returning nothing.

getNodeTypes() now names the two classes. getNodeType() stays Expr, so processNode()'s parameter type is unchanged, and the instanceof checks stay where they are — they still pick the operands apart, they are just no longer the filter.

Measured on phpstan-src's own analysis: these six rules plus DataProviderDataRule (phpstan/phpstan-phpunit#…) take the −2.80% of #6540 to −3.65%.

make check passes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QL6WG3TUaXc6ggSoDLdorG

Each of them handles one operator in its binary and its compound-assign
form, and registered for Expr to get both - so it was called for every
expression in the analysed code just to reject it. PHPStan 2.3.0's
MultipleNodeTypesRule lets them name the two classes instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QL6WG3TUaXc6ggSoDLdorG
@ondrejmirtes
ondrejmirtes merged commit 19b5b5b into 2.1.x Sep 22, 2026
45 checks passed
@ondrejmirtes
ondrejmirtes deleted the rule-multiple-node-types branch September 22, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant