diff --git a/.changelog/normative-interface-contracts.md b/.changelog/normative-interface-contracts.md new file mode 100644 index 0000000..e816948 --- /dev/null +++ b/.changelog/normative-interface-contracts.md @@ -0,0 +1,6 @@ +--- +category: fixed +type: patch +--- + +Document the process interfaces' normative execution, output and failure contracts, explain capture/run/result selection, and preserve descriptive PHPDoc and inheritance markers through qualified native formatting and refactoring. diff --git a/docs/api/index.rst b/docs/api/index.rst index 8885569..e9da497 100644 --- a/docs/api/index.rst +++ b/docs/api/index.rst @@ -5,3 +5,4 @@ API and extensions :maxdepth: 1 configuration + ../process-contracts diff --git a/docs/process-contracts.rst b/docs/process-contracts.rst new file mode 100644 index 0000000..c234a64 --- /dev/null +++ b/docs/process-contracts.rst @@ -0,0 +1,71 @@ +Process transport contracts +=========================== + +``ProcessRunner`` distinguishes successful output capture from command status. +``ResultProcessRunner`` optionally adds separate raw output channels and status. + +Normative language +------------------ + +Uppercase ``MUST``, ``MUST NOT``, ``SHOULD``, ``SHOULD NOT`` and ``MAY`` follow +`RFC 2119 `__ and +`RFC 8174 `__; lowercase words retain +their ordinary meaning. Use requirement terms sparingly for observable contracts. +Interface descriptions state semantic obligations, not implementation details. + +Choosing an operation +--------------------- + +=================== =========================== ============================= +Operation Return value Completed nonzero exit +=================== =========================== ============================= +``capture`` Successful stdout Raises an exception +``run`` Completed exit status Returned unchanged +``captureResult`` Status, stdout and stderr Returned as data +=================== =========================== ============================= + +Use ``capture`` for output consumed as data, such as a Git path list or Rector +JSON inventory. Use ``run`` to propagate tool status, with diagnostics permitted. +Use ``captureResult`` for raw stdout/stderr and status without streaming; a +nonzero status remains data. Failure before completion raises an exception. + +.. code-block:: php + + $paths = $process->capture(['git', 'ls-files', '-z'], $directory); + $status = $process->run([PHP_BINARY, '-n', '-l', $file], $directory); + + if (!$process instanceof \FastForward\CodeStandards\Process\ResultProcessRunner) { + throw new \RuntimeException('This transport does not support result capture.'); + } + + $result = $process->captureResult([PHP_BINARY, '-n', '-l', $file], $directory); + // Interpret $result->exitCode, stdout and stderr according to the selected tool. + +The examples assume an injected transport, existing working directory and +selected source file. Check the optional capability before using an arbitrary +``ProcessRunner`` override. Result capture does not parse ECS/Rector reports. + +Native transport +---------------- + +``NativeProcessRunner`` uses Symfony Process. It rejects invalid timeouts before +launch and keeps the parent's working directory unchanged. Declared defaults +are 120 seconds for ``capture`` and 300 for ``run``/``captureResult``. + +Its ``run`` forwards raw stdout/stderr chunks to the optional sink; without one +it is silent. Capture operations do not use the sink. Symfony retains buffers, +so sink delivery is not a bounded-memory guarantee. Execution and sink exceptions +can propagate. + +The native child environment excludes ``GH_TOKEN``, ``GITHUB_TOKEN`` and +``COMPOSER_AUTH``. This is not a sandbox or an empty environment. Callers own +command authority: an explicitly selected shell, application or configuration +can execute code and change files. Capturing output does not make it read-only. + +Preservation and extension +-------------------------- + +Golden fixtures exercise descriptive contracts and inheritance markers through +read-only checking, explicit fixing and idempotence. A native Rector regression +changes array syntax while retaining the same documentation. Consumer-selected +semantic rules can change PHPDoc; this is not a claim about every configuration. diff --git a/src/Process/ProcessRunner.php b/src/Process/ProcessRunner.php index a4e7bda..47d80a2 100644 --- a/src/Process/ProcessRunner.php +++ b/src/Process/ProcessRunner.php @@ -4,18 +4,33 @@ namespace FastForward\CodeStandards\Process; -/** Executes bounded argument arrays without shell interpolation. */ +/** Executes bounded commands with literal arguments. */ interface ProcessRunner { /** - * @param list $arguments Returns stdout or throws on failure. - * @param positive-float $timeout Finite maximum seconds for this subprocess; defaults retain Git's existing limit. + * Captures successful standard output. + * + * A nonzero exit MUST raise an exception rather than return successful output. + * + * @param list $arguments Executable followed by literal arguments, without shell interpolation. + * @param string $directory Child working directory. + * @param float $timeout Positive, finite execution limit in seconds. + * @return string Successful stdout, excluding stderr diagnostics. + * @throws \Throwable When execution cannot complete successfully. */ public function capture(array $arguments, string $directory, float $timeout = 120): string; /** - * @param list $arguments Streams diagnostics and returns the actual exit code. - * @param positive-float $timeout Finite maximum seconds for this subprocess; default formatting limit is unchanged. + * Runs a command and returns its exit status. + * + * Completed statuses, including nonzero ones, MUST be returned unchanged. + * Diagnostics MAY be emitted; failure before completion MUST raise an exception. + * + * @param list $arguments Executable followed by literal arguments, without shell interpolation. + * @param string $directory Child working directory. + * @param float $timeout Positive, finite execution limit in seconds. + * @return int Completed command status; interpreting tool-specific outcomes belongs to the caller. + * @throws \Throwable When execution cannot complete. */ public function run(array $arguments, string $directory, float $timeout = 300): int; } diff --git a/src/Process/ResultProcessRunner.php b/src/Process/ResultProcessRunner.php index f5c08c1..73ed336 100644 --- a/src/Process/ResultProcessRunner.php +++ b/src/Process/ResultProcessRunner.php @@ -4,18 +4,20 @@ namespace FastForward\CodeStandards\Process; -/** Optional lossless capture capability; existing ProcessRunner implementations remain valid. */ +/** Adds optional capture of output and status as data. */ interface ResultProcessRunner extends ProcessRunner { /** - * Captures a completed process without streaming or throwing merely for a nonzero exit. + * Captures output and status as data. * - * @param list $arguments Literal executable and argument vector, without shell interpolation. - * @param string $directory Explicit child working directory; the caller's directory remains unchanged. - * @param positive-float $timeout Finite maximum seconds using the same transport bounds as run(). - * @return ProcessResult Exact native status and separate output bytes, without a clean/failure interpretation. - * @throws \InvalidArgumentException When the timeout is nonfinite or not positive. - * @throws \Symfony\Component\Process\Exception\RuntimeException When execution cannot complete, including startup or timeout failure. + * Stdout and stderr MUST remain separate, unmodified byte strings, without streaming. + * A completed nonzero status MUST remain data; failure before completion MUST raise an exception. + * + * @param list $arguments Executable followed by literal arguments, without shell interpolation. + * @param string $directory Child working directory, leaving the caller's unchanged. + * @param float $timeout Positive, finite execution limit in seconds. + * @return ProcessResult Completed result, without interpreting diagnostics or classifying success. + * @throws \Throwable When execution cannot complete or the timeout is invalid. */ public function captureResult(array $arguments, string $directory, float $timeout = 300): ProcessResult; } diff --git a/tests/Fixtures/normative-contract.after.php.inc b/tests/Fixtures/normative-contract.after.php.inc new file mode 100644 index 0000000..52e31cc --- /dev/null +++ b/tests/Fixtures/normative-contract.after.php.inc @@ -0,0 +1,34 @@ + Exactly one unchanged value, without normalization. + * @throws \InvalidArgumentException When the supplied value is negative. + */ + public function values(int $value): array; +} + +/** Supplies the inherited contract without duplicating its requirements. */ +final class NormativeImplementation implements NormativeContract +{ + /** {@inheritDoc} */ + public function values(int $value): array + { + if ($value < 0) { + throw new \InvalidArgumentException('Value must not be negative.'); + } + + return [$value]; + } +} diff --git a/tests/Fixtures/normative-contract.before.php.inc b/tests/Fixtures/normative-contract.before.php.inc new file mode 100644 index 0000000..f41da12 --- /dev/null +++ b/tests/Fixtures/normative-contract.before.php.inc @@ -0,0 +1,34 @@ + Exactly one unchanged value, without normalization. + * @throws \InvalidArgumentException When the supplied value is negative. + */ + public function values(int $value): array; +} + +/** Supplies the inherited contract without duplicating its requirements. */ +final class NormativeImplementation implements NormativeContract +{ + /** {@inheritDoc} */ + public function values(int $value): array + { + if ($value < 0) { + throw new \InvalidArgumentException('Value must not be negative.'); + } + + return array($value); + } +} diff --git a/tests/Integration/RectorTest.php b/tests/Integration/RectorTest.php index 85df401..f6c92a1 100644 --- a/tests/Integration/RectorTest.php +++ b/tests/Integration/RectorTest.php @@ -51,6 +51,30 @@ public function testPreviewApplyAndSecondPassWithExplicitNativeConfiguration(): self::assertSame($config, file_get_contents($this->directory . '/policy.php')); } + /** Preserves interface documentation during refactoring. */ + public function testNativeArrayRulePreservesNormativeInterfaceDocumentation(): void + { + $fixtures = dirname(__DIR__) . '/Fixtures/'; + $source = file_get_contents($fixtures . 'normative-contract.before.php.inc'); + $expected = file_get_contents($fixtures . 'normative-contract.after.php.inc'); + self::assertIsString($source); + self::assertIsString($expected); + file_put_contents($this->directory . '/source.php', $source); + $this->configuration('\\Rector\\Php54\\Rector\\Array_\\LongArrayToShortArrayRector', 70400); + $before = $this->ownedBytes(); + + $preview = $this->cli(); + self::assertSame(2, $preview->getExitCode(), $preview->getOutput() . $preview->getErrorOutput()); + self::assertSame($before, $this->ownedBytes()); + $apply = $this->cli(['--apply']); + self::assertSame(0, $apply->getExitCode(), $apply->getOutput() . $apply->getErrorOutput()); + self::assertSame($expected, file_get_contents($this->directory . '/source.php')); + $after = $this->ownedBytes(); + self::assertSame(0, $this->cli()->getExitCode()); + self::assertSame(0, $this->cli(['--apply'])->getExitCode()); + self::assertSame($after, $this->ownedBytes()); + } + /** Empty native custom config is rejected before Rector's successful-but-writing initializer. */ public function testEmptyConfigDoesNotGenerateRectorFile(): void { diff --git a/tests/Integration/StandardsTest.php b/tests/Integration/StandardsTest.php index 176caec..ba4c308 100644 --- a/tests/Integration/StandardsTest.php +++ b/tests/Integration/StandardsTest.php @@ -365,7 +365,7 @@ public function testGoldenPresetIsReadOnlyUntilFixAndPreservesContracts(): void $fixtures = dirname(__DIR__) . '/Fixtures/'; $original = []; - foreach (['style', 'preservation', 'documentation'] as $name) { + foreach (['style', 'preservation', 'documentation', 'normative-contract'] as $name) { $original[$name] = file_get_contents($fixtures . $name . '.before.php.inc'); $this->put($name . '.php', $original[$name]); }