Repository navigation
Rename .rector.php to rector.php and add composer scripts for the CI gates - #17
Closed
empiricompany wants to merge 1 commit into
Closed
empiricompany wants to merge 1 commit into
empiricompany wants to merge 1 commit into
Conversation
…gates Rector only auto-discovers a config named rector.php (no leading dot), so the dotfile forced every consumer to pass -c/--config explicitly: the lint workflow and the README all carried the flag, and plain vendor/bin/rector --dry-run would silently generate a default config that bypasses the org baseline rules. With the standard name the invocation becomes plain vendor/bin/rector --dry-run, consistent with phpstan and php-cs-fixer. Consequence handled: as a non-dotfile the config is now picked up by the root-level globs of both php-cs-fixer and rector itself, so it must satisfy its own rules. Applied the first-class callable rewrite Rector proposes for its own config (is_dir(...)) and updated the comments that claimed dotfiles were never scanned. Dotfiles remain excluded: PHP's glob() never matches hidden files and php-cs-fixer keeps ignoreDotFiles(true). Also add composer script aliases for the gates (cs, cs-fix, phpstan, rector, check) so the whole suite runs with a single composer check after composer install, matching the invocations the CI workflows use.
Contributor
|
I really don't like rector forcing this (my OCD...) but anyway it makes sense to do this, but we've to do it infrastructure wise, we'll do that right after 26.9 is released with MahoCommerce/infrastructure#22 ;-) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Rector auto-discovers a config only when it is named
rector.php(no leading dot). The dotfile therefore forced every consumer to pass the config explicitly: the README documentedvendor/bin/rector -c .rector.php --dry-runand the lint workflow carriedphp vendor/bin/rector -c .rector.php --dry-run— all places where phpstan and php-cs-fixer need no flag at all, since their configs are auto-discovered.Worse, the flag is a silent footgun: run plain
vendor/bin/rector --dry-runin a repo using this template and Rector does not fail — it prompts to generate a config and creates a defaultrector.phpthat bypasses the org baseline rules entirely. Verified during the migration of a real module to this convention.What changes
.rector.php→rector.php(git mv), so the invocation becomes plainvendor/bin/rector --dry-run, consistent with the other gateslint.yml:php vendor/bin/rector -c .rector.php --dry-run→php vendor/bin/rector --dry-runREADME.md: Rector invocation updated accordingly.gitattributes:/.rector.php export-ignore→/rector.php export-ignore(the config stays excluded from dist tarballs)rector.php+.php-cs-fixer.php: comments updated where they claimed "glob skips dotfiles, so this very config file isn't included" — no longer accurate for the renamed filecomposer.json: script aliases for the gates (cs,cs-fix,phpstan,rector,check) so the whole suite runs with a singlecomposer checkaftercomposer install, matching the invocations the CI workflows useConsequence handled: the config now lints itself
As a non-dotfile,
rector.phpis picked up by the root-levelglob(__DIR__ . '/*.php')of both php-cs-fixer and rector itself, so it must satisfy its own rules. Rector proposed the first-class callable rewrite for its own config ('is_dir'→is_dir(...)) — applied here so the template ships self-clean. Any repo bootstrapped from this template inherits the same guarantee.Dotfiles remain excluded from scanning as before: PHP's
glob()never matches hidden files, and php-cs-fixer's finder keepsignoreDotFiles(true)..php-cs-fixer.phpitself contains the same'is_dir'string-callable pattern and is correctly never flagged.Verified
Tested end-to-end while migrating a module to this convention:
vendor/bin/rector --dry-rungenerated a spurious default config (prompt "No rector.php config found").phpentry points are still scanned (verified with a tracked test file: Rector flagged it viaFunctionFirstClassCallableRector); untracked files are skipped by the existinggitUntrackedPaths()mechanism, which is what protects modules from the core files the composer plugin materializescomposer check: cs-fixer, phpstan, rector) is green with no flagsNote for maintainers
The last upstream commit is "chore: sync .rector.php from infrastructure (#16)": the same rename needs to be applied to the infrastructure source as well, or the next sync will revert this.