Skip to content

ForbidCheckedExceptionInYieldingMethod: skip closures handled by callable rule#356

Draft
janedbal wants to merge 1 commit intomasterfrom
fix/yielding-rule-closure-overlap-shared-helper
Draft

ForbidCheckedExceptionInYieldingMethod: skip closures handled by callable rule#356
janedbal wants to merge 1 commit intomasterfrom
fix/yielding-rule-closure-overlap-shared-helper

Conversation

@janedbal
Copy link
Copy Markdown
Member

@janedbal janedbal commented Apr 7, 2026

Summary

Alternative implementation for #349 (closes #346) — extracts a shared ImmediatelyInvokedCallableHelper instead of duplicating the reflection/argument analysis logic.

  • New ImmediatelyInvokedCallableHelper — shared service encapsulating reflection resolution, ArgumentsNormalizer dispatch, isImmediatelyInvokedCallable check, and isCallableExpression detection
  • Refactored ForbidCheckedExceptionInCallableRule — removed 5 duplicated methods (~116 lines), whitelistAllowedCallables now delegates to the helper
  • Fixed ForbidCheckedExceptionInYieldingMethodRule — only reports yielding closures that are @param-immediately-invoked-callable or directly invoked; non-immediately-invoked and standalone closures are left to ForbidCheckedExceptionInCallableRule

Why not just merge #349?

PR #349 solves the problem correctly but duplicates ~80 lines of reflection/argument analysis logic from the callable rule. This creates a maintenance burden — any PHPStan API change (like the fiber processing change in #340) would require identical fixes in both rules.

Improvements over #349

Test plan

  • All 63 existing tests pass (896 assertions)
  • PHPStan self-analysis clean
  • New test cases: non-immediately-invoked closure (no error), named arg to immediately-invoked (error), standalone closure (no error), directly invoked closure (error)

Co-Authored-By: Claude Code

…able rule

Extract shared ImmediatelyInvokedCallableHelper to eliminate duplication
between the callable and yielding rules. The yielding rule now only reports
closures that are @param-immediately-invoked-callable or directly invoked,
since non-immediately-invoked and standalone closures are already covered
by ForbidCheckedExceptionInCallableRule.

Closes #346

Co-Authored-By: Claude Code
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.

forbidCheckedExceptionInYieldingMethod/forbidCheckedExceptionInCallable overlap for closures

1 participant