diff --git a/CHANGELOG.md b/CHANGELOG.md index f21e6043..ba450ed6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,12 +7,26 @@ All notable changes of the PHP Mode 1.19.1 release series are documented in this ### Added * `php-project-get-root-dir` falls back to `project-current` when no PHP-specific marker is found, so `project.el` backends (Projectile 3, `project-vc-extra-root-markers`, etc.) can contribute project detection + * Add `php-ide-set-feature` command to interactively enable a PHP-IDE feature for the current buffer + * Only offers features whose backing package is actually available + * Add `php-ide-status` command to report the current PHP-IDE state for the current buffer + * Shows whether it is on, what is configured, and what is available + * Add `php-ide-eglot-activate`, which registers `php-ide-eglot-executable` into `eglot-server-programs` + * Buffer-local, so only buffers that set the variable are affected; previously the variable had no effect on Eglot at all ### Changed * Add `readonly` class modifier to [Imenu] ([#802]) * Add `enum` support to `php-current-class` ([#802]) * Remove hardcoding of implicit paths in `php` that are not guaranteed to exist ([#803]) + * `php-ide-turn-on` no longer errors when `php-ide-features` is unset + * It is now a silent no-op, matching the README's `hack-local-variables-hook` recipe, which previously errored on every PHP file until a feature was configured + * `php-ide-features` now also accepts a bare feature symbol, not just a list + * This was already shown (but not actually supported) in `php-ide.el`'s own Commentary + * Restrict `:safe` on `php-ide-features`/`php-ide-eglot-executable`, and drop `:safe` from `php-ide-mode-functions` entirely + * Previously these let a `.dir-locals.el` silently run an attacker-chosen command or Lisp function without Emacs's usual confirmation; now only PHP-IDE's own known feature symbols and bundled executable presets are accepted + * **If you now get prompted** by a `.dir-locals.el` you wrote and trust yourself (e.g. `php-ide-eglot-executable` set to a raw path/command, or any use of `php-ide-mode-functions`), this is expected — PHP-IDE can no longer vouch for that value as safe. Answer `!` at the prompt to permanently remember that exact value (all supported Emacs versions), or `+` to trust the whole directory from then on (Emacs 30.1+); see `(info "(emacs) Directory Variables")`. You can also pre-approve values ahead of time in your own init file via `safe-local-variable-values` / `safe-local-variable-directories`, so you are never prompted even on first visit. + * `php-ide-mode-functions` was always meant to be configured globally with `add-hook` in your init file (see the Commentary in `php-ide.el`), not set per-project via `.dir-locals.el`. For per-project behavior, branch on the `FEATURE` argument inside your hook function instead of varying the variable's value by directory. ### Fixed @@ -25,6 +39,18 @@ All notable changes of the PHP Mode 1.19.1 release series are documented in this * The first completion reordered the value the user had set, e.g. `(pcntl bcmath core)` became `(bcmath core pcntl)` * Fix function name completion offering module names as if they were PHP functions * Entries of `php-defs-functions-alist` are `(MODULE . FUNCTION-NAMES)` and the whole entry was appended, so every enabled module leaked its own name into the candidates + * Fix the `phpactor` PHP-IDE feature's `:deactivate` to actually deactivate + * It pointed at `php-ide-phpactor-activate` instead of `php-ide-phpactor-deactivate`, so turning `php-ide-mode` off re-activated Phpactor instead + * Fix `php-ide-eglot-server-program` to return a valid command list for symbolic `php-ide-eglot-executable` values + * e.g. `intelephense`, `phpactor` — the `assq` result was used without `cdr` + * Fix the `lsp-mode` PHP-IDE feature's `:deactivate` to no longer error when turning `php-ide-mode` off + * It called `lsp-workspace-shutdown`, which requires a `WORKSPACE` argument and signalled `wrong-number-of-arguments`; switched to the buffer-scoped `lsp-disconnect` + * Fix the `:safe` predicates of `php-ide-features`/`php-ide-eglot-executable`, which never actually applied + * Emacs checks directory-local values *before* `php-ide.el` is loaded, so the predicates ran as copied into the autoloads file, where they hit `void-function cl-loop` / `void-variable php-ide-lsp-command-alist`. `safe-local-variable-p` demotes such errors to nil, so every project setting these variables was prompted for confirmation regardless + * Fix Phpactor hover suppression to match its documented "any predicate matches" behavior + * `php-ide-phpactor-disable-hover-at-point-functions` was combined with AND, so an empty list disabled hover everywhere and multiple predicates only fired when all matched + * Fix `php-ide-mode` deactivation stopping Phpactor hover in unrelated buffers + * The hover timer is shared by all buffers but was cancelled unconditionally; it is now retired only once no live buffer uses hover ### Deprecated diff --git a/README.ja.md b/README.ja.md index d9bc528f..83b9ebdf 100644 --- a/README.ja.md +++ b/README.ja.md @@ -75,6 +75,39 @@ M-x package-install php-mode (php-project-coding-style . psr2))) ``` +### PHP-IDE: LSPクライアントおよびPhpactorとの連携 + +`php-ide`(`php-ide.el`)は、PHP Modeと[Eglot](https://github.com/joaotavora/eglot)・[lsp-mode](https://github.com/emacs-lsp/lsp-mode)・[lsp-bridge](https://github.com/manateelazycat/lsp-bridge)・[Phpactor](https://github.com/emacs-php/phpactor.el)のようなIDE的機能を橋渡しする**実験的**な機能です。これらの機能自体を実装するものではなく、選んだものを`php-ide-mode`という単一のマイナーモード経由で有効化・無効化するだけです。詳細な仕様は`php-ide.el`冒頭のCommentaryを参照してください。 + +```lisp +(defun my-php-mode-init () + (add-hook 'hack-local-variables-hook 'php-ide-turn-on nil t)) + +(with-eval-after-load 'php-ide + (custom-set-variables + '(php-ide-features '(eglot)) ;; '(phpactor)、'(lsp-mode)、'(lsp-bridge)も可 + '(php-ide-eglot-executable 'intelephense) ;; 'phpactor、パス文字列、文字列のリストも可 + '(php-ide-mode-lighter ""))) ;; モードラインからPHP-IDEを隠す +``` + +`php-ide-turn-on`は`php-ide-features`が未設定のときは何もしないため、上記のように無条件にフックへ追加しても安全です。上記のようにグローバルに、または下記のようにプロジェクト単位で`php-ide-features`を設定するまでPHP-IDEはオフのままです。 + +`php-ide`読み込み後に使えるコマンド: + +* `M-x php-ide-mode` — `php-ide-features`に基づき、現在のバッファでPHP-IDEをトグルする。 +* `M-x php-ide-turn-on` — 同様だが、`php-ide-features`が未設定でもエラーにならない。 +* `M-x php-ide-set-feature` — このシステムで実際に利用可能な(対応パッケージがインストールされている)機能から対話的に選び、現在のバッファで有効化する。 +* `M-x php-ide-status` — PHP-IDEが有効かどうか、設定内容、利用可能な機能を表示する。 + +#### プロジェクト単位のPHP-IDE設定 + +```lisp +((nil (php-project-root . git) + (php-ide-features . (eglot)))) +``` + +`php-ide-features`と`php-ide-eglot-executable`は、PHP-IDE組み込みの機能名やバンドル済み実行ファイルのプリセット(`intelephense`、`phpactor`など)を指す場合に限り、`.dir-locals.el`での設定が安全とみなされます。それ以外の値——生の実行ファイルパス、明示的なコマンド引数、カスタムの`php-ide-mode-functions`フックなど——は、これまで通りEmacsの「危険な変数」に対する通常の確認を経ます。そうしないと、ファイルを開くだけで任意のリポジトリが任意のコマンド(またはLisp関数)をあなたのEmacs上で実行できてしまうためです。 + ### `project.el`・Projectileとの連携 `php-project-get-root-dir`は、まずPHP固有のマーカー(`.projectile`、`composer.json`/`composer.lock`、続いてVCSディレクトリ)を探索します。モノレポではパッケージ単位の`vendor/autoload.php`やコーディングスタイルが`php-mode`にとって重要なため、VCSルートより`composer.json`を優先します。これらのマーカーが見つからない場合は`project-current`にフォールバックするので、任意の[`project.el`](https://www.gnu.org/software/emacs/manual/html_node/emacs/Projects.html)バックエンドが検出に寄与できます。 diff --git a/README.md b/README.md index 178257b8..b67db77f 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,39 @@ You can add project-specific settings by creating a `.dir-locals.el` or `.dir-lo (php-project-coding-style . psr2))) ``` +### PHP-IDE: integrating LSP clients and Phpactor + +`php-ide` (`php-ide.el`) is an **experimental** bridge between PHP Mode and IDE-like tools: [Eglot](https://github.com/joaotavora/eglot), [lsp-mode](https://github.com/emacs-lsp/lsp-mode), [lsp-bridge](https://github.com/manateelazycat/lsp-bridge), and [Phpactor](https://github.com/emacs-php/phpactor.el). It does not implement any of these features itself — it only activates or deactivates whichever one(s) you choose, through a single `php-ide-mode` minor mode. See the Commentary at the top of `php-ide.el` for the full reference. + +```lisp +(defun my-php-mode-init () + (add-hook 'hack-local-variables-hook 'php-ide-turn-on nil t)) + +(with-eval-after-load 'php-ide + (custom-set-variables + '(php-ide-features '(eglot)) ;; and/or '(phpactor), '(lsp-mode), '(lsp-bridge) + '(php-ide-eglot-executable 'intelephense) ;; or 'phpactor, a path string, or a list of strings + '(php-ide-mode-lighter ""))) ;; hide PHP-IDE from the mode line +``` + +`php-ide-turn-on` does nothing when `php-ide-features` is unset, so it is safe to add unconditionally as above — PHP-IDE stays off until a feature is configured, either globally as above or per project below. + +Useful commands once `php-ide` is loaded: + +* `M-x php-ide-mode` — toggle PHP-IDE for the current buffer, using `php-ide-features`. +* `M-x php-ide-turn-on` — same, but never errors when `php-ide-features` is unset. +* `M-x php-ide-set-feature` — pick, interactively, one of the features actually available on this system (i.e. whose backing package is installed) and enable it for the current buffer. +* `M-x php-ide-status` — report whether PHP-IDE is on, what is configured, and what is available. + +#### Per-project PHP-IDE configuration + +```lisp +((nil (php-project-root . git) + (php-ide-features . (eglot)))) +``` + +`php-ide-features` and `php-ide-eglot-executable` are only treated as safe for `.dir-locals.el` when they name one of PHP-IDE's own built-in features or bundled executable presets (e.g. `intelephense`, `phpactor`). Anything else — a raw executable path, explicit command arguments, or a custom `php-ide-mode-functions` hook — still goes through Emacs's normal confirmation for risky directory-local variables, since applying those silently would let any repository run an arbitrary command (or Lisp function) in your Emacs just by having you open a file in it. + ### Integration with `project.el` and Projectile `php-project-get-root-dir` first looks for a PHP-specific marker (`.projectile`, `composer.json`/`composer.lock`, then a VCS directory). Preferring `composer.json` over the VCS root matters in monorepos, where per-package `vendor/autoload.php` and coding styles are what `php-mode` cares about. When none of those markers is found, it now falls back to `project-current`, so any [`project.el`](https://www.gnu.org/software/emacs/manual/html_node/emacs/Projects.html) backend can contribute detection: diff --git a/lisp/php-ide-phpactor.el b/lisp/php-ide-phpactor.el index 82ffe33f..a99a3784 100644 --- a/lisp/php-ide-phpactor.el +++ b/lisp/php-ide-phpactor.el @@ -32,9 +32,17 @@ (require 'smart-jump nil t) (require 'cl-lib) -(defvar-local php-ide-phpactor-buffer nil) -(defvar-local php-ide-phpactor-hover-last-pos nil) -(defvar-local php-ide-phpactor-hover-last-msg nil) +(defvar-local php-ide-phpactor-buffer nil + "Non-nil when PHP-IDE has activated Phpactor in this buffer.") + +(defvar-local php-ide-phpactor-hover-last-pos nil + "Buffer position the last hover request was made for.") + +(defvar-local php-ide-phpactor-hover-last-msg nil + "Hover message Phpactor returned for `php-ide-phpactor-hover-last-pos'. + +It is consumed (and cleared) the next time point rests on that same +position, so that the popup only appears once the user has stayed put.") (declare-function phpactor--command-argments "ext:phpactor" (&rest arg-keys)) (declare-function phpactor--parse-json "ext:phpactor" (buffer)) @@ -61,15 +69,48 @@ (cl-every (lambda (x) (memq x '(all hover navigation))) xs)))) (defvar php-ide-phpactor-timer nil - "Timer object for execute Phpactor and display hover message.") + "Timer object for execute Phpactor and display hover message. + +There is a single timer shared by every buffer using Phpactor hover; it +dispatches to whichever buffer is current when it fires. It therefore +must not be cancelled until the last such buffer is deactivated, which +`php-ide-phpactor--stop-hover-timer' takes care of.") + +(defun php-ide-phpactor--hover-buffer-exists-p () + "Return non-nil if any live buffer still uses Phpactor hover." + (cl-some (lambda (buffer) + (buffer-local-value 'php-ide-phpactor-buffer buffer)) + (buffer-list))) + +(defun php-ide-phpactor--stop-hover-timer () + "Cancel the shared hover timer, but only once no buffer needs it." + (when (and php-ide-phpactor-timer + (not (php-ide-phpactor--hover-buffer-exists-p))) + (cancel-timer php-ide-phpactor-timer) + (setq php-ide-phpactor-timer nil))) + +(defun php-ide-phpactor--hover-timer-function () + "Show hover information, or retire the timer once no buffer needs it. + +Buffers that are killed while active never run +`php-ide-phpactor-deactivate', so the timer also checks here rather than +relying on deactivation alone." + (if (php-ide-phpactor--hover-buffer-exists-p) + (php-ide-phpactor-hover) + (php-ide-phpactor--stop-hover-timer))) (defvar php-ide-phpactor-disable-hover-at-point-functions - '(php-in-string-or-comment-p)) + '(php-in-string-or-comment-p) + "Predicates that suppress the hover popup at point. + +Each element is called with no arguments; a non-nil return from any of +them means no hover information is requested at point. An empty list +therefore never suppresses hover.") (defun php-ide-phpactor--disable-hover-at-point-p () - "Return non-NIL if any function return non-NIL for disable to hover at point." + "Return non-nil if any function returns non-nil to disable hover at point." (cl-loop for f in php-ide-phpactor-disable-hover-at-point-functions - never (not (funcall f)))) + thereis (funcall f))) (defun php-ide-phpactor-hover () "Show brief information about the symbol underneath the cursor." @@ -104,23 +145,26 @@ (local-set-key [remap xref-find-definitions] #'smart-jump-go) (local-set-key [remap xref-pop-marker-stack] #'smart-jump-back) (local-set-key [remap xref-find-references] #'smart-jump-references))) + (setq php-ide-phpactor-buffer t) (when (php-ide-phpactor--feature-activated-p 'hover) (unless php-ide-phpactor-timer - (setq php-ide-phpactor-timer (run-with-timer 0.8 0.8 #'php-ide-phpactor-hover)))) - (setq php-ide-phpactor-buffer t)) + (setq php-ide-phpactor-timer + (run-with-timer 0.8 0.8 #'php-ide-phpactor--hover-timer-function))))) ;;;###autoload (defun php-ide-phpactor-deactivate () - "Dectivate PHP-IDE using phpactor.el." + "Deactivate PHP-IDE using phpactor.el." (interactive) (local-unset-key [remap xref-find-definitions]) (local-unset-key [remap xref-pop-marker-stack]) (local-unset-key [remap xref-find-references]) - (when php-ide-phpactor-timer - (cancel-timer php-ide-phpactor-timer) - (setq php-ide-phpactor-timer nil)) - (setq php-ide-phpactor-buffer nil)) + (setq php-ide-phpactor-buffer nil + php-ide-phpactor-hover-last-pos nil + php-ide-phpactor-hover-last-msg nil) + ;; Must run after clearing `php-ide-phpactor-buffer' above, so that this + ;; buffer no longer counts as one that still needs the shared timer. + (php-ide-phpactor--stop-hover-timer)) (provide 'php-ide-phpactor) ;;; php-ide-phpactor.el ends here diff --git a/lisp/php-ide.el b/lisp/php-ide.el index 8880970d..aa004bd6 100644 --- a/lisp/php-ide.el +++ b/lisp/php-ide.el @@ -55,7 +55,7 @@ ;; ;; ## Configuration ;; -;; Put follows code into your .emacs (~/.emacs.d/init.el) file: +;; Put the following code into your .emacs (~/.emacs.d/init.el) file: ;; ;; (defun init-php-mode-setup () ;; (add-hook 'hack-local-variables-hook #'php-ide-mode t t)) @@ -71,7 +71,7 @@ ;; ;; (with-eval-after-load 'php-ide ;; (custom-set-variables -;; '(php-ide-features . 'eglot) ;; and/or 'none, 'phpactor, 'lsp-mode +;; '(php-ide-features '(eglot)) ;; and/or '(none), '(phpactor), '(lsp-mode) ;; '(php-ide-eglot-executable "psalm-language-server") ;; or "intelephense", '("php" "vendor/bin/path/to/server") ;; ;; If you want to hide php-ide-mode from the mode line, set an empty string ;; '(php-ide-mode-lighter "")) @@ -79,11 +79,12 @@ ;; (add-hook 'php-mode-hook #'init-php-mode-setup) ;; (add-hook 'php-ide-mode-functions #'init-php-ide-mode-setup)) ;; -;; If you don't enable IDE support by default, set '(php-ide-feature 'none) +;; If you don't want to enable any IDE support by default, leave `php-ide-features' +;; unset (its default, nil) or set it to '(none) explicitly. ;; ;; ### For per project configuration ;; -;; Put follows code into .dir-locals.el in project directory: +;; Put the following code into .dir-locals.el in project directory: ;; ;; ((nil (php-project-root . git) ;; (php-ide-features . (lsp-mode)))) @@ -105,29 +106,56 @@ (declare-function eglot--managed-mode-off "ext:eglot" ()) (declare-function phpactor--find-executable "ext:phpactor" ())) +;; Autoloaded because the `:safe' predicate of `php-ide-features' consults this +;; alist, and that predicate is copied into the package autoloads file, where it +;; runs while Emacs checks .dir-locals.el — long before php-ide.el itself loads. +;;;###autoload (defvar php-ide-feature-alist '((none :test (lambda () t) :activate (lambda () t) :deactivate (lambda () t)) (phpactor :test (lambda () (and (require 'phpactor nil t) (featurep 'phpactor))) :activate php-ide-phpactor-activate - :deactivate php-ide-phpactor-activate) + :deactivate php-ide-phpactor-deactivate) (eglot :test (lambda () (and (require 'eglot nil t) (featurep 'eglot))) - :activate eglot-ensure + :activate php-ide-eglot-activate + ;; `eglot--managed-mode-off' is Eglot's own internal (and unexported) function, + ;; but it is the only operation that turns Eglot off for just the current buffer + ;; without shutting down a server that other buffers may still be using. The + ;; public `eglot-shutdown' always kills the whole server, which would be a much + ;; more disruptive (and asymmetric) deactivation than `php-ide-eglot-activate'. :deactivate eglot--managed-mode-off) (lsp-bridge :test (lambda () (and (require 'lsp-bridge nil t) (featurep 'lsp-bridge))) :activate (lambda () (lsp-bridge-mode +1)) :deactivate (lambda () (lsp-bridge-mode -1))) (lsp-mode :test (lambda () (and (require 'lsp nil t) (featurep 'lsp))) :activate lsp - :deactivate lsp-workspace-shutdown))) + :deactivate lsp-disconnect)) + "Alist of PHP-IDE features and how to probe and (de)activate each one. + +Each element is (FEATURE . PLIST), where PLIST holds these keywords, +each bound to a function called with no arguments: + +`:test' Return non-nil when FEATURE is usable in this Emacs, + loading its backing package if necessary. +`:activate' Turn FEATURE on in the current buffer. +`:deactivate' Turn FEATURE off in the current buffer.") +;; Autoloaded for the same reason as `php-ide-feature-alist'; the `:safe' +;; predicate of `php-ide-eglot-executable' consults this alist. +;;;###autoload (defvar php-ide-lsp-command-alist '((intelephense "intelephense" "--stdio") (phpactor . (lambda () (list (if (fboundp 'phpactor--find-executable) (phpactor--find-executable) "phpactor") - "language-server"))))) + "language-server")))) + "Alist of bundled LSP server presets for `php-ide-eglot-executable'. + +Each element is (NAME . COMMAND), where COMMAND is either a list of +strings to execute or a function of no arguments returning such a list. +Only the NAME symbols listed here are accepted as safe directory-local +values; see `php-ide-eglot-executable'.") (defgroup php-ide nil "IDE-like support for PHP developing." @@ -142,8 +170,18 @@ :type `(set ,@(mapcar (lambda (feature) (list 'const (car feature))) php-ide-feature-alist) symbol) - :safe (lambda (v) (cl-loop for feature in (if (listp v) v (list v)) - always (symbolp feature)))) + ;; Only accept feature symbols already known to `php-ide-feature-alist' as safe + ;; for .dir-locals.el; an arbitrary symbol here could name a feature added by + ;; some future or third-party extension with its own (unvetted) side effects. + ;; + ;; Deliberately written without `cl-lib': this predicate is copied verbatim + ;; into the package autoloads file and runs there while Emacs checks + ;; .dir-locals.el, where cl-lib may not be loaded yet. + :safe (lambda (v) + (let ((features (if (proper-list-p v) v (list v)))) + (not (memq nil (mapcar (lambda (feature) + (and (assq feature php-ide-feature-alist) t)) + features)))))) ;;;###autoload (defcustom php-ide-eglot-executable nil @@ -153,10 +191,14 @@ (const intelephense) (const phpactor) string (repeat string)) - :safe (lambda (v) (cond - ((stringp v) (file-exists-p v)) - ((listp v) (cl-every #'stringp v)) - ((assq v php-ide-lsp-command-alist))))) + ;; Only a symbol naming one of the bundled presets in `php-ide-lsp-command-alist' + ;; is safe for .dir-locals.el: the actual command is then fully determined by + ;; this package, not by the (untrusted) directory-local value. A literal string + ;; or argument list lets the directory choose the executable/arguments outright, + ;; which `php-ide-eglot-server-program' would later pass straight to + ;; `start-process' — that must go through Emacs's normal unsafe-variable + ;; confirmation prompt rather than apply silently. + :safe (lambda (v) (and (assq v php-ide-lsp-command-alist) t))) ;;;###autoload (defun php-ide-eglot-server-program () @@ -164,13 +206,43 @@ (cond ((stringp php-ide-eglot-executable) (list php-ide-eglot-executable)) ((listp php-ide-eglot-executable) php-ide-eglot-executable) - ((when-let* ((command (assq php-ide-eglot-executable php-ide-lsp-command-alist))) + ((when-let* ((command (cdr (assq php-ide-eglot-executable php-ide-lsp-command-alist)))) (cond ((functionp command) (funcall command)) ((listp command) command)))))) +(defvar php-ide-eglot-managed-modes '(php-mode phps-mode php-ts-mode) + "Major modes keyed by the `eglot-server-programs' entry php-ide adds. + +`php-ide-eglot-activate' registers `php-ide-eglot-executable' for exactly +these modes.") + +(defun php-ide-eglot--contact-function (&optional _interactive _project) + "CONTACT function registered into `eglot-server-programs' by php-ide. +Ignores the INTERACTIVE and PROJECT arguments Eglot may pass; see +`php-ide-eglot-server-program' for the actual command lookup." + (php-ide-eglot-server-program)) + +;;;###autoload +(defun php-ide-eglot-activate () + "Activate Eglot for `php-ide-mode', honoring `php-ide-eglot-executable'. + +When `php-ide-eglot-executable' is set, this buffer-locally prepends +an entry to `eglot-server-programs' so Eglot uses it instead of its +own bundled default for PHP. Buffers where `php-ide-eglot-executable' +is unset are unaffected and keep using Eglot's default." + (when (and php-ide-eglot-executable + (not (eq (cdr (assoc php-ide-eglot-managed-modes eglot-server-programs)) + #'php-ide-eglot--contact-function))) + (setq-local eglot-server-programs + (cons (cons php-ide-eglot-managed-modes #'php-ide-eglot--contact-function) + eglot-server-programs))) + (eglot-ensure)) + (defcustom php-ide-mode-lighter " PHP-IDE" - "A symbol of PHP-IDE feature." + "Mode line indicator for `php-ide-mode'. + +Set it to an empty string to hide `php-ide-mode' from the mode line." :tag "PHP-IDE Mode Lighter" :type 'string :safe #'stringp) @@ -181,28 +253,33 @@ Notice that two arguments (FEATURE ACTIVATE) are given. FEATURE: A symbol, like \\='lsp-mode. -ACTIVATE: T is given when activeting, NIL when deactivating PHP-IDE." +ACTIVATE: T is given when activating, NIL when deactivating PHP-IDE." :tag "PHP-IDE Mode Functions" :type '(repeat function) - :safe (lambda (functions) - (and (listp functions) (cl-every #'functionp functions)))) + ;; Deliberately has no :safe predicate. This variable holds functions that + ;; `php-ide-mode' calls automatically, so a directory-local value naming an + ;; arbitrary (but already-`fboundp') function would let any repo run code in + ;; the visitor's Emacs just by having them open a file; that must always go + ;; through Emacs's normal unsafe-variable confirmation, never apply silently. + ) ;;;###autoload (define-minor-mode php-ide-mode "Minor mode for integrate IDE-like tools." :lighter php-ide-mode-lighter - (let ((ide-features php-ide-features)) + (let ((ide-features (if (listp php-ide-features) php-ide-features (list php-ide-features)))) (when-let* ((unavailable-features (cl-loop for feature in ide-features unless (assq feature php-ide-feature-alist) collect feature))) (user-error "%s includes unavailable PHP-IDE features. (available features are: %s)" ide-features (mapconcat (lambda (feature) (concat "'" (symbol-name feature))) - (php-ide--avilable-features) ", "))) + (php-ide--available-features) ", "))) + ;; Every feature in IDE-FEATURES is guaranteed to be in `php-ide-feature-alist' here, + ;; because the loop above already signals a `user-error' otherwise. (cl-loop for feature in ide-features - for ide-plist = (cdr-safe (assq feature php-ide-feature-alist)) - do (if (null ide-plist) - (message "Please set `php-ide-feature' variable in .dir-locals.el or custom variable") + for ide-plist = (cdr (assq feature php-ide-feature-alist)) + do (progn (run-hook-with-args 'php-ide-mode-functions feature php-ide-mode) (if php-ide-mode (php-ide--activate-buffer feature ide-plist) @@ -210,10 +287,14 @@ ACTIVATE: T is given when activeting, NIL when deactivating PHP-IDE." ;;;###autoload (defun php-ide-turn-on () - "Turn on PHP IDE-FEATURES and execute `php-ide-mode'." - (unless php-ide-features - (user-error "No PHP-IDE feature is installed. Install the lsp-mode, lsp-bridge, eglot or phpactor package")) - (php-ide-mode +1)) + "Turn on `php-ide-mode' if `php-ide-features' is set, otherwise do nothing. + +Unlike calling `php-ide-mode' directly, this never signals an error when +`php-ide-features' is unset, so it is safe to add unconditionally to +`php-mode-hook' or `hack-local-variables-hook'; buffers/projects that +never configure `php-ide-features' are silently left alone." + (when php-ide-features + (php-ide-mode +1))) (defun php-ide--activate-buffer (name ide-plist) "Activate php-ide implementation by NAME and IDE-PLIST." @@ -225,11 +306,44 @@ ACTIVATE: T is given when activeting, NIL when deactivating PHP-IDE." "Deactivate php-ide implementation by IDE-PLIST." (funcall (plist-get ide-plist :deactivate))) -(defun php-ide--avilable-features () +(defun php-ide--available-features () "Return list of available PHP-IDE features." (cl-loop for (ide . plist) in php-ide-feature-alist if (funcall (plist-get plist :test)) collect ide)) +;;;###autoload +(defun php-ide-set-feature (feature) + "Set `php-ide-features' to FEATURE for the current buffer and enable it. + +Interactively, prompt among the PHP-IDE features currently available on +this system (see `php-ide--available-features'); features whose backing +package (lsp-mode, lsp-bridge, Eglot or phpactor.el) is not installed +are not offered. + +This sets `php-ide-features' buffer-locally, so the choice does not +persist beyond the current buffer; put a matching entry in +\".dir-locals.el\" (or your `php-mode-hook') to make it stick." + (interactive + (let ((available (php-ide--available-features))) + (unless available + (user-error "No PHP-IDE feature is available. Install lsp-mode, lsp-bridge, eglot or phpactor")) + (list (intern (completing-read "PHP-IDE feature: " + (mapcar #'symbol-name available) nil t))))) + (when php-ide-mode + (php-ide-mode -1)) + (setq-local php-ide-features (list feature)) + (php-ide-mode +1)) + +;;;###autoload +(defun php-ide-status () + "Show `php-ide-mode' status for the current buffer in the echo area." + (interactive) + (let ((configured (if (listp php-ide-features) php-ide-features (list php-ide-features)))) + (message "PHP-IDE: %s (configured: %s; available on this system: %s)" + (if php-ide-mode "on" "off") + (if configured (mapconcat #'symbol-name configured ", ") "none") + (or (mapconcat #'symbol-name (php-ide--available-features) ", ") "none")))) + (provide 'php-ide) ;;; php-ide.el ends here diff --git a/tests/php-mode-test.el b/tests/php-mode-test.el index 3696a443..824f1896 100644 --- a/tests/php-mode-test.el +++ b/tests/php-mode-test.el @@ -36,6 +36,7 @@ (require 'php-mode) (require 'php-mode-debug) (require 'php-project) +(require 'php-ide) (require 'ert) (require 'cl-lib) (require 'imenu) @@ -959,6 +960,242 @@ project setting this variable gets a confirmation prompt anyway." (should (string-match-p "OK" (buffer-string))) (should (eq 0 status)))))) +;;; php-ide tests + +(ert-deftest php-ide-test-eglot-server-program () + "`php-ide-eglot-server-program' should resolve every shape of +`php-ide-eglot-executable' without touching Eglot itself." + (let ((php-ide-eglot-executable nil)) + (should (null (php-ide-eglot-server-program)))) + (let ((php-ide-eglot-executable "psalm-language-server")) + (should (equal '("psalm-language-server") (php-ide-eglot-server-program)))) + (let ((php-ide-eglot-executable '("php" "vendor/bin/path/to/server"))) + (should (equal '("php" "vendor/bin/path/to/server") (php-ide-eglot-server-program)))) + (let ((php-ide-eglot-executable 'intelephense)) + (should (equal '("intelephense" "--stdio") (php-ide-eglot-server-program)))) + (let ((php-ide-eglot-executable 'phpactor)) + (should (equal (list (if (fboundp 'phpactor--find-executable) + (phpactor--find-executable) + "phpactor") + "language-server") + (php-ide-eglot-server-program))))) + +(ert-deftest php-ide-test-feature-alist-arity () + "Regression test: `:test' must always be a callable 0-arg predicate, and +for every PHP-IDE feature actually available in this Emacs, `:activate' +and `:deactivate' must be callable with zero arguments too, since +`php-ide--activate-buffer'/`php-ide--deactivate-buffer' always `funcall' +them without arguments. (Features whose backing package, e.g. lsp-mode +or Eglot, is not installed are skipped for the :activate/:deactivate +check, since their symbols are not `fboundp' until that package loads.)" + (dolist (entry php-ide-feature-alist) + (let* ((ide-plist (cdr entry)) + (test-fn (plist-get ide-plist :test))) + (should (functionp test-fn)) + (should (eq 0 (car (func-arity test-fn)))) + (when (funcall test-fn) + (dolist (key '(:activate :deactivate)) + (let ((fn (plist-get ide-plist key))) + (should (functionp fn)) + (should (eq 0 (car (func-arity fn)))))))))) + +(ert-deftest php-ide-test-phpactor-activate-deactivate-not-swapped () + "Regression test: the `phpactor' feature's `:deactivate' must be +`php-ide-phpactor-deactivate', not (as it once was, by copy-paste) +`php-ide-phpactor-activate'." + (let ((ide-plist (cdr (assq 'phpactor php-ide-feature-alist)))) + (should (eq #'php-ide-phpactor-deactivate (plist-get ide-plist :deactivate))))) + +(ert-deftest php-ide-test-features-accepts-bare-symbol () + "Regression test: `php-ide-features' set to a bare symbol (as shown in +`php-ide.el's own Commentary) must not signal wrong-type-argument." + (with-temp-buffer + (php-mode) + (setq-local php-ide-features 'none) + (php-ide-mode +1) + (should php-ide-mode) + (php-ide-mode -1) + (should-not php-ide-mode))) + +(ert-deftest php-ide-test-turn-on-is-noop-when-unconfigured () + "Regression test: `php-ide-turn-on' must not signal an error (and must +not turn `php-ide-mode' on) when `php-ide-features' is unset, so it is +safe to add unconditionally to `hack-local-variables-hook'." + (with-temp-buffer + (php-mode) + (should-not php-ide-features) + (php-ide-turn-on) + (should-not php-ide-mode))) + +(ert-deftest php-ide-test-set-feature-and-status () + "`php-ide-set-feature' should set `php-ide-features' buffer-locally and +turn `php-ide-mode' on; `php-ide-status' should not error." + (with-temp-buffer + (php-mode) + (php-ide-set-feature 'none) + (should (equal '(none) php-ide-features)) + (should php-ide-mode) + ;; Re-selecting the same feature while already on must not error. + (php-ide-set-feature 'none) + (should php-ide-mode) + ;; `php-ide-status' just messages a summary; simply calling it must not error. + (php-ide-status))) + +(ert-deftest php-ide-test-safe-local-variables () + "`.dir-locals.el' safety predicates must only accept PHP-IDE's own +known feature symbols and bundled executable presets, never an +arbitrary function, path, or command list." + (let ((features-safe (get 'php-ide-features 'safe-local-variable)) + (eglot-exe-safe (get 'php-ide-eglot-executable 'safe-local-variable))) + (should (funcall features-safe '(none))) + (should-not (funcall features-safe '(totally-bogus-feature))) + (should (funcall eglot-exe-safe 'intelephense)) + (should (funcall eglot-exe-safe 'phpactor)) + (should-not (funcall eglot-exe-safe "/bin/ls")) + (should-not (funcall eglot-exe-safe '("curl" "https://example.com/x"))) + ;; `php-ide-mode-functions' can call arbitrary code, so it must have no + ;; safe-local-variable predicate at all and always go through Emacs's + ;; normal risky-variable confirmation. + (should-not (get 'php-ide-mode-functions 'safe-local-variable)))) + +(ert-deftest php-ide-test-safe-local-variables-work-from-autoloads () + "Regression test: the `:safe' predicates must work from the package +autoloads file alone. + +Emacs decides whether a .dir-locals.el value is safe *before* php-ide.el +is loaded (the README recipe only pulls php-ide in from +`hack-local-variables-hook', which runs afterwards), so the predicates +run as copied into php-mode-autoloads.el. There they must not depend on +cl-lib nor on variables that only php-ide.el defines, or +`safe-local-variable-p' demotes the resulting error to nil and every +project setting these variables gets a confirmation prompt anyway." + (let ((autoloads (expand-file-name "../lisp/php-mode-autoloads.el" php-mode-test-dir)) + (emacs (expand-file-name invocation-name invocation-directory))) + (skip-unless (file-exists-p autoloads)) + ;; Emacs 28 was the first to copy a defcustom's `:safe' predicate into the + ;; generated autoloads file; Emacs 27 drops it, so there is nothing to + ;; check there. + (skip-unless (with-temp-buffer + (insert-file-contents autoloads) + (search-forward "'php-ide-features 'safe-local-variable" nil t))) + (with-temp-buffer + (let ((status (call-process + emacs nil t nil "-Q" "--batch" + "--load" autoloads + "--eval" + (prin1-to-string + '(progn + ;; Guard against the predicate quietly working only + ;; because php-ide.el got loaded after all. + (when (featurep 'php-ide) + (error "php-ide must not be loaded in this check")) + (dolist (c '((php-ide-features (eglot) t) + (php-ide-features eglot t) + (php-ide-features nil t) + (php-ide-features (bogus-feature) nil) + (php-ide-eglot-executable intelephense t) + (php-ide-eglot-executable phpactor t) + (php-ide-eglot-executable "/bin/ls" nil))) + (let* ((pred (get (nth 0 c) 'safe-local-variable)) + (got (and (funcall pred (nth 1 c)) t))) + (unless (eq got (nth 2 c)) + (error "%s with %S: got %S, want %S" + (nth 0 c) (nth 1 c) got (nth 2 c))))) + (princ "OK")))))) + ;; Check the output first: on failure ERT then reports what the + ;; subprocess complained about, not just its exit status. + (should (string-match-p "OK" (buffer-string))) + (should (eq 0 status)))))) + +(ert-deftest php-ide-test-phpactor-disable-hover-at-point-p () + "Regression test: `php-ide-phpactor--disable-hover-at-point-p' must +suppress hover when *any* predicate matches, as its docstring says. + +It used to be written with `never (not ...)', i.e. logical AND, so an +empty list disabled hover everywhere (the exact opposite of the intent) +and a list of several predicates only fired when all of them matched." + (let ((always (lambda () t)) + (never (lambda () nil))) + (let ((php-ide-phpactor-disable-hover-at-point-functions nil)) + (should-not (php-ide-phpactor--disable-hover-at-point-p))) + (let ((php-ide-phpactor-disable-hover-at-point-functions (list always))) + (should (php-ide-phpactor--disable-hover-at-point-p))) + (let ((php-ide-phpactor-disable-hover-at-point-functions (list never))) + (should-not (php-ide-phpactor--disable-hover-at-point-p))) + (let ((php-ide-phpactor-disable-hover-at-point-functions (list always never))) + (should (php-ide-phpactor--disable-hover-at-point-p))) + (let ((php-ide-phpactor-disable-hover-at-point-functions (list never always))) + (should (php-ide-phpactor--disable-hover-at-point-p))) + (let ((php-ide-phpactor-disable-hover-at-point-functions (list never never))) + (should-not (php-ide-phpactor--disable-hover-at-point-p))))) + +(ert-deftest php-ide-test-phpactor-hover-timer-is-shared () + "Regression test: the Phpactor hover timer is shared by every buffer, +so deactivating one buffer must not stop hover in the others. + +It used to be cancelled unconditionally, which silently killed hover in +every remaining PHP buffer. A buffer killed while still active must not +strand the timer either." + (let ((php-ide-phpactor-timer nil) + (buffers nil)) + (unwind-protect + (let ((a (generate-new-buffer " *php-ide-test-a*")) + (b (generate-new-buffer " *php-ide-test-b*"))) + (setq buffers (list a b)) + (with-current-buffer a (php-ide-phpactor-activate)) + (with-current-buffer b (php-ide-phpactor-activate)) + (should php-ide-phpactor-timer) + ;; Deactivating only A must leave the timer running for B. + (with-current-buffer a (php-ide-phpactor-deactivate)) + (should php-ide-phpactor-timer) + (should (buffer-local-value 'php-ide-phpactor-buffer b)) + ;; Once the last buffer goes, the timer must be cancelled. + (with-current-buffer b (php-ide-phpactor-deactivate)) + (should-not php-ide-phpactor-timer) + ;; A buffer killed while active must not strand the timer: the + ;; timer function itself retires it on the next tick. + (let ((c (generate-new-buffer " *php-ide-test-c*"))) + (push c buffers) + (with-current-buffer c (php-ide-phpactor-activate)) + (should php-ide-phpactor-timer) + (kill-buffer c) + (php-ide-phpactor--hover-timer-function) + (should-not php-ide-phpactor-timer))) + (when php-ide-phpactor-timer + (cancel-timer php-ide-phpactor-timer)) + (dolist (buf buffers) + (when (buffer-live-p buf) + (kill-buffer buf)))))) + +(ert-deftest php-ide-test-eglot-server-programs-registration () + "`php-ide-eglot-activate' should buffer-locally prepend an +`eglot-server-programs' entry only when `php-ide-eglot-executable' is +set, and must never mutate the global value." + (skip-unless (require 'eglot nil t)) + (let ((global-before (copy-sequence eglot-server-programs))) + (unwind-protect + (progn + (with-temp-buffer + (php-mode) + ;; Unconfigured: must not touch `eglot-server-programs' at all. + (cl-letf (((symbol-function 'eglot-ensure) (lambda () nil))) + (php-ide-eglot-activate)) + (should-not (local-variable-p 'eglot-server-programs))) + (with-temp-buffer + (php-mode) + (setq-local php-ide-eglot-executable "psalm-language-server") + (cl-letf (((symbol-function 'eglot-ensure) (lambda () nil))) + (php-ide-eglot-activate)) + (should (equal '("psalm-language-server") + (funcall (cdr (assoc php-ide-eglot-managed-modes + eglot-server-programs))))) + ;; Re-activating must not prepend a duplicate entry. + (let ((len (length eglot-server-programs))) + (cl-letf (((symbol-function 'eglot-ensure) (lambda () nil))) + (php-ide-eglot-activate)) + (should (= len (length eglot-server-programs)))))) + (should (equal global-before eglot-server-programs))))) + ;; For developers: How to make .faces list file. ;; ;; 1. Press `M-x eval-buffer' in this file bufffer.