diff --git a/.changelog/conservative-parameter-commas.md b/.changelog/conservative-parameter-commas.md new file mode 100644 index 0000000..e2366f6 --- /dev/null +++ b/.changelog/conservative-parameter-commas.md @@ -0,0 +1,6 @@ +--- +category: fixed +type: patch +--- + +Avoid introducing PHP 8.0 parameter-list commas from the formatter's host runtime by default; preserve native consumer overrides and existing commas. diff --git a/docs/resolved-preset.md b/docs/resolved-preset.md index ce12534..daf567b 100644 --- a/docs/resolved-preset.md +++ b/docs/resolved-preset.md @@ -79,7 +79,7 @@ updating the fixture; do not regenerate it merely to silence a failure. | `switch_case_semicolon_to_colon` | `default` | | `switch_case_space` | `default` | | `ternary_operator_spaces` | `default` | -| `trailing_comma_in_multiline` | `{"after_heredoc":true,"elements":["arguments","array_destructuring","arrays","match","parameters"]}` | +| `trailing_comma_in_multiline` | `{"after_heredoc":true,"elements":["arguments","array_destructuring","arrays","match"]}` | | `types_spaces` | `default` | | `unary_operator_spaces` | `{"only_dec_inc":true}` | @@ -89,6 +89,28 @@ must not be presented as proof that broader profiles are safe. Native ECS builde `withSkip()` replaces the builder’s skip list, while configured rules of the same class use the last configuration. Consumers own their explicit overrides. +## Consumer language compatibility + +The formatter's PHP runtime is separate from the consumer's supported PHP versions. +The default deliberately excludes `parameters` from the native multiline trailing +comma rule: commas in parameter declarations require PHP 8.0, while running the +formatter on PHP 8.3 or later does not authorize upgrading a consumer's syntax. +Arguments, arrays, destructuring and match retain the selected native behavior, +including `after_heredoc`. Existing parameter commas are preserved, not removed. +This conservative exception is not a complete PER conformance claim or a PHP 7.4 +compatibility certification. Syntax checks use the executing PHP binary. + +A consumer that has reviewed PHP 8.0-or-later compatibility can explicitly include +`parameters` through `ECSConfig::configure()`'s native callback. The callback runs +last; it owns the complete `TrailingCommaInMultilineFixer` configuration supplied +through `withConfiguredRule()`. No target version is inferred from the host. +Explicit target selection and execution on consumer PHP versions remain part of +the open compatibility and policy work in issues #2 and #5. + +Sources: [PHP parameter syntax](https://www.php.net/manual/en/functions.arguments.php), +[native fixer options](https://cs.symfony.com/doc/rules/control_structure/trailing_comma_in_multiline.html), +and [PER language-version exceptions](https://www.php-fig.org/per/coding-style/#previous-language-versions). + 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 diff --git a/src/Config/ECSConfig.php b/src/Config/ECSConfig.php index 81d6701..873a764 100644 --- a/src/Config/ECSConfig.php +++ b/src/Config/ECSConfig.php @@ -4,6 +4,7 @@ namespace FastForward\CodeStandards\Config; +use PhpCsFixer\Fixer\ControlStructure\TrailingCommaInMultilineFixer; use PhpCsFixer\Fixer\FunctionNotation\MethodArgumentSpaceFixer; use PhpCsFixer\Fixer\Phpdoc\GeneralPhpdocAnnotationRemoveFixer; use PhpCsFixer\Fixer\Phpdoc\NoEmptyPhpdocFixer; @@ -33,6 +34,10 @@ public static function configure(?callable $customize = null): ECSConfigBuilder PhpdocNoEmptyReturnFixer::class, GeneralPhpdocAnnotationRemoveFixer::class, ]) + ->withConfiguredRule(TrailingCommaInMultilineFixer::class, [ + 'after_heredoc' => true, + 'elements' => ['arguments', 'array_destructuring', 'arrays', 'match'], + ]) ->withConfiguredRule(MethodArgumentSpaceFixer::class, ['on_multiline' => 'ignore']) ->withConfiguredRule(PhpdocAlignFixer::class, ['align' => 'left']) ->withCache(self::cacheDirectory()) diff --git a/tests/Fixtures/consumer-language.php.inc b/tests/Fixtures/consumer-language.php.inc new file mode 100644 index 0000000..28dcdb5 --- /dev/null +++ b/tests/Fixtures/consumer-language.php.inc @@ -0,0 +1,27 @@ +run(); +(new class() {})->run(); diff --git a/tests/Fixtures/preset-rules.json b/tests/Fixtures/preset-rules.json index 1d7e4dc..7abcf0e 100644 --- a/tests/Fixtures/preset-rules.json +++ b/tests/Fixtures/preset-rules.json @@ -393,8 +393,7 @@ "arguments", "array_destructuring", "arrays", - "match", - "parameters" + "match" ] }, "risky": false diff --git a/tests/Integration/StandardsTest.php b/tests/Integration/StandardsTest.php index fbaf3c3..176caec 100644 --- a/tests/Integration/StandardsTest.php +++ b/tests/Integration/StandardsTest.php @@ -156,6 +156,72 @@ public function testStyleCheckDoesNotWriteAndExplicitFixIsIdempotent(): void self::assertSame($fixed, file_get_contents($this->directory . '/' . $path)); } + /** Preserves older parameter syntax by default, existing commas and explicit fix/check boundaries. */ + public function testDefaultFormattingDoesNotIntroduceParameterCommasFromTheHostRuntime(): void + { + $source = file_get_contents(__DIR__ . '/../Fixtures/consumer-language.php.inc'); + $this->put('source.php', $source); + $this->commit(); + + $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(['source.php'], $this->trackedAndUntrackedSources()); + + $fix = $this->cli(['fix', '--all']); + self::assertSame(0, $fix->getExitCode(), $fix->getOutput() . $fix->getErrorOutput()); + $fixed = file_get_contents($this->directory . '/source.php'); + self::assertStringContainsString("?string \$name = null\n", $fixed); + self::assertStringContainsString("string \$label\n", $fixed); + self::assertStringContainsString("string \$name,\n", $fixed); + self::assertStringContainsString("'array value',\n", $fixed); + self::assertStringContainsString("'argument value',\n", $fixed); + self::assertStringContainsString('(new Example())->run();', $fixed); + self::assertStringContainsString('(new class', $fixed); + self::assertStringContainsString('})->run();', $fixed); + + foreach (['check', 'fix', 'check'] as $command) { + $result = $this->cli([$command, '--all']); + self::assertSame(0, $result->getExitCode(), $result->getOutput() . $result->getErrorOutput()); + self::assertSame($fixed, file_get_contents($this->directory . '/source.php')); + } + } + + /** Allows an explicitly reviewed native override to select parameter commas for PHP 8.0 or later. */ + public function testConsumerCanExplicitlyOverrideTheConservativeParameterCommaPolicy(): void + { + $source = file_get_contents(__DIR__ . '/../Fixtures/consumer-language.php.inc'); + $this->put('source.php', $source); + $this->put('.build/ecs.php', <<<'PHP' +withConfiguredRule( + \PhpCsFixer\Fixer\ControlStructure\TrailingCommaInMultilineFixer::class, + ['after_heredoc' => true, 'elements' => ['parameters']], + ); + }, +); +PHP); + $this->commit(); + + $check = $this->cli(['check', '--all', '--config=.build/ecs.php']); + self::assertSame(2, $check->getExitCode(), $check->getOutput() . $check->getErrorOutput()); + self::assertSame($source, file_get_contents($this->directory . '/source.php')); + + $fix = $this->cli(['fix', '--all', '--config=.build/ecs.php']); + self::assertSame(0, $fix->getExitCode(), $fix->getOutput() . $fix->getErrorOutput()); + $fixed = file_get_contents($this->directory . '/source.php'); + self::assertStringContainsString("?string \$name = null,\n", $fixed); + self::assertStringContainsString("string \$label,\n", $fixed); + self::assertStringNotContainsString("'array value',\n", $fixed); + self::assertStringNotContainsString("'argument value',\n", $fixed); + self::assertSame(0, $this->cli(['check', '--all', '--config=.build/ecs.php'])->getExitCode()); + self::assertSame(0, $this->cli(['fix', '--all', '--config=.build/ecs.php'])->getExitCode()); + self::assertSame($fixed, file_get_contents($this->directory . '/source.php')); + } + /** Proves parsing never executes selected source and syntax failures leave its bytes intact. */ public function testSyntaxIsParsedWithoutExecutingSource(): void {