diff --git a/.changeset/vale-3-21-0.md b/.changeset/vale-3-21-0.md new file mode 100644 index 00000000..f32b7d1c --- /dev/null +++ b/.changeset/vale-3-21-0.md @@ -0,0 +1,21 @@ +--- +"@taskless/cli": patch +--- + +Update the bundled Vale to 3.21.0. + +For a rule under `.taskless/rules/vale/`, what you can now write: + +- `scope: doc()` picks part of a document by CSS selector, where a heading and everything under it is a `section`: `text & doc(section:has(> h2:contains("Decision")))` is one section's prose, `~doc(...)` is everything outside it, and a `metric` scoped to `doc(...)` puts a word budget on that section alone. A leaf element on its own (`doc(h2)`) is inert; chain it (`text & doc(h2)`). `verify` accepts the family and leaves the selector to Vale, which rejects one it cannot compile at load. +- A `metric` honors its `scope`: `scope: sentence` measures each sentence rather than the whole document. An absent scope, or `scope: text`, still measures the document. +- `.ipynb` is a format: Markdown cells are read as Markdown, code cells as their kernel's comments, raw cells and outputs not at all. Findings point at the notebook file's own lines. +- `BlockIgnores` and `TokenIgnores` in a rule's `.vale.ini` now apply under an `[*.html]` matcher. + +What changes for a rule you already have: + +- A `[glob]` section repeated in a rule's `.vale.ini`, or a key repeated inside one, now keeps its last assignment rather than its first. Precedence is last-wins in both directions, so a disable placed after the enable it narrows works in every shape. +- An unknown `action` name is refused when the rule loads, and one config serves the whole run, so a typo there fails every Vale rule in the project. `verify` now rejects a name outside `replace`, `remove`, `suggest`, `convert`, `edit`. +- A `metric` that declared a `scope` was measuring the whole document anyway; it now measures what the scope names, so its findings move. +- A rule matching `[*.ipynb]` was linting the notebook's JSON; it now reads cells, so findings from outputs and metadata are gone. + +`taskless agent update` carries the same list, with what to do about each. diff --git a/packages/cli/package.json b/packages/cli/package.json index 4e8e933e..4beb79b6 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -89,11 +89,11 @@ "@ast-grep/cli-win32-arm64-msvc": "0.45.3", "@ast-grep/cli-win32-ia32-msvc": "0.45.3", "@ast-grep/cli-win32-x64-msvc": "0.45.3", - "@taskless/vale-darwin-arm64": "3.20.0-20260907164938", - "@taskless/vale-darwin-x64": "3.20.0-20260907164938", - "@taskless/vale-linux-arm64": "3.20.0-20260907164938", - "@taskless/vale-linux-x64": "3.20.0-20260907164938", - "@taskless/vale-win32-arm64": "3.20.0-20260907164938", - "@taskless/vale-win32-x64": "3.20.0-20260907164938" + "@taskless/vale-darwin-arm64": "3.21.0-20260915061224", + "@taskless/vale-darwin-x64": "3.21.0-20260915061224", + "@taskless/vale-linux-arm64": "3.21.0-20260915061224", + "@taskless/vale-linux-x64": "3.21.0-20260915061224", + "@taskless/vale-win32-arm64": "3.21.0-20260915061224", + "@taskless/vale-win32-x64": "3.21.0-20260915061224" } } diff --git a/packages/cli/scripts/generate-vale-schema.ts b/packages/cli/scripts/generate-vale-schema.ts index 5cc5efe2..657855cd 100644 --- a/packages/cli/scripts/generate-vale-schema.ts +++ b/packages/cli/scripts/generate-vale-schema.ts @@ -471,6 +471,10 @@ const FIELD_CANDIDATES: readonly string[] = [ "chars", "pattern", "tag", + // New in Vale 3.21.0: `conditional` looks for its consequent in another + // View's scope. Offered to every check, like every other candidate, so the + // partition records which checks own it rather than assuming one does. + "in", ].toSorted(); /** @@ -1022,6 +1026,22 @@ const SCOPE_CANDIDATES: readonly ScopeCandidate[] = [ documented: true, note: "the same operand in the TypeScript tier.", }, + { + operand: "doc(section)", + fixture: "# Do bogus things\n\nFine.\n", + ext: "md", + documented: true, + prefix: "doc(", + note: + "new in Vale 3.21.0: a CSS selector over the document's elements. The " + + "tail is a selector, so this is a family. The schema owns the closing " + + "paren; the selector's syntax is Vale's to reject (E201 at load). " + + "Probed over `section` (a heading plus what follows it) rather than " + + "`h1` because a standalone term lints what is INSIDE the element as " + + "one block, and a leaf element has nothing inside it: `doc(h1)` alone " + + "is measured silent where `text & doc(h1)` fires. `test/vale-corpus.ts` " + + "carries both shapes.", + }, { operand: "fenced", fixture: "Prose bogus.\n\n```\nbogus fenced\n```\n", diff --git a/packages/cli/src/agent/create-vale-rule.md b/packages/cli/src/agent/create-vale-rule.md index b937dfa8..3f95e1eb 100644 --- a/packages/cli/src/agent/create-vale-rule.md +++ b/packages/cli/src/agent/create-vale-rule.md @@ -1,4 +1,4 @@ -# Topic: create-vale-rule (CLI v%(CLI_VERSION)s / topic v7) +# Topic: create-vale-rule (CLI v%(CLI_VERSION)s / topic v8) ## You are here This is `create-vale-rule`. It helps you write a Vale rule: a check over @@ -82,7 +82,7 @@ it. | a word repeated back to back | `repetition` | | picking one of two acceptable spellings, consistently | `consistency` | | "if X appears, Y must also appear" | `conditional` | -| a document-level length or ratio threshold | `metric` | +| a length or ratio threshold, over the document or one `scope` of it | `metric` | | a readability grade, against a named formula | `readability` | | a misspelling, against a dictionary | `spelling` | | phrases that must appear in a fixed order | `sequence` | @@ -185,6 +185,33 @@ it. | `comment` | every comment, in a comment-tier format | | `comment.line` | `//`-style comments | | `comment.block` | `/* … */`-style comments | +| `doc()` | elements matched by a CSS selector; see below | + + **`doc()` picks part of a document by CSS selector**, the + same way in every markup format, and a heading with everything under + it is a `section`, so one section of a document is + `doc(section:has(> h2:contains("Decision")))`. Chain it to narrow an + ordinary scope to that element: `text & doc(...)` is prose inside it, + `sentence & doc(...)` one sentence at a time inside it, `~doc(...)` is + everything outside it. On its own, `doc(...)` lints what is INSIDE the + element as one block, which is what `occurrence` (a section must say + "we will") and `metric` (a section runs over budget) want. Measured: a + `metric` with `scope: doc(section:has(> h2:contains("Consequences")))` + and `formula: words` counts that section's words, not the document's. + + **A leaf element on its own is inert.** `doc(h2)` alone selects a + heading, and a heading has nothing inside it to lint as a block, so the + rule matches nothing, with no error anywhere. Write `text & doc(h2)` + for the heading's own text. The same holds for `doc(p)` and `doc(li)`. + `verify` accepts both spellings, because telling a leaf from a container + needs the document; `test` shows which one fires. + + **The selector is Vale's to check, not `verify`'s.** A selector Vale + cannot compile (`doc(h2[)`) fails the whole run at load with + `E201 invalid selector in 'doc(...)'`, which `test` reports. A selector + that compiles and matches nothing is silent, like any scope with no + construct to find. `verify` checks that the term is `doc(` … `)` with + something between, and no more. **`raw` subsumes `code` and `text`.** Measured on one document holding the token in prose, in an inline span, and in a fenced block: `text` diff --git a/packages/cli/src/agent/update.md b/packages/cli/src/agent/update.md index f9f8ef26..09f6dfaf 100644 --- a/packages/cli/src/agent/update.md +++ b/packages/cli/src/agent/update.md @@ -1,4 +1,4 @@ -# Topic: update (CLI v%(CLI_VERSION)s / topic v5) +# Topic: update (CLI v%(CLI_VERSION)s / topic v6) ## You are here This is `update`. It tells you what an upgrade changed for the rules @@ -227,6 +227,72 @@ leaked text into people's files. (ast-grep/ast-grep#2868), but no shape we tried reproduced a difference, including the TSX case that PR names. Nothing to do unless you see one. +### Migrating to 0.11.2 + +Vale moves from 3.20.0 to 3.21.0, and ast-grep from 0.45.2 to 0.45.3. +Nothing installed migrates for ast-grep: its one user-visible change is +that an inline `ast-grep-ignore` comment takes effect only as the +comment's first alphabetic text, which the 0.45.3 changeset describes +and no rule file carries. Six things follow for existing Vale rules, +every one measured against both binaries; the last is behaviour a rule +can now use, not a change to one it has. + +**A duplicated matcher now keeps its LAST assignment.** Where a rule's +`.vale.ini` repeats a `[glob]` section, or repeats a key inside one, +3.20.0 kept the first value and 3.21.0 keeps the last (upstream 1e4f6ed, +"let the project's rule settings win"). Precedence across different +matchers was already last-wins, so the two directions now agree, and the +guidance to put a disable AFTER the enable it narrows is right for both +shapes. A rule that relied on the old order is one whose second +assignment was being ignored; it now takes effect, and the rule turns on +or off where it did not before. `git grep -c '^\[' .taskless/rules/vale` +finds a config with more than one section per rule to read. + +**A `metric` rule's `scope` is honored.** Through 3.20.0 a `metric` +measured the whole document whatever its `scope` said; 3.21.0 measures +the blocks the scope names, and only an absent scope, or `scope: text`, +still means the document. A `metric` with `scope: sentence` or +`scope: paragraph` now reports per block, so its findings move, appear, +or disappear. If the document-wide reading was what you wanted, delete +the `scope`. + +**Notebooks are read cell by cell.** `.ipynb` had no parser, so a rule +matching `[*.ipynb]` was linting the notebook's JSON: cell source, +outputs, and metadata alike. 3.21.0 reads a Markdown cell as Markdown +and a code cell as its kernel's comments, and reads raw cells and +outputs not at all. Findings drop, and `Line` now points into the +notebook file. Nothing warns; a suspiciously clean run over notebooks is +this. + +**An unknown `action` name fails the run at load.** Measured on +3.20.0, a rule carrying `action: {name: bogus}` loaded, and the run +died only when that rule fired, as an `E100` with no file and no line. +3.21.0 refuses it when the rule loads, as an `E201` naming the file, so +a rule that has been carrying a bad action without ever matching now +fails every check, not just the ones where it matched. `verify` rejects +a name outside `replace`, `remove`, `suggest`, `convert`, `edit`, so run +it: the rule is named directly. + +**`BlockIgnores` and `TokenIgnores` apply to HTML.** A rule's +`.vale.ini` carrying either key under an `[*.html]` matcher was ignored +through 3.20.0 and takes effect on 3.21.0. Findings inside the ignored +region disappear. + +Upstream also says a `sequence` rule with a negated scope (`~list`, +`~code`) reported every match twice (vale-cli/vale#1169). No shape we +tried reproduced a difference between the two binaries, including the +one that PR's own test uses, so it is recorded the way the ast-grep +root-metavariable case is: nothing to do unless you see one. + +**`scope: doc()` selects part of a document by CSS selector**, +which a rule could not do before: `text & doc(section:has(> h2:contains("Decision")))` +is the prose of one section, `~doc(...)` is everything outside it, and +a `metric` scoped to `doc(...)` puts a word budget on that section +alone. `%(TASKLESS_CLI)s agent create-vale-rule` has the shapes and the +one trap (a leaf element on its own, `doc(h2)`, is inert; chain it). +No existing rule changes; this is a reason to revisit one that was +narrowed by hand. + ## Errors With `--json`, `--rules` failures emit `{ ok: false, code, message }`: diff --git a/packages/cli/src/generated/vale-vocabulary-report.md b/packages/cli/src/generated/vale-vocabulary-report.md index 69cfabc9..dc9bb6ff 100644 --- a/packages/cli/src/generated/vale-vocabulary-report.md +++ b/packages/cli/src/generated/vale-vocabulary-report.md @@ -1,10 +1,10 @@ -# Vale 3.20.0 vocabulary: divergence report +# Vale 3.21.0 vocabulary: divergence report GENERATED FILE — DO NOT EDIT. Produced by `pnpm generate:vale-schema` alongside `vale-vocabulary.ts`. Every value in the vocabulary is the recorded answer of the vendored Vale -3.20.0 binary. This file is what the binary said that its own +3.21.0 binary. This file is what the binary said that its own documentation does not, in both directions. A generator that dropped these would be quietly deciding which of the two to believe. @@ -28,25 +28,25 @@ the worse failure. ### `scope: meta` -Vale 3.20.0 documents this operand and it never fired, on any fixture probed (.md). +Vale 3.21.0 documents this operand and it never fired, on any fixture probed (.md). **Consequence.** It is omitted from the vocabulary, so `verify` rejects it. A rule written from the documentation would otherwise load, run, and match nothing, with no error reported anywhere. ### `scope: meta.class.title` -Vale 3.20.0 documents this operand and it never fired, on any fixture probed (.md). +Vale 3.21.0 documents this operand and it never fired, on any fixture probed (.md). **Consequence.** It is omitted from the vocabulary, so `verify` rejects it. A rule written from the documentation would otherwise load, run, and match nothing, with no error reported anywhere. ### `scope: frontmatter` -This operand fired and Vale 3.20.0 documents it nowhere. +This operand fired and Vale 3.21.0 documents it nowhere. **Consequence.** It is included in the vocabulary. It is also the standing counterexample to trusting the candidate list: a real operand nobody proposes is simply absent, and the schema then rejects a rule the binary honors. ### `scope: frontmatter.title` -This operand fired and Vale 3.20.0 documents it nowhere. +This operand fired and Vale 3.21.0 documents it nowhere. **Consequence.** It is included in the vocabulary. It is also the standing counterexample to trusting the candidate list: a real operand nobody proposes is simply absent, and the schema then rejects a rule the binary honors. @@ -64,7 +64,7 @@ This check accepted 'taskless_generator_sentinel', a key no check has. It does n ### `field probes: membership inferred from a type complaint` -10 probes drew an E201 that was not an invalid-key list: capitalization.action: expected a map, got 'bool'; conditional.action: expected a map, got 'bool'; existence.action: expected a map, got 'bool'; metric.action: expected a map, got 'bool'; occurrence.action: expected a map, got 'bool'; readability.action: expected a map, got 'bool'; repetition.action: expected a map, got 'bool'; script.action: expected a map, got 'bool'; sequence.action: expected a map, got 'bool'; substitution.action: expected a map, got 'bool'. +11 probes drew an E201 that was not an invalid-key list: capitalization.action: expected a map, got 'bool'; conditional.action: expected a map, got 'bool'; conditional.in: no View defines a scope named '1'; existence.action: expected a map, got 'bool'; metric.action: expected a map, got 'bool'; occurrence.action: expected a map, got 'bool'; readability.action: expected a map, got 'bool'; repetition.action: expected a map, got 'bool'; script.action: expected a map, got 'bool'; sequence.action: expected a map, got 'bool'; substitution.action: expected a map, got 'bool'. **Consequence.** Each is recorded as a member: Vale recognized the key and objected to the probe's arbitrary value instead, which is membership evidence. They are listed so the inference is auditable rather than assumed. @@ -108,5 +108,6 @@ This check accepted 'taskless_generator_sentinel', a key no check has. It does n | `comment.block` | `.js` | yes | fires | | `comment.line` | `.ts` | yes | fires | | `comment.block` | `.ts` | yes | fires | +| `doc(section)` | `.md` | yes | fires | | `fenced` | `.md` | no | silent | | `banana` | `.md` | no | silent | diff --git a/packages/cli/src/generated/vale-vocabulary.ts b/packages/cli/src/generated/vale-vocabulary.ts index 9347bbff..d15dd834 100644 --- a/packages/cli/src/generated/vale-vocabulary.ts +++ b/packages/cli/src/generated/vale-vocabulary.ts @@ -6,7 +6,7 @@ * `scripts/generate-vale-schema.ts`, and its header explains what each value * below was measured with and what it is worth. * - * Derived against Vale 3.20.0. Every value here is the recorded answer + * Derived against Vale 3.21.0. Every value here is the recorded answer * of that binary to a rule the generator wrote and ran; nothing is transcribed * from documentation. Where the binary and the documentation disagree, the * disagreement is in `vale-vocabulary-report.md` rather than dropped. @@ -15,7 +15,7 @@ */ /** The binary this vocabulary was derived from. */ -export const VALE_VOCABULARY_VERSION = "3.20.0"; +export const VALE_VOCABULARY_VERSION = "3.21.0"; /** * Vale's check types, self-enumerated: an unknown `extends` makes the binary @@ -118,7 +118,7 @@ export const VALE_CHECK_FIELDS = { "threshold", "vocab", ], - conditional: ["exceptions", "first", "ignorecase", "second", "vocab"], + conditional: ["exceptions", "first", "ignorecase", "in", "second", "vocab"], existence: [ "append", "exceptions", @@ -190,10 +190,14 @@ export const VALE_SCOPE_OPERANDS = [ * `text.class.callout` names an HTML class. Rejecting an unfamiliar tail would * be the too-strict failure against a value the binary honors. */ -export const VALE_SCOPE_PREFIXES = ["frontmatter.", "text.class."] as const; +export const VALE_SCOPE_PREFIXES = [ + "doc(", + "frontmatter.", + "text.class.", +] as const; /** - * Where Vale 3.20.0 and its documentation disagree. + * Where Vale 3.21.0 and its documentation disagree. * * Carried in the artifact rather than only in the report, so that a consumer * can render them and a reviewer cannot miss them in a diff. @@ -202,26 +206,26 @@ export const VALE_DIVERGENCES = [ { subject: "scope: meta", finding: - "Vale 3.20.0 documents this operand and it never fired, on any fixture probed (.md).", + "Vale 3.21.0 documents this operand and it never fired, on any fixture probed (.md).", consequence: "It is omitted from the vocabulary, so `verify` rejects it. A rule written from the documentation would otherwise load, run, and match nothing, with no error reported anywhere.", }, { subject: "scope: meta.class.title", finding: - "Vale 3.20.0 documents this operand and it never fired, on any fixture probed (.md).", + "Vale 3.21.0 documents this operand and it never fired, on any fixture probed (.md).", consequence: "It is omitted from the vocabulary, so `verify` rejects it. A rule written from the documentation would otherwise load, run, and match nothing, with no error reported anywhere.", }, { subject: "scope: frontmatter", - finding: "This operand fired and Vale 3.20.0 documents it nowhere.", + finding: "This operand fired and Vale 3.21.0 documents it nowhere.", consequence: "It is included in the vocabulary. It is also the standing counterexample to trusting the candidate list: a real operand nobody proposes is simply absent, and the schema then rejects a rule the binary honors.", }, { subject: "scope: frontmatter.title", - finding: "This operand fired and Vale 3.20.0 documents it nowhere.", + finding: "This operand fired and Vale 3.21.0 documents it nowhere.", consequence: "It is included in the vocabulary. It is also the standing counterexample to trusting the candidate list: a real operand nobody proposes is simply absent, and the schema then rejects a rule the binary honors.", }, @@ -242,7 +246,7 @@ export const VALE_DIVERGENCES = [ { subject: "field probes: membership inferred from a type complaint", finding: - "10 probes drew an E201 that was not an invalid-key list: capitalization.action: expected a map, got 'bool'; conditional.action: expected a map, got 'bool'; existence.action: expected a map, got 'bool'; metric.action: expected a map, got 'bool'; occurrence.action: expected a map, got 'bool'; readability.action: expected a map, got 'bool'; repetition.action: expected a map, got 'bool'; script.action: expected a map, got 'bool'; sequence.action: expected a map, got 'bool'; substitution.action: expected a map, got 'bool'.", + "11 probes drew an E201 that was not an invalid-key list: capitalization.action: expected a map, got 'bool'; conditional.action: expected a map, got 'bool'; conditional.in: no View defines a scope named '1'; existence.action: expected a map, got 'bool'; metric.action: expected a map, got 'bool'; occurrence.action: expected a map, got 'bool'; readability.action: expected a map, got 'bool'; repetition.action: expected a map, got 'bool'; script.action: expected a map, got 'bool'; sequence.action: expected a map, got 'bool'; substitution.action: expected a map, got 'bool'.", consequence: "Each is recorded as a member: Vale recognized the key and objected to the probe's arbitrary value instead, which is membership evidence. They are listed so the inference is auditable rather than assumed.", }, diff --git a/packages/cli/src/rules/assemble.ts b/packages/cli/src/rules/assemble.ts index 10773dea..6188f05c 100644 --- a/packages/cli/src/rules/assemble.ts +++ b/packages/cli/src/rules/assemble.ts @@ -19,11 +19,13 @@ import { RULE_TESTS_DIRECTORY, RULES_DIRECTORY } from "./layout"; * generated here and gitignored. * * **Determinism is a correctness constraint, not tidiness.** Vale's matcher - * precedence is positional — across matchers the last wins, within one matcher - * the first assignment wins — so a config assembled in directory-iteration - * order would give a rule a different effective scope depending on the machine - * it ran on. Rules are therefore emitted in sorted id order, and each rule's own - * matcher order is preserved verbatim. + * precedence is positional — across matchers the last wins, and since Vale + * 3.21.0 so does the last assignment within one matcher (through 3.20.0 it was + * the first; `vale-vendor-contract.test.ts` pins the current answer) — so a + * config assembled in directory-iteration order would give a rule a different + * effective scope depending on the machine it ran on. Rules are therefore + * emitted in sorted id order, and each rule's own matcher order is preserved + * verbatim. * * A consequence worth naming: a rule cannot override another rule's matchers, * because it cannot know its own position in the assembled file. That coupling diff --git a/packages/cli/src/rules/capabilities.ts b/packages/cli/src/rules/capabilities.ts index d64a2bbd..036e030a 100644 --- a/packages/cli/src/rules/capabilities.ts +++ b/packages/cli/src/rules/capabilities.ts @@ -209,7 +209,7 @@ export const AST_GREP_TSX_SPLIT: Readonly< * Pinned against the binary by `test/vale-vendor-contract.test.ts` * ("engine capabilities" → "reports the pinned version"). */ -export const VALE_VERSION = "3.20.0"; +export const VALE_VERSION = "3.21.0"; /** * Which tier Vale routes an extension to. @@ -271,6 +271,23 @@ const CONVERTER_TIER_PREFIX = "converter:"; * plain text today, but the moment Vale routes it to a converter the same * omission is a crash that takes down every Vale rule in the run. * + * 3.20.0 → 3.21.0 LEARNED ONE FORMAT, AND THE SOURCE CHECK IS WHAT FOUND IT. + * Every existing row was re-probed against the 3.21.0 binary and none moved. + * The v3.20.0...v3.21.0 tree adds `internal/lint/notebook.go` and routes + * `.ipynb` to it from `lintFile`'s markup switch, so Jupyter notebooks left + * the unnamed plaintext fallback for `markup` and are a new row below. The + * benign direction again, and a narrowing: on 3.20.0 a notebook was linted as + * the JSON it is, so a rule matching `[*.ipynb]` fired on cell source, on + * outputs, and on metadata alike. On 3.21.0 a Markdown cell is read as + * Markdown and a code cell as its kernel's language (comments only); raw + * cells and outputs are not read at all, and `Line`/`Span` point into the + * notebook file. Findings drop, nothing warns. Measured on the pinned binary, + * and pinned in `test/vale-vendor-contract.test.ts`. + * + * Also in that tree, and not a tier change: `internal/lint/selection.go` (the + * `doc(...)` scope, see `src/schemas/vale-rule.ts`), and `html.go` now applies + * `BlockIgnores`/`TokenIgnores` to `.html`, where 3.20.0 ignored both keys. + * * NOTHING MOVED ACROSS 3.19.0 → 3.20.0, AND THE SECOND HALF OF THAT CLAIM IS * THE ONE THAT COST SOMETHING. Every row below was re-probed against the * 3.20.0 binary and none of them moved, which this table can report. It cannot @@ -335,6 +352,7 @@ export const VALE_FORMAT_TIERS: Readonly> = { // markup — parsed, the format's own constructs skipped ".htm": "markup", ".html": "markup", + ".ipynb": "markup", ".markdown": "markup", ".md": "markup", ".mdx": "markup", @@ -395,7 +413,7 @@ export const VALE_FORMAT_TIERS: Readonly> = { ".mkdn": "plaintext", ".tex": "plaintext", // plaintext HERE, though Vale's own docs list it as comment-tier. Measured on - // the pinned 3.20.0 binary a bare non-comment line lints, which is the + // the pinned 3.21.0 binary a bare non-comment line lints, which is the // plaintext signature. `.qml` and `.scss` sat here for the same reason until // 3.18.0 made the docs true for them; `.pyi` is the row where transcribing // the docs would still ship the wrong tier — the case for probing rather than diff --git a/packages/cli/src/rules/vale/map.ts b/packages/cli/src/rules/vale/map.ts index 09343b8f..dbb63b2d 100644 --- a/packages/cli/src/rules/vale/map.ts +++ b/packages/cli/src/rules/vale/map.ts @@ -18,6 +18,13 @@ export interface ValeFinding { Span: [number, number]; Match: string; Action?: { Name?: string; Params?: string[] | null }; + /** + * Replacement text Vale computed for the match (3.21.0+). Measured equal + * to a `replace` action's `Params` and `[]` otherwise; `toFix` reads + * `Action`, the stricter of the two, and the vendor contract pins that they + * agree. + */ + Suggestions?: string[]; } /** Vale's whole payload: findings keyed by the path they were found in. */ diff --git a/packages/cli/src/schemas/vale-rule.ts b/packages/cli/src/schemas/vale-rule.ts index abfbdc93..5bd92d41 100644 --- a/packages/cli/src/schemas/vale-rule.ts +++ b/packages/cli/src/schemas/vale-rule.ts @@ -181,6 +181,25 @@ const SCOPE_OPERANDS = new Set(DERIVED_SCOPE_OPERANDS); */ const SCOPE_PREFIXES: readonly string[] = VALE_SCOPE_PREFIXES; +/** + * The one family whose tail is bracketed rather than dotted. + * + * `doc()` (Vale 3.21.0) names document elements by CSS selector. + * The generator records it as a prefix like the dotted families, because + * that is the shape its measurement takes, but the grammar around it differs + * in two ways this module has to know about: the term ends in `)`, and the + * selector between may itself contain `&`, `.` and `~`, which are the + * selector's own and not this grammar's. Both are Vale's rules, from + * `internal/check/scope.go` — `docSelection` and `splitOutside`. + * + * What is between the parens is NOT checked here. Vale compiles every + * selector at load and reports a bad one as `E201 invalid selector in + * 'doc(...)'`, so an author hears about it from `test`, the same way a + * malformed regex in `tokens` is reported. Parsing CSS here would buy a + * dependency to duplicate a check the binary already makes loudly. + */ +const DOC_PREFIX = "doc("; + /** * Operands, for the message `verify` shows an author. * @@ -190,14 +209,74 @@ const SCOPE_PREFIXES: readonly string[] = VALE_SCOPE_PREFIXES; */ export const VALE_SCOPE_OPERANDS: readonly string[] = [ ...SCOPE_OPERANDS, - ...SCOPE_PREFIXES.map((prefix) => `${prefix}`), + ...SCOPE_PREFIXES.map((prefix) => + prefix === DOC_PREFIX ? `${prefix})` : `${prefix}` + ), ].toSorted(); function isScopeOperand(operand: string): boolean { if (SCOPE_OPERANDS.has(operand)) return true; - return SCOPE_PREFIXES.some( - (prefix) => operand.startsWith(prefix) && operand.length > prefix.length - ); + return SCOPE_PREFIXES.some((prefix) => { + if (!operand.startsWith(prefix)) return false; + if (prefix === DOC_PREFIX) { + // `doc()` is rejected by Vale at load ("expected selector, found EOF") + // and `doc(h1` is read as a dotted operand Vale does not have. Neither + // is a scope, so neither is accepted. + return ( + operand.endsWith(")") && operand.slice(prefix.length, -1).trim() !== "" + ); + } + return operand.length > prefix.length; + }); +} + +/** + * Split a scope on `&`, leaving alone any `&` inside parentheses or quotes. + * + * A port of Vale's `splitOutside`, because a `doc(...)` selector may contain + * the separator — `doc(a:has(> b)) & text` must split into two terms, and + * `doc(a[title="x & y"])` into one. A plain `split("&")` cut the second in + * half and reported a selector fragment as an unknown operand. + */ +function splitOutside(scope: string): string[] { + const parts: string[] = []; + let quote: string | undefined; + let depth = 0; + let start = 0; + for (let index = 0; index < scope.length; index++) { + const char = scope[index]; + if (quote !== undefined) { + if (char === quote) quote = undefined; + continue; + } + switch (char) { + case '"': + case "'": { + quote = char; + break; + } + case "(": { + depth++; + break; + } + case ")": { + if (depth > 0) depth--; + break; + } + case "&": { + if (depth === 0) { + parts.push(scope.slice(start, index)); + start = index + 1; + } + break; + } + default: { + break; + } + } + } + parts.push(scope.slice(start)); + return parts; } const scopeVocabulary = @@ -224,7 +303,7 @@ const scopeVocabulary = */ function scopeMessages(scope: string): string[] { const messages: string[] = []; - for (const part of scope.split("&")) { + for (const part of splitOutside(scope)) { const negated = part.trim().startsWith("~"); const operand = part.trim().replace(/^~/, "").trim(); if (operand === "") { @@ -538,6 +617,77 @@ function fatalShapeMessages( return fatal; } +/** + * The action names Vale 3.21.0 accepts at load. + * + * Transcribed, not derived, and the provenance is `checkAction` in upstream + * `internal/check/definition.go` (commit e0a2250d, "validate actions at + * load"). There is no oracle to derive it from: an unknown name draws + * `E201 unknown action ''`, which names the bad one and never the + * accepted set, the same limit the field tables have. `test/vale-corpus.ts` + * holds each name to the binary, so a name that stops loading fails there. + * + * Worth a check at all because of what changed. Measured on 3.20.0, a rule + * with an unknown action LOADED, and the run died only when that rule fired, + * as an `E100` with no file and no line. 3.21.0 validates the action when the + * rule loads, as an `E201` naming the file: one config for the whole run, so + * a typo in one rule's `action` now suppresses every other Vale rule's + * findings on every check, not only on the ones where the rule matched. That + * is the blast radius this module exists to catch before the assembled + * config is ever handed over. + * + * Compared case-sensitively, which is the binary's behaviour and not an + * oversight: the action map's KEY is decoded case-insensitively (`Name:` is + * `name:`), but the VALUE is matched verbatim — measured, `Replace` draws + * `E201 unknown action 'Replace'`. `test/vale-corpus.ts` pins both halves + * (`action/mixed-case-name-key`, `action/mixed-case-name-value`). + * + * Only the name is checked. `suggest`, `convert` and `edit` also constrain + * their `params`, and Vale reports those at load too, but each is a shape of + * its own and none is something the recipe teaches; the name is the part an + * author can typo. + */ +const ACTION_NAMES: readonly string[] = [ + "replace", + "remove", + "suggest", + "convert", + "edit", +]; + +function actionMessages( + rule: Record +): { path: PropertyKey[]; message: string }[] { + const { action } = rule; + if (typeof action !== "object" || action === null || Array.isArray(action)) { + // Not a map: Vale reports `expected a map` itself, as an E201 the field + // tables already let through as a value error rather than a key error. + return []; + } + // The action map is decoded case-insensitively, like a check's own fields. + const nameKey = Object.keys(action).find( + (key) => key.toLowerCase() === "name" + ); + const name = + nameKey === undefined + ? undefined + : (action as Record)[nameKey]; + if (typeof name !== "string" || name === "" || ACTION_NAMES.includes(name)) { + return []; + } + return [ + { + path: ["action", "name"], + message: + `action name ${JSON.stringify(name)} is not one Vale ` + + `${PINNED_VALE_VERSION} has. Since 3.21.0 Vale checks the action when ` + + `the rule loads rather than when it fires, and reports this as E201 — ` + + `one config for the whole run, so every other Vale rule's findings ` + + `are suppressed with it. Accepted: ${ACTION_NAMES.join(", ")}.`, + }, + ]; +} + /** * The union's members, spelled out, and the guard that keeps them honest. * @@ -639,7 +789,10 @@ const valeBodySchema = z ) .check((context) => { const rule = context.value as Record; - for (const { path, message } of fatalShapeMessages(rule)) { + for (const { path, message } of [ + ...fatalShapeMessages(rule), + ...actionMessages(rule), + ]) { context.issues.push({ code: "custom", input: rule, path, message }); } }); diff --git a/packages/cli/test/reconcile-marker.test.ts b/packages/cli/test/reconcile-marker.test.ts index 30c6f819..14c81425 100644 --- a/packages/cli/test/reconcile-marker.test.ts +++ b/packages/cli/test/reconcile-marker.test.ts @@ -86,7 +86,7 @@ describe("recording a rules reconciliation", () => { expect(rules?.reconciledTo).toBe(version); // Engine versions are the input a later differential needs. Recorded here // and nowhere else, so an upgrade cannot silently refresh them. - expect(rules?.engines).toEqual({ sg: "0.45.3", vale: "3.20.0" }); + expect(rules?.engines).toEqual({ sg: "0.45.3", vale: "3.21.0" }); }); it("reports the marker through info", async () => { @@ -266,6 +266,17 @@ describe("taskless update with no flags", () => { expect(result.stdout).toContain("Matching semantics moved"); }); + it("carries the 0.11.2 ledger entry", async () => { + const result = await runCli(["update"]); + expect(result.stdout).toContain("Migrating to 0.11.2"); + // The four things an author cannot discover from the diff: each is a + // change in what an unchanged rule reports, with no error anywhere. + expect(result.stdout).toContain("keeps its LAST assignment"); + expect(result.stdout).toContain("`metric` rule's `scope` is honored"); + expect(result.stdout).toContain("Notebooks are read cell by cell"); + expect(result.stdout).toContain("`action` name fails the run at load"); + }); + it("warns that kind: link is loud, not a silent zero-match", async () => { // The correction that took a verified 0.45.2 binary to establish: an // invalid kind aborts config parsing and takes every other rule down. diff --git a/packages/cli/test/vale-corpus.ts b/packages/cli/test/vale-corpus.ts index ac67a113..e72082c4 100644 --- a/packages/cli/test/vale-corpus.ts +++ b/packages/cli/test/vale-corpus.ts @@ -115,6 +115,14 @@ const JS_COMMENTS = "// simply line\n/*\n simply block\n*/\nconst x = 1;\n"; const heading = (level: number): string => `${"#".repeat(level)} Level ${String(level)} simply heading\n`; +/** + * Two `h2` sections, with `simply` in exactly one of them, so a `doc(...)` + * that selects the Context section fires and one that selects Decision does + * not — which is what lets a negation be told from a selection. + */ +const DOC_SECTIONS = + "Intro.\n\n## Context\n\nWe simply here.\n\n## Decision\n\nWe will decide.\n"; + // --- The corpus -------------------------------------------------------------- /** @@ -353,6 +361,31 @@ const SCOPES: ValeCorpusEntry[] = [ // every one of these, which is worse than the gap the schema closes. { name: "scope/negation", scope: "~code", control: MIXED }, { name: "scope/chain", scope: "text & ~code", control: MIXED }, + // `doc()`, new in Vale 3.21.0: elements of the document by CSS + // selector, where a heading and everything under it is a `section`. Four + // shapes, because they behave differently and the schema accepts all four: + // a container on its own is linted as one block; `text &` narrows blocks to + // those inside the element; `~` is everything outside it; and a selector + // may carry the grammar's own `&` inside quotes, which the split must leave + // alone. The standalone LEAF case is in INVALID_SCOPES below — it is the + // trap. + { name: "scope/doc-container", scope: "doc(section)", control: HEADINGS }, + { name: "scope/doc-chain", scope: "text & doc(h1)", control: HEADINGS }, + { + name: "scope/doc-sentence-in-section", + scope: 'sentence & doc(section:has(> h2:contains("Context")))', + control: DOC_SECTIONS, + }, + { + name: "scope/doc-negation", + scope: '~doc(section:has(> h2:contains("Decision")))', + control: DOC_SECTIONS, + }, + { + name: "scope/doc-ampersand-inside-selector", + scope: 'text & doc(section:not([data-x="a & b"]))', + control: DOC_SECTIONS, + }, ].map(({ name, scope, control, ext }) => ({ name, construct: `scope: ${scope}`, @@ -396,6 +429,10 @@ const INVALID_SCOPES: ValeCorpusEntry[] = [ // exist: the v3.18.0 addition is `frontmatter`, above. { name: "scope/meta", scope: "meta", control: FRONTMATTER }, { name: "scope/meta.class", scope: "meta.class.title", control: FRONTMATTER }, + // A `doc(` with no closing paren is not a selection: Vale reads the whole + // term as a dotted operand it does not have, and the rule is inert. The + // schema rejects it for the same reason it rejects `fenced`. + { name: "scope/doc-unclosed", scope: "doc(h1", control: HEADINGS }, ].map(({ name, scope, control }) => ({ name, construct: `scope: ${scope}`, @@ -405,6 +442,20 @@ const INVALID_SCOPES: ValeCorpusEntry[] = [ expected: "ignored" as const, })); +/** + * `doc(...)` shapes the binary refuses at load, with an `E201` that names the + * selector. The schema agrees on the one it can see without parsing CSS. + */ +const REJECTED_DOC_SCOPES: ValeCorpusEntry[] = [ + { + name: "scope/doc-empty", + construct: "scope: doc()", + rule: scoped("doc()"), + control: HEADINGS, + expected: "rejected", + }, +]; + /** * The one place the schema deliberately disagrees with the binary. * @@ -426,6 +477,62 @@ const DIVERGENCES: ValeCorpusEntry[] = [ "fires on everything, having silently lost the exclusion it was written " + "for. The schema rejects it.", }, + // The `doc(...)` family, new in Vale 3.21.0, adds three carve-outs in the + // OTHER direction — the schema accepts, Vale does not honor — and each is + // the same decision: what is between the parens is a CSS selector, and + // judging it means parsing CSS against the document's element tree, which + // the schema does not do and should not buy a dependency to do. Vale + // reports the bad selector itself, at load, as an E201 that `test` + // surfaces; the inert ones it does not report, and that is recorded here + // so the gap is a row someone can count rather than a silence. + { + name: "scope/doc-leaf-standalone", + construct: "scope: doc(h1)", + rule: scoped("doc(h1)"), + control: HEADINGS, + proof: SCOPE_PROOF, + expected: "ignored", + divergence: + "A standalone `doc(...)` lints what is INSIDE the selected element as " + + "one block, and a leaf element (a heading, a paragraph) has nothing " + + "inside it, so the rule is inert. `text & doc(h1)` is the working " + + "spelling. Telling a leaf from a container needs the document's " + + "element tree, so the schema accepts both and the recipe teaches the " + + "difference.", + }, + { + name: "scope/doc-invalid-selector", + construct: "scope: doc(h2[)", + rule: scoped("doc(h2[)"), + control: HEADINGS, + expected: "rejected", + divergence: + "Vale compiles every selector at load and refuses this one with `E201 " + + "invalid selector in 'doc(...)': expected identifier, found EOF`. The " + + "schema does not parse CSS, so it passes the term through and `test` " + + "reports Vale's own message — the same path a malformed regex in " + + "`tokens` takes.", + }, + // `in` on a conditional, also new in 3.21.0, names a View scope the + // consequent is looked for in. It is a measured member of the check's field + // table, and every use of it fails at load until a View defines that scope. + // Views live under `/config/views/`, a directory the rule + // layout has no home for, so through this CLI the field cannot be made to + // work. The schema keeps the vocabulary honest and `test` reports the E201. + { + name: "field/conditional+in", + construct: "in on a conditional check, with no View to name", + rule: + 'extends: conditional\nmessage: "x %s"\nlevel: warning\nscope: text\n' + + "first: '\\b([A-Z]{3,5})\\b'\nsecond: '(?:\\b[A-Z][a-z]+ )+\\(([A-Z]{3,5})\\)'\nin: body\n", + control: "The ABC is here simply.\n", + expected: "rejected", + divergence: + "Vale rejects the rule at load with `E201 no View defines a scope " + + "named 'body'`. The schema accepts `in` because the binary measured it " + + "as a field of `conditional`; whether a View exists is a property of " + + "the run's config, not of the rule, and this CLI's layout defines none.", + }, ]; /** @@ -616,6 +723,61 @@ const FIELDS: ValeCorpusEntry[] = [ }, ]; +/** + * `action` names, held to the binary since 3.21.0 checks them at LOAD. + * + * Through 3.20.0 a bad action surfaced when its rule fired, on one alert. + * Now it is an `E201` at load, and one config serves the whole run, so a + * typo here silences every other Vale rule. The schema transcribes the + * accepted names from upstream `checkAction`; these rows are what keep that + * transcription true. `replace` and `remove` are the two the recipe could + * teach; the three others also constrain `params`, which the schema leaves + * to Vale. + */ +const ACTIONS: ValeCorpusEntry[] = [ + { + name: "action/replace", + construct: "action: replace", + rule: existence("action:\n name: replace\n params:\n - just\n"), + control: PROSE, + expected: "accepted", + }, + { + name: "action/remove", + construct: "action: remove", + rule: existence("action:\n name: remove\n"), + control: PROSE, + expected: "accepted", + }, + { + name: "action/unknown-name", + construct: "an action name Vale does not have", + rule: existence("action:\n name: bogus\n"), + control: PROSE, + expected: "rejected", + }, + { + name: "action/mixed-case-name-key", + construct: "Name: on an action, decoded case-insensitively", + rule: existence("action:\n Name: bogus\n"), + control: PROSE, + expected: "rejected", + }, + // The KEY is decoded case-insensitively (above); the VALUE is not. Measured: + // `Replace` and `REPLACE` both draw `E201 unknown action 'Replace'`, with a + // params shape that loads clean under `replace`. So the schema's + // case-sensitive compare is the binary's, and this row is what says so — + // a schema that lowercased the value to be forgiving would accept a rule + // Vale refuses to load. + { + name: "action/mixed-case-name-value", + construct: "action name Replace, spelled with a capital", + rule: existence("action:\n name: Replace\n params:\n - just\n"), + control: PROSE, + expected: "rejected", + }, +]; + /** * Shapes that panic the binary. * @@ -769,8 +931,10 @@ export const VALE_CORPUS: readonly ValeCorpusEntry[] = [ ...SCOPES, ...SCOPE_LISTS, ...INVALID_SCOPES, + ...REJECTED_DOC_SCOPES, ...DIVERGENCES, ...FIELDS, + ...ACTIONS, ...SHAPES, ...HEADER, ]; diff --git a/packages/cli/test/vale-run.test.ts b/packages/cli/test/vale-run.test.ts index 291b025e..9234e518 100644 --- a/packages/cli/test/vale-run.test.ts +++ b/packages/cli/test/vale-run.test.ts @@ -285,7 +285,7 @@ withVale("runVale against the real binary", () => { // // The metric that matters is the ABSOLUTE margin (duration minus budget), // not a ratio: what has to happen is the child finishing before a delayed - // timer callback runs. Measured on this fixture, warm: + // timer callback runs. Measured on this fixture, warm, on Vale 3.20.0: // // | fixture | bytes | duration | headroom over 100ms | // | --------------- | ------ | -------- | ------------------- | @@ -293,19 +293,37 @@ withVale("runVale against the real binary", () => { // | 8,000 | 152KB | ~1020ms | ~920ms | // | 17,000 (sibling)| 323KB | ~4430ms | ~4330ms | // - // 8,000 is chosen over the sibling's 17,000 deliberately: it is 20x the - // margin that actually flaked while costing a quarter of the suite time, - // and this test asserts the message rather than the blocking flag, which - // the sibling already covers with the larger fixture. + // THE FIXTURE IS A PROPERTY OF THE PINNED BINARY, AND A BUMP RE-MEASURES + // IT. Vale 3.21.0 shipped two perf commits (rune-position indexing and + // walker-context indexing) that made this workload ~20x faster: the + // 8,000-repetition fixture ran in ~45ms, UNDER the 100ms budget, and this + // test failed outright with `status: "ok"` — the same failure mode as the + // original 19-repetition flake, reached from the other side. Re-measured + // on 3.21.0, the binary alone, warm, three runs each: + // + // | fixture | bytes | duration | headroom over 100ms | + // | --------- | ------ | --------- | ------------------- | + // | 8,000 | 152KB | ~45ms | NEGATIVE — failed | + // | 80,000 | 1.5MB | ~235ms | ~135ms | + // | 320,000 | 6.1MB | ~890ms | ~790ms | + // | 350,000 | 6.7MB | ~1000ms | ~900ms | + // | 640,000 | 12MB | ~1900ms | ~1800ms | + // + // 350,000 restores the ~900ms headroom the 8,000 fixture had on 3.20.0. + // The cost scales linearly (~2.8µs per repetition), so the next bump can + // pick a number from one timing rather than a search. It is chosen over + // the sibling's 1,000,000 for the same reason 8,000 was chosen over + // 17,000: this test asserts the message rather than the blocking flag, + // which the sibling covers with the larger fixture. // // `maxFileBytes` raises `VALE_MAX_FILE_BYTES` for THIS CALL ONLY — not a - // CLI flag, not a config surface, just a seam. Without it a 152KB document - // is excluded before Vale sees it (taskless/cli#321) and reports + // CLI flag, not a config surface, just a seam. Without it a document this + // size is excluded before Vale sees it (taskless/cli#321) and reports // `status: "ok"` with a notice, never exercising the timeout at all. const cwd = makeProject( `${header}\n[*.md]\nno-simply.no-simply = YES\n`, { "no-simply": existenceRule("simply", "Avoid 'simply'") }, - { "doc.md": `${"Just simply do it. ".repeat(8000)}\n` } + { "doc.md": `${"Just simply do it. ".repeat(350_000)}\n` } ); const outcome = await runVale({ @@ -712,10 +730,19 @@ withVale("ValeRunOutcome.blocking against the real binary", () => { // CLI flag or a config surface, just a seam for a test that needs its // fixture back — so the original 320KB fixture and its ~3200ms headroom // are restored without touching the production default. + // + // VALE 3.21.0 RE-MEASURED THE FIXTURE, AGAIN. Its perf work made this + // workload ~20x faster, so 17,000 repetitions (323KB) ran in ~67ms on the + // binary alone: the headroom was gone and this test was passing on spawn + // overhead, the exact state the table above calls out as the one that + // flaked. The 1,000,000-repetition fixture (19MB) measures ~2.8s on + // 3.21.0 (linear at ~2.8µs per repetition; see the sibling's table), + // restoring ~2.7s of headroom. The file is written and killed at 100ms, + // so the size costs the write and nothing else. const cwd = makeProject( `${header}\n[*.md]\nno-simply.no-simply = YES\n`, { "no-simply": existenceRule("simply", "Avoid 'simply'") }, - { "doc.md": `${"Just simply do it. ".repeat(17_000)}\n` } + { "doc.md": `${"Just simply do it. ".repeat(1_000_000)}\n` } ); expect( diff --git a/packages/cli/test/vale-vendor-contract.test.ts b/packages/cli/test/vale-vendor-contract.test.ts index 372d1eec..c2a17bf1 100644 --- a/packages/cli/test/vale-vendor-contract.test.ts +++ b/packages/cli/test/vale-vendor-contract.test.ts @@ -119,6 +119,50 @@ const hedgingFindings = (rule: string, document: string) => { return parsed["doc.md"]?.length ?? 0; }; +/** + * Findings for one rule over one document, as the lines they landed on. + * + * The rule is enabled under `[*]` so the document's extension decides only + * which parser Vale runs, not whether the rule applies. The raw streams and + * the exit status ride along, for the cases that assert a load failure. + */ +function lines( + rule: string, + document: string, + name = "doc.md" +): { + lines: number[]; + messages: string[]; + stderr: string; + status: number | null; +} { + const cwd = project( + `${header}\n[*]\nrules.r = YES\n`, + { r: rule }, + { [name]: document } + ); + const result = runRaw(cwd, [name], ["--no-exit"]); + const parsed = JSON.parse(result.stdout || "{}") as Record< + string, + Array<{ Line: number; Message: string }> + >; + const findings = parsed[name] ?? []; + return { + lines: findings.map((finding) => finding.Line), + messages: findings.map((finding) => finding.Message), + stderr: result.stderr, + status: result.status, + }; +} + +/** An existence rule over `worth noting`, at some `doc(...)`-bearing scope. */ +const hedge = (scope: string) => + `extends: existence\nmessage: "hedge: %s"\nlevel: warning\nscope: '${scope}'\ntokens:\n - worth noting\n`; + +/** A word budget of 8 over whatever `scope` names. */ +const budget = (scope: string) => + `extends: metric\nmessage: "%s words"\nlevel: error\nscope: '${scope}'\nformula: words\ncondition: "> 8"\n`; + withVale("Vale vendor contract", () => { it("reports its own name in --version", () => { // Depended on by: PlatformBinarySpec.identity (/vale/i). If Vale stops @@ -174,6 +218,14 @@ withVale("Vale vendor contract", () => { // Depended on by: the ValeFinding interface and toValeCheckResults, which // pushes the outer key down as `file`. A rename on any of these arrives as // `undefined` in a CheckResult rather than as an error. + // + // `Suggestions` arrived in 3.21.0, beside `Action` rather than replacing + // it. Measured: it carries the same replacement a `replace` action names + // in `Params`, and is `[]` for a rule with no action, a `substitution` + // included. `toFix` keeps reading `Action`, which is the older and the + // stricter of the two: `Suggestions` will fill for other action kinds as + // upstream teaches them to compute a result, and "here is a suggestion" + // is not the promise `fix` makes. const cwd = project( `${header}\n[*.md]\nrules.no-simply = YES\n`, { "no-simply": existence("simply") }, @@ -198,9 +250,37 @@ withVale("Vale vendor contract", () => { "Message", "Severity", "Span", + "Suggestions", ]); }); + it("mirrors a replace action's parameter into Suggestions", () => { + // Depended on by: nothing yet, and that is the point of pinning it. If + // `Suggestions` ever carries a replacement `Action.Params` does not (a + // computed `edit`, say), `toFix` is leaving a real fix on the floor and + // should be taught to read both. Until then the two agree, and this is + // what says so. + const cwd = project( + `${header}\n[*.md]\nrules.swap = YES\n`, + { + swap: `extends: existence\nmessage: "Avoid '%s'"\nlevel: warning\naction:\n name: replace\n params:\n - just\ntokens:\n - simply\n`, + }, + { "doc.md": "Just simply do it.\n" } + ); + const parsed = JSON.parse( + runRaw(cwd, ["doc.md"], ["--no-exit"]).stdout + ) as Record< + string, + Array<{ + Action: { Name: string; Params: string[] }; + Suggestions: string[]; + }> + >; + const finding = parsed["doc.md"]?.[0]; + expect(finding?.Action).toEqual({ Name: "replace", Params: ["just"] }); + expect(finding?.Suggestions).toEqual(["just"]); + }); + it("prefixes check names with the StylesPath directory", () => { // Depended on by: stripRulesPrefix. The prefix is the *directory* name, so // it is `rules.` only because the engine layout puts styles in @@ -381,21 +461,51 @@ withVale("Vale vendor contract", () => { ).toBe(true); }); - it("keeps the FIRST assignment when one matcher sets a key twice", () => { - // Duplicate `[glob]` sections are merged, and the merge discards the - // later value — the opposite of the across-matcher rule above. Tooling - // that appends a disable to an existing matcher would therefore write a - // line Vale ignores. + it("keeps the LAST assignment when one matcher sets a key twice", () => { + // Duplicate `[glob]` sections are merged, and since 3.21.0 the merge + // keeps the LATER value, so precedence is positional in both + // directions: across matchers and within one. Through 3.20.0 it was the + // FIRST value, the opposite of the across-matcher rule above, and this + // test asserted that. + // + // What moved it is upstream 1e4f6ed, "let the project's rule settings + // win": a rule's level read `Key.String()`, the first of a key's + // shadowed values, so a package's setting held against the project's + // own. It now reads the last (`ValueWithShadows`), and a duplicated + // section in ONE file shadows the same way a package's file does. The + // two orders below are both asserted, as in the sibling: a test named + // for one direction that exercised only the convenient order is how the + // wrong claim survived last time. + // + // Depended on by: `assemble.ts`, whose docstring states the rule, and + // the recipe's guidance that a disable goes AFTER the enable it narrows. + // That guidance is now right for both shapes. Tooling that appends a + // disable to an existing matcher writes a line Vale honors, where it + // used to write one Vale ignored. expect( ran( `${header}\n[*.md]\nrules.no-simply = YES\n\n[*.md]\nrules.no-simply = NO\n` ) - ).toBe(true); + ).toBe(false); expect( ran( `${header}\n[*.md]\nrules.no-simply = NO\n\n[*.md]\nrules.no-simply = YES\n` ) + ).toBe(true); + }); + + it("keeps the LAST assignment when one section sets a key twice", () => { + // The same fact without a duplicate section header: two lines for one + // key in one `[glob]`. Same mechanism (`ValueWithShadows`), same + // answer, asserted separately because an ini reader could treat a + // repeated key and a repeated section differently and this file would + // otherwise not notice. + expect( + ran(`${header}\n[*.md]\nrules.no-simply = YES\nrules.no-simply = NO\n`) ).toBe(false); + expect( + ran(`${header}\n[*.md]\nrules.no-simply = NO\nrules.no-simply = YES\n`) + ).toBe(true); }); }); @@ -495,6 +605,241 @@ withVale("Vale vendor contract", () => { expect(hedgingFindings(rawRule, zoned)).toBe(1); }); }); + + /** + * What 3.21.0 added that a rule under `.taskless/rules/vale/` can reach. + * + * Each case is a behaviour the recipe or the changeset now promises, so + * each says what promise breaks. Not here, deliberately: TextFSM Views and + * the `conditional` check's `in` key. Both need a View file under + * `/config/views/`, a directory the rule layout has no home + * for, so neither can be made to work through this CLI — `in` is a + * measured member of the field table (see `vale-corpus.ts`, + * `field/conditional+in`) and every use of it fails at load. + */ + describe("Vale 3.21.0", () => { + // Two `h2` sections. `worth noting` appears once in each, on lines 5 and + // 9, so a selection of one section is told from the other by the line. + const adr = [ + "# Use a bearer token", + "", + "## Context", + "", + "It is worth noting that this hedges.", + "", + "## Decision", + "", + "We will issue tokens. It is worth noting that too.", + "", + ].join("\n"); + const inContext = 'doc(section:has(> h2:contains("Context")))'; + + describe("doc(...) selects elements by CSS selector", () => { + // Depended on by: the `doc(` family in `src/schemas/vale-rule.ts` and + // the recipe's guidance on scoping a rule to one section of a + // document. If any shape here stops firing, `verify` is accepting a + // scope that Vale silently ignores, which is the failure that module + // exists to catch. + + it("narrows a text scope to blocks inside the selected element", () => { + expect(lines(hedge(`text & ${inContext}`), adr).lines).toEqual([5]); + }); + + it("narrows a sentence scope the same way", () => { + expect(lines(hedge(`sentence & ${inContext}`), adr).lines).toEqual([5]); + }); + + it("negates to everything outside the element", () => { + expect(lines(hedge(`~${inContext}`), adr).lines).toEqual([9]); + }); + + it("lints a selected container as one block on its own", () => { + // A standalone term lints what is INSIDE the element as one block. A + // section is a container. A match is still placed where it lies in + // the file (line 5, not the section's line 3); it is a metric, which + // has no match, that reports at the block's first line — see below. + expect(lines(hedge(inContext), adr).lines).toEqual([5]); + }); + + it("is inert on a leaf element on its own, and fires when chained", () => { + // THE TRAP. `doc(h2)` alone selects a heading, and a heading has + // nothing inside it to aggregate, so the rule matches nothing with + // no error anywhere. `text & doc(h2)` reads the heading's own block. + // The recipe teaches the chained spelling; if the standalone one + // starts firing, that guidance is merely redundant, but if the + // chained one stops, it is wrong. + const heading = `extends: existence\nmessage: "%s"\nlevel: warning\nscope: 'SCOPE'\ntokens:\n - Decision\n`; + expect(lines(heading.replace("SCOPE", "doc(h2)"), adr).lines).toEqual( + [] + ); + expect( + lines(heading.replace("SCOPE", "text & doc(h2)"), adr).lines + ).toEqual([7]); + }); + + it("rejects a selector it cannot compile at load, with E201", () => { + // Depended on by: the schema's decision NOT to parse the selector. + // That is safe only while Vale reports a bad one itself, loudly and + // at load. If this ever becomes a silent no-op, the schema has to + // grow a selector check. + const result = lines(hedge("doc(h2[)"), adr); + expect(result.status).not.toBe(0); + const diagnostic = asValeConfigError(JSON.parse(result.stderr)); + expect(diagnostic?.Code).toBe("E201"); + expect(diagnostic?.Text).toContain("invalid selector in 'doc(...)'"); + }); + + it("lets a metric measure the selected block rather than the document", () => { + // Depended on by: the changeset's claim that a word budget can be + // put on one section. `words` over the whole document is 20; over + // the Decision section it is 11, and over Context it is 7, so a + // budget of 8 separates the three answers. + expect( + lines(budget('doc(section:has(> h2:contains("Decision")))'), adr) + .lines + ).toEqual([7]); + expect(lines(budget(inContext), adr).lines).toEqual([]); + }); + }); + + it("lets a metric honor an ordinary scope instead of forcing the summary", () => { + // Depended on by: any metric rule that declares a scope. Through + // 3.20.0 `NewMetric` overwrote the scope with `summary`, so a + // `scope: sentence` metric measured the whole document; 3.21.0 keeps + // a declared scope (`measuredScope`) and only an absent one, or + // `text`, still means the document. The fixture separates the two: + // the document is 6 words, every sentence is 2, so a budget of 3 fires + // on the old reading and not on the new. + const perSentence = `extends: metric\nmessage: "%s words"\nlevel: error\nscope: sentence\nformula: words\ncondition: "> 3"\n`; + const whole = perSentence.replace("scope: sentence\n", ""); + const document = "One two. Three four. Five six.\n"; + expect(lines(perSentence, document).lines).toEqual([]); + expect(lines(whole, document).lines).toEqual([1]); + }); + + it("reports a sequence match once under a negated scope", () => { + // Depended on by: any `sequence` rule with `scope: ~code` or similar, + // the spelling the recipe suggests for keeping a grammar rule out of + // inline code. Upstream #1169 says 3.20.0 reported each match twice. + // MEASURED AGAINST THE 3.20.0 BINARY, IT DID NOT, on this shape or on + // upstream's own (`pattern: widget` / `pattern: arrived, skip: 1` + // under `~list`): both binaries report one. Upstream's regression test + // dispatches blocks by hand below the linter, so whatever de-duplicated + // the pair on the way out is above it. Pinned at one anyway, because + // that is the number a rule author sees and the number the recipe's + // guidance assumes; the ledger says "nothing to do unless you see one". + const modal = `extends: sequence\nmessage: "%s is a modal and a verb"\nlevel: warning\nscope: "~code"\ntokens:\n - tag: MD\n - tag: VB\n`; + expect(lines(modal, "We could keep sessions.\n").lines).toEqual([1]); + }); + + describe("BlockIgnores and TokenIgnores apply to HTML", () => { + // Depended on by: a rule's own `.vale.ini`, which is carried into the + // assembled config verbatim, so an `[*.html]` matcher can carry these + // keys. Through 3.20.0 both were silently ignored for `.html`. + const document = + '

Just simply do it.

\n\n

And [[simply]] too.

\n'; + const html = (extra: string) => { + const cwd = project( + `${header}\n[*.html]\nrules.no-simply = YES\n${extra}`, + { "no-simply": existence("simply") }, + { "doc.html": document } + ); + const parsed = JSON.parse( + runRaw(cwd, ["doc.html"], ["--no-exit"]).stdout + ) as Record>; + return (parsed["doc.html"] ?? []).map((finding) => finding.Line); + }; + + it("reads every occurrence without them", () => { + expect(html("")).toEqual([1, 2, 3]); + }); + + it("drops a block a BlockIgnores pattern matches", () => { + expect( + html('BlockIgnores = (?s)\n') + ).toEqual([1, 3]); + }); + + it("drops a token a TokenIgnores pattern matches", () => { + expect(html(String.raw`TokenIgnores = \[\[.*?\]\]` + "\n")).toEqual([ + 1, 2, + ]); + }); + }); + + it("rejects an unknown action name at load, taking the run with it", () => { + // Depended on by: `actionMessages` in `src/schemas/vale-rule.ts`, + // which exists because of this. Measured on 3.20.0: the rule loaded, + // and a document its token matched died with an `E100` carrying no + // path, while a document it did not match linted normally. 3.21.0 + // checks the action when the rule loads, and a load failure is one + // E201 for the whole run — the second rule here is valid and reports + // nothing, on a document where the first would not even have fired. + const cwd = project( + `${header}\n[*.md]\nrules.no-simply = YES\nrules.act = YES\n`, + { + "no-simply": existence("simply"), + act: `extends: existence\nmessage: "%s"\nlevel: warning\naction:\n name: bogus\ntokens:\n - just\n`, + }, + { "doc.md": "Just simply do it.\n" } + ); + const result = runRaw(cwd, ["doc.md"], ["--no-exit"]); + expect(result.status).toBe(2); + expect(result.stdout.trim()).toBe(""); + const diagnostic = asValeConfigError(JSON.parse(result.stderr)); + expect(diagnostic?.Code).toBe("E201"); + expect(diagnostic?.Text).toBe("unknown action 'bogus'"); + }); + + describe("a Jupyter notebook is read cell by cell", () => { + // Depended on by: the `.ipynb` row of VALE_FORMAT_TIERS, which the + // markup probe above pins by tier only. These pin what the tier means + // for a notebook, which the changeset states: Markdown cells as + // Markdown, code cells as their kernel's comments, nothing else. + const simply = existence("simply"); + const cells = (rows: NotebookCell[], name = "nb.ipynb") => + lines(simply, notebook(rows), name); + + it("lints a code cell's comments and not its code", () => { + expect( + cells([{ cell_type: "code", source: ["x = 1\n", "# simply\n"] }]) + .lines + ).toHaveLength(1); + expect( + cells([{ cell_type: "code", source: ["simply = 1\n"] }]).lines + ).toHaveLength(0); + }); + + it("skips raw cells and outputs", () => { + expect( + cells([{ cell_type: "raw", source: ["simply raw\n"] }]).lines + ).toHaveLength(0); + expect( + cells([ + { + cell_type: "code", + source: ["x = 1\n"], + outputs: [ + { output_type: "stream", name: "stdout", text: ["simply\n"] }, + ], + }, + ]).lines + ).toHaveLength(0); + }); + + it("places a finding on the notebook file's own line", () => { + // The fixture puts each cell on its own line: line 3 is the first + // cell, line 4 the second. A finding in the second cell's Markdown + // is reported at line 4 of the .ipynb, not at line 2 of the cell. + expect( + cells([ + { cell_type: "code", source: ["x = 1\n"] }, + { cell_type: "markdown", source: ["One.\n", "Two simply.\n"] }, + ]).lines + ).toEqual([4]); + }); + }); + }); }); /** @@ -562,6 +907,42 @@ function comment(extension: string): string { return "// simply\n"; } +/** A cell of a Jupyter notebook, as the fixture builder below needs it. */ +interface NotebookCell { + cell_type: "markdown" | "code" | "raw"; + source: string[]; + outputs?: unknown[]; +} + +/** + * A minimal nbformat 4 notebook with a Python kernel, one cell per line. + * + * Pretty-printed with each cell on its own line, so a `Line` in a finding can + * be read against the fixture. The kernel matters: Vale lints a code cell as + * its kernel's language, so `# simply` is a comment only because this says + * `python`. + */ +function notebook(cells: NotebookCell[]): string { + const rendered = cells.map((cell) => + JSON.stringify({ + ...cell, + metadata: {}, + ...(cell.cell_type === "code" ? { execution_count: null } : {}), + }) + ); + return [ + "{", + ' "cells": [', + rendered.map((line) => ` ${line}`).join(",\n"), + " ],", + ' "metadata": {"kernelspec": {"name": "python3", "language": "python"}, "language_info": {"name": "python"}},', + ' "nbformat": 4,', + ' "nbformat_minor": 5', + "}", + "", + ].join("\n"); +} + withVale("Vale engine capabilities", () => { /** A project whose single rule applies to every extension. */ const anyExtension = (documents: Record) => @@ -633,6 +1014,17 @@ withVale("Vale engine capabilities", () => { prose: "

We simply do it.

\n", skipped: "\n", }, + // Native as of 3.21.0 (`internal/lint/notebook.go`). The skipped construct + // is the token in a CODE cell's body: on 3.20.0 a notebook was read as the + // JSON it is and this fired, which is what a plaintext fallback looks like. + ".ipynb": { + prose: notebook([ + { cell_type: "markdown", source: ["We simply do it.\n"] }, + ]), + skipped: notebook([ + { cell_type: "code", source: ["simply = 1\n"], outputs: [] }, + ]), + }, }; it("reports the pinned version", () => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d32a6c82..ed466aef 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -155,23 +155,23 @@ importers: specifier: 0.45.3 version: 0.45.3 '@taskless/vale-darwin-arm64': - specifier: 3.20.0-20260907164938 - version: 3.20.0-20260907164938 + specifier: 3.21.0-20260915061224 + version: 3.21.0-20260915061224 '@taskless/vale-darwin-x64': - specifier: 3.20.0-20260907164938 - version: 3.20.0-20260907164938 + specifier: 3.21.0-20260915061224 + version: 3.21.0-20260915061224 '@taskless/vale-linux-arm64': - specifier: 3.20.0-20260907164938 - version: 3.20.0-20260907164938 + specifier: 3.21.0-20260915061224 + version: 3.21.0-20260915061224 '@taskless/vale-linux-x64': - specifier: 3.20.0-20260907164938 - version: 3.20.0-20260907164938 + specifier: 3.21.0-20260915061224 + version: 3.21.0-20260915061224 '@taskless/vale-win32-arm64': - specifier: 3.20.0-20260907164938 - version: 3.20.0-20260907164938 + specifier: 3.21.0-20260915061224 + version: 3.21.0-20260915061224 '@taskless/vale-win32-x64': - specifier: 3.20.0-20260907164938 - version: 3.20.0-20260907164938 + specifier: 3.21.0-20260915061224 + version: 3.21.0-20260915061224 packages/vale-darwin-arm64: {} @@ -849,33 +849,33 @@ packages: '@standard-schema/spec@1.1.0': resolution: {integrity: sha512-l2aFy5jALhniG5HgqrD6jXLi/rUWrKvqN/qJx6yoJsgKhblVd+iqqU4RCXavm/jPityDo5TCvKMnpjKnOriy0w==} - '@taskless/vale-darwin-arm64@3.20.0-20260907164938': - resolution: {integrity: sha512-xq+RmlsgHCG//TSXwZLOgcsmvJGBqI6tsjqSMkdJ52gSR2lO7u+jcscfJUN2R28Yhjqv0ZM9/t3K1T987Lu+Ug==} + '@taskless/vale-darwin-arm64@3.21.0-20260915061224': + resolution: {integrity: sha512-HNatxojTyY5HW5n9/hJm7lWTB7QrgLNb5MvkwVpFKCtjWta6ztToq+kOSWxn3I/QJRnhRC0wGdwSDsRLCdBKAw==} cpu: [arm64] os: [darwin] - '@taskless/vale-darwin-x64@3.20.0-20260907164938': - resolution: {integrity: sha512-Zz2SjbkVQ6/KB2XmWU2J5qx6i5uROH3L84u8HDl1c1FsM8UzFjwdSiiivHbQpGzN/EUD4HNBOmzzzuQqxSsXfw==} + '@taskless/vale-darwin-x64@3.21.0-20260915061224': + resolution: {integrity: sha512-1ht+pCb6+tukCQLtB9gMR7J08pJJ528B71BpaHzukEq1CgLvMUE/mVDoAYKbRIYfdVNCNXQ1yp9T4t0t7RTeag==} cpu: [x64] os: [darwin] - '@taskless/vale-linux-arm64@3.20.0-20260907164938': - resolution: {integrity: sha512-VNff5AoLPcQ0hhQJPzuZErwFeBe/QyQiUB5oZo6HShvnmjwp5GKUf85hf2b3md1hmUuP7WrBcRld1edQ4DWx+Q==} + '@taskless/vale-linux-arm64@3.21.0-20260915061224': + resolution: {integrity: sha512-ZlhwgeVZw+uhVLSq7mMlJv7sXSVdJdocrzpZYVC7Wbzu4MFernSEfga40bWrPrxVMEE+THiBpY0k+GSloYyEZg==} cpu: [arm64] os: [linux] - '@taskless/vale-linux-x64@3.20.0-20260907164938': - resolution: {integrity: sha512-Tt/FqNYqgX8P0KtkmukqxJqUrtHlrnONPB+rJHiCNp6KzyNNnTI3doU8tzw/jtTZ3Tl8zCFSjSU7cLuwnowFAw==} + '@taskless/vale-linux-x64@3.21.0-20260915061224': + resolution: {integrity: sha512-OHJvKCZKHWmB1DAlivAlFAemgHR1Z4WFCGmlr25LFf/IGjCUd8Oyl/+4PG5cpUpkiNq//IjmSZYIaCZvHAdrew==} cpu: [x64] os: [linux] - '@taskless/vale-win32-arm64@3.20.0-20260907164938': - resolution: {integrity: sha512-DDj7sh18fjiwndoxCmD4DAwrFp6TZD7wNvGt67DvB+7oJpJIVvAtCQROXVR9jXhWoutIKuthb240pMWw1/oHAg==} + '@taskless/vale-win32-arm64@3.21.0-20260915061224': + resolution: {integrity: sha512-froiKcI/b7ZhfqPTVDosU+zMcdRqB1pRJ3RgeWmg2D98PNpJFyoq0qJGYRP5fFDB24WCbaK/mWJE8KlhFj1bBw==} cpu: [arm64] os: [win32] - '@taskless/vale-win32-x64@3.20.0-20260907164938': - resolution: {integrity: sha512-ZaH12oV2R6zxW4nkFrmlS1W51RA1FLKv9VbuLTIm5UrRFEFXL9UHLO0BYLQCqu+vmT1eVU/V1WZBQTPizAg4Ow==} + '@taskless/vale-win32-x64@3.21.0-20260915061224': + resolution: {integrity: sha512-OH9jjZ46kS7D9Txri4vizOfc8+d6Rmxe/Howdt+fGEzJhyu/V+B2kUIQepOaRSPOL0AvFMhvYX7hu7/onAyuzw==} cpu: [x64] os: [win32] @@ -3144,22 +3144,22 @@ snapshots: '@standard-schema/spec@1.1.0': {} - '@taskless/vale-darwin-arm64@3.20.0-20260907164938': + '@taskless/vale-darwin-arm64@3.21.0-20260915061224': optional: true - '@taskless/vale-darwin-x64@3.20.0-20260907164938': + '@taskless/vale-darwin-x64@3.21.0-20260915061224': optional: true - '@taskless/vale-linux-arm64@3.20.0-20260907164938': + '@taskless/vale-linux-arm64@3.21.0-20260915061224': optional: true - '@taskless/vale-linux-x64@3.20.0-20260907164938': + '@taskless/vale-linux-x64@3.21.0-20260915061224': optional: true - '@taskless/vale-win32-arm64@3.20.0-20260907164938': + '@taskless/vale-win32-arm64@3.21.0-20260915061224': optional: true - '@taskless/vale-win32-x64@3.20.0-20260907164938': + '@taskless/vale-win32-x64@3.21.0-20260915061224': optional: true '@types/chai@5.2.3':