Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/cli/src/filesystem/migrations/0001-init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ Rules are partitioned by the engine that runs them. Each engine directory holds
that tool's own native config, its \`rules/\`, and its \`rule-tests/\`:

- \`sg/\` - ast-grep: \`sgconfig.yml\`, generated rules (managed by Taskless), and their pass/fail test cases
- \`vale/\` - Vale prose rules: \`.vale.ini\`. Scaffolded and inert; nothing runs it yet
- \`vale/\` - Vale prose rules: \`.vale.ini\`, \`rules/\`, and their pass/fail fixtures. Run by \`check\` alongside ast-grep
- \`runtime/\` - Rules that execute a \`check.ts\`, each in its own \`rules/<name>/\` directory
`;

Expand Down
20 changes: 16 additions & 4 deletions packages/cli/src/filesystem/migrations/0004-vale-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,19 @@ import { CLIError } from "../../util/cli-error";
const SG_CONFIG_CONTENT = `ruleDirs:\n - rules\ntestConfigs:\n - testDir: rule-tests\n`;

/**
* Minimal, inert `.vale.ini`. Nothing executes Vale yet; this exists so the
* engine directory has its native config in the canonical place from day one.
* The scaffolded `.vale.ini`.
*
* `StylesPath` is the engine directory, NOT `rules/`. Vale treats StylesPath as
* a directory *of styles*, so a rule at `vale/rules/no-simply.yml` is the
* `no-simply` rule of the `rules` style, and its check is `rules.no-simply` —
* which is the name `stripRulesPrefix` in `vale/map.ts` exists to undo, and the
* shape `verify.ts` generates. Pointing StylesPath at `rules/` instead makes
* that same file a style directory with no rules in it: every check resolves to
* nothing, Vale reports `{}`, and a prose check passes clean with every rule
* silently disabled. Measured against the real binary, which is the only way
* this is visible — the layout is identical either way.
*/
const VALE_CONFIG_CONTENT = `StylesPath = rules\nMinAlertLevel = suggestion\n\n[*]\n`;
const VALE_CONFIG_CONTENT = `StylesPath = .\nMinAlertLevel = suggestion\n\n[*]\n`;
Comment thread
thecodedrift marked this conversation as resolved.

/** Directories that must exist after the migration, tracked when empty. */
const SCAFFOLD_DIRECTORIES = [
Expand Down Expand Up @@ -247,7 +256,10 @@ async function ensureTrackedDirectory(path: string): Promise<void> {
*
* `rules/`, `rule-tests/`, and `sgconfig.yml` move under `sg/`; the runtime
* tier moves to `runtime/rules/` and `runtime/rule-tests/`; `vale/` is
* scaffolded with its native config but stays inert (no engine reads it yet).
* scaffolded with its native config, which `check` reads and runs alongside
* ast-grep. The scaffold enables no rules, so it is quiet until a user adds
* one — quiet, not inert, and the difference is why `VALE_CONFIG_CONTENT`
* above has to be a config Vale can actually resolve rules under.
*
* The move edits no file contents. `sgconfig.yml`'s `ruleDirs: [rules]` is
* relative to the config file, so it stays valid after the move with no path
Expand Down
16 changes: 14 additions & 2 deletions packages/cli/src/rules/dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,20 @@ export interface EngineOutcome {

export interface DispatchOptions {
cwd: string;
/** Target paths, already filtered to those that exist. */
/**
* Target paths, already filtered to those that exist.
*
* **Empty means "the whole project", and each engine is responsible for
* expressing that in its own terms.** The engines do not agree on what an
* empty target list means natively: ast-grep takes its targets from the
* config and is content with none, while Vale given no input prints its usage
* text and exits 0. Passing the empty list straight through therefore ran
* ast-grep correctly and reduced Vale to a parse failure on every whole-
* project check — findings silently absent, engine reported as broken.
*
* A new engine must decide what empty means for its own executor rather than
* assuming the caller narrowed it.
*/
paths: string[];
/**
* One `--config` path per ast-grep rule source, already resolved. The source
Expand Down Expand Up @@ -221,4 +234,3 @@ export async function runEngines(
: 0,
};
}

32 changes: 30 additions & 2 deletions packages/cli/src/rules/vale/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ import { buildPath } from "../scan";
import { findValeBinary, valeUnavailableMessage } from "./binary";
import { asValeConfigError, toValeCheckResults, type ValeOutput } from "./map";

/** Taskless's own directory, as a project-relative path. */
const TASKLESS_DIRECTORY = ".taskless";

/** The committed Vale config, relative to the project root. */
export const COMMITTED_VALE_CONFIG = `.taskless/${ENGINE_LAYOUTS.vale.configFile}`;
export const COMMITTED_VALE_CONFIG = `${TASKLESS_DIRECTORY}/${ENGINE_LAYOUTS.vale.configFile}`;

/**
* How long a single Vale invocation may run before it is killed.
Expand Down Expand Up @@ -96,14 +99,39 @@ export async function runVale(
const paths = options.paths ?? [];
const timeoutMs = options.timeoutMs ?? VALE_TIMEOUT_MS;

// Vale needs somewhere to look. Given no input it prints its usage text and
// exits 0, which reaches the mapper as "not JSON" and reports the engine as
// failed on every run — so a whole-project `check`, which passes no paths at
// all, produced zero Vale findings and one spurious failure. ast-grep is the
// reason this is easy to miss: it takes its targets from the config and is
// content with none, so the two engines disagree about what "no paths" means.
// `cwd` is the project root, so `.` is the whole project.
const wholeProject = paths.length === 0;
const targets = wholeProject ? ["."] : paths;

// Walking the whole project reaches `.taskless/` too, and Vale has no reason
// to know that directory is ours: with a rule enabled it reports findings in
// the committed `.vale.ini` and in the user's own rule definitions — prose
// complaints about the machinery, pointing at files nobody wrote as prose.
// Section globs do not help, since `.taskless/README.md` matches `[*.md]` as
// readily as any document. `--glob` filters which files are walked without
// touching which rules apply to them, so a user's scoping still decides that.
//
// Applied ONLY when we chose `.` ourselves. An explicit path is a request,
// and silently declining to check a file someone named would be worse than
// checking one they did not.
const exclude = wholeProject ? [`--glob=!${TASKLESS_DIRECTORY}/**`] : [];

// `--` separates flags from positional paths, so a path beginning with `-`
// is not read as a flag.
const argv = [
"--config",
configPath,
"--output=JSON",
"--no-exit",
...(paths.length > 0 ? ["--", ...paths] : []),
...exclude,
"--",
...targets,
];

return new Promise<ValeRunOutcome>((resolve) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
id: no-console-warn
language: javascript
severity: warning
rule:
pattern: console.warn($$$)
message: Prefer a structured logger over console.warn()
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
id: no-eval
language: javascript
severity: error
rule:
pattern: eval($$$)
message: Avoid using eval()
note: eval() is unsafe. Use alternatives like Function() or JSON.parse().
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
ruleDirs:
- rules
testConfigs:
- testDir: rule-tests
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"version": 4
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
StylesPath = .
MinAlertLevel = suggestion

[*.md]
BasedOnStyles =
rules.no-simply = YES
rules.no-obviously = YES
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
extends: existence
message: "Avoid 'obviously' — it tells the reader they should already know"
level: error
tokens:
- obviously
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
extends: existence
message: "Avoid 'simply' — it hides the work from the reader"
level: warning
tokens:
- simply
5 changes: 5 additions & 0 deletions packages/cli/test/fixtures/mixed-engines-project/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Sample

This document obviously trips one rule, and simply trips another.

Nothing else here is objectionable.
4 changes: 4 additions & 0 deletions packages/cli/test/fixtures/mixed-engines-project/sample.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Trips the sg engine: one error-severity rule and one warning-severity rule.
const result = eval("2 + 2");
console.warn("this is a warning");
console.log("this is fine");
Loading
Loading