Fix and document php-ide, and complete its Eglot integration

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
 

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)バックエンドが検出に寄与できます。

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:

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