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

@@ -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();
+  });
+});

src/editor/lsp/files.ts

@@ -53,8 +53,28 @@ export async function withDocument<T>(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;

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,

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),

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);