Skip to content

[dx] Set Option::SOURCE parameter from the resolved paths - #8424

Merged
TomasVotruba merged 2 commits into
rectorphp:mainfrom
SanderMuller:feature/publish-source-paths-parameter
Sep 6, 2026
Merged

[dx] Set Option::SOURCE parameter from the resolved paths#8424
TomasVotruba merged 2 commits into
rectorphp:mainfrom
SanderMuller:feature/publish-source-paths-parameter

Conversation

@SanderMuller

@SanderMuller SanderMuller commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Option::SOURCE is only written to SimpleParameterProvider by AbstractRectorTestCase. A real run leaves it empty, so an extension reading the processed paths from the parameter works under the test harness and gets [] in production.

I hit this in a rules package that reads Option::PATHS and falls back to Option::SOURCE to learn which paths a run covers. The fallback works in its tests and never fires in a real run.

ConfigurationFactory::resolvePaths() now sets it to the paths it resolved: the command line when given, Option::PATHS otherwise. It already sets IS_RUN_NARROWED and FILES_WITHOUT_EXTENSION there.

Option::SOURCE is also in CACHE_IGNORED_PARAMETER_NAMES: the resolved paths change per invocation, so hashing them would drop the cache whenever the target changes.

AbstractRectorTestCase sets it, a real run never does, so code reading the
parameter works under the test harness and is empty in production.

Excluded from the cache invalidation hash: the paths change per invocation, so
hashing them would drop the cache whenever the target changes.
Comment thread tests/Configuration/ConfigurationFactoryTest.php Outdated
Set the parameters and assert, as UnusedSkipResolverTest does.
* @var array<Option::*>
*/
private const array CACHE_IGNORED_PARAMETER_NAMES = [
Option::SOURCE,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why this change?

@SanderMuller SanderMuller Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Without it, the new setParameter() call puts the resolved paths into hashForCacheInvalidation(). They differ per invocation, so rector process src/Foo.php followed by rector process would produce two different hashes and drop the unchanged-files cache each time. With it, the hash is the same as it is today, the parameter becomes visible without changing caching at all.

It also matches what the list already describes: the target narrows which files are visited, it does not change how any file is refactored.

Removing just this line fails testSourceParameterIsOutsideTheCacheInvalidationHash.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see 👍

@TomasVotruba
TomasVotruba merged commit a8d8435 into rectorphp:main Sep 6, 2026
44 checks passed
@TomasVotruba

Copy link
Copy Markdown
Member

Thanks 👍

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.

2 participants