Merge pull request #70 from emacs-php/fix/json-false-object

Map JSON false and null to nil in phpstan--parse-json

Author: zonuexe

CHANGELOG.md

@@ -42,6 +42,7 @@ All notable changes of the `phpstan.el` are documented in this file using the [K
 * `phpstan-version` and `phpstan-editor-mode-available-p` now take the whole command line, as returned by `phpstan-get-executable-and-args`.  A bare string is still accepted.  `phpstan-version` no longer merges STDERR into the version string, which a container runtime pollutes with its progress report.
 * Fix `declare-function` forms for `tramp` that quoted the function name and argument list (and misspelled `tramp` as `tamp`), so the byte compiler warned that `tramp-dissect-file-name` might not be defined at runtime.
 * Fix a container run erroring when the project has no configuration file.  `phpstan-normalize-path` was handed the nil from `phpstan-get-config-file` and passed it to `replace-regexp-in-string`; it now returns nil for a nil path, so the command line simply omits `-c`.
+* Fix `phpstan--parse-json` reading JSON `false` (and, on `json-parse-buffer`, `null`) as a truthy symbol.  A non-ignorable message (`"ignorable":false`) was treated as ignorable by both backends, so its identifier was shown with the 🪪 prefix and `phpstan-insert-ignore` offered it — even though it cannot be ignored.  Both parser paths now map `false`/`null` to nil.
 
 ### Removed
 

phpstan.el

@@ -429,11 +429,18 @@ it returns the value of `SOURCE' as it is."
       (when (search-forward-regexp "^{" nil t)
         (backward-char 1)
         (delete-region (point-min) (point))))
+    ;; Map JSON false and null to nil.  PHPStan's `ignorable' is a boolean read
+    ;; in a truthy context, and by default both parsers return a truthy symbol
+    ;; for false (`:false' / `:json-false'); `json-parse-buffer' also returns a
+    ;; truthy `:null'.  Without this, a non-ignorable message looks ignorable,
+    ;; and the two parsers disagree on null.
     (if (eval-when-compile (and (fboundp 'json-serialize)
                                 (fboundp 'json-parse-buffer)))
         (with-no-warnings
-          (json-parse-buffer :object-type 'plist :array-type 'list))
-      (let ((json-object-type 'plist) (json-array-type 'list))
+          (json-parse-buffer :object-type 'plist :array-type 'list
+                             :false-object nil :null-object nil))
+      (let ((json-object-type 'plist) (json-array-type 'list)
+            (json-false nil) (json-null nil))
         (json-read-object)))))
 
 (defun phpstan--expand-file-name (name)

test/flycheck-phpstan-test.el

@@ -25,6 +25,7 @@
 
 ;;; Code:
 (require 'ert)
+(require 'cl-lib)
 (require 'flycheck-phpstan)
 
 (defconst flycheck-phpstan-test--json
@@ -66,5 +67,24 @@ A swallowed fallback would show a failing PHPStan as a clean buffer."
     (should (string-match-p "Bootstrap file"
                             (flycheck-error-message (car errors))))))
 
+(ert-deftest flycheck-phpstan-test-identifier-only-when-ignorable ()
+  "The identifier prefix is shown only for an ignorable message.
+A non-ignorable message (\"ignorable\":false) reads its flag as nil now, so
+it must not look like something `phpstan-insert-ignore' can act on."
+  (let* ((phpstan-disable-buffer-errors t)
+         (phpstan-identifier-prefix "ID:")
+         (flycheck-phpstan-ignore-metadata-list nil)
+         (output (concat "{\"files\":{\"/x\":{\"messages\":["
+                         "{\"message\":\"parse error\",\"line\":1,\"ignorable\":false,"
+                         "\"identifier\":\"ignore.parseError\"},"
+                         "{\"message\":\"undefined\",\"line\":2,\"ignorable\":true,"
+                         "\"identifier\":\"variable.undefined\"}]}}}"))
+         (errors (flycheck-phpstan-parse-output output))
+         (by-line (lambda (n) (car (cl-remove-if-not
+                                    (lambda (e) (= n (flycheck-error-line e))) errors)))))
+    (should-not (string-match-p "ID:" (flycheck-error-message (funcall by-line 1))))
+    (should (string-match-p "ID:variable.undefined"
+                            (flycheck-error-message (funcall by-line 2))))))
+
 (provide 'flycheck-phpstan-test)
 ;;; flycheck-phpstan-test.el ends here

test/flymake-phpstan-test.el

@@ -77,6 +77,19 @@ This is what lets `phpstan-insert-ignore' work from Flymake."
       (should (equal '((4 "function.notFound") (7 "constant.notFound"))
                      phpstan--ignorable-errors)))))
 
+(ert-deftest flymake-phpstan-test-parse-skips-non-ignorable ()
+  "A non-ignorable message (\"ignorable\":false) is not offered to ignore."
+  (with-temp-buffer
+    (let ((source (current-buffer))
+          (phpstan-disable-buffer-errors nil)
+          (json (concat "{\"files\":{\"/x\":{\"messages\":["
+                        "{\"message\":\"parse error\",\"line\":1,\"ignorable\":false,"
+                        "\"identifier\":\"ignore.parseError\"},"
+                        "{\"message\":\"undefined\",\"line\":2,\"ignorable\":true,"
+                        "\"identifier\":\"variable.undefined\"}]}}}")))
+      (flymake-phpstan--parse json source)
+      (should (equal '((2 "variable.undefined")) 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

test/phpstan-test.el

@@ -35,6 +35,33 @@
                  (phpstan--plist-to-alist '(:a 1 :b 2))))
   (should (equal nil (phpstan--plist-to-alist nil))))
 
+;;; JSON parsing
+
+(ert-deftest phpstan-test-parse-json-false-is-nil ()
+  "JSON false and null read as nil, not a truthy symbol.
+`ignorable' is read in a truthy context, so `false' must be nil."
+  (with-temp-buffer
+    (insert "{\"a\":false,\"b\":null,\"c\":true}")
+    (let ((data (phpstan--parse-json (current-buffer))))
+      (should-not (plist-get data :a))
+      (should-not (plist-get data :b))
+      (should (eq t (plist-get data :c))))))
+
+(ert-deftest phpstan-test-update-ignorable-skips-non-ignorable ()
+  "Only ignorable messages feed `phpstan--ignorable-errors'.
+A non-ignorable message (\"ignorable\":false) must not be offered to
+`phpstan-insert-ignore'."
+  (with-temp-buffer
+    (insert (concat "{\"files\":{\"/x\":{\"messages\":["
+                    "{\"message\":\"parse error\",\"line\":1,\"ignorable\":false,"
+                    "\"identifier\":\"ignore.parseError\"},"
+                    "{\"message\":\"undefined\",\"line\":2,\"ignorable\":true,"
+                    "\"identifier\":\"variable.undefined\"}]}}}"))
+    (let ((errors (phpstan--plist-to-alist
+                   (plist-get (phpstan--parse-json (current-buffer)) :files))))
+      (phpstan-update-ignorebale-errors-from-json-buffer errors)
+      (should (equal '((2 "variable.undefined")) phpstan--ignorable-errors)))))
+
 ;;; Container runtime detection
 
 (ert-deftest phpstan-test-container-runtime-command ()