Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/check-shared-root-findings.md
Original file line number Diff line number Diff line change
@@ -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<HTMLElement>` 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.
10 changes: 8 additions & 2 deletions docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -589,8 +589,14 @@ the target's `useRef` returns `RefObject<HTMLElement | null>`) 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
Expand Down
42 changes: 42 additions & 0 deletions packages/cli/src/format-check-details.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/format-check-details.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`),
Expand Down
48 changes: 48 additions & 0 deletions packages/cli/src/html/render.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,54 @@ describe('HTML report', () => {
expect(html.match(/<\/script>/g)).toHaveLength(1);
expect(html).not.toContain('<img');
});
it('lists the call sites under a parameter anchor, with an N call sites line', () => {
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('<p class="muted call-sites">2 call sites</p>');
expect(html).toContain('<li>apps/examples/src/x.tsx:11 <span class="muted">TS2345</span></li>');
expect(html).toContain(
'<li>packages/tldraw/src/y.tsx:59 <span class="muted">TS2345</span></li>',
);
});

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];
Expand Down
27 changes: 26 additions & 1 deletion packages/cli/src/html/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
`<p class="muted call-sites">${e(anchor.callSites)} ${anchor.callSites === 1 ? 'call site' : 'call sites'}</p><ul class="mono call-sites">${shown.map((s) => `<li>${e(`${s.file}:${s.line}`)} <span class="muted">TS${e(s.code)}</span></li>`).join('')}${sites.length > shown.length ? `<li class="muted">and ${e(sites.length - shown.length)} more</li>` : ''}</ul>`;
}
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(
Expand Down Expand Up @@ -108,7 +132,8 @@ export function renderHtml(report: CheckReport, opts: HtmlOptions): string {
? `<details class="compiler"><summary>compiler message</summary><a href="#${id}">Same message, shown above</a></details>`
: `<details class="compiler" id="${id}"><summary>compiler message</summary><pre>${escapeHtml(message)}</pre></details>`;
}
const article = `<article class="site"><div class="mono">${link}</div>${excerpt}${compiler}</article>`;
const callers = callSitesAt(p, site.file, site.line);
const article = `<article class="site"><div class="mono">${link}</div>${excerpt}${compiler}${callers ? callers(e) : ''}</article>`;
if (index < siteLimit) body += article;
else overflow += article;
}
Expand Down
11 changes: 11 additions & 0 deletions packages/core/src/adapters/typescript/cause.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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<number>` no longer accepts what the target gives it; widen the parameter's type there",
anchorOnly: true,
});
});
});
2 changes: 2 additions & 0 deletions packages/core/src/adapters/typescript/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
70 changes: 70 additions & 0 deletions packages/core/src/check/check.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<SVGSVGElement | null>' is not assignable.",
},
sharedCause: {
name: 'ref',
file: '../../packages/editor/src/lib/hooks/useTransform.ts',
line: 6,
reason:
'whose parameter `ref: React.RefObject<HTMLElement | SVGElement>` 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(
[
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/check/check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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:')
? {
Expand Down Expand Up @@ -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.
Expand Down
Loading
Loading