diff --git a/.changeset/check-shared-root-findings.md b/.changeset/check-shared-root-findings.md new file mode 100644 index 00000000..74bb8602 --- /dev/null +++ b/.changeset/check-shared-root-findings.md @@ -0,0 +1,13 @@ +--- +"uptide": minor +--- + +**One finding for one edit, across workspaces.** When call sites in different workspaces of a +repository fail because of the same local declaration (a hook's parameter typed +`RefObject` that `useRef` no longer satisfies), `check` now reports one breaking +finding at that declaration and lists the call sites under it as evidence, as it already did +when two sites in one workspace tripped it. The JSON report adds `callSites` to such an anchor +(how many call sites trip it, every workspace counted; `downstream` lists them) and `sharedCause` +to a lone site's finding before it is joined; nothing is renamed or removed. `check --details` +and the HTML report print `N call sites` under the anchor. `uptide pack test` scores the anchor +at the declaration; the call sites under it are neither true nor false positives. diff --git a/docs/architecture.md b/docs/architecture.md index c7b8689d..e9af8bfb 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -589,8 +589,14 @@ 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 a workspace is left to that site +there, carrying the parameter as a candidate (`sharedCause`). When the workspaces' reports are +merged (`mergeAcrossWorkspaces`, `check/shared-root.ts`), a lone site joins the anchor another +workspace already reports for the same declaration, and lone sites of two or more workspaces +make one anchor: one breaking finding at the declaration, `callSites` counting every workspace's +call sites, `downstream` listing them. A parameter only one site in the whole repository trips +stays that site's own finding. `pack test` scores the anchor at the declaration; the call sites +under it are evidence, neither true nor false positives. 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/packages/cli/src/format-check-details.test.ts b/packages/cli/src/format-check-details.test.ts index 823b3079..70199270 100644 --- a/packages/cli/src/format-check-details.test.ts +++ b/packages/cli/src/format-check-details.test.ts @@ -131,6 +131,48 @@ const report: CheckReport = { }; describe('formatCheckDetails', () => { + it('says how many call sites trip a parameter anchor, and shows them as evidence', () => { + const anchored = finding({ + fixability: 'assisted', + anchorOnly: true, + callSites: 2, + reason: 'whose parameter `ref` no longer accepts what the target gives it', + change: { package: 'react', path: 'cause:ref', kind: 'cause' }, + usage: { file: 'packages/editor/src/lib/hooks/useTransform.ts', line: 6 }, + downstream: [ + { + file: 'apps/examples/src/x.tsx', + line: 11, + code: 2345, + message: 'Argument not assignable.', + }, + { + file: 'packages/tldraw/src/y.tsx', + line: 59, + code: 2345, + message: 'Argument not assignable.', + }, + ], + }); + const out = formatCheck( + { + ...report, + packages: [ + { + ...(report.packages[0] as PackageReport), + name: 'react', + findings: [anchored], + }, + ], + }, + { color: false }, + ); + expect(out).toContain('packages/editor/src/lib/hooks/useTransform.ts:6 `ref`'); + expect(out).toContain('2 call sites trip it at the target, for example'); + expect(out).toContain('apps/examples/src/x.tsx:11 TS2345: Argument not assignable.'); + expect(out).toContain('packages/tldraw/src/y.tsx:59 TS2345: Argument not assignable.'); + }); + it('renders every site with its snippet and reason', () => { const out = formatCheck(report, { color: false }); expect(out).toBe( diff --git a/packages/cli/src/format-check-details.ts b/packages/cli/src/format-check-details.ts index e90e2d91..d1780fa0 100644 --- a/packages/cli/src/format-check-details.ts +++ b/packages/cli/src/format-check-details.ts @@ -99,7 +99,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.callSites !== undefined + ? ` ${plural(f.callSites, 'call site')} trip it at the target${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}`), diff --git a/packages/cli/src/html/render.test.ts b/packages/cli/src/html/render.test.ts index 6fb4c5f0..1fae708d 100644 --- a/packages/cli/src/html/render.test.ts +++ b/packages/cli/src/html/render.test.ts @@ -115,6 +115,54 @@ describe('HTML report', () => { expect(html.match(/<\/script>/g)).toHaveLength(1); expect(html).not.toContain(' { + const report = fixture('zod'); + const p = report.packages[0]; + assert(p); + const decl = { file: 'packages/editor/src/lib/hooks/useTransform.ts', line: 6 }; + p.findings.push({ + change: { + package: 'zod', + from: p.installed, + to: p.target, + path: 'cause:ref', + kind: 'cause', + severity: 'breaking', + source: 'types', + confidence: 1, + }, + usage: { + ...decl, + column: 1, + endLine: 6, + endColumn: 1, + symbolPath: 'cause:ref', + access: 'read', + snippet: '', + via: 'inferred', + }, + anchorOnly: true, + callSites: 2, + downstream: [ + { file: 'apps/examples/src/x.tsx', line: 11, code: 2345, message: 'x' }, + { file: 'packages/tldraw/src/y.tsx', line: 59, code: 2345, message: 'x' }, + ], + severity: 'breaking', + confidence: 1, + fixability: 'assisted', + reason: 'whose parameter `ref` no longer accepts what the target gives it', + }); + const g = p.plan?.[0]; + assert(g); + g.locations = [decl]; + const html = renderHtml(report, opts); + expect(html).toContain('

2 call sites

'); + expect(html).toContain('
  • apps/examples/src/x.tsx:11 TS2345
    • '); + expect(html).toContain( + '
  • packages/tldraw/src/y.tsx:59 TS2345
    • ', + ); + }); + it('caps excerpts at 50 per group but retains additional sites within the size budget', () => { const report = fixture('zod'); const g = report.packages[0]?.plan?.[0]; diff --git a/packages/cli/src/html/render.ts b/packages/cli/src/html/render.ts index ef59dc77..2cd6951c 100644 --- a/packages/cli/src/html/render.ts +++ b/packages/cli/src/html/render.ts @@ -47,6 +47,30 @@ function compilerAt(p: PackageReport, group: PlanGroup, file: string, line: numb ].join(' ยท ') || (group.severity !== 'deprecated' ? group.detail : '') ); } +/** + * Under a parameter anchor: `N call sites`, then where (the first few). The anchor is the one + * edit; the call sites, in whatever workspace, are its evidence. + */ +function callSitesAt( + p: PackageReport, + file: string, + line: number, +): ((e: (s: unknown) => string) => string) | undefined { + const anchor = p.findings.find( + (f) => + f.change.kind === 'cause' && + f.callSites !== undefined && + f.usage.file === file && + f.usage.line === line, + ); + if (!anchor?.callSites) return undefined; + const sites = anchor.downstream ?? []; + const shown = sites.slice(0, CALL_SITES_SHOWN); + return (e) => + `

    ${e(anchor.callSites)} ${anchor.callSites === 1 ? 'call site' : 'call sites'}

      ${shown.map((s) => `
    • ${e(`${s.file}:${s.line}`)} TS${e(s.code)}
      • `).join('')}${sites.length > shown.length ? `
    • and ${e(sites.length - shown.length)} more
      • ` : ''}
    `; +} +const CALL_SITES_SHOWN = 10; + /** 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,8 @@ export function renderHtml(report: CheckReport, opts: HtmlOptions): string { ? `
    compiler messageSame message, shown above
    ` : `
    compiler message
    ${escapeHtml(message)}
    `; } - const article = `
    ${link}
    ${excerpt}${compiler}
    `; + const callers = callSitesAt(p, site.file, site.line); + const article = `
    ${link}
    ${excerpt}${compiler}${callers ? callers(e) : ''}
    `; 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 84e74e86..30924120 100644 --- a/packages/core/src/adapters/typescript/cause.test.ts +++ b/packages/core/src/adapters/typescript/cause.test.ts @@ -99,6 +99,7 @@ describe('root cause guard', () => { (d) => d.file === 'src/param.ts' && d.code === 2345, ); expect(mismatches.map((d) => d.line)).toEqual([5, 6]); + for (const d of mismatches) expect(d.sharedCause).toBeUndefined(); for (const d of mismatches) expect(d.cause).toEqual({ name: 'b', @@ -126,5 +127,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(); + // Not a cause in this workspace, but offered to the others: another workspace's lone site + // that trips the same parameter makes one finding at it (check/shared-root.ts). + expect(at5?.sharedCause).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 0680b3e2..c272763f 100644 --- a/packages/core/src/adapters/typescript/compile.ts +++ b/packages/core/src/adapters/typescript/compile.ts @@ -924,6 +924,8 @@ export async function compileAgainstTargets( : undefined) ?? findCause(programs, d, overlaid, repo.dir, erroredLines); if (cause) diagnostic.cause = cause; + // A parameter only this site trips: left to the site here, offered to the other workspaces. + else if (parameter !== undefined) diagnostic.sharedCause = byParameter.get(parameter); return diagnostic; }); return { diff --git a/packages/core/src/check/check.test.ts b/packages/core/src/check/check.test.ts index 67b8dc4a..3929d88e 100644 --- a/packages/core/src/check/check.test.ts +++ b/packages/core/src/check/check.test.ts @@ -414,6 +414,76 @@ describe('mergeAcrossWorkspaces', () => { ...over, }); + it('makes one finding at a repository parameter that one call site trips in each of two workspaces', () => { + // tldraw: useTransform(rSvg) in apps/examples and in packages/tldraw, both rejected by the + // hook's parameter in packages/editor. Each workspace has one site; together they are one edit. + const lone = (file: string, line: number): 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: 15, + endLine: line, + endColumn: 19, + symbolPath: 'TS2345', + access: 'read', + snippet: 'useTransform(rSvg, x, y)', + via: 'inferred', + compileError: "Argument of type 'RefObject' is not assignable.", + }, + sharedCause: { + name: 'ref', + file: '../../packages/editor/src/lib/hooks/useTransform.ts', + line: 6, + reason: + 'whose parameter `ref: React.RefObject` no longer accepts what the target gives it', + anchorOnly: true, + }, + severity: 'breaking', + confidence: 1, + fixability: 'unknown', + reason: 'compile error not attributed to a known API change', + }); + const [merged] = mergeAcrossWorkspaces( + [ + base('apps/examples', 'react', { + status: 'breaking', + findings: [lone('src/examples/CustomComponentsExample.tsx', 11)], + }), + base('packages/tldraw', 'react', { + status: 'breaking', + findings: [lone('src/lib/canvas/TldrawSelectionForeground.tsx', 59)], + }), + ], + {}, + ); + expect(merged?.findings).toHaveLength(1); + const anchor = merged?.findings[0] as Finding; + expect(anchor).toMatchObject({ + change: { kind: 'cause', path: 'cause:ref' }, + usage: { file: 'packages/editor/src/lib/hooks/useTransform.ts', line: 6 }, + anchorOnly: true, + callSites: 2, + severity: 'breaking', + }); + expect(anchor.downstream?.map((d) => `${d.file}:${d.line}`)).toEqual([ + 'apps/examples/src/examples/CustomComponentsExample.tsx:11', + 'packages/tldraw/src/lib/canvas/TldrawSelectionForeground.tsx:59', + ]); + expect(anchor.sharedCause).toBeUndefined(); + expect(sitesOf(anchor)).toBe(1); + }); + it('merges the same dependency at the same versions across workspaces, and marks catalog entries', () => { const out = mergeAcrossWorkspaces( [ diff --git a/packages/core/src/check/check.ts b/packages/core/src/check/check.ts index 083f627f..afd1709d 100644 --- a/packages/core/src/check/check.ts +++ b/packages/core/src/check/check.ts @@ -54,6 +54,7 @@ import { mapWithLimit, mapWithSerialRetry } from './pool.js'; import { isBehind, rankCandidates } from './rank.js'; import { groupRootCauses } from './root-cause.js'; import { runtimeChangeFindings } from './runtime-changes.js'; +import { joinSharedRoots } from './shared-root.js'; import { confirmBreaking, evidenceOf, tierOf } from './tier.js'; import { typesPackageOf, typesReleaseFor } from './types-release.js'; import { unattributedFindings } from './unattributed.js'; @@ -572,6 +573,9 @@ export function mergeAcrossWorkspaces( p.findings.map((f) => ({ ...f, usage: { ...f.usage, file: prefixed(p, f.usage.file) }, + ...(f.sharedCause + ? { sharedCause: { ...f.sharedCause, file: prefixed(p, f.sharedCause.file) } } + : {}), // A cluster's description names its cause by file: the same prefix applies there. change: f.change.path.startsWith('cause:') ? { @@ -605,6 +609,8 @@ export function mergeAcrossWorkspaces( runtimeMs: sum((p) => p.timing.runtimeMs ?? 0), }, }; + // Sites that trip one repository declaration from different workspaces are one edit there. + combined.findings = joinSharedRoots(combined.findings); if (first.runtime) combined.runtime = first.runtime; // What moves with the package is one plan for the repository, carried by whichever // workspace reports it; the merged entry keeps every companion and conflict named. diff --git a/packages/core/src/check/shared-root.test.ts b/packages/core/src/check/shared-root.test.ts new file mode 100644 index 00000000..d7ff9280 --- /dev/null +++ b/packages/core/src/check/shared-root.test.ts @@ -0,0 +1,124 @@ +import { describe, expect, it } from 'vitest'; +import type { Finding } from '../domain/report.js'; +import type { DiagnosticCause } from '../domain/usage.js'; +import { joinSharedRoots } from './shared-root.js'; + +const cause: DiagnosticCause = { + name: 'ref', + file: 'packages/editor/src/lib/hooks/useTransform.ts', + line: 6, + reason: + 'whose parameter `ref: RefObject` no longer accepts what the target gives it', + anchorOnly: true, +}; +const meta = { package: 'react', from: '18.3.1', to: '19.2.1' }; + +const lone = (file: string, line: number, over: Partial = {}): Finding => ({ + change: { + ...meta, + path: 'TS2345', + kind: 'type', + severity: 'breaking', + source: 'types', + confidence: 1, + }, + usage: { + file, + line, + column: 1, + endLine: line, + endColumn: 1, + symbolPath: 'TS2345', + access: 'read', + snippet: '', + via: 'inferred', + compileError: 'Argument of type X is not assignable to parameter of type Y.', + }, + sharedCause: cause, + severity: 'breaking', + confidence: 1, + fixability: 'unknown', + reason: 'compile error not attributed to a known API change', + ...over, +}); + +/** The anchor one workspace already reports (two of its own sites trip the parameter). */ +const anchor: Finding = { + change: { + ...meta, + path: 'cause:ref', + kind: 'cause', + severity: 'breaking', + source: 'types', + confidence: 1, + }, + usage: { + file: cause.file, + line: cause.line, + column: 1, + endLine: cause.line, + endColumn: 1, + symbolPath: 'cause:ref', + access: 'read', + snippet: '', + via: 'inferred', + }, + downstream: [ + { file: 'packages/editor/src/lib/a.tsx', line: 3, code: 2345, message: 'x' }, + { file: 'packages/editor/src/lib/b.tsx', line: 4, code: 2345, message: 'x' }, + ], + anchorOnly: true, + callSites: 2, + severity: 'breaking', + confidence: 1, + fixability: 'assisted', + reason: cause.reason, +}; + +describe('joinSharedRoots', () => { + it('adds a lone site of another workspace to the anchor already reported for the declaration', () => { + const out = joinSharedRoots([anchor, lone('apps/examples/src/x.tsx', 11)]); + expect(out).toHaveLength(1); + expect(out[0]?.callSites).toBe(3); + expect(out[0]?.downstream?.map((d) => `${d.file}:${d.line}`)).toEqual([ + 'packages/editor/src/lib/a.tsx:3', + 'packages/editor/src/lib/b.tsx:4', + 'apps/examples/src/x.tsx:11', + ]); + expect(out[0]?.change.notes).toContain('3 errors caused by `ref`'); + }); + + it('makes one anchor of two lone sites in different workspaces, under the rule that claimed them', () => { + const out = joinSharedRoots([ + lone('apps/examples/src/x.tsx', 11, { rule: 'ref-object-nullable' }), + lone('packages/tldraw/src/y.tsx', 59, { rule: 'ref-object-nullable' }), + ]); + expect(out).toHaveLength(1); + expect(out[0]).toMatchObject({ + usage: { file: cause.file, line: 6 }, + anchorOnly: true, + callSites: 2, + rule: 'ref-object-nullable', + }); + }); + + it('leaves a site no other shares as its own finding, without the candidate', () => { + const other = lone('apps/examples/src/x.tsx', 11); + const out = joinSharedRoots([other]); + expect(out).toHaveLength(1); + expect(out[0]?.usage.file).toBe('apps/examples/src/x.tsx'); + expect(out[0]?.sharedCause).toBeUndefined(); + }); + + it('keeps sites of different declarations apart', () => { + const elsewhere = { ...cause, file: 'packages/editor/src/lib/hooks/other.ts' }; + const out = joinSharedRoots([ + lone('apps/examples/src/x.tsx', 11), + lone('packages/tldraw/src/y.tsx', 59, { sharedCause: elsewhere }), + ]); + expect(out.map((f) => `${f.usage.file}:${f.usage.line}`)).toEqual([ + 'apps/examples/src/x.tsx:11', + 'packages/tldraw/src/y.tsx:59', + ]); + }); +}); diff --git a/packages/core/src/check/shared-root.ts b/packages/core/src/check/shared-root.ts new file mode 100644 index 00000000..ed657cf9 --- /dev/null +++ b/packages/core/src/check/shared-root.ts @@ -0,0 +1,103 @@ +import type { Finding } from '../domain/report.js'; +import type { CompileDiagnostic, DiagnosticCause } from '../domain/usage.js'; +import { clusterFinding, describeCluster } from './unattributed.js'; + +const keyOf = (c: Pick): string => + `${c.file}:${c.line}:${c.name}`; + +/** The cause an anchor finding stands for, read back from the finding. */ +function anchorKey(f: Finding): string | undefined { + if (f.change.kind !== 'cause' || !f.anchorOnly || !f.change.path.startsWith('cause:')) + return undefined; + return keyOf({ file: f.usage.file, line: f.usage.line, name: f.change.path.slice(6) }); +} + +/** A lone site as the compile error it was, repository-relative. */ +function asDiagnostic(f: Finding, cause: DiagnosticCause): CompileDiagnostic { + return { + file: f.usage.file, + line: f.usage.line, + column: f.usage.column, + endLine: f.usage.endLine, + endColumn: f.usage.endColumn, + code: Number(/^TS(\d+)$/.exec(f.change.path)?.[1] ?? 0), + message: f.usage.compileError ?? '', + snippet: f.usage.snippet, + cause, + }; +} + +/** + * Several workspaces, one edit. A repository parameter that one call site trips in each of + * several workspaces (`useTransform(ref)` in `apps/examples` and in `packages/tldraw`, both + * rejected by `packages/editor/src/lib/hooks/useTransform.ts:6`) is one breaking finding at the + * declaration, with the call sites under it as evidence, as one workspace with two such sites + * already is. A lone site joins the anchor another workspace already reports for the same + * declaration; two lone sites in different workspaces make one. A site no other shares stays + * its own finding. Files are repository-relative (`mergeAcrossWorkspaces` prefixes them first). + */ +export function joinSharedRoots(findings: Finding[]): Finding[] { + const candidates = new Map(); + for (const f of findings) { + if (!f.sharedCause) continue; + const key = keyOf(f.sharedCause); + candidates.set(key, [...(candidates.get(key) ?? []), f]); + } + const joined = new Set(); + const replaced = new Map(); + const created = new Map(); + for (const [key, lone] of candidates) { + const anchor = findings.find((f) => anchorKey(f) === key); + if (anchor) { + const cause = lone[0]?.sharedCause as DiagnosticCause; + const downstream = [ + ...(anchor.downstream ?? []), + ...lone.map((f) => { + const d = asDiagnostic(f, cause); + return { + file: d.file, + line: d.line, + code: d.code, + message: d.message.split('\n')[0] ?? '', + }; + }), + ]; + replaced.set(anchor, { + ...(replaced.get(anchor) ?? anchor), + downstream, + callSites: downstream.length, + change: { + ...anchor.change, + notes: describeCluster(downstream.length, { + ...cause, + file: anchor.usage.file, + line: anchor.usage.line, + }), + }, + }); + for (const f of lone) joined.add(f); + continue; + } + if (lone.length < 2) continue; + const first = lone[0] as Finding; + const cause = first.sharedCause as DiagnosticCause; + const meta = { package: first.change.package, from: first.change.from, to: first.change.to }; + const group = lone.map((f) => asDiagnostic(f, cause)); + const cluster = clusterFinding(group, meta); + // The rule that claimed the lone sites claims the anchor too. + created.set(first, first.rule ? { ...cluster, rule: first.rule } : cluster); + for (const f of lone) joined.add(f); + } + const out: Finding[] = []; + for (const f of findings) { + const made = created.get(f); + if (made) out.push(made); + if (joined.has(f)) continue; + const kept = replaced.get(f) ?? f; + if (kept.sharedCause) { + const { sharedCause: _unshared, ...rest } = kept; + out.push(rest); + } else out.push(kept); + } + return out; +} diff --git a/packages/core/src/check/unattributed.ts b/packages/core/src/check/unattributed.ts index 3b660bcb..c1d8ff5d 100644 --- a/packages/core/src/check/unattributed.ts +++ b/packages/core/src/check/unattributed.ts @@ -55,53 +55,65 @@ export function unattributedFindings( list.push(d); clusters.set(key, list); } - const clustered: Finding[] = [...clusters.values()].map((group) => { - const cause = (group[0] as CompileDiagnostic).cause as DiagnosticCause; - const usage: Usage = { - file: cause.file, - line: cause.line, - column: 1, - endLine: cause.line, - endColumn: 1, - symbolPath: `cause:${cause.name}`, - access: 'read', - snippet: '', - via: 'inferred', - }; - const change: Change = { - ...meta, - path: `cause:${cause.name}`, - kind: 'cause', - severity: 'breaking', - source: 'types', - confidence: 1, - evidence: 'checker', - notes: describeCluster(group.length, { - ...cause, - file: posix.normalize(`${filePrefix}${cause.file}`), - }), - }; - return { - change, - usage, - downstream: group.map((d) => ({ - file: posix.normalize(`${filePrefix}${d.file}`), - line: d.line, - code: d.code, - message: d.message.split('\n')[0] ?? d.message, - })), - // 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 } : {}), - severity: 'breaking', - confidence: 1, - fixability: cause.anchorOnly || cause.config ? 'assisted' : 'unknown', - reason: cause.anchorOnly || cause.config ? cause.reason : UNATTRIBUTED_REASON, - }; - }); + const clustered: Finding[] = [...clusters.values()].map((group) => + clusterFinding(group, meta, filePrefix), + ); return [...clustered, ...plainFindings(single, meta, surfaceB, unresolvedFiles)]; } +/** One finding at a traced cause, with the errors it explains under it. */ +export function clusterFinding( + group: CompileDiagnostic[], + meta: { package: string; from: string; to: string }, + /** Prefix that makes a workspace-relative file repository-relative (`packages/api/`). */ + filePrefix = '', +): Finding { + const cause = (group[0] as CompileDiagnostic).cause as DiagnosticCause; + const usage: Usage = { + file: cause.file, + line: cause.line, + column: 1, + endLine: cause.line, + endColumn: 1, + symbolPath: `cause:${cause.name}`, + access: 'read', + snippet: '', + via: 'inferred', + }; + const change: Change = { + ...meta, + path: `cause:${cause.name}`, + kind: 'cause', + severity: 'breaking', + source: 'types', + confidence: 1, + evidence: 'checker', + notes: describeCluster(group.length, { + ...cause, + file: posix.normalize(`${filePrefix}${cause.file}`), + }), + }; + return { + change, + usage, + downstream: group.map((d) => ({ + file: posix.normalize(`${filePrefix}${d.file}`), + line: d.line, + code: d.code, + message: d.message.split('\n')[0] ?? d.message, + })), + // 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 } : {}), + // A parameter's errors are its call sites: the report says how many. + ...(cause.anchorOnly && !cause.config ? { callSites: group.length } : {}), + severity: 'breaking', + confidence: 1, + fixability: cause.anchorOnly || cause.config ? 'assisted' : 'unknown', + reason: cause.anchorOnly || cause.config ? cause.reason : UNATTRIBUTED_REASON, + }; +} + function plainFindings( diagnostics: CompileDiagnostic[], meta: { package: string; from: string; to: string }, @@ -142,6 +154,7 @@ function plainFindings( return { change, usage, + ...(d.sharedCause ? { sharedCause: d.sharedCause } : {}), 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 23de32f5..1e131dcc 100644 --- a/packages/core/src/domain/report.ts +++ b/packages/core/src/domain/report.ts @@ -1,7 +1,13 @@ import type { ErrorCode } from '../errors.js'; import type { Change, Severity } from './change.js'; import type { RuntimeChange, RuntimeLoad } from './runtime.js'; -import type { CompileCoverage, CompileDiagnostic, Unanalyzed, Usage } from './usage.js'; +import type { + CompileCoverage, + CompileDiagnostic, + DiagnosticCause, + Unanalyzed, + Usage, +} from './usage.js'; /** `none`: an additive finding, nothing to change; it counts in callSitesChecked only. */ export type Fixability = 'mechanical' | 'assisted' | 'manual' | 'unknown' | 'none'; @@ -30,6 +36,18 @@ export interface Finding { * one site to fix, and `downstream` is the evidence; it counts as one site, not as many. */ anchorOnly?: true; + /** + * The repository parameter this finding's compile error descends from, when it stands alone in + * its workspace: merged across workspaces, sites that trip the same declaration become one + * `cause` anchor at it. Dropped once joined or when no other site shares it. + */ + sharedCause?: DiagnosticCause; + /** + * For a `cause` anchor at a repository parameter: how many call sites trip it, across every + * workspace (`downstream` lists them). Absent for a compiler-option anchor, whose errors are + * not call sites. + */ + callSites?: number; /** * 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 b7030f48..1de45937 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 this error descends from when only this site in its workspace trips + * it: not a cause yet (one site keeps its own diagnostic), but a candidate to join the sites of + * the other workspaces that trip the same declaration (`check/shared-root.ts`). + */ + sharedCause?: DiagnosticCause; } export interface DiagnosticCause { diff --git a/packages/core/src/packs/tooling/pack-test.test.ts b/packages/core/src/packs/tooling/pack-test.test.ts index 55981964..69f62f3d 100644 --- a/packages/core/src/packs/tooling/pack-test.test.ts +++ b/packages/core/src/packs/tooling/pack-test.test.ts @@ -1,10 +1,12 @@ import { readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { expect, it } from 'vitest'; +import type { CheckReport } from '../../domain/report.js'; import { aiPack } from '../ai/index.js'; import type { PackVerification } from '../contract.js'; +import { definePack } from '../contract.js'; import { registeredPacks } from '../index.js'; -import { testPack } from './pack-test.js'; +import { predictedSites, scoreSites, testPack } from './pack-test.js'; const root = fileURLToPath(new URL('../../../../../', import.meta.url)); const verification = JSON.parse( @@ -26,3 +28,49 @@ 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 declaration; the call sites under it are neither true nor false positives', () => { + const pack = definePack({ + meta: { + package: 'react', + from: '>=18 <19', + to: '>=19 <20', + sources: [{ title: 'guide', url: 'https://example.com' }], + maintainer: 'uptide-dev', + }, + rules: [], + instructions: '', + }); + const decl = { file: 'packages/editor/src/lib/hooks/useTransform.ts', line: 6 }; + const call = { file: 'apps/examples/src/x.tsx', line: 11 }; + const report = { + packages: [ + { + workspace: '*', + name: 'react', + findings: [ + { + change: { path: 'cause:ref', kind: 'cause' }, + usage: decl, + anchorOnly: true, + callSites: 2, + downstream: [ + { ...call, code: 2345, message: 'x' }, + { file: 'packages/tldraw/src/y.tsx', line: 59, code: 2345, message: 'x' }, + ], + }, + ], + // A call site that is also planned on its own (say, by a rule's detect) is still evidence. + plan: [ + { rule: 'cause:ref', severity: 'breaking', locations: [decl] }, + { rule: 'TS2345', severity: 'breaking', locations: [call] }, + ], + }, + ], + } as unknown as CheckReport; + const predicted = predictedSites(report, pack); + expect(predicted.map((s) => `${s.file}:${s.line}`)).toEqual([`${decl.file}:${decl.line}`]); + const score = scoreSites(predicted, [{ ...decl, rule: 'generic' }]); + expect(score.falsePositives).toEqual([]); + expect(score.falseNegatives).toEqual([]); +}); diff --git a/packages/core/src/packs/tooling/pack-test.ts b/packages/core/src/packs/tooling/pack-test.ts index 345d87d4..36646bc0 100644 --- a/packages/core/src/packs/tooling/pack-test.ts +++ b/packages/core/src/packs/tooling/pack-test.ts @@ -103,12 +103,36 @@ function tally(tp: number, fp: number, fn: number): Tally { const key = (s: { file: string; line: number }): string => `${s.file}:${s.line}`; +/** + * The call sites listed under a one-edit anchor (a repository parameter, a compiler option): + * evidence of the anchor, never sites of their own. Repository-relative. + */ +export function evidenceSites(report: CheckReport, pack: Pack): Set { + const evidence = new Set(); + for (const p of report.packages) { + 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}/`; + for (const f of p.findings) { + if (f.change.kind !== 'cause' || !f.anchorOnly) continue; + for (const d of f.downstream ?? []) + evidence.add( + key({ file: d.file.startsWith(prefix) ? d.file : `${prefix}${d.file}`, line: d.line }), + ); + } + } + return evidence; +} + /** * The sites `check` shows for the package, from its plan: the findings worth acting on, * grouped under the rule `fix` would apply. Repository-relative, as ground truth is written. + * An anchored finding is scored at its anchor (the declaration a ground-truth finding names); + * the call sites under it are evidence, neither true nor false positives. */ export function predictedSites(report: CheckReport, pack: Pack): ScoredSite[] { const own = new Set([...pack.rules.map((r) => r.id), ...pack.behavior.map((b) => b.id)]); + const evidence = evidenceSites(report, pack); const sites = new Map(); for (const p of report.packages) { const names = p.members ? p.members.map((m) => m.name) : [p.name]; @@ -117,6 +141,7 @@ export function predictedSites(report: CheckReport, pack: Pack): ScoredSite[] { for (const group of p.plan ?? []) for (const location of group.locations) { const file = location.file.startsWith(prefix) ? location.file : `${prefix}${location.file}`; + if (evidence.has(key({ file, line: location.line }))) continue; const site: ScoredSite = { file, line: location.line,