From 00443207e395e17da6b65490fc3d4957e669cf43 Mon Sep 17 00:00:00 2001 From: kkdev92 <112151103+kkdev92@users.noreply.github.com> Date: Sun, 30 Aug 2026 00:04:52 +0900 Subject: [PATCH] feat(testing): return manifest disagreements as data `assertManifestMatches` compared `package.json` with what `src` declares and reported every disagreement as a sentence in one thrown error. That is the right shape for a test and the wrong one for anything else: a tool that wants to print the disagreements, count them, or apply the mechanical part of the fix had to parse prose. `diffManifest` is the comparison on its own, returning each disagreement as data: which contribution point, which side is missing the entry -- or `drift`, when both have it and disagree about its type, default, enum or scope -- the id it concerns, and the JSON that would settle it when the fix is mechanical. A missing command or setting carries that JSON; a missing view does not, because a view needs a container and which one is a decision the declaration does not carry. The assertion is unchanged in what it reports, and is now built on the diff, so the two cannot disagree about what disagrees. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 8 ++ docs/guide.md | 5 ++ src/testing/index.ts | 4 +- src/testing/manifest.ts | 137 ++++++++++++++++++++++++--------- tests/testing/manifest.test.ts | 84 +++++++++++++++++++- 5 files changed, 200 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c100be..70fd159 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,14 @@ Pre-1.0 releases followed it in spirit; their breaking changes are marked **Brea lives in `bin/` and is exercised by `verify:package` against the installed tarball, not just the repository's own layout. +- **`diffManifest` returns what `assertManifestMatches` used to only throw.** + The assertion compared `package.json` with the declarations in `src` and + reported every disagreement as a sentence in one error. The comparison is now + its own function, returning each disagreement as data — which contribution + point, which side is missing it or whether both have it and disagree, the id + it concerns, and the JSON that would settle it when the fix is mechanical. + The assertion is unchanged and built on top of it. + ### Changed - **`defineExtension` is single-use, like the extension host it serves.** A diff --git a/docs/guide.md b/docs/guide.md index dfa1507..15e0167 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -869,6 +869,11 @@ generating the manifest from TypeScript turns `package.json` into a file a human still has to hand-edit. Verifying the overlap costs one test and leaves both files written by the people who own them. +The comparison is also available as data: `diffManifest` returns every +disagreement with the contribution point it concerns, which side is missing it +— or `drift`, when both have it and disagree — the id, and the JSON that would +settle it when the fix is mechanical. The assertion above is built on it. + ## The escape hatch diff --git a/src/testing/index.ts b/src/testing/index.ts index a173a9e..b5dfa81 100644 --- a/src/testing/index.ts +++ b/src/testing/index.ts @@ -49,8 +49,8 @@ export type { ApplicationInspection } from '../foundation/application/applicatio // Manifest and source remain separate because VS Code consumes contributions // before activation. This assertion makes source declarations authoritative // only for their mechanical overlap; human-facing manifest text stays manual. -export { assertManifestMatches } from './manifest.js'; -export type { DeclaredContributions } from './manifest.js'; +export { assertManifestMatches, diffManifest } from './manifest.js'; +export type { DeclaredContributions, ManifestMismatch } from './manifest.js'; export { createFakeMemento, createFakeSecrets, createFakeStorage } from './fakes/fake-storage.js'; export type { FakeMemento, FakeSecrets, FakeStorage } from './fakes/fake-storage.js'; diff --git a/src/testing/manifest.ts b/src/testing/manifest.ts index 0aaff8c..9b4d545 100644 --- a/src/testing/manifest.ts +++ b/src/testing/manifest.ts @@ -26,8 +26,20 @@ export interface DeclaredContributions { readonly views?: readonly string[]; } -/** A single disagreement between the manifest and what `src` declares. */ -interface Mismatch { +/** + * One disagreement between `package.json` and what `src` declares. + * + * `kind` says which contribution point, `direction` which side is missing + * something — or `drift`, when both have the entry and disagree about it — and + * `id` the command, setting key or view it concerns. `summary` says the same + * thing to a person; `paste` is the JSON that would settle it, present when the + * fix is mechanical and absent when only a person can supply it (a view needs + * a container; a command needs a title). + */ +export interface ManifestMismatch { + readonly kind: 'command' | 'setting' | 'view'; + readonly direction: 'missing-in-manifest' | 'missing-in-src' | 'drift'; + readonly id: string; readonly summary: string; /** JSON to paste into `contributes`, when the fix is mechanical. */ readonly paste?: string; @@ -102,17 +114,20 @@ function viewIds(manifest: Manifest): readonly string[] { .filter((id): id is string => typeof id === 'string'); } -function checkCommands(manifest: Manifest, declared: DeclaredContributions): Mismatch[] { +function checkCommands(manifest: Manifest, declared: DeclaredContributions): ManifestMismatch[] { if (declared.commands === undefined) { return []; } const inManifest = new Set(commandIds(manifest)); const inSource = new Set(declared.commands.map((contract) => contract.descriptor.id)); - const mismatches: Mismatch[] = []; + const mismatches: ManifestMismatch[] = []; for (const id of inSource) { if (!inManifest.has(id)) { mismatches.push({ + kind: 'command', + direction: 'missing-in-manifest', + id, summary: `command "${id}" is declared in src but missing from contributes.commands`, paste: JSON.stringify({ command: id, title: 'TODO' }, null, 2), }); @@ -120,8 +135,10 @@ function checkCommands(manifest: Manifest, declared: DeclaredContributions): Mis } for (const id of inManifest) { if (!inSource.has(id)) { - // The palette would offer a command nothing handles. mismatches.push({ + kind: 'command', + direction: 'missing-in-src', + id, summary: `command "${id}" is in contributes.commands but no contract declares it`, }); } @@ -129,13 +146,19 @@ function checkCommands(manifest: Manifest, declared: DeclaredContributions): Mis return mismatches; } -function checkSettings(manifest: Manifest, declared: DeclaredContributions): Mismatch[] { +function checkSettings(manifest: Manifest, declared: DeclaredContributions): ManifestMismatch[] { if (declared.settings === undefined) { return []; } const properties = manifest.contributes?.configuration?.properties ?? {}; - const mismatches: Mismatch[] = []; + const mismatches: ManifestMismatch[] = []; const expected = new Set(); + const drift = (id: string, summary: string): ManifestMismatch => ({ + kind: 'setting', + direction: 'drift', + id, + summary, + }); for (const group of declared.settings) { for (const [name, spec] of Object.entries(group.values)) { @@ -144,40 +167,49 @@ function checkSettings(manifest: Manifest, declared: DeclaredContributions): Mis const entry = properties[key]; if (entry === undefined) { mismatches.push({ + kind: 'setting', + direction: 'missing-in-manifest', + id: key, summary: `setting "${key}" is declared in src but missing from contributes.configuration`, paste: JSON.stringify({ [key]: contributionFor(spec) }, null, 2), }); continue; } - // Only the machine-facing facts. Descriptions, ordering and - // `markdownDescription` are the manifest's to own. if (!sameType(entry['type'], spec.type)) { - mismatches.push({ - summary: + mismatches.push( + drift( + key, `setting "${key}" is ${JSON.stringify(entry['type'])} in the manifest ` + - `and ${JSON.stringify(spec.type)} in src`, - }); + `and ${JSON.stringify(spec.type)} in src` + ) + ); } if (!equalJson(entry['default'], spec.default)) { - mismatches.push({ - summary: + mismatches.push( + drift( + key, `setting "${key}" defaults to ${JSON.stringify(entry['default'])} in the manifest ` + - `and ${JSON.stringify(spec.default)} in src`, - }); + `and ${JSON.stringify(spec.default)} in src` + ) + ); } if (spec.enum !== undefined && !equalJson(entry['enum'], [...spec.enum])) { - mismatches.push({ - summary: + mismatches.push( + drift( + key, `setting "${key}" allows ${JSON.stringify(entry['enum'])} in the manifest ` + - `and ${JSON.stringify(spec.enum)} in src`, - }); + `and ${JSON.stringify(spec.enum)} in src` + ) + ); } if ((entry['scope'] ?? DEFAULT_MANIFEST_SCOPE) !== spec.scope) { - mismatches.push({ - summary: + mismatches.push( + drift( + key, `setting "${key}" is scoped ${JSON.stringify(entry['scope'] ?? DEFAULT_MANIFEST_SCOPE)} ` + - `in the manifest and "${spec.scope}" in src`, - }); + `in the manifest and "${spec.scope}" in src` + ) + ); } } } @@ -185,8 +217,10 @@ function checkSettings(manifest: Manifest, declared: DeclaredContributions): Mis const sections = declared.settings.map((group) => `${group.section}.`); for (const key of Object.keys(properties)) { if (sections.some((prefix) => key.startsWith(prefix)) && !expected.has(key)) { - // A setting the user can change that the extension never reads. mismatches.push({ + kind: 'setting', + direction: 'missing-in-src', + id: key, summary: `setting "${key}" is contributed but no declaration in src reads it`, }); } @@ -194,7 +228,7 @@ function checkSettings(manifest: Manifest, declared: DeclaredContributions): Mis return mismatches; } -function checkViews(manifest: Manifest, declared: DeclaredContributions): Mismatch[] { +function checkViews(manifest: Manifest, declared: DeclaredContributions): ManifestMismatch[] { if (declared.views === undefined) { return []; } @@ -203,12 +237,50 @@ function checkViews(manifest: Manifest, declared: DeclaredContributions): Mismat return [ ...[...inSource] .filter((id) => !inManifest.has(id)) - .map((id) => ({ + .map((id): ManifestMismatch => ({ + kind: 'view', + direction: 'missing-in-manifest', + id, + // No `paste`: a view needs a container, and which one is a design + // decision the declaration does not carry. summary: `view "${id}" is registered in src but missing from contributes.views`, })), ...[...inManifest] .filter((id) => !inSource.has(id)) - .map((id) => ({ summary: `view "${id}" is contributed but nothing in src registers it` })), + .map((id): ManifestMismatch => ({ + kind: 'view', + direction: 'missing-in-src', + id, + summary: `view "${id}" is contributed but nothing in src registers it`, + })), + ]; +} + +/** + * Every disagreement between `package.json` and the declarations in `src`, as + * data, in the order the checks run: commands, then settings, then views. + * + * The same comparison {@link assertManifestMatches} makes, without the throw — + * for a tool that wants to print, count or apply the mechanical part of the + * fix itself. An empty result means the two agree on everything this checks. + * + * @example + * ```ts + * const mismatches = diffManifest(manifest, { commands: Object.values(Contracts) }); + * for (const mismatch of mismatches.filter((m) => m.direction === 'missing-in-manifest')) { + * console.log(mismatch.paste ?? mismatch.summary); + * } + * ``` + */ +export function diffManifest( + manifest: unknown, + declared: DeclaredContributions +): readonly ManifestMismatch[] { + const parsed = (manifest ?? {}) as Manifest; + return [ + ...checkCommands(parsed, declared), + ...checkSettings(parsed, declared), + ...checkViews(parsed, declared), ]; } @@ -246,12 +318,7 @@ function checkViews(manifest: Manifest, declared: DeclaredContributions): Mismat * @throws when anything disagrees, listing every disagreement at once */ export function assertManifestMatches(manifest: unknown, declared: DeclaredContributions): void { - const parsed = (manifest ?? {}) as Manifest; - const mismatches = [ - ...checkCommands(parsed, declared), - ...checkSettings(parsed, declared), - ...checkViews(parsed, declared), - ]; + const mismatches = diffManifest(manifest, declared); if (mismatches.length === 0) { return; } diff --git a/tests/testing/manifest.test.ts b/tests/testing/manifest.test.ts index 23d14e2..568a888 100644 --- a/tests/testing/manifest.test.ts +++ b/tests/testing/manifest.test.ts @@ -10,7 +10,7 @@ import { describe, expect, it } from 'vitest'; import { defineCommandContract } from '../../src/foundation/commands/contract.js'; import { defineSettings, setting } from '../../src/foundation/settings/definition.js'; -import { assertManifestMatches } from '../../src/testing/manifest.js'; +import { assertManifestMatches, diffManifest } from '../../src/testing/manifest.js'; const Refresh = defineCommandContract({ id: 'sample.refresh' }); const Clear = defineCommandContract({ id: 'sample.clear' }); @@ -251,3 +251,85 @@ describe('assertManifestMatches, on a nullable setting', () => { }); }); }); + +/** + * The same comparison, as data. + * + * A tool that prints, counts or applies the mechanical part of the fix needs + * more than a sentence: which contribution point, which side, which id, and + * whether there is JSON that settles it. The assertion is built on this, so + * the two can never disagree about what disagrees. + */ +describe('diffManifest', () => { + const declared = { commands: [Refresh, Clear], settings: [Options], views: ['sample.tree'] }; + + /** The agreeing manifest with one problem of each kind introduced. */ + function disagreeingManifest(): unknown { + const manifest = agreeingManifest() as { + contributes: { + commands: unknown[]; + configuration: { properties: Record> }; + views: Record; + }; + }; + // One command gone, one nobody handles. + manifest.contributes.commands = [ + { command: 'sample.refresh', title: 'Refresh' }, + { command: 'sample.ghost', title: 'Ghost' }, + ]; + // A type that drifted. + manifest.contributes.configuration.properties['sample.limit'] = { + type: 'number', + default: 10, + scope: 'window', + }; + // The declared view missing, an undeclared one present. + manifest.contributes.views = { sampleContainer: [{ id: 'sample.other', name: 'Other' }] }; + return manifest; + } + + it('returns nothing when the two agree', () => { + expect(diffManifest(agreeingManifest(), declared)).toEqual([]); + }); + + it('reports each disagreement as data, in the order the checks run', () => { + const mismatches = diffManifest(disagreeingManifest(), declared); + + expect(mismatches.map((m) => [m.kind, m.direction, m.id])).toEqual([ + ['command', 'missing-in-manifest', 'sample.clear'], + ['command', 'missing-in-src', 'sample.ghost'], + ['setting', 'drift', 'sample.limit'], + ['view', 'missing-in-manifest', 'sample.tree'], + ['view', 'missing-in-src', 'sample.other'], + ]); + }); + + it('attaches the JSON to paste only where the fix is mechanical', () => { + const mismatches = diffManifest(disagreeingManifest(), declared); + const byId = new Map(mismatches.map((m) => [m.id, m])); + + // A missing command has a mechanical shape; the title is a placeholder. + expect(byId.get('sample.clear')?.paste).toContain('"command": "sample.clear"'); + // A missing view needs a container, which is a decision, not a fact in src. + expect(byId.get('sample.tree')?.paste).toBeUndefined(); + // Drift is a disagreement about a value, not an absence; there is nothing to paste. + expect(byId.get('sample.limit')?.paste).toBeUndefined(); + }); + + it('is exactly what the assertion reports, one summary per line', () => { + const manifest = disagreeingManifest(); + const summaries = diffManifest(manifest, declared).map((m) => m.summary); + + let message = ''; + try { + assertManifestMatches(manifest, declared); + } catch (error) { + message = error instanceof Error ? error.message : String(error); + } + + expect(message).toContain(`${String(summaries.length)} place(s)`); + for (const summary of summaries) { + expect(message).toContain(summary); + } + }); +});