From 9547d0c3809ff071fd3fa74a1d9c820d395b58fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Say=C3=A3o=20Lobato=20Abreu?= Date: Tue, 27 Oct 2026 02:33:41 -0300 Subject: [PATCH 1/2] Add opt-in native magic-number analysis --- .changelog/optional-magic-number-analysis.md | 6 + README.md | 7 + composer.json | 2 + composer.lock | 344 ++++++++++++++---- docs/magic-numbers.rst | 115 ++++++ .../fast-forward-code-standards/SKILL.md | 6 + src/Analysis/MagicNumbersRunner.php | 128 +++++++ src/Console/Application.php | 10 +- src/Console/Command/MagicNumbersCommand.php | 32 ++ src/Console/Input/MagicNumbersInput.php | 35 ++ src/Console/MagicNumbersOperation.php | 43 +++ src/Project/ProjectDirectoryResolver.php | 31 ++ src/Project/ProjectScopeResolver.php | 13 +- .../StandardsServiceProvider.php | 14 +- tests/Analysis/MagicNumbersRunnerTest.php | 57 +++ tests/Fixtures/magic-numbers-runtime.php.inc | 51 +++ tests/Integration/MagicNumbersTest.php | 275 ++++++++++++++ tests/Refactoring/RefactorCommandTest.php | 9 +- 18 files changed, 1099 insertions(+), 79 deletions(-) create mode 100644 .changelog/optional-magic-number-analysis.md create mode 100644 docs/magic-numbers.rst create mode 100644 src/Analysis/MagicNumbersRunner.php create mode 100644 src/Console/Command/MagicNumbersCommand.php create mode 100644 src/Console/Input/MagicNumbersInput.php create mode 100644 src/Console/MagicNumbersOperation.php create mode 100644 src/Project/ProjectDirectoryResolver.php create mode 100644 tests/Analysis/MagicNumbersRunnerTest.php create mode 100644 tests/Fixtures/magic-numbers-runtime.php.inc create mode 100644 tests/Integration/MagicNumbersTest.php diff --git a/.changelog/optional-magic-number-analysis.md b/.changelog/optional-magic-number-analysis.md new file mode 100644 index 0000000..d57ae90 --- /dev/null +++ b/.changelog/optional-magic-number-analysis.md @@ -0,0 +1,6 @@ +--- +category: added +type: minor +--- + +Add an explicit `magic-numbers` command backed by published PHPMND, separate from check/fix. Reuse exact Git/PHP selection and consumer ignores, preserve native policy overrides and exit status, and offer deterministic native XML without loading consumer autoload implicitly. Document opt-in adoption, baseline limits and bounded invocation size. diff --git a/README.md b/README.md index c3fa7d0..2b64146 100644 --- a/README.md +++ b/README.md @@ -190,6 +190,13 @@ Composer package constraint. ## ❓ FAQ +### Optional magic-number analysis + +Run `vendor/bin/code-standards magic-numbers --all` explicitly for PHPMND +analysis. Add `--format=xml` for its native machine-readable report. The command +uses the same tracked-file/Git ignore policy and does not run as part of +`check` or `fix`. See [options, status and adoption guidance](docs/magic-numbers.rst). + **Can I use this as a staged-only pre-commit check?** The CLI selects working-tree files from Git, not staged index blobs. Changed mode requires a clean tracked checkout and cannot certify staged-only content. diff --git a/composer.json b/composer.json index d76c4a2..0343d53 100644 --- a/composer.json +++ b/composer.json @@ -42,6 +42,7 @@ "container-interop/service-provider": "^0.4.1", "fast-forward/container": "~1.10.1", "phpstan/phpstan": "~2.1.51", + "povils/phpmnd": "^3.6.1", "psr/container": "^1.1 || ^2.0", "rector/rector": "~2.4.2", "symfony/console": "^7.4 || ^8.0", @@ -54,6 +55,7 @@ "symfony/http-client": "^7.4 || ^8.0" }, "suggest": { + "ext-dom": "Generate native XML reports for optional magic-number analysis.", "llm/skills": "Optionally synchronize the bundled agent skill in consumer projects." }, "minimum-stability": "stable", diff --git a/composer.lock b/composer.lock index 8b9e2a6..c425c3e 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "dbded3ef4a3e25aa79065ba546255638", + "content-hash": "ef32170a573b526d80de2de4ecd4ebad", "packages": [ { "name": "brick/varexporter", @@ -420,6 +420,107 @@ }, "time": "2026-09-13T18:51:52+00:00" }, + { + "name": "php-parallel-lint/php-console-color", + "version": "v1.0.1", + "source": { + "type": "git", + "url": "https://github.com/php-parallel-lint/PHP-Console-Color.git", + "reference": "7adfefd530aa2d7570ba87100a99e2483a543b88" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-parallel-lint/PHP-Console-Color/zipball/7adfefd530aa2d7570ba87100a99e2483a543b88", + "reference": "7adfefd530aa2d7570ba87100a99e2483a543b88", + "shasum": "" + }, + "require": { + "php": ">=5.3.2" + }, + "replace": { + "jakub-onderka/php-console-color": "*" + }, + "require-dev": { + "php-parallel-lint/php-code-style": "^2.0", + "php-parallel-lint/php-parallel-lint": "^1.0", + "php-parallel-lint/php-var-dump-check": "0.*", + "phpunit/phpunit": "^4.8.36 || ^5.7.21 || ^6.0 || ^7.0 || ^8.0 || ^9.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "PHP_Parallel_Lint\\PhpConsoleColor\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-2-Clause" + ], + "authors": [ + { + "name": "Jakub Onderka", + "email": "jakub.onderka@gmail.com" + } + ], + "description": "Simple library for creating colored console ouput.", + "support": { + "issues": "https://github.com/php-parallel-lint/PHP-Console-Color/issues", + "source": "https://github.com/php-parallel-lint/PHP-Console-Color/tree/v1.0.1" + }, + "time": "2021-12-25T06:49:29+00:00" + }, + { + "name": "php-parallel-lint/php-console-highlighter", + "version": "v1.0.0", + "source": { + "type": "git", + "url": "https://github.com/php-parallel-lint/PHP-Console-Highlighter.git", + "reference": "5b4803384d3303cf8e84141039ef56c8a123138d" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-parallel-lint/PHP-Console-Highlighter/zipball/5b4803384d3303cf8e84141039ef56c8a123138d", + "reference": "5b4803384d3303cf8e84141039ef56c8a123138d", + "shasum": "" + }, + "require": { + "ext-tokenizer": "*", + "php": ">=5.3.2", + "php-parallel-lint/php-console-color": "^1.0.1" + }, + "replace": { + "jakub-onderka/php-console-highlighter": "*" + }, + "require-dev": { + "php-parallel-lint/php-code-style": "^2.0", + "php-parallel-lint/php-parallel-lint": "^1.0", + "php-parallel-lint/php-var-dump-check": "0.*", + "phpunit/phpunit": "^4.8.36 || ^5.7.21 || ^6.0 || ^7.0 || ^8.0 || ^9.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "PHP_Parallel_Lint\\PhpConsoleHighlighter\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Jakub Onderka", + "email": "acci@acci.cz", + "homepage": "http://www.acci.cz/" + } + ], + "description": "Highlight PHP code in terminal", + "support": { + "issues": "https://github.com/php-parallel-lint/PHP-Console-Highlighter/issues", + "source": "https://github.com/php-parallel-lint/PHP-Console-Highlighter/tree/v1.0.0" + }, + "time": "2022-02-18T08:23:19+00:00" + }, { "name": "phpstan/phpstan", "version": "2.1.51", @@ -473,6 +574,119 @@ ], "time": "2026-04-21T18:22:01+00:00" }, + { + "name": "phpunit/php-timer", + "version": "8.0.0", + "source": { + "type": "git", + "url": "https://github.com/sebastianbergmann/php-timer.git", + "reference": "f258ce36aa457f3aa3339f9ed4c81fc66dc8c2cc" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/sebastianbergmann/php-timer/zipball/f258ce36aa457f3aa3339f9ed4c81fc66dc8c2cc", + "reference": "f258ce36aa457f3aa3339f9ed4c81fc66dc8c2cc", + "shasum": "" + }, + "require": { + "php": ">=8.3" + }, + "require-dev": { + "phpunit/phpunit": "^12.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-main": "8.0-dev" + } + }, + "autoload": { + "classmap": [ + "src/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-3-Clause" + ], + "authors": [ + { + "name": "Sebastian Bergmann", + "email": "sebastian@phpunit.de", + "role": "lead" + } + ], + "description": "Utility class for timing", + "homepage": "https://github.com/sebastianbergmann/php-timer/", + "keywords": [ + "timer" + ], + "support": { + "issues": "https://github.com/sebastianbergmann/php-timer/issues", + "security": "https://github.com/sebastianbergmann/php-timer/security/policy", + "source": "https://github.com/sebastianbergmann/php-timer/tree/8.0.0" + }, + "funding": [ + { + "url": "https://github.com/sebastianbergmann", + "type": "github" + } + ], + "time": "2025-02-07T04:59:38+00:00" + }, + { + "name": "povils/phpmnd", + "version": "v3.6.1", + "source": { + "type": "git", + "url": "https://github.com/povils/phpmnd.git", + "reference": "2002af50f627a03d94d01529a3eb02754a23578d" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/povils/phpmnd/zipball/2002af50f627a03d94d01529a3eb02754a23578d", + "reference": "2002af50f627a03d94d01529a3eb02754a23578d", + "shasum": "" + }, + "require": { + "composer-runtime-api": "^2.0", + "nikic/php-parser": "^4.18 || ^5.0", + "php": "^7.4 || ^8.0", + "php-parallel-lint/php-console-highlighter": "^1.0", + "phpunit/php-timer": "^2.0 || ^3.0 || ^4.0 || ^5.0 || ^6.0 || ^7.0 || ^8.0 || ^9.0", + "symfony/console": "^4.4 || ^5.0 || ^6.0 || ^7.0 || ^8.0", + "symfony/finder": "^4.4 || ^5.0 || ^6.0 || ^7.0 || ^8.0" + }, + "require-dev": { + "phpunit/phpunit": "^9.6", + "squizlabs/php_codesniffer": "^2.8.1 || ^3.5" + }, + "bin": [ + "bin/phpmnd" + ], + "type": "application", + "autoload": { + "psr-4": { + "Povils\\PHPMND\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Povilas Susinskas", + "email": "povilassusinskas@gmail.com" + } + ], + "description": "A tool to detect Magic numbers in codebase", + "support": { + "issues": "https://github.com/povils/phpmnd/issues", + "source": "https://github.com/povils/phpmnd/tree/v3.6.1" + }, + "time": "2026-02-21T20:13:10+00:00" + }, { "name": "psr/container", "version": "2.0.2", @@ -806,6 +1020,74 @@ ], "time": "2026-06-05T06:23:12+00:00" }, + { + "name": "symfony/finder", + "version": "v7.4.20", + "source": { + "type": "git", + "url": "https://github.com/symfony/finder.git", + "reference": "2dd31e5208db7a0fda7bfc998ad727b79c2e4d1e" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/finder/zipball/2dd31e5208db7a0fda7bfc998ad727b79c2e4d1e", + "reference": "2dd31e5208db7a0fda7bfc998ad727b79c2e4d1e", + "shasum": "" + }, + "require": { + "php": ">=8.2" + }, + "require-dev": { + "symfony/filesystem": "^6.4|^7.0|^8.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\Finder\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Fabien Potencier", + "email": "fabien@symfony.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Finds files and directories via an intuitive fluent interface", + "homepage": "https://symfony.com", + "support": { + "source": "https://github.com/symfony/finder/tree/v7.4.20" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2026-09-26T06:31:10+00:00" + }, { "name": "symfony/polyfill-ctype", "version": "v1.37.0", @@ -1968,66 +2250,6 @@ ], "time": "2026-10-03T04:51:29+00:00" }, - { - "name": "phpunit/php-timer", - "version": "8.0.0", - "source": { - "type": "git", - "url": "https://github.com/sebastianbergmann/php-timer.git", - "reference": "f258ce36aa457f3aa3339f9ed4c81fc66dc8c2cc" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/php-timer/zipball/f258ce36aa457f3aa3339f9ed4c81fc66dc8c2cc", - "reference": "f258ce36aa457f3aa3339f9ed4c81fc66dc8c2cc", - "shasum": "" - }, - "require": { - "php": ">=8.3" - }, - "require-dev": { - "phpunit/phpunit": "^12.0" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-main": "8.0-dev" - } - }, - "autoload": { - "classmap": [ - "src/" - ] - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "BSD-3-Clause" - ], - "authors": [ - { - "name": "Sebastian Bergmann", - "email": "sebastian@phpunit.de", - "role": "lead" - } - ], - "description": "Utility class for timing", - "homepage": "https://github.com/sebastianbergmann/php-timer/", - "keywords": [ - "timer" - ], - "support": { - "issues": "https://github.com/sebastianbergmann/php-timer/issues", - "security": "https://github.com/sebastianbergmann/php-timer/security/policy", - "source": "https://github.com/sebastianbergmann/php-timer/tree/8.0.0" - }, - "funding": [ - { - "url": "https://github.com/sebastianbergmann", - "type": "github" - } - ], - "time": "2025-02-07T04:59:38+00:00" - }, { "name": "phpunit/phpunit", "version": "12.5.40", diff --git a/docs/magic-numbers.rst b/docs/magic-numbers.rst new file mode 100644 index 0000000..557f226 --- /dev/null +++ b/docs/magic-numbers.rst @@ -0,0 +1,115 @@ +Optional magic-number analysis +============================== + +``magic-numbers`` runs PHPMND independently of syntax/style checking. Neither +``check`` nor ``fix`` invokes it. The analyzer reads the selected source; it +does not replace literals, load ECS configuration, install dependencies or +update Composer locks. PHP DOM is optional for the package and required only +for the native XML reporter; text analysis and style checking do not require it. + +Usage and selection +------------------- + +.. code-block:: sh + + vendor/bin/code-standards magic-numbers --all + vendor/bin/code-standards magic-numbers --all --format=xml > magic-numbers.xml + vendor/bin/code-standards magic-numbers --base= --head= --format=xml + +Use exactly ``--all`` or both immutable 40-character ``--base``/``--head`` SHAs. +``--directory`` (``-C``) selects the Git checkout root. Changed mode requires the +requested HEAD and a clean tracked checkout. Selection uses the PR merge base, +ignores deleted paths and respects the consumer's root/nested ``.gitignore`` +rules, including force-added tracked files matching those rules. Only tracked +PHP files and PHP scripts under ``bin/`` are eligible; escaping/symlink paths +are rejected. Paths are sorted and passed as separate absolute arguments, +including names containing spaces, newlines or UTF-8. + +PHPMND runs from its own installed package directory. Its implicit +``cwd/vendor/autoload.php`` discovery therefore does not load the consumer's +autoload file. The tool's own Composer bootstrap remains intentional. No +consumer PHP configuration or optional application bootstrap is accepted by +this command. + +Native policy overrides +----------------------- + +The published PHPMND profile checks condition, return and switch-case contexts +by default, ignoring numeric values ``0`` and ``1``. The supported options map +directly to native PHPMND options, without an additional configuration language: + +* ``--ignore-numbers=42`` adds numeric exceptions to PHPMND's fixed zero/one + exceptions. An explicit empty value still preserves those native exceptions; + it does not enable checking of zero or one. Non-numeric or infinite values + (such as an overflowing exponent) fail before analysis. +* ``--extensions=argument`` adds a native analysis context to the defaults. + ``--extensions=all,-argument`` selects all native contexts except argument. + These are AST contexts, **not** filename extensions. PHPMND validates names. +* ``--ignore-funcs=output,fixture`` provides native function exceptions when the + argument context is enabled. It does not enable that context itself. + +.. code-block:: sh + + vendor/bin/code-standards magic-numbers --all --extensions=all,-argument + vendor/bin/code-standards magic-numbers --all --extensions=argument --ignore-funcs=output + +Output and status +----------------- + +``--format=text`` is the native human-readable report. Its resource/timing +footer is not byte deterministic. ``--format=xml`` streams the native XML +document to stdout, without ANSI, progress, banners or timing. Diagnostics use +stderr. For the same selected paths, contents, options and installed PHPMND +version, XML is byte deterministic; file paths are absolute and therefore +depend on the checkout location. XML attributes preserve unusual path bytes +through normal XML escaping. + +If PHP DOM is unavailable, XML mode fails with status ``2`` and a stderr +diagnostic before checking source encoding, executing PHPMND or writing stdout, +including for an empty selection. Enable DOM or choose text output explicitly. + +XML mode requires every selected path and the complete contents of every +selected source to be valid UTF-8 containing only XML 1.0 characters. It rejects +unsupported control characters or invalid encoding before starting the analyzer +or writing stdout, even when the affected file would produce no findings. +No bytes are sanitized or silently omitted. Tabs, newlines, UTF-8 and embedded +CDATA terminators are preserved through the native DOM serializer. Text mode +keeps the native analyzer's encoding behavior without this XML-specific check. + +When selection is empty, the engine is not executed. The command emits the +native empty-result XML structure with the installed version, or a text +selection message. It does not report that any file was analyzed. + +Exit ``0`` means no findings; exit ``1`` preserves native findings/analyzer +failure status. Invalid command settings, Git scope or process errors fail +with status ``2``. Failure is never converted to a clean result. Redirected XML +is an artifact explicitly requested by the caller; source and lock files are +unchanged. + +One invocation is limited to 64 KiB of argument bytes, including options and +paths. An oversized selection fails before execution, without silently dropping +files or returning partial XML. Changed-file selection can reduce scope but +does not guarantee it fits; large full scans need a separate batching increment. +Concatenating multiple XML documents is not supported as one report. + +Adoption and baseline guidance +------------------------------ + +Start with an explicit local invocation and inspect the native XML artifact. +Keep the command outside mandatory checks until the team agrees on useful +contexts and exceptions. Prefer named constants when they clarify intent; +numeric-heavy domains may need carefully reviewed native exceptions. + +Numeric/function/context exceptions apply broadly; they are **not** a +per-finding baseline. PHPMND's native whitelist selects filenames and is not +a finding baseline either. This command does not expose that newline-based +transport, automatically accept existing debt, generate suppression files or +claim baseline enforcement. Teams needing precise accepted-debt comparison +must retain the report and review new findings explicitly until such a contract +is implemented. No repository-wide rollout is performed by enabling the tool. + +References +---------- + +* `PHPMND published source and CLI `_ +* `Optional profile issue `_ diff --git a/resources/skills/fast-forward-code-standards/SKILL.md b/resources/skills/fast-forward-code-standards/SKILL.md index 034b01c..8e88c19 100644 --- a/resources/skills/fast-forward-code-standards/SKILL.md +++ b/resources/skills/fast-forward-code-standards/SKILL.md @@ -34,6 +34,12 @@ metadata: the selected ref after v0 points to a confirmed 0.x release, and use `pull_request` with read-only permissions and a hosted runner. No secrets/inherit, write token, target event or automatic fix. Required-check settings remain separately owned. +7. Only when magic-number analysis is requested, run `magic-numbers --all` or + the same immutable `--base`/`--head` selection. It is independent of check/fix + and does not execute consumer configuration/autoload implicitly. Use + `--format=xml` for native machine output; preserve failing status and stderr. + Review native numeric/function/context exceptions explicitly. They are broad + policy overrides, not per-finding baselines; do not auto-accept existing debt. This directory is the sole source payload. Installation/host activation is explicit; copying this skill does not install the runtime or authorize consumer rollout. diff --git a/src/Analysis/MagicNumbersRunner.php b/src/Analysis/MagicNumbersRunner.php new file mode 100644 index 0000000..1c7075c --- /dev/null +++ b/src/Analysis/MagicNumbersRunner.php @@ -0,0 +1,128 @@ + $files Ordered absolute files validated by the shared PHP/Git selection. + * @param string $format Text for people, or native XML without banners/ANSI/timing on stdout. + * @param string $ignoreNumbers Native comma-separated additions to PHPMND's fixed zero/one exceptions. + * @param string|null $ignoreFuncs Native function exceptions for argument-context analysis. + * @param string|null $extensions Native context additions/removals; these are not file suffixes. + * @return int Exact native exit status: zero is clean and one indicates findings or analyzer failure. + * @throws InvalidArgumentException When input or invocation size is outside the supported contract. + */ + public function run(array $files, string $format, string $ignoreNumbers, ?string $ignoreFuncs, ?string $extensions): int + { + $arguments = $this->arguments($format, $ignoreNumbers, $ignoreFuncs, $extensions); + if ('xml' === $format && !extension_loaded('dom')) { + throw new InvalidArgumentException('XML format requires the PHP DOM extension. Use text format or enable DOM.'); + } + $arguments = [...$arguments, '--', ...$files]; + if (array_sum(array_map(static fn(string $argument): int => strlen($argument) + 1, $arguments)) > self::MAX_ARGUMENT_BYTES) { + throw new InvalidArgumentException('PHPMND invocation exceeds 64 KiB; a smaller scope or a batching increment is required.'); + } + if ('xml' === $format) { + $this->assertXmlInputs($files); + } + if ([] === $files) { + $this->emptyOutput($format); + + return 0; + } + + return $this->process->run($arguments, dirname($this->entrypoint, 2)); + } + + /** @return list Validated native options; no opaque option forwarding can change selection or output paths. */ + private function arguments(string $format, string $ignoreNumbers, ?string $ignoreFuncs, ?string $extensions): array + { + if (!in_array($format, ['text', 'xml'], true)) { + throw new InvalidArgumentException('Format must be text or xml.'); + } + foreach ('' === $ignoreNumbers ? [] : explode(',', $ignoreNumbers) as $number) { + if (!is_numeric($number) || !is_finite((float) $number)) { + throw new InvalidArgumentException('Ignored numbers must be a comma-separated list of finite numeric values.'); + } + } + + $arguments = [PHP_BINARY, $this->entrypoint, '--no-ansi', '--ignore-numbers=' . $ignoreNumbers]; + if (null !== $ignoreFuncs) { + $arguments[] = '--ignore-funcs=' . $ignoreFuncs; + } + if (null !== $extensions) { + if ('' === $extensions || in_array('', explode(',', $extensions), true)) { + throw new InvalidArgumentException('Extensions must name native PHPMND analysis contexts.'); + } + (new ExtensionResolver())->resolve(explode(',', $extensions)); + $arguments[] = '--extensions=' . $extensions; + } + if ('xml' === $format) { + $arguments[] = '--quiet'; + $arguments[] = '--xml-output=php://stdout'; + } + + return $arguments; + } + + /** + * Rejects bytes the native XML reporter cannot represent before it can stream a partial document. + * + * XML 1.0 permits tab/newline/carriage return but excludes other C0 controls and invalid UTF-8. + * Native DOM splits embedded CDATA terminators correctly; no source/path bytes are sanitized. + * + * @param list $files All selected paths, not only files that eventually produce findings. + * @throws InvalidArgumentException When a selected path or source cannot be represented in XML 1.0. + */ + private function assertXmlInputs(array $files): void + { + $characters = '/\A[\x{9}\x{A}\x{D}\x{20}-\x{D7FF}\x{E000}-\x{FFFD}\x{10000}-\x{10FFFF}]*\z/u'; + foreach ($files as $file) { + if (1 !== preg_match($characters, $file)) { + throw new InvalidArgumentException('XML format requires UTF-8 XML 1.0 characters in every selected path.'); + } + $source = @file_get_contents($file); + if (false === $source) { + throw new InvalidArgumentException('XML format could not read a selected source file.'); + } + if (1 !== preg_match($characters, $source)) { + throw new InvalidArgumentException('XML format requires UTF-8 XML 1.0 characters in every selected source file.'); + } + } + } + + /** Emits the native empty-result document even when Git selection contains no PHP files. */ + private function emptyOutput(string $format): void + { + if ('text' === $format) { + $this->output->writeln('No PHP files selected.'); + + return; + } + + $version = htmlspecialchars(InstalledVersions::getPrettyVersion('povils/phpmnd') ?? '', ENT_XML1 | ENT_QUOTES, 'UTF-8'); + $this->output->write('' . "\n" . '' . "\n", options: OutputInterface::OUTPUT_RAW); + } +} diff --git a/src/Console/Application.php b/src/Console/Application.php index 0bdbbe4..fd8c7ca 100644 --- a/src/Console/Application.php +++ b/src/Console/Application.php @@ -6,6 +6,7 @@ use FastForward\CodeStandards\Console\Command\CheckCommand; use FastForward\CodeStandards\Console\Command\FixCommand; +use FastForward\CodeStandards\Console\Command\MagicNumbersCommand; use FastForward\CodeStandards\Console\Command\RefactorCommand; use Symfony\Component\Console\Application as SymfonyApplication; use Symfony\Component\Console\Command\Command; @@ -18,13 +19,18 @@ final class Application extends SymfonyApplication { /** Registers independent operations without resolving project state or executing tools. */ - public function __construct(CheckCommand $check, FixCommand $fix, RefactorCommand $refactor) - { + public function __construct( + CheckCommand $check, + FixCommand $fix, + RefactorCommand $refactor, + MagicNumbersCommand $magicNumbers, + ) { parent::__construct('Fast Forward Code Standards'); $this->addCommand($check); $this->addCommand($fix); $this->addCommand($refactor); + $this->addCommand($magicNumbers); $this->setAutoExit(false); } diff --git a/src/Console/Command/MagicNumbersCommand.php b/src/Console/Command/MagicNumbersCommand.php new file mode 100644 index 0000000..96983ee --- /dev/null +++ b/src/Console/Command/MagicNumbersCommand.php @@ -0,0 +1,32 @@ +operation->run($settings, $io); + } +} diff --git a/src/Console/Input/MagicNumbersInput.php b/src/Console/Input/MagicNumbersInput.php new file mode 100644 index 0000000..edf2263 --- /dev/null +++ b/src/Console/Input/MagicNumbersInput.php @@ -0,0 +1,35 @@ +directories->resolve($settings->all, $settings->base, $settings->head, $settings->directory); + $paths = $this->selection->select($directory, $settings->base, $settings->head); + $files = $this->phpFiles->filter($directory, $paths); + + return $this->analysis->run($files, $settings->format, $settings->ignoreNumbers, $settings->ignoreFuncs, $settings->extensions); + } catch (InvalidArgumentException|RuntimeException $exception) { + $io->getErrorStyle()->error($exception->getMessage()); + + return Command::INVALID; + } + } +} diff --git a/src/Project/ProjectDirectoryResolver.php b/src/Project/ProjectDirectoryResolver.php new file mode 100644 index 0000000..5329f8c --- /dev/null +++ b/src/Project/ProjectDirectoryResolver.php @@ -0,0 +1,31 @@ +directories->resolve($all, $base, $head, $directory); $configuration = $this->configuration($root, $config); return new ProjectScope($root, $configuration, $base, $head); diff --git a/src/ServiceProvider/StandardsServiceProvider.php b/src/ServiceProvider/StandardsServiceProvider.php index 051cd3a..40cc472 100644 --- a/src/ServiceProvider/StandardsServiceProvider.php +++ b/src/ServiceProvider/StandardsServiceProvider.php @@ -4,8 +4,10 @@ namespace FastForward\CodeStandards\ServiceProvider; +use FastForward\CodeStandards\Analysis\MagicNumbersRunner; use FastForward\CodeStandards\Process\NativeProcessRunner; use FastForward\CodeStandards\Process\ProcessRunner; +use FastForward\CodeStandards\Project\ProjectDirectoryResolver; use FastForward\CodeStandards\Project\ProjectScopeResolver; use FastForward\CodeStandards\Refactoring\RectorRunner; use FastForward\CodeStandards\Standards\StandardsRunner; @@ -53,7 +55,17 @@ public function getFactories(): array return [ OutputInterface::class => new ServiceFactory($this->output), ProcessRunner::class => new CallableFactory($this->process(...)), - ProjectScopeResolver::class => new InvokableFactory(ProjectScopeResolver::class, dirname(__DIR__, 2) . '/config/ecs.php'), + ProjectScopeResolver::class => new InvokableFactory( + ProjectScopeResolver::class, + dirname(__DIR__, 2) . '/config/ecs.php', + ProjectDirectoryResolver::class, + ), + MagicNumbersRunner::class => new InvokableFactory( + MagicNumbersRunner::class, + ProcessRunner::class, + OutputInterface::class, + $this->vendorDirectory . '/povils/phpmnd/bin/phpmnd', + ), StandardsRunner::class => new InvokableFactory( StandardsRunner::class, ProcessRunner::class, diff --git a/tests/Analysis/MagicNumbersRunnerTest.php b/tests/Analysis/MagicNumbersRunnerTest.php new file mode 100644 index 0000000..cb4fb37 --- /dev/null +++ b/tests/Analysis/MagicNumbersRunnerTest.php @@ -0,0 +1,57 @@ +createMock(ProcessRunner::class); + $process->expects(self::never())->method('run'); + $runner = new MagicNumbersRunner($process, new BufferedOutput(), '/owned/vendor/povils/phpmnd/bin/phpmnd'); + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('exceeds 64 KiB'); + $runner->run(['/' . str_repeat('a', 65536) . '.php'], 'xml', '0,1', null, null); + } + + /** Native options cannot supply a different cwd, autoloader, scanner directory or report path. */ + public function testNativeStatusAndExplicitFileArgumentsArePreserved(): void + { + $process = $this->createMock(ProcessRunner::class); + $entrypoint = '/owned/vendor/povils/phpmnd/bin/phpmnd'; + $process->expects(self::once())->method('run')->with( + [PHP_BINARY, $entrypoint, '--no-ansi', '--ignore-numbers=0,1', '--', '/consumer/a file.php', "/consumer/new\nline.php"], + '/owned/vendor/povils/phpmnd', + )->willReturn(1); + $runner = new MagicNumbersRunner($process, new BufferedOutput(), $entrypoint); + self::assertSame(1, $runner->run(['/consumer/a file.php', "/consumer/new\nline.php"], 'text', '0,1', null, null)); + } + + /** Invalid path encoding is rejected before file IO, including on hosts whose filesystem cannot create such names. */ + public function testXmlInvalidPathsFailBeforeExecutionOrOutput(): void + { + $process = $this->createMock(ProcessRunner::class); + $process->expects(self::never())->method('run'); + $output = new BufferedOutput(); + $runner = new MagicNumbersRunner($process, $output, '/owned/vendor/povils/phpmnd/bin/phpmnd'); + foreach (["/consumer/control\x01.php", "/consumer/encoding\xFF.php"] as $path) { + try { + $runner->run([$path], 'xml', '0,1', null, null); + self::fail('Invalid XML path bytes must be rejected before file IO.'); + } catch (InvalidArgumentException $exception) { + self::assertStringContainsString('selected path', $exception->getMessage()); + self::assertSame('', $output->fetch()); + } + } + } +} diff --git a/tests/Fixtures/magic-numbers-runtime.php.inc b/tests/Fixtures/magic-numbers-runtime.php.inc new file mode 100644 index 0000000..1c89a13 --- /dev/null +++ b/tests/Fixtures/magic-numbers-runtime.php.inc @@ -0,0 +1,51 @@ +capture($arguments, $directory, $timeout); + } + + /** Records the analyzer boundary without requiring another executable or loading application code. */ + public function run(array $arguments, string $directory, float $timeout = 300): int + { + ++$this->runs; + + return 17; + } +}; + +$container = ContainerFactory::create(new AggregateServiceProvider( + new StandardsServiceProvider(dirname($argv[1]), new ConsoleOutput()), + new ArrayServiceProvider([ProcessRunner::class => static fn(): ProcessRunner => $process]), +)); +$status = $container->get(Application::class)->run(new ArrayInput([ + 'command' => 'magic-numbers', + '--all' => true, + '--directory' => $argv[2], + '--format' => $argv[3], +])); +file_put_contents($argv[4], (string) $process->runs); +exit($status); diff --git a/tests/Integration/MagicNumbersTest.php b/tests/Integration/MagicNumbersTest.php new file mode 100644 index 0000000..4a9808b --- /dev/null +++ b/tests/Integration/MagicNumbersTest.php @@ -0,0 +1,275 @@ +directory = realpath(sys_get_temp_dir()) . '/standards-mnd-' . bin2hex(random_bytes(8)); + mkdir($this->directory); + $this->put('.gitignore', "/vendor/\n/generated/\n"); + $this->git(['init', '--quiet']); + $this->git(['config', 'user.email', 'fixture@example.test']); + $this->git(['config', 'user.name', 'Synthetic fixture']); + } + + /** Removes only the unique fixture owned by this test. */ + protected function tearDown(): void + { + (new Filesystem())->remove($this->directory); + } + + /** Native XML remains deterministic, round-trips binary-safe names and never executes implicit consumer bootstrap. */ + public function testNativeXmlPreservesPathsAndSourceWithoutConsumerAutoload(): void + { + $source = 'put($path, $source); + } + $this->put('generated/ignored.php', $source); + $this->put('vendor/autoload.php', 'put('composer.lock', 'synthetic lock'); + $this->git(['add', '--all']); + $this->git(['add', '-f', '--', 'generated/ignored.php', 'vendor/autoload.php']); + + $result = $this->cli(['--all', '--format=xml']); + self::assertSame(1, $result->getExitCode(), $result->getErrorOutput()); + self::assertSame('', $result->getErrorOutput()); + self::assertStringNotContainsString("\033", $result->getOutput()); + $document = $this->xml($result->getOutput()); + $reported = []; + foreach ($document->getElementsByTagName('file') as $file) { + $reported[] = $file->getAttribute('path'); + } + sort($paths, SORT_STRING); + self::assertSame(array_map(fn(string $path): string => $this->directory . '/' . $path, $paths), $reported); + self::assertSame('3', $document->documentElement->getAttribute('errorCount')); + self::assertSame($result->getOutput(), $this->cli(['--all', '--format=xml'])->getOutput()); + foreach ($paths as $path) { + self::assertSame($source, file_get_contents($this->directory . '/' . $path)); + } + self::assertSame('synthetic lock', file_get_contents($this->directory . '/composer.lock')); + self::assertFileDoesNotExist($this->directory . '/executed'); + } + + /** Native numeric/context/function overrides remain explicit, with no formatter configuration involved. */ + public function testOverridesAndTextStatus(): void + { + $this->put('source.php', 'git(['add', '--all']); + self::assertSame(1, $this->cli(['--all'])->getExitCode()); + $override = $this->cli(['--all', '--format=xml', '--ignore-numbers=0,1,42']); + self::assertSame(0, $override->getExitCode(), $override->getErrorOutput()); + self::assertSame('0', $this->xml($override->getOutput())->documentElement->getAttribute('errorCount')); + + $this->put('source.php', 'cli(['--all'])->getExitCode()); + self::assertSame(1, $this->cli(['--all', '--extensions=argument'])->getExitCode()); + self::assertSame(0, $this->cli(['--all', '--extensions=argument', '--ignore-funcs=output'])->getExitCode()); + } + + /** Native zero/one exceptions cannot be removed; user-supplied numbers are additions rather than replacements. */ + public function testNumericExceptionsAreAdditiveAndFinite(): void + { + $this->put('source.php', 'git(['add', '--all']); + $empty = $this->cli(['--all', '--format=xml', '--ignore-numbers=']); + self::assertSame(1, $empty->getExitCode(), $empty->getErrorOutput()); + self::assertSame('1', $this->xml($empty->getOutput())->documentElement->getAttribute('errorCount')); + $addition = $this->cli(['--all', '--format=xml', '--ignore-numbers=42']); + self::assertSame(0, $addition->getExitCode(), $addition->getErrorOutput()); + self::assertSame('0', $this->xml($addition->getOutput())->documentElement->getAttribute('errorCount')); + $this->put('source.php', 'cli(['--all', '--format=xml', '--ignore-numbers=' . $number]); + self::assertSame(0, $addition->getExitCode(), $addition->getErrorOutput()); + self::assertSame('0', $this->xml($addition->getOutput())->documentElement->getAttribute('errorCount')); + } + $overflow = $this->cli(['--all', '--format=xml', '--ignore-numbers=1e9999']); + self::assertSame(2, $overflow->getExitCode()); + self::assertSame('', $overflow->getOutput()); + self::assertStringContainsString('finite numeric', $overflow->getErrorOutput()); + } + + /** Invalid XML path/source bytes fail before native streaming and without source mutation or sanitization. */ + public function testXmlRejectsUnsupportedBytesBeforeWritingOutput(): void + { + $valid = 'put('a-valid.php', $valid); + $cases = [ + ["path-control\x01.php", $valid, 'selected path'], + ['source-control.php', 'put($path, $source); + $this->git(['add', '--all']); + $result = $this->cli(['--all', '--format=xml']); + self::assertSame(2, $result->getExitCode(), $result->getErrorOutput()); + self::assertSame('', $result->getOutput()); + self::assertStringContainsString($diagnostic, $result->getErrorOutput()); + self::assertSame($source, file_get_contents($this->directory . '/' . $path)); + self::assertSame($valid, file_get_contents($this->directory . '/a-valid.php')); + (new Filesystem())->remove($this->directory . '/' . $path); + } + } + + /** The native DOM reporter splits CDATA terminators without changing the source snippet. */ + public function testXmlRoundTripsCdataTerminator(): void + { + $source = '"; }' . "\n"; + $this->put('source.php', $source); + $this->git(['add', '--all']); + $result = $this->cli(['--all', '--format=xml']); + self::assertSame(1, $result->getExitCode(), $result->getErrorOutput()); + self::assertSame(rtrim($source, "\n"), $this->xml($result->getOutput())->getElementsByTagName('snippet')->item(0)->textContent); + self::assertSame($source, file_get_contents($this->directory . '/source.php')); + } + + /** Empty selection is represented by the same native XML structure as a clean selected file. */ + public function testEmptySelectionMatchesNativeCleanDocument(): void + { + $this->git(['add', '--all']); + $empty = $this->cli(['--all', '--format=xml']); + self::assertSame(0, $empty->getExitCode(), $empty->getErrorOutput()); + $this->xml($empty->getOutput()); + $this->put('clean.php', 'git(['add', '--all']); + $clean = $this->cli(['--all', '--format=xml']); + self::assertSame(0, $clean->getExitCode(), $clean->getErrorOutput()); + self::assertSame($empty->getOutput(), $clean->getOutput()); + } + + /** Changed-file analysis preserves exact-head/clean-checkout validation and does not scan unrelated debt. */ + public function testChangedSelectionUsesExactCommits(): void + { + $this->put('old.php', 'git(['add', '--all']); + $this->commit(); + $base = trim($this->git(['rev-parse', 'HEAD'])); + $this->put('new.php', 'git(['add', '--all']); + $this->commit(); + $head = trim($this->git(['rev-parse', 'HEAD'])); + $arguments = ['--base=' . $base, '--head=' . $head, '--format=xml']; + self::assertSame(0, $this->cli($arguments)->getExitCode()); + self::assertSame(1, $this->cli(['--all'])->getExitCode()); + $this->put('new.php', 'cli($arguments)->getExitCode()); + } + + /** Invalid input produces failing diagnostics on stderr rather than corrupting machine stdout. */ + public function testInvalidArgumentsAndHelp(): void + { + $this->git(['add', '--all']); + $invalid = [ + [], + ['--all', '--format=json'], + ['--all', '--ignore-numbers=not-a-number'], + ['--all', '--extensions=missing-context'], + ['--all', '--config=arbitrary.php'], + ['--all', '--fix'], + ]; + foreach ($invalid as $arguments) { + $result = $this->cli($arguments); + self::assertSame(2, $result->getExitCode(), implode(' ', $arguments)); + self::assertSame('', $result->getOutput()); + self::assertNotSame('', $result->getErrorOutput()); + } + $help = $this->cli(['--help']); + self::assertSame(0, $help->getExitCode()); + self::assertStringContainsString('magic-numbers', $help->getOutput()); + self::assertStringContainsString('--ignore-numbers', $help->getOutput()); + } + + /** Analyzer parsing failures are failures, and no source repair is attempted. */ + public function testNativeParseFailureIsNotReportedAsClean(): void + { + $source = 'put('broken.php', $source); + $this->git(['add', '--all']); + self::assertNotSame(0, $this->cli(['--all', '--format=xml'])->getExitCode()); + self::assertSame($source, file_get_contents($this->directory . '/broken.php')); + } + + /** A real DOM-free runtime permits text execution but rejects XML before process execution or stdout. */ + public function testDomFreeRuntimeKeepsTextAndRejectsXml(): void + { + $probe = new Process([PHP_BINARY, '-n', '-r', 'exit(extension_loaded("dom") ? 1 : 0);'], timeout: 10); + if (1 === $probe->run()) { + self::markTestSkipped('DOM remains statically available with php -n on this host; no DOM-free runtime is installed.'); + } + self::assertSame(0, $probe->getExitCode(), $probe->getErrorOutput()); + $this->put('source.php', 'git(['add', '--all']); + $root = dirname(__DIR__, 2); + foreach (['text' => [17, '1'], 'xml' => [2, '0']] as $format => [$status, $runs]) { + $counter = $this->directory . '/runs-' . $format; + $process = new Process([ + PHP_BINARY, '-n', $root . '/tests/Fixtures/magic-numbers-runtime.php.inc', + $root . '/vendor/autoload.php', $this->directory, $format, $counter, + ], $this->directory, timeout: 15); + self::assertSame($status, $process->run(), $process->getErrorOutput()); + self::assertSame('', $process->getOutput()); + self::assertSame($runs, file_get_contents($counter)); + if ('xml' === $format) { + self::assertStringContainsString('PHP DOM extension', $process->getErrorOutput()); + } else { + self::assertSame('', $process->getErrorOutput()); + } + } + } + + /** @param list $arguments Explicit command options without shell parsing. */ + private function cli(array $arguments): Process + { + $process = new Process([PHP_BINARY, dirname(__DIR__, 2) . '/bin/code-standards', 'magic-numbers', ...$arguments], $this->directory, timeout: 60); + $process->run(); + + return $process; + } + + /** Parses only the command's owned XML output, with network entity loading disabled. */ + private function xml(string $output): DOMDocument + { + $document = new DOMDocument(); + self::assertTrue($document->loadXML($output, LIBXML_NONET)); + + return $document; + } + + /** Writes only within this test's exclusive synthetic fixture. */ + private function put(string $path, string $source): void + { + (new Filesystem())->mkdir(dirname($this->directory . '/' . $path)); + file_put_contents($this->directory . '/' . $path, $source); + } + + /** @param list $arguments Native Git operations confined to the disposable fixture. */ + private function git(array $arguments): string + { + $process = new Process(['git', ...$arguments], $this->directory, timeout: 30); + $process->mustRun(); + + return $process->getOutput(); + } + + /** Commits synthetic files with fixture-local signing/hooks disabled; real repository hooks remain unchanged. */ + private function commit(): void + { + $this->git(['-c', 'commit.gpgsign=false', '-c', 'core.hooksPath=/dev/null', 'commit', '--quiet', '-m', 'Synthetic fixture']); + } +} diff --git a/tests/Refactoring/RefactorCommandTest.php b/tests/Refactoring/RefactorCommandTest.php index 96f4d7f..48d9687 100644 --- a/tests/Refactoring/RefactorCommandTest.php +++ b/tests/Refactoring/RefactorCommandTest.php @@ -7,6 +7,7 @@ use FastForward\CodeStandards\Console\Command\RefactorCommand; use FastForward\CodeStandards\Process\NativeProcessRunner; use FastForward\CodeStandards\Process\ProcessRunner; +use FastForward\CodeStandards\Project\ProjectDirectoryResolver; use FastForward\CodeStandards\Project\ProjectScopeResolver; use FastForward\CodeStandards\Refactoring\RectorRunner; use PHPUnit\Framework\TestCase; @@ -29,7 +30,7 @@ public function testNativeBindingAndRequiredConfiguration(): void $process->expects(self::once())->method('run')->with(self::callback(static fn(array $arguments): bool => in_array('--dry-run', $arguments, true)), $directory)->willReturn(2); $application = new Application(); $application->setAutoExit(false); - $application->addCommand(new RefactorCommand(new ProjectScopeResolver('/unused-default'), new RectorRunner($process, '/tool/rector'))); + $application->addCommand(new RefactorCommand(new ProjectScopeResolver('/unused-default', new ProjectDirectoryResolver()), new RectorRunner($process, '/tool/rector'))); $tester = new ApplicationTester($application); try { @@ -99,7 +100,7 @@ public function testTimeoutIsFailureAndCleansThePreflight(int $columns): void $process->expects(self::never())->method('run'); $application = new Application(); $application->setAutoExit(false); - $application->addCommand(new RefactorCommand(new ProjectScopeResolver('/unused-default'), new RectorRunner($process, '/tool/rector'))); + $application->addCommand(new RefactorCommand(new ProjectScopeResolver('/unused-default', new ProjectDirectoryResolver()), new RectorRunner($process, '/tool/rector'))); $tester = new ApplicationTester($application); $previousColumns = getenv('COLUMNS'); @@ -135,7 +136,7 @@ public function testInvalidTimeoutIsRejectedWithoutLaunchingProcesses(string $ti $process->expects(self::never())->method('run'); $application = new Application(); $application->setAutoExit(false); - $application->addCommand(new RefactorCommand(new ProjectScopeResolver('/unused-default'), new RectorRunner($process, '/tool/rector'))); + $application->addCommand(new RefactorCommand(new ProjectScopeResolver('/unused-default', new ProjectDirectoryResolver()), new RectorRunner($process, '/tool/rector'))); $tester = new ApplicationTester($application); try { self::assertSame(2, $tester->run(['command' => 'refactor', '--config' => $config, '--timeout' => $timeout])); @@ -155,7 +156,7 @@ public function testPositiveTimeoutOptionIsForwardedWithoutCoercion(): void $process->expects(self::once())->method('run')->with(self::isArray(), $directory, 600.0)->willReturn(0); $application = new Application(); $application->setAutoExit(false); - $application->addCommand(new RefactorCommand(new ProjectScopeResolver('/unused-default'), new RectorRunner($process, '/tool/rector'))); + $application->addCommand(new RefactorCommand(new ProjectScopeResolver('/unused-default', new ProjectDirectoryResolver()), new RectorRunner($process, '/tool/rector'))); $tester = new ApplicationTester($application); try { self::assertSame(0, $tester->run(['command' => 'refactor', '--config' => $config, '--directory' => $directory, '--timeout' => '600'])); From 63ce634b65924bf3beb9ec934ea41998039e2a1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Say=C3=A3o=20Lobato=20Abreu?= Date: Tue, 27 Oct 2026 11:10:48 -0300 Subject: [PATCH 2/2] Clarify analyzer status and cover validation boundaries --- docs/magic-numbers.rst | 11 +- src/Analysis/MagicNumbersRunner.php | 2 +- tests/Analysis/MagicNumbersRunnerTest.php | 131 ++++++++++++++++++++++ tests/Integration/MagicNumbersTest.php | 30 +++-- 4 files changed, 163 insertions(+), 11 deletions(-) diff --git a/docs/magic-numbers.rst b/docs/magic-numbers.rst index 4f42c23..3e7c8f7 100644 --- a/docs/magic-numbers.rst +++ b/docs/magic-numbers.rst @@ -81,9 +81,14 @@ When selection is empty, the engine is not executed. The command emits the native empty-result XML structure with the installed version, or a text selection message. It does not report that any file was analyzed. -Exit ``0`` means no findings; exit ``1`` preserves native findings/analyzer -failure status. Invalid command settings, Git scope or process errors fail -with status ``2``. Failure is never converted to a clean result. Redirected XML +Exit ``0`` means no findings, or an empty selection for which the engine did +not run. Native exit ``1`` can indicate findings or an analyzer failure. +Invalid command settings, Git scope or process exceptions fail with status +``2``, but native status ``2`` can also pass through; it does not exclusively +identify input validation. Every other native nonzero status is returned +unchanged, including statuses such as ``17`` or ``255``. Inspect stderr and the +native report before classifying a nonzero result. Failure is never converted +to a clean result. Redirected XML is an artifact explicitly requested by the caller; source and lock files are unchanged. diff --git a/src/Analysis/MagicNumbersRunner.php b/src/Analysis/MagicNumbersRunner.php index 1c7075c..63b3c5a 100644 --- a/src/Analysis/MagicNumbersRunner.php +++ b/src/Analysis/MagicNumbersRunner.php @@ -31,7 +31,7 @@ public function __construct(private ProcessRunner $process, private OutputInterf * @param string $ignoreNumbers Native comma-separated additions to PHPMND's fixed zero/one exceptions. * @param string|null $ignoreFuncs Native function exceptions for argument-context analysis. * @param string|null $extensions Native context additions/removals; these are not file suffixes. - * @return int Exact native exit status: zero is clean and one indicates findings or analyzer failure. + * @return int Exact native exit status, or zero for empty selection; native nonzero statuses pass through unchanged. * @throws InvalidArgumentException When input or invocation size is outside the supported contract. */ public function run(array $files, string $format, string $ignoreNumbers, ?string $ignoreFuncs, ?string $extensions): int diff --git a/tests/Analysis/MagicNumbersRunnerTest.php b/tests/Analysis/MagicNumbersRunnerTest.php index cb4fb37..e6e44e2 100644 --- a/tests/Analysis/MagicNumbersRunnerTest.php +++ b/tests/Analysis/MagicNumbersRunnerTest.php @@ -4,15 +4,136 @@ namespace FastForward\CodeStandards\Tests\Analysis; +use Composer\InstalledVersions; +use DOMDocument; use FastForward\CodeStandards\Analysis\MagicNumbersRunner; use FastForward\CodeStandards\Process\ProcessRunner; use InvalidArgumentException; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Symfony\Component\Console\Output\BufferedOutput; /** Verifies process authority and bounded arguments independently of native timing/presentation. */ final class MagicNumbersRunnerTest extends TestCase { + /** Empty text selection is an explicit no-op, without querying or starting a process. */ + public function testEmptyTextSelectionNeverStartsTheTool(): void + { + $output = new BufferedOutput(); + $runner = $this->runnerWithoutExecution($output); + + self::assertSame(0, $runner->run([], 'text', '0,1', null, null)); + self::assertSame('No PHP files selected.' . PHP_EOL, $output->fetch()); + } + + /** Empty XML exposes the installed engine identity without claiming an analyzer invocation. */ + public function testEmptyXmlSelectionNeverStartsTheTool(): void + { + $output = new BufferedOutput(); + $runner = $this->runnerWithoutExecution($output); + + self::assertSame(0, $runner->run([], 'xml', '0,1', null, null)); + $document = new DOMDocument(); + self::assertTrue($document->loadXML($output->fetch(), LIBXML_NONET)); + $root = $document->documentElement; + self::assertNotNull($root); + self::assertSame('phpmnd', $root->tagName); + self::assertSame(InstalledVersions::getPrettyVersion('povils/phpmnd'), $root->getAttribute('version')); + self::assertSame('0', $root->getAttribute('fileCount')); + self::assertSame('0', $root->getAttribute('errorCount')); + self::assertSame(0, $document->getElementsByTagName('entry')->length); + self::assertSame(1, $document->getElementsByTagName('files')->length); + } + + /** Validation also applies to empty selections and cannot emit a false clean report. */ + #[DataProvider('invalidOptions')] + public function testInvalidOptionsNeverStartTheTool(string $format, string $numbers, ?string $extensions, string $diagnostic): void + { + $output = new BufferedOutput(); + $runner = $this->runnerWithoutExecution($output); + + try { + $runner->run([], $format, $numbers, null, $extensions); + self::fail('Invalid analysis options must not return a clean empty result.'); + } catch (InvalidArgumentException $exception) { + self::assertStringContainsString($diagnostic, $exception->getMessage()); + } + + self::assertSame('', $output->fetch()); + } + + /** @return iterable Invalid public options and their diagnostic contracts. */ + public static function invalidOptions(): iterable + { + yield 'unsupported format' => ['json', '0,1', null, 'Format must be text or xml']; + yield 'empty format' => ['', '0,1', null, 'Format must be text or xml']; + yield 'not a number' => ['text', 'invalid', null, 'finite numeric values']; + yield 'not a finite number' => ['text', 'NAN', null, 'finite numeric values']; + yield 'positive infinity' => ['text', 'INF', null, 'finite numeric values']; + yield 'negative infinity' => ['text', '-INF', null, 'finite numeric values']; + yield 'overflowing number' => ['text', '1e9999', null, 'finite numeric values']; + yield 'empty numeric entry' => ['text', '0,1,', null, 'finite numeric values']; + yield 'empty extensions' => ['text', '0,1', '', 'name native PHPMND analysis contexts']; + yield 'empty extension entry' => ['text', '0,1', 'all,,argument', 'name native PHPMND analysis contexts']; + yield 'unknown extension' => ['text', '0,1', 'missing-context', 'does not exist']; + } + + /** XML output and native policy overrides remain literal arguments in one bounded invocation. */ + public function testXmlOptionsAndNativeStatusArePreserved(): void + { + $file = tempnam(sys_get_temp_dir(), 'standards-mnd-unit-'); + self::assertNotFalse($file); + $source = 'createMock(ProcessRunner::class); + $entrypoint = '/owned/vendor/povils/phpmnd/bin/phpmnd'; + $process->expects(self::never())->method('capture'); + $process->expects(self::once())->method('run')->with( + [ + PHP_BINARY, + $entrypoint, + '--no-ansi', + '--ignore-numbers=42', + '--ignore-funcs=output,fixture', + '--extensions=all,-argument', + '--quiet', + '--xml-output=php://stdout', + '--', + $file, + ], + '/owned/vendor/povils/phpmnd', + )->willReturn(17); + $output = new BufferedOutput(); + $runner = new MagicNumbersRunner($process, $output, $entrypoint); + + try { + self::assertSame(17, $runner->run([$file], 'xml', '42', 'output,fixture', 'all,-argument')); + self::assertSame('', $output->fetch()); + self::assertSame($source, file_get_contents($file)); + } finally { + unlink($file); + } + } + + /** A missing file exercises real read failure without relying on platform-specific permission behavior. */ + public function testMissingXmlSourceFailsBeforeExecutionOrOutput(): void + { + $file = sys_get_temp_dir() . '/standards-mnd-missing-' . bin2hex(random_bytes(8)) . '.php'; + self::assertFileDoesNotExist($file); + $output = new BufferedOutput(); + $runner = $this->runnerWithoutExecution($output); + + try { + $runner->run([$file], 'xml', '0,1', null, null); + self::fail('An unreadable XML source must not return a clean result.'); + } catch (InvalidArgumentException $exception) { + self::assertStringContainsString('could not read a selected source', $exception->getMessage()); + } + + self::assertSame('', $output->fetch()); + self::assertFileDoesNotExist($file); + } + /** Oversized arguments fail before execution, without truncation or partial reporting. */ public function testOversizedInvocationNeverStartsTheTool(): void { @@ -54,4 +175,14 @@ public function testXmlInvalidPathsFailBeforeExecutionOrOutput(): void } } } + + /** Both transport methods must remain unused for rejected inputs and empty selections. */ + private function runnerWithoutExecution(BufferedOutput $output): MagicNumbersRunner + { + $process = $this->createMock(ProcessRunner::class); + $process->expects(self::never())->method('capture'); + $process->expects(self::never())->method('run'); + + return new MagicNumbersRunner($process, $output, '/owned/vendor/povils/phpmnd/bin/phpmnd'); + } } diff --git a/tests/Integration/MagicNumbersTest.php b/tests/Integration/MagicNumbersTest.php index 4a9808b..8fac938 100644 --- a/tests/Integration/MagicNumbersTest.php +++ b/tests/Integration/MagicNumbersTest.php @@ -56,7 +56,7 @@ public function testNativeXmlPreservesPathsAndSourceWithoutConsumerAutoload(): v } sort($paths, SORT_STRING); self::assertSame(array_map(fn(string $path): string => $this->directory . '/' . $path, $paths), $reported); - self::assertSame('3', $document->documentElement->getAttribute('errorCount')); + self::assertSame('3', $this->errorCount($document)); self::assertSame($result->getOutput(), $this->cli(['--all', '--format=xml'])->getOutput()); foreach ($paths as $path) { self::assertSame($source, file_get_contents($this->directory . '/' . $path)); @@ -73,7 +73,7 @@ public function testOverridesAndTextStatus(): void self::assertSame(1, $this->cli(['--all'])->getExitCode()); $override = $this->cli(['--all', '--format=xml', '--ignore-numbers=0,1,42']); self::assertSame(0, $override->getExitCode(), $override->getErrorOutput()); - self::assertSame('0', $this->xml($override->getOutput())->documentElement->getAttribute('errorCount')); + self::assertSame('0', $this->errorCount($this->xml($override->getOutput()))); $this->put('source.php', 'cli(['--all'])->getExitCode()); @@ -88,15 +88,15 @@ public function testNumericExceptionsAreAdditiveAndFinite(): void $this->git(['add', '--all']); $empty = $this->cli(['--all', '--format=xml', '--ignore-numbers=']); self::assertSame(1, $empty->getExitCode(), $empty->getErrorOutput()); - self::assertSame('1', $this->xml($empty->getOutput())->documentElement->getAttribute('errorCount')); + self::assertSame('1', $this->errorCount($this->xml($empty->getOutput()))); $addition = $this->cli(['--all', '--format=xml', '--ignore-numbers=42']); self::assertSame(0, $addition->getExitCode(), $addition->getErrorOutput()); - self::assertSame('0', $this->xml($addition->getOutput())->documentElement->getAttribute('errorCount')); + self::assertSame('0', $this->errorCount($this->xml($addition->getOutput()))); $this->put('source.php', 'cli(['--all', '--format=xml', '--ignore-numbers=' . $number]); self::assertSame(0, $addition->getExitCode(), $addition->getErrorOutput()); - self::assertSame('0', $this->xml($addition->getOutput())->documentElement->getAttribute('errorCount')); + self::assertSame('0', $this->errorCount($this->xml($addition->getOutput()))); } $overflow = $this->cli(['--all', '--format=xml', '--ignore-numbers=1e9999']); self::assertSame(2, $overflow->getExitCode()); @@ -135,7 +135,9 @@ public function testXmlRoundTripsCdataTerminator(): void $this->git(['add', '--all']); $result = $this->cli(['--all', '--format=xml']); self::assertSame(1, $result->getExitCode(), $result->getErrorOutput()); - self::assertSame(rtrim($source, "\n"), $this->xml($result->getOutput())->getElementsByTagName('snippet')->item(0)->textContent); + $snippet = $this->xml($result->getOutput())->getElementsByTagName('snippet')->item(0); + self::assertNotNull($snippet); + self::assertSame(rtrim($source, "\n"), $snippet->textContent); self::assertSame($source, file_get_contents($this->directory . '/source.php')); } @@ -205,7 +207,12 @@ public function testNativeParseFailureIsNotReportedAsClean(): void self::assertSame($source, file_get_contents($this->directory . '/broken.php')); } - /** A real DOM-free runtime permits text execution but rejects XML before process execution or stdout. */ + /** + * Exercises actual extension availability; a mocked capability callback would not prove this runtime boundary. + * + * A real DOM-free subprocess permits the recording text transport but rejects XML before transport or stdout. + * Hosts with statically available DOM skip honestly rather than simulating its absence in production code. + */ public function testDomFreeRuntimeKeepsTextAndRejectsXml(): void { $probe = new Process([PHP_BINARY, '-n', '-r', 'exit(extension_loaded("dom") ? 1 : 0);'], timeout: 10); @@ -251,6 +258,15 @@ private function xml(string $output): DOMDocument return $document; } + /** Requires the native report root before reading its finding count. */ + private function errorCount(DOMDocument $document): string + { + $root = $document->documentElement; + self::assertNotNull($root); + + return $root->getAttribute('errorCount'); + } + /** Writes only within this test's exclusive synthetic fixture. */ private function put(string $path, string $source): void {