diff --git a/.changeset/check-shared-roots.md b/.changeset/check-shared-roots.md new file mode 100644 index 0000000..1d35b60 --- /dev/null +++ b/.changeset/check-shared-roots.md @@ -0,0 +1,13 @@ +--- +"uptide": minor +--- + +`check` reports one breaking finding when call sites in different workspaces trace back to +the same local declaration (a parameter or prop whose type names something from the upgraded +package): the finding is anchored at the declaration, where the one edit is, and the call +sites are listed under it as evidence with an `N call sites in M workspaces` line, in the +terminal, the HTML report and the JSON report. Before, a site alone in its workspace stayed a +finding of its own, so a hook typed `RefObject` and called from three +workspaces was three findings plus the one that mattered. The JSON report gains `root` on +call-site findings and `workspace` on evidence sites; nothing is renamed or removed. The pack +test scorer counts the anchor as the predicted site and never the evidence under it. diff --git a/docs/architecture.md b/docs/architecture.md index c7b8689..c3ad001 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -589,8 +589,19 @@ the target's `useRef` returns `RefObject`) is anchored at th `anchorOnly`, when at least two sites trip it: the one edit is the parameter's type, as the migration guides say, and the call sites are evidence. The parameter may sit in another workspace whose source the program maps (`packages/editor` for a call in `packages/tldraw`); -its path is the repository's. A parameter one site trips is left to -that site, which may as well be the argument's. +its path is the repository's. A parameter one site trips in its workspace is left to that +site, which may as well be the argument's, but the declaration it traces to is kept on the +diagnostic (`root`) and on the finding (`Finding.root`, repository-relative). After the +workspaces are merged, `foldSharedRoots` (`check/root-cause.ts`) compares roots across them: +two or more sites at one declaration, or a site plus the anchor another workspace already +produced, become one anchored finding at the declaration (`anchorOnly`, the sites as +`downstream`, each with its `workspace`, `details` opening with `N call sites in M +workspaces`), filed under the pack rule the sites agreed on. tldraw's `useTransform(ref)` is +called from three workspaces and was fixed upstream once, in the hook's parameter; `check` +now reports that one site. The JSON report only gains fields for this (`root`, +`downstream[].workspace`); nothing is renamed or removed. The pack test scorer counts the +anchor as the predicted site and never the evidence under it, even when another finding +lands on an evidence line. A diagnostic confirms exactly one usage: the innermost whose span contains the diagnostic's start. `z.string().trim().url()` is three usages on one line, and an error diff --git a/docs/cli.md b/docs/cli.md index afc74e9..ff53dfc 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -126,7 +126,13 @@ from it, every element in every file errors with one fix: `check` reports one fi diagnostics as evidence, and `--details` shows a few of them. A repository parameter that several call sites trip over (a hook typed `RefObject` once `useRef` returns `RefObject`) is reported the same way: one finding at the parameter, with -the call sites as evidence. +the call sites as evidence, and `--details` prints `N call sites in M workspaces` under it +with a few of them. The call sites may sit in other workspaces than the parameter (a hook in +`packages/editor` called from `apps/examples` and `packages/tldraw`): a site alone in its +workspace still folds into the one finding at the declaration once the workspaces are merged. +The HTML report lists the call sites, with their workspaces, under the anchor; the JSON report +carries them as the finding's `downstream`, and `root` on each call-site finding names the +declaration it traces to. ### Partial results diff --git a/packages/cli/src/format-check-details.test.ts b/packages/cli/src/format-check-details.test.ts index 823b307..8d886a6 100644 --- a/packages/cli/src/format-check-details.test.ts +++ b/packages/cli/src/format-check-details.test.ts @@ -450,3 +450,62 @@ it('prints one native warning for a package regardless of how many findings it c const text = formatCheck({ ...report, packages: [p] }); expect(text.match(/native package, runtime probe skipped/g)).toHaveLength(1); }); + +it('prints the call sites under a declaration several workspaces trip, then a few as examples', () => { + const root = { + name: 'ref', + file: 'packages/editor/src/useTransform.ts', + line: 6, + reason: + "whose parameter `ref: RefObject` no longer accepts what the target gives it; widen the parameter's type there", + }; + const anchor = finding({ + change: { package: 'react', from: '18.3.1', to: '19.2.1', kind: 'cause', path: 'cause:ref' }, + usage: { file: root.file, line: 6, symbolPath: 'cause:ref', access: 'read', via: 'inferred' }, + anchorOnly: true, + root, + downstream: [ + { + file: 'apps/examples/src/Custom.tsx', + line: 11, + code: 2345, + message: 'not assignable', + workspace: 'apps/examples', + }, + { + file: 'packages/tldraw/src/Foreground.tsx', + line: 59, + code: 2345, + message: 'not assignable', + workspace: 'packages/tldraw', + }, + ], + details: ['2 call sites in 2 workspaces'], + fixability: 'assisted', + reason: root.reason, + evidence: 'compiler', + }); + const out = formatCheck( + { + ...report, + packages: [ + { + ...(report.packages[0] as PackageReport), + name: 'react', + workspace: '*', + installed: '18.3.1', + target: '19.2.1', + latest: '19.2.1', + findings: [anchor], + }, + ], + }, + { color: false }, + ); + expect(out).toContain(' ✗ packages/editor/src/useTransform.ts:6 `ref`'); + expect(out).toContain(' 2 call sites in 2 workspaces, for example'); + expect(out).toContain(' apps/examples/src/Custom.tsx:11 TS2345: not assignable'); + expect(out).toContain(' packages/tldraw/src/Foreground.tsx:59 TS2345: not assignable'); + // The count is printed once, not again from the details. + expect(out.split('2 call sites in 2 workspaces')).toHaveLength(2); +}); diff --git a/packages/cli/src/format-check-details.ts b/packages/cli/src/format-check-details.ts index e90e2d9..2efbefb 100644 --- a/packages/cli/src/format-check-details.ts +++ b/packages/cli/src/format-check-details.ts @@ -1,5 +1,6 @@ import { type CheckReport, + callSitesLine, type Finding, isNativeProbeSkip, type PackageReport, @@ -99,7 +100,9 @@ function siteLines( ` ${tint(mark)} ${f.usage.file}:${f.usage.line} \`${f.change.path.replace(/^cause:/, '')}\``, ` ${f.reason}`, colors.dim( - ` evidence: ${plural(sites.length, 'error')} at the target${shown.length ? ', for example' : ''}`, + f.root + ? ` ${callSitesLine(sites)}${shown.length ? ', for example' : ''}` + : ` evidence: ${plural(sites.length, 'error')} at the target${shown.length ? ', for example' : ''}`, ), ...shown.map((site) => colors.dim(` ${site.file}:${site.line} TS${site.code}: ${site.message}`), @@ -107,7 +110,10 @@ function siteLines( ...(sites.length > shown.length ? [colors.dim(` … and ${sites.length - shown.length} more`)] : []), - ...(f.details ?? []).map((detail) => colors.dim(` ${detail}`)), + // The call-site count is printed above; the rest of the details follow. + ...(f.details ?? []) + .filter((detail) => !(f.root && /^\d+ call sites?\b/.test(detail))) + .map((detail) => colors.dim(` ${detail}`)), ]; } return [ diff --git a/packages/cli/src/html/render.test.ts b/packages/cli/src/html/render.test.ts index 6fb4c5f..a9ff2c3 100644 --- a/packages/cli/src/html/render.test.ts +++ b/packages/cli/src/html/render.test.ts @@ -183,3 +183,65 @@ it('omits workspace paths from check rows by default and counts breaking fixes c expect(html).toContain('25Auto-fixable'); expect(html).toContain('7Need the agent (LLM)'); }); + +it('lists the call sites under a declaration anchor, with their workspaces', () => { + const report = fixture('stripe'); + const p = report.packages.find( + (pkg) => pkg.findings.length > 0, + ) as (typeof report.packages)[number]; + const first = p.findings[0] as NonNullable<(typeof p.findings)[number]>; + const root = { + name: 'ref', + file: 'packages/editor/src/useTransform.ts', + line: 6, + reason: 'whose parameter changed', + }; + p.findings = [ + { + ...first, + change: { ...first.change, kind: 'cause', path: 'cause:ref' }, + usage: { ...first.usage, file: root.file, line: 6, compileError: undefined }, + anchorOnly: true, + root, + severity: 'breaking', + reason: root.reason, + downstream: [ + { + file: 'apps/examples/src/Custom.tsx', + line: 11, + code: 2345, + message: 'm', + workspace: 'apps/examples', + }, + { + file: 'packages/tldraw/src/Foreground.tsx', + line: 59, + code: 2345, + message: 'm', + workspace: 'packages/tldraw', + }, + ], + details: ['2 call sites in 2 workspaces'], + }, + ]; + p.plan = [ + { + rule: 'TS2345', + title: 'ref', + severity: 'breaking', + by: { rule: 0, agent: 1, manual: 0 }, + sites: 1, + fixes: 1, + locations: [{ file: root.file, line: 6 }], + detail: root.reason, + }, + ]; + const html = renderHtml(report, opts); + expect(html).toContain('2 call sites in 2 workspaces'); + expect(html).toContain( + '
  • apps/examples/src/Custom.tsx:11 apps/examples
    • ', + ); + expect(html).toContain( + '
  • packages/tldraw/src/Foreground.tsx:59 packages/tldraw
    • ', + ); +}); diff --git a/packages/cli/src/html/render.ts b/packages/cli/src/html/render.ts index ef59dc7..d540978 100644 --- a/packages/cli/src/html/render.ts +++ b/packages/cli/src/html/render.ts @@ -1,5 +1,12 @@ import { basename } from 'node:path'; -import { type CheckReport, isFailure, type PackageReport, type PlanGroup } from '@uptide/core'; +import { + type CheckReport, + callSitesLine, + type Finding, + isFailure, + type PackageReport, + type PlanGroup, +} from '@uptide/core'; import { byLine, checkRows, type FormatCheckOptions, nextCommands } from '../format-check.js'; import { groupReason, groupsForHtml, notesOf, stripeNote, verdict } from './content.js'; import { escapeHtml } from './escape.js'; @@ -47,6 +54,23 @@ function compilerAt(p: PackageReport, group: PlanGroup, file: string, line: numb ].join(' · ') || (group.severity !== 'deprecated' ? group.detail : '') ); } +/** The anchored finding at a site, when the site is the one edit several call sites trace to. */ +function anchorAt( + p: PackageReport, + group: PlanGroup, + file: string, + line: number, +): Finding | undefined { + return p.findings.find( + (f) => + f.anchorOnly && + f.root !== undefined && + f.severity === group.severity && + f.usage.file === file && + f.usage.line === line && + (f.downstream?.length ?? 0) > 0, + ); +} /** No repo data is interpolated into executable JS, CSS, attribute names or element names. */ export function renderHtml(report: CheckReport, opts: HtmlOptions): string { function render( @@ -108,7 +132,20 @@ export function renderHtml(report: CheckReport, opts: HtmlOptions): string { ? `
    compiler messageSame message, shown above
    ` : `
    compiler message
    ${escapeHtml(message)}
    `; } - const article = `
    ${link}
    ${excerpt}${compiler}
    `; + // One edit at the anchor resolves every call site listed under it. + const anchor = anchorAt(p, g, site.file, site.line); + const callSites = anchor?.downstream + ? `
    ${e(callSitesLine(anchor.downstream))}
      ${anchor.downstream + .slice(0, 50) + .map( + (d) => + `
    • ${e(`${d.file}:${d.line}`)}${d.workspace && d.workspace !== '.' ? ` ${e(d.workspace)}` : ''}
      • `, + ) + .join( + '', + )}${anchor.downstream.length > 50 ? `
    • and ${anchor.downstream.length - 50} more
      • ` : ''}
    ` + : ''; + const article = `
    ${link}
    ${excerpt}${compiler}${callSites}
    `; if (index < siteLimit) body += article; else overflow += article; } diff --git a/packages/core/src/adapters/typescript/cause.test.ts b/packages/core/src/adapters/typescript/cause.test.ts index 84e74e8..73394a9 100644 --- a/packages/core/src/adapters/typescript/cause.test.ts +++ b/packages/core/src/adapters/typescript/cause.test.ts @@ -126,5 +126,15 @@ describe('root cause guard', () => { const at5 = signal.diagnostics.find((d) => d.file === 'src/single.ts' && d.line === 5); expect(at5?.code).toBe(2345); expect(at5?.cause?.anchorOnly).toBeUndefined(); + // The declaration it traces to is kept: a site in another workspace at the same + // parameter folds with it after the merge (check/root-cause.ts). + expect(at5?.root).toEqual({ + name: 'b', + file: 'src/single.ts', + line: 2, + reason: + "whose parameter `b: Box` no longer accepts what the target gives it; widen the parameter's type there", + anchorOnly: true, + }); }); }); diff --git a/packages/core/src/adapters/typescript/compile.ts b/packages/core/src/adapters/typescript/compile.ts index 0680b3e..8f078d9 100644 --- a/packages/core/src/adapters/typescript/compile.ts +++ b/packages/core/src/adapters/typescript/compile.ts @@ -924,6 +924,10 @@ export async function compileAgainstTargets( : undefined) ?? findCause(programs, d, overlaid, repo.dir, erroredLines); if (cause) diagnostic.cause = cause; + // Traced to a parameter but alone in this workspace: the root is kept, so another + // workspace's site at the same declaration can fold with it after the merge. + const root = parameter !== undefined ? byParameter.get(parameter) : undefined; + if (root) diagnostic.root = root; return diagnostic; }); return { diff --git a/packages/core/src/check/check.test.ts b/packages/core/src/check/check.test.ts index 67b8dc4..2ccb6a2 100644 --- a/packages/core/src/check/check.test.ts +++ b/packages/core/src/check/check.test.ts @@ -438,6 +438,80 @@ describe('mergeAcrossWorkspaces', () => { }); }); + it('folds sites in two workspaces that trace to one declaration, prefixing the root by workspace', () => { + const root = (file: string) => ({ + name: 'ref', + file, + line: 6, + reason: 'whose parameter changed', + }); + const site = (file: string, line: number, rootFile: string): Finding => ({ + change: { + package: 'react', + from: '18.3.1', + to: '19.2.1', + path: 'TS2345', + kind: 'type', + severity: 'breaking', + source: 'types', + confidence: 1, + }, + usage: { + file, + line, + column: 1, + endLine: line, + endColumn: 2, + symbolPath: 'TS2345', + access: 'read', + via: 'inferred', + snippet: '', + compileCode: 2345, + compileError: 'mismatch', + }, + root: root(rootFile), + severity: 'breaking', + confidence: 1, + fixability: 'unknown', + reason: 'compile error', + evidence: 'compiler', + }); + const out = mergeAcrossWorkspaces( + [ + base('apps/examples', 'react', { + installed: '18.3.1', + findings: [site('src/Custom.tsx', 11, '../../packages/editor/src/useTransform.ts')], + }), + base('packages/tldraw', 'react', { + installed: '18.3.1', + findings: [site('src/Foreground.tsx', 59, '../editor/src/useTransform.ts')], + }), + ], + {}, + ); + expect(out).toHaveLength(1); + expect(out[0]?.findings).toHaveLength(1); + expect(out[0]?.findings[0]).toMatchObject({ + anchorOnly: true, + usage: { file: 'packages/editor/src/useTransform.ts', line: 6 }, + root: root('packages/editor/src/useTransform.ts'), + details: ['2 call sites in 2 workspaces'], + }); + expect(out[0]?.findings[0]?.downstream).toEqual([ + expect.objectContaining({ + file: 'apps/examples/src/Custom.tsx', + line: 11, + workspace: 'apps/examples', + }), + expect.objectContaining({ + file: 'packages/tldraw/src/Foreground.tsx', + line: 59, + workspace: 'packages/tldraw', + }), + ]); + expect(out[0]?.status).toBe('breaking'); + }); + it('merges coverage across workspaces, naming each compiler once', () => { const compile = (version: string, own: boolean) => ({ baselineErrors: 0, diff --git a/packages/core/src/check/check.ts b/packages/core/src/check/check.ts index 083f627..34543d3 100644 --- a/packages/core/src/check/check.ts +++ b/packages/core/src/check/check.ts @@ -52,7 +52,7 @@ import { } from './module-format.js'; import { mapWithLimit, mapWithSerialRetry } from './pool.js'; import { isBehind, rankCandidates } from './rank.js'; -import { groupRootCauses } from './root-cause.js'; +import { foldSharedRoots, groupRootCauses } from './root-cause.js'; import { runtimeChangeFindings } from './runtime-changes.js'; import { confirmBreaking, evidenceOf, tierOf } from './tier.js'; import { typesPackageOf, typesReleaseFor } from './types-release.js'; @@ -563,15 +563,16 @@ export function mergeAcrossWorkspaces( const first = list[0] as PackageReport; const sum = (pick: (p: PackageReport) => number): number => list.reduce((n, p) => n + pick(p), 0); - const combined: PackageReport = { - ...first, - workspace: '*', - workspaces: list.map((p) => p.workspace), - ...(catalog ? { source: 'catalog' as const } : {}), - findings: list.flatMap((p) => - p.findings.map((f) => ({ + const workspaceOf = new WeakMap(); + const findings = list.flatMap((p) => + p.findings.map((f) => { + const moved: Finding = { ...f, usage: { ...f.usage, file: prefixed(p, f.usage.file) }, + ...(f.root ? { root: { ...f.root, file: prefixed(p, f.root.file) } } : {}), + ...(f.downstream + ? { downstream: f.downstream.map((d) => ({ workspace: p.workspace, ...d })) } + : {}), // A cluster's description names its cause by file: the same prefix applies there. change: f.change.path.startsWith('cause:') ? { @@ -582,8 +583,19 @@ export function mergeAcrossWorkspaces( ), } : f.change, - })), - ), + }; + workspaceOf.set(moved, p.workspace); + return moved; + }), + ); + // Sites in several workspaces that trace to one declaration: one finding, at the declaration. + const folded = foldSharedRoots(findings, (f) => workspaceOf.get(f)); + const combined: PackageReport = { + ...first, + workspace: '*', + workspaces: list.map((p) => p.workspace), + ...(catalog ? { source: 'catalog' as const } : {}), + findings: folded, callSitesChecked: sum((p) => p.callSitesChecked), unanalyzed: list.flatMap((p) => p.unanalyzed.map((u) => ({ ...u, file: prefixed(p, u.file) })), @@ -593,7 +605,7 @@ export function mergeAcrossWorkspaces( ? { skipReason: list.find((p) => p.skipReason)?.skipReason } : {}), status: statusOf( - list.flatMap((p) => p.findings), + folded, sum((p) => p.callSitesChecked), sum((p) => p.unanalyzed.length), ), diff --git a/packages/core/src/check/root-cause.test.ts b/packages/core/src/check/root-cause.test.ts index 074bbdc..7e30edf 100644 --- a/packages/core/src/check/root-cause.test.ts +++ b/packages/core/src/check/root-cause.test.ts @@ -1,8 +1,8 @@ -import { expect, it } from 'vitest'; +import { describe, expect, it } from 'vitest'; import type { Finding, PackageReport } from '../domain/report.js'; import { planPackage } from '../fix/plan.js'; import { sitesOf } from './check.js'; -import { groupRootCauses } from './root-cause.js'; +import { callSitesLine, foldSharedRoots, groupRootCauses } from './root-cause.js'; it('groups missing TypeScript API members without losing sites or unrelated diagnostics', () => { const finding = (line: number, code = 2339): Finding => ({ @@ -64,3 +64,138 @@ it('groups missing TypeScript API members without losing sites or unrelated diag groupRootCauses(withoutEvidence); expect(withoutEvidence.findings[0]?.change.kind).toBe('removed'); }); + +describe('foldSharedRoots', () => { + const root = { + name: 'ref', + file: 'packages/editor/src/useTransform.ts', + line: 6, + reason: + "whose parameter `ref: RefObject` no longer accepts what the target gives it; widen the parameter's type there", + }; + const site = (file: string, line: number, over: Partial = {}): Finding => ({ + change: { + package: 'react', + from: '18.3.1', + to: '19.2.1', + path: 'TS2345', + kind: 'type', + severity: 'breaking', + source: 'types', + confidence: 1, + evidence: 'checker', + }, + usage: { + file, + line, + column: 16, + endLine: line, + endColumn: 20, + symbolPath: 'TS2345', + access: 'read', + via: 'inferred', + snippet: 'useTransform(rSvg, x, y)', + compileCode: 2345, + compileError: "Argument of type 'RefObject' is not assignable", + }, + root, + severity: 'breaking', + confidence: 1, + fixability: 'unknown', + reason: 'compile error not attributed to a known API change', + evidence: 'compiler', + rule: 'ref-object-nullable', + ...over, + }); + const workspaces = new Map(); + const at = (workspace: string, f: Finding): Finding => { + workspaces.set(f, workspace); + return f; + }; + + it('folds sites in different workspaces that trace to one declaration into one anchored finding', () => { + const a = at('apps/examples', site('apps/examples/src/Custom.tsx', 11)); + const b = at('packages/tldraw', site('packages/tldraw/src/Foreground.tsx', 59)); + const other = at('apps/examples', site('apps/examples/src/Other.tsx', 3, { root: undefined })); + const out = foldSharedRoots([a, other, b], (f) => workspaces.get(f)); + expect(out).toHaveLength(2); + const anchor = out[0] as Finding; + expect(anchor).toMatchObject({ + change: { kind: 'cause', path: 'cause:ref', severity: 'breaking' }, + usage: { file: root.file, line: 6, symbolPath: 'cause:ref' }, + anchorOnly: true, + root, + severity: 'breaking', + fixability: 'assisted', + reason: root.reason, + rule: 'ref-object-nullable', + evidence: 'compiler', + details: ['2 call sites in 2 workspaces'], + }); + expect(anchor.change.notes).toBe( + `2 errors caused by \`ref\` in ${root.file}:6: ${root.reason}. One edit there resolves them.`, + ); + expect(anchor.downstream).toEqual([ + { + file: 'apps/examples/src/Custom.tsx', + line: 11, + column: 16, + snippet: 'useTransform(rSvg, x, y)', + code: 2345, + message: "Argument of type 'RefObject' is not assignable", + workspace: 'apps/examples', + }, + expect.objectContaining({ + file: 'packages/tldraw/src/Foreground.tsx', + line: 59, + workspace: 'packages/tldraw', + }), + ]); + expect(sitesOf(anchor)).toBe(1); + expect(out[1]).toBe(other); + }); + + it('leaves a site alone at its root, and joins a site to the anchor another workspace already produced', () => { + const alone = site('apps/examples/src/Custom.tsx', 11); + expect(foldSharedRoots([alone], () => 'apps/examples')).toEqual([alone]); + const anchor: Finding = { + ...site(root.file, 6, { fixability: 'assisted', reason: root.reason }), + change: { ...site('', 0).change, kind: 'cause', path: 'cause:ref' }, + anchorOnly: true, + downstream: [ + { + file: 'packages/editor/src/A.tsx', + line: 4, + code: 2345, + message: 'm', + workspace: 'packages/editor', + }, + { + file: 'packages/editor/src/B.tsx', + line: 9, + code: 2345, + message: 'm', + workspace: 'packages/editor', + }, + ], + }; + const out = foldSharedRoots([anchor, at('apps/examples', alone)], (f) => workspaces.get(f)); + expect(out).toHaveLength(1); + expect(out[0]?.downstream?.map((d) => `${d.file}:${d.line}`)).toEqual([ + 'apps/examples/src/Custom.tsx:11', + 'packages/editor/src/A.tsx:4', + 'packages/editor/src/B.tsx:9', + ]); + expect(out[0]?.details).toEqual(['3 call sites in 2 workspaces']); + }); + + it('counts workspaces only when the sites span more than one', () => { + expect(callSitesLine([{ file: 'a', line: 1, code: 1, message: '' }])).toBe('1 call site'); + expect( + callSitesLine([ + { file: 'a', line: 1, code: 1, message: '', workspace: 'x' }, + { file: 'b', line: 1, code: 1, message: '', workspace: 'x' }, + ]), + ).toBe('2 call sites'); + }); +}); diff --git a/packages/core/src/check/root-cause.ts b/packages/core/src/check/root-cause.ts index 5dadfee..db4a867 100644 --- a/packages/core/src/check/root-cause.ts +++ b/packages/core/src/check/root-cause.ts @@ -1,6 +1,102 @@ -import type { Finding, PackageReport } from '../domain/report.js'; +import type { DownstreamSite, Finding, PackageReport, SharedRoot } from '../domain/report.js'; +import { describeCluster } from './unattributed.js'; import { parseVersion } from './version.js'; +const rootKey = (r: SharedRoot): string => `${r.file}:${r.line}:${r.name}`; + +/** `3 call sites in 2 workspaces`: the line under a shared root's reason. */ +export function callSitesLine(sites: readonly DownstreamSite[]): string { + const workspaces = new Set(sites.map((s) => s.workspace).filter((w) => w !== undefined)); + const count = (n: number, noun: string): string => `${n} ${noun}${n === 1 ? '' : 's'}`; + return `${count(sites.length, 'call site')}${workspaces.size > 1 ? ` in ${count(workspaces.size, 'workspace')}` : ''}`; +} + +/** + * Sites in different workspaces that trace to one repository declaration are one finding, + * anchored at the declaration with the sites as evidence. Within a workspace the compile + * signal already folds two or more such sites (adapters/typescript/compile.ts); a site alone + * in its workspace keeps its `root`, and here, after the merge, roots are compared across + * workspaces: two or more sites at one root, or one site plus the anchor another workspace + * already produced, become (or join) the anchored finding. tldraw's `useTransform(ref)`, + * called from three workspaces, is fixed once, in the hook's parameter. + */ +export function foldSharedRoots( + findings: Finding[], + workspaceOf: (f: Finding) => string | undefined, +): Finding[] { + const groups = new Map(); + for (const f of findings) { + if (!f.root) continue; + const list = groups.get(rootKey(f.root)) ?? []; + list.push(f); + groups.set(rootKey(f.root), list); + } + const folded = new Map(); + for (const group of groups.values()) { + const anchors = group.filter((f) => f.anchorOnly && f.change.kind === 'cause'); + const sites = group.filter((f) => !anchors.includes(f)); + // One site, no anchor anywhere: nothing shared, the site stands on its own. + if (sites.length === 0 || anchors.length + sites.length < 2) continue; + const root = (group[0] as Finding).root as SharedRoot; + const evidence: DownstreamSite[] = [ + ...anchors.flatMap((a) => a.downstream ?? []), + ...sites.map((f) => ({ + file: f.usage.file, + line: f.usage.line, + column: f.usage.column, + snippet: f.usage.snippet, + code: f.usage.compileCode ?? Number(/^TS(\d+)$/.exec(f.change.path)?.[1] ?? 0), + message: f.usage.compileError ?? f.reason, + ...(workspaceOf(f) !== undefined ? { workspace: workspaceOf(f) } : {}), + })), + ].sort((a, b) => a.file.localeCompare(b.file) || a.line - b.line); + const first = (anchors[0] ?? sites[0]) as Finding; + // The pack rule the sites were filed under, when they agree on one. + const rules = [...new Set(group.map((f) => f.rule).filter((r) => r !== undefined))]; + const anchor: Finding = { + ...first, + change: { + ...first.change, + path: `cause:${root.name}`, + kind: 'cause', + severity: 'breaking', + evidence: 'checker', + notes: describeCluster(evidence.length, { ...root, anchorOnly: true }), + }, + usage: { + file: root.file, + line: root.line, + column: 1, + endLine: root.line, + endColumn: 1, + symbolPath: `cause:${root.name}`, + access: 'read', + snippet: '', + via: 'inferred', + }, + downstream: evidence, + anchorOnly: true, + root, + severity: 'breaking', + confidence: 1, + fixability: 'assisted', + reason: root.reason, + details: [ + callSitesLine(evidence), + ...(first.details ?? []).filter((d) => !/^\d+ call sites?/.test(d)), + ], + evidence: 'compiler', + ...(rules.length === 1 ? { rule: rules[0] } : {}), + }; + folded.set(first, anchor); + for (const f of group) if (f !== first) folded.set(f, undefined); + } + if (folded.size === 0) return findings; + return findings.flatMap((f) => + folded.has(f) ? (folded.get(f) ? [folded.get(f) as Finding] : []) : [f], + ); +} + /** Known package-level cause, gated on compiler evidence. Other diagnostics remain separate. */ export function groupRootCauses(report: PackageReport): void { if (report.name !== 'typescript' || parseVersion(report.target)?.major !== 7) return; diff --git a/packages/core/src/check/unattributed.ts b/packages/core/src/check/unattributed.ts index 3b660bc..4308f96 100644 --- a/packages/core/src/check/unattributed.ts +++ b/packages/core/src/check/unattributed.ts @@ -1,6 +1,6 @@ import { posix } from 'node:path'; import type { Change } from '../domain/change.js'; -import type { Finding } from '../domain/report.js'; +import type { Finding, SharedRoot } from '../domain/report.js'; import type { ApiSurface } from '../domain/surface.js'; import type { CompileDiagnostic, DiagnosticCause, Usage } from '../domain/usage.js'; @@ -93,6 +93,7 @@ export function unattributedFindings( // A cause that is itself the one edit (a compiler option, a parameter's type) is the // site; the errors under it are evidence. ...(cause.anchorOnly || cause.config ? { anchorOnly: true as const } : {}), + ...(cause.anchorOnly && !cause.config ? { root: sharedRoot(cause) } : {}), severity: 'breaking', confidence: 1, fixability: cause.anchorOnly || cause.config ? 'assisted' : 'unknown', @@ -102,6 +103,15 @@ export function unattributedFindings( return [...clustered, ...plainFindings(single, meta, surfaceB, unresolvedFiles)]; } +/** + * The declaration as the workspace names it (`../editor/x.ts`), like `usage.file`: the merge + * across workspaces prefixes both the same way, and `packages/tldraw/../editor/x.ts` + * normalizes to the repository path. + */ +function sharedRoot(cause: DiagnosticCause): SharedRoot { + return { name: cause.name, file: cause.file, line: cause.line, reason: cause.reason }; +} + function plainFindings( diagnostics: CompileDiagnostic[], meta: { package: string; from: string; to: string }, @@ -142,6 +152,7 @@ function plainFindings( return { change, usage, + ...(d.root ? { root: sharedRoot(d.root) } : {}), severity: inconclusive ? 'unverified' : 'breaking', confidence: 1, fixability: 'unknown', diff --git a/packages/core/src/domain/report.ts b/packages/core/src/domain/report.ts index 23de32f..0e8f389 100644 --- a/packages/core/src/domain/report.ts +++ b/packages/core/src/domain/report.ts @@ -14,12 +14,24 @@ export interface DownstreamSite { line: number; code: number; message: string; + /** The workspace the site is in, when the anchor gathers sites from several. */ + workspace?: string; } /** `verified`: a migration pack covers the upgrade. `generic`: analysis only, no pack. */ export type Tier = 'verified' | 'generic'; /** A change joined to one usage of the changed symbol, with severity resolved for that usage. */ +/** A local declaration several call sites trace back to: where one edit resolves them all. */ +export interface SharedRoot { + /** Declared name (`ref`). */ + name: string; + file: string; + line: number; + /** Why it is the edit, as a clause: "whose parameter `ref: RefObject` no longer accepts ...". */ + reason: string; +} + export interface Finding { change: Change; usage: Usage; @@ -30,6 +42,13 @@ export interface Finding { * one site to fix, and `downstream` is the evidence; it counts as one site, not as many. */ anchorOnly?: true; + /** + * The repository declaration (a parameter, a prop) a compile error at a call site traces + * to, repository-relative. On a call-site finding it says where the one edit is; sites in + * different workspaces that share it are folded into one anchored finding at the + * declaration (`anchorOnly`, the sites as `downstream`), which carries `root` too. + */ + root?: SharedRoot; /** * For a `module-format` finding: every `require()` site of the package in this file, the * finding's own first. Switching a file to `import()` is one unit of work however many diff --git a/packages/core/src/domain/usage.ts b/packages/core/src/domain/usage.ts index b7030f4..0c3f7b2 100644 --- a/packages/core/src/domain/usage.ts +++ b/packages/core/src/domain/usage.ts @@ -109,6 +109,12 @@ export interface CompileDiagnostic { snippet: string; /** The repo declaration this error descends from, when one could be traced (see adapters/typescript/cause.ts). */ cause?: DiagnosticCause; + /** + * The repository parameter or prop this mismatch traces to (`parameterCause`), set whether + * or not enough sites in this workspace share it for `cause` to be. Sites in other + * workspaces that trace to the same declaration fold into one finding at it (check/root-cause.ts). + */ + root?: DiagnosticCause; } export interface DiagnosticCause { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a09d1f9..81bd83a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -18,6 +18,7 @@ export { resolveDirection } from './check/direction.js'; export { deprecationReplacement, fixabilityOf } from './check/fixability.js'; export { match } from './check/match.js'; export { type MergedSignals, mergeSignals, symbolFromMessage } from './check/merge.js'; +export { callSitesLine } from './check/root-cause.js'; export { TIER_LEGEND } from './check/tier.js'; export { formatTruthTable, diff --git a/packages/core/src/packs/tooling/pack-test.test.ts b/packages/core/src/packs/tooling/pack-test.test.ts index 5598196..b40a0d8 100644 --- a/packages/core/src/packs/tooling/pack-test.test.ts +++ b/packages/core/src/packs/tooling/pack-test.test.ts @@ -1,10 +1,11 @@ import { readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { expect, it } from 'vitest'; +import type { CheckReport, Finding } from '../../domain/report.js'; import { aiPack } from '../ai/index.js'; import type { PackVerification } from '../contract.js'; import { registeredPacks } from '../index.js'; -import { testPack } from './pack-test.js'; +import { predictedSites, testPack } from './pack-test.js'; const root = fileURLToPath(new URL('../../../../../', import.meta.url)); const verification = JSON.parse( @@ -26,3 +27,94 @@ it('passes every registered pack: each companion it names carries its source', a expect(report.problems, entry.pack.name).toEqual([]); } }); + +it('scores an anchored finding at its anchor; the call sites listed as evidence are not sites', () => { + const finding = (file: string, line: number, over: Partial = {}): Finding => ({ + change: { + package: 'ai', + from: '6.0.0', + to: '7.0.0', + path: 'TS2345', + kind: 'type', + severity: 'breaking', + source: 'types', + confidence: 1, + }, + usage: { + file, + line, + column: 1, + endLine: line, + endColumn: 2, + symbolPath: 'TS2345', + access: 'read', + via: 'inferred', + snippet: '', + }, + severity: 'breaking', + confidence: 1, + fixability: 'unknown', + reason: 'compile error', + ...over, + }); + const anchor = finding('packages/lib/hook.ts', 6, { + change: { ...finding('', 0).change, kind: 'cause', path: 'cause:ref' }, + anchorOnly: true, + downstream: [ + { file: 'apps/a/src/x.tsx', line: 11, code: 2345, message: 'm' }, + { file: 'apps/b/src/y.tsx', line: 59, code: 2345, message: 'm' }, + ], + }); + // A pack finding that happens to land on an evidence line is not scored either. + const onEvidence = finding('apps/a/src/x.tsx', 11, { rule: 'tool-context' }); + const report: CheckReport = { + repo: '/repo', + workspaces: ['apps/a', 'apps/b', 'packages/lib'], + packages: [ + { + workspace: '*', + name: 'ai', + installed: '6.0.0', + latest: '7.0.0', + target: '7.0.0', + majorsBehind: 1, + findings: [anchor, onEvidence], + callSitesChecked: 3, + unanalyzed: [], + status: 'breaking', + notes: [], + timing: { fetchMs: 0, diffMs: 0, usagesMs: 0, compileMs: 0 }, + plan: [ + { + rule: 'tool-context', + title: 't', + severity: 'breaking', + by: { rule: 0, agent: 2, manual: 0 }, + sites: 2, + fixes: 2, + locations: [ + { file: 'apps/a/src/x.tsx', line: 11 }, + { file: 'packages/lib/hook.ts', line: 6 }, + ], + detail: '', + }, + ], + }, + ], + summary: { + packagesNeedingAttention: 1, + breaking: 1, + deprecated: 0, + unverified: 0, + unaffected: 0, + notImported: 0, + partiallyAnalyzed: 0, + autoFixable: 0, + skippedForTime: 0, + failed: 0, + }, + }; + expect(predictedSites(report, aiPack).map((s) => `${s.file}:${s.line} ${s.rule}`)).toEqual([ + 'packages/lib/hook.ts:6 tool-context', + ]); +}); diff --git a/packages/core/src/packs/tooling/pack-test.ts b/packages/core/src/packs/tooling/pack-test.ts index 345d87d..04331de 100644 --- a/packages/core/src/packs/tooling/pack-test.ts +++ b/packages/core/src/packs/tooling/pack-test.ts @@ -114,9 +114,32 @@ export function predictedSites(report: CheckReport, pack: Pack): ScoredSite[] { const names = p.members ? p.members.map((m) => m.name) : [p.name]; if (!names.includes(pack.name)) continue; const prefix = p.workspace === '.' || p.workspace === '*' ? '' : `${p.workspace}/`; + const absolute = (file: string): string => + file.startsWith(prefix) ? file : `${prefix}${file}`; + // A call site an anchored finding lists as evidence is neither a true nor a false + // positive: the one site is the anchor (the declaration, a compiler option), which the + // ground truth names, and the evidence under it is not scored even when another finding + // lands on the same line. + const evidence = new Set( + p.findings + .filter((f) => f.anchorOnly) + .flatMap((f) => + (f.downstream ?? []).map((d) => key({ file: absolute(d.file), line: d.line })), + ), + ); + const anchors = new Set( + p.findings + .filter((f) => f.anchorOnly) + .map((f) => key({ file: absolute(f.usage.file), line: f.usage.line })), + ); for (const group of p.plan ?? []) for (const location of group.locations) { - const file = location.file.startsWith(prefix) ? location.file : `${prefix}${location.file}`; + const file = absolute(location.file); + if ( + evidence.has(key({ file, line: location.line })) && + !anchors.has(key({ file, line: location.line })) + ) + continue; const site: ScoredSite = { file, line: location.line,