mirror of
https://github.com/l0ng-ai/openidea.git
synced 2026-11-14 16:07:52 +00:00
perf(editor): find a type's overridden methods with one subtype walk (#2680)
The gutter's "is implemented / overridden" markers sent one implementation request per visible method, and jdtls builds the type hierarchy again for each, so a large interface in a big project kept it busy long enough to hold up navigation. When outline names carry the signature (jdtls), a type's methods are now resolved from one walk of its subtypes and their outlines, as the super-method markers are; used only while it takes fewer requests than it replaces, and a method whose name a subtype reuses with another signature (generic overloads) still gets its own request. Refs #2314
This commit is contained in:
l0ng-ai
@@ -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<MarkerCandidate, Location[]>();
|
||||
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<Map<MarkerCandidate, Location[]>> {
|
||||
const known = new Map<MarkerCandidate, Location[]>();
|
||||
if (!supers.subtypes) return known;
|
||||
const byOwner = new Map<string, MarkerCandidate[]>();
|
||||
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<HierarchyItem[] | null> {
|
||||
const all: HierarchyItem[] = [];
|
||||
const seen = new Set<string>();
|
||||
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[],
|
||||
|
||||
@@ -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<T> {",
|
||||
" 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<T>",
|
||||
"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<string, HierarchyItem[]>) {
|
||||
const calls = { implementations: [] as Pos[][], subtypes: 0 };
|
||||
const outlines: Record<string, OutlineItem[]> = {
|
||||
"/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<typeof emptyCache>, 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 },
|
||||
],
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<HierarchyItem[]>;
|
||||
/** Direct subtypes, when the type hierarchy answers it (fallback for implementation requests). */
|
||||
|
||||
Reference in New Issue
Block a user