diff --git a/.changeset/vale-3-26-0.md b/.changeset/vale-3-26-0.md new file mode 100644 index 00000000..f589bb39 --- /dev/null +++ b/.changeset/vale-3-26-0.md @@ -0,0 +1,5 @@ +--- +"@taskless/cli": patch +--- + +Update the bundled Vale to 3.26.0. `.mjs`, `.cjs`, `.mts` and `.cts` files are now read as code, so a Vale rule over them sees comments only, as it already did for `.js` and `.ts`. A phrase in a rule now matches across a line wrap, and a `spelling` rule no longer flags common technical words such as `OAuth` and `npm`. diff --git a/.github/scripts/vale-upgrade-report.cjs b/.github/scripts/vale-upgrade-report.cjs index 94ef5914..4ee5b6cb 100644 --- a/.github/scripts/vale-upgrade-report.cjs +++ b/.github/scripts/vale-upgrade-report.cjs @@ -34,6 +34,14 @@ * run. Listing the files upstream ADDED under `internal/lint/` is the source * check, mechanically. * + * AND THE ROUTING TABLE, BECAUSE A FORMAT CAN BE LEARNED WITHOUT A NEW FILE. + * 3.26.0 sent `.mjs`, `.cjs`, `.mts` and `.cts` to the existing JavaScript and + * TypeScript readers with a two-line edit to `FormatByExtension` in + * `internal/core/format.go`, while the four files it did add were build-tag + * splits for a WebAssembly build that the native CLI never reaches. The list + * above pointed a reviewer at four non-formats and said nothing about four + * real ones. So the report also prints that file's patch, whenever it changed. + * * 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". @@ -43,6 +51,9 @@ const { readFileSync, writeFileSync } = require("node:fs"); const { baseVersion } = require("./vale-upgrade-detect.cjs"); +/** Where upstream maps an extension to the reader that handles it. */ +const ROUTING_FILE = "internal/core/format.go"; + /** The compare API returns at most this many files, with no marker when cut. */ const COMPARE_FILE_CAP = 300; @@ -66,7 +77,9 @@ function summarizeContract(report) { for (const assertion of file.assertionResults ?? []) { if (assertion.status !== "failed") continue; failed.push({ - file: String(file.name ?? "").split("/").pop(), + file: String(file.name ?? "") + .split("/") + .pop(), name: assertion.fullName, message: String(assertion.failureMessages?.[0] ?? "").split("\n")[0], }); @@ -105,24 +118,32 @@ async function fetchAddedReaders(repository, from, to) { throw new Error(`GET ${url} responded ${response.status}`); } const files = (await response.json()).files ?? []; + const routing = files.find((file) => file.filename === ROUTING_FILE); return { added: files .filter((file) => file.status === "added" && isReader(file.filename)) .map((file) => file.filename) .toSorted(), + // The patch text, or `undefined` when the file did not change. The API + // omits `patch` for a diff too large to inline, which reads as a change + // with nothing to show rather than as no change. + routing: + routing === undefined + ? undefined + : (routing.patch ?? "(patch too large to inline; read it in the diff)"), 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) { +function fence(text, info = "") { const longest = Math.max( 2, ...[...text.matchAll(/`+/g)].map((match) => match[0].length) ); const marker = "`".repeat(longest + 1); - return `${marker}\n${text}\n${marker}`; + return `${marker}${info}\n${text}\n${marker}`; } function formatContract(contract) { @@ -147,11 +168,20 @@ function formatReaders(readers, error) { 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}).`; + const routing = formatRouting(readers.routing); 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`; + return `${heading}\n\nNone under \`${READER_PREFIX}\` ([diff](${readers.url})).${caveat}\n${routing}`; } 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`; + 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 \`${ROUTING_FILE}\` 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. A file whose first line is \`//go:build js\` belongs to the WebAssembly build only, and the CLI never reaches it. ([diff](${readers.url}))${caveat}\n${routing}`; +} + +function formatRouting(patch) { + const heading = `### Extension routing (\`${ROUTING_FILE}\`)`; + if (patch === undefined) { + return `\n${heading}\n\nUnchanged. With no reader added either, no extension can have changed tier without a row that re-probes it.\n`; + } + return `\n${heading}\n\n**Changed.** An extension added to \`FormatByExtension\` is a format Vale learned, even when no reader file was added. Probe each one against the new binary and add a row to \`VALE_FORMAT_TIERS\`.\n\n${fence(patch, "diff")}\n`; } function formatReport({ from, to, contract, readers, readersError }) { @@ -176,7 +206,9 @@ async function main({ const repository = readArgument(argv, "--repository"); const out = readArgument(argv, "--out"); - const contract = summarizeContract(readReport(readArgument(argv, "--contract"))); + const contract = summarizeContract( + readReport(readArgument(argv, "--contract")) + ); let readers; let readersError; try { @@ -193,6 +225,7 @@ async function main({ module.exports = { COMPARE_FILE_CAP, + ROUTING_FILE, fetchAddedReaders, formatReport, isReader, diff --git a/.github/scripts/vale-upgrade-report.test.cjs b/.github/scripts/vale-upgrade-report.test.cjs index 10ad7604..16a7d76e 100644 --- a/.github/scripts/vale-upgrade-report.test.cjs +++ b/.github/scripts/vale-upgrade-report.test.cjs @@ -75,7 +75,10 @@ test("contract: names each failure with its file and first message line", () => ]); const section = report({ contract }); assert.match(section, /\*\*1 of 204 failed\.\*\*/); - assert.match(section, /vale-schema-contract\.test\.ts > Vale schema contract/); + assert.match( + section, + /vale-schema-contract\.test\.ts > Vale schema contract/ + ); assert.doesNotMatch(section, /at somewhere/); }); @@ -108,7 +111,10 @@ test("readers: lists only added, non-test Go files under internal/lint/", () => 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"]), + 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/); @@ -122,7 +128,10 @@ test("readers: none added is said, with the diff to check it by", () => { }); 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" }); + 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/); }); @@ -188,8 +197,28 @@ test("main: a fetch that throws is reported, not raised", async () => { }, readReport: () => vitestReport(), }); - assert.match(readFileSync(out, "utf8"), /Could not check\*\* \(GET x responded 403\)/); + assert.match( + readFileSync(out, "utf8"), + /Could not check\*\* \(GET x responded 403\)/ + ); } finally { rmSync(directory, { recursive: true, force: true }); } }); + +test("routing: a changed format.go prints its patch, even with no reader added", () => { + // 3.26.0: four `.mjs`-style spellings learned in format.go, no reader added + // for them, and the added readers were all WebAssembly build splits. + const patch = + '- `\\.(?:js|jsx)$`: {".js", "code"},\n+ `\\.(?:js|jsx|mjs|cjs)$`: {".js", "code"},'; + const section = report({ readers: { ...readers([]), routing: patch } }); + assert.match(section, /\*\*Changed\.\*\*/); + assert.match(section, /^```diff$/m); + assert.ok(section.includes("mjs|cjs"), "the patch itself is in the body"); + assert.doesNotMatch(section, /Unchanged\./); +}); + +test("routing: an unchanged format.go is said, so silence is not mistaken for a check", () => { + const section = report({}); + assert.match(section, /internal\/core\/format\.go`\)\n\nUnchanged\./); +}); diff --git a/.taskless/taskless.json b/.taskless/taskless.json index d81bc43a..9ec0ff8b 100644 --- a/.taskless/taskless.json +++ b/.taskless/taskless.json @@ -28,7 +28,7 @@ "reconciledTo": "0.13.4", "engines": { "sg": "0.51.0", - "vale": "3.24.0" + "vale": "3.26.0" } } } diff --git a/packages/cli/package.json b/packages/cli/package.json index a9b24af3..968d921a 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -92,11 +92,11 @@ "@ast-grep/cli-win32-arm64-msvc": "0.51.0", "@ast-grep/cli-win32-ia32-msvc": "0.51.0", "@ast-grep/cli-win32-x64-msvc": "0.51.0", - "@taskless/vale-darwin-arm64": "3.24.0-20261005234241", - "@taskless/vale-darwin-x64": "3.24.0-20261005234241", - "@taskless/vale-linux-arm64": "3.24.0-20261005234241", - "@taskless/vale-linux-x64": "3.24.0-20261005234241", - "@taskless/vale-win32-arm64": "3.24.0-20261005234241", - "@taskless/vale-win32-x64": "3.24.0-20261005234241" + "@taskless/vale-darwin-arm64": "3.26.0-20261102164455", + "@taskless/vale-darwin-x64": "3.26.0-20261102164455", + "@taskless/vale-linux-arm64": "3.26.0-20261102164455", + "@taskless/vale-linux-x64": "3.26.0-20261102164455", + "@taskless/vale-win32-arm64": "3.26.0-20261102164455", + "@taskless/vale-win32-x64": "3.26.0-20261102164455" } } diff --git a/packages/cli/src/agent/update.md b/packages/cli/src/agent/update.md index fb09d153..846a6769 100644 --- a/packages/cli/src/agent/update.md +++ b/packages/cli/src/agent/update.md @@ -630,6 +630,29 @@ What needs you, after `init`: with the reason, and `verify` fails the rule until it is gone. Move those files by hand into case directories, then delete `.tests/`. +### Migrating to 0.13.5 + +Vale moves from 3.24.0 to 3.26.0. Nothing here needs work by hand. +Three things change which findings an existing rule reports, each +measured against both binaries on the same fixture. + +**`.mjs`, `.cjs`, `.mts` and `.cts` are read as code.** Through 3.24.0 +Vale linted them as one block of prose, so a rule matching `[*.mjs]` +fired on `const simply = 1` as readily as on a comment. They now go +through the JavaScript and TypeScript readers, which read comments +only, the way `.js` and `.ts` always were. Findings in code disappear; +findings in comments stay. + +**A phrase matches across a line wrap.** A rule with the token +`click here` missed `click` at the end of one line and `here` at the +start of the next. It now reports it, so a wrapped Markdown paragraph +can produce findings it did not before. + +**Vale's built-in dictionary knows common technical words.** A +`spelling` rule no longer flags words like `OAuth` and `npm`, so +findings disappear and a project vocabulary entry that existed only +for such a word is now redundant. Leave it; it does no harm. + ## 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 55fc45fd..dc0e02e3 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.24.0 vocabulary: divergence report +# Vale 3.26.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.24.0 binary. This file is what the binary said that its own +3.26.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.24.0 documents this operand and it never fired, on any fixture probed (.md). +Vale 3.26.0 documents this operand and it never fired, on any fixture probed (.md). **Consequence.** It is omitted from the vocabulary, so `validate` 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.24.0 documents this operand and it never fired, on any fixture probed (.md). +Vale 3.26.0 documents this operand and it never fired, on any fixture probed (.md). **Consequence.** It is omitted from the vocabulary, so `validate` 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.24.0 documents it nowhere. +This operand fired and Vale 3.26.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.24.0 documents it nowhere. +This operand fired and Vale 3.26.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. diff --git a/packages/cli/src/generated/vale-vocabulary.ts b/packages/cli/src/generated/vale-vocabulary.ts index dd596730..0d5d477d 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.24.0. Every value here is the recorded answer + * Derived against Vale 3.26.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.24.0"; +export const VALE_VOCABULARY_VERSION = "3.26.0"; /** * Vale's check types, self-enumerated: an unknown `extends` makes the binary @@ -199,7 +199,7 @@ export const VALE_SCOPE_PREFIXES = [ ] as const; /** - * Where Vale 3.24.0 and its documentation disagree. + * Where Vale 3.26.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. @@ -208,26 +208,26 @@ export const VALE_DIVERGENCES = [ { subject: "scope: meta", finding: - "Vale 3.24.0 documents this operand and it never fired, on any fixture probed (.md).", + "Vale 3.26.0 documents this operand and it never fired, on any fixture probed (.md).", consequence: "It is omitted from the vocabulary, so `validate` 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.24.0 documents this operand and it never fired, on any fixture probed (.md).", + "Vale 3.26.0 documents this operand and it never fired, on any fixture probed (.md).", consequence: "It is omitted from the vocabulary, so `validate` 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.24.0 documents it nowhere.", + finding: "This operand fired and Vale 3.26.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.24.0 documents it nowhere.", + finding: "This operand fired and Vale 3.26.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.", }, diff --git a/packages/cli/src/rules/capabilities.ts b/packages/cli/src/rules/capabilities.ts index f32df291..aff4571f 100644 --- a/packages/cli/src/rules/capabilities.ts +++ b/packages/cli/src/rules/capabilities.ts @@ -210,7 +210,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.24.0"; +export const VALE_VERSION = "3.26.0"; /** * Which tier Vale routes an extension to. @@ -278,6 +278,22 @@ const CONVERTER_TIER_PREFIX = "converter:"; * extensions, so each one still needs `internal/core/format.go` read and the * extension probed before a row is added. * + * 3.24.0 → 3.26.0 LEARNED FOUR SPELLINGS, AND THE UPGRADE REPORT POINTED AT + * THE WRONG FILES. It named four readers added under `internal/lint/`, and + * none of them is a format: `code_js.go`, `code_ts.go` and + * `native_stubs_js.go` split the existing code and converter handlers by + * build tag for the new WebAssembly build, and `built.go` lints a converted + * format from HTML a JavaScript host supplies, which only that build reaches. + * The native CLI routes exactly as before through all four. The learned + * formats came from a two-line edit to `internal/core/format.go` (upstream + * a07aa34c), which no added file announces: `.mjs` and `.cjs` now route to + * `.js`, and `.mts` and `.cts` to `.ts`, so all four left the unnamed + * plaintext fallback for the `comment` tier and are new rows below. The + * benign narrowing again: measured on both binaries, a rule matching + * `[*.mjs]` fired on `const simply = 1` through 3.24.0 and reads only the + * comment on 3.26.0. Read the `format.go` diff on every bump, not only the + * added files. + * * 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 @@ -406,10 +422,12 @@ export const VALE_FORMAT_TIERS: Readonly> = { ".c++": "comment", ".bsh": "comment", ".cc": "comment", + ".cjs": "comment", ".clj": "comment", ".cpp": "comment", ".cs": "comment", ".css": "comment", + ".cts": "comment", ".csx": "comment", ".cxx": "comment", ".ex": "comment", @@ -427,6 +445,8 @@ export const VALE_FORMAT_TIERS: Readonly> = { ".kts": "comment", ".less": "comment", ".lua": "comment", + ".mjs": "comment", + ".mts": "comment", ".php": "comment", ".pl": "comment", ".pm": "comment", diff --git a/packages/cli/src/rules/vale/run.ts b/packages/cli/src/rules/vale/run.ts index 4b4c6798..847f7659 100644 --- a/packages/cli/src/rules/vale/run.ts +++ b/packages/cli/src/rules/vale/run.ts @@ -111,22 +111,93 @@ export type ValeRunOutcome = | { status: "failed"; blocking: true; message: string }; /** - * Parse Vale's stderr as its one-object config-error document, or `undefined` - * when it is not that shape. + * Split concatenated top-level JSON objects into one string each, or + * `undefined` when `text` is anything else. + * + * Tracks brace depth outside strings, so a `{` or `}` inside an error's + * `Text` cannot end an object early. Whitespace between objects is allowed; + * anything else between them means this was not an object stream. + */ +function splitJsonObjects(text: string): string[] | undefined { + const objects: string[] = []; + let depth = 0; + let start = -1; + let inString = false; + let escaped = false; + for (let index = 0; index < text.length; index += 1) { + const char = text[index]; + if (inString) { + if (escaped) escaped = false; + else if (char === "\\") escaped = true; + else if (char === '"') inString = false; + continue; + } + if (depth === 0) { + if (char === "{") { + start = index; + depth = 1; + } else if (!/\s/.test(char ?? "")) { + return undefined; + } + continue; + } + switch (char) { + case '"': { + inString = true; + break; + } + case "{": { + depth += 1; + break; + } + case "}": { + depth -= 1; + if (depth === 0) objects.push(text.slice(start, index + 1)); + break; + } + } + } + return depth === 0 && objects.length > 0 ? objects : undefined; +} + +/** + * Parse Vale's stderr as the error objects it holds, or `undefined` when any + * part of it is not that shape. + * + * Through 3.24.0 a failed run wrote ONE object and stopped at the first + * document it could not parse. Since 3.26.0 (upstream b5b8e429) Vale lints + * every other document, exits 2, and writes one object per document it could + * not read, back to back. Parsing the stream as one JSON document failed on + * the second object, which lost every `Path` and with it the narrowing that + * keeps unparseable files from taking every other file's findings down + * (taskless/cli#300). Measured on both binaries with two documents whose front + * matter is invalid YAML. + * + * All or nothing: a stream with one object that is not an error is returned + * as `undefined`, so the caller reports the raw bytes rather than a partial + * reading of them. * * Split out from {@link describeValeStderr} so the non-zero-exit branch in * {@link spawnVale} can parse `stderr` exactly once and use the result both to - * build the failure message and to populate `ValeAttempt.configError` — the - * value {@link targetFileParseError} reads to decide whether this failure can - * be narrowed to one target file and retried. Without this split, the same - * bytes were parsed twice: once here, once again inside `describeValeStderr`. + * build the failure message and to populate `ValeAttempt.configErrors`, which + * {@link targetFileParseError} reads to decide whether this failure can be + * narrowed to target files and retried. */ -function parseValeConfigError(stderr: string): ValeConfigError | undefined { - try { - return asValeConfigError(JSON.parse(stderr)); - } catch { - return undefined; +function parseValeConfigErrors(stderr: string): ValeConfigError[] | undefined { + const objects = splitJsonObjects(stderr); + if (objects === undefined) return undefined; + const errors: ValeConfigError[] = []; + for (const object of objects) { + let error: ValeConfigError | undefined; + try { + error = asValeConfigError(JSON.parse(object)); + } catch { + return undefined; + } + if (error === undefined) return undefined; + errors.push(error); } + return errors; } /** A line Vale's `core.Warn` wrote: its code, a space, then the message. */ @@ -173,17 +244,19 @@ function splitValeWarnings(stderr: string): { * decoding ast-grep's stderr rather than forwarding bytes — the message is the * only thing the user has to act on. * - * Takes the already-parsed error rather than re-parsing `stderr` itself — see - * {@link parseValeConfigError}. Anything that did not parse to that shape is + * Takes the already-parsed errors rather than re-parsing `stderr` itself — see + * {@link parseValeConfigErrors}. Anything that did not parse to that shape is * returned untouched. A best-effort decoder that swallows what it cannot read * would be worse than none. */ function describeValeStderr( stderr: string, - configError: ValeConfigError | undefined + configErrors: readonly ValeConfigError[] | undefined ): string { - if (configError === undefined) return stderr; - return formatValeConfigError(configError, { withPath: true }); + if (configErrors === undefined) return stderr; + return configErrors + .map((error) => formatValeConfigError(error, { withPath: true })) + .join("; "); } /** @@ -330,7 +403,11 @@ async function targetFileParseError( type ValeAttempt = | { status: "ok"; results: CheckResult[]; notices: string[] } | { status: "timeout"; message: string } - | { status: "failed"; message: string; configError?: ValeConfigError }; + | { + status: "failed"; + message: string; + configErrors?: readonly ValeConfigError[]; + }; /** * Spawn Vale once and map what it reports. Extracted from {@link runVale} so @@ -426,18 +503,18 @@ async function spawnVale( const { warnings, rest } = splitValeWarnings( stderrChunks.join("").trim() ); - const configError = parseValeConfigError(rest); + const configErrors = parseValeConfigErrors(rest); // The error first, because it is why the run stopped; the warnings // after it, because one may be the reason (a rule switched off by a // misspelled level). const said = [ - ...(rest === "" ? [] : [describeValeStderr(rest, configError)]), + ...(rest === "" ? [] : [describeValeStderr(rest, configErrors)]), ...warnings, ].join(" "); settle({ status: "failed", message: `Vale exited ${String(code)}${said === "" ? "" : `: ${said}`}`, - ...(configError === undefined ? {} : { configError }), + ...(configErrors === undefined ? {} : { configErrors }), }); return; } @@ -487,7 +564,7 @@ async function spawnVale( settle({ status: "failed", message: `Vale rejected the configuration: ${formatValeConfigError(configError, { withPath: true })}`, - configError, + configErrors: [configError], }); return; } @@ -685,16 +762,18 @@ export async function runVale( // one file that triggered it. Unlike that case there is no format to // preemptively exclude; which file is bad is only known once Vale says so. // - // So this retries: on a failure Vale's own error object attributes to one - // of *our* target files (`targetFileParseError`), that file is added to the + // So this retries: when every error object Vale wrote names one of *our* + // target files (`targetFileParseError`), those files are added to the // exclusion glob and the whole thing is asked again, with a finding - // recorded for the file that was dropped. A failure that cannot be - // attributed to a single target file — a bad rule, a timeout, a crash — is - // not this bug, and is reported exactly as before: blocking, with nothing - // to retry around. + // recorded for each file that was dropped. Since 3.26.0 Vale reports every + // unreadable document in one run, so one retry usually covers them all; + // through 3.24.0 it stopped at the first, and this took one retry per file. + // A failure that cannot be attributed to target files — a bad rule, a + // timeout, a crash — is not this bug, and is reported exactly as before: + // blocking, with nothing to retry around. // // Bounded by construction rather than by a counter: every successful - // iteration excludes one target file that was not already excluded, and + // iteration excludes at least one target file that was not already excluded, and // there are finitely many files to exclude. Re-reporting the same path // twice in a row is the only way this could spin, and that path is refused // rather than retried (see the `excludedTargets.has` check below). @@ -746,21 +825,28 @@ export async function runVale( return { status: "timeout", blocking: true, message: attempt.message }; } - const { configError } = attempt; - const candidate = - configError === undefined - ? undefined - : await targetFileParseError(configError, options.cwd); - - if (candidate === undefined || configError === undefined) { + // Every error must name a target file this run has not excluded yet. + // One that does not (a bad rule, a style, a file already dropped) is a + // failure no exclusion can route around, and the run reports it whole + // rather than retrying past the part it can attribute. + const { configErrors } = attempt; + if (configErrors === undefined) { return { status: "failed", blocking: true, message: attempt.message }; } - if (excludedTargets.has(candidate)) { - return { status: "failed", blocking: true, message: attempt.message }; + const dropped: { candidate: string; error: ValeConfigError }[] = []; + for (const error of configErrors) { + const candidate = await targetFileParseError(error, options.cwd); + if (candidate === undefined || excludedTargets.has(candidate)) { + return { status: "failed", blocking: true, message: attempt.message }; + } + dropped.push({ candidate, error }); } - excludedTargets.add(candidate); - excludedFindings.push(parseErrorResult(candidate, configError)); + for (const { candidate, error } of dropped) { + if (excludedTargets.has(candidate)) continue; + excludedTargets.add(candidate); + excludedFindings.push(parseErrorResult(candidate, error)); + } } } diff --git a/packages/cli/test/reconcile-marker.test.ts b/packages/cli/test/reconcile-marker.test.ts index 1868b943..a41f76f9 100644 --- a/packages/cli/test/reconcile-marker.test.ts +++ b/packages/cli/test/reconcile-marker.test.ts @@ -596,6 +596,15 @@ describe("taskless update with no flags", () => { expect(result.stdout).toContain("Do not rename it by hand"); }); + it("carries the 0.13.5 ledger entry", async () => { + const result = await runCli(["update"]); + expect(result.stdout).toContain("Migrating to 0.13.5"); + expect(result.stdout).toContain("Vale moves from 3.24.0 to 3.26.0"); + expect(result.stdout).toContain( + "`.mjs`, `.cjs`, `.mts` and `.cts` are read as code" + ); + }); + 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-vendor-contract.test.ts b/packages/cli/test/vale-vendor-contract.test.ts index bb660974..f0470f36 100644 --- a/packages/cli/test/vale-vendor-contract.test.ts +++ b/packages/cli/test/vale-vendor-contract.test.ts @@ -501,6 +501,38 @@ withVale("Vale vendor contract", () => { expect(isAbsolute(ruleError.Path ?? "")).toBe(true); }); + it("reports every unreadable target file, one error object each, and lints the rest", () => { + // Depended on by: `parseValeConfigErrors` in run.ts, which reads stderr + // as a stream of objects rather than one document. Through 3.24.0 Vale + // stopped at the first unreadable document: one object, empty stdout. + // Since 3.26.0 (upstream b5b8e429) it lints the others, prints their + // alerts, exits 2, and writes one object per unreadable document. Read + // as one JSON document, two objects did not parse, the narrowing lost + // every `Path`, and the run failed with every finding gone: #300 again. + const bad = + "---\ndescription: has a colon: right here\n---\n\nJust simply do it.\n"; + const cwd = project( + `${header}\n[*.md]\nrules.lvl = YES\n`, + { lvl: existence("simply") }, + { "good.md": "Just simply do it.\n", "bad-1.md": bad, "bad-2.md": bad } + ); + const result = runRaw( + cwd, + ["good.md", "bad-1.md", "bad-2.md"], + ["--no-exit"] + ); + expect(result.status).toBe(2); + expect(Object.keys(JSON.parse(result.stdout) as object)).toEqual([ + "good.md", + ]); + const objects = result.stderr.trim().split(/\n(?=\{)/); + expect( + objects + .map((object) => (JSON.parse(object) as { Path?: string }).Path) + .toSorted() + ).toEqual(["bad-1.md", "bad-2.md"]); + }); + describe("matcher semantics", () => { const rules = { "no-simply": existence("simply"), diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a7bb6ead..33803d5a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -164,23 +164,23 @@ importers: specifier: 0.51.0 version: 0.51.0 '@taskless/vale-darwin-arm64': - specifier: 3.24.0-20261005234241 - version: 3.24.0-20261005234241 + specifier: 3.26.0-20261102164455 + version: 3.26.0-20261102164455 '@taskless/vale-darwin-x64': - specifier: 3.24.0-20261005234241 - version: 3.24.0-20261005234241 + specifier: 3.26.0-20261102164455 + version: 3.26.0-20261102164455 '@taskless/vale-linux-arm64': - specifier: 3.24.0-20261005234241 - version: 3.24.0-20261005234241 + specifier: 3.26.0-20261102164455 + version: 3.26.0-20261102164455 '@taskless/vale-linux-x64': - specifier: 3.24.0-20261005234241 - version: 3.24.0-20261005234241 + specifier: 3.26.0-20261102164455 + version: 3.26.0-20261102164455 '@taskless/vale-win32-arm64': - specifier: 3.24.0-20261005234241 - version: 3.24.0-20261005234241 + specifier: 3.26.0-20261102164455 + version: 3.26.0-20261102164455 '@taskless/vale-win32-x64': - specifier: 3.24.0-20261005234241 - version: 3.24.0-20261005234241 + specifier: 3.26.0-20261102164455 + version: 3.26.0-20261102164455 packages/vale-darwin-arm64: {} @@ -877,33 +877,33 @@ packages: '@standard-schema/spec@1.1.0': resolution: {integrity: sha512-l2aFy5jALhniG5HgqrD6jXLi/rUWrKvqN/qJx6yoJsgKhblVd+iqqU4RCXavm/jPityDo5TCvKMnpjKnOriy0w==} - '@taskless/vale-darwin-arm64@3.24.0-20261005234241': - resolution: {integrity: sha512-G3ku4OFH1gf6qccS2dOT0NrKIigCK1oo2kcJCYkZAju+t1RAcwM0eJCnK/ofrOFUViCWtzQy4uh+CzxLIDZQaA==} + '@taskless/vale-darwin-arm64@3.26.0-20261102164455': + resolution: {integrity: sha512-xr26EEnknO0gUrIK2bnUbNFjPF+dA++Wqeb/r/GN98moa6BqTNwAiZD+Gc+KEarhSWInxJMSTatLdCEP3vAEFg==} cpu: [arm64] os: [darwin] - '@taskless/vale-darwin-x64@3.24.0-20261005234241': - resolution: {integrity: sha512-Mx8HKLl+QgxMBGvIvWFc8cUhV1wdhhf/vZsooB8mKHh1oNPYOyLfpz327FuNxZ2h7WM3r/a4j3/1zFKh0r/E4w==} + '@taskless/vale-darwin-x64@3.26.0-20261102164455': + resolution: {integrity: sha512-swP5oYGaOeaNNHNxnxBoOSa1yEbPomrcvTRZe2efQcc6DNg6U/FdGm2tGJ/0QKVfdQtDP0sI3pYfPcyevfIRFQ==} cpu: [x64] os: [darwin] - '@taskless/vale-linux-arm64@3.24.0-20261005234241': - resolution: {integrity: sha512-HzksZWxemT2uynqHQOFIL2EjffZH+07L6jtRU4VygvIw/XDN7nOfwxbCakZxsCTeiYhazR4Ox21RamZwMYWzlQ==} + '@taskless/vale-linux-arm64@3.26.0-20261102164455': + resolution: {integrity: sha512-hHGaE5ohR3Cl3KggoJlVp+hsrRZQKiXZyY/s73QmHgV8qaIj2oZLYYjPbfJyo+YERHVemlZBAnzEesXe/NuuaQ==} cpu: [arm64] os: [linux] - '@taskless/vale-linux-x64@3.24.0-20261005234241': - resolution: {integrity: sha512-La3Jd52tGcrJz9l6hzuoo2YwLyA2lIgx/V8Dc2oI7Q/YB3oW1OvDmsh2DoDl4t/QxvOMerBItgtMX4l8GwaLHw==} + '@taskless/vale-linux-x64@3.26.0-20261102164455': + resolution: {integrity: sha512-ULZPna6eb3hX8RsHte3GphvR9aTH9zKvV+D86RQM2mOg/z73qpgfRBdi3LNfA2vbpOPCGtz/keEFEcmREGFciQ==} cpu: [x64] os: [linux] - '@taskless/vale-win32-arm64@3.24.0-20261005234241': - resolution: {integrity: sha512-YOsmyUUMa14wM/Qr6AJ8RMmukUeVWRLgt/BUJ/WYYBVS5ojLZ4wZGWVegPGSMCQaGuHaOUlkLYIYnRDeqOFnrQ==} + '@taskless/vale-win32-arm64@3.26.0-20261102164455': + resolution: {integrity: sha512-1p8pqygEkReBAzEwH8HorqZGnCqF9z34BXqiHta84ajrdo/skKog/9fm7MfDrriGaF8ObJiclUX6zCPOOhQp7A==} cpu: [arm64] os: [win32] - '@taskless/vale-win32-x64@3.24.0-20261005234241': - resolution: {integrity: sha512-jqL3ZnWmW+ZXhFqhcA9aPvq6YSKDTEx+Q5CTCi6y3so1i5xuKbJ3j07uDh/sM8PYre3RVGcAwxYIteOzI5i+aA==} + '@taskless/vale-win32-x64@3.26.0-20261102164455': + resolution: {integrity: sha512-S8ViSg3PZvJjrC0ClNuwmdYwmOnAxaOMxi6HlYSHgsnsXbf/0/ddf/oAGi/3FMXNh4PSUxyczZurGTcbu3DRqA==} cpu: [x64] os: [win32] @@ -3354,22 +3354,22 @@ snapshots: '@standard-schema/spec@1.1.0': {} - '@taskless/vale-darwin-arm64@3.24.0-20261005234241': + '@taskless/vale-darwin-arm64@3.26.0-20261102164455': optional: true - '@taskless/vale-darwin-x64@3.24.0-20261005234241': + '@taskless/vale-darwin-x64@3.26.0-20261102164455': optional: true - '@taskless/vale-linux-arm64@3.24.0-20261005234241': + '@taskless/vale-linux-arm64@3.26.0-20261102164455': optional: true - '@taskless/vale-linux-x64@3.24.0-20261005234241': + '@taskless/vale-linux-x64@3.26.0-20261102164455': optional: true - '@taskless/vale-win32-arm64@3.24.0-20261005234241': + '@taskless/vale-win32-arm64@3.26.0-20261102164455': optional: true - '@taskless/vale-win32-x64@3.24.0-20261005234241': + '@taskless/vale-win32-x64@3.26.0-20261102164455': optional: true '@tybys/wasm-util@0.10.4':