Skip to content

Rename .rector.php to rector.php and add composer scripts for the CI gates - #17

Closed
empiricompany wants to merge 1 commit into
MahoCommerce:mainfrom
empiricompany:main
Closed

empiricompany wants to merge 1 commit into
MahoCommerce:mainfrom
empiricompany:main

Conversation

@empiricompany

Copy link
Copy Markdown

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 documented vendor/bin/rector -c .rector.php --dry-run and the lint workflow carried php 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-run in a repo using this template and Rector does not fail — it prompts to generate a config and creates a default rector.php that 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 plain vendor/bin/rector --dry-run, consistent with the other gates
  • lint.yml: php vendor/bin/rector -c .rector.php --dry-run → php vendor/bin/rector --dry-run
  • README.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 file
  • composer.json: 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

Consequence handled: the config now lints itself

As a non-dotfile, rector.php is picked up by the root-level glob(__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 keeps ignoreDotFiles(true). .php-cs-fixer.php itself 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:

  • without the rename, plain vendor/bin/rector --dry-run generated a spurious default config (prompt "No rector.php config found")
  • after the rename, auto-discovery loads the template config and the config self-lints clean
  • root-level .php entry points are still scanned (verified with a tracked test file: Rector flagged it via FunctionFirstClassCallableRector); untracked files are skipped by the existing gitUntrackedPaths() mechanism, which is what protects modules from the core files the composer plugin materializes
  • the full suite (composer check: cs-fixer, phpstan, rector) is green with no flags

Note 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.

…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.
@fballiano

Copy link
Copy Markdown
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 ;-)

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.

2 participants