diff --git a/.changeset/vale-3-23-0.md b/.changeset/vale-3-23-0.md index e511f483..54f9b14a 100644 --- a/.changeset/vale-3-23-0.md +++ b/.changeset/vale-3-23-0.md @@ -3,3 +3,7 @@ --- Update the bundled Vale to 3.23.0. + +A Vale rule over code comments reads less than it did: a comment addressed to a tool (`//nolint`, `# noqa`, `eslint-disable`) and a Python docstring's `:param:` field are no longer linted, and Kotlin (`.kt`, `.kts`) is now comment-aware, where it was linted as one block of prose. Those findings disappear, and none appear in their place. + +A standalone `scope: doc()`, such as `doc(h2)`, now lints the element's own text, where it matched nothing; `text & doc(h2)` behaves as before. `verify` now accepts a rule's `tests:` key, which Vale reads for `vale test` and was already loading without complaint. diff --git a/.github/scripts/vale-upgrade-report.cjs b/.github/scripts/vale-upgrade-report.cjs new file mode 100644 index 00000000..94ef5914 --- /dev/null +++ b/.github/scripts/vale-upgrade-report.cjs @@ -0,0 +1,208 @@ +#!/usr/bin/env node +// SPDX-License-Identifier: MIT +"use strict"; + +/** + * What a Vale upgrade pull request needs a reviewer to see, and CI does not + * show them: the contract suites' verdict, and the format readers upstream + * added. + * + * THE CONTRACT VERDICT, BECAUSE VALIDATE DOES NOT RUN. vale-upgrade.yml opens + * its pull request as `github-actions[bot]`, and every `pull_request` run that + * actor triggers waits at `action_required` until a maintainer approves it. + * Measured on every bot-opened upgrade so far, Vale and ast-grep alike: #427's + * Validate sat at `action_required` with a 0s runtime, and on each earlier one + * Validate first ran when a human pushed a child commit. So the red check the + * workflow header relies on ("left to Validate, it is a red check on a pull + * request that already carries upstream's notes") never appeared: #427 read as + * a pull request with no checks, while three contract tests were failing. + * + * The suites are still NOT a gate. The header's reasoning stands: a changed + * verdict is a measured edit for a child pull request, and gating on it turns + * a scheduled run red every morning with no pull request to read. So the job + * runs them, keeps going whatever they say, and this script puts the verdict + * in the body. + * + * THE FORMAT READERS, BECAUSE NO TABLE CAN REPORT ONE. `VALE_FORMAT_TIERS` in + * capabilities.ts is re-probed row by row on every bump, which catches a format + * that MOVED. It cannot catch a format Vale LEARNED, since an extension with no + * row is never probed; the table's own header asks for a source check instead, + * by hand. 3.23.0 is the bump where nobody did it: `internal/lint/code/kt.go` + * moved Kotlin from plaintext to the comment tier and every test stayed green. + * The benign direction. The dangerous one is a reader that shells out to a + * converter, which turns the same missing row into a crash that takes down the + * run. Listing the files upstream ADDED under `internal/lint/` is the source + * check, mechanically. + * + * Both halves report and never throw. A failed fetch or a missing test report + * is said in the body rather than failing the run: the proposal is still worth + * opening, and "we could not check" must not read like "nothing to see". + */ + +const { readFileSync, writeFileSync } = require("node:fs"); + +const { baseVersion } = require("./vale-upgrade-detect.cjs"); + +/** The compare API returns at most this many files, with no marker when cut. */ +const COMPARE_FILE_CAP = 300; + +/** Where upstream keeps its format readers, and which of those files are. */ +const READER_PREFIX = "internal/lint/"; +const isReader = (filename) => + filename.startsWith(READER_PREFIX) && + filename.endsWith(".go") && + !filename.endsWith("_test.go"); + +/** + * The failures in a vitest `--reporter=json` report. + * + * `undefined` when there is no report at all, which is a different finding + * from zero failures: the suites did not run, or crashed before writing. + */ +function summarizeContract(report) { + if (report === undefined) return undefined; + const failed = []; + for (const file of report.testResults ?? []) { + for (const assertion of file.assertionResults ?? []) { + if (assertion.status !== "failed") continue; + failed.push({ + file: String(file.name ?? "").split("/").pop(), + name: assertion.fullName, + message: String(assertion.failureMessages?.[0] ?? "").split("\n")[0], + }); + } + } + return { total: report.numTotalTests ?? 0, failed }; +} + +/** Read a vitest JSON report, or `undefined` if it was never written. */ +function readContractReport(path) { + try { + return JSON.parse(readFileSync(path, "utf8")); + } catch { + return undefined; + } +} + +/** + * The reader files upstream added between two tags. + * + * Only ADDED files. A modified reader changes what a format sees, which the + * release notes and the contract suites cover; an added one can route an + * extension no row names, which only this covers. + */ +async function fetchAddedReaders(repository, from, to) { + const headers = { + accept: "application/vnd.github+json", + "user-agent": "taskless-vale-upgrade-report", + }; + if (process.env.GITHUB_TOKEN) { + headers.authorization = `Bearer ${process.env.GITHUB_TOKEN}`; + } + const url = `https://api.github.com/repos/${repository}/compare/v${from}...v${to}`; + const response = await fetch(url, { headers }); + if (!response.ok) { + throw new Error(`GET ${url} responded ${response.status}`); + } + const files = (await response.json()).files ?? []; + return { + added: files + .filter((file) => file.status === "added" && isReader(file.filename)) + .map((file) => file.filename) + .toSorted(), + complete: files.length < COMPARE_FILE_CAP, + url: `https://github.com/${repository}/compare/v${from}...v${to}`, + }; +} + +/** A fence no line of `text` can close early. */ +function fence(text) { + const longest = Math.max( + 2, + ...[...text.matchAll(/`+/g)].map((match) => match[0].length) + ); + const marker = "`".repeat(longest + 1); + return `${marker}\n${text}\n${marker}`; +} + +function formatContract(contract) { + const heading = "### Contract suites"; + if (contract === undefined) { + return `${heading}\n\n**Did not run.** No test report was written, so this upgrade has not been checked against the recorded contract. Run \`pnpm --filter @taskless/cli test --project cli vale-schema-contract vale-vendor-contract\` on this branch.\n`; + } + if (contract.failed.length === 0) { + return `${heading}\n\nAll ${contract.total} tests in \`vale-schema-contract\` and \`vale-vendor-contract\` pass against the new binary. No recorded verdict changed.\n`; + } + const lines = contract.failed.map( + (failure) => `${failure.file} > ${failure.name}\n ${failure.message}` + ); + return `${heading}\n\n**${contract.failed.length} of ${contract.total} failed.** The new Vale changed a recorded behaviour. The measured edit, the \`update\` ledger, and any migration belong in a child pull request that merges down into this branch.\n\n${fence(lines.join("\n"))}\n`; +} + +function formatReaders(readers, error) { + const heading = "### Format readers upstream added"; + if (readers === undefined) { + return `${heading}\n\n**Could not check** (${error}). Compare \`internal/lint/\` between the two tags by hand before merging.\n`; + } + const caveat = readers.complete + ? "" + : `\n\nThe comparison touched ${COMPARE_FILE_CAP} or more files, where the API stops listing them, so this may be incomplete. Check [the full diff](${readers.url}).`; + if (readers.added.length === 0) { + return `${heading}\n\nNone under \`${READER_PREFIX}\` ([diff](${readers.url})). No extension can have changed tier without a row that re-probes it.${caveat}\n`; + } + const list = readers.added.map((file) => `- \`${file}\``).join("\n"); + return `${heading}\n\n${list}\n\nEach can route an extension that \`VALE_FORMAT_TIERS\` in \`capabilities.ts\` has no row for, and an unlisted extension is never probed. Find where \`internal/core/format.go\` sends it, probe the extension against the new binary, and add a row. A reader that needs an external converter is the dangerous case: the missing row becomes a crash that aborts every Vale rule in the run. ([diff](${readers.url}))${caveat}\n`; +} + +function formatReport({ from, to, contract, readers, readersError }) { + return `## Measured against Vale ${to}\n\nCompared with ${from}, which is what \`main\` pins. Validate on a pull request this workflow opens waits for a maintainer to approve it, so this section is the check that ran.\n\n${formatContract(contract)}\n${formatReaders(readers, readersError)}`; +} + +function readArgument(argv, name) { + const index = argv.indexOf(name); + if (index === -1 || index + 1 >= argv.length) { + throw new Error(`missing ${name} `); + } + return argv[index + 1]; +} + +async function main({ + argv = process.argv.slice(2), + addedReaders = fetchAddedReaders, + readReport = readContractReport, +} = {}) { + const from = baseVersion(readArgument(argv, "--pinned")); + const to = readArgument(argv, "--to"); + const repository = readArgument(argv, "--repository"); + const out = readArgument(argv, "--out"); + + const contract = summarizeContract(readReport(readArgument(argv, "--contract"))); + let readers; + let readersError; + try { + readers = await addedReaders(repository, from, to); + } catch (error) { + readersError = error.message; + } + + const report = formatReport({ from, to, contract, readers, readersError }); + writeFileSync(out, report); + console.log(report); + return report; +} + +module.exports = { + COMPARE_FILE_CAP, + fetchAddedReaders, + formatReport, + isReader, + main, + summarizeContract, +}; + +if (require.main === module) { + main().catch((error) => { + console.error(`\nvale-upgrade-report failed: ${error.message}`); + process.exitCode = 1; + }); +} diff --git a/.github/scripts/vale-upgrade-report.test.cjs b/.github/scripts/vale-upgrade-report.test.cjs new file mode 100644 index 00000000..10ad7604 --- /dev/null +++ b/.github/scripts/vale-upgrade-report.test.cjs @@ -0,0 +1,195 @@ +// SPDX-License-Identifier: MIT +"use strict"; + +/** + * Tests for vale-upgrade-report.cjs. + * + * The failure worth guarding against is a report that reads as reassurance + * when nothing was checked. "No failures" and "no report", "no new readers" + * and "could not ask", each pair must render differently, because #427 was a + * pull request whose silence looked exactly like a pass. + */ + +const test = require("node:test"); +const assert = require("node:assert/strict"); +const { mkdtempSync, readFileSync, rmSync } = require("node:fs"); +const { tmpdir } = require("node:os"); +const { join } = require("node:path"); + +const { + COMPARE_FILE_CAP, + formatReport, + isReader, + main, + summarizeContract, +} = require("./vale-upgrade-report.cjs"); + +/** The shape vitest's JSON reporter writes, reduced to what is read. */ +const vitestReport = (...failures) => ({ + numTotalTests: 204, + testResults: [ + { + name: "/w/packages/cli/test/vale-schema-contract.test.ts", + assertionResults: [ + { status: "passed", fullName: "a passing test", failureMessages: [] }, + ...failures.map(([fullName, message]) => ({ + status: "failed", + fullName, + failureMessages: [`${message}\n at somewhere (file.ts:1:1)`], + })), + ], + }, + ], +}); + +const readers = (added, complete = true) => ({ + added, + complete, + url: "https://github.com/vale-cli/vale/compare/v3.22.0...v3.23.0", +}); + +const report = (overrides) => + formatReport({ + from: "3.22.0", + to: "3.23.0", + contract: summarizeContract(vitestReport()), + readers: readers([]), + ...overrides, + }); + +test("contract: names each failure with its file and first message line", () => { + const contract = summarizeContract( + vitestReport([ + "Vale schema contract agrees with every recorded verdict", + "AssertionError: scope/doc-leaf-standalone (scope: doc(h1)): recorded ignored, Vale 3.23.0 says accepted", + ]) + ); + assert.equal(contract.total, 204); + assert.deepEqual(contract.failed, [ + { + file: "vale-schema-contract.test.ts", + name: "Vale schema contract agrees with every recorded verdict", + message: + "AssertionError: scope/doc-leaf-standalone (scope: doc(h1)): recorded ignored, Vale 3.23.0 says accepted", + }, + ]); + const section = report({ contract }); + assert.match(section, /\*\*1 of 204 failed\.\*\*/); + assert.match(section, /vale-schema-contract\.test\.ts > Vale schema contract/); + assert.doesNotMatch(section, /at somewhere/); +}); + +test("contract: a missing report says it did not run, never that it passed", () => { + const section = report({ contract: summarizeContract(undefined) }); + assert.match(section, /\*\*Did not run\.\*\*/); + assert.doesNotMatch(section, /pass against the new binary/); +}); + +test("contract: a clean report says so, with the count", () => { + assert.match(report({}), /All 204 tests .* pass against the new binary/); +}); + +test("contract: a failure message cannot close the fence around it", () => { + const contract = summarizeContract( + vitestReport(["x", "expected ```` to equal ``` ```"]) + ); + const section = report({ contract }); + const fences = section.match(/^`{5}$/gm) ?? []; + assert.equal(fences.length, 2, "a fence longer than any run in the text"); +}); + +test("readers: lists only added, non-test Go files under internal/lint/", () => { + assert.equal(isReader("internal/lint/code/kt.go"), true); + assert.equal(isReader("internal/lint/quote.go"), true); + assert.equal(isReader("internal/lint/code/kt_test.go"), false); + assert.equal(isReader("internal/core/format.go"), false); + assert.equal(isReader("internal/lint/testdata/a.md"), false); +}); + +test("readers: an added reader asks for a probe and a row", () => { + const section = report({ + readers: readers(["internal/lint/code/kt.go", "internal/lint/code/toml.go"]), + }); + assert.match(section, /- `internal\/lint\/code\/kt\.go`/); + assert.match(section, /VALE_FORMAT_TIERS/); + assert.match(section, /internal\/core\/format\.go/); +}); + +test("readers: none added is said, with the diff to check it by", () => { + const section = report({}); + assert.match(section, /None under `internal\/lint\/`/); + assert.match(section, /compare\/v3\.22\.0\.\.\.v3\.23\.0/); +}); + +test("readers: a failed fetch says it could not check, never that none were added", () => { + const section = report({ readers: undefined, readersError: "GET x responded 502" }); + assert.match(section, /\*\*Could not check\*\* \(GET x responded 502\)/); + assert.doesNotMatch(section, /None under/); +}); + +test("readers: a comparison at the API's file cap is flagged as possibly incomplete", () => { + const section = report({ readers: readers([], false) }); + assert.match(section, new RegExp(`${COMPARE_FILE_CAP} or more files`)); +}); + +test("main: compares from the pinned base version and writes the report", async () => { + const directory = mkdtempSync(join(tmpdir(), "vale-upgrade-report-")); + try { + const out = join(directory, "report.md"); + const asked = []; + await main({ + argv: [ + "--pinned", + "3.22.0-20260921180930", + "--to", + "3.23.0", + "--repository", + "vale-cli/vale", + "--contract", + join(directory, "absent.json"), + "--out", + out, + ], + addedReaders: async (...args) => { + asked.push(args); + return readers(["internal/lint/code/kt.go"]); + }, + readReport: () => undefined, + }); + assert.deepEqual(asked, [["vale-cli/vale", "3.22.0", "3.23.0"]]); + const written = readFileSync(out, "utf8"); + assert.match(written, /^## Measured against Vale 3\.23\.0$/m); + assert.match(written, /Compared with 3\.22\.0/); + assert.match(written, /\*\*Did not run\.\*\*/); + } finally { + rmSync(directory, { recursive: true, force: true }); + } +}); + +test("main: a fetch that throws is reported, not raised", async () => { + const directory = mkdtempSync(join(tmpdir(), "vale-upgrade-report-")); + try { + const out = join(directory, "report.md"); + await main({ + argv: [ + "--pinned", + "3.22.0-20260921180930", + "--to", + "3.23.0", + "--repository", + "vale-cli/vale", + "--contract", + "unused", + "--out", + out, + ], + addedReaders: async () => { + throw new Error("GET x responded 403"); + }, + readReport: () => vitestReport(), + }); + assert.match(readFileSync(out, "utf8"), /Could not check\*\* \(GET x responded 403\)/); + } finally { + rmSync(directory, { recursive: true, force: true }); + } +}); diff --git a/.github/workflows/vale-upgrade.yml b/.github/workflows/vale-upgrade.yml index 2ba0eb4b..61211e2d 100644 --- a/.github/workflows/vale-upgrade.yml +++ b/.github/workflows/vale-upgrade.yml @@ -56,7 +56,9 @@ # (the measured `Vale ` block in vale-vendor-contract.test.ts and the # verdict corpus in vale-schema-contract.test.ts), the `update` ledger, any # migration, and the changeset prose. Those tests are NOT gated here, on -# purpose: see the check step. +# purpose: see the check step. They ARE run, and their verdict is written into +# the pull request body, because Validate does not run on a pull request this +# job opens until a maintainer approves it (see the report step). # # Action refs are pinned to commit SHAs; the trailing comment records the tag. @@ -215,14 +217,56 @@ jobs: # DOES, and a new Vale that changes a verdict fails them — correctly, and # the fix is a measured edit that belongs in the child pull request. Gated # here, that signal would become a scheduled run failing every morning - # with no pull request and no changelog to read; left to Validate, it is - # a red check on a pull request that already carries upstream's notes. + # with no pull request and no changelog to read. The next two steps run + # them anyway and report the verdict in the body instead. - name: Check the bump is self-consistent if: steps.detect.outputs.update == 'true' run: >- pnpm --filter @taskless/cli test --project cli engine-version-consistency + # REPORTED, NOT GATED, AND NOT LEFT TO VALIDATE. The comment above used to + # end "left to Validate, it is a red check on a pull request". It was + # not: a pull_request run triggered by github-actions[bot] waits at + # `action_required` until a maintainer approves it. Measured on every + # bot-opened upgrade, Vale and ast-grep alike, and #427 read as a pull + # request with no checks while three contract tests were failing. + # + # So the suites run here, `continue-on-error` so a changed verdict still + # proposes the upgrade, and write a JSON report the next step renders. + # The report path is absolute because `--filter` runs vitest from + # packages/cli. + - name: Run the Vale contract suites + if: steps.detect.outputs.update == 'true' + continue-on-error: true + run: >- + pnpm --filter @taskless/cli exec vitest run --project cli + vale-schema-contract vale-vendor-contract + --reporter=default --reporter=json + --outputFile.json="${RUNNER_TEMP}/vale-contract.json" + + # Renders the contract verdict, and the source check capabilities.ts asks + # for on every bump by hand: the files upstream ADDED under + # internal/lint/, where a new format reader lands. A reader adds an + # extension no VALE_FORMAT_TIERS row probes, so no test can catch it; + # 3.23.0's kt.go is the one that slipped through. See the script header. + # `continue-on-error` because the body is worth opening without it, and + # the propose step says so when the file is missing. + - name: Report the contract and upstream's new format readers + if: steps.detect.outputs.update == 'true' + continue-on-error: true + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PINNED_VERSION: ${{ steps.detect.outputs.pinned_version }} + BASE_VERSION: ${{ steps.detect.outputs.base_version }} + run: | + node .github/scripts/vale-upgrade-report.cjs \ + --pinned "$PINNED_VERSION" \ + --to "$BASE_VERSION" \ + --repository "$(node -p 'require("./.github/scripts/vale-manifest.json").upstream.repository')" \ + --contract "${RUNNER_TEMP}/vale-contract.json" \ + --out "${RUNNER_TEMP}/vale-upgrade-report.md" + - name: Propose the upgrade if: steps.detect.outputs.update == 'true' env: @@ -269,11 +313,11 @@ jobs: \`${BASE_VERSION}\` and regenerates \`src/generated/vale-vocabulary.ts\` from the new binary, and the workflow ran \`engine-version-consistency\` against the result before pushing. What - this does **not** carry is judgment: if \`vale-schema-contract\` or - \`vale-vendor-contract\` is red on this pull request, Vale - ${BASE_VERSION} changed a measured behaviour, and that edit, the - \`update\` ledger, any migration, and the release note's prose belong - in a child pull request. + this does **not** carry is judgment: if the contract suites below + report a failure, Vale ${BASE_VERSION} changed a measured behaviour, + and that edit, the \`update\` ledger, any migration, and the release + note's prose belong in a child pull request. So does a row for each + format reader listed below. **This is the half that reaches a user.** Republishing the platform packages changes nobody's install, because the CLI pins each one @@ -286,6 +330,17 @@ jobs: not write. BODY + # Ours, rendered from the two steps above. Missing only if the report + # step itself failed, which must not read as a clean upgrade. + report="${RUNNER_TEMP}/vale-upgrade-report.md" + if [ -f "$report" ]; then + printf '\n' >> "$body" + cat "$report" >> "$body" + else + printf '\n## Measured against Vale %s\n\n**Not measured.** The report step failed; read its log in this run.\n' "$BASE_VERSION" >> "$body" + echo "::warning::no upgrade report at ${report}" + fi + # Absent only if the detect step reported ahead and then wrote no # notes, which it cannot — so warn rather than skip silently. notes="${RUNNER_TEMP}/vale-release-notes.md" diff --git a/packages/cli/scripts/generate-vale-schema.ts b/packages/cli/scripts/generate-vale-schema.ts index 4ada9fb9..33cbd17c 100644 --- a/packages/cli/scripts/generate-vale-schema.ts +++ b/packages/cli/scripts/generate-vale-schema.ts @@ -483,6 +483,11 @@ const FIELD_CANDIDATES: readonly string[] = [ // 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", + // Documented in Vale 3.23.0's release notes ("a rule can carry its own + // `tests:`"), read by `vale test`. Measured accepted on 3.22.0 as well, with + // any value, so it was a member before it was announced, and `verify` was + // rejecting a key the binary runs, calling it an E201. + "tests", ].toSorted(); /** @@ -708,6 +713,7 @@ const CASING_VALUES: Record = { message: "x %s", name: "a name", scope: "raw", + tests: ["fixture.md"], }; /** diff --git a/packages/cli/src/agent/create-vale-rule.md b/packages/cli/src/agent/create-vale-rule.md index 44e5b316..f4dc4a60 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 v14) +# Topic: create-vale-rule (CLI v%(CLI_VERSION)s / topic v15) ## You are here This is `create-vale-rule`. It helps you write a Vale rule: a check over @@ -199,12 +199,11 @@ it. `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. + **A leaf element reads its own text either way.** `doc(h2)` alone and + `text & doc(h2)` both lint the heading's text, and the same holds for + `doc(p)` and `doc(li)`. Prefer the chained form anyway: it says which + text you mean, and a rule written for Vale 3.22.0 or earlier, where + the standalone form matched nothing, already uses it. **The selector is Vale's to check, not `verify`'s.** A selector Vale cannot compile (`doc(h2[)`) fails the whole run at load with @@ -778,8 +777,11 @@ it. - **comment text only**: the comments are linted and the code body is invisible, which is exactly right for "comments must not say - 'obviously'": + 'obviously'". + A comment addressed to a tool (`//nolint`, `# noqa`, + `eslint-disable`) and a docstring's `:param:` field are not read at + all, so a rule cannot police what a suppression comment says: %(VALE_COMMENT_FORMATS)s - **plaintext fallback**: everything else, `.yml` `.toml` `.sh` `.sql` and every extension not named above included. There is no diff --git a/packages/cli/src/agent/update.md b/packages/cli/src/agent/update.md index 93bccb27..bf31bcaf 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 v11) +# Topic: update (CLI v%(CLI_VERSION)s / topic v12) ## You are here This is `update`. It tells you what an upgrade changed for the rules @@ -371,9 +371,11 @@ on a fixture path you name the matcher is what silences the rule. Delete it. `%(TASKLESS_CLI)s agent create-vale-rule` lists every rejection and advisory. -Vale also moves from 3.21.0 to 3.22.0 in this release. Two things -follow for existing Vale rules, both measured against both binaries; -the rest of the release is additions a rule can now use. +Vale also moves from 3.21.0 to 3.23.0 in this release, by way of +3.22.0. Two things from 3.22.0 change what an existing Vale rule does, +and 3.23.0 narrows what a rule over code comments reads; each was +measured against the binaries on both sides of its step. The rest is +additions a rule can now use. **`BasedOnStyles =` is deleted from every rule's `.vale.ini`, and `verify` rejects one that still carries it.** Every version of the @@ -428,6 +430,27 @@ each at its own position (85992f2a); 3.21.0 accepted the key and did neither. `UNSET` as a rule's value behaves as `NO` and is not accepted by the schema; YES and NO remain the two values. +**Vale 3.23.0 reads less of a code file.** Nothing to run; findings +disappear, and none appear that were not already there. A comment +addressed to a tool (`//nolint` in Go, `# noqa` in Python, +`eslint-disable` in JavaScript) and a Python docstring's `:param:` +field are no longer read, so a rule over comments stops reporting on +them. Kotlin (`.kt`, `.kts`) was linted as one block of prose, string +literals and identifiers included, and is now comment-aware like the +other code formats: a rule matching `[*.kt]` sees comments only. If a +rule was written to catch something in Kotlin code rather than its +comments, it no longer can; an ast-grep rule is the tool for that. + +**`doc()` on its own now fires.** Through 3.22.0 `scope: doc(h2)` +matched nothing, and the recipe taught `text & doc(h2)` instead. Both +spellings now lint the heading's text, so a rule written the chained +way needs nothing. A rule that used the standalone form was inert +until now and starts reporting. + +**A rule's `tests:` key is accepted.** It is read by `vale test`, not +by `check`, and `verify` rejected it before this release while Vale +loaded it without complaint. + ## Errors With `--json`, `--rules` failures emit `{ ok: false, code, message }`: diff --git a/packages/cli/src/generated/vale-vocabulary.ts b/packages/cli/src/generated/vale-vocabulary.ts index 7180930c..2215d0e7 100644 --- a/packages/cli/src/generated/vale-vocabulary.ts +++ b/packages/cli/src/generated/vale-vocabulary.ts @@ -66,6 +66,7 @@ export const VALE_COMMON_FIELDS = [ "message", "name", "scope", + "tests", ] as const; /** @@ -99,6 +100,7 @@ export const VALE_LITERAL_KEYS = [ "message", "name", "scope", + "tests", ] as const; /** diff --git a/packages/cli/src/rules/capabilities.ts b/packages/cli/src/rules/capabilities.ts index 52a00a55..5b25304b 100644 --- a/packages/cli/src/rules/capabilities.ts +++ b/packages/cli/src/rules/capabilities.ts @@ -271,6 +271,31 @@ 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. * + * The upgrade pull request lists the files upstream ADDED under + * `internal/lint/` (`.github/scripts/vale-upgrade-report.cjs`), which is the + * source check the notes below did by hand. It names the readers, not the + * extensions, so each one still needs `internal/core/format.go` read and the + * extension probed before a row is added. + * + * 3.22.0 → 3.23.0 LEARNED ONE FORMAT, AND NO TEST NOTICED UNTIL IT WAS + * PROBED. Every existing row was re-probed against the 3.23.0 binary and none + * moved, so the contract suite was green. The source check is what shows the + * rest: v3.22.0...v3.23.0 adds `internal/lint/code/kt.go` and + * `internal/lint/code/toml.go`, and `internal/core/format.go` routes `.kt` and + * `.kts` to `code`, so Kotlin left the unnamed plaintext fallback for the + * `comment` tier and is two new rows below. The benign direction, and a + * narrowing like `.ex` on 3.19.0: a rule matching `[*.kt]` fired on string + * literals and identifiers through 3.22.0 and now sees comments only. `.toml` + * did NOT move with it. `format.go` routes `.toml`, `.yml` and `.yaml` to + * `data`, which reads comments only through a tree-sitter View, and a rule + * config here cannot define one, so all three still lint as plaintext. + * + * Also in that tree, and not a tier change: comment-tier formats now drop a + * comment addressed to a tool (`//nolint`, `# noqa`, `eslint-disable`) and a + * docstring's `:param:` field, and a standalone `doc()` scope reads the + * leaf's text where it was inert. All three are pinned in + * `test/vale-vendor-contract.test.ts`. + * * 3.21.0 → 3.22.0 LEARNED NO FORMAT. Every row was re-probed against the * 3.22.0 binary and none moved. The v3.21.0...v3.22.0 tree adds no * `internal/lint/.go` (its additions there are two `_test.go` files); @@ -397,6 +422,8 @@ export const VALE_FORMAT_TIERS: Readonly> = { ".jl": "comment", ".js": "comment", ".jsx": "comment", + ".kt": "comment", + ".kts": "comment", ".less": "comment", ".lua": "comment", ".php": "comment", diff --git a/packages/cli/test/vale-corpus.ts b/packages/cli/test/vale-corpus.ts index b2fe65e6..217496a9 100644 --- a/packages/cli/test/vale-corpus.ts +++ b/packages/cli/test/vale-corpus.ts @@ -512,28 +512,25 @@ 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. + // The `doc(...)` family, new in Vale 3.21.0, added 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. + // + // A leaf element on its own was the silent one, inert from 3.21.0 through + // 3.22.0. Vale 3.23.0 reads the leaf's own text, so the row now agrees with + // the schema and carries no divergence. It stays, because the recipe used to + // teach around the old verdict, and a Vale that went back would need the + // recipe to go back too. { 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.", + expected: "accepted", }, { name: "scope/doc-invalid-selector", @@ -683,6 +680,16 @@ const FIELDS: ValeCorpusEntry[] = [ control: PROSE, expected: "accepted", }, + { + // Read by `vale test`, announced in 3.23.0 and already accepted by + // 3.22.0. Absent from the generator's candidates until 3.23.0, so + // `verify` rejected it as an E201 Vale never raised. + name: "field/existence+tests", + construct: "tests", + rule: existence("tests:\n - fixture.md\n"), + control: PROSE, + expected: "accepted", + }, { name: "field/existence+vocab", construct: "vocab", diff --git a/packages/cli/test/vale-vendor-contract.test.ts b/packages/cli/test/vale-vendor-contract.test.ts index 7f98862a..097ff67f 100644 --- a/packages/cli/test/vale-vendor-contract.test.ts +++ b/packages/cli/test/vale-vendor-contract.test.ts @@ -1175,17 +1175,15 @@ withVale("Vale vendor contract", () => { 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. + it("reads a leaf element's own text when chained", () => { + // `text & doc(h2)` reads the heading's own block. On 3.21.0 and + // 3.22.0 it was the only spelling that did: `doc(h2)` alone matched + // nothing, with no error anywhere, and the recipe taught the chained + // form around that. 3.23.0 made the standalone form fire too (pinned + // in the 3.23.0 block below), so the chained one is no longer the + // workaround, but it is still what the recipe shows, and if it stops + // firing the recipe 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]); @@ -1753,6 +1751,71 @@ withVale("Vale vendor contract", () => { ]); }); }); + + describe("Vale 3.23.0", () => { + // Every case below was run against both binaries, 3.22.0 and 3.23.0, on + // the same fixture, and differs between them. Each is a narrowing or a + // closed trap: findings appear where a rule was inert, or disappear from + // text that was never prose. Nothing here fails a run. + + it("reads a leaf element on its own, where 3.22.0 was inert", () => { + // Upstream 459d3cda, "Selectors Level 4 in doc(...)". Through 3.22.0 a + // standalone `doc(h2)` selected the heading and linted what was inside + // it as one block, which for a leaf is nothing: no finding, no error. + // The corpus row `scope/doc-leaf-standalone` recorded that as a + // divergence and the recipe taught `text & doc(h2)` around it. Both + // spellings now fire on the same line. + const document = "# Title\n\n## Decision\n\nWe will do it.\n"; + const heading = `extends: existence\nmessage: "%s"\nlevel: warning\nscope: 'SCOPE'\ntokens:\n - Decision\n`; + expect( + lines(heading.replace("SCOPE", "doc(h2)"), document).lines + ).toEqual([3]); + expect( + lines(heading.replace("SCOPE", "text & doc(h2)"), document).lines + ).toEqual([3]); + }); + + describe("a comment addressed to a tool is not read", () => { + // Upstream 0ea6b7a1, "mask doc-comment conventions". 3.22.0 linted each + // of these directive comments as prose, so a rule over comments fired + // on a linter's suppression list. The ordinary comment beside each one + // is the control: the file is still comment-aware, only the directive + // is dropped. The masking is per language: a `// eslint-disable` line + // in a `.go` file is still read, because it is not Go's convention. + const rule = existence("simply"); + + it("drops a Go //nolint line", () => { + expect( + lines(rule, "//nolint:simply\n// simply here\nfunc f() {}\n", "a.go") + .lines + ).toEqual([2]); + }); + + it("drops a Python # noqa line", () => { + expect( + lines(rule, "# noqa: simply\n# simply here\nx = 1\n", "a.py").lines + ).toEqual([2]); + }); + + it("drops a JavaScript eslint-disable line", () => { + expect( + lines( + rule, + "// eslint-disable-next-line simply\n// simply here\nconst a = 1;\n", + "a.js" + ).lines + ).toEqual([2]); + }); + }); + + it("does not read a docstring's :param: field as prose", () => { + // Same upstream commit. The docstring's own sentence is still linted; + // the field naming the parameter is not. 3.22.0 reported both. + const document = + 'def f(simply):\n """Do it simply.\n\n :param simply: a flag\n """\n'; + expect(lines(existence("simply"), document, "a.py").lines).toEqual([2]); + }); + }); }); /**