mirror of
https://github.com/FOSSBilling/FOSSBilling.git
synced 2026-10-23 03:37:01 +09:00
Adam Daley
5f6397cb12
* 🎨 Apply pending Rector/PHP-CS-Fixer fixes Drift left over from the auto-cs-fix.yml run that failed to open its PR due to an expired BOT_TOKEN. * 🐛 Fix code quality workflow and add PR-level style check - Rotate the sweep's push credential from BOT_TOKEN (expired) to FOSSBILLING_BOT_TOKEN, a fresh fine-grained PAT scoped to just this repo (contents, pull-requests, issues: read/write). - Factor the shared Rector/PHP-CS-Fixer setup out of auto-cs-fix.yml into a local composite action, .github/actions/php-quality-setup. - Add code-quality-check.yml: a read-only dry-run check that runs on every pull request, including forks, scoped to just the files each PR changes so pre-existing drift on main can't fail an unrelated PR. Complements the scheduled sweep, which now only needs to catch drift from rule/tool-version changes on code no open PR touches. * 🎨 Replace peter-evans/create-pull-request with gh CLI Removes the third-party Action dependency for opening the sweep's PR in favor of the officially-maintained gh CLI, which ships preinstalled on GitHub-hosted runners: - gh auth setup-git wires GH_TOKEN into git's credential helper, so plain git push/fetch authenticate without a hand-rolled http.extraheader (what silently broke with the old BOT_TOKEN). - chore/cs-fix is fully regenerated every run, so there's no need to diff against existing remote content — force-with-lease push plus gh pr list/create covers create-or-update. - Also more portable: the same script runs unchanged outside GitHub Actions' action-invocation model (other CI, cron, a terminal), whereas a third-party Action cannot. - Top-level permissions trimmed to contents: read — nothing in the job uses the implicit GITHUB_TOKEN anymore. * 🔒 Pin 1password/load-secrets-action to v5.0.1 by SHA Was floating on the mutable @v4 tag, inconsistent with every other third-party action in this repo (all pinned by commit SHA), and a version behind — v5.0.1 is current as of last week and includes 1Password's own CI hardening. No breaking changes for our usage (OP_SERVICE_ACCOUNT_TOKEN path is untouched; v5.0.0 only adds an alternative OIDC auth method). * 🎨 Remove unused create_pr operation output Set in both branches but never read — the job summary only uses .outputs.url, and the same created-vs-updated info is already in the step's own log output. * 🐛 Fix two Rector rules that generate code breaking Pest CI on PR #4206 caught these — I'd only run php -l (syntax check) on the earlier drift-cleanup commit, not the actual test suite, and both slipped through: - AddArrowFunctionReturnTypeRector typed Pest expect()-chain closures as their IDE-only @mixin stub (Pest\Mixins\Expectation) instead of the real runtime class (Pest\Expectation) — throws a TypeError. - ArrowFunctionDelegatingCallToFirstClassCallableRector converted a Pest ->with() dataset closure to a first-class callable reference, which breaks Pest's Closure::bindTo() lazy-evaluation step (no scope to rebind on a closure derived from a plain function). Both only misfire against Pest's dynamic patterns in tests/, so they're skipped there specifically rather than repo-wide — src/ keeps the benefit. Reverts the two bad transformations and re-verifies with the full Pest suite this time, not just php -l. * 🎨 Apply Rector fixes for drift introduced by #4205 RecastingRemovalRector: three redundant (string) casts, plus one PHP 8.3 typed-const/literal-return-type modernization in CacheFactory.php — all from files #4205 touched after this branch forked from main. Verified with the full suite this time (2812 tests, 10979 assertions) and PHPStan, not just php -l. * 🎨 Fix import order in rector.php Caught by the new PR check itself — it passes rector.php as one of the changed files, so PHP-CS-Fixer processes it directly (path-mode override), unlike the scheduled sweep whose Finder never scans repo-root config files at all. * 🔒 Fix script-injection risk and stale fetch ref from CodeRabbit review - code-quality-check.yml: the changed-file list was interpolated via ${{ steps.changed_files.outputs.files }} directly into a run: script. That's literal text substitution before bash parses anything, so a PHP filename containing shell metacharacters (or just a space) could break argument parsing or execute as a command. Fixed by writing the NUL-delimited path list to a file and reading it back into a real bash array with mapfile in every step that needs it — only a plain boolean (has_php_files) now crosses the ${{ }} boundary. Verified: a space-containing filename was silently split into two broken arguments under the old design; the array-based version preserves it as one element. - auto-cs-fix.yml: pin the fetch to update the actual remote-tracking ref (refs/remotes/origin/chore/cs-fix), not just FETCH_HEAD, so --force-with-lease always has a real baseline to compare against. * 🔒 Add option-terminator before changed-file paths A filename beginning with '-' would otherwise be parsed as a Rector/ PHP-CS-Fixer CLI option rather than a path. Not currently reachable given rector.php/.php-cs-fixer.dist.php scope to src/ and tests/ (a repo-relative diff path always has that prefix), but zero-cost to harden regardless. Verified the composed composer script -- ... -- ... invocation forwards the second terminator correctly. * 🎨 Apply Rector/CS-Fixer fixes for drift introduced by #4204 readonly on Registrar_Rdap::$httpClient (verified: only ever assigned in the constructor), PHP 8.3 typed consts, redundant @param mixed removal, pre-increment style. Verified with the full suite (2854 tests, 11055 assertions) and PHPStan.
44 lines
1.7 KiB
PHP
44 lines
1.7 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
use Rector\Caching\ValueObject\Storage\FileCacheStorage;
|
|
use Rector\CodingStyle\Rector\ArrowFunction\ArrowFunctionDelegatingCallToFirstClassCallableRector;
|
|
use Rector\Config\RectorConfig;
|
|
use Rector\TypeDeclaration\Rector\ArrowFunction\AddArrowFunctionReturnTypeRector;
|
|
|
|
return RectorConfig::configure()
|
|
->withPaths([
|
|
__DIR__ . '/src',
|
|
__DIR__ . '/tests',
|
|
])
|
|
->withSkip([
|
|
__DIR__ . '/src/vendor',
|
|
__DIR__ . '/src/data/cache',
|
|
// Both rules only misbehave against Pest's more dynamic patterns, which
|
|
// exist in tests/ but not src/ — kept active there.
|
|
//
|
|
// Rector infers this from Pest's IDE-only @mixin stub (Pest\Mixins\Expectation),
|
|
// which doesn't match the real runtime class (Pest\Expectation) — declaring it
|
|
// throws a TypeError the moment the closure runs. Only unsafe for closures whose
|
|
// own body resolves to an expect() chain.
|
|
AddArrowFunctionReturnTypeRector::class => [
|
|
__DIR__ . '/tests',
|
|
__DIR__ . '/src/modules/*/tests/*',
|
|
],
|
|
// Pest's ->with() dataset closures get Closure::bindTo() called on them for
|
|
// lazy evaluation, which throws for a closure derived from a first-class
|
|
// callable reference to a plain function (no scope to rebind).
|
|
ArrowFunctionDelegatingCallToFirstClassCallableRector::class => [
|
|
__DIR__ . '/tests',
|
|
__DIR__ . '/src/modules/*/tests/*',
|
|
],
|
|
])
|
|
->withAttributesSets()
|
|
->withPhpSets()
|
|
->withComposerBased(doctrine: true, phpunit: true)
|
|
->withTypeCoverageLevel(30)
|
|
->withDeadCodeLevel(30)
|
|
->withCache('./cache/rector', FileCacheStorage::class)
|
|
->withParallel(120, 8, 10);
|