diff --git a/CHANGELOG.md b/CHANGELOG.md index 450ad09..5f45a41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,14 +13,23 @@ All notable changes of the `phpstan.el` are documented in this file using the [K * Add `phpstan-hover-show-kind-label` custom variable to toggle verbose labels like `return:` / `yield:` in hover text. * Add `phpstan-hover-message-prefix` custom variable preset choices, including emoji labels. * `phpstan-copy-dumped-type` now prefers PHPDoc type from hover data by default, and can copy non-PHPDoc type with a prefix argument (C-u). +* Add `container` to `phpstan-executable` to run PHPStan with [Apple container](https://github.com/apple/container) on macOS. It uses the same `phpstan-docker-image` as `docker`. ### Changed * `phpstan-copy-dumped-type` command now prioritizes `phpstan-hover-mode` data at point before falling back to dumped-type messages. +* `flycheck-phpstan` is now a Flycheck *generic* checker instead of a command checker. + * The whole command line is built from `phpstan-executable`, so the checker no longer injects an executable into flycheck's `flycheck-phpstan-executable` variable as a side effect of its `:enabled` predicate, and no longer advises `flycheck-finish-checker-process`. + * `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. ### Fixed * Fix `phpstan-get-command-args` to keep `:options` in the correct position and pass target arguments correctly when editor mode options are used. +* 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. ## [0.9.0] diff --git a/README.org b/README.org index 7b673e1..ed8ef86 100644 --- a/README.org +++ b/README.org @@ -66,6 +66,20 @@ If you always use Docker for PHPStan, add the following into your ~.emacs~ file (setq-default phpstan-executable 'docker) #+END_SRC +*** Using Apple container (macOS) +[[https://github.com/apple/container][Apple container]] runs the same OCI images as Docker with the same ~run~ command line, so the ~phpstan/phpstan~ image works unchanged. It requires an Apple Silicon Mac (macOS 26 recommended). + +#+BEGIN_SRC shell +$ brew install container +$ container system start +#+END_SRC + +#+BEGIN_SRC emacs-lisp +(setq-default phpstan-executable 'container) +#+END_SRC + +The image is taken from ~phpstan-docker-image~, the same variable Docker uses. + Put the following into ~.dir-locals.el~ files on the root directory of project. #+BEGIN_SRC emacs-lisp ((nil . ((php-project-root . git) @@ -191,14 +205,15 @@ Rule level of PHPStan analysis. Please see [[https://github.com/phpstan/phpstan - STRING :: Absolute path to `phpstan' executable file. - ex) ~"/path/to/phpstan.phar"~ - SYMBOL ~docker~ :: Use Docker using phpstan/docker-image. +- SYMBOL ~container~ :: Use [[https://github.com/apple/container][Apple container]] (macOS) using phpstan/docker-image. - ~(root . STRING)~ :: Relative path to `phpstan' executable file from project root directory. - ex) ~(root . "script/phpstan")~ - ~(STRING . (ARGUMENTS ...))~ :: Command name and arguments. - ex) ~("docker" "run" "--rm" "-v" "/path/to/project-dir/:/app" "your/docker-image")~ - ~nil~ :: Auto detect ~phpstan~ executable file by composer dependencies of the project or executable command in ~PATH~ environment variable. -*** Custom variable ~phpstan-flycheck-auto-set-executable~ -Set flycheck phpstan-executable automatically when non-NIL. +*** Custom variable ~phpstan-flycheck-auto-set-executable~ (obsolete) +Obsolete and ignored. ~flycheck-phpstan~ now builds the whole command line from ~phpstan-executable~, so it no longer injects an executable into flycheck's ~flycheck-phpstan-executable~ variable. *** Custom variable ~phpstan-memory-limit~ Use phpstan memory limit option when non-NIL. diff --git a/flycheck-phpstan.el b/flycheck-phpstan.el index 69c563b..f590551 100644 --- a/flycheck-phpstan.el +++ b/flycheck-phpstan.el @@ -36,15 +36,22 @@ ;; ;; (add-hook 'php-mode-hook 'my-php-mode-setup) ;; +;; ## For Lisp maintainers +;; +;; This is a generic checker (`flycheck-define-generic-checker'), not a command +;; checker (`flycheck-define-checker'). A command checker takes its executable +;; from the car of `:command', which must be a literal string, overridable only +;; through the single string variable `flycheck-CHECKER-executable'. PHPStan +;; does not fit that shape: `phpstan-executable' may expand to a whole command +;; line such as `docker run --rm -v ...', and it is chosen per project. So we +;; drive the process ourselves and build the command from `phpstan-executable'. ;;; Code: +(require 'cl-lib) (require 'flycheck) (require 'phpstan) -;; Usually it is defined dynamically by flycheck -(defvar flycheck-phpstan-executable) (defvar flycheck-phpstan--temp-buffer-name "*Flycheck PHPStan*") -(defvar flycheck-phpstan--output-filter-added nil) (defconst flycheck-phpstan--nofiles-message (eval-when-compile (regexp-quote "[ERROR] No files found to analyse."))) (defcustom flycheck-phpstan-ignore-metadata-list nil @@ -59,44 +66,7 @@ :safe #'stringp :group 'phpstan) -(defun flycheck-phpstan--suppress-no-files-error (next checker exit-status files output callback cwd) - "Suppress Flycheck errors if PHPStan reports no files in a modified buffer. - -This function is intended to be used as an :around advice for -`flycheck-finish-checker-process'. - -It prevents Flycheck from displaying an error when: -- CHECKER is `phpstan', -- the current buffer is modified, -- and OUTPUT contains the message `flycheck-phpstan--nofiles-message'. - -NEXT, EXIT-STATUS, FILES, OUTPUT, CALLBACK, and CWD are the original arguments -passed to `flycheck-finish-checker-process'." - (unless (and (eq checker 'phpstan) - (buffer-modified-p) - (string-match-p flycheck-phpstan--nofiles-message output)) - (funcall next checker exit-status files output callback cwd))) - -(defun flycheck-phpstan--enabled-and-set-variable () - "Return path to phpstan configure file, and set buffer execute in side effect." - (let ((enabled (phpstan-enabled))) - (prog1 enabled - (unless flycheck-phpstan--output-filter-added - (advice-add 'flycheck-finish-checker-process - :around #'flycheck-phpstan--suppress-no-files-error) - (setq flycheck-phpstan--output-filter-added t)) - (when (and enabled - phpstan-flycheck-auto-set-executable - (null (bound-and-true-p flycheck-phpstan-executable)) - (or (stringp phpstan-executable) - (eq 'docker phpstan-executable) - (and (eq 'root (car-safe phpstan-executable)) - (stringp (cdr-safe phpstan-executable))) - (and (stringp (car-safe phpstan-executable)) - (listp (cdr-safe phpstan-executable))) - (null phpstan-executable))) - (setq-local flycheck-phpstan-executable (car (phpstan-get-executable-and-args))))))) - +;; 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) @@ -143,21 +113,171 @@ passed to `flycheck-finish-checker-process'." "Return non-NIL if ORIGINAL is non-NIL and buffer is not modified." (and original (not (buffer-modified-p)))) -(flycheck-define-checker phpstan +;; Running PHPStan: +(defun flycheck-phpstan--command () + "Return the whole PHPStan command line to check the current buffer. + +This has the side effect of saving the buffer to a temporary file, which is +registered in `flycheck-temporaries' for later deletion." + (phpstan-get-command-args + :include-executable t + :format "json" + :editor (list + :analyze-original #'flycheck-phpstan-analyze-original + :original-file buffer-file-name + :temp-file (lambda () (flycheck-save-buffer-to-temp #'flycheck-temp-file-system)) + :inplace (lambda () (flycheck-save-buffer-to-temp #'flycheck-temp-file-inplace))))) + +(defun flycheck-phpstan--start (checker callback) + "Start CHECKER, reporting the result to CALLBACK. + +Return the process, which Flycheck hands back to `:interrupt'." + (let (process) + (condition-case err + (let ((command (flycheck-phpstan--command)) + ;; `flycheck-phpstan--nofiles-message' matches PHPStan's English + ;; output, so keep the checker process in the C locale. Only + ;; LC_MESSAGES is set, to leave the encoding of the source alone. + (process-environment (cons "LC_MESSAGES=C" process-environment))) + (setq process + (make-process + :name (format "flycheck-%s" checker) + ;; Do not associate a buffer, to avoid the side effects of + ;; attaching a process to the buffer being checked. + :buffer nil + :command command + :noquery t + :connection-type 'pipe + :filter #'flycheck-phpstan--receive-output + :sentinel #'flycheck-phpstan--handle-signal)) + (process-put process 'flycheck-phpstan-checker checker) + (process-put process 'flycheck-phpstan-callback callback) + (process-put process 'flycheck-phpstan-buffer (current-buffer)) + ;; Flycheck binds `default-directory' to `:working-directory' around + ;; this function, so remember it for resolving relative file names. + (process-put process 'flycheck-phpstan-cwd default-directory) + ;; Track the temporaries in the process itself, to get rid of the + ;; buffer-local state as soon as possible. + (process-put process 'flycheck-phpstan-temporaries flycheck-temporaries) + (setq flycheck-temporaries nil) + process) + (error + (flycheck-safe-delete-temporaries) + ;; Deleting the process triggers the sentinel, which deletes the + ;; temporary files of the process anyway. + (when process + (delete-process process)) + (signal (car err) (cdr err)))))) + +(defun flycheck-phpstan--interrupt (_checker process) + "Interrupt PROCESS." + ;; Deleting the process always triggers the sentinel, which does the cleanup. + (when process + (delete-process process))) + +(defun flycheck-phpstan--receive-output (process output) + "Accumulate OUTPUT of the PHPStan PROCESS for later parsing." + (process-put process 'flycheck-phpstan-pending-output + (cons output (process-get process 'flycheck-phpstan-pending-output)))) + +(defun flycheck-phpstan--get-output (process) + "Return the complete output of the PHPStan PROCESS." + (with-demoted-errors "Error while retrieving process output: %S" + (apply #'concat (nreverse (process-get process 'flycheck-phpstan-pending-output))))) + +(defun flycheck-phpstan--handle-signal (process _event) + "Handle a signal from the PHPStan PROCESS. + +_EVENT is ignored." + (when (memq (process-status process) '(signal exit)) + (let ((files (process-get process 'flycheck-phpstan-temporaries)) + (buffer (process-get process 'flycheck-phpstan-buffer)) + (callback (process-get process 'flycheck-phpstan-callback)) + (cwd (process-get process 'flycheck-phpstan-cwd))) + (mapc #'flycheck-safe-delete files) + (when (buffer-live-p buffer) + (with-current-buffer buffer + (condition-case err + (pcase (process-status process) + (`signal + (funcall callback 'interrupted)) + (`exit + (flycheck-phpstan--finish + (process-get process 'flycheck-phpstan-checker) + (process-exit-status process) + files + (flycheck-phpstan--get-output process) + callback cwd))) + ((debug error) + (funcall callback 'errored (error-message-string err))))))))) + +(defun flycheck-phpstan--finish (checker exit-status files output callback cwd) + "Parse OUTPUT of CHECKER and report the result to CALLBACK. + +EXIT-STATUS is the exit status of the PHPStan process. FILES is the list of +temporary files given to PHPStan, used to map reported file names back onto +the buffer. Relative file names are resolved against CWD. + +CALLBACK is always invoked with a status that finishes the syntax check, +because Flycheck gets stuck on the current check otherwise." + (if (and (buffer-modified-p) + (string-match-p flycheck-phpstan--nofiles-message output)) + ;; PHPStan found nothing to analyse because the buffer is being edited. + ;; That is not a result worth showing, but the check must still finish. + (funcall callback 'finished nil) + (let ((errors (flycheck-phpstan-parse-output output checker (current-buffer)))) + (when (and (not (equal exit-status 0)) (null errors)) + ;; Warn about a suspicious result, but keep going: `suspicious' does + ;; not finish the syntax check on its own. + (funcall callback 'suspicious + (format "Flycheck checker %S returned %S, but its output \ +contained no errors: %s\nTry installing a more recent version of PHPStan, and \ +please open a bug report if the issue persists in the latest release. Thanks!" + checker exit-status output))) + (funcall callback 'finished + ;; Fix error file names, by substituting them backwards from the + ;; temporaries. + (mapcar (lambda (e) (flycheck-fix-error-filename e files cwd)) + errors))))) + +(defun flycheck-phpstan--verify (_checker) + "Verify the PHPStan setup of the current buffer." + (let* ((executable-and-args (ignore-errors (phpstan-get-executable-and-args))) + (program (car executable-and-args)) + (found (and program + (if (file-name-absolute-p program) + (and (file-executable-p program) program) + (executable-find program)))) + (config-file (phpstan-get-config-file))) + (list + (flycheck-verification-result-new + :label "executable" + :message (cond (found (format "Found at %s" found)) + (program (format "%s not found" program)) + (t "Not found")) + :face (if found 'success '(bold error))) + (flycheck-verification-result-new + :label "command" + :message (if executable-and-args + (mapconcat #'shell-quote-argument executable-and-args " ") + "Not available") + :face (if executable-and-args 'success 'warning)) + (flycheck-verification-result-new + :label "configuration file" + :message (if config-file (format "Found at %S" config-file) "Not found") + :face (if config-file 'success 'warning))))) + +(flycheck-define-generic-checker 'phpstan "PHP static analyzer based on PHPStan." - :command ("php" - (eval - (phpstan-get-command-args - :format "json" - :editor (list - :analyze-original #'flycheck-phpstan-analyze-original - :original-file buffer-file-name - :temp-file (lambda () (flycheck-save-buffer-to-temp #'flycheck-temp-file-system)) - :inplace (lambda () (flycheck-save-buffer-to-temp #'flycheck-temp-file-inplace)))))) + :start #'flycheck-phpstan--start + :interrupt #'flycheck-phpstan--interrupt + :verify #'flycheck-phpstan--verify + ;; `flycheck-define-checker' installs this filter for command checkers, but + ;; generic checkers default to `identity'. + :error-filter #'flycheck-sanitize-errors :working-directory (lambda (_) (phpstan-get-working-dir)) - :enabled (lambda () (flycheck-phpstan--enabled-and-set-variable)) - :error-parser flycheck-phpstan-parse-output - :modes (php-mode php-ts-mode phps-mode)) + :enabled (lambda () (phpstan-enabled)) + :modes '(php-mode php-ts-mode phps-mode)) (add-to-list 'flycheck-checkers 'phpstan t) (flycheck-add-next-checker 'php 'phpstan) diff --git a/phpstan.el b/phpstan.el index 7481b71..a375387 100644 --- a/phpstan.el +++ b/phpstan.el @@ -77,8 +77,15 @@ :link '(url-link :tag "phpstan.el" "https://github.com/emacs-php/phpstan.el")) (defcustom phpstan-flycheck-auto-set-executable t - "Set flycheck phpstan-executable automatically." + "Set flycheck phpstan-executable automatically. + +This variable no longer has any effect. `flycheck-phpstan' now builds the +whole command line from `phpstan-executable', so it never has to inject an +executable into flycheck's own `flycheck-phpstan-executable' variable." :type 'boolean) +(make-obsolete-variable 'phpstan-flycheck-auto-set-executable + "the executable is always derived from `phpstan-executable'." + "0.10.0") (defcustom phpstan-enable-on-no-config-file t "If T, activate config from composer even when `phpstan.neon' is not found." @@ -94,7 +101,10 @@ :local t) (defcustom phpstan-docker-image "ghcr.io/phpstan/phpstan" - "Docker image URL or Docker Hub image name or NIL." + "Docker image URL or Docker Hub image name or NIL. + +This image is also used when `phpstan-executable' is `container', +because Apple container runs the same OCI images as Docker." :type '(choice (string :tag "URL or image name of Docker Hub.") (const :tag "Official Docker container" "ghcr.io/phpstan/phpstan") @@ -265,6 +275,15 @@ NIL (lambda (v) (or (null v) (stringp v))))) (defconst phpstan-docker-executable "docker") +(defconst phpstan-container-executable "container") + +(defconst phpstan-container-executables + (list (cons 'docker phpstan-docker-executable) + (cons 'container phpstan-container-executable)) + "Alist of `phpstan-executable' symbols and their container runtime commands. + +Both runtimes take the same `run --rm -v HOST:/app IMAGE' command line and +run the same OCI image, so they only differ in the command name.") ;;;###autoload (progn @@ -277,6 +296,9 @@ STRING `docker' Use Docker using phpstan/docker-image. +`container' + Use Apple container (macOS) using phpstan/docker-image. + `(root . STRING)' Relative path to `phpstan' executable file. @@ -289,9 +311,31 @@ NIL #'(lambda (v) (if (consp v) (or (and (eq 'root (car v)) (stringp (cdr v))) (and (stringp (car v)) (listp (cdr v)))) - (or (eq 'docker v) (null v) (stringp v)))))) + (or (memq v '(docker container)) (null v) (stringp v)))))) ;; Utilities: +(defun phpstan--container-runtime-command () + "Return the container runtime command to build a `run' command line, or NIL. + +Only the symbol forms of `phpstan-executable' (`docker' and `container') ask +phpstan.el to build the command line. The `(STRING . (ARGUMENTS ...))' form +supplies a complete command line of its own and must not be rewritten here." + (and (symbolp phpstan-executable) + (alist-get phpstan-executable phpstan-container-executables))) + +(defun phpstan--container-executable-p () + "Return non-NIL if PHPStan is executed inside a container. + +Unlike `phpstan--container-runtime-command', this also recognizes the +`(STRING . (ARGUMENTS ...))' form whose car names a known runtime, because +such a command line still needs project paths rewritten to its mount point." + (or (phpstan--container-runtime-command) + (and (consp phpstan-executable) + (stringp (car phpstan-executable)) + (member (car phpstan-executable) + (mapcar #'cdr phpstan-container-executables)) + t))) + (defun phpstan--plist-to-alist (plist) "Convert PLIST to association list." (let (alist) @@ -347,16 +391,12 @@ NIL (defun phpstan-normalize-path (source-original &optional source) "Return normalized source file path to pass by SOURCE-ORIGINAL or SOURCE. -If neither `phpstan-replace-path-prefix' nor executable docker is set, +If neither `phpstan-replace-path-prefix' nor a container executable is set, it returns the value of `SOURCE' as it is." (let ((root-directory (expand-file-name (php-project-get-root-dir))) (prefix (or phpstan-replace-path-prefix - (cond - ((eq 'docker phpstan-executable) "/app") - ((and (consp phpstan-executable) - (string= "docker" (car phpstan-executable))) - "/app"))))) + (and (phpstan--container-executable-p) "/app")))) (if prefix (expand-file-name (replace-regexp-in-string (concat "\\`" (regexp-quote root-directory)) @@ -453,8 +493,8 @@ it returns the value of `SOURCE' as it is." (defun phpstan-get-executable-and-args () "Return PHPStan excutable file and arguments." (cond - ((eq 'docker phpstan-executable) - (list phpstan-docker-executable "run" "--rm" "-v" + ((phpstan--container-runtime-command) + (list (phpstan--container-runtime-command) "run" "--rm" "-v" (concat (expand-file-name (php-project-get-root-dir)) ":/app") phpstan-docker-image)) ((and (consp phpstan-executable) @@ -471,11 +511,10 @@ it returns the value of `SOURCE' as it is." (if (file-executable-p phpstan-executable) (list phpstan-executable) (list php-executable phpstan-executable))) - ((and phpstan-flycheck-auto-set-executable - (listp phpstan-executable) + ((and (consp phpstan-executable) (stringp (car phpstan-executable)) (listp (cdr phpstan-executable))) - (cdr phpstan-executable)) + phpstan-executable) ((null phpstan-executable) (let* ((vendor-phpstan (expand-file-name "vendor/bin/phpstan" (php-project-get-root-dir))) @@ -499,42 +538,46 @@ it returns the value of `SOURCE' as it is." (autoload (phpstan-get-autoload-file)) (memory-limit (phpstan-get-memory-limit)) (level (phpstan-get-level))) - (nconc (if include-executable (list (car executable-and-args)) nil) - (cdr executable-and-args) - (list "analyze" - (format "--error-format=%s" (or format "raw")) - "--no-progress" "--no-interaction") - (and use-pro (list "--pro" "--no-ansi")) - (and config (list "-c" (phpstan--expand-file-name config))) - (and autoload (list "-a" autoload)) - (and memory-limit (list "--memory-limit" memory-limit)) - (and level (list "-l" level)) - (cond - ((null verbose) nil) - ((memq verbose '(1 t)) (list "-v")) - ((eq verbose 2) (list "-vv")) - ((eq verbose 3) (list "-vvv")) - ((error ":verbose option should be 1, 2, 3 or `t'"))) - (cond - (phpstan--use-xdebug-option (list phpstan--use-xdebug-option)) - ((eq phpstan-use-xdebug-option 'auto) - (setq-local phpstan--use-xdebug-option - (when (string= "1" (php-runtime-expr "extension_loaded('xdebug')")) - "--xdebug")) - (list phpstan--use-xdebug-option)) - (phpstan-use-xdebug-option (list "--xdebug"))) - options - (when editor - (let ((original-file (plist-get editor :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))))))) - (if editor args (cons "--" args))))) + ;; NOTE: Use `append', never `nconc'. Both `executable-and-args' and + ;; `options' may be shared structure owned by the caller (typically the + ;; value of `phpstan-executable' or `phpstan-generate-baseline-options'), + ;; and `nconc' would destructively grow them on every call. + (append (if include-executable (list (car executable-and-args)) nil) + (cdr executable-and-args) + (list "analyze" + (format "--error-format=%s" (or format "raw")) + "--no-progress" "--no-interaction") + (and use-pro (list "--pro" "--no-ansi")) + (and config (list "-c" (phpstan--expand-file-name config))) + (and autoload (list "-a" autoload)) + (and memory-limit (list "--memory-limit" memory-limit)) + (and level (list "-l" level)) + (cond + ((null verbose) nil) + ((memq verbose '(1 t)) (list "-v")) + ((eq verbose 2) (list "-vv")) + ((eq verbose 3) (list "-vvv")) + ((error ":verbose option should be 1, 2, 3 or `t'"))) + (cond + (phpstan--use-xdebug-option (list phpstan--use-xdebug-option)) + ((eq phpstan-use-xdebug-option 'auto) + (setq-local phpstan--use-xdebug-option + (when (string= "1" (php-runtime-expr "extension_loaded('xdebug')")) + "--xdebug")) + (list phpstan--use-xdebug-option)) + (phpstan-use-xdebug-option (list "--xdebug"))) + options + (when editor + (let ((original-file (plist-get editor :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))))))) + (if editor args (cons "--" args))))) (defun phpstan-update-ignorebale-errors-from-json-buffer (errors) "Update `phpstan--ignorable-errors' variable by ERRORS."