Skip to content

Commit c46b6c8

Browse files
zonuexezonuexe
authored
Merge pull request #69 from emacs-php/feature/flymake-json
Parse PHPStan JSON in flymake-phpstan, matching flycheck
2 parents 2de1e0b + 9d38351 commit c46b6c8

3 files changed

Lines changed: 203 additions & 25 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ All notable changes of the `phpstan.el` are documented in this file using the [K
2323
* `php` is no longer required in `exec-path` to enable the checker. Previously the dummy `"php"` in `:command` was resolved by Flycheck *before* the `:enabled` predicate ran, so a Docker-only setup could not enable the checker at all.
2424
* `M-x flycheck-verify-setup` now reports the real PHPStan command and configuration file instead of `"php"`.
2525
* `phpstan-flycheck-auto-set-executable` is obsolete and ignored.
26+
* `flymake-phpstan` now reads PHPStan's JSON output instead of the raw text format, bringing it to parity with `flycheck-phpstan`.
27+
* Each message shows its identifier and tip (configurable with `flymake-phpstan-ignore-metadata-list` and `flymake-phpstan-metadata-separator`).
28+
* `phpstan-insert-ignore` and `phpstan-copy-dumped-type` now work from a Flymake session, because the backend refreshes `phpstan--ignorable-errors` and `phpstan--dumped-types`.
29+
* Progress a container runtime writes to STDERR no longer confuses the parser, and a PHPStan failure that produces no report is surfaced as a warning rather than dropped.
2630

2731
### Fixed
2832

‎flymake-phpstan.el‎

Lines changed: 87 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@
3131
;;
3232
;; (add-hook 'php-mode-hook #'flymake-phpstan-turn-on)
3333
;;
34+
;; Like `flycheck-phpstan', this backend reads PHPStan's JSON output, so the
35+
;; identifier and tip of each message are shown, and `phpstan-insert-ignore'
36+
;; and `phpstan-copy-dumped-type' work from a Flymake session too.
37+
;;
3438
;; For Lisp maintainers: see [GNU Flymake manual - 2.2.2 An annotated example backend]
3539
;; https://www.gnu.org/software/emacs/manual/html_node/flymake/An-annotated-example-backend.html
3640

@@ -53,8 +57,82 @@
5357
:type 'boolean
5458
:group 'flymake-phpstan)
5559

60+
(defcustom flymake-phpstan-ignore-metadata-list nil
61+
"Set of metadata items to ignore in PHPStan messages for Flymake."
62+
:type '(set (const identifier)
63+
(const tip))
64+
:group 'flymake-phpstan)
65+
66+
(defcustom flymake-phpstan-metadata-separator "\n"
67+
"Separator of PHPStan message and metadata."
68+
:type 'string
69+
:safe #'stringp
70+
:group 'flymake-phpstan)
71+
72+
(defconst flymake-phpstan--nofiles-message
73+
(eval-when-compile (regexp-quote "[ERROR] No files found to analyse.")))
74+
5675
(defvar-local flymake-phpstan--proc nil)
5776

77+
(defun flymake-phpstan--build-message (message)
78+
"Build the diagnostic text for a PHPStan MESSAGE plist.
79+
80+
Append the identifier and tip, unless disabled by
81+
`flymake-phpstan-ignore-metadata-list', mirroring `flycheck-phpstan'."
82+
(let* ((msg (plist-get message :message))
83+
(ignorable (plist-get message :ignorable))
84+
(identifier (unless (memq 'identifier flymake-phpstan-ignore-metadata-list)
85+
(plist-get message :identifier)))
86+
(tip (unless (memq 'tip flymake-phpstan-ignore-metadata-list)
87+
(plist-get message :tip)))
88+
(lines (delq nil
89+
(list (when (and identifier ignorable)
90+
(concat phpstan-identifier-prefix identifier))
91+
(when tip
92+
(concat phpstan-tip-message-prefix tip))))))
93+
(if lines
94+
(concat msg flymake-phpstan-metadata-separator (string-join lines "\n"))
95+
msg)))
96+
97+
(defun flymake-phpstan--build-diagnostics (errors source)
98+
"Build Flymake diagnostics for SOURCE from PHPStan ERRORS.
99+
100+
ERRORS is the alist produced by `phpstan--plist-to-alist' from the JSON
101+
`:files' object. Every message is attributed to SOURCE by its line, since
102+
editor mode analyzes the one file being edited."
103+
(cl-loop for (_file . entry) in errors
104+
append (cl-loop for message in (plist-get entry :messages)
105+
for text = (flymake-phpstan--build-message message)
106+
for (beg . end) = (flymake-diag-region
107+
source (plist-get message :line))
108+
collect (flymake-make-diagnostic source beg end :error text))))
109+
110+
(defun flymake-phpstan--parse (output source)
111+
"Parse PHPStan OUTPUT and return Flymake diagnostics for SOURCE.
112+
113+
As a side effect, refresh `phpstan--ignorable-errors' and
114+
`phpstan--dumped-types' in SOURCE, so `phpstan-insert-ignore' and
115+
`phpstan-copy-dumped-type' work from Flymake too."
116+
;; Look for a line starting with `{', the condition `phpstan--parse-json'
117+
;; acts on: it skips anything before that line, so progress a container
118+
;; runtime writes to STDERR (merged into STDOUT here) is ignored.
119+
(if (not (string-match-p "^{" output))
120+
;; No report. A modified buffer with nothing to analyse is expected and
121+
;; stays silent; anything else is surfaced as a warning.
122+
(if (string-match-p flymake-phpstan--nofiles-message output)
123+
nil
124+
(list (flymake-make-diagnostic source (point-min) (point-max)
125+
:warning (string-trim output))))
126+
(with-temp-buffer
127+
(insert output)
128+
(let ((errors (phpstan--plist-to-alist
129+
(plist-get (phpstan--parse-json (current-buffer)) :files))))
130+
(with-current-buffer source
131+
(unless phpstan-disable-buffer-errors
132+
(phpstan-update-ignorebale-errors-from-json-buffer errors))
133+
(phpstan-update-dumped-types errors))
134+
(flymake-phpstan--build-diagnostics errors source)))))
135+
58136
(defun flymake-phpstan-make-process (root command-args report-fn source)
59137
"Make PHPStan process by ROOT, COMMAND-ARGS, REPORT-FN and SOURCE."
60138
(let ((default-directory root))
@@ -64,30 +142,14 @@
64142
:command command-args
65143
:sentinel
66144
(lambda (proc _event)
67-
(pcase (process-status proc)
68-
(`exit
69-
(unwind-protect
70-
(when (with-current-buffer source (eq proc flymake-phpstan--proc))
71-
(with-current-buffer (process-buffer proc)
72-
(goto-char (point-min))
73-
(cl-loop
74-
while (search-forward-regexp
75-
(eval-when-compile
76-
(rx line-start (1+ (not (any ":"))) ":"
77-
(group-n 1 (one-or-more (not (any ":")))) ":"
78-
(group-n 2 (one-or-more not-newline)) line-end))
79-
nil t)
80-
for msg = (match-string 2)
81-
for (beg . end) = (flymake-diag-region
82-
source
83-
(string-to-number (match-string 1)))
84-
for type = :warning
85-
collect (flymake-make-diagnostic source beg end type msg)
86-
into diags
87-
finally (funcall report-fn diags)))
88-
(flymake-log :warning "Canceling obsolete check %s" proc))
89-
(kill-buffer (process-buffer proc))))
90-
(code (user-error "PHPStan error (exit status: %s)" code)))))))
145+
(when (eq (process-status proc) 'exit)
146+
(unwind-protect
147+
(when (with-current-buffer source (eq proc flymake-phpstan--proc))
148+
(funcall report-fn
149+
(flymake-phpstan--parse
150+
(with-current-buffer (process-buffer proc) (buffer-string))
151+
source)))
152+
(kill-buffer (process-buffer proc))))))))
91153

92154
(defun flymake-phpstan-analyze-original (original)
93155
"Return non-NIL if ORIGINAL is non-NIL and buffer is not modified."
@@ -109,7 +171,7 @@
109171
(let* ((source (current-buffer))
110172
(args (phpstan-get-command-args
111173
:include-executable t
112-
:format "raw"
174+
:format "json"
113175
:editor (list
114176
:analyze-original #'flymake-phpstan-analyze-original
115177
:original-file buffer-file-name

‎test/flymake-phpstan-test.el‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
;;; flymake-phpstan-test.el --- Tests for flymake-phpstan.el -*- lexical-binding: t; -*-
2+
3+
;; Copyright (C) 2025 Friends of Emacs-PHP development
4+
5+
;; License: GPL-3.0-or-later
6+
7+
;; This program is free software; you can redistribute it and/or modify
8+
;; it under the terms of the GNU General Public License as published by
9+
;; the Free Software Foundation, either version 3 of the License, or
10+
;; (at your option) any later version.
11+
12+
;; This program is distributed in the hope that it will be useful,
13+
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
;; GNU General Public License for more details.
16+
17+
;; You should have received a copy of the GNU General Public License
18+
;; along with this program. If not, see <https://www.gnu.org/licenses/>.
19+
20+
;;; Commentary:
21+
22+
;; ERT tests for `flymake-phpstan's JSON parsing: the metadata a message
23+
;; carries, the output shapes a container runtime produces, and the side
24+
;; effect that feeds `phpstan-insert-ignore'.
25+
26+
;;; Code:
27+
(require 'ert)
28+
(require 'cl-lib)
29+
(require 'flymake-phpstan)
30+
31+
(defconst flymake-phpstan-test--json
32+
(concat "{\"totals\":{\"errors\":0,\"file_errors\":2},"
33+
"\"files\":{\"/app/test.php\":{\"errors\":2,\"messages\":["
34+
"{\"message\":\"Function f not found.\",\"line\":4,\"ignorable\":true,"
35+
"\"identifier\":\"function.notFound\",\"tip\":\"Learn more.\"},"
36+
"{\"message\":\"Constant Fooo not found.\",\"line\":7,\"ignorable\":true,"
37+
"\"identifier\":\"constant.notFound\"}"
38+
"]}},\"errors\":[]}")
39+
"A PHPStan JSON report with two errors, one carrying a tip.")
40+
41+
;;; Message building
42+
43+
(ert-deftest flymake-phpstan-test-build-message ()
44+
(let ((flymake-phpstan-ignore-metadata-list nil)
45+
(phpstan-identifier-prefix "ID:")
46+
(phpstan-tip-message-prefix "TIP:")
47+
(flymake-phpstan-metadata-separator "\n"))
48+
;; Message, identifier (ignorable), and tip.
49+
(should (equal "Function f not found.\nID:function.notFound\nTIP:Learn more."
50+
(flymake-phpstan--build-message
51+
'(:message "Function f not found." :line 4 :ignorable t
52+
:identifier "function.notFound" :tip "Learn more."))))
53+
;; A plain message with no metadata is returned unchanged.
54+
(should (equal "Bare."
55+
(flymake-phpstan--build-message '(:message "Bare." :line 1))))))
56+
57+
(ert-deftest flymake-phpstan-test-build-message-ignore-metadata ()
58+
"`flymake-phpstan-ignore-metadata-list' drops the chosen metadata."
59+
(let ((phpstan-identifier-prefix "ID:")
60+
(phpstan-tip-message-prefix "TIP:")
61+
(flymake-phpstan-metadata-separator "\n"))
62+
(let ((flymake-phpstan-ignore-metadata-list '(identifier tip)))
63+
(should (equal "Just the message."
64+
(flymake-phpstan--build-message
65+
'(:message "Just the message." :line 1 :ignorable t
66+
:identifier "x.y" :tip "hint.")))))))
67+
68+
;;; Parsing
69+
70+
(ert-deftest flymake-phpstan-test-parse-updates-ignorable-errors ()
71+
"Parsing a report refreshes `phpstan--ignorable-errors' in the source.
72+
This is what lets `phpstan-insert-ignore' work from Flymake."
73+
(with-temp-buffer
74+
(let ((source (current-buffer))
75+
(phpstan-disable-buffer-errors nil))
76+
(flymake-phpstan--parse flymake-phpstan-test--json source)
77+
(should (equal '((4 "function.notFound") (7 "constant.notFound"))
78+
phpstan--ignorable-errors)))))
79+
80+
(ert-deftest flymake-phpstan-test-parse-json-with-stderr-prefix ()
81+
"The report is found even when a container prefixes it with progress."
82+
(with-temp-buffer
83+
(let* ((source (current-buffer))
84+
(phpstan-disable-buffer-errors t)
85+
(output (concat "[0/6] Fetching image\n"
86+
"[6/6] Starting container\n"
87+
flymake-phpstan-test--json))
88+
(diags (flymake-phpstan--parse output source)))
89+
(should (= 2 (length diags)))
90+
(should (cl-every (lambda (d) (eq :error (flymake-diagnostic-type d))) diags)))))
91+
92+
(ert-deftest flymake-phpstan-test-parse-nofiles-is-silent ()
93+
"\"No files found to analyse.\" produces no diagnostics."
94+
(with-temp-buffer
95+
(let ((source (current-buffer)))
96+
(should-not (flymake-phpstan--parse
97+
" [ERROR] No files found to analyse." source)))))
98+
99+
(ert-deftest flymake-phpstan-test-parse-other-non-json-is-surfaced ()
100+
"Any other non-JSON output is surfaced as a warning, not dropped.
101+
A container that cannot start, for example, must not look like success."
102+
(with-temp-buffer
103+
(let* ((source (current-buffer))
104+
(diags (flymake-phpstan--parse
105+
"failed to connect to the docker API" source)))
106+
(should (= 1 (length diags)))
107+
(should (eq :warning (flymake-diagnostic-type (car diags))))
108+
(should (string-match-p "docker API"
109+
(flymake-diagnostic-text (car diags)))))))
110+
111+
(provide 'flymake-phpstan-test)
112+
;;; flymake-phpstan-test.el ends here

0 commit comments

Comments
 (0)