Skip to content

Commit ef5f28a

Browse files
committed
refactor(lint): re-point validate-chart-bindings onto recordsOf (#15636 seam)
1 parent b970332 commit ef5f28a

2 files changed

Lines changed: 61 additions & 19 deletions

File tree

packages/lint/src/validate-chart-bindings.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,49 @@ describe('validateChartBindings — floor', () => {
609609
expect(findings).toEqual([]);
610610
});
611611

612+
// The #15636 seam this file carries: its collection reader was a hand-copied
613+
// `asArray` whose array branch was an unchecked cast, so a junk member was
614+
// DEREFERENCED (`strName(entry.name)` on `null`) rather than skipped and the
615+
// rule threw where it should have reported. Now `recordsOf`, which filters.
616+
it('survives a null member in every collection it reads — skipped, not dereferenced', () => {
617+
const stack = {
618+
datasets: [
619+
null,
620+
{
621+
name: 'task_metrics',
622+
object: 'showcase_task',
623+
dimensions: [null, { name: 'status', field: 'status' }],
624+
measures: [null, { name: 'task_count', aggregate: 'count' }],
625+
},
626+
],
627+
reports: [
628+
null,
629+
{
630+
name: 'r',
631+
dataset: 'task_metrics',
632+
values: ['task_count'],
633+
chart: {
634+
type: 'bar',
635+
xAxis: 'status',
636+
yAxis: 'task_count',
637+
series: [null, { name: 'ghost_measure' }],
638+
},
639+
},
640+
],
641+
} as unknown as Record<string, unknown>;
642+
643+
const findings = validateChartBindings(stack);
644+
// The junk members are gone rather than fatal, and the real declarations
645+
// around them still resolve: `status`/`task_count` are found (no unknown-
646+
// ref finding for either), and the one genuine defect is still reported.
647+
expect(findings).toHaveLength(1);
648+
expect(findings[0].rule).toBe(CHART_MEASURE_UNKNOWN);
649+
expect(findings[0].severity).toBe('warning');
650+
// Positions are indexes into the FILTERED collection — a dropped member
651+
// shifts them, which is the honest price of not crashing on junk.
652+
expect(findings[0].path).toBe('reports[0].chart.series[0].name');
653+
});
654+
612655
it('does not mistake a tree view named "org_chart" for a chart', () => {
613656
const findings = validateChartBindings({
614657
...baseStack(),

packages/lint/src/validate-chart-bindings.ts

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -121,17 +121,16 @@ export interface ChartBindingFinding {
121121
hint: string;
122122
}
123123

124-
import { suggestName } from './object-graph.js';
124+
// `recordsOf` — the ONE collection reader (`object-graph.ts`), not the
125+
// hand-copied `asArray` this file used to carry. That copy spelled the array
126+
// branch as an unchecked `v as AnyRec[]`, so a junk member (`reports: [null,
127+
// …]`) was dereferenced rather than skipped and the rule threw instead of
128+
// reporting. Behaviour is otherwise identical, including the map branch that
129+
// keeps a member whose value is not a record under the key the author named
130+
// it with; see that function's header for why the seam reports nothing itself.
131+
import { recordsOf, suggestName } from './object-graph.js';
125132
import { walkPageComponents, type AnyRec } from './page-walk.js';
126133

127-
function asArray(v: unknown): AnyRec[] {
128-
if (Array.isArray(v)) return v as AnyRec[];
129-
if (v && typeof v === 'object') {
130-
return Object.entries(v as AnyRec).map(([name, def]) => ({ name, ...(def as AnyRec) }));
131-
}
132-
return [];
133-
}
134-
135134
function strName(v: unknown): string | undefined {
136135
return typeof v === 'string' && v.length > 0 ? v : undefined;
137136
}
@@ -157,16 +156,16 @@ interface DatasetNames {
157156

158157
function indexDatasets(stack: AnyRec): Map<string, DatasetNames> {
159158
const out = new Map<string, DatasetNames>();
160-
for (const ds of asArray(stack.datasets)) {
159+
for (const ds of recordsOf(stack.datasets)) {
161160
const name = strName(ds.name);
162161
if (!name) continue;
163162
const dimensions = new Set<string>();
164-
for (const d of asArray(ds.dimensions)) {
163+
for (const d of recordsOf(ds.dimensions)) {
165164
const n = strName(d.name);
166165
if (n) dimensions.add(n);
167166
}
168167
const measures = new Set<string>();
169-
for (const m of asArray(ds.measures)) {
168+
for (const m of recordsOf(ds.measures)) {
170169
const n = strName(m.name);
171170
if (n) measures.add(n);
172171
}
@@ -390,7 +389,7 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] {
390389
};
391390

392391
// ── 1. Report charts (report.chart + report.blocks[].chart) ──
393-
const reports = asArray(stack.reports);
392+
const reports = recordsOf(stack.reports);
394393
for (let ri = 0; ri < reports.length; ri++) {
395394
const report = reports[ri];
396395
if (!isRec(report)) continue;
@@ -412,7 +411,7 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] {
412411
values: { names: values, path: `${path}.values` },
413412
xAxis: strName(chart.xAxis) ? { name: strName(chart.xAxis)!, path: `${path}.chart.xAxis` } : undefined,
414413
yAxis: strName(chart.yAxis) ? { name: strName(chart.yAxis)!, path: `${path}.chart.yAxis` } : undefined,
415-
series: asArray(chart.series)
414+
series: recordsOf(chart.series)
416415
.map((s, si) => ({
417416
name: strName(s.name),
418417
path: `${path}.chart.series[${si}].name`,
@@ -460,7 +459,7 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] {
460459
});
461460
};
462461

463-
const views = asArray(stack.views);
462+
const views = recordsOf(stack.views);
464463
for (let vi = 0; vi < views.length; vi++) {
465464
const view = views[vi];
466465
if (!isRec(view)) continue;
@@ -473,7 +472,7 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] {
473472
}
474473
}
475474

476-
const objects = asArray(stack.objects);
475+
const objects = recordsOf(stack.objects);
477476
for (let oi = 0; oi < objects.length; oi++) {
478477
const obj = objects[oi];
479478
if (!isRec(obj) || !isRec(obj.listViews)) continue;
@@ -491,7 +490,7 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] {
491490
// A chart component arrives through the untyped `properties` bag. The
492491
// presence of a `dataset` key is what marks it dataset-bound (and so
493492
// checkable); an object-bound chart has none and is left alone.
494-
const pages = asArray(stack.pages);
493+
const pages = recordsOf(stack.pages);
495494
for (let pi = 0; pi < pages.length; pi++) {
496495
const page = pages[pi];
497496
if (!isRec(page)) continue;
@@ -502,10 +501,10 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] {
502501
// A page chart mixes the list-chart selection (`dataset`/`dimensions`/
503502
// `values`) with ChartConfig-style axes (`yAxis: [{ field }]`), so both
504503
// shapes are read here.
505-
const axisRefs = asArray(props.yAxis)
504+
const axisRefs = recordsOf(props.yAxis)
506505
.map((a, ai) => ({ name: strName(a.field), path: `${path}.properties.yAxis[${ai}].field` }))
507506
.filter((a): a is { name: string; path: string } => !!a.name);
508-
const seriesRefs = asArray(props.series)
507+
const seriesRefs = recordsOf(props.series)
509508
.map((s, si) => ({
510509
name: strName(s.name),
511510
path: `${path}.properties.series[${si}].name`,

0 commit comments

Comments
 (0)