Skip to content
Merged
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-roots.md
Original file line number Diff line number Diff line change
@@ -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<HTMLElement>` 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.
15 changes: 13 additions & 2 deletions docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -589,8 +589,19 @@ 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 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
Expand Down
8 changes: 7 additions & 1 deletion docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<HTMLElement>` once `useRef` returns
`RefObject<HTMLElement | null>`) 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

Expand Down
59 changes: 59 additions & 0 deletions packages/cli/src/format-check-details.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<HTMLElement>` 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);
});
10 changes: 8 additions & 2 deletions packages/cli/src/format-check-details.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
type CheckReport,
callSitesLine,
type Finding,
isNativeProbeSkip,
type PackageReport,
Expand Down Expand Up @@ -99,15 +100,20 @@ 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}`),
),
...(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 [
Expand Down
62 changes: 62 additions & 0 deletions packages/cli/src/html/render.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,3 +183,65 @@ it('omits workspace paths from check rows by default and counts breaking fixes c
expect(html).toContain('<strong>25</strong><span class="label">Auto-fixable</span>');
expect(html).toContain('<strong>7</strong><span class="label">Need the agent (LLM)</span>');
});

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('<summary>2 call sites in 2 workspaces</summary>');
expect(html).toContain(
'<li>apps/examples/src/Custom.tsx:11 <span class="muted">apps/examples</span></li>',
);
expect(html).toContain(
'<li>packages/tldraw/src/Foreground.tsx:59 <span class="muted">packages/tldraw</span></li>',
);
});
41 changes: 39 additions & 2 deletions packages/cli/src/html/render.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -108,7 +132,20 @@ 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>`;
// 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
? `<details class="compiler"><summary>${e(callSitesLine(anchor.downstream))}</summary><ul class="mono">${anchor.downstream
.slice(0, 50)
.map(
(d) =>
`<li>${e(`${d.file}:${d.line}`)}${d.workspace && d.workspace !== '.' ? ` <span class="muted">${e(d.workspace)}</span>` : ''}</li>`,
)
.join(
'',
)}${anchor.downstream.length > 50 ? `<li class="muted">and ${anchor.downstream.length - 50} more</li>` : ''}</ul></details>`
: '';
const article = `<article class="site"><div class="mono">${link}</div>${excerpt}${compiler}${callSites}</article>`;
if (index < siteLimit) body += article;
else overflow += article;
}
Expand Down
10 changes: 10 additions & 0 deletions packages/core/src/adapters/typescript/cause.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<number>` no longer accepts what the target gives it; widen the parameter's type there",
anchorOnly: true,
});
});
});
4 changes: 4 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,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 {
Expand Down
74 changes: 74 additions & 0 deletions packages/core/src/check/check.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading
Loading