Parse PHPStan JSON in flymake-phpstan, matching flycheck - #69
Merged
Conversation
`flymake-phpstan' asked PHPStan for the `raw' format and scraped
`file:line:message' with a regexp, so it showed only the bare message and
none of the metadata `flycheck-phpstan' already surfaced.
Switch it to `--error-format=json' and parse with the shared
`phpstan--parse-json', so a Flymake session reaches parity with Flycheck:
* each diagnostic carries the identifier and tip, gated by
`flymake-phpstan-ignore-metadata-list' and joined with
`flymake-phpstan-metadata-separator' (mirrors of the flycheck options);
* `phpstan--ignorable-errors' and `phpstan--dumped-types' are refreshed
in the source buffer, so `phpstan-insert-ignore' and
`phpstan-copy-dumped-type' work from Flymake, not just Flycheck;
* the JSON is located by the first line starting with `{', so the
progress a container runtime writes to STDERR is skipped; and
* output with no JSON report is surfaced -- silent for "No files found
to analyse." in a modified buffer, a warning otherwise -- rather than
scraped into nothing.
Diagnostics are now reported at `:error' level (they were `:warning'),
matching the severity flycheck uses.
Tests in test/flymake-phpstan-test.el cover message building, the metadata
toggle, the STDERR-prefixed and no-JSON output shapes, and the
ignorable-errors side effect; each was checked to fail when its behaviour
is reverted. Verified end to end against local PHPStan and, through a
container, Apple `container'.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings
flymake-phpstanup toflycheck-phpstan's level by parsing PHPStan's JSON output.Why
flymake-phpstanasked for therawformat and scrapedfile:line:messagewith a regexp, so it showed only the bare message.flycheck-phpstanhas read JSON for a while and surfaces much more — identifier, tip, and the metadata that powersphpstan-insert-ignore/phpstan-copy-dumped-type. This closes that gap.What changes
--error-format=json, parsed with the sharedphpstan--parse-json. A Flymake session now:flymake-phpstan-ignore-metadata-listand joined withflymake-phpstan-metadata-separator— mirrors of the existing flycheck options;phpstan--ignorable-errorsandphpstan--dumped-typesin the source buffer, sophpstan-insert-ignoreandphpstan-copy-dumped-typework from Flymake, not just Flycheck;{, so the progress a container writes to STDERR is skipped (same handling as flycheck);Diagnostics are now
:errorlevel (were:warning), matching flycheck.Verification
Ran the backend for real:
phpstan--ignorable-errorspopulated((4 "function.notFound") (7 "constant.notFound") …).container: 2 diagnostics with identifiers — confirms container path normalization (config and target rewritten to/app/…) and that the STDERR progress is skipped.test/flymake-phpstan-test.eladds ERT coverage (message building, metadata toggle, STDERR-prefixed and no-JSON shapes, the ignorable-errors side effect); each test was checked to fail when its behaviour is reverted. 21/21 pass on system Emacs and Emacs 27.2; flymake-phpstan.el compiles warning-free on both.Follow-up (not in this PR)
While testing I found that
phpstan--parse-jsonrepresents JSONfalseas:false, which is truthy in Elisp — so a non-ignorable message is currently treated as ignorable by both backends (identifier shown, offered tophpstan-insert-ignore). That's a shared, pre-existing bug in the parser; I left it out of this PR to keep the flymake change focused and filed it separately. This PR matches flycheck's current behaviour exactly.