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
60 changes: 55 additions & 5 deletions scripts/check-adr-0087-registration.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,18 @@ const isChangesetFile = (p) => p.startsWith('.changeset/') && p.endsWith('.md')
/**
* Split a changeset into its frontmatter bump entries and its body.
*
* The entry regex is deliberately the SAME shape `check-changeset-no-major.mjs`
* and `check-empty-changeset.mjs` use. Three gates reading one block must agree on
* what counts as a declaration, or one of them is judging a different file than it
* appears to.
* The entry regex is deliberately the SAME shape `check-changeset-no-major.mjs`,
* `check-empty-changeset.mjs` and `objectui-changeset-digest.mjs` use. Four
* readers of one block must agree on what counts as a declaration, or one of them
* is judging a different file than it appears to. `check-empty-changeset.mjs`'s
* self-test asserts that agreement byte-for-byte across all four (#7004).
*
* This parser's stake in #7004 is signal (1) of `breakingDeclaration` below: a
* `major` carrying a trailing YAML comment (or a quoted bump value) used to read
* as no bump at all, so the frontmatter signal went missing and only signals (2)
* `**BREAKING` and (3) the `!` summary could still carry the declaration. A
* changeset using (1) alone — which the ADR-0087 worklist treats as a full
* declaration — was invisible to this gate.
*
* @param {string} text
* @returns {{ fenced: boolean, bumps: {pkg: string, bump: string}[], body: string }}
Expand All @@ -205,7 +213,10 @@ export function parseChangeset(text) {
let end = -1;
for (let j = i + 1; j < lines.length; j++) {
if (lines[j].trim() === '---') { end = j; break; }
const m = /^\s*["']?([^"':]+)["']?\s*:\s*([A-Za-z]+)\s*$/.exec(lines[j]);
if (/^\s*#/.test(lines[j])) continue; // a whole-line YAML comment declares nothing
// "<name>": <bump> | '<name>': <bump> | <name>: <bump>
// with an optionally quoted bump value and an optional trailing ` # comment`.
const m = /^\s*["']?([^"':]+)["']?\s*:\s*["']?([A-Za-z]+)["']?(?:\s+#.*)?\s*$/.exec(lines[j]);
if (m) bumps.push({ pkg: m[1].trim(), bump: m[2].trim().toLowerCase() });
}
if (end < 0) return { fenced: false, bumps: [], body: text };
Expand Down Expand Up @@ -2030,6 +2041,45 @@ function selfTest() {

assert(breakingDeclaration(parseChangeset(CS({ body: 'feat(spec)!: x\n' }))).breaking, 'P6: a conventional-commit bang is a declaration');
assert(!breakingDeclaration(parseChangeset(CS({ bumps: [['a', 'patch']], body: 'plain\n' }))).breaking, 'P7: a plain patch is not');

// ---- P9-P14 (#7004): the shapes the old entry anchor hid from signal (1) ---
//
// This gate's stake in #7004 is the PARTIAL miss: `breakingDeclaration` reads
// three signals, and a `major` wearing a trailing YAML comment (or a quoted
// bump value) used to vanish from signal (1) — leaving (2) `**BREAKING` and
// (3) the `!` summary to carry a declaration they may not carry at all. So
// each fixture below states the bump WITHOUT either of the other two signals:
// a plain body, so `major` is the only thing that can make it breaking.
//
// Predicted direction on reverse verification: restoring the old anchor
// (`([A-Za-z]+)\s*$`) turns P9-P13 red (breaking goes false, signals loses
// `major`) and P14 red in the other direction (a phantom `# note` bump).
const PLAIN = 'a summary line\n\nsome prose that is quite long indeed and explains the change.\n';
const bumpsOf = (text) => parseChangeset(text).bumps.map((b) => `${b.pkg}=${b.bump}`);
assert(
bumpsOf(`---\n'@objectstack/spec': major # keep\n---\n\n${PLAIN}`).join(',') === '@objectstack/spec=major',
'P9 (#7004): a trailing YAML comment still yields the `major` bump — changesets reads it as one',
);
assert(
breakingDeclaration(parseChangeset(`---\n'@objectstack/spec': major # keep\n---\n\n${PLAIN}`)).signals.includes('major'),
'P10 (#7004): signal (1) fires on a comment-bearing major, with no `**BREAKING` and no `!` in the body to carry it instead',
);
assert(
breakingDeclaration(parseChangeset(`---\n'@objectstack/spec': "major"\n---\n\n${PLAIN}`)).signals.includes('major'),
'P11 (#7004): signal (1) fires on a QUOTED bump value',
);
assert(
breakingDeclaration(parseChangeset(`---\n'@objectstack/spec': 'major' # keep\n---\n\n${PLAIN}`)).signals.includes('major'),
'P12 (#7004): signal (1) fires on a quoted bump value carrying a comment',
);
assert(
!breakingDeclaration(parseChangeset(`---\n'@objectstack/spec': minor # keep\n---\n\n${PLAIN}`)).breaking,
'P13 (#7004): control — the same comment-bearing shape with `minor` is NOT breaking, so P9-P12 are about the bump word and not about the comment merely being tolerated',
);
assert(
bumpsOf(`---\n# note: major\n'@objectstack/real': patch\n---\n\n${PLAIN}`).join(',') === '@objectstack/real=patch',
'P14 (#7004): a whole-line comment containing a colon is not a bump — it used to parse as a package named `# note` bumped major, i.e. a phantom breaking declaration',
);
assert(extractIds(" id: 'object-titleFormat-to-nameField',\n").length === 1, 'P8: an id with a capital letter must be extracted');

// ---- I1 (#6566): a bare `import` of this module must NOT run the gate -----
Expand Down
108 changes: 85 additions & 23 deletions scripts/check-changeset-no-major.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,24 @@
* a leading blank line before `---` | major | caught (see below)
* "@objectstack/spec": MAJOR | THROWS invalid type | caught (harmless)
* no closing `---` fence | THROWS missing fm | caught (harmless)
* "@objectstack/spec": major # note | major | MISSED (see below)
* "@objectstack/spec": major # note | major | caught (#7004)
* "@objectstack/spec": "major" | major | caught (#7004)
* "@objectstack/spec": 'major' # n | major | caught (#7004)
* # note: major (comment line) | declares NOTHING | ignored (#7004)
* "@objectstack/spec": major# note | THROWS invalid type | missed (harmless)
*
* Rows marked "harmless" are this file being STRICTER than changesets on a file
* changesets refuses outright: the guard names a major in a changeset that could
* never version anything. That direction costs an author one confusing message
* about a file that is already broken. The opposite direction is the one that
* matters, because it is silent.
*
* The last row is the one place a `#` does NOT start a comment: YAML requires
* whitespace before an inline `#`, so `major# note` is the scalar `major# note`
* and changesets throws `invalid version type`. The regex therefore spells the
* comment `(?:\s+#.*)?` rather than `(?:#.*)?` — matching YAML exactly, so this
* file misses only what changesets refuses.
*
* LEADING BLANK LINES (fixed in #6923). This parser used to require the fence on
* line 1 (`if (lines[0]?.trim() !== '---') return []`), so a changeset opening
* with one blank line declared, to this guard, nothing at all — while changesets
Expand All @@ -91,14 +101,24 @@
* blanks, and all three carry a comment saying the three read the same block —
* so this was also the one place that comment was false. It now skips them too.
*
* TRAILING YAML COMMENTS are still missed, and that is a KNOWN GAP recorded
* rather than implied: the entry regex ends `([A-Za-z]+)\s*$`, so
* `"@objectstack/spec": major # keep` matches nothing, while changesets reads it
* as a major. All three parsers in this family share the regex and therefore the
* gap, with a different consequence in each, so closing it is a family-wide
* change and not this file's to make alone. Filed as #7004; the fixture below
* pins the CURRENT behaviour so that closing it turns this file red on purpose
* rather than by surprise.
* TRAILING YAML COMMENTS were missed until #7004, together with two more shapes
* the same anchoring hid. The entry regex used to end `([A-Za-z]+)\s*$`, which
* accepts nothing after the bump word, so all of these read as no declaration at
* all while changesets read a real bump:
*
* "@objectstack/spec": major # keep a trailing comment
* "@objectstack/spec": "major" a QUOTED bump value (not in #7004's report)
* "@objectstack/spec": 'major' # keep both at once
*
* And one shape ran the other way — invented rather than hidden. A whole-line
* comment that happens to contain a colon is entry-shaped, so `# note: major`
* parsed as a package literally named `# note` bumped `major`. Measured against
* @changesets/parse@0.4.3, which declares nothing for it.
*
* All four parsers in this family shared the regex and therefore all four gaps,
* with a different consequence in each, so #7004 closed them family-wide in one
* change. Measured after: 19 shapes changesets ACCEPTS now agree, 0 regressions,
* and every surviving difference is on a file changesets throws on.
*
* ## RESIDUAL: an unreadable `.changeset/` still exits 0
*
Expand Down Expand Up @@ -143,11 +163,14 @@ const REPO_ROOT = resolve(__dirname, '..');
* A frontmatter line looks like: "@objectstack/spec": major
* (single or double quotes, any surrounding whitespace).
*
* The entry regex is deliberately the SAME shape `check-empty-changeset.mjs`
* and `check-adr-0087-registration.mjs` use. Three gates reading one block must
* agree on what counts as a declaration, or one of them is judging a different
* file than it appears to. See the dialect table in the header for where they
* agree with `@changesets/parse` and where they do not.
* The entry regex is deliberately the SAME shape `check-empty-changeset.mjs`,
* `check-adr-0087-registration.mjs` and `objectui-changeset-digest.mjs` use.
* Four readers of one block must agree on what counts as a declaration, or one
* of them is judging a different file than it appears to. That agreement is no
* longer only a comment: `check-empty-changeset.mjs`'s self-test extracts the
* regex literal from all four files and asserts they are byte-identical (#7004).
* See the dialect table in the header for where they agree with
* `@changesets/parse` and where they deliberately do not.
*
* @param {string} text
* @returns {string[]}
Expand All @@ -161,8 +184,10 @@ export function majorPackagesIn(text) {
const majors = [];
for (let j = i + 1; j < lines.length; j++) {
if (lines[j].trim() === '---') break; // end of frontmatter
if (/^\s*#/.test(lines[j])) continue; // a whole-line YAML comment declares nothing
// "<name>": <bump> | '<name>': <bump> | <name>: <bump>
const m = /^\s*["']?([^"':]+)["']?\s*:\s*([A-Za-z]+)\s*$/.exec(lines[j]);
// with an optionally quoted bump value and an optional trailing ` # comment`.
const m = /^\s*["']?([^"':]+)["']?\s*:\s*["']?([A-Za-z]+)["']?(?:\s+#.*)?\s*$/.exec(lines[j]);
if (m && m[2].toLowerCase() === 'major') majors.push(m[1].trim());
}
return majors;
Expand Down Expand Up @@ -428,15 +453,52 @@ function selfTest() {
'parser: lines that are not `<name>: <bump>` are not declarations',
);

// KNOWN GAP, pinned as current behaviour rather than endorsed. Measured:
// @changesets/parse@0.4.3 reads this as a real major. The entry regex ends
// `([A-Za-z]+)\s*$`, so the trailing comment defeats it — in all three parsers
// of this family, which is why closing it is not this file's change to make
// alone. When it IS closed, this assertion goes red on purpose: flip it, do
// not delete it.
// ── THE FIX (#7004): the shapes the old `([A-Za-z]+)\s*$` anchor hid ──────
//
// This block is #6923's KNOWN-GAP pin, FLIPPED rather than deleted, as the
// note it carried asked. It used to assert `.length === 0` — the gap — with
// the instruction to invert it on the day the family-wide regex was fixed.
// That day is #7004, so the same inputs are asserted to be CAUGHT now.
//
// Predicted direction on reverse verification: restoring the old anchor
// (`([A-Za-z]+)\s*$`) turns exactly these red. Measured with
// @changesets/parse@0.4.3: every one of them DOES release a major, so a miss
// here is a whole-stack major promoted past a guard that printed a tick.
caught('a trailing YAML comment', '---\n"@objectstack/spec": major # keep\n---\n\nbody\n', ['@objectstack/spec']);
caught('a trailing comment after a tab', '---\n"@objectstack/spec": major\t# keep\n---\n\nbody\n', ['@objectstack/spec']);
caught('a trailing comment containing a colon', '---\n"@objectstack/spec": major # note: keep\n---\n\nbody\n', ['@objectstack/spec']);
caught('an empty trailing comment', '---\n"@objectstack/spec": major #\n---\n\nbody\n', ['@objectstack/spec']);
caught('a double-quoted bump value', '---\n"@objectstack/spec": "major"\n---\n\nbody\n', ['@objectstack/spec']);
caught('a single-quoted bump value', '---\n"@objectstack/spec": \'major\'\n---\n\nbody\n', ['@objectstack/spec']);
caught('a quoted bump value AND a comment', '---\n"@objectstack/spec": "major" # keep\n---\n\nbody\n', ['@objectstack/spec']);
caught('a package name containing #, plus a comment', '---\n"@objectstack/a#b": major # keep\n---\n\nbody\n', ['@objectstack/a#b']);
caught('a commented major beside an uncommented minor', '---\n"@objectstack/a": major # keep\n"@objectstack/b": minor\n---\n\nbody\n', [
'@objectstack/a',
]);

// The other direction #7004 measured: a whole-line comment that happens to
// contain a colon is entry-shaped, and used to parse as a package literally
// named `# note`. @changesets/parse declares nothing for it, so neither does
// this. The control below is what keeps this from passing vacuously.
assert(
majorPackagesIn('---\n# note: major\n---\n\nbody\n').length === 0,
'parser: a whole-line YAML comment is not a declaration, even when it contains a colon (#7004)',
);
assert(
majorPackagesIn('---\n # note: major\n---\n\nbody\n').length === 0,
'parser: an INDENTED whole-line comment is not a declaration either (#7004)',
);
caught('control — a real entry beside a colon-bearing comment line', '---\n# note: major\n"@objectstack/real": major\n---\n\nbody\n', [
'@objectstack/real',
]);

// YAML requires whitespace before an inline `#`, so this one is the scalar
// `major# keep` and @changesets/parse THROWS `invalid version type`. Missing
// it is the harmless direction (a file that can version nothing), and the
// regex spells the comment `(?:\s+#.*)?` precisely to keep it that way.
assert(
majorPackagesIn('---\n"@objectstack/spec": major # keep\n---\n\nbody\n').length === 0,
'parser: KNOWN GAP (#7004) — a trailing YAML comment hides a major from this parser (changesets reads it as a major); flip this when the family-wide regex is fixed, never delete it',
majorPackagesIn('---\n"@objectstack/spec": major# keep\n---\n\nbody\n').length === 0,
'parser: `major# keep` (no space before #) is not a comment in YAML — changesets throws on it, so missing it is the harmless direction (#7004)',
);

// ── The exemption switch, in BOTH directions ──────────────────────────────
Expand Down
Loading
Loading