diff --git a/.changelog/multiline-boolean-conditions.md b/.changelog/multiline-boolean-conditions.md new file mode 100644 index 0000000..49ce6f7 --- /dev/null +++ b/.changelog/multiline-boolean-conditions.md @@ -0,0 +1,8 @@ +--- +category: added +type: minor +--- + +Check and fix one symbolic boolean part per line in existing multiline if, elseif, while and do-while conditions through an owned native ECS Sniff. Preserve single-line conditions, function/arrow/class expressions and native spacing/skip overrides. Fingerprint owned policy source for cache invalidation and record the Sniff separately from native fixer risk classifications. + +Enforce first-expression and closing-wrapper placement through the native PHPCS PSR-12 implementation, adapted to the ECS spacing service. Default to four spaces/LF; document nonstandard spacing overrides explicitly and preserve alternative syntax without claiming complete standards certification. diff --git a/.github/OptionalAnalysis/AnalysisResult.php b/.github/OptionalAnalysis/AnalysisResult.php index 8d91034..68fbf27 100644 --- a/.github/OptionalAnalysis/AnalysisResult.php +++ b/.github/OptionalAnalysis/AnalysisResult.php @@ -29,11 +29,16 @@ public function __construct( public string $details = '', public ?int $nativeExit = null, ) { - if (!in_array($tool, ['Magic Numbers', 'Rector'], true) + if ( + !in_array($tool, ['Magic Numbers', 'Rector'], true) || !in_array($status, ['disabled', 'not-executed', 'clean', 'findings', 'operational-error'], true) - || (null !== $selectedCount && $selectedCount < 0) - || (null !== $findingCount && $findingCount < 0) - || (null !== $nativeExit && $nativeExit < 0)) { + || (null !== $selectedCount + && $selectedCount < 0) + || (null !== $findingCount + && $findingCount < 0) + || (null !== $nativeExit + && $nativeExit < 0) + ) { throw new InvalidArgumentException('Invalid optional-analysis result.'); } } diff --git a/.github/OptionalAnalysis/BoundedProcess.php b/.github/OptionalAnalysis/BoundedProcess.php index d07e71f..6e44b46 100644 --- a/.github/OptionalAnalysis/BoundedProcess.php +++ b/.github/OptionalAnalysis/BoundedProcess.php @@ -96,11 +96,13 @@ private function assertPrerequisites(float $timeout): void if (!is_finite($timeout) || $timeout <= 0 || $timeout > self::TIMEOUT_SECONDS) { throw new InvalidArgumentException('Optional analyzer timeout must be positive, finite and at most 300 seconds.'); } - if ('Linux' !== PHP_OS_FAMILY + if ( + 'Linux' !== PHP_OS_FAMILY || !is_executable('/usr/bin/setsid') || !function_exists('posix_getpgid') || !function_exists('posix_getsid') - || !function_exists('posix_kill')) { + || !function_exists('posix_kill') + ) { throw new RuntimeException('Optional workflow execution requires hosted Linux, setsid and PHP POSIX process-group support.'); } } diff --git a/.github/OptionalAnalysis/NativeReportParser.php b/.github/OptionalAnalysis/NativeReportParser.php index 6ecd1b8..010f820 100644 --- a/.github/OptionalAnalysis/NativeReportParser.php +++ b/.github/OptionalAnalysis/NativeReportParser.php @@ -135,8 +135,11 @@ private function children(DOMElement $parent, string $name): array continue; } - if (XML_COMMENT_NODE === $child->nodeType - || (XML_TEXT_NODE === $child->nodeType && '' === trim($child->textContent))) { + if ( + XML_COMMENT_NODE === $child->nodeType + || (XML_TEXT_NODE === $child->nodeType + && '' === trim($child->textContent)) + ) { continue; } @@ -187,10 +190,15 @@ private function rectorErrors(array $errors): array { $diagnostics = []; foreach ($errors as $error) { - if (!is_array($error) - || (isset($error['file']) && !is_string($error['file'])) + if ( + !is_array($error) + || (isset($error['file']) + && !is_string($error['file'])) || !is_string($error['message'] ?? null) - || (isset($error['line']) && (!is_int($error['line']) || $error['line'] < 1))) { + || (isset($error['line']) + && (!is_int($error['line']) + || $error['line'] < 1)) + ) { throw new InvalidArgumentException('Rector JSON error location is invalid.'); } $diagnostics[] = ['file' => $error['file'] ?? '[unavailable]', 'line' => $error['line'] ?? null, 'message' => $error['message']]; diff --git a/.github/OptionalAnalysis/WorkflowInput.php b/.github/OptionalAnalysis/WorkflowInput.php index 5f633c9..9729fc3 100644 --- a/.github/OptionalAnalysis/WorkflowInput.php +++ b/.github/OptionalAnalysis/WorkflowInput.php @@ -20,9 +20,11 @@ public function __construct( public string $rectorPolicy, public string $rectorConfig, ) { - if (!str_starts_with($directory, '/') + if ( + !str_starts_with($directory, '/') || 1 !== preg_match('/\A[a-fA-F0-9]{40}\z/D', $base) - || 1 !== preg_match('/\A[a-fA-F0-9]{40}\z/D', $head)) { + || 1 !== preg_match('/\A[a-fA-F0-9]{40}\z/D', $head) + ) { throw new InvalidArgumentException('Optional analyses require an absolute consumer directory and exact base/head SHAs.'); } foreach ([$magicNumbersPolicy, $rectorPolicy] as $policy) { diff --git a/.github/ReleaseChannelUpdater.php b/.github/ReleaseChannelUpdater.php index f4bb960..1b64563 100644 --- a/.github/ReleaseChannelUpdater.php +++ b/.github/ReleaseChannelUpdater.php @@ -39,10 +39,16 @@ public function __construct( public function update(string $repository, string $tag, string $sha): array { $selected = self::version($tag); - if (null === $selected || '0' !== $selected['major'] || 'php-fast-forward/code-standards' !== $repository || !self::sha($sha) || 1 !== preg_match( - '~^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$~D', - $repository, - )) { + if ( + null === $selected + || '0' !== $selected['major'] + || 'php-fast-forward/code-standards' !== $repository + || !self::sha($sha) + || 1 !== preg_match( + '~^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$~D', + $repository, + ) + ) { throw new RuntimeException('Select an exact repository, complete stable release tag and full commit SHA.'); } @@ -204,8 +210,11 @@ private function revalidate( throw new RuntimeException('Release or conflicting-tag evidence changed before the channel update.'); } - if (null !== $previous && (!self::contains($finalCatalog, $previous) - || $this->commit($repository, $previous['tag_name']) !== $current['sha'])) { + if ( + null !== $previous + && (!self::contains($finalCatalog, $previous) + || $this->commit($repository, $previous['tag_name']) !== $current['sha']) + ) { throw new RuntimeException('The previous channel release evidence changed before the update.'); } } @@ -280,8 +289,11 @@ private function reference(string $repository, string $ref): ?array if (null === $data) { return null; } - if ('refs/' . $ref !== ($data['ref'] ?? null) || !self::sha($data['object']['sha'] ?? null) - || !is_string($data['object']['type'] ?? null)) { + if ( + 'refs/' . $ref !== ($data['ref'] ?? null) + || !self::sha($data['object']['sha'] ?? null) + || !is_string($data['object']['type'] ?? null) + ) { throw new RuntimeException('Malformed exact Git reference evidence.'); } @@ -361,8 +373,12 @@ private function catalog(string $repository, string $major): array throw new RuntimeException('Malformed release catalog.'); } foreach ($data as $release) { - if (!is_array($release) || !is_bool($release['draft'] ?? null) - || !is_bool($release['prerelease'] ?? null) || !is_string($release['tag_name'] ?? null)) { + if ( + !is_array($release) + || !is_bool($release['draft'] ?? null) + || !is_bool($release['prerelease'] ?? null) + || !is_string($release['tag_name'] ?? null) + ) { throw new RuntimeException('Partial release catalog metadata cannot establish the latest version.'); } if ($release['draft'] || $release['prerelease']) { @@ -393,10 +409,13 @@ private function catalog(string $repository, string $major): array $left['tag_name'], )), ); - if (count($releases) > 1 && 0 === self::compare( - self::version($releases[0]['tag_name']), - self::version($releases[1]['tag_name']), - )) { + if ( + count($releases) > 1 + && 0 === self::compare( + self::version($releases[0]['tag_name']), + self::version($releases[1]['tag_name']), + ) + ) { throw new RuntimeException('Build-only release ties cannot choose a unique channel destination.'); } @@ -461,10 +480,13 @@ private static function published(array $release): bool */ private static function publicationTime(mixed $value): bool { - if (!is_string($value) || 1 !== preg_match( - '/^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$/D', - $value, - )) { + if ( + !is_string($value) + || 1 !== preg_match( + '/^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$/D', + $value, + ) + ) { return false; } $date = DateTimeImmutable::createFromFormat('!Y-m-d\TH:i:s\Z', $value, new DateTimeZone('UTC')); @@ -481,11 +503,15 @@ private static function publicationTime(mixed $value): bool */ private static function version(mixed $tag): ?array { - if (!is_string($tag) || strlen($tag) > 128 || 1 !== preg_match( - '/^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)(?:\+[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?$/D', - $tag, - $matches, - )) { + if ( + !is_string($tag) + || strlen($tag) > 128 + || 1 !== preg_match( + '/^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)(?:\+[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?$/D', + $tag, + $matches, + ) + ) { return null; } diff --git a/README.md b/README.md index 41598d4..dfb12a3 100644 --- a/README.md +++ b/README.md @@ -77,8 +77,10 @@ including native ECS findings, failure classification and remaining-work counts. See the [structured check contract](docs/structured-check.rst). Text is the default; `fix` uses text output. The default preset stores its ECS cache beneath the system temporary directory, -in `fast-forward-code-standards/`. -Different checkouts cannot invalidate each other's cache. Native +in `fast-forward-code-standards//`. +The fingerprint includes the installed preset and owned checker paths and source +bytes. Different checkouts cannot invalidate each other's cache, and a changed +owned rule cannot reuse an earlier passing result. Native `withCache()` overrides remain supported and apply last; share cache directories only when their invalidation and concurrent use are intentionally coordinated. `fix` explicitly applies ECS fixes. It does not commit, push or update dependencies. @@ -128,6 +130,26 @@ both ECS and a competing PHP-CS-Fixer pass. Standalone ECS users may import the same library preset and set their own paths. CLI selection uses the scope below; ECS's configured paths do not expand the CLI's explicit selected file list. +For braced multiline `if`, `elseif`, `while` and `do-while` conditions, the +native PSR-12 wrapper places the first expression on the next line and the +closing parenthesis on its own line (`) {`, or `);` for do-while). The native +leading-operator policy follows PER 3.0. An owned whitespace policy additionally +puts each symbolic `&&` and `||` on its own line in those multiline conditions. +The policy includes grouped expressions, call arguments +and array indexes within that condition; function, arrow and anonymous-class +declarations remain opaque. Single-line conditions and other expression contexts +retain their native rules. Splitting every operator, including nested expression +parts, is a project extension rather than an additional PER requirement. +The default is four spaces and LF. Native ECS `withSpacing()` overrides for two +spaces, tabs or CRLF remain supported as explicit deviations from PSR-12/PER, +not standards-compliant alternatives. +Consumers can explicitly skip `FastForward\CodeStandards\Sniff\MultilineBooleanConditionSniff` +through `withSkip()`; that native method replaces the skip list, so preserve any +other intended preservation skips. See [the resolved preset](docs/resolved-preset.rst). +The separate `MultilineConditionSpacingSniff` adapts the native wrapper to the +same spacing service. Alternative colon syntax is preserved; the wrapper tests +do not certify that syntax, or the complete preset, against every PSR/PER rule. + The formatter worker uses ECS's public factory and command APIs with this tool's own Composer installation. An isolated tool does not implicitly load the target checkout's `vendor/autoload.php`. Explicit PHP configuration retains the checkout diff --git a/bin/ecs-worker b/bin/ecs-worker index f7d3e96..8735cab 100644 --- a/bin/ecs-worker +++ b/bin/ecs-worker @@ -16,7 +16,8 @@ try { $argc < $separator + 2 || '--' !== $argv[$separator] || !in_array($argv[4], ['--check', '--fix'], true) - || ($json && '--check' !== $argv[4]) + || ($json + && '--check' !== $argv[4]) ) { throw new InvalidArgumentException('ECS worker requires installation paths, an explicit mode and selected files.'); } diff --git a/docs/policy.rst b/docs/policy.rst index 166bfe3..7fec2f7 100644 --- a/docs/policy.rst +++ b/docs/policy.rst @@ -64,4 +64,4 @@ The `PHPDoc comparison `__ records actual custom Rector a Selected PER evolution ---------------------- -The current preset replaces prepared PSR-12 with ECS 13.3.8's native non-risky PER Coding Style 3.0 set, then applies the owned preservation skips and explicit spacing overrides. It does not combine duplicate PSR/PER/common/docblock sets. The resolved snapshot records 63 rules:20 added and5 removed from the earlier 48-rule snapshot, with explicit option changes captured for review. PER supersedes several older checker choices; the two snapshots are not identical contracts. No migration or risky rule is enabled. See the resolved preset and PHPDoc golden fixture for information-preservation decisions. Earlier comparison tables above remain the initial extraction evidence rather than a claim of current parity. +The current preset replaces prepared PSR-12 with ECS 13.3.9's native non-risky PER Coding Style 3.0 set, then applies the owned preservation skips and explicit spacing overrides. It does not combine duplicate PSR/PER/common/docblock sets. The resolved snapshot records 63 native fixers and two allowlisted PHPCS Sniffs: owned boolean whitespace and a native PSR-12 condition-wrapper adapter. The native PER transition added 20 and removed five fixers from the earlier 48-rule snapshot, with explicit option changes captured for review. PER supersedes several older checker choices; the two snapshots are not identical contracts. No migration or risky rule is enabled. See the resolved preset and PHPDoc golden fixture for information-preservation decisions. Earlier comparison tables above remain the initial extraction evidence rather than a claim of current parity. diff --git a/docs/resolved-preset.rst b/docs/resolved-preset.rst index eb8022d..1dd70a7 100644 --- a/docs/resolved-preset.rst +++ b/docs/resolved-preset.rst @@ -1,9 +1,9 @@ Resolved PER preset =================== -Snapshot: ECS 13.3.9 from this repository’s Composer lock. The compiled container contains 63 distinct PHP-CS-Fixer rules, all reporting ``isRisky() === false``. This is the actual Code Standards preset, not a resolved DevTools or CoiSA preset. +Snapshot: ECS 13.3.9 from this repository’s Composer lock. The compiled container contains 63 distinct PHP-CS-Fixer rules, all reporting ``isRisky() === false``, and two allowlisted PHPCS Sniffs: 65 checkers in total. This is the actual Code Standards preset, not a resolved DevTools or CoiSA preset. -The fixture ``tests/Fixtures/preset-rules.json`` records the installed ECS version and exact reference, fully qualified classes, explicit configuration and risky classification after container boot/skip passes. Unspecified options retain the fixer's defaults at that exact version; the table does not expand those defaults. ``tests/Config/PresetTest.php`` rebuilds that configuration and compares it exactly. It reads this installation's Composer metadata and verifies every fixer's source path inside that exact ECS package. This avoids querying the global ``InstalledVersions`` registry after the bundled ECS autoloader has registered its own graph: locally that changed the reported ECS version from ``13.3.8`` to ``dev-main``. Installation separation alone does not isolate that global registry. Dependency updates must review any resulting rule/configuration change before updating the fixture; do not regenerate it merely to silence a failure. +The fixture ``tests/Fixtures/preset-rules.json`` records the installed ECS version and exact reference, fully qualified classes, explicit configuration and risky classification after container boot/skip passes. Unspecified options retain the fixer's defaults at that exact version; the table does not expand those defaults. ``tests/Config/PresetTest.php`` rebuilds that configuration and compares it exactly. It reads this installation's Composer metadata and verifies every fixer's source path inside that exact ECS package. The two Sniffs have exact class/source allowlists; an unknown default Sniff fails the inventory test. PHPCS has no native ``isRisky()`` contract: the snapshot records ``risky: null``, checker ``type: sniff`` and reviewed classifications ``owned-whitespace-only`` and ``native-psr12-whitespace-adapter``. Fixer entries retain native risk checks. This avoids querying the global ``InstalledVersions`` registry after the bundled ECS autoloader has registered its own graph: locally that changed the reported ECS version from ``13.3.8`` to ``dev-main``. Installation separation alone does not isolate that global registry. Dependency updates must review any resulting rule/configuration change before updating the fixture; do not regenerate it merely to silence a failure. +---------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ | Rule | Explicit effective configuration | @@ -160,6 +160,68 @@ Sources: `PHP parameter syntax `_, and `PER language-version exceptions `_. +Owned multiline boolean policy +------------------------------ + +The normative braced wrapper follows `PSR-12 sections 5.1 and 5.3 `__ +and `PER Coding Style 3.0.0 sections 5.1 and 5.3 `__: +first expression on the next line, closing parenthesis on its own line, followed +by the opening brace with one space (``) {``), or ``);`` for do-while. PER 3.0 requires +broken boolean operators at the beginning. The default uses four-space +indentation and LF. Two-space/tab/CRLF native overrides are intentional deviations +from those standards, not alternative compliant defaults. + +``MultilineConditionSpacingSniff`` extends PHPCS's public PSR-12 +``ControlStructureSpacingSniff``, restricting registration to if/elseif/while +(including do-while). It delegates token interpretation and fixes to that native +implementation, deriving indent width from injected ECS spacing and the public +PHPCS File tab-width configuration. No structural diagnostic is skipped. +The normative golden cases use braced forms. Alternative colon syntax remains +unchanged apart from whitespace; its compatibility fixture does not certify +PSR/PER compliance or authorize a semantic conversion into braces. + +The PHPCS 4.0.4 implementation excludes text/comment/ +heredoc tokens from its indentation loop. ECS processes Sniffs before Fixers: +after a newly wrapped first literal is left at column one, native +``StatementIndentationFixer`` can indent that literal and also amplify already +correct continuation offsets. The adapter's compatibility guard uses only +public File metadata and Fixer APIs to add padding **before** that first token, +when it is unindented and the native implementation has not already padded it. +It neither edits literal/comment contents nor realigns existing continuations. +The quoted ``refs/`` guard has exact golden output across native spacing options; +tests also preserve a legitimate stable extra offset. Adapter indentation is +controlled by ``withSpacing()``, not a second consumer option for its inherited +managed ``indent`` property. + +``FastForward\CodeStandards\Sniff\MultilineBooleanConditionSniff`` participates in +the existing ECS PHPCS pass. It uses the installed PHPCS public token/scope +metadata, not another parser, dependency or formatter pass. It changes whitespace +only and reports a failing diagnostic if a required native expression boundary +cannot be established. Its source is ``src/Sniff/MultilineBooleanConditionSniff.php``. + +An ``if``, ``elseif``, ``while`` or ``do-while`` condition whose opening and closing +parentheses occupy different lines must put each symbolic ``&&`` and ``||`` on its +own line. This is a project extension beyond the standards' placement rule and +includes operators inside grouped expressions, call arguments and +array indexes. Function/closure/arrow and anonymous-class declarations are +opaque; separately nested conditional statements are checked independently. +Single-line conditions, expressions outside those conditions and keyword +``and``/``or``/``xor`` retain their native policy. This extension does not claim that +PER requires one boolean part per line. + +The native ECS spacing service supplies two/four-space or tab indentation and +LF/CRLF endings after the consumer callback. Native ``withSkip()`` can disable the +owned checker explicitly; it replaces the skip list, so callers preserve their +other intentional skips. The default cache fingerprints all three installed policy +source paths and bytes (preset plus both Sniffs), in addition to checkout isolation. An explicit +``withCache()`` override remains authoritative. + +CLI regressions cover the reported mixed-line guard, groups/calls/indexes, +declaration boundaries, comments/PHPDoc/strings, native overrides, no-write check, +token-content equivalence and second-pass idempotence. A disposable installation +test changes only its owned checker source and verifies that unchanged consumer +source is rechecked without deleting another checkout's cache. + Golden fixtures prove formatting, check-without-writes, exact fixed bytes and second-pass idempotence. Preservation covers author/package/typed-parameter/ void-return documentation, property visibility, mutable DateTime construction and the existing copyright/header position/year. No fixture executes source. Remaining #5 work: resolve DevTools ECS/Rector/PHPDoc and CoiSA against explicit compatible dependency graphs, compare all rules and header/Finder behavior, and decide separately authorized opt-in profiles. The CoiSA source snapshot does not track a Composer lock; an ignored local lock is not a reproducible published dependency graph. This initial evidence cannot authorize consumer migration. diff --git a/resources/skills/fast-forward-code-standards/references/configuration-and-ci.md b/resources/skills/fast-forward-code-standards/references/configuration-and-ci.md index 7267d9f..922ab10 100644 --- a/resources/skills/fast-forward-code-standards/references/configuration-and-ci.md +++ b/resources/skills/fast-forward-code-standards/references/configuration-and-ci.md @@ -83,3 +83,20 @@ supported by that ref. Keep `pull_request`, read-only contents and hosted runners. No secrets/inherit, privileged target events, implicit plugin installation or automatic fixes. Required-check settings and rollout across consumers are separately authorized. + +## Multiline conditions + +The preset uses native PSR-12 wrapper layout for braced multiline `if`, `elseif`, +`while` and `do-while` conditions, with PER 3.0 leading boolean operators. An owned +whitespace rule additionally puts each symbolic `&&`/`||` on its own line inside +those conditions, including grouped expressions, arguments and indexes. Function, +arrow and anonymous-class declarations remain opaque; single-line conditions +retain native policy. The operator-per-line requirement is a project extension. + +The default uses four spaces/LF. Native `withSpacing()` overrides for two spaces, +tabs or CRLF remain explicit deviations from PSR/PER. A consumer can select native +`withSkip()` entries for `MultilineBooleanConditionSniff` and +`MultilineConditionSpacingSniff`; preserve other intentional skips because that +method replaces the list. The default temporary cache includes the installed +preset/checker paths and bytes, so an owned-rule change cannot reuse an older +passing result. Explicit cache overrides remain authoritative. diff --git a/src/Config/ECSConfig.php b/src/Config/ECSConfig.php index 873a764..30749cb 100644 --- a/src/Config/ECSConfig.php +++ b/src/Config/ECSConfig.php @@ -4,6 +4,8 @@ namespace FastForward\CodeStandards\Config; +use FastForward\CodeStandards\Sniff\MultilineBooleanConditionSniff; +use FastForward\CodeStandards\Sniff\MultilineConditionSpacingSniff; use PhpCsFixer\Fixer\ControlStructure\TrailingCommaInMultilineFixer; use PhpCsFixer\Fixer\FunctionNotation\MethodArgumentSpaceFixer; use PhpCsFixer\Fixer\Phpdoc\GeneralPhpdocAnnotationRemoveFixer; @@ -22,12 +24,13 @@ final class ECSConfig * * @param callable(ECSConfigBuilder):void|null $customize Native rule imports, skips or overrides; null uses the preset unchanged. * @return ECSConfigBuilder The native builder, ready to return from an explicit ECS configuration file. - * @throws RuntimeException When the executing checkout cannot be identified for cache isolation. + * @throws RuntimeException When the checkout or owned policy source cannot be identified for cache isolation. */ public static function configure(?callable $customize = null): ECSConfigBuilder { $config = (new ECSConfigBuilder()) ->withPreparedSets(perCs: true) + ->withRules([MultilineBooleanConditionSniff::class, MultilineConditionSpacingSniff::class]) ->withSkip([ NoSuperfluousPhpdocTagsFixer::class, NoEmptyPhpdocFixer::class, @@ -49,7 +52,7 @@ public static function configure(?callable $customize = null): ECSConfigBuilder return $config; } - /** Keeps ECS invalidation within the executing checkout's temporary cache; native overrides still run last. */ + /** Keeps each checkout and owned policy revision in its own temporary cache; native overrides still run last. */ private static function cacheDirectory(): string { $directory = getcwd(); @@ -57,6 +60,27 @@ private static function cacheDirectory(): string throw new RuntimeException('Cannot determine the checkout directory for the ECS cache.'); } - return sys_get_temp_dir() . '/fast-forward-code-standards/' . hash('sha256', $directory); + return sys_get_temp_dir() . '/fast-forward-code-standards/' . hash('sha256', $directory) . '/' . self::policyFingerprint(); + } + + /** Includes both source bytes and installation paths so an unchanged consumer cannot retain an older owned rule result. */ + private static function policyFingerprint(): string + { + $paths = [ + __FILE__, + dirname(__DIR__) . '/Sniff/MultilineBooleanConditionSniff.php', + dirname(__DIR__) . '/Sniff/MultilineConditionSpacingSniff.php', + ]; + $sources = []; + foreach ($paths as $path) { + $hash = hash_file('sha256', $path); + if (false === $hash) { + throw new RuntimeException('Cannot fingerprint formatter policy source: ' . $path); + } + + $sources[$path] = $hash; + } + + return hash('sha256', serialize($sources)); } } diff --git a/src/Console/Command/RefactorCommand.php b/src/Console/Command/RefactorCommand.php index 5ec154e..4aadaa3 100644 --- a/src/Console/Command/RefactorCommand.php +++ b/src/Console/Command/RefactorCommand.php @@ -73,8 +73,10 @@ private function timeoutSeconds(string $timeout): int { $seconds = (int) $timeout; - if (1 !== preg_match('/^[1-9][0-9]*$/D', $timeout) - || (string) $seconds !== $timeout) { + if ( + 1 !== preg_match('/^[1-9][0-9]*$/D', $timeout) + || (string) $seconds !== $timeout + ) { throw new InvalidArgumentException('--timeout must be a positive whole number of seconds.'); } diff --git a/src/Git/FileSelection.php b/src/Git/FileSelection.php index 5dd24c1..9ef82d0 100644 --- a/src/Git/FileSelection.php +++ b/src/Git/FileSelection.php @@ -44,8 +44,11 @@ public function select(string $directory, ?string $base, ?string $head, ?string ], $directory)); foreach ($allChanges as $path) { // Case-folding hosts can load mixed-case policy names; an extra full scan elsewhere is conservative. - if ((null !== $config && $directory . '/' . $path === $config) - || 1 === preg_match('~(^|/)(composer\.(json|lock)|ecs[^/]*\.php|\.editorconfig|\.gitignore)$|^src/Config/|^config/~i', $path)) { + if ( + (null !== $config + && $directory . '/' . $path === $config) + || 1 === preg_match('~(^|/)(composer\.(json|lock)|ecs[^/]*\.php|\.editorconfig|\.gitignore)$|^src/Config/|^config/~i', $path) + ) { return $this->withoutIgnored($directory, $this->paths($this->process->capture(['git', 'ls-files', '-z'], $directory))); } } diff --git a/src/Refactoring/RectorRunner.php b/src/Refactoring/RectorRunner.php index 6790d45..567ebc4 100644 --- a/src/Refactoring/RectorRunner.php +++ b/src/Refactoring/RectorRunner.php @@ -59,10 +59,12 @@ private function assertRules(string $directory, string $config, float $timeout): throw new InvalidArgumentException('Rector did not return a valid native rule inventory.', previous: $error); } $rules = is_array($inventory) ? ($inventory['rectors'] ?? null) : null; - if (!is_array($rules) + if ( + !is_array($rules) || [] === $rules || !array_is_list($rules) - || count(array_filter($rules, static fn(mixed $rule): bool => is_string($rule) && '' !== $rule)) !== count($rules)) { + || count(array_filter($rules, static fn(mixed $rule): bool => is_string($rule) && '' !== $rule)) !== count($rules) + ) { throw new InvalidArgumentException('Select a native Rector configuration with at least one rule; automatic initialization is disabled.'); } } diff --git a/src/Sniff/MultilineBooleanConditionSniff.php b/src/Sniff/MultilineBooleanConditionSniff.php new file mode 100644 index 0000000..f2cba58 --- /dev/null +++ b/src/Sniff/MultilineBooleanConditionSniff.php @@ -0,0 +1,134 @@ + The while token covers both while and do-while conditions. */ + public function register(): array + { + return [T_IF, T_ELSEIF, T_WHILE]; + } + + /** Checks only an existing multiline condition; application code is never evaluated. */ + public function process(File $phpcsFile, int $stackPtr): void + { + $tokens = $phpcsFile->getTokens(); + $range = $this->conditionRange($phpcsFile, $tokens, $stackPtr); + if (null === $range) { + return; + } + + [$open, $close] = $range; + $indent = $this->baseIndent($tokens, $stackPtr); + $outerDepth = count($tokens[$open]['nested_parenthesis'] ?? []); + + for ($index = $open + 1; $index < $close; ++$index) { + if (in_array($tokens[$index]['code'], [T_FUNCTION, T_CLOSURE, T_FN, T_ANON_CLASS], true)) { + $end = $this->declarationEnd($phpcsFile, $tokens, $index, $close); + if (null === $end) { + return; + } + + $index = $end; + continue; + } + + if (in_array($tokens[$index]['code'], [T_BOOLEAN_OR, T_BOOLEAN_AND], true)) { + $depth = max(1, count($tokens[$index]['nested_parenthesis'] ?? []) - $outerDepth); + $this->splitInlineOperator($phpcsFile, $tokens, $index, $open, $indent, $depth); + } + } + } + + /** + * Finds the bounds of an existing multiline condition. + * + * @param array> $tokens Native PHPCS token records. + * @return array{int, int}|null Validated parentheses, or no eligible condition. + */ + private function conditionRange(File $file, array $tokens, int $index): ?array + { + if (!isset($tokens[$index]['parenthesis_opener'], $tokens[$index]['parenthesis_closer'])) { + $file->addError('Cannot determine the native condition boundary.', $index, 'MissingConditionBoundary'); + + return null; + } + + $open = $tokens[$index]['parenthesis_opener']; + $close = $tokens[$index]['parenthesis_closer']; + + return $tokens[$open]['line'] === $tokens[$close]['line'] ? null : [$open, $close]; + } + + /** + * Locates the end of an opaque function or class expression. + * + * @param array> $tokens Native scopes, including arrow bodies without braces. + * @return int|null The declaration's inclusive end; an unknown boundary is a visible failing diagnostic. + */ + private function declarationEnd(File $file, array $tokens, int $index, int $conditionEnd): ?int + { + $end = $tokens[$index]['scope_closer'] ?? null; + if (null === $end || $end <= $index || $end > $conditionEnd) { + $file->addError('Cannot determine the native function/class expression boundary.', $index, 'MissingExpressionBoundary'); + + return null; + } + + return $end; + } + + /** + * Moves an inline boolean operator to its own continuation line. + * + * @param array> $tokens Original token positions remain stable within the fix changeset. + * @param string $indent Existing statement indentation, retained before native continuation spacing. + * @param int $depth Native parenthesis nesting relative to the condition opener. + */ + private function splitInlineOperator(File $file, array $tokens, int $index, int $open, string $indent, int $depth): void + { + $previous = $file->findPrevious(T_WHITESPACE, $index - 1, $open, true); + if (false === $previous || $tokens[$previous]['line'] < $tokens[$index]['line']) { + return; + } + + if (!$file->addFixableError('Each symbolic boolean part in a multiline condition must start on its own line.', $index, 'InlineBooleanPart')) { + return; + } + + $whitespace = $this->spacing->getLineEnding() . $indent . str_repeat($this->spacing->getIndent(), $depth); + $file->fixer->beginChangeset(); + if (T_WHITESPACE === $tokens[$index - 1]['code']) { + $file->fixer->replaceToken($index - 1, $whitespace); + } else { + $file->fixer->addContentBefore($index, $whitespace); + } + $file->fixer->endChangeset(); + } + + /** + * Reads the statement's existing leading indentation. + * + * @param array> $tokens Statement-line whitespace, including literal tabs. + */ + private function baseIndent(array $tokens, int $index): string + { + $start = $index; + while ($start > 0 && $tokens[$start - 1]['line'] === $tokens[$index]['line']) { + --$start; + } + + return T_WHITESPACE === $tokens[$start]['code'] ? rtrim($tokens[$start]['content'], "\r\n") : ''; + } +} diff --git a/src/Sniff/MultilineConditionSpacingSniff.php b/src/Sniff/MultilineConditionSpacingSniff.php new file mode 100644 index 0000000..534ad38 --- /dev/null +++ b/src/Sniff/MultilineConditionSpacingSniff.php @@ -0,0 +1,67 @@ + Restricts the native policy to this package's reviewed conditional-statement scope. */ + public function register(): array + { + return [T_IF, T_ELSEIF, T_WHILE]; + } + + /** Delegates wrapper placement and indentation to PHPCS using its own public tab-width configuration. */ + public function process(File $phpcsFile, int $stackPtr): void + { + $tab = str_repeat(' ', $phpcsFile->config->tabWidth); + $this->indent = strlen(str_replace("\t", $tab, $this->spacing->getIndent())); + parent::process($phpcsFile, $stackPtr); + $this->completeFirstExpressionIndent($phpcsFile, $stackPtr); + } + + /** + * Pads outside the first token when PHPCS's text/comment exclusion leaves it at column one. + * ECS runs Sniffs before Fixers; otherwise StatementIndentation shifts valid continuations with that token. + */ + private function completeFirstExpressionIndent(File $file, int $statement): void + { + $tokens = $file->getTokens(); + $open = $tokens[$statement]['parenthesis_opener'] ?? null; + $close = $tokens[$statement]['parenthesis_closer'] ?? null; + if ( + null === $open + || null === $close + || $tokens[$open]['line'] === $tokens[$close]['line'] + ) { + return; + } + + $first = $file->findNext(T_WHITESPACE, $open + 1, $close, true); + if ( + false === $first + || 1 !== $tokens[$first]['column'] + || $tokens[$first]['content'] !== $file->fixer->getTokenContent($first) + ) { + return; + } + + $statementStart = $file->findFirstOnLine(T_WHITESPACE, $statement, true); + $padding = str_repeat(' ', $tokens[$statementStart]['column'] + $this->indent - 1); + if ($file->addFixableError('Indent the first expression before the native fixer pass.', $first, 'FirstExpressionIndentCompatibility')) { + $file->fixer->addContentBefore($first, $padding); + } + } +} diff --git a/tests/Config/PresetTest.php b/tests/Config/PresetTest.php index a7babb2..08ea801 100644 --- a/tests/Config/PresetTest.php +++ b/tests/Config/PresetTest.php @@ -4,8 +4,12 @@ namespace FastForward\CodeStandards\Tests\Config; +use FastForward\CodeStandards\Sniff\MultilineBooleanConditionSniff; +use FastForward\CodeStandards\Sniff\MultilineConditionSpacingSniff; +use PHP_CodeSniffer\Sniffs\Sniff; use PhpCsFixer\Fixer\FixerInterface; use PHPUnit\Framework\TestCase; +use Symplify\EasyCodingStandard\Config\ECSConfig; use Symplify\EasyCodingStandard\DependencyInjection\ServiceContainerFactory; /** Captures the actual compiled preset, including upstream configuration and risky-rule classification. */ @@ -24,10 +28,28 @@ public function testCompiledPresetMatchesReviewedRules(): void $container = (new ServiceContainerFactory())->create([dirname(__DIR__, 2) . '/config/ecs.php']); $container->boot(); $configuration = $container->getCheckerConfiguration(); - $rules = []; + $rules = $this->fixerRules($container, $configuration, $packageRoot) + $this->sniffRules($container, $configuration); + + ksort($rules); + $expected = json_decode(file_get_contents(__DIR__ . '/../Fixtures/preset-rules.json'), true, flags: JSON_THROW_ON_ERROR); + self::assertSame($expected, [ + 'ecs_version' => $package['version'], + 'ecs_reference' => $package['source']['reference'], + 'rules' => $rules, + ]); + self::assertCount(count($container->getCheckerClasses()), $rules, 'Every active checker must appear in this snapshot.'); + } + /** + * @param array $configuration Effective native options after skip/boot passes. + * @return array> Native risky classification remains mandatory for every fixer. + */ + private function fixerRules(ECSConfig $container, array $configuration, string $packageRoot): array + { + $rules = []; foreach ($container->findByContract(FixerInterface::class) as $fixer) { $rules[$fixer->getName()] = [ + 'type' => 'fixer', 'class' => $fixer::class, 'configuration' => $configuration[$fixer::class], 'risky' => $fixer->isRisky(), @@ -36,13 +58,35 @@ public function testCompiledPresetMatchesReviewedRules(): void self::assertStringStartsWith($packageRoot . '/', (new \ReflectionClass($fixer))->getFileName()); } - ksort($rules); - $expected = json_decode(file_get_contents(__DIR__ . '/../Fixtures/preset-rules.json'), true, flags: JSON_THROW_ON_ERROR); - self::assertSame($expected, [ - 'ecs_version' => $package['version'], - 'ecs_reference' => $package['source']['reference'], - 'rules' => $rules, - ]); - self::assertCount(count($container->getCheckerClasses()), $rules, 'Every active checker must appear in this snapshot.'); + return $rules; + } + + /** + * @param array $configuration Effective options, including the absence of owned scalar options. + * @return array> PHPCS has no native isRisky method; only the reviewed classes below are allowed. + */ + private function sniffRules(ECSConfig $container, array $configuration): array + { + $sniffs = array_values($container->findByContract(Sniff::class)); + $policies = [ + MultilineBooleanConditionSniff::class => 'owned-whitespace-only', + MultilineConditionSpacingSniff::class => 'native-psr12-whitespace-adapter', + ]; + self::assertSame(array_keys($policies), array_map(static fn(Sniff $sniff): string => $sniff::class, $sniffs), 'Unknown default Sniffs require an explicit policy review.'); + $rules = []; + foreach ($sniffs as $sniff) { + $source = 'src/Sniff/' . (new \ReflectionClass($sniff))->getShortName() . '.php'; + self::assertSame(realpath(dirname(__DIR__, 2) . '/' . $source), (new \ReflectionClass($sniff))->getFileName()); + $rules[$sniff::class] = [ + 'type' => 'sniff', + 'class' => $sniff::class, + 'configuration' => $configuration[$sniff::class], + 'risky' => null, + 'risk_policy' => $policies[$sniff::class], + 'source' => $source, + ]; + } + + return $rules; } } diff --git a/tests/Evaluation/Twig/evaluate.php b/tests/Evaluation/Twig/evaluate.php index 09a5b1c..10b9e1e 100644 --- a/tests/Evaluation/Twig/evaluate.php +++ b/tests/Evaluation/Twig/evaluate.php @@ -77,8 +77,12 @@ function evaluate(string $autoload, string $directory, string $config, string $m $autoload = realpath($autoload); $repository = realpath(dirname(__DIR__, 3)); $temporary = realpath(getenv('TWIG_EVALUATION_TEMP') ?: sys_get_temp_dir()); - if (false === $temporary || !is_dir($temporary) || str_starts_with($temporary . '/', $repository . '/') - || str_starts_with($autoload, $repository . '/')) { + if ( + false === $temporary + || !is_dir($temporary) + || str_starts_with($temporary . '/', $repository . '/') + || str_starts_with($autoload, $repository . '/') + ) { throw new InvalidArgumentException('Evaluation runtime and temporary fixtures must be provisioned outside the checkout.'); } require $autoload; diff --git a/tests/Fixtures/multiline-condition-expected.php.inc b/tests/Fixtures/multiline-condition-expected.php.inc new file mode 100644 index 0000000..86cc81f --- /dev/null +++ b/tests/Fixtures/multiline-condition-expected.php.inc @@ -0,0 +1,20 @@ +directory = realpath(sys_get_temp_dir()) . '/standards-condition-' . bin2hex(random_bytes(8)); + (new Filesystem())->mkdir($this->directory . '/.build/tmp'); + file_put_contents($this->directory . '/.gitignore', "/.build/\n"); + $this->git(['init', '-q']); + } + + /** Removes only the fixture exclusively allocated by this test. */ + protected function tearDown(): void + { + (new Filesystem())->remove($this->directory); + } + + /** The exact mixed-line guard reported in PR55 must fail check without writing and fix deterministically. */ + public function testDefaultCheckRejectsMixedBooleanLayout(): void + { + $source = file_get_contents(__DIR__ . '/../Fixtures/multiline-condition-input.php.inc'); + file_put_contents($this->directory . '/source.php', $source); + $this->git(['add', 'source.php']); + + $check = $this->cli(['check', '--all']); + + self::assertSame(2, $check->getExitCode(), $check->getOutput() . $check->getErrorOutput()); + self::assertSame($source, file_get_contents($this->directory . '/source.php')); + self::assertSame(0, $this->cli(['fix', '--all'])->getExitCode()); + $fixed = file_get_contents($this->directory . '/source.php'); + self::assertSame(file_get_contents(__DIR__ . '/../Fixtures/multiline-condition-expected.php.inc'), $fixed); + self::assertSame(0, $this->cli(['check', '--all'])->getExitCode()); + self::assertSame(0, $this->cli(['fix', '--all'])->getExitCode()); + self::assertSame($fixed, file_get_contents($this->directory . '/source.php')); + } + + /** @return iterable Braced forms prove normative wrapper bytes, not a global standards certificate. */ + public static function bracedWrappers(): iterable + { + yield 'if' => ["if (\$a\n || \$b) {\n return;\n}", "if (\n \$a\n || \$b\n) {\n return;\n}"]; + yield 'elseif' => ["if (\$a) {\n return;\n} elseif (\$b\n || \$c) {\n return;\n}", "if (\$a) {\n return;\n} elseif (\n \$b\n || \$c\n) {\n return;\n}"]; + yield 'while' => ["while (\$a\n || \$b) {\n break;\n}", "while (\n \$a\n || \$b\n) {\n break;\n}"]; + yield 'do-while' => ["do {\n break;\n} while (\$a\n || \$b);", "do {\n break;\n} while (\n \$a\n || \$b\n);"]; + yield 'without boolean' => ["if (check(\n \$a,\n)) {\n return;\n}", "if (\n check(\n \$a,\n )\n) {\n return;\n}"]; + yield 'quoted first expression' => ["if ('a' === \$a\n || \$b) {\n return;\n}", "if (\n 'a' === \$a\n || \$b\n) {\n return;\n}"]; + yield 'comments preserve descriptions' => ["if (/* opening description */ \$a\n || \$b /* closing description */) {\n return;\n}", "if (\n /* opening description */ \$a\n || \$b /* closing description */\n) {\n return;\n}"]; + yield 'leading line comment' => ["if (// first reason\n \$a\n || \$b) {\n return;\n}", "if (\n // first reason\n \$a\n || \$b\n) {\n return;\n}"]; + yield 'nowdoc first expression' => ["if (<<<'TEXT'\n|| && literal unchanged\nTEXT\n || \$a) {\n return;\n}", "if (\n <<<'TEXT'\n|| && literal unchanged\nTEXT\n || \$a\n) {\n return;\n}"]; + } + + /** Native wrapper fixes preserve tokens, comments, grouping and explicit source-only check semantics. */ + #[DataProvider('bracedWrappers')] + public function testNativeBracedWrappersMatchExactReviewedLayout(string $input, string $expected): void + { + $source = "directory . '/source.php', $source); + $this->git(['add', 'source.php']); + $check = $this->cli(['check', '--all']); + + self::assertSame(2, $check->getExitCode(), $check->getOutput() . $check->getErrorOutput()); + self::assertSame($source, file_get_contents($this->directory . '/source.php')); + self::assertSame(0, $this->cli(['fix', '--all'])->getExitCode()); + $fixed = file_get_contents($this->directory . '/source.php'); + self::assertSame("significantTokens($source), $this->significantTokens($fixed)); + self::assertSame(0, $this->cli(['check', '--all'])->getExitCode()); + self::assertSame(0, $this->cli(['fix', '--all'])->getExitCode()); + self::assertSame($fixed, file_get_contents($this->directory . '/source.php')); + } + + /** @return iterable}> Owned source-only fixtures and deliberate scope exclusions. */ + public static function conditions(): iterable + { + yield 'if' => ["if (\$a || \$b\n || \$c\n) { return; }", true, []]; + yield 'elseif' => ["if (\$a) { return; } elseif (\$a || \$b\n || \$c\n) { return; }", true, []]; + yield 'while' => ["while (\$a || \$b\n || \$c\n) { break; }", true, []]; + yield 'do-while' => ["do { break; } while (\$a || \$b\n || \$c\n);", true, []]; + yield 'mixed grouping' => ["if (\$a || (\$b && \$c)\n || \$d\n) { return; }", true, []]; + yield 'multiline call cannot mask inline parts' => ["if (check(\n \$first,\n \$second,\n \$third,\n \$fourth,\n) || \$a || \$b || \$c\n || \$d\n) { return; }", true, []]; + yield 'call argument belongs to the condition' => ["if (check(\$a || \$b)\n || \$outer\n) { return; }", true, []]; + yield 'array index belongs to the condition' => ["if (\$values[\$a || \$b]\n || \$outer\n) { return; }", true, []]; + yield 'arrow expression is opaque' => ["if ((fn() => (\$a || \$b) && \$inner)() || \$c\n || \$d\n) { return; }", true, ['fn() => ($a || $b) && $inner']]; + yield 'nested arrow expressions are opaque' => ["if ((fn() => fn() => \$a || \$b)() || \$c\n || \$d\n) { return; }", true, ['fn() => fn() => $a || $b']]; + yield 'closure body is opaque' => ["if ((function () use (\$a, \$b) { return \$a || \$b; })() || \$c\n || \$d\n) { return; }", true, ['return $a || $b;']]; + yield 'anonymous class body is opaque' => ["if (new class { public function value() { return \$this->a || \$this->b; } } || \$outer\n || \$last\n) { return; }", true, ['return $this->a || $this->b;']]; + yield 'descriptions comments and strings' => ["/** Useful description and {@inheritDoc}. */\nif (check('|| && text') /* keep exactly */ || \$b // keep line note\n || \$c\n) { return; }", true, ['/** Useful description and {@inheritDoc}. */', '/* keep exactly */', '// keep line note', "'|| && text'"]]; + yield 'single-line condition remains single-line' => ['if ($a || $b && $c) { return; }', false, ['if ($a || $b && $c)']]; + yield 'arrow-only condition contains no eligible outer operator' => ["if (\n fn() => \$a || \$b\n) { return; }", false, ['fn() => $a || $b']]; + yield 'logical keywords are not this policy' => ["if (\$a or \$b\n or \$c\n) { return; }", false, []]; + yield 'non-condition expression remains native policy' => ["\$result = \$a || \$b\n || \$c;", false, []]; + } + + /** + * Proves the new policy rather than unrelated PER differences; baseline uses an explicit native skip. + * + * @param list $preserved Exact information or opaque body content that the new policy must retain. + */ + #[DataProvider('conditions')] + public function testConditionScopesPreserveTokensAndReachStableOutput(string $body, bool $mustChange, array $preserved): void + { + $before = $this->normalizedSource("cli(['check', '--all']); + + self::assertSame($mustChange ? 2 : 0, $check->getExitCode(), $check->getOutput() . $check->getErrorOutput()); + self::assertSame($before, file_get_contents($this->directory . '/source.php')); + self::assertSame(0, $this->cli(['fix', '--all'])->getExitCode()); + $after = file_get_contents($this->directory . '/source.php'); + self::assertSame($mustChange, $before !== $after); + self::assertSame($this->significantTokens($before), $this->significantTokens($after)); + foreach ($preserved as $text) { + self::assertStringContainsString($text, $after); + } + + self::assertSame(0, $this->cli(['check', '--all'])->getExitCode()); + self::assertSame(0, $this->cli(['fix', '--all'])->getExitCode()); + self::assertSame($after, file_get_contents($this->directory . '/source.php')); + } + + /** @return iterable All native ECS indentation/line-ending combinations. */ + public static function spacing(): iterable + { + foreach (['two spaces' => ' ', 'four spaces' => ' ', 'tabs' => "\t"] as $name => $indent) { + yield $name . ' LF' => [$indent, "\n"]; + yield $name . ' CRLF' => [$indent, "\r\n"]; + } + } + + /** The injected native spacing value must reflect the consumer callback, not a separate Sniff default. */ + #[DataProvider('spacing')] + public function testNativeSpacingOverrideRemainsAuthoritative(string $indent, string $lineEnding): void + { + $source = file_get_contents(__DIR__ . '/../Fixtures/multiline-condition-input.php.inc'); + $source = $this->withSpacing($source, $indent, $lineEnding); + file_put_contents($this->directory . '/source.php', $source); + $this->git(['add', 'source.php']); + $this->configure('$config->withSpacing(indentation: ' . var_export($indent, true) . ', lineEnding: ' . var_export($lineEnding, true) . ');'); + $arguments = ['--all', '--config=.build/ecs.php']; + $check = $this->cli(['check', ...$arguments]); + + self::assertSame(2, $check->getExitCode(), $check->getOutput() . $check->getErrorOutput()); + self::assertSame($source, file_get_contents($this->directory . '/source.php')); + self::assertSame(0, $this->cli(['fix', ...$arguments])->getExitCode()); + $fixed = file_get_contents($this->directory . '/source.php'); + $golden = file_get_contents(__DIR__ . '/../Fixtures/multiline-condition-expected.php.inc'); + self::assertSame($this->withSpacing($golden, $indent, $lineEnding), $fixed); + self::assertSame(0, $this->cli(['check', ...$arguments])->getExitCode()); + self::assertSame(0, $this->cli(['fix', ...$arguments])->getExitCode()); + self::assertSame($fixed, file_get_contents($this->directory . '/source.php')); + } + + /** The exact quoted-reference guard must not amplify continuation offsets in the Sniff-then-Fixer pipeline. */ + #[DataProvider('spacing')] + public function testQuotedFirstExpressionDoesNotAmplifyContinuationIndent(string $indent, string $lineEnding): void + { + $source = $this->withSpacing(file_get_contents(__DIR__ . '/../Fixtures/quoted-condition-input.php.inc'), $indent, $lineEnding); + file_put_contents($this->directory . '/source.php', $source); + $this->git(['add', 'source.php']); + $this->configure('$config->withSpacing(indentation: ' . var_export($indent, true) . ', lineEnding: ' . var_export($lineEnding, true) . ');'); + $arguments = ['--all', '--config=.build/ecs.php']; + $check = $this->cli(['check', ...$arguments]); + + self::assertSame(2, $check->getExitCode(), $check->getOutput() . $check->getErrorOutput()); + self::assertSame($source, file_get_contents($this->directory . '/source.php')); + self::assertSame(0, $this->cli(['fix', ...$arguments])->getExitCode()); + $fixed = file_get_contents($this->directory . '/source.php'); + self::assertSame($this->withSpacing(file_get_contents(__DIR__ . '/../Fixtures/quoted-condition-expected.php.inc'), $indent, $lineEnding), $fixed); + self::assertSame($this->significantTokens($source), $this->significantTokens($fixed)); + self::assertSame(0, $this->cli(['check', ...$arguments])->getExitCode()); + self::assertSame(0, $this->cli(['fix', ...$arguments])->getExitCode()); + self::assertSame($fixed, file_get_contents($this->directory . '/source.php')); + } + + /** A legitimate stable extra offset must survive; the compatibility guard only pads an unindented first token. */ + public function testQuotedConditionPreservesExistingStableContinuationOffset(): void + { + $source = str_replace(' ||', ' ||', file_get_contents(__DIR__ . '/../Fixtures/quoted-condition-expected.php.inc')); + file_put_contents($this->directory . '/source.php', $source); + $this->git(['add', 'source.php']); + foreach (['check', 'fix', 'check', 'fix'] as $mode) { + $result = $this->cli([$mode, '--all']); + self::assertSame(0, $result->getExitCode(), $result->getOutput() . $result->getErrorOutput()); + self::assertSame($source, file_get_contents($this->directory . '/source.php')); + } + } + + /** Consumer rule skips remain native, explicit and last in precedence. */ + public function testNativeSkipCanDisableTheOwnedPolicy(): void + { + $source = file_get_contents(__DIR__ . '/../Fixtures/multiline-condition-input.php.inc'); + $source = str_replace(' if (!str_starts_with', " if (\n !str_starts_with", $source); + file_put_contents($this->directory . '/source.php', $source); + $this->git(['add', 'source.php']); + $this->configure('$config->withSkip([FastForward\\CodeStandards\\Sniff\\MultilineBooleanConditionSniff::class]);'); + foreach (['check', 'fix', 'check'] as $mode) { + $result = $this->cli([$mode, '--all', '--config=.build/ecs.php']); + self::assertSame(0, $result->getExitCode(), $result->getOutput() . $result->getErrorOutput()); + self::assertSame($source, file_get_contents($this->directory . '/source.php')); + } + } + + /** Existing continuation offsets remain native policy; this Sniff only separates inline operators. */ + public function testExistingContinuationOffsetIsNotSilentlyRealigned(): void + { + $source = file_get_contents(__DIR__ . '/../Fixtures/multiline-condition-input.php.inc'); + file_put_contents($this->directory . '/source.php', $source); + $this->git(['add', 'source.php']); + $spacing = '$config->withSpacing(indentation: " ");'; + $arguments = ['--all', '--config=.build/ecs.php']; + $this->configure($spacing . '$config->withSkip([FastForward\\CodeStandards\\Sniff\\MultilineBooleanConditionSniff::class]);'); + self::assertSame(0, $this->cli(['fix', ...$arguments])->getExitCode()); + $native = file_get_contents($this->directory . '/source.php'); + self::assertStringContainsString("\n || !is_readable(\$vendorDirectory", $native); + + $this->configure($spacing); + self::assertSame(2, $this->cli(['check', ...$arguments])->getExitCode()); + self::assertSame($native, file_get_contents($this->directory . '/source.php')); + self::assertSame(0, $this->cli(['fix', ...$arguments])->getExitCode()); + $fixed = file_get_contents($this->directory . '/source.php'); + self::assertStringContainsString("\n || !is_readable(\$vendorDirectory", $fixed); + self::assertSame($this->significantTokens($native), $this->significantTokens($fixed)); + self::assertSame(0, $this->cli(['check', ...$arguments])->getExitCode()); + self::assertSame(0, $this->cli(['fix', ...$arguments])->getExitCode()); + self::assertSame($fixed, file_get_contents($this->directory . '/source.php')); + } + + /** Source changes in an installed owned checker invalidate unchanged consumer files without clearing other caches. */ + public function testOwnedRuleRevisionInvalidatesDefaultCache(): void + { + $root = dirname(__DIR__, 2); + $copy = $this->directory . '/.build/tool'; + $filesystem = new Filesystem(); + $filesystem->mkdir([$copy . '/src/Config', $copy . '/src/Sniff', $copy . '/vendor/symplify', $copy . '/vendor/bin']); + copy($root . '/src/Config/ECSConfig.php', $copy . '/src/Config/ECSConfig.php'); + copy($root . '/src/Sniff/MultilineBooleanConditionSniff.php', $copy . '/src/Sniff/MultilineBooleanConditionSniff.php'); + copy($root . '/src/Sniff/MultilineConditionSpacingSniff.php', $copy . '/src/Sniff/MultilineConditionSpacingSniff.php'); + symlink(realpath($root . '/vendor/symplify/easy-coding-standard'), $copy . '/vendor/symplify/easy-coding-standard'); + file_put_contents($copy . '/vendor/autoload.php', 'addPsr4("FastForward\\\\CodeStandards\\\\", ' . var_export($copy . '/src', true) . '); $loader->register(true);'); + file_put_contents($copy . '/vendor/bin/code-standards', 'directory . '/source.php', $source); + $this->git(['add', 'source.php']); + $command = [PHP_BINARY, $copy . '/vendor/bin/code-standards', 'check', '--all']; + $first = $this->process($command); + self::assertSame(0, $first->getExitCode(), $first->getOutput() . $first->getErrorOutput()); + + // Replace only the disposable installation's checker, retaining its class/options and the consumer source. + file_put_contents($copy . '/src/Sniff/MultilineBooleanConditionSniff.php', 'addError("owned policy revision", $index, "SyntheticRevision"); } }'); + $second = $this->process($command); + self::assertSame(2, $second->getExitCode(), $second->getOutput() . $second->getErrorOutput()); + self::assertStringContainsString('owned policy revision', $second->getOutput()); + self::assertSame($source, file_get_contents($this->directory . '/source.php')); + } + + /** Normalizes only native upstream style first so later differences belong to the owned rule. */ + private function normalizedSource(string $source): string + { + file_put_contents($this->directory . '/source.php', $source); + $this->git(['add', 'source.php']); + $this->configure('$config->withSkip([FastForward\\CodeStandards\\Sniff\\MultilineBooleanConditionSniff::class]);'); + foreach (['fix', 'check'] as $mode) { + $result = $this->cli([$mode, '--all', '--config=.build/ecs.php']); + self::assertSame(0, $result->getExitCode(), $result->getOutput() . $result->getErrorOutput()); + } + + return file_get_contents($this->directory . '/source.php'); + } + + /** @return list Token content/kind equality excludes only whitespace and line-number metadata. */ + private function significantTokens(string $source): array + { + $tokens = array_filter(token_get_all($source, TOKEN_PARSE), static fn($token): bool => !is_array($token) || T_WHITESPACE !== $token[0]); + + return array_values(array_map(static fn($token) => is_array($token) ? [$token[0], $token[1]] : $token, $tokens)); + } + + /** Expresses each existing fixture indentation level using the consumer's native spacing. */ + private function withSpacing(string $source, string $indent, string $lineEnding): string + { + $source = preg_replace_callback('/^(?: {4})+/m', static fn(array $match): string => str_repeat($indent, intdiv(strlen($match[0]), 4)), $source); + + return str_replace("\n", $lineEnding, $source); + } + + /** Writes explicitly selected trusted consumer configuration under the ignored fixture build directory. */ + private function configure(string $body): void + { + file_put_contents($this->directory . '/.build/ecs.php', ' $arguments Explicit user-visible CLI arguments. */ + private function cli(array $arguments): Process + { + return $this->process([PHP_BINARY, dirname(__DIR__, 2) . '/bin/code-standards', ...$arguments]); + } + + /** @param list $command Executable and separate arguments for this exclusive fixture. */ + private function process(array $command): Process + { + $process = new Process($command, $this->directory, ['TMPDIR' => $this->directory . '/.build/tmp'], timeout: 60); + $process->run(); + + return $process; + } + + /** @param list $arguments Git operations scoped exclusively to this synthetic repository. */ + private function git(array $arguments): void + { + (new Process(['git', ...$arguments], $this->directory, timeout: 30))->mustRun(); + } +} diff --git a/tests/ReleaseChannel/ReleaseChannelFixture.php b/tests/ReleaseChannel/ReleaseChannelFixture.php index d33bc5c..938366f 100644 --- a/tests/ReleaseChannel/ReleaseChannelFixture.php +++ b/tests/ReleaseChannel/ReleaseChannelFixture.php @@ -166,15 +166,20 @@ private function respond(string $method, string $url, array $options): MockRespo $current = $this->git(['show-ref', '--verify', '--hash', $change['name']], true); if (null !== $this->mutationErrors) { $data = ['errors' => $this->mutationErrors]; - } elseif (null !== $current && null === $this->git( - ['merge-base', '--is-ancestor', $current, $change['afterOid']], - true, - )) { + } elseif ( + null !== $current + && null === $this->git( + ['merge-base', '--is-ancestor', $current, $change['afterOid']], + true, + ) + ) { $data = ['errors' => [['message' => 'Synthetic non-fast-forward rejection']]]; - } elseif (null === $this->git( - ['update-ref', $change['name'], $change['afterOid'], $change['beforeOid']], - true, - )) { + } elseif ( + null === $this->git( + ['update-ref', $change['name'], $change['afterOid'], $change['beforeOid']], + true, + ) + ) { $data = ['errors' => [['message' => 'Synthetic old-OID lease rejection']]]; } else { $this->mutations[] = $change;