diff --git a/__tests__/ui-map-api.test.ts b/__tests__/ui-map-api.test.ts index 6ba6d292e..75e083162 100644 --- a/__tests__/ui-map-api.test.ts +++ b/__tests__/ui-map-api.test.ts @@ -28,6 +28,7 @@ import { moduleIdFor, normalizeRoot, pickDefaultDepth, + passThroughDirs, pickDefaultRoot, resetMapCache, } from '../src/ui-server/api/map'; @@ -227,20 +228,20 @@ afterAll(async () => { describe('moduleIdFor', () => { it('names a module after the first `depth` segments under the root', () => { - expect(moduleIdFor('src/core/engine.ts', 'src', 1)).toEqual({ id: 'src/core', facade: false }); - expect(moduleIdFor('src/a/b/c.ts', 'src', 2)).toEqual({ id: 'src/a/b', facade: false }); - expect(moduleIdFor('a/b/c.ts', '', 1)).toEqual({ id: 'a', facade: false }); + expect(moduleIdFor('src/core/engine.ts', 'src', 1)).toEqual({ id: 'src/core', label: 'src/core', facade: false }); + expect(moduleIdFor('src/a/b/c.ts', 'src', 2)).toEqual({ id: 'src/a/b', label: 'src/a/b', facade: false }); + expect(moduleIdFor('a/b/c.ts', '', 1)).toEqual({ id: 'a', label: 'a', facade: false }); }); it('keeps a façade as its own box and buckets the other loose files', () => { - expect(moduleIdFor('src/index.ts', 'src', 1)).toEqual({ id: 'src/index.ts', facade: true }); + expect(moduleIdFor('src/index.ts', 'src', 1)).toEqual({ id: 'src/index.ts', label: 'src/index.ts', facade: true }); expect(moduleIdFor('src/lib.rs', 'src', 1)?.facade).toBe(true); expect(moduleIdFor('pkg/__init__.py', 'pkg', 1)?.facade).toBe(true); expect(moduleIdFor('src/types.ts', 'src', 1)).toEqual({ id: 'src/(root files)', - facade: false, + label: 'src/(root files)', facade: false, }); - expect(moduleIdFor('types.ts', '', 1)).toEqual({ id: '(root files)', facade: false }); + expect(moduleIdFor('types.ts', '', 1)).toEqual({ id: '(root files)', label: '(root files)', facade: false }); }); it('buckets a loose file into the directory it is actually in, not the top one', () => { @@ -249,7 +250,7 @@ describe('moduleIdFor', () => { // file lives somewhere it does not. expect(moduleIdFor('src/a/loose.ts', 'src', 2)).toEqual({ id: 'src/a/(root files)', - facade: false, + label: 'src/a/(root files)', facade: false, }); }); @@ -258,6 +259,73 @@ describe('moduleIdFor', () => { // A sibling whose name merely starts with the root is not under it. expect(moduleIdFor('srcx/y.ts', 'src', 1)).toBeNull(); }); + +describe('folders nothing forks in (Maven, Gradle)', () => { + const MAVEN = [ + 'src/main/java/org/springframework/samples/petclinic/PetClinicApplication.java', + 'src/main/java/org/springframework/samples/petclinic/owner/Owner.java', + 'src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java', + 'src/main/java/org/springframework/samples/petclinic/vet/Vet.java', + 'src/main/java/org/springframework/samples/petclinic/visit/Visit.java', + 'src/main/java/org/springframework/samples/petclinic/model/BaseEntity.java', + 'src/test/java/org/springframework/samples/petclinic/owner/OwnerControllerTests.java', + ]; + + it('finds the folders with one subfolder and no file of their own', () => { + expect([...passThroughDirs(MAVEN)].sort()).toEqual([ + 'src/main', + 'src/main/java', + 'src/main/java/org', + 'src/main/java/org/springframework', + 'src/main/java/org/springframework/samples', + 'src/test', + 'src/test/java', + 'src/test/java/org', + 'src/test/java/org/springframework', + 'src/test/java/org/springframework/samples', + 'src/test/java/org/springframework/samples/petclinic', + ]); + // A folder holding a file is a boundary even with one subfolder. + expect(passThroughDirs(['a/b/c.ts', 'a/x.ts']).has('a')).toBe(false); + }); + + it('counts depth in folders that fork, and elides the chain in the label', () => { + const through = passThroughDirs(MAVEN); + expect(moduleIdFor(MAVEN[1]!, 'src', 1, through)).toEqual({ + id: 'src/main/java/org/springframework/samples/petclinic', + label: 'src/main/…/petclinic', + facade: false, + }); + expect(moduleIdFor(MAVEN[1]!, 'src', 2, through)).toEqual({ + id: 'src/main/java/org/springframework/samples/petclinic/owner', + label: 'src/main/…/petclinic/owner', + facade: false, + }); + // The application class sits loose in the package root. + expect(moduleIdFor(MAVEN[0]!, 'src', 2, through)).toEqual({ + id: 'src/main/java/org/springframework/samples/petclinic/(root files)', + label: 'src/main/…/petclinic/(root files)', + facade: false, + }); + // Without the set, nothing changes: one level per folder. + expect(moduleIdFor(MAVEN[1]!, 'src', 2)?.id).toBe('src/main/java'); + }); + + it('opens a Maven project on its packages, not on one box', () => { + const files = MAVEN.map((p) => ({ path: p, symbols: 10, test: p.includes('/test/') })); + const through = passThroughDirs(MAVEN); + const depth = pickDefaultDepth(files, 'src', through); + const ids = new Set(files.filter((f) => !f.test).map((f) => moduleIdFor(f.path, 'src', depth, through)!.id)); + expect(depth).toBe(2); + expect([...ids].sort()).toEqual([ + 'src/main/java/org/springframework/samples/petclinic/(root files)', + 'src/main/java/org/springframework/samples/petclinic/model', + 'src/main/java/org/springframework/samples/petclinic/owner', + 'src/main/java/org/springframework/samples/petclinic/vet', + 'src/main/java/org/springframework/samples/petclinic/visit', + ]); + }); +}); }); describe('normalizeRoot', () => { diff --git a/docs/viewer-launch-changelog.md b/docs/viewer-launch-changelog.md index ba98fcfa9..910fb905e 100644 --- a/docs/viewer-launch-changelog.md +++ b/docs/viewer-launch-changelog.md @@ -161,6 +161,8 @@ These describe `codegraph ui` and its screens. They were taken out of `## [Unrel ## Fixes — Symbols, tests and the viewer +- **The Map opens a Maven or Gradle project on its packages.** A Java project keeps every file under `src/main/java/org///`, and those folders hold nothing but the next one, so the Map drew the whole program as one `src/main/java/org` box and no grouping option reached further. A folder with one subfolder and no files of its own no longer counts as a level: petclinic opens on `owner`, `vet`, `model` and `system`, each labelled `src/main/java/…/petclinic/owner`, with the full path on hover. + - In `codegraph ui`, routes whose handlers live in more than 60 different files are all linked to their handler, instead of the later ones showing "not in the index". (#1975) - The viewer continues to count module-level initializer calls as top-level file activity in entry points and file screens. diff --git a/src/ui-server/api/map.ts b/src/ui-server/api/map.ts index b593e86bd..a78f63e7f 100644 --- a/src/ui-server/api/map.ts +++ b/src/ui-server/api/map.ts @@ -106,7 +106,11 @@ export function rootFilesId(root: string): string { export interface WireMapModule { /** Directory path, or the `(root files)` bucket, or a façade file's own path. */ id: string; - /** Last path segment — what the node label shows when the id is long. */ + /** + * What the box says: the id, with a folder chain the repository never forks + * in written `first/…/last` — Maven's `src/main/java/…/petclinic/owner` + * rather than all seven segments. Equal to `id` everywhere else. + */ label: string; files: number; symbols: number; @@ -222,18 +226,82 @@ function stemOf(basename: string): string { return dot <= 0 ? basename : basename.slice(0, dot); } +/** + * Directories a module boundary never falls on: exactly one subdirectory and + * no file of their own. A Maven project keeps every line of Java under + * `src/main/java/org/springframework/samples/petclinic/`, and the first four + * of those folders split nothing — cutting at any of them draws the whole + * program as one box, and no depth the reader can pick gets past them. Such a + * folder joins the level below it, so depth counts folders that fork. + * + * Read from every indexed file, tests included, so a module's id does not + * change when the reader toggles tests. The repository root is never one. + */ +export function passThroughDirs(paths: Iterable): Set { + const children = new Map>(); + const holdsFiles = new Set(); + for (const raw of paths) { + const parts = toPosixPath(raw).split('/').filter(Boolean); + let dir = ''; + for (let i = 0; i < parts.length - 1; i++) { + let kids = children.get(dir); + if (!kids) children.set(dir, (kids = new Set())); + kids.add(parts[i]!); + dir = dir ? `${dir}/${parts[i]}` : parts[i]!; + } + holdsFiles.add(dir); + } + const out = new Set(); + for (const [dir, kids] of children) { + if (dir !== '' && kids.size === 1 && !holdsFiles.has(dir)) out.add(dir); + } + return out; +} + +/** + * Where each level of a file's directory path ends, as indexes into `dirs`: a + * level is one folder plus every pass-through folder that follows it. + */ +function levelEnds(root: string, dirs: readonly string[], passThrough: ReadonlySet | undefined): number[] { + const ends: number[] = []; + for (let i = 0; i < dirs.length; ) { + i++; + while (passThrough && i < dirs.length && passThrough.has(joinPath(root, dirs.slice(0, i)))) i++; + ends.push(i); + } + return ends; +} + +function joinPath(root: string, segments: readonly string[]): string { + return [root, ...segments].filter(Boolean).join('/'); +} + +/** The first `levels` levels of `dirs`, a chain of three or more folders written `first/…/last`. */ +function levelLabel(root: string, dirs: readonly string[], ends: readonly number[]): string { + const out: string[] = root ? [root] : []; + let from = 0; + for (const end of ends) { + const chain = dirs.slice(from, end); + out.push(chain.length >= 3 ? `${chain[0]}/…/${chain[chain.length - 1]}` : chain.join('/')); + from = end; + } + return out.join('/'); +} + /** * Which module a file belongs to, or `null` when it is outside the root. * - * `depth` segments under the root name the module. A file with fewer segments - * than that is loose in the root: a façade keeps its own box, everything else - * joins the `(root files)` bucket. + * `depth` levels under the root name the module — a level being a folder and + * the {@link passThroughDirs} that follow it (without them, one level per + * folder). A file with fewer levels than that is loose in the root: a façade + * keeps its own box, everything else joins the `(root files)` bucket. */ export function moduleIdFor( filePath: string, root: string, - depth: number -): { id: string; facade: boolean } | null { + depth: number, + passThrough?: ReadonlySet +): { id: string; facade: boolean; label: string } | null { const path = toPosixPath(filePath); let rel = path; if (root) { @@ -242,16 +310,24 @@ export function moduleIdFor( } const parts = rel.split('/').filter(Boolean); if (parts.length === 0) return null; - if (parts.length <= depth) { + const dirs = parts.slice(0, -1); + const ends = levelEnds(root, dirs, passThrough); + if (ends.length < depth) { // A loose file. The directories it DOES have still qualify it, so // `src/a/b.ts` at depth 2 lands in `src/a/(root files)`, not the top one. - const dir = [root, ...parts.slice(0, -1)].filter(Boolean).join('/'); - if (FACADE_STEMS.has(stemOf(parts[parts.length - 1] ?? ''))) { - return { id: [root, ...parts].filter(Boolean).join('/'), facade: true }; + const dir = joinPath(root, dirs); + const dirLabel = levelLabel(root, dirs, ends); + const file = parts[parts.length - 1] ?? ''; + if (FACADE_STEMS.has(stemOf(file))) { + return { id: joinPath(root, parts), facade: true, label: dirLabel ? `${dirLabel}/${file}` : file }; } - return { id: rootFilesId(dir), facade: false }; + return { id: rootFilesId(dir), facade: false, label: rootFilesId(dirLabel) }; } - return { id: [root, ...parts.slice(0, depth)].filter(Boolean).join('/'), facade: false }; + return { + id: joinPath(root, dirs.slice(0, ends[depth - 1])), + facade: false, + label: levelLabel(root, dirs, ends.slice(0, depth)), + }; } /** @@ -318,13 +394,14 @@ const MAX_MODULES = 60; function tallyModules( files: ReadonlyArray<{ path: string; symbols: number; test: boolean }>, root: string, - depth: number + depth: number, + passThrough?: ReadonlySet ): { count: number; share: number; largestFiles: number } { const byModule = new Map(); let total = 0; for (const file of files) { if (file.test) continue; - const assigned = moduleIdFor(file.path, root, depth); + const assigned = moduleIdFor(file.path, root, depth, passThrough); if (assigned === null) continue; let entry = byModule.get(assigned.id); if (!entry) byModule.set(assigned.id, (entry = { symbols: 0, files: 0 })); @@ -364,7 +441,8 @@ function tallyModules( */ export function pickDefaultDepth( files: ReadonlyArray<{ path: string; symbols: number; test: boolean }>, - root: string + root: string, + passThrough?: ReadonlySet ): number { // Past the deepest directory, a bigger number only renames boxes to // `src/a/(root files)`. There is nothing below the leaves. @@ -374,13 +452,14 @@ export function pickDefaultDepth( const path = toPosixPath(file.path); if (root && !path.startsWith(`${root}/`)) continue; const rel = root ? path.slice(root.length + 1) : path; - deepest = Math.max(deepest, rel.split('/').filter(Boolean).length - 1); + const dirs = rel.split('/').filter(Boolean).slice(0, -1); + deepest = Math.max(deepest, levelEnds(root, dirs, passThrough).length); } let fallback = DEFAULT_DEPTH; let fallbackCount = 0; for (let depth = DEFAULT_DEPTH; depth <= Math.min(MAX_DEPTH, deepest); depth += 1) { - const tally = tallyModules(files, root, depth); + const tally = tallyModules(files, root, depth, passThrough); if (tally.count === 0) break; // Deeper only gets more crowded from here. if (tally.count > MAX_MODULES) break; @@ -494,10 +573,11 @@ export function buildMap(cg: CodeGraph, projectRoot: string, query: URLSearchPar }); const root = requestedRoot ?? pickDefaultRoot(fileRecords); + const passThrough = passThroughDirs(fileRecords.map((f) => f.path)); // Root first, then depth against THAT root: how finely to cut depends on // what is being cut. Choosing `src` and then asking for one level under it // is the same question as choosing the whole project and asking for two. - const depth = requestedDepth ?? pickDefaultDepth(fileRecords, root); + const depth = requestedDepth ?? pickDefaultDepth(fileRecords, root, passThrough); const stats = cg.getStats(); const key = [ projectRoot, @@ -531,12 +611,18 @@ export function buildMap(cg: CodeGraph, projectRoot: string, query: URLSearchPar >(); const moduleOfFile = new Map(); - const assigned = new Map(); + const assigned = new Map(); + const labelOf = new Map(); for (const file of fileRecords) { - const at = moduleIdFor(file.path, root, depth); - if (at !== null) assigned.set(file.path, at); + const at = moduleIdFor(file.path, root, depth, passThrough); + if (at === null) continue; + assigned.set(file.path, at); + labelOf.set(at.id, at.label); } const renamed = collapseLoneRootFiles(new Set([...assigned.values()].map((a) => a.id))); + for (const [from, to] of renamed) { + labelOf.set(to, (labelOf.get(from) ?? to).replace(/\/\(root files\)$/, '')); + } for (const file of fileRecords) { const at = assigned.get(file.path); @@ -625,7 +711,7 @@ export function buildMap(cg: CodeGraph, projectRoot: string, query: URLSearchPar const shown = entry.paths.slice().sort().slice(0, MAX_FILES_PER_MODULE); return { id: entry.id, - label: entry.id.slice(entry.id.lastIndexOf('/') + 1) || entry.id, + label: labelOf.get(entry.id) ?? entry.id, files: entry.files, symbols: entry.symbols, languages: [...entry.languages] diff --git a/ui/src/components/map/ModuleNode.svelte b/ui/src/components/map/ModuleNode.svelte index 9d2f2bbd8..8748a21c5 100644 --- a/ui/src/components/map/ModuleNode.svelte +++ b/ui/src/components/map/ModuleNode.svelte @@ -60,7 +60,8 @@ layout.generated ? '. Every file in it is tool-generated.' : '' }`} > - {module.id} + + {module.label || module.id} {moduleMetaLabel(module, layout.island)} { const island = islands.has(m.id); - const lines = options.sizing?.(m, island) ?? { label: m.id, meta: moduleMetaLabel(m, island) }; + const lines = options.sizing?.(m, island) ?? { label: m.label || m.id, meta: moduleMetaLabel(m, island) }; const count = portCount.get(m.id) ?? { top: 0, bottom: 0 }; const forPorts = (Math.max(count.top, count.bottom) + 1) * portPitch; return [m.id, Math.max(nodeWidth(lines.label, lines.meta), forPorts)]; diff --git a/ui/src/lib/wire.ts b/ui/src/lib/wire.ts index 95d04f09c..227989e65 100644 --- a/ui/src/lib/wire.ts +++ b/ui/src/lib/wire.ts @@ -596,6 +596,7 @@ export interface WireFlowPayload { export interface WireMapModule { /** Directory path, the `(root files)` bucket, or a façade file's own path. */ id: string; + /** What the box says: the id, a folder chain nothing forks in written `first/…/last`. */ label: string; files: number; symbols: number;