diff --git a/.changeset/rule-id-barrel-export-gap.md b/.changeset/rule-id-barrel-export-gap.md new file mode 100644 index 0000000000..531c22b665 --- /dev/null +++ b/.changeset/rule-id-barrel-export-gap.md @@ -0,0 +1,21 @@ +--- +"@objectstack/lint": patch +--- + +fix(lint): 七个规则 id 常量补进 barrel —— 消费者不必再对字面量,并加一条测试面门禁 (#5648) + +规则把自己的 id 写进每条 finding 的 `f.rule`,而那个字符串就是 `os lint --json` / `os validate` 递到消费者手上的东西:Studio 的 finding 渲染、下游按规则过滤/抑制、以及被授权元数据里的 `suppressWarnings: ['']`。规则文件为此导出同名常量,消费者本该比对常量而不是重敲 slug。 + +但 `packages/lint` 的 `package.json#exports` 只开 `"."` 与 `"./runtime"`(`tsup.config.ts` 的 entry 也只有这两个),所以**没进 barrel 就不是「不好取」,而是完全取不到** —— 没有深路径可绕,消费者唯一的退路正是那个常量本来要消灭的字符串字面量。 + +#5648 报的是其中一个(`FLOW_TRIGGER_UNKNOWN_EVENT`,#3427/#3457/#3481 三条规则共用的 id)。它要求的全量清查又翻出**六个**,散在五个规则文件、成因都远早于它:`APPROVAL_APPROVER_TYPE_UNSUPPORTED`、`SECURITY_FLS_UNQUALIFIED_KEY`、`FIELD_GROUP_SHADOWED`、`WIDGET_LEGACY_ANALYTICS_SHAPE`、`WIDGET_LEGACY_ANALYTICS_UNRENDERABLE`、`REACT_CHART_DRILLDOWN_INVALID`。其中 `WIDGET_LEGACY_ANALYTICS_SHAPE` 最能说明代价:规则打给用户的提示原话就是 `Suppress with suppressWarnings: ['widget-legacy-analytics-shape']`,即它主动教消费者用这个 id,却不让消费者拿到承载它的常量。 + +漏项之所以能一路静默:规则照常工作,它自己的单测**从规则文件直接 import 常量**(不经 barrel),于是唯一会发现的时刻是有人从包外去消费它。同一种一行漏项独立发生七次,不是「下次记牢」能解决的记性问题,而是缺一条判定。 + +**因此判定权移进 `packages/lint` 测试面**(`src/rule-id-barrel-exports.test.ts`):新增规则时忘了 barrel 那行,会在新规则自己的测试转绿的同一次 run 里失败。分层理由是这条不变量完全是包内的 —— 没有别处定义 lint 规则 id —— 且它要**真的 import** barrel 来核验取值,vitest 天然给得到;换成 `scripts/` 门禁则要么自己写一个 ES 解析器,要么先构建 dist,还会把反馈挪到另一个 job。 + +门禁按两类假绿反向设计:发现面是对 `src/` 的**文件系统读取**而非手写清单(新规则文件一存在即被枚举),并对 id 条数压一条下限,避免将来改坏提取式后在空集上「全绿」;两个 entry 虽是静态列出(全动态 import 无法可靠打包),但另有一条用例从 `package.json#exports` 与 `tsup.config.ts` **各自独立**推导出同一集合并比对,新增第三个 entry 若不登记就会红,而不是悄悄不被检查。 + +分类按**取值形状**而非发射位置判定:早先一版靠「定义旁边有 `rule: NAME`」来认,结果漏掉了经辅助函数参数发射的四个 `REACT_CHART_*` —— 恰好包含本次真实漏项之一。 + +仅新增导出面,无行为变化:任何既有 `f.rule` 字符串都没有改动,原先对字面量的消费者继续可用。 diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 55d1da2fde..add2995cff 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -20,6 +20,8 @@ export { CHART_CONFIG_MISSING, TABLE_COUNT_ONLY, MEASURE_AGGREGATE_INCOHERENT, + WIDGET_LEGACY_ANALYTICS_SHAPE, + WIDGET_LEGACY_ANALYTICS_UNRENDERABLE, DASHBOARD_FILTER_FIELD_UNKNOWN, } from './validate-widget-bindings.js'; export type { WidgetBindingFinding, WidgetBindingSeverity } from './validate-widget-bindings.js'; @@ -53,6 +55,7 @@ export { validateFlowTriggerReadiness, FLOW_TRIGGER_UNKNOWN_OBJECT, FLOW_DRAFT_STATUS_AMBIGUOUS, + FLOW_TRIGGER_UNKNOWN_EVENT, FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID, } from './validate-flow-trigger-readiness.js'; export type { @@ -101,6 +104,7 @@ export { REACT_CHART_FIELD_UNKNOWN, REACT_CHART_AGGREGATE_INVALID, REACT_CHART_AXIS_UNKNOWN, + REACT_CHART_DRILLDOWN_INVALID, REACT_BLOCK_NEEDS_RECORD_CONTEXT, } from './validate-react-page-props.js'; export type { ReactPropFinding, ReactPropSeverity } from './validate-react-page-props.js'; @@ -118,6 +122,7 @@ export { validateSemanticRoles, FIELD_GROUP_UNDECLARED, FIELD_GROUP_EMPTY, + FIELD_GROUP_SHADOWED, SEMANTIC_ROLE_FIELD_UNKNOWN, } from './validate-semantic-roles.js'; export type { SemanticRoleFinding, SemanticRoleSeverity } from './validate-semantic-roles.js'; @@ -152,6 +157,7 @@ export { APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER, APPROVAL_APPROVER_TYPE_DEPRECATED, APPROVAL_APPROVER_TYPE_UNKNOWN, + APPROVAL_APPROVER_TYPE_UNSUPPORTED, APPROVAL_ESCALATION_REASSIGN_NO_TARGET, APPROVAL_APPROVERS_MAY_RESOLVE_EMPTY, APPROVAL_EXPRESSION_INVALID, @@ -184,6 +190,7 @@ export { SECURITY_BOOK_AUDIENCE_UNKNOWN_SET, SECURITY_PRIVATE_NO_READSCOPE, SECURITY_MASTER_DETAIL_UNGRANTED, + SECURITY_FLS_UNQUALIFIED_KEY, SECURITY_GRANT_EXPIRED_AT_AUTHORING, SECURITY_DELEGATION_MISSING_REASON, } from './validate-security-posture.js'; diff --git a/packages/lint/src/rule-id-barrel-exports.test.ts b/packages/lint/src/rule-id-barrel-exports.test.ts new file mode 100644 index 0000000000..9e0a7fd0f7 --- /dev/null +++ b/packages/lint/src/rule-id-barrel-exports.test.ts @@ -0,0 +1,205 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Public-surface contract (#5648): every rule id constant a rule file exports +// must be reachable from a PUBLISHED barrel entry. +// +// Why this can silently rot, and why it matters. A rule emits its id into every +// finding (`f.rule`), and that string is what `os lint --json` / `os validate` +// put in front of consumers — Studio's finding renderer, downstream filtering, +// and `suppressWarnings: ['']` in authored metadata. The rule files +// export a named constant for exactly that reason: a consumer should compare +// against the constant, not retype the slug. But `package.json#exports` opens +// only "." and "./runtime" (and `tsup.config.ts` builds only those two +// entries), so a constant that no barrel re-exports is not merely inconvenient +// to reach — it is UNREACHABLE. There is no deep path to fall back to, and the +// consumer's only remaining option is the string literal the constant existed +// to eliminate. +// +// Nothing failed when the barrel line was forgotten: the rule kept working, its +// own unit test imported the constant from the rule file directly (not from the +// barrel), and the omission surfaced only when someone tried to consume it from +// outside the package. #5648 was filed for one such gap +// (`FLOW_TRIGGER_UNKNOWN_EVENT`); the sweep it asked for found SIX more, spread +// over five rule files and going back well before it — +// `APPROVAL_APPROVER_TYPE_UNSUPPORTED`, `SECURITY_FLS_UNQUALIFIED_KEY`, +// `FIELD_GROUP_SHADOWED`, `WIDGET_LEGACY_ANALYTICS_SHAPE`, +// `WIDGET_LEGACY_ANALYTICS_UNRENDERABLE`, `REACT_CHART_DRILLDOWN_INVALID`. Seven +// independent omissions of the same one-line kind is not a memory problem to +// solve by remembering harder; it is a missing check. So the judgement moves +// here: adding a rule id and forgetting the barrel line now fails a test in the +// same package, in the same run the new rule's own tests go green. +// +// Two shapes of false green this is built to refuse: +// +// 1. A rule file the check never looked at. The discovery surface is a +// filesystem read of `src/`, never a hand-maintained list of rule files, so +// a new rule file is enumerated the moment it exists. `covers the whole +// rule surface` additionally pins a floor on the number of ids found, so a +// future edit that breaks the extraction pattern fails loudly instead of +// passing over an empty set. +// 2. A barrel entry the check never imported. The two entries are named +// statically below (a fully dynamic import cannot be bundled reliably), but +// `barrel entries match the published exports map` proves that list is the +// COMPLETE set of published entries by deriving it independently from +// `package.json#exports` and from `tsup.config.ts`. Adding a third entry +// without registering it here fails rather than going unchecked. +// +// The reverse direction (`no barrel export names a rule id that no rule file +// defines`) is deliberately weaker than it looks, and should be read as +// visibility rather than as the primary guard: a re-export of a name the source +// module no longer exports cannot survive `tsc` or the `dts` build at all. What +// this direction does catch is a name that still resolves but has drifted off +// its rule — a barrel line pointing at a different module than the definition, +// or a rule-id-shaped string exported from somewhere that no longer emits it. +import { readFileSync, readdirSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; + +import * as indexBarrel from './index.js'; +import * as runtimeBarrel from './runtime.js'; + +const srcDir = dirname(fileURLToPath(import.meta.url)); +const pkgDir = join(srcDir, '..'); + +/** + * The published entries, keyed by the `src/.ts` file each one builds from. + * Static imports because a bundler cannot follow a fully dynamic one — kept + * honest by `barrel entries match the published exports map`, which derives the + * same set from `package.json` and `tsup.config.ts` and fails if this record + * has drifted. + */ +const BARREL_ENTRIES: Record> = { + index: indexBarrel as unknown as Record, + runtime: runtimeBarrel as unknown as Record, +}; + +/** + * A rule id is a lowercase slug: `flow-trigger-unknown-event`, + * `unique/double-declaration` (namespaced), `page-source-className-tailwind` + * (a camelCase segment where the id names an authored key). + * + * Shape, not emission site, decides. An earlier draft of this test classified + * by looking for `rule: NAME` next to the definition and MISSED the four + * `REACT_CHART_*` ids, which reach their finding through a helper argument + * instead — including the one gap that turned out to be real. Shape is the + * property every id has regardless of how it travels. + * + * What this correctly leaves out today is the one exported string const in the + * package that is not an id: `RELATED_LIST_TYPE = 'record:related_list'`, a + * page block type (the `:` and `_` are not id spelling). If a future non-id + * const IS slug-shaped, prefer re-exporting it over loosening this pattern — + * the barrel is the package's public surface, and an exported const that no + * consumer can import is the defect this test exists to name. + */ +const RULE_ID_SHAPE = /^[a-z][A-Za-z0-9]*(?:[-/][A-Za-z0-9]+)*$/; + +/** `export const NAME = 'value';` on one line — how every rule id is declared. */ +const EXPORTED_STRING_CONST = /^export const ([A-Z][A-Z0-9_]*) = '([^']*)';\s*$/; + +interface RuleId { + name: string; + value: string; + file: string; + line: number; +} + +/** Every rule id constant declared anywhere under `src/`, found by reading the directory. */ +function declaredRuleIds(): RuleId[] { + const found: RuleId[] = []; + for (const file of readdirSync(srcDir).sort()) { + if (!file.endsWith('.ts') || file.endsWith('.test.ts')) continue; + readFileSync(join(srcDir, file), 'utf8').split('\n').forEach((line, i) => { + const m = EXPORTED_STRING_CONST.exec(line); + if (m && RULE_ID_SHAPE.test(m[2])) { + found.push({ name: m[1], value: m[2], file, line: i + 1 }); + } + }); + } + return found; +} + +describe('rule id constants are reachable from a published barrel (#5648)', () => { + it('barrel entries match the published exports map', () => { + const pkg = JSON.parse(readFileSync(join(pkgDir, 'package.json'), 'utf8')) as { + exports: Record; + }; + // "." -> dist/index.js -> "index"; "./runtime" -> dist/runtime.js -> "runtime" + const published = Object.values(pkg.exports) + .map((e) => /^\.\/dist\/(.+)\.js$/.exec(e.import)?.[1]) + .filter((n): n is string => !!n) + .sort(); + expect(published.length).toBe(Object.keys(pkg.exports).length); + + // tsup builds the entries; an entry published but not built (or the + // reverse) would make this test judge reachability against a file that + // never ships. + const tsup = readFileSync(join(pkgDir, 'tsup.config.ts'), 'utf8'); + const entryBlock = /entry:\s*\[([^\]]*)\]/.exec(tsup)?.[1] ?? ''; + const built = [...entryBlock.matchAll(/'src\/(.+?)\.ts'/g)].map((m) => m[1]).sort(); + + expect(built).toEqual(published); + expect(Object.keys(BARREL_ENTRIES).sort()).toEqual(published); + }); + + it('covers the whole rule surface', () => { + const ids = declaredRuleIds(); + // A floor, not an exact count — new rules are expected. Its only job is to + // fail if the extraction above ever stops finding anything, which would + // make every assertion below vacuously true. + expect(ids.length).toBeGreaterThan(100); + expect(new Set(ids.map((r) => r.file)).size).toBeGreaterThan(30); + + // One id, one name. Two files exporting the same name could only ever have + // one of them re-exported, and the barrel would silently answer for the + // wrong rule. + const byName = new Map(); + for (const id of ids) byName.set(id.name, [...(byName.get(id.name) ?? []), id]); + expect([...byName].filter(([, v]) => v.length > 1).map(([n]) => n)).toEqual([]); + }); + + it('every declared rule id constant is exported from a barrel, with its own value', () => { + const unreachable: string[] = []; + const mismatched: string[] = []; + + for (const id of declaredRuleIds()) { + const carrying = Object.entries(BARREL_ENTRIES).filter(([, ns]) => id.name in ns); + if (carrying.length === 0) { + unreachable.push(`${id.name} ('${id.value}') — ${id.file}:${id.line}`); + continue; + } + // Reachable by name is not enough: the name must carry THIS definition's + // value, or the barrel is answering for a different constant. + for (const [entry, ns] of carrying) { + if (ns[id.name] !== id.value) { + mismatched.push( + `${id.name} — ${id.file}:${id.line} declares '${id.value}', ` + + `barrel '${entry}' exports '${String(ns[id.name])}'`, + ); + } + } + } + + // `package.json#exports` opens only these entries, so an id missing from + // both has NO import path at all — add it to the rule module's existing + // `export { … } from './.js'` block in `src/index.ts`. + expect({ unreachable, mismatched }).toEqual({ unreachable: [], mismatched: [] }); + }); + + it('no barrel export names a rule id that no rule file defines', () => { + const declared = new Map(declaredRuleIds().map((r) => [r.name, r.value])); + const orphaned: string[] = []; + + for (const [entry, ns] of Object.entries(BARREL_ENTRIES)) { + for (const [name, value] of Object.entries(ns)) { + if (typeof value !== 'string' || !RULE_ID_SHAPE.test(value)) continue; + if (!/^[A-Z][A-Z0-9_]*$/.test(name)) continue; + if (declared.get(name) !== value) { + orphaned.push(`${entry}: ${name} = '${value}'`); + } + } + } + + expect(orphaned).toEqual([]); + }); +});