From b90e364ed0409418576452b43fa7c0175af355db Mon Sep 17 00:00:00 2001 From: Aaron Ware Date: Sat, 12 Sep 2026 11:20:15 -0400 Subject: [PATCH] feat(NO-TASK): Share the branch-scoped coding-standards check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `linchpin/actions`' `php-checks.yml` runs `composer check-branch-cs`, so every repository calling that shared workflow has to provide it. Each one carried its own copy under `dev/Composer/Actions.php` — mantle and block-alchemy both did, and the two had already drifted. Discovery was about to add a third. One copy here, in a package all of them already require as a dev dependency, is one place to fix a bug in. Scoped to changed *lines* rather than changed files, which is the one behavioural difference from the copies it replaces. Sniffing each changed file whole is fine on a clean tree and wrong on any other: editing one line of a file carrying inherited findings reports all of them against the pull request that touched it, so the check fails for reasons its author did not cause and cannot fix in passing — and a gate like that gets switched off rather than fixed. On the repository this was written for, one such change reported 397 findings, of which 5 were on a line it had touched. `check_staged_cs` is new: the same filtering against a pre-commit diff, so a husky hook can catch a violation where it is cheapest to fix. It diffs the working tree against HEAD rather than the index, because lint-staged stashes unstaged work before running tasks and re-stages their modifications only after every task finishes — reading `--cached` reports the errors an autofixer task fixed a moment earlier. `run()` is public and takes plain arguments so it can be exercised without constructing a Composer event. Written to this package's PHP 8.0 floor rather than the callers' 8.4. Co-Authored-By: Claude Opus 5 (1M context) --- Linchpin/Composer/Actions.php | 320 ++++++++++++++++++++++++++++++++++ composer.json | 4 +- 2 files changed, 323 insertions(+), 1 deletion(-) create mode 100644 Linchpin/Composer/Actions.php diff --git a/Linchpin/Composer/Actions.php b/Linchpin/Composer/Actions.php new file mode 100644 index 0000000..051d6d5 --- /dev/null +++ b/Linchpin/Composer/Actions.php @@ -0,0 +1,320 @@ +getArguments(); + $compare = empty($arguments) ? 'main' : (string) $arguments[0]; + + exit(self::run([ escapeshellarg($compare) ])); + } + + /** + * Check the lines a commit is about to introduce. + * + * For a pre-commit hook. Diffs the working tree against HEAD rather than the index, + * because lint-staged stashes unstaged work before running tasks and re-stages their + * modifications only after every task finishes — so reading `--cached` reports the + * errors an autofixer task fixed a moment earlier. + * + * @param Event $event Composer event. Takes no arguments; the diff names the set. + * @return void + */ + public static function check_staged_cs( Event $event ) + { + unset($event); + + exit(self::run([ 'HEAD' ])); + } + + /** + * Run PHPCS and report only what falls on the changed lines. + * + * Public so it can be exercised without constructing a Composer event. + * + * @param array $diff_arguments Arguments identifying the diff, already shell-escaped. + * @param string $standard PHPCS standard to run against. + * @return int Zero when nothing was reported on a changed line. + */ + public static function run( array $diff_arguments, $standard = 'phpcs.xml.dist' ) + { + $diff_spec = implode(' ', $diff_arguments); + $files = self::changed_files($diff_spec); + + if (empty($files) ) { + echo 'No PHP files changed.' . PHP_EOL; + + return 0; + } + + $changed_lines = self::changed_lines($diff_spec); + $report = self::phpcs_report($files, $standard); + + if (null === $report ) { + echo 'PHPCS produced no parseable report.' . PHP_EOL; + + return 1; + } + + $findings = self::findings_on_changed_lines($report, $changed_lines); + + if (empty($findings) ) { + printf('PHPCS: no findings on the %d changed line(s).%s', self::count_lines($changed_lines), PHP_EOL); + + return 0; + } + + self::print_findings($findings); + + // The checkstyle report is what `cs2pr` turns into inline annotations, and it is + // written here rather than asked of PHPCS because PHPCS's own would carry every + // inherited finding in each touched file — which is what the filtering removes. + if (false !== getenv('CI') ) { + self::write_checkstyle($findings, './phpcs-report.xml'); + } + + return 1; + } + + /** + * The PHP files in the diff that still exist. + * + * @param string $diff_spec Escaped diff arguments. + * @return array + */ + protected static function changed_files( $diff_spec ) + { + $files = []; + + // `--diff-filter=d` drops deletions: a removed file has nothing left to sniff. + exec('git diff --name-only --diff-filter=d ' . $diff_spec, $files); + + $php_files = []; + + foreach ( $files as $file ) { + if (substr($file, -4) === '.php' && file_exists($file) ) { + $php_files[] = $file; + } + } + + return $php_files; + } + + /** + * The line numbers the diff adds or modifies, keyed by file. + * + * Read from a zero-context diff, so each hunk header names exactly the new lines rather + * than those plus three either side. + * + * @param string $diff_spec Escaped diff arguments. + * @return array + */ + protected static function changed_lines( $diff_spec ) + { + $output = []; + + exec('git diff -U0 --diff-filter=d ' . $diff_spec, $output); + + $lines = []; + $current = ''; + + foreach ( $output as $line ) { + if (strpos($line, '+++ b/') === 0 ) { + $current = substr($line, 6); + $lines[ $current ] = []; + + continue; + } + + if ('' === $current || strpos($line, '@@') !== 0 ) { + continue; + } + + // @@ -old,count +new,count @@ — only the "+" side matters. + if (! preg_match('/^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/', $line, $matches) ) { + continue; + } + + $start = (int) $matches[1]; + $count = isset($matches[2]) ? (int) $matches[2] : 1; + + for ( $i = 0; $i < $count; $i++ ) { + $lines[ $current ][ $start + $i ] = true; + } + } + + return $lines; + } + + /** + * PHPCS's JSON report for the given files. + * + * @param array $files Files to sniff. + * @param string $standard PHPCS standard. + * @return array|null Decoded report, or null when it could not be read. + */ + protected static function phpcs_report( array $files, $standard ) + { + $binary = file_exists('./vendor/bin/phpcs') ? './vendor/bin/phpcs' : 'phpcs'; + + $command = sprintf( + '%s --standard=%s --report=json %s 2>/dev/null', + escapeshellarg($binary), + escapeshellarg($standard), + implode(' ', array_map('escapeshellarg', $files)) + ); + + $output = shell_exec($command); + + if (! is_string($output) || '' === trim($output) ) { + return null; + } + + $decoded = json_decode($output, true); + + return is_array($decoded) && isset($decoded['files']) ? $decoded : null; + } + + /** + * Keep only the messages sitting on a line the diff touched. + * + * PHPCS reports absolute paths; the diff speaks in repository-relative ones. + * + * @param array $report Decoded PHPCS report. + * @param array $changed_lines Changed lines keyed by file. + * @return array + */ + protected static function findings_on_changed_lines( array $report, array $changed_lines ) + { + $root = rtrim((string) getcwd(), '/') . '/'; + $findings = []; + + foreach ( $report['files'] as $path => $data ) { + $path = (string) $path; + $relative = strpos($path, $root) === 0 ? substr($path, strlen($root)) : $path; + + if (! isset($changed_lines[ $relative ]) ) { + continue; + } + + $messages = isset($data['messages']) ? $data['messages'] : []; + + foreach ( $messages as $message ) { + $line = isset($message['line']) ? (int) $message['line'] : 0; + + if (isset($changed_lines[ $relative ][ $line ]) ) { + $findings[ $relative ][] = $message; + } + } + } + + return $findings; + } + + /** + * Print the findings in something close to PHPCS's own full report. + * + * @param array $findings Findings keyed by file. + * @return void + */ + protected static function print_findings( array $findings ) + { + $total = 0; + + foreach ( $findings as $file => $messages ) { + echo PHP_EOL . 'FILE: ' . $file . PHP_EOL; + echo str_repeat('-', 80) . PHP_EOL; + + foreach ( $messages as $message ) { + ++$total; + + printf( + '%4d | %-7s | %s (%s)%s', + isset($message['line']) ? (int) $message['line'] : 0, + isset($message['type']) ? (string) $message['type'] : 'ERROR', + isset($message['message']) ? (string) $message['message'] : '', + isset($message['source']) ? (string) $message['source'] : '', + PHP_EOL + ); + } + } + + echo PHP_EOL . sprintf('PHPCS: %d finding(s) on changed lines.', $total) . PHP_EOL; + } + + /** + * Write a checkstyle report holding only the filtered findings. + * + * @param array $findings Findings keyed by file. + * @param string $path Where to write. + * @return void + */ + protected static function write_checkstyle( array $findings, $path ) + { + $xml = new \SimpleXMLElement(''); + + foreach ( $findings as $file => $messages ) { + $file_node = $xml->addChild('file'); + $file_node->addAttribute('name', $file); + + foreach ( $messages as $message ) { + $type = isset($message['type']) ? $message['type'] : ''; + $error = $file_node->addChild('error'); + $error->addAttribute('line', (string) ( isset($message['line']) ? $message['line'] : 0 )); + $error->addAttribute('column', (string) ( isset($message['column']) ? $message['column'] : 1 )); + $error->addAttribute('severity', 'WARNING' === $type ? 'warning' : 'error'); + $error->addAttribute('message', (string) ( isset($message['message']) ? $message['message'] : '' )); + $error->addAttribute('source', (string) ( isset($message['source']) ? $message['source'] : '' )); + } + } + + $xml->asXML($path); + } + + /** + * How many lines the diff touched, across every file. + * + * @param array $changed_lines Changed lines keyed by file. + * @return int + */ + protected static function count_lines( array $changed_lines ) + { + return array_sum(array_map('count', $changed_lines)); + } +} diff --git a/composer.json b/composer.json index 3d37d65..3610083 100644 --- a/composer.json +++ b/composer.json @@ -23,7 +23,9 @@ "phpunit/phpunit": "^13.0" }, "scripts": { - "test": "phpunit" + "test": "phpunit", + "check-branch-cs": "Linchpin\\Composer\\Actions::check_branch_cs", + "check-staged-cs": "Linchpin\\Composer\\Actions::check_staged_cs" }, "config": { "allow-plugins": {