PHPCS does actually support reading data from stdin, so let's make ArcanistExternalLinter aware of this.
- Group Reviewers
- Maniphest Tasks
- T7071: PHPCS is broken
- rARC3d5aa332fe6d: PHPCS supports reading from stdin.
The problem with this diff is that it actually breaks some functionality within phpcs ruleset, e.g. it no longer can follow exclude-pattern setting for specific standards. Maybe it would make sense to use configuration to define should phpcs be used with stdin, or not? Currently the only workaround I see for this is setting every rule in .arclint with different flags instead of using usual ruleset.xml, but that would be a mess.
Now the linter does also fail when you have an empty php file. Instead of showing the correct message
arc lint foo.php >>> Lint for foo.php: Warning (PHPCS.W.Internal.NoCodeFound) No PHP code was found in this file and short open tags are not allowed by this install of PHP. This file may be using short open tags but PHP does not allow them. >>> 1
arc lint foo.php Exception Some linters failed: - ArcanistPhpcsLinter: CommandException: Command failed with error #2! COMMAND 'vendor/bin/phpcs' '--report=xml' '--standard=PSR2' STDOUT ERROR: You must supply at least one file or directory to process. Usage: phpcs [-nwlsaepvi] [-d key[=value]] [--colors] [--no-colors] [--report=<report>] [--report-file=<reportFile>] [--report-<report>=<reportFile>] ... [--report-width=<reportWidth>] [--generator=<generator>] [--tab-width=<tabWidth>] [--severity=<severity>] [--error-severity=<severity>] [--warning-severity=<severity>] [--runtime-set key value] [--config-set key value] [--config-delete key] [--config-show] [--standard=<standard>] [--sniffs=<sniffs>] [--encoding=<encoding>] [--extensions=<extensions>] [--ignore=<patterns>] <file> ... Set runtime value (see --config-set) -n Do not print warnings (shortcut for --warning-severity=0) -w Print both warnings and errors (this is the default) -l Local directory only, no recursion -s Show sniff codes in all reports -a Run interactively ... (2,049 more bytes) ... STDERR (empty) (Run with `--trace` for a full exception trace.)
What I don't know is why STDIN support was enabled for this linter. If it was enabled just because PHPCS supports STDIN, then surely commit must be reverted because in STDIN mode PHPCS capabilities are limited compared to mode, when you provide file path to it.
From that I can get that making PHPCS accept input over STDIN was made to satisfy hooks at some point. But since commit hooks are now removed then we can change that sniff behavior to accept data from files as it should.