Skip to content

Fix internal error when using first class callable with NullsafeMethodCall - #5888

Open
phpstan-bot wants to merge 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-3pnp41f
Open

Fix internal error when using first class callable with NullsafeMethodCall#5888
phpstan-bot wants to merge 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-3pnp41f

Conversation

@phpstan-bot

@phpstan-bot phpstan-bot commented Jun 16, 2026 •
edited by staabm
Loading

Copy link
Copy Markdown
Collaborator

Summary

Using the first-class callable syntax together with the nullsafe operator
($foo?->bar(...)) crashed PHPStan with Internal error.. While this is a
fatal error in PHP itself ("Cannot combine nullsafe operator with Closure
creation"), PHPStan must not crash on it. This change makes the analyser treat
such an expression as a regular method-call first-class callable.

Fixes phpstan/phpstan#9746

staabm
staabm requested changes Jun 16, 2026
View reviewed changes

@staabm staabm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crash bug fixes need a AnalyserIntegrationTest

@phpstan-bot

phpstan-bot commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

Done. I addressed staabm's review by adding the requested AnalyserIntegrationTest coverage for the crash fix.

What I did

  1. New data file tests/PHPStan/Analyser/data/bug-9746.php — reproduces the original crash path (the first-class callable NullsafeMethodCall used as a statement, which is exactly what hit CallToMethodStatementWithoutSideEffectsRule → MutatingScope::resolveType() → ShouldNotHappenException in the issue's stack trace).

  2. New test testBug9746() in AnalyserIntegrationTest — runs the full analyser over the file and asserts the three resulting errors, confirming analysis completes without an internal error.

Verification

  • With the fix: test passes (10 assertions).
  • Without the fix (temporarily reverted the committed NullsafeMethodCall branch): the test errors with the ShouldNotHappenException internal error — confirming it genuinely catches the crash.
  • make phpstan: No errors.

Committed as a2149b421 and pushed to create-pull-request/patch-3pnp41f. The existing nsrt/bug-9746.php type-inference test remains as-is; this adds the integration-level crash regression the reviewer asked for.

staabm
staabm reviewed Jun 17, 2026
View reviewed changes
Comment thread tests/PHPStan/Analyser/data/bug-9746.php
staabm
staabm reviewed Jun 17, 2026
View reviewed changes
Comment thread src/Node/MethodCallableNode.php Outdated
staabm
staabm reviewed Jun 17, 2026
View reviewed changes
Comment thread src/Analyser/NodeScopeResolver.php Outdated
@staabm staabm changed the title Treat first-class callable NullsafeMethodCall as a MethodCall instead of throwing Fix internal error when using first class callable with NullsafeMethodCall Jun 17, 2026
@staabm
staabm requested a review from VincentLanglet June 17, 2026 08:24
@VincentLanglet

VincentLanglet commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Since there is a new rule, it has to be approved by ondrej

@staabm

staabm commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

@ondrejmirtes this implements a simple new rule. would be great to have your approval.

staabm
staabm reviewed Jun 17, 2026
View reviewed changes
Comment thread tests/PHPStan/Rules/Methods/data/nullsafe-first-class-callable.php
@VincentLanglet

VincentLanglet commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

@ondrejmirtes this implements a simple new rule. would be great to have your approval.

Maybe you could split the PR in two and merge the bugfix @staabm ?

@staabm

staabm commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

I don't think its worth splitting up, as the internal error exists since 2023 and it seems noone else triggered it yet.

@staabm
staabm force-pushed the create-pull-request/patch-3pnp41f branch from 3bf98fb to 23ac1ac Compare June 25, 2026 05:05
@VincentLanglet VincentLanglet self-assigned this Sep 20, 2026
@staabm @VincentLanglet
Treat first-class callable NullsafeMethodCall as a MethodCall ins…
577799a 
…tead of throwing

- `NodeScopeResolver::processExprNode()` handled the first-class callable
  syntax (`foo(...)`) for `FuncCall`, `MethodCall`, `StaticCall` and `New_`,
  but `NullsafeMethodCall` (`$foo?->bar(...)`) fell through to the `else`
  branch and threw `ShouldNotHappenException`, surfacing as an internal error.
- `NullsafeMethodCall` is the only `Expr\CallLike` subclass that was missing
  from this branch. It is added by wrapping the call in a `MethodCallableNode`
  built from an equivalent `MethodCall`, since `$foo?->bar(...)` is a fatal
  error in PHP ("Cannot combine nullsafe operator with Closure creation") and
  must not crash the analyser.
- Type resolution already routed correctly through `NullsafeMethodCallHandler`,
  so no internal error occurs anywhere the expression is typed (assignment,
  argument, array element, return).
@VincentLanglet
VincentLanglet force-pushed the create-pull-request/patch-3pnp41f branch from 23ac1ac to 577799a Compare September 20, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@staabm staabm staabm approved these changes
@VincentLanglet VincentLanglet VincentLanglet approved these changes
@ondrejmirtes ondrejmirtes Awaiting requested review from ondrejmirtes

Assignees

@VincentLanglet VincentLanglet

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Internal error when using first class callable with NullsafeMethodCall

3 participants

@phpstan-bot @VincentLanglet @staabm