From 9d38351928a2f929fcdb18dea0c561b01ed1fd46 Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Sat, 18 Jul 2026 01:35:36 +0900 Subject: [PATCH] Parse PHPStan JSON in flymake-phpstan, matching flycheck `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'. --- CHANGELOG.md | 4 ++ flymake-phpstan.el | 112 +++++++++++++++++++++++++++-------- test/flymake-phpstan-test.el | 112 +++++++++++++++++++++++++++++++++++ 3 files changed, 203 insertions(+), 25 deletions(-) create mode 100644 test/flymake-phpstan-test.el diff --git a/CHANGELOG.md b/CHANGELOG.md index fe58097..a1d30e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ All notable changes of the `phpstan.el` are documented in this file using the [K * `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. * `M-x flycheck-verify-setup` now reports the real PHPStan command and configuration file instead of `"php"`. * `phpstan-flycheck-auto-set-executable` is obsolete and ignored. +* `flymake-phpstan` now reads PHPStan's JSON output instead of the raw text format, bringing it to parity with `flycheck-phpstan`. + * Each message shows its identifier and tip (configurable with `flymake-phpstan-ignore-metadata-list` and `flymake-phpstan-metadata-separator`). + * `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`. + * 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. ### Fixed diff --git a/flymake-phpstan.el b/flymake-phpstan.el index 80bc794..ce5da89 100644 --- a/flymake-phpstan.el +++ b/flymake-phpstan.el @@ -31,6 +31,10 @@ ;; ;; (add-hook 'php-mode-hook #'flymake-phpstan-turn-on) ;; +;; Like `flycheck-phpstan', this backend reads PHPStan's JSON output, so the +;; identifier and tip of each message are shown, and `phpstan-insert-ignore' +;; and `phpstan-copy-dumped-type' work from a Flymake session too. +;; ;; For Lisp maintainers: see [GNU Flymake manual - 2.2.2 An annotated example backend] ;; https://www.gnu.org/software/emacs/manual/html_node/flymake/An-annotated-example-backend.html @@ -53,8 +57,82 @@ :type 'boolean :group 'flymake-phpstan) +(defcustom flymake-phpstan-ignore-metadata-list nil + "Set of metadata items to ignore in PHPStan messages for Flymake." + :type '(set (const identifier) + (const tip)) + :group 'flymake-phpstan) + +(defcustom flymake-phpstan-metadata-separator "\n" + "Separator of PHPStan message and metadata." + :type 'string + :safe #'stringp + :group 'flymake-phpstan) + +(defconst flymake-phpstan--nofiles-message + (eval-when-compile (regexp-quote "[ERROR] No files found to analyse."))) + (defvar-local flymake-phpstan--proc nil) +(defun flymake-phpstan--build-message (message) + "Build the diagnostic text for a PHPStan MESSAGE plist. + +Append the identifier and tip, unless disabled by +`flymake-phpstan-ignore-metadata-list', mirroring `flycheck-phpstan'." + (let* ((msg (plist-get message :message)) + (ignorable (plist-get message :ignorable)) + (identifier (unless (memq 'identifier flymake-phpstan-ignore-metadata-list) + (plist-get message :identifier))) + (tip (unless (memq 'tip flymake-phpstan-ignore-metadata-list) + (plist-get message :tip))) + (lines (delq nil + (list (when (and identifier ignorable) + (concat phpstan-identifier-prefix identifier)) + (when tip + (concat phpstan-tip-message-prefix tip)))))) + (if lines + (concat msg flymake-phpstan-metadata-separator (string-join lines "\n")) + msg))) + +(defun flymake-phpstan--build-diagnostics (errors source) + "Build Flymake diagnostics for SOURCE from PHPStan ERRORS. + +ERRORS is the alist produced by `phpstan--plist-to-alist' from the JSON +`:files' object. Every message is attributed to SOURCE by its line, since +editor mode analyzes the one file being edited." + (cl-loop for (_file . entry) in errors + append (cl-loop for message in (plist-get entry :messages) + for text = (flymake-phpstan--build-message message) + for (beg . end) = (flymake-diag-region + source (plist-get message :line)) + collect (flymake-make-diagnostic source beg end :error text)))) + +(defun flymake-phpstan--parse (output source) + "Parse PHPStan OUTPUT and return Flymake diagnostics for SOURCE. + +As a side effect, refresh `phpstan--ignorable-errors' and +`phpstan--dumped-types' in SOURCE, so `phpstan-insert-ignore' and +`phpstan-copy-dumped-type' work from Flymake too." + ;; Look for a line starting with `{', the condition `phpstan--parse-json' + ;; acts on: it skips anything before that line, so progress a container + ;; runtime writes to STDERR (merged into STDOUT here) is ignored. + (if (not (string-match-p "^{" output)) + ;; No report. A modified buffer with nothing to analyse is expected and + ;; stays silent; anything else is surfaced as a warning. + (if (string-match-p flymake-phpstan--nofiles-message output) + nil + (list (flymake-make-diagnostic source (point-min) (point-max) + :warning (string-trim output)))) + (with-temp-buffer + (insert output) + (let ((errors (phpstan--plist-to-alist + (plist-get (phpstan--parse-json (current-buffer)) :files)))) + (with-current-buffer source + (unless phpstan-disable-buffer-errors + (phpstan-update-ignorebale-errors-from-json-buffer errors)) + (phpstan-update-dumped-types errors)) + (flymake-phpstan--build-diagnostics errors source))))) + (defun flymake-phpstan-make-process (root command-args report-fn source) "Make PHPStan process by ROOT, COMMAND-ARGS, REPORT-FN and SOURCE." (let ((default-directory root)) @@ -64,30 +142,14 @@ :command command-args :sentinel (lambda (proc _event) - (pcase (process-status proc) - (`exit - (unwind-protect - (when (with-current-buffer source (eq proc flymake-phpstan--proc)) - (with-current-buffer (process-buffer proc) - (goto-char (point-min)) - (cl-loop - while (search-forward-regexp - (eval-when-compile - (rx line-start (1+ (not (any ":"))) ":" - (group-n 1 (one-or-more (not (any ":")))) ":" - (group-n 2 (one-or-more not-newline)) line-end)) - nil t) - for msg = (match-string 2) - for (beg . end) = (flymake-diag-region - source - (string-to-number (match-string 1))) - for type = :warning - collect (flymake-make-diagnostic source beg end type msg) - into diags - finally (funcall report-fn diags))) - (flymake-log :warning "Canceling obsolete check %s" proc)) - (kill-buffer (process-buffer proc)))) - (code (user-error "PHPStan error (exit status: %s)" code))))))) + (when (eq (process-status proc) 'exit) + (unwind-protect + (when (with-current-buffer source (eq proc flymake-phpstan--proc)) + (funcall report-fn + (flymake-phpstan--parse + (with-current-buffer (process-buffer proc) (buffer-string)) + source))) + (kill-buffer (process-buffer proc)))))))) (defun flymake-phpstan-analyze-original (original) "Return non-NIL if ORIGINAL is non-NIL and buffer is not modified." @@ -109,7 +171,7 @@ (let* ((source (current-buffer)) (args (phpstan-get-command-args :include-executable t - :format "raw" + :format "json" :editor (list :analyze-original #'flymake-phpstan-analyze-original :original-file buffer-file-name diff --git a/test/flymake-phpstan-test.el b/test/flymake-phpstan-test.el new file mode 100644 index 0000000..a424919 --- /dev/null +++ b/test/flymake-phpstan-test.el @@ -0,0 +1,112 @@ +;;; flymake-phpstan-test.el --- Tests for flymake-phpstan.el -*- lexical-binding: t; -*- + +;; Copyright (C) 2025 Friends of Emacs-PHP development + +;; License: GPL-3.0-or-later + +;; This program is free software; you can redistribute it and/or modify +;; it under the terms of the GNU General Public License as published by +;; the Free Software Foundation, either version 3 of the License, or +;; (at your option) any later version. + +;; This program is distributed in the hope that it will be useful, +;; but WITHOUT ANY WARRANTY; without even the implied warranty of +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;; GNU General Public License for more details. + +;; You should have received a copy of the GNU General Public License +;; along with this program. If not, see . + +;;; Commentary: + +;; ERT tests for `flymake-phpstan's JSON parsing: the metadata a message +;; carries, the output shapes a container runtime produces, and the side +;; effect that feeds `phpstan-insert-ignore'. + +;;; Code: +(require 'ert) +(require 'cl-lib) +(require 'flymake-phpstan) + +(defconst flymake-phpstan-test--json + (concat "{\"totals\":{\"errors\":0,\"file_errors\":2}," + "\"files\":{\"/app/test.php\":{\"errors\":2,\"messages\":[" + "{\"message\":\"Function f not found.\",\"line\":4,\"ignorable\":true," + "\"identifier\":\"function.notFound\",\"tip\":\"Learn more.\"}," + "{\"message\":\"Constant Fooo not found.\",\"line\":7,\"ignorable\":true," + "\"identifier\":\"constant.notFound\"}" + "]}},\"errors\":[]}") + "A PHPStan JSON report with two errors, one carrying a tip.") + +;;; Message building + +(ert-deftest flymake-phpstan-test-build-message () + (let ((flymake-phpstan-ignore-metadata-list nil) + (phpstan-identifier-prefix "ID:") + (phpstan-tip-message-prefix "TIP:") + (flymake-phpstan-metadata-separator "\n")) + ;; Message, identifier (ignorable), and tip. + (should (equal "Function f not found.\nID:function.notFound\nTIP:Learn more." + (flymake-phpstan--build-message + '(:message "Function f not found." :line 4 :ignorable t + :identifier "function.notFound" :tip "Learn more.")))) + ;; A plain message with no metadata is returned unchanged. + (should (equal "Bare." + (flymake-phpstan--build-message '(:message "Bare." :line 1)))))) + +(ert-deftest flymake-phpstan-test-build-message-ignore-metadata () + "`flymake-phpstan-ignore-metadata-list' drops the chosen metadata." + (let ((phpstan-identifier-prefix "ID:") + (phpstan-tip-message-prefix "TIP:") + (flymake-phpstan-metadata-separator "\n")) + (let ((flymake-phpstan-ignore-metadata-list '(identifier tip))) + (should (equal "Just the message." + (flymake-phpstan--build-message + '(:message "Just the message." :line 1 :ignorable t + :identifier "x.y" :tip "hint."))))))) + +;;; Parsing + +(ert-deftest flymake-phpstan-test-parse-updates-ignorable-errors () + "Parsing a report refreshes `phpstan--ignorable-errors' in the source. +This is what lets `phpstan-insert-ignore' work from Flymake." + (with-temp-buffer + (let ((source (current-buffer)) + (phpstan-disable-buffer-errors nil)) + (flymake-phpstan--parse flymake-phpstan-test--json source) + (should (equal '((4 "function.notFound") (7 "constant.notFound")) + phpstan--ignorable-errors))))) + +(ert-deftest flymake-phpstan-test-parse-json-with-stderr-prefix () + "The report is found even when a container prefixes it with progress." + (with-temp-buffer + (let* ((source (current-buffer)) + (phpstan-disable-buffer-errors t) + (output (concat "[0/6] Fetching image\n" + "[6/6] Starting container\n" + flymake-phpstan-test--json)) + (diags (flymake-phpstan--parse output source))) + (should (= 2 (length diags))) + (should (cl-every (lambda (d) (eq :error (flymake-diagnostic-type d))) diags))))) + +(ert-deftest flymake-phpstan-test-parse-nofiles-is-silent () + "\"No files found to analyse.\" produces no diagnostics." + (with-temp-buffer + (let ((source (current-buffer))) + (should-not (flymake-phpstan--parse + " [ERROR] No files found to analyse." source))))) + +(ert-deftest flymake-phpstan-test-parse-other-non-json-is-surfaced () + "Any other non-JSON output is surfaced as a warning, not dropped. +A container that cannot start, for example, must not look like success." + (with-temp-buffer + (let* ((source (current-buffer)) + (diags (flymake-phpstan--parse + "failed to connect to the docker API" source))) + (should (= 1 (length diags))) + (should (eq :warning (flymake-diagnostic-type (car diags)))) + (should (string-match-p "docker API" + (flymake-diagnostic-text (car diags))))))) + +(provide 'flymake-phpstan-test) +;;; flymake-phpstan-test.el ends here