Skip to content

fix(analyzer): fix crash when analzing first class callable#11443

Open
azjezz wants to merge 1 commit intovimeo:6.xfrom
azjezz:fix-panic
Open

fix(analyzer): fix crash when analzing first class callable#11443
azjezz wants to merge 1 commit intovimeo:6.xfrom
azjezz:fix-panic

Conversation

@azjezz
Copy link
Contributor

@azjezz azjezz commented May 19, 2025

This PR fixes a runtime crash with first-class callables and significantly refactors conditional logic analysis for clarity and maintainability.

Example:

<?php

while (count(...) > 1) { /* panics */ }

Key Changes:

  • Fixes Crash: Resolves an assertion exception when ->getArgs() is called on a FuncCall node representing a first-class callable. The new isFunctionCallToOneOf() helper now safely handles this.
  • Reduces Duplication & Improves Clarity:
    • Consolidated duplicated logic in various conditional checks (e.g., numeric comparisons like hasInferior/SuperiorNumberCheck are now handled by getComparisonLiteralOperand() and getLiteralIntegerFromExpression()).
    • Similar refactoring applied to gettype, get_debug_type, get_class, and count/sizeof checks.
    • Introduced multiple helper methods to centralize checks and make intent clearer.
  • Optimizes: Implements short-circuiting for some boolean expressions.

@azjezz azjezz force-pushed the fix-panic branch 2 times, most recently from 6b9c211 to f67bb1a Compare May 19, 2025 05:52
@azjezz
Copy link
Contributor Author

azjezz commented May 19, 2025

@danog please let me know if i should split this PR into smaller ones. I initially was just reading through AssertionFinder and came across the bug mentioned above, and got carried away with refactoring and optimizing things :)

…rst class callables

Signed-off-by: azjezz <azjezz@protonmail.com>
@danog danog added the release:fix The PR will be included in 'Fixes' section of the release notes label May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:fix The PR will be included in 'Fixes' section of the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants