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
This commit is contained in:
l0ng-ai
2026-10-30 19:46:08 +08:00
committed by GitHub
parent bc70ec7028
commit b2c183dbd4
10 changed files with 195 additions and 10 deletions
+21
View File
@@ -56,6 +56,27 @@ function item(over: Partial<CompletionItem> = {}): 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(
+7
View File
@@ -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<CompletionItem>
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<CompletionResult | null
}
export function makeCompletion(item: CompletionItem, req: RequestContext): LspCompletion {
if (item.kind === "keyword" && isEmptyMarkup(item.documentation)) {
const value = phpConstructDocumentation(req.language, item.label);
if (value) item = { ...item, documentation: { kind: "markdown", value } };
}
// The lookup handles commit characters itself (focused lookups only, see `lookupKeys`).
const { commitCharacters: _server, ...cm } = toCmCompletion(item);
return Object.assign(cm, {
+19 -1
View File
@@ -35,6 +35,7 @@ import { applyCodeAction, quickFixesAt } from "./intentions";
import { editorPrefs } from "@/platform/editorSettings";
import { CONTRIBUTED_HOVER, contributedAt, type ContributedHover } from "@/editor/api";
import { navigationModifierHeld, onNavigationModifier, pointerTarget } from "@/platform/navigationModifier";
import { phpConstructAt } from "./phpConstructs";
let hoverRequest: RequestId | null = null;
@@ -48,6 +49,12 @@ export async function requestHover(view: EditorView, pos: number): Promise<Hover
}
const doc = docOf(view);
if (!doc) return null;
const construct = phpConstructAt(view.state, pos, doc.language);
if (construct)
return {
contents: { kind: "markdown", value: construct.markdown },
range: { start: offsetToPos(view.state.doc, construct.from), end: offsetToPos(view.state.doc, construct.to) },
};
if (hoverRequest !== null && isTauri()) void commands.cancelRequest(hoverRequest);
const id = (hoverRequest = nextRequestId());
try {
@@ -230,6 +237,13 @@ function showDoc(view: EditorView, entry: DocEntry, at: number) {
});
}
export function showPhpConstructDocumentation(view: EditorView, pos: number): boolean {
const construct = phpConstructAt(view.state, pos, docOf(view)?.language ?? null);
if (!construct) return false;
showDoc(view, { markup: { kind: "markdown", value: construct.markdown }, language: "PHP" }, construct.from);
return true;
}
/**
* F1: documentation for the symbol at the caret in a popup (or the
* Documentation tool window while it is open); in an open lookup, for its
@@ -263,7 +277,10 @@ export async function quickDocumentation(view: EditorView) {
{
markup: hover.contents,
language: ctx?.uiMode ?? null,
source: ctx ? definitionOf(ctx.doc, offsetToPos(view.state.doc, pos)) : undefined,
source:
ctx && !phpConstructAt(view.state, pos, ctx.language)
? definitionOf(ctx.doc, offsetToPos(view.state.doc, pos))
: undefined,
},
hover.range ? rangeToOffsets(view.state.doc, hover.range).from : pos,
);
@@ -327,6 +344,7 @@ export async function quickDefinition(view: EditorView, kind: "definition" | "ty
const doc = docOf(view);
if (!doc) return;
const pos = view.state.selection.main.head;
if (kind === "definition" && showPhpConstructDocumentation(view, pos)) return;
try {
const found = await lspRequest(() => commands.lspGoto(doc.doc, offsetToPos(view.state.doc, pos), kind));
const seen = new Set<string>();
+2 -1
View File
@@ -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<ContributedDefinition>(view.state, CONTRIBUTED_DEFINITION, offset);
if (own?.(view, offset)) return;
if (showPhpConstructDocumentation(view, offset)) return;
}
const doc = docOf(view);
if (!doc) return;
+77
View File
@@ -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(`<?php ${name}($value);`);
for (const pos of [6, 8, 6 + name.length]) {
const doc = phpConstructAt(s, pos, "php");
expect(doc).toMatchObject({ from: 6, to: 6 + name.length });
expect(doc?.markdown).toContain("PHP language construct");
expect(doc?.markdown).toContain(`function.${name.toLowerCase()}.php`);
}
});
it.each([
'<?php $empty = "empty($x)";',
"<?php // empty($x)",
"<?php /* empty($x) */",
"<?php $obj->empty();",
"<?php $obj?->empty();",
"<?php Foo::empty();",
"<?php Foo\\empty();",
"<?php function empty() {}",
"<p>empty($x)</p><?php echo 1;",
])("does not hijack other occurrences: %s", (text) => {
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("<?php empty($x);"), 8, "javascript")).toBeNull();
expect(phpConstructDocumentation("PHP", "strtoupper")).toBeNull();
expect(phpConstructDocumentation("php", "constructor")).toBeNull();
});
it("serves hover, declaration and quick definition without a running server", async () => {
const view = new EditorView({
state: EditorState.create({
doc: "<?php empty($value);",
selection: { anchor: 8 },
extensions: [php, lspDoc.of({ doc: 1, path: "/p/a.php", language: "php", uiMode: "PHP", readOnly: false })],
}),
parent: document.body,
});
views.push(view);
expect((await requestHover(view, 8))?.contents.value).toContain("empty(mixed $var): bool");
for (const action of [
() => 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();
}
});
});
+40
View File
@@ -0,0 +1,40 @@
import type { EditorState } from "@codemirror/state";
import { syntaxTree } from "@codemirror/language";
const CONSTRUCTS: Readonly<Record<string, { signature: string; description: string }>> = {
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;
}
+6 -5
View File
@@ -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<boolean> {
async function startSyntaxRename(view: EditorView, doc: EditorDocContext, pos: Pos, cause?: unknown): Promise<boolean> {
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;
+18
View File
@@ -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
+1 -1
View File
@@ -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));
+4 -2
View File
@@ -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 的变量即停止检查。"
}