diff --git a/src/editor/navigation/implementationGutter.ts b/src/editor/navigation/implementationGutter.ts index 5caa0145d..aa5be1e84 100644 --- a/src/editor/navigation/implementationGutter.ts +++ b/src/editor/navigation/implementationGutter.ts @@ -46,7 +46,7 @@ import { type MarkerCandidate, type MarkerGroup, } from "./lineMarkers"; -import { superMethodTargets, type SuperDeps, type SuperMethod } from "./structureNav"; +import { overridersInType, superMethodTargets, type SuperDeps, type SuperMethod } from "./structureNav"; import styles from "./LineMarkers.module.css"; /** Lines above / below the viewport that are covered too, so scrolling feels instant. */ @@ -172,16 +172,20 @@ export async function fillMarkers( : []; let implementations: Location[][] = []; + const known = down.length ? await fromHierarchy(path, down, deps.supers) : new Map(); if (down.length) { - try { - implementations = (await deps.implementations(down.map((c) => ({ line: c.line, col: c.col })))).map((locs, i) => - withoutSelf(path, down[i]!, locs), - ); - } catch (e) { - if (isCancelled(e)) throw e; - if (isUnavailable(e)) cache.unsupported.add("inheritors"); - down.length = 0; + const ask = down.filter((c) => !known.has(c)); + let asked: Location[][] = []; + if (ask.length) { + try { + asked = await deps.implementations(ask.map((c) => ({ line: c.line, col: c.col }))); + } catch (e) { + if (isCancelled(e)) throw e; + if (isUnavailable(e)) cache.unsupported.add("inheritors"); + for (let i = down.length - 1; i >= 0; i--) if (!known.has(down[i]!)) down.splice(i, 1); + } } + implementations = down.map((c) => withoutSelf(path, c, known.get(c) ?? asked[ask.indexOf(c)] ?? [])); } // Some servers (Pyrefly) answer implementation requests with nothing while their // type hierarchy still knows the subtypes: use it for the ↓ markers of types. @@ -189,7 +193,7 @@ export async function fillMarkers( if (subtypes) { await Promise.all( down.map(async (c, i) => { - if (c.method || (implementations[i]?.length ?? 0)) return; + if (c.method || known.has(c) || (implementations[i]?.length ?? 0)) return; try { const found = await subtypes(path, { line: c.line, col: c.col }); implementations[i] = found.map((s) => ({ path: s.path, range: s.selectionRange })); @@ -210,6 +214,90 @@ export async function fillMarkers( up.forEach((c, i) => (entry(c).supers = supersMarker(c, supers[i]!, files))); } +/** + * Overriders of a type's methods from one walk of its subtypes and their outlines, instead of one + * implementation request per method: jdtls builds the type hierarchy again for every implementation + * request, so a large interface kept it busy long enough to hold up navigation. Only for servers + * whose outline names carry the signature (`save(T, int)`), and only while the walk takes fewer + * requests than it replaces. A method whose name a subtype reuses with another signature + * (`save(Order)` for a generic `save(T)`) is left to its implementation request. The type's own + * marker lists the subtypes found. + */ +async function fromHierarchy( + path: string, + down: readonly MarkerCandidate[], + supers: SuperDeps, +): Promise> { + const known = new Map(); + if (!supers.subtypes) return known; + const byOwner = new Map(); + for (const c of down) { + if (!c.method || !c.owner || !c.symbol.includes("(")) continue; + const key = `${c.owner.line}:${c.owner.col}`; + byOwner.set(key, [...(byOwner.get(key) ?? []), c]); + } + for (const methods of byOwner.values()) { + if (methods.length < 2) continue; + const owner = methods[0]!.owner!; + const subtypes = await allSubtypes(path, owner, supers, methods.length - 1); + if (!subtypes) continue; + const type = down.find((c) => !c.method && c.line === owner.line && c.col === owner.col); + if (type) + known.set( + type, + subtypes.map((s) => ({ path: s.path, range: s.selectionRange })), + ); + for (const m of methods) { + const targets: Location[] = []; + let unsure = false; + for (const s of subtypes) { + const outline = await supers.outline(s.path).catch((e) => { + if (isCancelled(e)) throw e; + return null; + }); + const found = outline && overridersInType(outline, s, m.symbol); + if (!found) { + unsure = true; + break; + } + for (const o of found) targets.push({ path: s.path, range: o.selection_range }); + } + if (!unsure) known.set(m, targets); + } + } + return known; +} + +/** Every subtype of the type at `pos`, or null when the walk would take more than `budget` requests or fails. */ +async function allSubtypes(path: string, pos: Pos, supers: SuperDeps, budget: number): Promise { + const all: HierarchyItem[] = []; + const seen = new Set(); + let level = [{ path, pos }]; + let asked = 0; + while (level.length) { + const next: Array<{ path: string; pos: Pos }> = []; + for (const t of level) { + if (++asked > budget) return null; + let found: HierarchyItem[]; + try { + found = await supers.subtypes!(t.path, t.pos); + } catch (e) { + if (isCancelled(e)) throw e; + return null; + } + for (const s of found) { + const key = `${s.path}:${s.selectionRange.start.line}:${s.selectionRange.start.col}`; + if (seen.has(key)) continue; + seen.add(key); + all.push(s); + next.push({ path: s.path, pos: s.selectionRange.start }); + } + } + level = next; + } + return all; +} + /** Markers known for `candidates`, in document order (↓ before ↑ on a line). */ export function cachedMarkers( candidates: readonly MarkerCandidate[], diff --git a/src/editor/navigation/lineMarkers.test.ts b/src/editor/navigation/lineMarkers.test.ts index 7d17b405a..29f12aac9 100644 --- a/src/editor/navigation/lineMarkers.test.ts +++ b/src/editor/navigation/lineMarkers.test.ts @@ -188,7 +188,9 @@ function server( }, subtypes: async (p) => { calls.subtypes++; - return subtypes[p] ?? []; + // A path without an entry: the server has no type hierarchy for it. + if (!(p in subtypes)) throw new IpcError({ kind: "unavailable", message: "no type hierarchy" }); + return subtypes[p]!; }, outline: async (p) => outlines[p] ?? [], }, @@ -406,3 +408,151 @@ describe("clicking a marker", () => { expect(nav.openPopup.mock.calls[0]![0].items).toHaveLength(2); }); }); + +describe("overriders of a type's methods from its subtypes", () => { + // Shapes and names as jdtls answers them (outline names carry the signature). + const REPO = [ + "public interface Repo {", + " void save(T item);", + " void save(T item, int times);", + " T find(String id);", + " default int count() { return 0; }", + " void flush();", + " void onlyAnon();", + "}", + ]; + const REPO_OUTLINE = [ + item( + "Repo", + "interface", + 0, + 7, + [ + item("save(T)", "method", 1, 1, [], 9), + item("save(T, int)", "method", 2, 2, [], 9), + item("find(String)", "method", 3, 3, [], 6), + item("count()", "method", 4, 4, [], 16), + item("flush()", "method", 5, 5, [], 9), + item("onlyAnon()", "method", 6, 6, [], 9), + ], + 17, + ), + ]; + const ORDER_REPO_OUTLINE = [ + item("OrderRepo", "class", 0, 7, [ + item("save(Order)", "method", 1, 1), + item("save(Order, int)", "method", 2, 2), + item("find(String)", "method", 3, 3), + item("count()", "method", 4, 4), + // An overload, not an override. + item("save(String)", "method", 5, 5), + item("onlyAnon()", "method", 6, 6), + ]), + ]; + const SUB_REPO_OUTLINE = [item("SubRepo", "class", 0, 3, [item("find(String)", "method", 1, 1)])]; + // jdtls lists anonymous classes as subtypes, nested under the field they initialize. + const ANON_OUTLINE = [ + item("Anon", "class", 0, 9, [ + item("repo", "field", 2, 8, [ + item("new Repo() {...}", "class", 2, 8, [ + item("save(String)", "method", 3, 3), + item("save(String, int)", "method", 4, 4), + item("find(String)", "method", 5, 5), + item("flush()", "method", 6, 6), + item("onlyAnon()", "method", 7, 7), + ]), + ]), + ]), + ]; + const at = (name: string, path: string, line: number, col = 4): HierarchyItem => ({ + ...hier(name, "class", path), + selectionRange: { start: { line, col }, end: { line, col: col + name.length } }, + }); + const orderRepo = at("OrderRepo", "/p/OrderRepo.java", 0); + const subRepo = at("SubRepo", "/p/SubRepo.java", 0); + const anon = at("new Repo() {...}", "/p/Anon.java", 2, 30); + + function repoServer(subtypes: Record) { + const calls = { implementations: [] as Pos[][], subtypes: 0 }; + const outlines: Record = { + "/p/Repo.java": REPO_OUTLINE, + "/p/OrderRepo.java": ORDER_REPO_OUTLINE, + "/p/SubRepo.java": SUB_REPO_OUTLINE, + "/p/Anon.java": ANON_OUTLINE, + }; + const deps: MarkerDeps = { + implementations: async (positions) => { + calls.implementations.push(positions); + return positions.map(() => [loc("/p/OrderRepo.java", 1)]); + }, + supers: { + supertypes: async () => [], + subtypes: async (p) => { + calls.subtypes++; + return subtypes[p] ?? []; + }, + outline: async (p) => outlines[p] ?? [], + }, + readFiles: async (paths) => new Map(paths.map((p) => [p, null])), + }; + return { calls, deps }; + } + const targetsOf = (cache: ReturnType, candidates: MarkerCandidate[]) => + cachedMarkers(candidates, ALL, cache).map((m) => [ + m.line, + m.targets.map((t) => `${t.path.slice(3)}:${t.range.start.line}`).sort(), + ]); + + it("walks the subtypes once instead of asking for each method", async () => { + const { deps, calls } = repoServer({ + "/p/Repo.java": [orderRepo, anon], + "/p/OrderRepo.java": [subRepo], + }); + const candidates = markerCandidates(REPO_OUTLINE, lines(REPO)); + const cache = emptyCache(); + await fillMarkers("/p/Repo.java", candidates, ALL, deps, cache); + // Repo, OrderRepo, Anon and SubRepo: fewer requests than the six methods. + expect(calls.subtypes).toBe(4); + // Only the generic save(T) overloads need the server: save(Order) / save(String) may or may not override them. + expect(calls.implementations).toEqual([ + [ + { line: 1, col: 9 }, + { line: 2, col: 9 }, + ], + ]); + expect(targetsOf(cache, candidates)).toEqual([ + [0, ["Anon.java:2", "OrderRepo.java:0", "SubRepo.java:0"]], + [1, ["OrderRepo.java:1"]], + [2, ["OrderRepo.java:1"]], + [3, ["Anon.java:5", "OrderRepo.java:3", "SubRepo.java:1"]], + [4, ["OrderRepo.java:4"]], + [5, ["Anon.java:6"]], + [6, ["Anon.java:7", "OrderRepo.java:6"]], + ]); + }); + + it("needs one request for a type without subtypes", async () => { + const { deps, calls } = repoServer({}); + const candidates = markerCandidates(REPO_OUTLINE, lines(REPO)); + const cache = emptyCache(); + await fillMarkers("/p/Repo.java", candidates, ALL, deps, cache); + expect(calls.subtypes).toBe(1); + expect(calls.implementations).toEqual([]); + expect(cachedMarkers(candidates, ALL, cache)).toEqual([]); + }); + + it("asks for each method when the walk would cost more than it saves", async () => { + const { deps, calls } = repoServer({ "/p/Repo.java": [orderRepo], "/p/OrderRepo.java": [subRepo] }); + // Two methods: walking Repo, OrderRepo and SubRepo would take more requests than asking twice. + const candidates = markerCandidates(REPO_OUTLINE, lines(REPO)).filter((c) => !c.method || c.line >= 5); + await fillMarkers("/p/Repo.java", candidates, ALL, deps, emptyCache()); + expect(calls.subtypes).toBe(1); + expect(calls.implementations).toEqual([ + [ + { line: 0, col: 17 }, + { line: 5, col: 9 }, + { line: 6, col: 9 }, + ], + ]); + }); +}); diff --git a/src/editor/navigation/structureNav.ts b/src/editor/navigation/structureNav.ts index b1cb3d064..32cc38448 100644 --- a/src/editor/navigation/structureNav.ts +++ b/src/editor/navigation/structureNav.ts @@ -90,6 +90,24 @@ export function methodsInType(outline: readonly OutlineItem[], item: HierarchyIt return exact.length ? exact : named; } +/** + * Methods of the type declared at `item` that override `signature` (`save(T, int)` as jdtls names + * it): those with the same signature, or `null` when one with the same name has another signature + * (`save(Order)` for a generic `save(T)`) and only the server can tell whether it overrides. + */ +export function overridersInType( + outline: readonly OutlineItem[], + item: HierarchyItem, + signatureName: string, +): OutlineItem[] | null { + const type = typeOf(outline, item); + if (!type) return null; + const bare = baseName(signatureName); + const named = type.children.filter((c) => METHODS.has(c.kind) && baseName(c.name) === bare); + const exact = named.filter((c) => signature(c.name) === signature(signatureName)); + return named.length > exact.length ? null : exact; +} + export interface SuperDeps { supertypes: (path: string, pos: Pos) => Promise; /** Direct subtypes, when the type hierarchy answers it (fallback for implementation requests). */