Skip to content

Refactor: Add BleedingEdgeToggle::withBleedingEdge() and use it instead of manual set/restore in tests#5915

Merged
staabm merged 2 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-29mu162
Jun 22, 2026
Merged

Refactor: Add BleedingEdgeToggle::withBleedingEdge() and use it instead of manual set/restore in tests#5915
staabm merged 2 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-29mu162

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

Several tests manually back up the global bleeding-edge toggle with BleedingEdgeToggle::isBleedingEdge(), call setBleedingEdge(), and restore the previous value afterwards — sometimes via ad-hoc try/finally blocks, sometimes by restoring at the end of a data provider that yields while the toggle is held. This is repetitive and, in data providers, fragile: holding the toggle across a yield can leak it into unrelated tests.

This PR introduces a single helper, BleedingEdgeToggle::withBleedingEdge(), that encapsulates the set/restore handling, and rewrites every manual set/restore site to use it.

Changes

  • src/DependencyInjection/BleedingEdgeToggle.php: add withBleedingEdge(bool $bleedingEdge, callable $callback). It backs up the current value, sets the toggle, runs the callback, and restores the previous value in a finally. It throws ShouldNotHappenException if the callback returns a Generator, because that would mean the toggle is held across yield.
  • tests/PHPStan/Type/TypeCombinatorTest.php: replace the four identical backup/setBleedingEdge(true)/restore blocks in testUnion, testUnionInversed, testIntersect, testIntersectInversed with a withBleedingEdge(true, ...) call that resolves the type strings inside the callback.
  • tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php:
    • testIsSuperTypeOf, testEqualsTreatsLegacyNullAndSealedMarkerAsEqual, testSealedness: replace try/finally set/restore with withBleedingEdge(), returning the constructed types from the callback and asserting on them afterwards.
    • dataAccepts, dataGetArraySize: build the data sets inside withBleedingEdge() callbacks and yield from the returned arrays, instead of holding the toggle across yield.
  • tests/PHPStan/Type/Constant/ConstantArrayTypeBuilderTest.php: testGetArraySealedEmptyStaysConstantArrayType now constructs its array via withBleedingEdge(true, ...).
  • tests/PHPStan/DependencyInjection/BleedingEdgeToggleTest.php (new): unit tests for the helper.
  • src/DependencyInjection/ContainerFactory.php is intentionally left untouched: it performs a one-way set when building the container, not a set/restore, so it is not a candidate for withBleedingEdge().

Root cause

This is a refactor, not a bug fix. The repeated isBleedingEdge() backup + setBleedingEdge() restore pattern was scattered across tests. The data-provider variants additionally restored the toggle only when the generator was fully consumed, so the toggle stayed mutated while PHPUnit held the generator open between yields. withBleedingEdge() centralizes the set/restore in a finally and forbids returning a Generator, forcing data providers to construct their objects while the toggle is set and return them, eliminating the leak.

Test

  • New BleedingEdgeToggleTest verifies: the toggle is set during the callback and restored afterwards (both when previously disabled and enabled), the callback's return value is propagated, the previous value is restored even when the callback throws, returning a Generator throws ShouldNotHappenException while still restoring the toggle, and data sets are produced while the toggle is set.
  • All refactored call sites (TypeCombinatorTest, ConstantArrayTypeTest, ConstantArrayTypeBuilderTest) continue to pass, confirming behavior is preserved.

Fixes phpstan/phpstan#14859

staabm and others added 2 commits June 22, 2026 07:02
…nual set/restore in tests

- Add `BleedingEdgeToggle::withBleedingEdge(bool, callable)` that sets the toggle for the duration of the callback and restores the previous value in a `finally`, so the global toggle is never observably mutated outside the call.
- Reject callbacks that return a `Generator` with a `ShouldNotHappenException`: holding the toggle across a `yield` would leak it into unrelated tests, so data providers must build their data sets inside the callback.
- Replace the manual `isBleedingEdge()` backup + `setBleedingEdge()` restore (including `try`/`finally` blocks) in `TypeCombinatorTest`, `ConstantArrayTypeTest` and `ConstantArrayTypeBuilderTest` with `withBleedingEdge()`.
- Restructure the `dataAccepts` and `dataGetArraySize` data providers so the yielded data sets are constructed inside `withBleedingEdge()` callbacks and returned, then `yield from`-ed, instead of holding the toggle across `yield`.
- Add `BleedingEdgeToggleTest` covering toggling, restoration (including on exception), return value passing, the no-yield guard, and data-set production.
@staabm staabm requested a review from VincentLanglet June 22, 2026 07:17
@staabm staabm changed the title Add BleedingEdgeToggle::withBleedingEdge() and use it instead of manual set/restore in tests Refactor: Add BleedingEdgeToggle::withBleedingEdge() and use it instead of manual set/restore in tests Jun 22, 2026
@staabm staabm merged commit 02ee8ba into phpstan:2.2.x Jun 22, 2026
670 of 672 checks passed
@staabm staabm deleted the create-pull-request/patch-29mu162 branch June 22, 2026 07:59
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.

Refactor: simplify BleedingEdgeToggle set/restore handling

3 participants