Skip to content

Commit 86e0831

Browse files
committed
feat(update): move a Vale rule's tests: cases into fixtures on 3.24.0
`validate` refuses a Vale rule's `tests:` key from 3.24.0 on, and 0.12.0 had accepted it. A rule migration keyed on Vale 3.24.0 now writes each case's input as a fixture under .tests/fail when it expects an alert and .tests/pass when it expects none, named for the case with its format as the extension, then deletes the block by its byte range. It never overwrites a fixture, reruns as a no-op, and leaves a rule whose cases read through a view or name another rule file untouched. No issued rule carries the key.
1 parent e0d3995 commit 86e0831

6 files changed

Lines changed: 559 additions & 15 deletions

File tree

‎.changeset/vale-3-24-0.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@ Vale 3.24.0 warns about any key it does not recognise inside a matcher, includin
88

99
Two findings change for existing rules: C# `// ReSharper disable` and `// NOSONAR` comments are no longer linted, and a word after a link and a non-breaking space is now matched. `update` (topic v18) walks both.
1010

11-
`validate` and `verify` now reject a Vale rule's `tests:` key. Vale 3.24.0 runs those cases with `vale test`, which Taskless never calls; a Vale rule is verified by its `.tests/pass` and `.tests/fail` documents.
11+
`validate` and `verify` now reject a Vale rule's `tests:` key. Vale 3.24.0 runs those cases with `vale test`, which Taskless never calls; a Vale rule is verified by its `.tests/pass` and `.tests/fail` documents. `update --rules` moves each case into one of those fixtures for you.

‎packages/cli/src/agent/update.md‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -498,8 +498,7 @@ loaded it without complaint.
498498
### Migrating to 0.13.3
499499

500500
The vendored ast-grep moves from 0.45.3 to 0.51.0, and Vale from 3.23.0
501-
to 3.24.0. Nothing here needs work by hand except a Vale rule carrying
502-
a `tests:` key, described at the end.
501+
to 3.24.0. Nothing here needs work by hand.
503502

504503
**A Haskell type synonym is `type_synonym` now, not `type_synomym`, and
505504
`update --rules` renames it for you.** ast-grep's Haskell grammar moved
@@ -553,17 +552,24 @@ Additions a rule config can use: a matcher may carry a condition,
553552
`[*.md if .Meta.draft]`, which applies it only to the matched files
554553
whose front matter satisfies it. `validate` accepts it.
555554

556-
**A rule's `tests:` key is refused.** Vale 3.24.0 ships `vale test`,
557-
which runs cases written in a rule's `tests:` list. Taskless never runs
558-
it: `verify` judges a Vale rule by the documents under `.tests/pass`
559-
and `.tests/fail`. So `validate` and `verify` now reject the key, which
555+
**A Vale rule's `tests:` cases move into its fixtures, and
556+
`update --rules` moves them for you.** Vale 3.24.0 ships `vale test`, which runs
557+
cases written in a rule's `tests:` list. Taskless never runs it:
558+
`verify` judges a Vale rule by the documents under `.tests/pass` and
559+
`.tests/fail`. So `validate` and `verify` now reject the key, which
560560
0.12.0 had started accepting, rather than let a rule look tested by
561-
cases nothing runs. `check` still runs such a rule. Search
562-
`.taskless/rules/vale/*/*.yml` for a top-level `tests:`; for each case,
563-
write its `input` as a document under `.tests/fail` if the case expects
564-
the rule to fire, or `.tests/pass` if it expects no alert, named with
565-
the case's `format` as its extension (`.md` when it has none). Then
566-
delete the key and run `verify` on the rule.
561+
cases nothing runs. `check` still runs such a rule.
562+
563+
Do not move them by hand. Step 5 writes each case's `input` as a
564+
document named for the case, under `.tests/fail` when the case expects
565+
an alert and `.tests/pass` when it expects none, with the case's
566+
`format` as its extension. It then deletes the `tests:` block, leaves
567+
everything else in the rule as it was, never overwrites a fixture, and
568+
names each file it wrote. No rule Taskless issued carries `tests:`, so
569+
it never touches a signed rule. A rule with a case that reads through a
570+
`view`, or names another rule file in `rule:`, is left as it is, and
571+
`validate` keeps naming it; move that rule's cases by hand the same
572+
way.
567573

568574
## Errors
569575

‎packages/cli/src/rules/rule-migrate.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { join } from "node:path";
22

33
import { AST_GREP_VERSION, VALE_VERSION } from "./capabilities";
44
import haskellTypeSynonym from "./rule-migrations/sg-0-50-0-haskell-type-synonym";
5+
import valeTestsToFixtures from "./rule-migrations/vale-3-24-0-tests-to-fixtures";
56
import { compareVersions } from "../util/version-compare";
67

78
/**
@@ -60,7 +61,10 @@ export interface AppliedRuleMigration {
6061
files: string[];
6162
}
6263

63-
const RULE_MIGRATIONS: readonly RuleMigration[] = [haskellTypeSynonym];
64+
const RULE_MIGRATIONS: readonly RuleMigration[] = [
65+
haskellTypeSynonym,
66+
valeTestsToFixtures,
67+
];
6468

6569
const BUNDLED: Record<RuleEngine, string> = {
6670
sg: AST_GREP_VERSION,
Lines changed: 253 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,253 @@
1+
import { mkdir, readdir, readFile, writeFile } from "node:fs/promises";
2+
import { join } from "node:path";
3+
4+
import { isMap, isScalar, isSeq, parseAllDocuments } from "yaml";
5+
6+
import { RULE_TESTS_DIRECTORY } from "../layout";
7+
import type { RuleMigration } from "../rule-migrate";
8+
9+
/**
10+
* Move a Vale rule's `tests:` cases into `.tests/pass` and `.tests/fail`.
11+
*
12+
* Vale 3.24.0 runs the cases a rule carries under `tests:` with `vale test`.
13+
* Taskless never runs it: `verify` judges a Vale rule by the documents under
14+
* `.tests/`, and `validate` refuses the key from 3.24.0 on, so a rule written
15+
* against 0.12.0, which accepted it, would otherwise stop validating.
16+
*
17+
* A case is one document and whether the rule fires on it, which is exactly
18+
* what a fixture is. An in-rule case loads only the rule it lives in, so any
19+
* alert it expects is that rule's:
20+
*
21+
* | the case | becomes |
22+
* | ------------------------------ | ---------------- |
23+
* | `want` with any text | `.tests/fail/` |
24+
* | `want` that is empty | `.tests/pass/` |
25+
* | no `want`, but `contains` | `.tests/fail/` |
26+
* | only `absent` | `.tests/pass/` |
27+
*
28+
* What `want` pins beyond that, the exact line, column and message, is not
29+
* something a rule's verification needs, and it is not carried.
30+
*
31+
* The fixture holds the case's `input` byte for byte, is named for the case,
32+
* and takes its extension from `format` (`md` when there is none, as Vale
33+
* reads it). A rule with a case this cannot place is left exactly as it is:
34+
* one that reads its input through a `view`, which this layout never defines,
35+
* or that names another rule file in `rule:`. `validate` keeps refusing that
36+
* rule, and its message says what to do.
37+
*
38+
* The `tests:` block is removed by the parser's byte range, not by
39+
* re-serializing the file, so everything else in the rule stays as written.
40+
* An existing fixture with the same bytes counts as written, which is what
41+
* makes a rerun after a partial failure a no-op; one with other bytes is
42+
* never overwritten, and the case takes the next free name.
43+
*
44+
* The Taskless service has issued no Vale rule carrying `tests:` (confirmed
45+
* against its corpus, 2026-10-20), so no issued rule's signature is affected.
46+
*/
47+
48+
/** One case, reduced to the two facts a fixture records. */
49+
interface PlacedCase {
50+
bucket: "pass" | "fail";
51+
name: string;
52+
extension: string;
53+
input: string;
54+
}
55+
56+
/** A rule this migration rewrites: its cases, and the block to remove. */
57+
interface Plan {
58+
cases: PlacedCase[];
59+
blockStart: number;
60+
blockEnd: number;
61+
}
62+
63+
function text(value: unknown): string | undefined {
64+
return typeof value === "string" ? value : undefined;
65+
}
66+
67+
/** Whether `contains` or `absent` says anything: one excerpt, or a list. */
68+
function listed(value: unknown): boolean {
69+
return (
70+
typeof value === "string" || (Array.isArray(value) && value.length > 0)
71+
);
72+
}
73+
74+
/** A case's bucket, or `undefined` when it asserts nothing Vale would run. */
75+
function bucketOf(
76+
item: Record<string, unknown>
77+
): PlacedCase["bucket"] | undefined {
78+
const want = text(item.want);
79+
if (want !== undefined) return want.trim() === "" ? "pass" : "fail";
80+
if (listed(item.contains)) return "fail";
81+
if (listed(item.absent)) return "pass";
82+
return undefined;
83+
}
84+
85+
/** `flags a hedge` as `flags-a-hedge`; `case-<n>` when nothing survives. */
86+
function slug(name: string | undefined, index: number): string {
87+
const cleaned = (name ?? "")
88+
.toLowerCase()
89+
.replaceAll(/[^a-z0-9]+/g, "-")
90+
.replaceAll(/^-+|-+$/g, "");
91+
return cleaned === "" ? `case-${String(index + 1)}` : cleaned;
92+
}
93+
94+
/**
95+
* What to do with one rule file, or `undefined` to leave it alone.
96+
*
97+
* Alone means: not a single-document rule with a top-level `tests` list, or
98+
* one whose cases cannot all be placed. Partly migrating a rule would leave
99+
* it refused by `validate` with half its cases moved, which is harder to
100+
* finish by hand than none.
101+
*/
102+
export function planTestsMigration(
103+
source: string,
104+
ruleFile: string
105+
): Plan | undefined {
106+
let documents;
107+
try {
108+
documents = parseAllDocuments(source);
109+
} catch {
110+
return undefined;
111+
}
112+
if (!Array.isArray(documents) || documents.length !== 1) return undefined;
113+
const [document] = documents;
114+
if (document === undefined || document.errors.length > 0) return undefined;
115+
const root = document.contents;
116+
if (!isMap(root) || !root.has("extends")) return undefined;
117+
const pair = root.items.find(
118+
(item) => isScalar(item.key) && item.key.value === "tests"
119+
);
120+
if (
121+
pair === undefined ||
122+
!isScalar(pair.key) ||
123+
!isSeq(pair.value) ||
124+
pair.key.range === undefined ||
125+
pair.key.range === null ||
126+
pair.value.range === undefined ||
127+
pair.value.range === null
128+
) {
129+
return undefined;
130+
}
131+
132+
const items: unknown = pair.value.toJSON();
133+
if (!Array.isArray(items)) return undefined;
134+
const cases: PlacedCase[] = [];
135+
for (const [index, raw] of items.entries()) {
136+
if (typeof raw !== "object" || raw === null || Array.isArray(raw)) {
137+
return undefined;
138+
}
139+
const item = raw as Record<string, unknown>;
140+
if (item.view !== undefined && item.view !== null) return undefined;
141+
const rule = text(item.rule);
142+
if (rule !== undefined && rule !== "" && rule !== ruleFile) {
143+
return undefined;
144+
}
145+
const bucket = bucketOf(item);
146+
if (bucket === undefined) return undefined;
147+
const format = text(item.format) ?? "";
148+
cases.push({
149+
bucket,
150+
name: slug(text(item.name), index),
151+
extension: format.replace(/^\./, "") || "md",
152+
input: text(item.input) ?? "",
153+
});
154+
}
155+
156+
// From the start of the key's line through the end of the value's line.
157+
const keyStart = pair.key.range[0];
158+
const blockStart = source.lastIndexOf("\n", keyStart - 1) + 1;
159+
let blockEnd = pair.value.range[1];
160+
if (blockEnd > 0 && source[blockEnd - 1] !== "\n") {
161+
const newline = source.indexOf("\n", blockEnd);
162+
blockEnd = newline === -1 ? source.length : newline + 1;
163+
}
164+
return { cases, blockStart, blockEnd };
165+
}
166+
167+
async function readExisting(path: string): Promise<string | undefined> {
168+
try {
169+
return await readFile(path, "utf8");
170+
} catch (error) {
171+
if ((error as NodeJS.ErrnoException).code === "ENOENT") return undefined;
172+
throw error;
173+
}
174+
}
175+
176+
/**
177+
* Write one case's fixture, returning its path relative to the rule
178+
* directory, or `undefined` when identical bytes were already there.
179+
*/
180+
async function writeFixture(
181+
ruleDirectory: string,
182+
placed: PlacedCase,
183+
taken: Set<string>
184+
): Promise<string | undefined> {
185+
const bucket = join(RULE_TESTS_DIRECTORY, placed.bucket);
186+
await mkdir(join(ruleDirectory, bucket), { recursive: true });
187+
for (let attempt = 1; ; attempt += 1) {
188+
const stem =
189+
attempt === 1 ? placed.name : `${placed.name}-${String(attempt)}`;
190+
const relative = join(bucket, `${stem}.${placed.extension}`);
191+
if (taken.has(relative)) continue;
192+
const existing = await readExisting(join(ruleDirectory, relative));
193+
if (existing === placed.input) {
194+
taken.add(relative);
195+
return undefined;
196+
}
197+
if (existing !== undefined) continue;
198+
taken.add(relative);
199+
await writeFile(join(ruleDirectory, relative), placed.input, "utf8");
200+
return relative;
201+
}
202+
}
203+
204+
const migration: RuleMigration = {
205+
engine: "vale",
206+
engineVersion: "3.24.0",
207+
summary:
208+
"Vale rule `tests:` cases moved into `.tests/pass` and `.tests/fail`",
209+
async apply(engineDirectory) {
210+
let ruleDirectories;
211+
try {
212+
ruleDirectories = await readdir(engineDirectory, { withFileTypes: true });
213+
} catch (error) {
214+
// No Vale rules tree: nothing to move. Anything else has to fail, not
215+
// read as "nothing to do": the stamp that follows would record this
216+
// migration as applied, and no later run would retry it.
217+
if ((error as NodeJS.ErrnoException).code === "ENOENT") return [];
218+
throw error;
219+
}
220+
const written: string[] = [];
221+
for (const ruleDirectory of ruleDirectories) {
222+
if (!ruleDirectory.isDirectory()) continue;
223+
const directory = join(engineDirectory, ruleDirectory.name);
224+
const files = await readdir(directory, { withFileTypes: true });
225+
for (const file of files) {
226+
if (!file.isFile() || !/\.ya?ml$/.test(file.name)) continue;
227+
const path = join(directory, file.name);
228+
const source = await readFile(path, "utf8");
229+
const plan = planTestsMigration(source, file.name);
230+
if (plan === undefined) continue;
231+
// Fixtures first, rule last: a failure in between leaves the cases
232+
// in the rule, so the rerun finds them, sees its own fixtures, and
233+
// only finishes the removal.
234+
const taken = new Set<string>();
235+
for (const placed of plan.cases) {
236+
const fixture = await writeFixture(directory, placed, taken);
237+
if (fixture !== undefined) {
238+
written.push(join(ruleDirectory.name, fixture));
239+
}
240+
}
241+
await writeFile(
242+
path,
243+
source.slice(0, plan.blockStart) + source.slice(plan.blockEnd),
244+
"utf8"
245+
);
246+
written.push(join(ruleDirectory.name, file.name));
247+
}
248+
}
249+
return written;
250+
},
251+
};
252+
253+
export default migration;

‎packages/cli/test/reconcile-marker.test.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,56 @@ describe("recording a rules reconciliation", () => {
201201
vale: VALE_VERSION,
202202
});
203203
});
204+
205+
it("moves a Vale rule's tests: cases into fixtures when it crosses 3.24.0", async () => {
206+
const ruleDirectory = join(cwd, ".taskless", "rules", "vale", "hedge");
207+
await mkdir(ruleDirectory, { recursive: true });
208+
await writeFile(
209+
join(ruleDirectory, "hedge.yml"),
210+
'extends: existence\nmessage: "x"\nlevel: warning\ntokens:\n - perhaps\n' +
211+
"tests:\n - name: flags it\n input: Perhaps.\n contains: x\n"
212+
);
213+
const manifestPath = join(cwd, ".taskless", "taskless.json");
214+
const manifest = JSON.parse(
215+
await readFile(manifestPath, "utf8")
216+
) as Record<string, unknown>;
217+
manifest["rules"] = {
218+
reconciledTo: "0.11.0",
219+
engines: { sg: AST_GREP_VERSION, vale: "3.23.0" },
220+
};
221+
await writeFile(manifestPath, JSON.stringify(manifest, undefined, 2));
222+
223+
const result = await runCli(["update", "--rules", "--json", "-d", cwd]);
224+
expect(result.exitCode).toBe(0);
225+
const payload = JSON.parse(result.stdout) as {
226+
migrated: { engineVersion: string; files: string[] }[];
227+
};
228+
expect(payload.migrated).toContainEqual(
229+
expect.objectContaining({
230+
engineVersion: "3.24.0",
231+
files: [
232+
join(
233+
".taskless",
234+
"rules",
235+
"vale",
236+
"hedge",
237+
".tests",
238+
"fail",
239+
"flags-it.md"
240+
),
241+
join(".taskless", "rules", "vale", "hedge", "hedge.yml"),
242+
],
243+
})
244+
);
245+
expect(
246+
await readFile(join(ruleDirectory, "hedge.yml"), "utf8")
247+
).not.toContain("tests:");
248+
const rules = await readRules();
249+
expect(rules?.engines).toEqual({
250+
sg: AST_GREP_VERSION,
251+
vale: VALE_VERSION,
252+
});
253+
});
204254
});
205255

206256
it("refuses when there is no .taskless/ to reconcile", async () => {

0 commit comments

Comments
 (0)