From b2c183dbd4e54559a6faaa82b01fce6489efafac Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Fri, 30 Oct 2026 19:46:08 +0800 Subject: [PATCH] fix(php): document language constructs and clarify rename failures (#354) PHP language constructs such as empty and isset had no completion documentation, hover or quick documentation because the language server reports them as plain keywords. They now get built-in documentation in completion, hover, quick documentation and Go to Declaration (which shows the docs instead of failing), without matching strings, comments, methods or other languages. A refused syntax-based rename now states why the language server couldn't rename first. Closes #340 Refs #221 --- src/editor/lsp/completion.test.ts | 21 ++++++++ src/editor/lsp/completion.ts | 7 +++ src/editor/lsp/hover.ts | 20 ++++++- src/editor/lsp/navigation.ts | 3 +- src/editor/lsp/phpConstructs.test.ts | 77 +++++++++++++++++++++++++++ src/editor/lsp/phpConstructs.ts | 40 ++++++++++++++ src/editor/lsp/rename.ts | 11 ++-- src/editor/lsp/syntaxFallback.test.ts | 18 +++++++ src/editor/lsp/syntaxFallback.ts | 2 +- src/platform/locales/text/zh.json | 6 ++- 10 files changed, 195 insertions(+), 10 deletions(-) create mode 100644 src/editor/lsp/phpConstructs.test.ts create mode 100644 src/editor/lsp/phpConstructs.ts diff --git a/src/editor/lsp/completion.test.ts b/src/editor/lsp/completion.test.ts index 83f537c3..e75cf69b 100644 --- a/src/editor/lsp/completion.test.ts +++ b/src/editor/lsp/completion.test.ts @@ -56,6 +56,27 @@ function item(over: Partial = {}): CompletionItem { }; } +describe("PHP construct completion documentation", () => { + const req = { doc: 1, path: "/p/a.php", language: "PHP", pos: 3, text: EditorState.create({ doc: "emp" }).doc }; + + it("documents a keyword without changing its insertion", () => { + const c = makeCompletion(item({ label: "empty", kind: "keyword", insertText: "empty" }), req); + expect(c.lsp.documentation?.value).toContain("empty(mixed $var): bool"); + expect(c.lsp.insertText).toBe("empty"); + }); + + it("preserves server documentation and leaves methods and other languages alone", () => { + const documentation = { kind: "markdown" as const, value: "Server documentation" }; + expect(makeCompletion(item({ label: "empty", kind: "keyword", documentation }), req).lsp.documentation).toEqual( + documentation, + ); + expect(makeCompletion(item({ label: "empty", kind: "method" }), req).lsp.documentation).toBeNull(); + expect( + makeCompletion(item({ label: "empty", kind: "keyword" }), { ...req, language: "JavaScript" }).lsp.documentation, + ).toBeNull(); + }); +}); + describe("toCmCompletion", () => { it("maps label, detail, kind and sort text", () => { const c = toCmCompletion( diff --git a/src/editor/lsp/completion.ts b/src/editor/lsp/completion.ts index 7a9bf7ca..4cbb7543 100644 --- a/src/editor/lsp/completion.ts +++ b/src/editor/lsp/completion.ts @@ -56,6 +56,7 @@ import { editorPrefs } from "@/platform/editorSettings"; import { tx } from "@/platform/i18n"; import { showHint } from "./hint"; import { inplaceRenameActive } from "./inplaceRename"; +import { phpConstructDocumentation } from "./phpConstructs"; interface RequestContext { doc: number; @@ -87,6 +88,8 @@ function resolveItem(doc: number, item: CompletionItem): Promise if (!p) { p = lspRequest(() => commands.lspResolveCompletion(doc, item)).then( (r) => { + if (isEmptyMarkup(r.documentation) && !isEmptyMarkup(item.documentation)) + r = { ...r, documentation: item.documentation }; resolvedItems.set(item, r); return r; }, @@ -365,6 +368,10 @@ async function fallback(ctx: CompletionContext): Promise commands.lspGoto(doc.doc, offsetToPos(view.state.doc, pos), kind)); const seen = new Set(); diff --git a/src/editor/lsp/navigation.ts b/src/editor/lsp/navigation.ts index 33052b67..ca7b84ad 100644 --- a/src/editor/lsp/navigation.ts +++ b/src/editor/lsp/navigation.ts @@ -51,7 +51,7 @@ import { caretAnchor, openPopup, type ListPopupItem } from "./popupStore"; import { identifierAt, readManyFileLines } from "./files"; import { documentLinks, followReference, knownLinks, linkReference, referenceAt, textReference } from "./links"; import { featureBackoff } from "./availability"; -import { requestHover } from "./hover"; +import { requestHover, showPhpConstructDocumentation } from "./hover"; import { declarationSummary, renderCode } from "./markdown"; import { editorPrefs } from "@/platform/editorSettings"; import { isVirtualUri, virtualContainer } from "@/platform/virtualDocs"; @@ -157,6 +157,7 @@ export async function gotoTargets(view: EditorView, kind: GotoKind, at?: number) if (kind === "definition" || kind === "declaration") { const own = contributedAt(view.state, CONTRIBUTED_DEFINITION, offset); if (own?.(view, offset)) return; + if (showPhpConstructDocumentation(view, offset)) return; } const doc = docOf(view); if (!doc) return; diff --git a/src/editor/lsp/phpConstructs.test.ts b/src/editor/lsp/phpConstructs.test.ts new file mode 100644 index 00000000..a51aa6b0 --- /dev/null +++ b/src/editor/lsp/phpConstructs.test.ts @@ -0,0 +1,77 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { EditorState } from "@codemirror/state"; +import { EditorView } from "@codemirror/view"; +import { languages } from "@codemirror/language-data"; +import { phpConstructAt, phpConstructDocumentation } from "./phpConstructs"; +import { lspDoc } from "./lsp"; +import { quickDefinition, requestHover } from "./hover"; +import { gotoTargets } from "./navigation"; +import { closePopup, useLspPopup } from "./popupStore"; + +const php = await languages.find((l) => l.name === "PHP")!.load(); +const views: EditorView[] = []; +afterEach(() => { + closePopup(); + for (const view of views.splice(0)) view.destroy(); +}); + +function state(doc: string) { + return EditorState.create({ doc, extensions: [php] }); +} + +describe("PHP language constructs", () => { + it.each(["empty", "isset", "EMPTY"])("documents %s at either edge of its name", (name) => { + const s = state(`empty();", + "empty();", + "empty($x)

{ + const s = state(text); + for (const match of text.matchAll(/empty/g)) expect(phpConstructAt(s, match.index + 2, "php")).toBeNull(); + }); + + it("does not attach PHP docs to other languages or ordinary functions", () => { + expect(phpConstructAt(state(" { + const view = new EditorView({ + state: EditorState.create({ + doc: " gotoTargets(view, "definition"), + () => gotoTargets(view, "declaration"), + () => quickDefinition(view), + ]) { + await action(); + const popup = useLspPopup.getState().popup; + expect(popup?.kind).toBe("doc"); + if (popup?.kind === "doc") expect(popup.entries[0]?.markup.value).toContain("no source declaration"); + closePopup(); + } + }); +}); diff --git a/src/editor/lsp/phpConstructs.ts b/src/editor/lsp/phpConstructs.ts new file mode 100644 index 00000000..47dd905f --- /dev/null +++ b/src/editor/lsp/phpConstructs.ts @@ -0,0 +1,40 @@ +import type { EditorState } from "@codemirror/state"; +import { syntaxTree } from "@codemirror/language"; + +const CONSTRUCTS: Readonly> = { + empty: { + signature: "empty(mixed $var): bool", + description: + "Returns true when the value is falsey or the variable does not exist. Checking an undefined variable does not emit a warning.", + }, + isset: { + signature: "isset(mixed $var, mixed ...$vars): bool", + description: + "Returns true only when every supplied variable exists and is not null. Checks stop at the first unset or null variable.", + }, +}; + +export function phpConstructDocumentation(language: string | null, name: string): string | null { + if (language?.toLowerCase() !== "php") return null; + const key = name.toLowerCase(); + const entry = Object.hasOwn(CONSTRUCTS, key) ? CONSTRUCTS[key] : undefined; + if (!entry) return null; + return [ + "```php\n" + entry.signature + "\n```", + entry.description, + "PHP language construct; there is no source declaration to navigate to.", + `[PHP manual](https://www.php.net/manual/en/function.${key}.php)`, + ].join("\n\n"); +} + +/** Only an unqualified call, never a method, variable, string, comment or HTML text. */ +export function phpConstructAt(state: EditorState, pos: number, language: string | null) { + if (language?.toLowerCase() !== "php") return null; + for (const side of [-1, 1] as const) { + const node = syntaxTree(state).resolveInner(pos, side); + if (node.name !== "Name" || node.parent?.name !== "CallExpression" || node.parent.from !== node.from) continue; + const markdown = phpConstructDocumentation(language, state.sliceDoc(node.from, node.to)); + if (markdown) return { from: node.from, to: node.to, markdown }; + } + return null; +} diff --git a/src/editor/lsp/rename.ts b/src/editor/lsp/rename.ts index 1726b6d2..a7fb9892 100644 --- a/src/editor/lsp/rename.ts +++ b/src/editor/lsp/rename.ts @@ -11,10 +11,10 @@ * symbols — is left to the server. Ambiguous cases ask first. */ import type { EditorView } from "@codemirror/view"; -import { commands, isCancelled, type Pos, type WorkspaceEdit } from "@/platform/ipc"; +import { commands, isCancelled, unavailableText, type Pos, type WorkspaceEdit } from "@/platform/ipc"; import { whenSynced, type EditorDocContext } from "@/editor/api"; import { structure } from "@/editor/structure"; -import { docOf, lspRequest, reportLspError } from "./lsp"; +import { docOf, isUnavailable, lspRequest, reportLspError } from "./lsp"; import { comparePos, offsetToPos, rangeToOffsets } from "./positions"; import { showHint } from "./hint"; import { caretAnchor, openPopup, type PreviewOccurrence } from "./popupStore"; @@ -69,7 +69,7 @@ export async function startRename(view: EditorView) { range = await lspRequest(() => commands.lspPrepareRename(doc.doc, pos)); } catch (e) { if (isCancelled(e)) return; - if (!(await startSyntaxRename(view, doc, pos))) reportLspError(view, e, true); + if (!(await startSyntaxRename(view, doc, pos, e))) reportLspError(view, e, true); return; } if (!range) { @@ -258,7 +258,7 @@ async function syntaxPlan(view: EditorView, doc: EditorDocContext, pos: Pos): Pr } /** Opens the rename popup for a syntax-based local rename. False when the fallback can't help. */ -async function startSyntaxRename(view: EditorView, doc: EditorDocContext, pos: Pos): Promise { +async function startSyntaxRename(view: EditorView, doc: EditorDocContext, pos: Pos, cause?: unknown): Promise { let result; try { await whenSynced(view); @@ -269,7 +269,8 @@ async function startSyntaxRename(view: EditorView, doc: EditorDocContext, pos: P } if (result.kind === "unsupported") return false; if (result.kind === "refused") { - showHint(view, result.message); + const reason = isUnavailable(cause) ? unavailableText(cause) : cause instanceof Error ? cause.message : null; + showHint(view, reason ? `${reason}. ${result.message}` : result.message); return true; } const { plan } = result; diff --git a/src/editor/lsp/syntaxFallback.test.ts b/src/editor/lsp/syntaxFallback.test.ts index e709a8e3..00027899 100644 --- a/src/editor/lsp/syntaxFallback.test.ts +++ b/src/editor/lsp/syntaxFallback.test.ts @@ -350,6 +350,24 @@ describe("navigation wiring", () => { }); describe("rename wiring", () => { + it("preserves the server's missing capability when syntax cannot rename a method", async () => { + cmd.lspPrepareRename.mockReturnValueOnce( + Promise.resolve({ + status: "error", + error: { kind: "unavailable", message: "intelephense doesn't support rename" }, + }), + ); + cmd.syntaxLocals.mockReturnValueOnce(ok({ status: "notLocal", name: "run", range: r(0, 4, 7), reason: "member" })); + const view = editor("obj.run();", "php", 5); + await startRename(view); + const message = document.querySelector(".cm-lsp-hint")?.textContent; + expect(message).toContain("intelephense doesn't support rename"); + expect(message).toContain("only local variables"); + expect(message).not.toContain("without the language server"); + expect(view.state.doc.toString()).toBe("obj.run();"); + expect(useLspPopup.getState().popup).toBeNull(); + }); + it("renames a certain local in place when the server is not ready", async () => { cmd.syntaxLocals.mockReturnValueOnce(ok(local({ references: [r(1, 4, 5)] }))); cmd.syntaxNodeAt diff --git a/src/editor/lsp/syntaxFallback.ts b/src/editor/lsp/syntaxFallback.ts index 496c0e1e..f9ad9334 100644 --- a/src/editor/lsp/syntaxFallback.ts +++ b/src/editor/lsp/syntaxFallback.ts @@ -243,7 +243,7 @@ export async function planLocalRename( if (locals.status === "notLocal") return { kind: "refused", - message: tx`Can't rename '${locals.name}' without the language server: only local variables can be renamed based on syntax`, + message: tx`Can't rename '${locals.name}' based on syntax: only local variables can be renamed this way`, }; const { symbol } = locals; const ranges = [symbol.definition, ...symbol.references].sort((a, b) => comparePos(a.start, b.start)); diff --git a/src/platform/locales/text/zh.json b/src/platform/locales/text/zh.json index b8242c55..7115b777 100644 --- a/src/platform/locales/text/zh.json +++ b/src/platform/locales/text/zh.json @@ -3806,7 +3806,6 @@ "Save '{word}' to project-level dictionary": "将 '{word}' 保存到项目级词典", "Save '{word}' to application-level dictionary": "将 '{word}' 保存到应用程序级词典", "Typo: In word '{word}'": "拼写错误：单词 '{word}'", - "Can't rename '{name}' without the language server: only local variables can be renamed based on syntax": "没有语言服务器时无法重命名 '{name}'：基于语法只能重命名局部变量", "Choose Super Class or Interface ({count} found)": "选择超类或接口（找到 {count} 个）", "Choose Super Method of {name} ({count} found)": "选择 {name} 的超方法（找到 {count} 个）", "Choose Implementation of {name} ({count} found)": "选择 {name} 的实现（找到 {count} 个）", @@ -4959,5 +4958,8 @@ "Tomcat server.xml has no HTTP connector": "Tomcat 的 server.xml 中没有 HTTP 连接器", "WAR contains an invalid path": "WAR 中包含无效路径", "Context path must start with / and contain only letters, digits, /, - or _": "上下文路径必须以 / 开头，且只能包含字母、数字、/、- 或 _", - "Tomcat home must contain bin/catalina.sh or bin/catalina.bat and lib/catalina.jar": "Tomcat 安装目录必须包含 bin/catalina.sh 或 bin/catalina.bat，以及 lib/catalina.jar" + "Tomcat home must contain bin/catalina.sh or bin/catalina.bat and lib/catalina.jar": "Tomcat 安装目录必须包含 bin/catalina.sh 或 bin/catalina.bat，以及 lib/catalina.jar", + "Can't rename '{name}' based on syntax: only local variables can be renamed this way": "无法基于语法重命名 '{name}'：这种方式只能重命名局部变量", + "Returns true when the value is falsey or the variable does not exist. Checking an undefined variable does not emit a warning.": "当值为假值或变量不存在时返回 true。检查未定义的变量不会产生警告。", + "Returns true only when every supplied variable exists and is not null. Checks stop at the first unset or null variable.": "仅当传入的每个变量都存在且不为 null 时返回 true。遇到第一个未设置或为 null 的变量即停止检查。" }