Skip to content

Commit a40d7da

Browse files
rvbenrvben
committed
fix(code-block-tools): stop a linter in a format slot from overwriting the block
The format path takes a tool's stdout as the block's replacement, so a linter configured there wrote its report into the document: `format = ["ruff:check"]` rewrote a clean Python block to the literal text `All checks passed!`. Neither existing guard caught it, because a linter with nothing to complain about exits 0 and can still print a summary line. A tool that cannot format is now skipped, using the same predicate that already reports the mistake as a config warning. User-defined tools are unaffected: the user wrote the command, so rumdl has no opinion about what it does. Documents the slot rules alongside the built-in tools table: what a bare tool id resolves to in each slot, that `terraform-fmt` is an alias of `terraform:format`, that a formatter in a lint slot is a formatting check, and how a finding without a machine-readable position is placed.
1 parent 31cc6cb commit a40d7da

4 files changed

Lines changed: 183 additions & 36 deletions

File tree

‎docs/code-block-tools.md‎

Lines changed: 83 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -107,42 +107,42 @@ rumdl includes definitions for common tools:
107107

108108
<!-- BEGIN builtin-tools (generated) -->
109109

110-
| Tool ID | Language | Type | Command |
111-
| ------------------ | ---------- | ------ | --------------------------------------- |
112-
| `ruff:check` | Python | Lint | `ruff check --output-format=concise -` |
113-
| `ruff:format` | Python | Format | `ruff format -` |
114-
| `black` | Python | Format | `black --quiet -` |
115-
| `prettier` | Multi | Format | `prettier --stdin-filepath=_.EXT` |
116-
| `shellcheck` | Shell | Lint | `shellcheck --shell=bash -` |
117-
| `shfmt` | Shell | Format | `shfmt` |
118-
| `shuck` | Shell | Lint | `shuck check --output-format concise -` |
119-
| `shuck:format` | Shell | Format | `shuck format -` |
120-
| `rustfmt` | Rust | Format | `rustfmt` |
121-
| `gofmt` | Go | Format | `gofmt` |
122-
| `goimports` | Go | Format | `goimports` |
123-
| `clang-format` | C/C++ | Format | `clang-format` |
124-
| `sqlfluff:lint` | SQL | Lint | `sqlfluff lint --dialect ansi -` |
125-
| `sqlfluff:fix` | SQL | Format | `sqlfluff fix --dialect ansi -` |
126-
| `jq` | JSON | Both | `jq .` |
127-
| `yamlfmt` | YAML | Format | `yamlfmt -` |
128-
| `taplo` | TOML | Format | `taplo fmt -` |
129-
| `terraform:format` | Terraform | Format | `terraform fmt -` |
130-
| `nixfmt` | Nix | Format | `nixfmt -` |
131-
| `stylua` | Lua | Format | `stylua -` |
132-
| `ormolu` | Haskell | Format | `ormolu --stdin-input-file=_.hs` |
133-
| `elm-format` | Elm | Format | `elm-format --stdin` |
134-
| `swift-format` | Swift | Format | `swift-format format -` |
135-
| `ktfmt` | Kotlin | Format | `ktfmt -` |
136-
| `djlint` | Jinja/HTML | Both | `djlint - / djlint - --reformat` |
137-
| `djlint:lint` | Jinja/HTML | Lint | `djlint -` |
138-
| `djlint:reformat` | Jinja/HTML | Format | `djlint - --reformat` |
139-
| `beautysh` | Shell | Format | `beautysh -` |
140-
| `tombi` | TOML | Lint | `tombi lint -` |
141-
| `tombi:format` | TOML | Format | `tombi format -` |
142-
| `tombi:lint` | TOML | Lint | `tombi lint -` |
143-
| `oxfmt` | Multi | Format | `oxfmt --stdin-filepath=_.EXT` |
144-
| `deno-fmt` | Multi | Format | `deno fmt --ext=EXT -` |
145-
| `rumdl` | Markdown | Lint | `built-in markdown linting` |
110+
| Tool ID | Language | Type | Command |
111+
| ------------------ | ---------- | ------ | ------------------------------------------------------------------ |
112+
| `ruff:check` | Python | Lint | `ruff check --output-format=concise -` |
113+
| `ruff:format` | Python | Format | `ruff format -` |
114+
| `black` | Python | Format | `black --quiet -` |
115+
| `prettier` | Multi | Format | `prettier --stdin-filepath=_.EXT` |
116+
| `shellcheck` | Shell | Lint | `shellcheck --shell=bash -` |
117+
| `shfmt` | Shell | Format | `shfmt` |
118+
| `shuck` | Shell | Lint | `shuck check --output-format concise -` |
119+
| `shuck:format` | Shell | Format | `shuck format -` |
120+
| `rustfmt` | Rust | Format | `rustfmt` |
121+
| `gofmt` | Go | Format | `gofmt` |
122+
| `goimports` | Go | Format | `goimports` |
123+
| `clang-format` | C/C++ | Format | `clang-format` |
124+
| `sqlfluff:lint` | SQL | Lint | `sqlfluff lint --dialect ansi --format github-annotation-native -` |
125+
| `sqlfluff:fix` | SQL | Format | `sqlfluff fix --dialect ansi -` |
126+
| `jq` | JSON | Both | `jq .` |
127+
| `yamlfmt` | YAML | Format | `yamlfmt -` |
128+
| `taplo` | TOML | Format | `taplo fmt -` |
129+
| `terraform:format` | Terraform | Format | `terraform fmt -` |
130+
| `nixfmt` | Nix | Format | `nixfmt -` |
131+
| `stylua` | Lua | Format | `stylua -` |
132+
| `ormolu` | Haskell | Format | `ormolu --stdin-input-file=_.hs` |
133+
| `elm-format` | Elm | Format | `elm-format --stdin` |
134+
| `swift-format` | Swift | Format | `swift-format format -` |
135+
| `ktfmt` | Kotlin | Format | `ktfmt -` |
136+
| `djlint` | Jinja/HTML | Both | `djlint - / djlint - --reformat` |
137+
| `djlint:lint` | Jinja/HTML | Lint | `djlint -` |
138+
| `djlint:reformat` | Jinja/HTML | Format | `djlint - --reformat` |
139+
| `beautysh` | Shell | Format | `beautysh -` |
140+
| `tombi` | TOML | Lint | `tombi lint -` |
141+
| `tombi:format` | TOML | Format | `tombi format -` |
142+
| `tombi:lint` | TOML | Lint | `tombi lint -` |
143+
| `oxfmt` | Multi | Format | `oxfmt --stdin-filepath=_.EXT` |
144+
| `deno-fmt` | Multi | Format | `deno fmt --ext=EXT -` |
145+
| `rumdl` | Markdown | Lint | `built-in markdown linting` |
146146

147147
<!-- END builtin-tools (generated) -->
148148

@@ -153,6 +153,46 @@ built-in YAML linter. To lint YAML blocks, wire in a custom tool such as
153153
[ryl](https://github.com/owenlamont/ryl) (see
154154
[Linting YAML blocks with ryl](#linting-yaml-blocks-with-ryl)).
155155

156+
### Tool IDs and Slots
157+
158+
A tool with more than one mode is registered as `tool:mode` (`ruff:check`,
159+
`ruff:format`, `sqlfluff:lint`, `tombi:format`). A bare name resolves to the
160+
variant that fits the slot it is written in, so `lint = ["sqlfluff"]` runs
161+
`sqlfluff:lint` and `format = ["tombi"]` runs `tombi:format`. `terraform-fmt` is
162+
kept as an alias of `terraform:format`, so a config written either way works.
163+
164+
**A formatter in a `lint` slot is a formatting check.** rumdl runs the formatter,
165+
compares its output with the block, and reports `Code block is not formatted` when
166+
they differ:
167+
168+
```toml
169+
[code-block-tools.languages]
170+
python = { lint = ["black"], format = ["black"] }
171+
```
172+
173+
The comparison is exactly what `rumdl fmt` would rewrite, so `check` and `fmt`
174+
cannot disagree. rumdl does not pass a tool's own `--check` or `--diff` flag:
175+
those disagree across tools on exit code, on what they print, and on whether the
176+
flag is even accepted next to the stdin argument the tool requires.
177+
178+
**A linter in a `format` slot is declined.** A linter writes its report to stdout,
179+
which is where the formatted code would come from, so running one would replace
180+
the block with its own output. rumdl skips such a tool and reports the
181+
configuration instead:
182+
183+
```text
184+
Tool in code-block-tools.languages.python.format cannot format: ruff:check is a linter (move it to lint)
185+
```
186+
187+
An id that names no tool at all is reported the same way, with a suggestion:
188+
189+
```text
190+
Unknown tool in code-block-tools.languages.python.format: blackk (did you mean: black?)
191+
```
192+
193+
Both warnings are emitted whether or not `enabled` is set, so a typo surfaces
194+
before the feature is switched on.
195+
156196
### Embedded Markdown Linting
157197

158198
The special `rumdl` tool enables linting of markdown content inside fenced code blocks:
@@ -266,6 +306,13 @@ With this configuration:
266306

267307
Tool output references lines within the code block. rumdl maps these to the actual markdown file line numbers so diagnostics point to the correct location.
268308

309+
A tool that reports a position only in prose (`jq`'s "at line 1, column 9") is
310+
mapped from that prose. A tool that reports no position at all is anchored on the
311+
opening fence, which is the most precise place rumdl can honestly point to. The
312+
built-in definitions ask for a machine-readable format where the tool has one, so
313+
findings land on their own line rather than on the fence: `sqlfluff:lint` uses
314+
GitHub annotations and `djlint` uses an explicit `--linter-output-format`.
315+
269316
### Indented Code Blocks
270317

271318
For code blocks inside lists or blockquotes, rumdl:

‎src/code_block_tools/processor.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -917,6 +917,20 @@ impl<'a> CodeBlockToolProcessor<'a> {
917917
continue;
918918
};
919919

920+
// A linter writes its report to stdout, so running one here replaces the
921+
// block with that report: `ruff:check` in a format slot turns a clean
922+
// Python block into the literal text `All checks passed!`. The
923+
// empty-output guard below does not catch it, because a linter with
924+
// nothing to say still exits 0 and can still print a summary line.
925+
// Config validation reports this too, but a warning cannot undo an
926+
// overwritten block, so the run declines the tool as well.
927+
if self.registry.fills_format_slot(tool_id) == Some(false) {
928+
log::warn!(
929+
"Tool '{tool_id}' is a linter and cannot format '{canonical_lang}' code blocks; move it to the lint slot"
930+
);
931+
continue;
932+
}
933+
920934
// Check if tool binary exists before running
921935
let tool_name = tool_def.command.first().map_or("", String::as_str);
922936
if !tool_name.is_empty() && !self.executor.is_tool_available(tool_name) {

‎src/code_block_tools/registry.rs‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1712,6 +1712,60 @@ mod tests {
17121712
}
17131713
}
17141714

1715+
/// A `format` slot either runs something that formats, or answers `false` so the run
1716+
/// declines it.
1717+
///
1718+
/// Both the config check and the format run decline a tool that answers `false`, because
1719+
/// a linter run in a format slot replaces the block with its own report. The interesting
1720+
/// case is a bare id whose linter and formatter are separate entries (`tombi`, `shuck`):
1721+
/// the slot must pick the sibling that formats rather than answering for the linter the
1722+
/// bare id names in a lint slot. Stated over the whole table, so a new entry is covered
1723+
/// the moment it is added.
1724+
#[test]
1725+
fn format_slot_never_resolves_to_a_linter() {
1726+
let registry = ToolRegistry::default();
1727+
let documented_kind = |id: &str| {
1728+
BUILTIN_TOOLS_DOCS
1729+
.iter()
1730+
.find(|m| m.id == id && m.runtime)
1731+
.map(|m| m.kind)
1732+
};
1733+
1734+
for meta in BUILTIN_TOOLS_DOCS.iter().filter(|m| m.runtime) {
1735+
let fills = registry.fills_format_slot(meta.id);
1736+
let resolved = registry.resolve_id(meta.id, ToolSlot::Format);
1737+
1738+
match fills {
1739+
Some(true) => {
1740+
let resolved = resolved.expect("a tool that fills the slot resolves in it");
1741+
assert!(
1742+
!matches!(documented_kind(&resolved), Some(ToolKind::Lint)),
1743+
"{} fills a format slot by running {resolved}, which only lints",
1744+
meta.id
1745+
);
1746+
}
1747+
Some(false) => assert_eq!(
1748+
documented_kind(meta.id),
1749+
Some(ToolKind::Lint),
1750+
"{} declines the format slot, so it must be documented as a linter",
1751+
meta.id
1752+
),
1753+
None => panic!("{} is in the registry, so it resolves somewhere", meta.id),
1754+
}
1755+
}
1756+
1757+
// The tools this is really about: each is documented as a linter, and each answers
1758+
// for the format slot the way its own entries allow.
1759+
assert_eq!(registry.fills_format_slot("ruff:check"), Some(false));
1760+
assert_eq!(registry.fills_format_slot("shellcheck"), Some(false));
1761+
assert_eq!(registry.fills_format_slot("sqlfluff:lint"), Some(false));
1762+
assert_eq!(registry.fills_format_slot("tombi"), Some(true));
1763+
assert_eq!(
1764+
registry.resolve_id("tombi", ToolSlot::Format).as_deref(),
1765+
Some("tombi:format")
1766+
);
1767+
}
1768+
17151769
/// A user-defined tool is run exactly as written, even when it shadows a built-in id.
17161770
#[test]
17171771
fn user_tool_is_never_lint_checked_by_formatting() {

‎tests/integration/code_block_tools_execution_test.rs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,38 @@ lint_by_format_test!(
714714
"a=1"
715715
);
716716

717+
// ---- linters in a `format` slot -------------------------------------------
718+
719+
/// A built-in linter in a `format` slot must leave the block exactly as it was.
720+
///
721+
/// A linter writes its report to stdout, which is where the format path reads the block's
722+
/// replacement from. Before rumdl declined these, `format = ["ruff:check"]` rewrote a clean
723+
/// Python block to the literal text `All checks passed!` - stdout was a summary line, not
724+
/// code, and the empty-output guard never saw it because the linter had nothing to complain
725+
/// about and exited 0. Config validation warns about the same mistake, but a warning cannot
726+
/// undo an overwritten block.
727+
///
728+
/// The positive control is the same tool and sample in the slot it belongs in: `ruff:format`
729+
/// must rewrite `x=1`, so a ruff that stopped working fails this test instead of passing it.
730+
#[test]
731+
fn builtin_linter_in_format_slot_leaves_the_block_alone() {
732+
require_tool!("ruff");
733+
734+
let declined = format("python", "ruff:check", "python", "x=1");
735+
assert_eq!(
736+
fenced_block(&declined),
737+
"x=1",
738+
"a linter in a format slot must not touch the block:\n{declined}"
739+
);
740+
741+
let formatted = format("python", "ruff:format", "python", "x=1");
742+
assert_eq!(
743+
fenced_block(&formatted),
744+
"x = 1",
745+
"ruff did not format the sample, so the assertion above proves nothing:\n{formatted}"
746+
);
747+
}
748+
717749
// ---- coverage gate --------------------------------------------------------
718750

719751
/// Built-in tool ids with a dedicated `lint`-slot execution test above.

0 commit comments

Comments
 (0)