Merge container fixes from #63 and #64

#63 and #64 were merged into their base branches rather than master:
#62 landed on master first, which left the rest of the stack behind on
feature/flycheck-generic-checker and fix/container-path-normalization.

Bring in the four commits that never reached master.

Author: zonuexe

CHANGELOG.md

@@ -30,6 +30,12 @@ All notable changes of the `phpstan.el` are documented in this file using the [K
 * Fix `phpstan-executable` in the `(STRING . (ARGUMENTS ...))` form dropping the command name, which made the first *argument* run as the program (`("docker" "run" ...)` executed `run`).
 * Fix `phpstan-get-command-args` destructively modifying its inputs with `nconc`.  Each call appended the PHPStan arguments onto the caller's own list, growing `phpstan-executable` in the `(STRING . (ARGUMENTS ...))` form on every check, and appending `"--"` to `phpstan-generate-baseline-options` on every `phpstan-generate-baseline`.
 * Fix Flycheck getting stuck on a syntax check when PHPStan reported no files to analyse in a modified buffer.  The check now finishes with an empty result instead of never reporting a status.
+* Fix the analyzed file being passed to a containerized PHPStan as a host path.  `flycheck-phpstan` reported no errors at all with `(phpstan-executable . docker)`, because PHPStan answered `Path /Users/... does not exist`.  `flymake-phpstan` was affected for unmodified buffers.
+* Fix the `--tmp-file` copy being created in the system temporary directory when running containerized, where the container cannot see it.
+* Fix the JSON report being ignored when the container runtime prefixes it with progress output on STDERR, which made every check with `(phpstan-executable . container)` report no errors.
+* Fix `flycheck-phpstan` silently discarding the fallback warning when PHPStan produced no JSON report, hiding failures such as a broken configuration file behind a clean buffer.
+* Fix editor mode detection asking the wrong program for its version.  Only the first element of the command line was probed, which is the container runtime for `(phpstan-executable . docker)` / `container` and `php` for a PHAR without the executable bit — so `docker --version` and `php --version` were parsed as PHPStan versions (`d1c06ef`, `Technologies`) and editor mode was silently disabled for every setup except a directly executable `phpstan`.
+* `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.
 
 ## [0.9.0]
 

flycheck-phpstan.el

@@ -69,18 +69,26 @@
 ;; Parsing PHPStan output:
 (defun flycheck-phpstan-parse-output (output &optional _checker _buffer)
   "Parse PHPStan errors from OUTPUT."
-  (let* ((json-buffer (with-current-buffer (flycheck-phpstan--temp-buffer)
-                        (erase-buffer)
-                        (insert output)
-                        (current-buffer)))
-         (data (if (string-prefix-p "{" output)
-                   (phpstan--parse-json json-buffer)
-                 (list (flycheck-error-new-at 1 1 'warning (string-trim output)))))
-         (errors (phpstan--plist-to-alist (plist-get data :files))))
-    (unless phpstan-disable-buffer-errors
-      (phpstan-update-ignorebale-errors-from-json-buffer errors))
-    (phpstan-update-dumped-types errors)
-    (flycheck-phpstan--build-errors errors)))
+  ;; Look for a line starting with `{', the same condition
+  ;; `phpstan--parse-json' acts on: it skips everything before that line so
+  ;; that output written to STDERR is ignored, since the checker process
+  ;; merges STDERR into STDOUT.  Anchoring at the start of OUTPUT instead
+  ;; would miss the JSON whenever the runtime prefixes it, as Apple container
+  ;; does with its progress report.
+  (if (not (string-match-p "^{" output))
+      ;; PHPStan produced no report at all, so OUTPUT is a failure of some
+      ;; kind.  Surface it rather than reporting a clean buffer.
+      (list (flycheck-error-new-at 1 1 'warning (string-trim output)))
+    (let* ((json-buffer (with-current-buffer (flycheck-phpstan--temp-buffer)
+                          (erase-buffer)
+                          (insert output)
+                          (current-buffer)))
+           (data (phpstan--parse-json json-buffer))
+           (errors (phpstan--plist-to-alist (plist-get data :files))))
+      (unless phpstan-disable-buffer-errors
+        (phpstan-update-ignorebale-errors-from-json-buffer errors))
+      (phpstan-update-dumped-types errors)
+      (flycheck-phpstan--build-errors errors))))
 
 (defun flycheck-phpstan--temp-buffer ()
   "Return a temporary buffer for decode JSON."

flymake-phpstan.el

@@ -95,8 +95,9 @@
 
 (defun flymake-phpstan--create-temp-file ()
   "Create temp file and return the path."
-  (phpstan-normalize-path
-   (flymake-proc-init-create-temp-buffer-copy 'flymake-proc-create-temp-inplace)))
+  ;; `phpstan-get-command-args' translates the path for the container mount
+  ;; point, so return the path as it is on this side.
+  (flymake-proc-init-create-temp-buffer-copy 'flymake-proc-create-temp-inplace))
 
 (defun flymake-phpstan (report-fn &rest _ignored-args)
   "Flymake backend for PHPStan report using REPORT-FN."

phpstan.el

@@ -568,15 +568,27 @@ it returns the value of `SOURCE' as it is."
              (phpstan-use-xdebug-option (list "--xdebug")))
             options
             (when editor
-              (let ((original-file (plist-get editor :original-file)))
+              (let* ((original-file (plist-get editor :original-file))
+                     ;; PHPStan may see the project through a mount point, so
+                     ;; every path handed to it has to be translated, exactly
+                     ;; like the config file above.
+                     (target-file (phpstan-normalize-path original-file)))
                 (cond
                  ((funcall (plist-get editor :analyze-original) original-file)
-                  (list "--" original-file))
-                 ((phpstan-editor-mode-available-p (car (phpstan-get-executable-and-args)))
-                  (list "--tmp-file" (funcall (plist-get editor :temp-file))
-                        "--instead-of" original-file
-                        "--" original-file))
-                 ((list "--" (funcall (plist-get editor :inplace)))))))
+                  (list "--" target-file))
+                 ((phpstan-editor-mode-available-p executable-and-args)
+                  ;; A container only sees the project, so the temporary copy
+                  ;; has to be created inside it.  `:temp-file' puts it in the
+                  ;; system temporary directory, which is not mounted.
+                  (let ((temp-file (funcall (plist-get editor
+                                                       (if (phpstan--container-executable-p)
+                                                           :inplace
+                                                         :temp-file)))))
+                    (list "--tmp-file" (phpstan-normalize-path temp-file)
+                          "--instead-of" target-file
+                          "--" target-file)))
+                 ((list "--" (phpstan-normalize-path
+                              (funcall (plist-get editor :inplace))))))))
             (if editor args (cons "--" args)))))
 
 (defun phpstan-update-ignorebale-errors-from-json-buffer (errors)
@@ -601,36 +613,71 @@ it returns the value of `SOURCE' as it is."
                                    collect (cons (plist-get message :line)
                                                  (substring-no-properties msg (match-end 0))))))))
 
-(defun phpstan-version (executable)
-  "Return the PHPStan version of EXECUTABLE."
-  (if-let* ((cached-entry (assoc executable phpstan-executable-versions-alist)))
-      (cdr cached-entry)
-    (let* ((version (thread-first
-                      (mapconcat #'shell-quote-argument (list executable "--version") " ")
-                      (shell-command-to-string)
-                      (string-trim-right)
-                      (split-string " ")
-                      (last)
-                      (car-safe))))
-      (prog1 version
-        (push (cons executable version) phpstan-executable-versions-alist)))))
-
-(defun phpstan-editor-mode-available-p (executable)
-  "Check if the specified PHPStan EXECUTABLE supports editor mode.
-
-If a cached result for EXECUTABLE exists, it is returned directly.
+(defun phpstan--version-output (command)
+  "Run COMMAND with --version and return its standard output, or NIL.
+
+STDERR is discarded rather than merged: a container runtime reports its
+progress there, and it would otherwise be read as part of the version."
+  (with-temp-buffer
+    (let ((status (apply #'process-file (car command) nil (list t nil) nil
+                         (append (cdr command) (list "--version")))))
+      (when (eq 0 status)
+        (buffer-string)))))
+
+(defun phpstan--version-from-output (output)
+  "Return the version number reported in OUTPUT, or NIL.
+
+OUTPUT looks like \"PHPStan - PHP Static Analysis Tool 1.12.33\"."
+  (when output
+    (let ((last-line (car (last (split-string (string-trim output) "\n" t)))))
+      (when last-line
+        (car (last (split-string last-line " " t)))))))
+
+(defun phpstan-version (command)
+  "Return the PHPStan version of COMMAND.
+
+COMMAND is the command line that runs PHPStan, as returned by
+`phpstan-get-executable-and-args'.  A bare string is also accepted, and
+taken as the name of an executable.
+
+Passing the whole command line matters: PHPStan is not always the program
+being executed.  It is `docker' or `container' for a containerized PHPStan,
+and `php' for a PHAR without the executable bit, and asking either of those
+for its version answers a version that has nothing to do with PHPStan.
+
+The result is cached in `phpstan-executable-versions-alist', keyed by the
+command line, because probing may have to start a container."
+  (let* ((command (if (listp command) command (list command)))
+         (cache-key (mapconcat #'identity command " ")))
+    (if-let* ((cached-entry (assoc cache-key phpstan-executable-versions-alist)))
+        (cdr cached-entry)
+      (let ((version (phpstan--version-from-output
+                      (phpstan--version-output command))))
+        (prog1 version
+          (push (cons cache-key version) phpstan-executable-versions-alist))))))
+
+(defun phpstan-editor-mode-available-p (command)
+  "Check if the PHPStan invoked by COMMAND supports editor mode.
+
+COMMAND is the command line that runs PHPStan, as returned by
+`phpstan-get-executable-and-args'.  A bare string is also accepted, and
+taken as the name of an executable.
+
+If a cached result for COMMAND exists, it is returned directly.
 Otherwise, this function attempts to determine support by retrieving
-the PHPStan version using `phpstan --version' command."
+the PHPStan version using `phpstan --version' command.  Support is
+assumed to be absent when the version cannot be determined."
   (pcase phpstan-activate-editor-mode
     ('enabled t)
     ('disabled nil)
     ('nil
-     (let* ((version (phpstan-version executable)))
-       (if (string-match-p (eval-when-compile (regexp-quote "-dev@")) version)
-           t
-         (pcase (elt version 0)
-           (?1 (version<= "1.12.27" version))
-           (?2 (version<= "2.1.17" version))))))))
+     (let ((version (phpstan-version command)))
+       (when (and version (not (string-empty-p version)))
+         (if (string-match-p (eval-when-compile (regexp-quote "-dev@")) version)
+             t
+           (pcase (elt version 0)
+             (?1 (version<= "1.12.27" version))
+             (?2 (version<= "2.1.17" version)))))))))
 
 (defconst phpstan--re-ignore-tag
   (eval-when-compile