Skip to content

[CodeQuality] Skip DirectInstanceOverMockArgRector outside test classes - #776

Merged
TomasVotruba merged 1 commit into
mainfrom
fix-direct-instance-mock-non-test-crash
Aug 27, 2026
Merged

[CodeQuality] Skip DirectInstanceOverMockArgRector outside test classes#776
TomasVotruba merged 1 commit into
mainfrom
fix-direct-instance-mock-non-test-crash

Conversation

@TomasVotruba

@TomasVotruba TomasVotruba commented Aug 27, 2026

Copy link
Copy Markdown
Member

Fixes crash reported in rectorphp/rector#9869.

Problem

DirectInstanceOverMockArgRector called ScopeFetcher::fetch($node) as its first action, before any guard. PHPStan does not assign scope to args of a new expression that mixes an unpacked spread with a named argument, so the fetch threw Scope not available — crashing the entire Rector run.

Worse, this happened in non-test application code. The rule ships in the phpunit-code-quality set, so any project with this argument shape failed its whole run, even though the rule only ever touches Symfony Request/RequestStack mocks inside tests.

// non-test class — crashed the run before this PR
$data = new SomeData(
    ...$request->updateData(),
    user: (string) $request->user()->getKey(),
);

Fix

Gate on TestsNodeAnalyzer::isInTestClass() (resolves via reflection, no scope needed) and drop the eager ScopeFetcher::fetch() entirely. Non-test classes bail out early, before any scope is touched.

-$scope = ScopeFetcher::fetch($node);
-if (! $scope->isInClass()) {
-    return null;
-}
-
-$classReflection = $scope->getClassReflection();
-if (! $classReflection->is(PHPUnitClassName::TEST_CASE)) {
-    return null;
-}
+// run on test classes only, non-test code may lack scope on args and crash the whole run
+if (! $this->testsNodeAnalyzer->isInTestClass($node)) {
+    return null;
+}

Added a skip fixture reproducing the crashing shape in a non-test class.

Gate the rule on TestsNodeAnalyzer::isInTestClass() before fetching scope.
Non-test app code with a spread + named-arg new expression has no scope on
the arg, so the eager ScopeFetcher::fetch() threw "Scope not available" and
crashed the whole Rector run. The rule only handles Symfony Request mocks in
tests anyway.

Claude-Session: https://claude.ai/code/session_018faFsAZPx2pm78KBAwortK
@TomasVotruba
TomasVotruba enabled auto-merge (squash) August 27, 2026 17:46
@TomasVotruba
TomasVotruba disabled auto-merge August 27, 2026 17:46
@TomasVotruba
TomasVotruba merged commit 25ece8d into main Aug 27, 2026
7 checks passed
@TomasVotruba
TomasVotruba deleted the fix-direct-instance-mock-non-test-crash branch August 27, 2026 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant