From efd03630b64ae0cbc9fb88e0519b69c55a285e6c Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Tue, 3 Nov 2026 18:40:26 +0800 Subject: [PATCH] fix(navigation): label string-literal targets by their literal (#1025) Navigation targets that start at a string literal (a Spring @Value("${key}") consumer reached from a configuration key) are named by the literal instead of the first identifier fragment inside it, which showed as "$". Applies to the declaration chooser, the quick-definition popup and the gutter line-marker chooser. --- src/editor/lsp/files.test.ts | 36 ++++++++++++++++++++++++++++ src/editor/lsp/files.ts | 22 ++++++++++++++++- src/editor/lsp/hover.ts | 4 ++-- src/editor/lsp/navigation.ts | 4 ++-- src/editor/navigation/lineMarkers.ts | 4 ++-- 5 files changed, 63 insertions(+), 7 deletions(-) create mode 100644 src/editor/lsp/files.test.ts diff --git a/src/editor/lsp/files.test.ts b/src/editor/lsp/files.test.ts new file mode 100644 index 00000000..b34b0a1b --- /dev/null +++ b/src/editor/lsp/files.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from "vitest"; +import { targetLabel } from "./files"; + +describe("targetLabel", () => { + const at = (line: string, needle: string) => targetLabel(line, line.indexOf(needle)); + + it.each([ + // Spring configuration consumers start at the string literal, not a name. + [' @Value("${app.name}") private String appName;', '"${app', '"${app.name}"'], + [" @Value('${app.name}') String appName", "'${app", "'${app.name}'"], + [" const key = `app.${env}`;", "`app", "`app.${env}`"], + [' String s = "say \\"hi\\"" + x;', '"say', '"say \\"hi\\""'], + // Unterminated (multi-line) literals end at the line end. + [' String block = "unterminated', '"unterminated', '"unterminated'], + // Ordinary declarations keep the identifier. + [" private String appName;", "appName", "appName"], + [" public int getTimeOut() { return timeOut; }", "getTimeOut", "getTimeOut"], + [" int $inner = 1;", "$inner", "$inner"], + [" time-out: 30", "time", "time"], + ])("%s", (line, needle, expected) => { + expect(at(line, needle)).toBe(expected); + }); + + it("shortens long literals", () => { + const line = `x("${"a".repeat(100)}")`; + const label = targetLabel(line, 2)!; + expect(label).toHaveLength(60); + expect(label.endsWith("…")).toBe(true); + }); + + it("has no label for empty literals, numbers or blank positions", () => { + expect(targetLabel(' x = "";', 6)).toBeNull(); + expect(targetLabel(" port: 8080", 8)).toBeNull(); + expect(targetLabel(" ", 1)).toBeNull(); + }); +}); diff --git a/src/editor/lsp/files.ts b/src/editor/lsp/files.ts index 2e4d0ba2..cf2ac0e8 100644 --- a/src/editor/lsp/files.ts +++ b/src/editor/lsp/files.ts @@ -53,8 +53,28 @@ export async function withDocument(path: string, fn: (doc: number) => Promise } } +/** Longest literal label before it is shortened with an ellipsis. */ +const MAX_LITERAL_LABEL = 60; + +/** + * Display name of a navigation target starting at `col` of `line`: the string + * literal when the target is one (a Spring `@Value("${key}")` consumer, a + * configuration key in quotes), else the identifier there; null when neither. + */ +export function targetLabel(line: string, col: number): string | null { + const quote = line[col]; + if (quote === '"' || quote === "'" || quote === "`") { + let end = col + 1; + while (end < line.length && line[end] !== quote) end += line[end] === "\\" ? 2 : 1; + const literal = line.slice(col, Math.min(end + 1, line.length)); + if (literal.length > 2) + return literal.length > MAX_LITERAL_LABEL ? `${literal.slice(0, MAX_LITERAL_LABEL - 1)}…` : literal; + } + return identifierAt(line, col); +} + /** The identifier at (or starting right after) `col` of `line`; null when there is none. */ -export function identifierAt(line: string, col: number): string | null { +function identifierAt(line: string, col: number): string | null { const re = /[\p{L}\p{N}_$]+/gu; for (let m = re.exec(line); m; m = re.exec(line)) { const end = m.index + m[0].length; diff --git a/src/editor/lsp/hover.ts b/src/editor/lsp/hover.ts index 4da03f15..3dba332b 100644 --- a/src/editor/lsp/hover.ts +++ b/src/editor/lsp/hover.ts @@ -26,7 +26,7 @@ import { offsetToPos, rangeToOffsets } from "./positions"; import { showHint } from "./hint"; import { caretAnchor, focusPopup, openPopup, useLspPopup, type DefinitionTarget, type DocEntry } from "./popupStore"; import { selectedLookupDoc, showLookupDocs } from "./completion"; -import { identifierAt, readFileLines, withDocument } from "./files"; +import { readFileLines, targetLabel, withDocument } from "./files"; import { docWindowVisible, moveDocPopupToWindow, showInDocWindow } from "./docs"; import { structure } from "@/editor/structure"; import { basename } from "@/platform/project"; @@ -326,7 +326,7 @@ async function definitionTarget(loc: Location, fallbackLanguage: string | null): const [lines, declaration] = await Promise.all([readFileLines(loc.path), declarationRange(loc)]); if (!lines) return null; const { first, last } = definitionSpan(loc, lines.length, declaration); - const name = identifierAt(lines[loc.range.start.line] ?? "", loc.range.start.col) ?? basename(loc.path); + const name = targetLabel(lines[loc.range.start.line] ?? "", loc.range.start.col) ?? basename(loc.path); return { location: loc, name, diff --git a/src/editor/lsp/navigation.ts b/src/editor/lsp/navigation.ts index 7afdda09..848652c0 100644 --- a/src/editor/lsp/navigation.ts +++ b/src/editor/lsp/navigation.ts @@ -48,7 +48,7 @@ import { } from "./syntaxFallback"; import { showHint, showPendingHint } from "./hint"; import { caretAnchor, openPopup, type ListPopupItem } from "./popupStore"; -import { identifierAt, readManyFileLines } from "./files"; +import { readManyFileLines, targetLabel } from "./files"; import { documentLinks, followReference, knownLinks, linkReference, referenceAt, textReference } from "./links"; import { featureBackoff } from "./availability"; import { requestHover, showPhpConstructDocumentation } from "./hover"; @@ -111,7 +111,7 @@ export async function locationItems(locs: readonly DeclarationTarget[]): Promise run: () => navigateTo(l), }; // Server targets: the declared name (speed search matches it), file and line after it. - const name = identifierAt(files.get(l.path)?.[l.range.start.line] ?? "", l.range.start.col); + const name = targetLabel(files.get(l.path)?.[l.range.start.line] ?? "", l.range.start.col); return { id: `${i}`, label: name ?? basename(l.path), diff --git a/src/editor/navigation/lineMarkers.ts b/src/editor/navigation/lineMarkers.ts index b52da96f..50f9727b 100644 --- a/src/editor/navigation/lineMarkers.ts +++ b/src/editor/navigation/lineMarkers.ts @@ -6,7 +6,7 @@ */ import type { Location, OutlineItem, SymbolKind } from "@/platform/ipc"; import { tx } from "@/platform/i18n"; -import { identifierAt } from "@/editor/lsp/files"; +import { targetLabel } from "@/editor/lsp/files"; import { baseName, type SuperMethod } from "./structureNav"; export type MarkerKind = "implemented" | "overridden" | "implementing" | "overriding"; @@ -151,7 +151,7 @@ export function enclosingTypeName(lines: readonly string[] | null | undefined, t /** Display name of a target: the type itself for type markers, the type around it for methods. */ export function targetName(lines: readonly string[] | null | undefined, target: Location, method: boolean): string { if (!method) { - const name = identifierAt(lines?.[target.range.start.line] ?? "", target.range.start.col); + const name = targetLabel(lines?.[target.range.start.line] ?? "", target.range.start.col); if (name) return name; } return enclosingTypeName(lines, target);