|
24 | 24 | * slightly differently is how one entry comes to judge a different set of |
25 | 25 | * packages than the other while both look right — so the functions moved here |
26 | 26 | * unchanged and both entries call these. |
| 27 | + * |
| 28 | + * Since #18677 the module also carries the PASS those two seams exist to feed — |
| 29 | + * {@link runPerPackageAuthoringRules} — for the same reason one layer out: the |
| 30 | + * `os build` door ran it and the `os validate` door did not, and a second copy |
| 31 | + * of the loop is how that asymmetry would come back. |
27 | 32 | */ |
28 | 33 |
|
| 34 | +import { |
| 35 | + runAuthoringRules, |
| 36 | + splitBySeverity, |
| 37 | + type AuthoringCommand, |
| 38 | + type AuthoringFinding, |
| 39 | +} from '@objectstack/lint'; |
| 40 | + |
| 41 | +/** |
| 42 | + * Identity of one finding, for the per-package de-duplication below. |
| 43 | + * |
| 44 | + * Moved here from `compile.ts` unchanged (#18677): the two doors must |
| 45 | + * de-duplicate identically, or "the set the union could not see" means two |
| 46 | + * different things depending on which command the author happened to run. |
| 47 | + */ |
| 48 | +const findingKey = (f: { rule: string; where: string; path: string; message: string }): string => |
| 49 | + [f.rule, f.where, f.path, f.message].join('\u0000'); |
| 50 | + |
29 | 51 | /** |
30 | 52 | * The artifact's package entries, as `{ index, id, body }` (ADR-0130 D4). |
31 | 53 | * |
@@ -107,3 +129,90 @@ export function packageBodyAsStack( |
107 | 129 | ): Record<string, unknown> { |
108 | 130 | return { ...body, manifest: body, packages: artifactPackageEntries }; |
109 | 131 | } |
| 132 | + |
| 133 | +/** |
| 134 | + * The author-time rule table, run ONCE PER PACKAGE and de-duplicated against a |
| 135 | + * union run — the pass `os build` has run since #16611 and `os validate` did |
| 136 | + * not (#18677). |
| 137 | + * |
| 138 | + * ## Why it lives here and not in one of the two commands |
| 139 | + * |
| 140 | + * It is the THIRD entry to owe the shape the module header describes, and the |
| 141 | + * header's fence binds it: the only ways to reach `compile.ts`' loop from |
| 142 | + * `validate.ts` are to import one oclif command from another — pulling the |
| 143 | + * lowerer and the docs sweep into every `os validate` invocation — or to write |
| 144 | + * a second copy. ⛔ The second copy is what must not happen, and here it would |
| 145 | + * not be the `{index,id,body}` reading that drifted but the VERDICT: two loops |
| 146 | + * choosing their own de-duplication key, their own severity split or their own |
| 147 | + * `where` prefix is how one door comes to report a different set from the other |
| 148 | + * while both look right. That is the defect #18677 is, one layer down. |
| 149 | + * |
| 150 | + * ## What the asymmetry was, measured |
| 151 | + * |
| 152 | + * `os build` ran this pass; `os validate` ran the union fold and stopped, |
| 153 | + * importing neither seam above. `compile.ts`' own comment says what survives |
| 154 | + * the de-duplication is "exactly the set the union could not see" ⇒ that whole |
| 155 | + * set was findings `os build` reported and `os validate` structurally could |
| 156 | + * not. The direction is FALSE-CLEAN, and on the command an author runs BEFORE |
| 157 | + * shipping — the same direction and the same door #17069 fixed one layer up, |
| 158 | + * which is why `authoringRuleUnionStack` being in both commands did not settle |
| 159 | + * it. `packages/cli/test/build-json-advisory-parity.e2e.test.ts` already |
| 160 | + * asserted "nothing rides in build's `warnings` that validate does not also |
| 161 | + * report"; it stayed green because its fixture declares no `packages[]` at all, |
| 162 | + * so the pass it would have caught never ran there. |
| 163 | + * |
| 164 | + * ## The de-duplication key is the caller's, and it is not perfect |
| 165 | + * |
| 166 | + * `findingKey` below is `compile.ts`' key, moved unchanged: `rule`, `where`, |
| 167 | + * `path`, `message`. ⚠️ `path` is POSITIONAL, and a collection index in one |
| 168 | + * package's own body is not the index the flattened top level gives the same |
| 169 | + * item — so a finding on any package whose local index differs from its |
| 170 | + * flattened one survives the filter as an ECHO of a union finding rather than |
| 171 | + * as something the union could not see. Measured on `examples/app-multi-package` |
| 172 | + * (2 packages, `crm_account.industry`): 1 survivor, 0 of them new. ⛔ Not fixed |
| 173 | + * here — changing the key changes what `os build` reports, which is a separate |
| 174 | + * decision from making the two doors agree, and agreeing IMPERFECTLY at one |
| 175 | + * seam is strictly better than disagreeing at two. When it is fixed it is |
| 176 | + * fixed once, for both commands, which is the property this module buys. |
| 177 | + */ |
| 178 | +export function runPerPackageAuthoringRules(run: { |
| 179 | + /** Which door is asking — the same string its union run passed. */ |
| 180 | + command: AuthoringCommand; |
| 181 | + /** The PARSED stack, as `artifactPackages` reads it. */ |
| 182 | + parsed: Record<string, unknown>; |
| 183 | + /** The union run's findings, whose keys this pass de-duplicates against. */ |
| 184 | + unionFindings: readonly AuthoringFinding[]; |
| 185 | + sduiManifest?: unknown; |
| 186 | + loweredHookRefs?: ReadonlySet<string>; |
| 187 | +}): { |
| 188 | + /** How many package entries were walked — 0 means the pass did not run. */ |
| 189 | + packageCount: number; |
| 190 | + errors: Array<{ package: string } & AuthoringFinding>; |
| 191 | + advisories: AuthoringFinding[]; |
| 192 | +} { |
| 193 | + const artifactPackageEntries = run.parsed.packages; |
| 194 | + const packageEntries = artifactPackages(run.parsed); |
| 195 | + const errors: Array<{ package: string } & AuthoringFinding> = []; |
| 196 | + const advisories: AuthoringFinding[] = []; |
| 197 | + if (packageEntries.length === 0) return { packageCount: 0, errors, advisories }; |
| 198 | + |
| 199 | + const alreadyReported = new Set(run.unionFindings.map(findingKey)); |
| 200 | + for (const pkg of packageEntries) { |
| 201 | + const asStack = packageBodyAsStack(pkg.body, artifactPackageEntries); |
| 202 | + const pkgFindings = runAuthoringRules(run.command, { |
| 203 | + normalized: asStack, |
| 204 | + parsed: asStack, |
| 205 | + sduiManifest: run.sduiManifest, |
| 206 | + loweredHookRefs: run.loweredHookRefs, |
| 207 | + }).filter((f) => !alreadyReported.has(findingKey(f))); |
| 208 | + for (const f of pkgFindings) alreadyReported.add(findingKey(f)); |
| 209 | + const split = splitBySeverity(pkgFindings); |
| 210 | + advisories.push( |
| 211 | + ...split.advisories.map((a) => ({ ...a, where: `package '${pkg.id}' — ${a.where}` })), |
| 212 | + ); |
| 213 | + errors.push( |
| 214 | + ...split.errors.map((e) => ({ ...e, package: pkg.id, where: `package '${pkg.id}' — ${e.where}` })), |
| 215 | + ); |
| 216 | + } |
| 217 | + return { packageCount: packageEntries.length, errors, advisories }; |
| 218 | +} |
0 commit comments