| 1 | //! Post-edit syntax gate for the file-mutating tools (#6204, #6206). |
| 2 | //! |
| 3 | //! Every tool that rewrites a file — `File write`/`edit`/`patch` in |
| 4 | //! [`super::file`] and [`super::apply_patch`], plus [`super::fim`] — routes its |
| 5 | //! post-edit content through [`guard_edit`] *before* the bytes reach the disk. |
| 6 | //! When the edit would leave a file unparseable, the write never happens and |
| 7 | //! the model gets a `line:column` error it can act on this turn instead of a |
| 8 | //! compiler failure one or more turns later. |
| 9 | //! |
| 10 | //! Rust is parsed by `syn`, which implements the real language grammar and |
| 11 | //! reports grammar-exact errors (as opposed to an error-tolerant CST that |
| 12 | //! builds a tree *around* malformed input). TOML and JSON go through |
| 13 | //! `toml_edit` and `serde_json` — the parsers this crate already loads its own |
| 14 | //! config with. A malformed `Cargo.toml` fails the *entire* workspace build |
| 15 | //! rather than one file, so catching it at edit time is worth more there than |
| 16 | //! anywhere else. |
| 17 | //! |
| 18 | //! # The gate only rejects *newly* introduced breakage |
| 19 | //! |
| 20 | //! [`guard_edit`] fails open unless the file parsed **before** the edit and |
| 21 | //! fails to parse **after** it. That asymmetry is the whole safety argument: |
| 22 | //! repairing a file that is already broken — the single most common reason an |
| 23 | //! agent edits a source file at all — must never be blocked by a gate whose |
| 24 | //! job is to catch the edit that broke it. Creating a new file is likewise |
| 25 | //! ungated, since there is no "before" to have regressed. |
| 26 | //! |
| 27 | //! # Known limitations |
| 28 | //! |
| 29 | //! - **Grammar, not semantics.** A file that parses can still fail to compile; |
| 30 | //! type errors stay the compiler's and the LSP hook's job |
| 31 | //! (`core::engine::lsp_hooks`). |
| 32 | //! - **`syn` tracks the editions it knows.** Source using syntax newer than the |
| 33 | //! pinned `syn` would be reported as a parse error. Because the gate requires |
| 34 | //! the pre-edit file to have parsed, such a file is skipped entirely rather |
| 35 | //! than becoming uneditable. |
| 36 | //! - **Extension-driven.** A Rust file that is not named `*.rs` is not checked; |
| 37 | //! language detection by content is deliberately not attempted. `.jsonc`, |
| 38 | //! `.json5`, and `.jsonl` are *not* treated as JSON: they are different |
| 39 | //! grammars, and a strict parser would reject valid files. |
| 40 | //! - **A `.json` file that is really JSONC** — `tsconfig.json` with comments is |
| 41 | //! the usual one — does not parse before the edit either, so the gate skips |
| 42 | //! it rather than making it uneditable. |
| 43 | //! - **Bounded by [`MAX_CHECKED_BYTES`].** Larger files skip the check rather |
| 44 | //! than spend edit-path latency on a multi-megabyte parse. |
| 45 | |
| 46 | use std::fmt; |
| 47 | use std::path::Path; |
| 48 | |
| 49 | use super::spec::ToolError; |
| 50 | |
| 51 | /// Files larger than this skip the syntax gate. |
| 52 | /// |
| 53 | /// Parsing is linear and fast, but the check sits on the interactive edit path |
| 54 | /// and runs twice on a rejection. 2 MiB covers every hand-written source file |
| 55 | /// in this workspace by a wide margin; past that the file is generated or |
| 56 | /// vendored, where a syntax verdict is worth less than the latency. |
| 57 | const MAX_CHECKED_BYTES: usize = 2 * 1024 * 1024; |
| 58 | |
| 59 | /// A language the edit path can parse. Extensions outside this set fail open. |
| 60 | #[derive(Debug, Clone, Copy, PartialEq, Eq)] |
| 61 | pub(super) enum SyntaxLanguage { |
| 62 | Rust, |
| 63 | Toml, |
| 64 | Json, |
| 65 | } |
| 66 | |
| 67 | impl SyntaxLanguage { |
| 68 | /// Pick a parser from the file extension, or `None` to skip the check. |
| 69 | fn from_path(path: &Path) -> Option<Self> { |
| 70 | let extension = path.extension()?.to_str()?.to_ascii_lowercase(); |
| 71 | match extension.as_str() { |
| 72 | "rs" => Some(Self::Rust), |
| 73 | // Covers `Cargo.toml`, `deny.toml`, `.cargo/config.toml` and every |
| 74 | // ordinary `*.toml` uniformly — the extension is the whole rule. |
| 75 | "toml" => Some(Self::Toml), |
| 76 | "json" => Some(Self::Json), |
| 77 | _ => None, |
| 78 | } |
| 79 | } |
| 80 | |
| 81 | fn label(self) -> &'static str { |
| 82 | match self { |
| 83 | Self::Rust => "Rust", |
| 84 | Self::Toml => "TOML", |
| 85 | Self::Json => "JSON", |
| 86 | } |
| 87 | } |
| 88 | } |
| 89 | |
| 90 | /// A parse failure, located precisely enough for the model to fix it directly. |
| 91 | #[derive(Debug, Clone)] |
| 92 | pub(super) struct SyntaxIssue { |
| 93 | language: SyntaxLanguage, |
| 94 | /// 1-based line, as every editor and compiler reports it. |
| 95 | line: usize, |
| 96 | /// 1-based column, likewise. |
| 97 | column: usize, |
| 98 | message: String, |
| 99 | } |
| 100 | |
| 101 | impl fmt::Display for SyntaxIssue { |
| 102 | fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { |
| 103 | write!( |
| 104 | formatter, |
| 105 | "{} syntax error at line {}, column {}: {}", |
| 106 | self.language.label(), |
| 107 | self.line, |
| 108 | self.column, |
| 109 | self.message |
| 110 | ) |
| 111 | } |
| 112 | } |
| 113 | |
| 114 | /// Parse `source` as the language implied by `path`. |
| 115 | /// |
| 116 | /// `None` means "no objection": the content parses, the extension is not one |
| 117 | /// we parse, or the file is too large to be worth checking. |
| 118 | pub(super) fn syntax_check(path: &Path, source: &str) -> Option<SyntaxIssue> { |
| 119 | if source.len() > MAX_CHECKED_BYTES { |
| 120 | return None; |
| 121 | } |
| 122 | match SyntaxLanguage::from_path(path)? { |
| 123 | SyntaxLanguage::Rust => check_rust(source), |
| 124 | SyntaxLanguage::Toml => check_toml(source), |
| 125 | SyntaxLanguage::Json => check_json(source), |
| 126 | } |
| 127 | } |
| 128 | |
| 129 | /// Refuse an edit that would take a parseable file to an unparseable one. |
| 130 | /// |
| 131 | /// `before` is the pre-edit content, or `None` when the edit creates the file. |
| 132 | /// Callers invoke this before writing, so a rejection leaves the file on disk |
| 133 | /// byte-for-byte untouched and there is nothing to roll back. |
| 134 | pub(super) fn guard_edit( |
| 135 | path: &Path, |
| 136 | display_path: &str, |
| 137 | before: Option<&str>, |
| 138 | after: &str, |
| 139 | ) -> Result<(), ToolError> { |
| 140 | let Some(issue) = syntax_check(path, after) else { |
| 141 | return Ok(()); |
| 142 | }; |
| 143 | // Fail open on a file that was already broken (or is brand new): the gate |
| 144 | // exists to catch the edit that *introduces* a syntax error, never to |
| 145 | // strand a model that is repairing one. |
| 146 | let Some(before) = before else { |
| 147 | return Ok(()); |
| 148 | }; |
| 149 | if syntax_check(path, before).is_some() { |
| 150 | return Ok(()); |
| 151 | } |
| 152 | Err(ToolError::execution_failed(format!( |
| 153 | "Edit refused: it would leave {display_path} unparseable — {issue}. Nothing was written; \ |
| 154 | the file is unchanged. Recovery: re-read the file with File action=\"read\", check the \ |
| 155 | replacement for unbalanced delimiters or a truncated block, and retry." |
| 156 | ))) |
| 157 | } |
| 158 | |
| 159 | fn check_rust(source: &str) -> Option<SyntaxIssue> { |
| 160 | let error = syn::parse_file(source).err()?; |
| 161 | // A `syn::Error` can carry several diagnostics; the first is the earliest |
| 162 | // and the one worth showing. |
| 163 | let first = error.into_iter().next()?; |
| 164 | let start = first.span().start(); |
| 165 | Some(SyntaxIssue { |
| 166 | language: SyntaxLanguage::Rust, |
| 167 | line: start.line, |
| 168 | // `proc-macro2` columns are 0-based; editors and rustc are not. |
| 169 | column: start.column.saturating_add(1), |
| 170 | message: first.to_string(), |
| 171 | }) |
| 172 | } |
| 173 | |
| 174 | fn check_toml(source: &str) -> Option<SyntaxIssue> { |
| 175 | let error = source.parse::<toml_edit::DocumentMut>().err()?; |
| 176 | let (line, column) = error |
| 177 | .span() |
| 178 | .map_or((1, 1), |span| line_column(source, span.start)); |
| 179 | Some(SyntaxIssue { |
| 180 | language: SyntaxLanguage::Toml, |
| 181 | line, |
| 182 | column, |
| 183 | message: error.message().trim().to_string(), |
| 184 | }) |
| 185 | } |
| 186 | |
| 187 | fn check_json(source: &str) -> Option<SyntaxIssue> { |
| 188 | let error = serde_json::from_str::<serde_json::Value>(source).err()?; |
| 189 | let rendered = error.to_string(); |
| 190 | // `serde_json` already appends " at line L column C" to its message; the |
| 191 | // location is reported in its own fields, so drop the duplicate tail. |
| 192 | let message = rendered |
| 193 | .split_once(" at line ") |
| 194 | .map_or(rendered.as_str(), |(head, _)| head); |
| 195 | Some(SyntaxIssue { |
| 196 | language: SyntaxLanguage::Json, |
| 197 | // A zero means "position unknown" (e.g. an IO-shaped error); the |
| 198 | // 1-based floor keeps the rendered location honest either way. |
| 199 | line: error.line().max(1), |
| 200 | column: error.column().max(1), |
| 201 | message: message.to_string(), |
| 202 | }) |
| 203 | } |
| 204 | |
| 205 | /// Translate a byte offset into a 1-based line and column. |
| 206 | /// |
| 207 | /// `toml_edit` reports a byte span; every human-facing tool reports line and |
| 208 | /// column. Counting is over `char`s rather than bytes so a column lands where |
| 209 | /// the reader's cursor does in a file with non-ASCII content. |
| 210 | fn line_column(source: &str, offset: usize) -> (usize, usize) { |
| 211 | let offset = offset.min(source.len()); |
| 212 | let head = &source[..offset]; |
| 213 | let line = head.matches('\n').count() + 1; |
| 214 | let column = head |
| 215 | .rfind('\n') |
| 216 | .map_or(head, |index| &head[index + 1..]) |
| 217 | .chars() |
| 218 | .count() |
| 219 | + 1; |
| 220 | (line, column) |
| 221 | } |
| 222 | |
| 223 | #[cfg(test)] |
| 224 | mod tests { |
| 225 | use super::*; |
| 226 | use std::path::PathBuf; |
| 227 | |
| 228 | fn rust_path() -> PathBuf { |
| 229 | PathBuf::from("src/lib.rs") |
| 230 | } |
| 231 | |
| 232 | #[test] |
| 233 | fn valid_rust_passes() { |
| 234 | assert!(syntax_check(&rust_path(), "fn main() {}\n").is_none()); |
| 235 | } |
| 236 | |
| 237 | #[test] |
| 238 | fn missing_brace_reports_line_and_column() { |
| 239 | let issue = syntax_check(&rust_path(), "fn main() {\n let x = 1;\n") |
| 240 | .expect("unbalanced brace must be reported"); |
| 241 | assert_eq!(issue.language, SyntaxLanguage::Rust); |
| 242 | assert!(issue.line >= 1, "{issue}"); |
| 243 | assert!(issue.column >= 1, "{issue}"); |
| 244 | let rendered = issue.to_string(); |
| 245 | assert!(rendered.contains("Rust syntax error at line"), "{rendered}"); |
| 246 | } |
| 247 | |
| 248 | #[test] |
| 249 | fn valid_toml_passes() { |
| 250 | assert!(syntax_check(Path::new("Cargo.toml"), "[package]\nname = \"x\"\n").is_none()); |
| 251 | } |
| 252 | |
| 253 | #[test] |
| 254 | fn broken_toml_reports_line_and_column() { |
| 255 | let issue = syntax_check(Path::new("Cargo.toml"), "[package]\nname = \n") |
| 256 | .expect("a value-less key must be reported"); |
| 257 | assert_eq!(issue.language, SyntaxLanguage::Toml); |
| 258 | assert_eq!(issue.line, 2, "{issue}"); |
| 259 | let rendered = issue.to_string(); |
| 260 | assert!( |
| 261 | rendered.contains("TOML syntax error at line 2"), |
| 262 | "{rendered}" |
| 263 | ); |
| 264 | } |
| 265 | |
| 266 | #[test] |
| 267 | fn valid_json_passes() { |
| 268 | assert!(syntax_check(Path::new("data.json"), "{\"a\": [1, 2]}").is_none()); |
| 269 | } |
| 270 | |
| 271 | #[test] |
| 272 | fn broken_json_reports_line_and_column() { |
| 273 | let issue = syntax_check(Path::new("data.json"), "{\n \"a\": [1, 2,\n}\n") |
| 274 | .expect("a trailing comma must be reported"); |
| 275 | assert_eq!(issue.language, SyntaxLanguage::Json); |
| 276 | assert_eq!(issue.line, 3, "{issue}"); |
| 277 | let rendered = issue.to_string(); |
| 278 | assert!( |
| 279 | rendered.contains("JSON syntax error at line 3"), |
| 280 | "{rendered}" |
| 281 | ); |
| 282 | assert!( |
| 283 | !rendered.contains("at line 3 column"), |
| 284 | "serde_json's duplicate location tail must be stripped: {rendered}" |
| 285 | ); |
| 286 | } |
| 287 | |
| 288 | #[test] |
| 289 | fn jsonc_with_comments_is_not_parsed_as_json_before_the_edit() { |
| 290 | // A `.json` file that is really JSONC does not parse either way, so |
| 291 | // the before/after rule skips it instead of making it uneditable. |
| 292 | let commented = "{\n // note\n \"a\": 1\n}\n"; |
| 293 | assert!(syntax_check(Path::new("tsconfig.json"), commented).is_some()); |
| 294 | guard_edit( |
| 295 | Path::new("tsconfig.json"), |
| 296 | "tsconfig.json", |
| 297 | Some(commented), |
| 298 | "{\n // note\n \"a\": 2\n}\n", |
| 299 | ) |
| 300 | .expect("a JSONC file must stay editable"); |
| 301 | } |
| 302 | |
| 303 | #[test] |
| 304 | fn guard_rejects_an_edit_that_breaks_a_manifest() { |
| 305 | let error = guard_edit( |
| 306 | Path::new("Cargo.toml"), |
| 307 | "Cargo.toml", |
| 308 | Some("[package]\nname = \"x\"\n"), |
| 309 | "[package\nname = \"x\"\n", |
| 310 | ) |
| 311 | .expect_err("an unparseable manifest must be refused at edit time"); |
| 312 | let message = error.to_string(); |
| 313 | assert!(message.contains("TOML syntax error at line"), "{message}"); |
| 314 | assert!(message.contains("Nothing was written"), "{message}"); |
| 315 | } |
| 316 | |
| 317 | #[test] |
| 318 | fn unknown_extension_is_skipped() { |
| 319 | assert!(syntax_check(Path::new("notes.txt"), "fn main() {").is_none()); |
| 320 | } |
| 321 | |
| 322 | #[test] |
| 323 | fn oversized_source_is_skipped() { |
| 324 | let huge = format!("fn main() {{{}", " ".repeat(MAX_CHECKED_BYTES)); |
| 325 | assert!(syntax_check(&rust_path(), &huge).is_none()); |
| 326 | } |
| 327 | |
| 328 | #[test] |
| 329 | fn guard_rejects_newly_broken_rust() { |
| 330 | let error = guard_edit( |
| 331 | &rust_path(), |
| 332 | "src/lib.rs", |
| 333 | Some("fn main() {}\n"), |
| 334 | "fn main() {\n", |
| 335 | ) |
| 336 | .expect_err("an edit that breaks a parseable file must be refused"); |
| 337 | let message = error.to_string(); |
| 338 | assert!(message.contains("src/lib.rs"), "{message}"); |
| 339 | assert!(message.contains("Rust syntax error at line"), "{message}"); |
| 340 | assert!(message.contains("Nothing was written"), "{message}"); |
| 341 | } |
| 342 | |
| 343 | #[test] |
| 344 | fn guard_allows_repairing_an_already_broken_file() { |
| 345 | // Still broken after the edit, but it was broken before: a model |
| 346 | // mid-repair must not be locked out. |
| 347 | guard_edit( |
| 348 | &rust_path(), |
| 349 | "src/lib.rs", |
| 350 | Some("fn main() {\n"), |
| 351 | "fn main() {\n let x = 1;\n", |
| 352 | ) |
| 353 | .expect("pre-existing breakage must fail open"); |
| 354 | } |
| 355 | |
| 356 | #[test] |
| 357 | fn guard_allows_creating_a_new_file() { |
| 358 | guard_edit(&rust_path(), "src/lib.rs", None, "fn main() {\n") |
| 359 | .expect("file creation has no prior state to regress"); |
| 360 | } |
| 361 | |
| 362 | #[test] |
| 363 | fn guard_allows_a_valid_edit() { |
| 364 | guard_edit( |
| 365 | &rust_path(), |
| 366 | "src/lib.rs", |
| 367 | Some("fn main() {}\n"), |
| 368 | "fn main() {\n println!(\"hi\");\n}\n", |
| 369 | ) |
| 370 | .expect("a syntactically valid edit must pass"); |
| 371 | } |
| 372 | } |
| 373 |