From 933f72b6ee9a095bd6ae4f04c0254f18edf7ad04 Mon Sep 17 00:00:00 2001 From: Jay Bosamiya Date: Fri, 29 Dec 2023 07:13:17 -0500 Subject: [PATCH] Better error output, and verusfmt before rustfmt Details: * run verusfmt formatting before doing rustfmt; this allows us to give more precise error locations in case of parse failures; the two formatters should otherwise not interact with each other anyways, so the choice of which one first was not particularly an important decision anyways * switch from `anyhow` to `miette` to enable nicer outputs * start reporting the file name for the parse failure * produce nicer error output along with context lines --- Cargo.lock | 197 +++++++++++++++++++++++++++++++++-- Cargo.toml | 3 +- src/lib.rs | 79 +++++++++++++- src/main.rs | 32 +++--- tests/syntax-rs-unchanged.rs | 2 +- 5 files changed, 283 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ddb83c2..c0113ba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,21 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "addr2line" +version = "0.21.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a30b2e23b9e17a9f90641c7ab1549cd9b44f296d3ccbf309d2863cfe398a0cb" +dependencies = [ + "gimli", +] + +[[package]] +name = "adler" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f26201604c87b1e01bd3d98f8d5d9a8fcbb815e8cedb41ffccbeb4bf593a35fe" + [[package]] name = "aho-corasick" version = "1.1.1" @@ -60,18 +75,36 @@ dependencies = [ "windows-sys 0.48.0", ] -[[package]] -name = "anyhow" -version = "1.0.71" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c7d0618f0e0b7e8ff11427422b64564d5fb0be1940354bfe2e0529b18a9d9b8" - [[package]] name = "arrayvec" version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "23b62fc65de8e4e7f52534fb52b0f3ed04746ae267519eef2a83941e8085068b" +[[package]] +name = "backtrace" +version = "0.3.69" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2089b7e3f35b9dd2d0ed921ead4f6d318c27680d4a5bd167b3ee120edb105837" +dependencies = [ + "addr2line", + "cc", + "cfg-if", + "libc", + "miniz_oxide", + "object", + "rustc-demangle", +] + +[[package]] +name = "backtrace-ext" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "537beee3be4a18fb023b570f80e3ae28003db9167a751266b259926e25539d50" +dependencies = [ + "backtrace", +] + [[package]] name = "bitflags" version = "2.3.3" @@ -236,6 +269,12 @@ dependencies = [ "version_check", ] +[[package]] +name = "gimli" +version = "0.28.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4271d37baee1b8c7e4b708028c57d816cf9d2434acb33a549475f78c181f6253" + [[package]] name = "heck" version = "0.4.1" @@ -272,6 +311,12 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "is_ci" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "616cde7c720bb2bb5824a224687d8f77bfd38922027f01d825cd7453be5099fb" + [[package]] name = "itertools" version = "0.11.0" @@ -317,6 +362,47 @@ version = "2.6.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f665ee40bc4a3c5590afb1e9677db74a508659dfd71e126420da8274909a0167" +[[package]] +name = "miette" +version = "5.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59bb584eaeeab6bd0226ccf3509a69d7936d148cf3d036ad350abe35e8c6856e" +dependencies = [ + "backtrace", + "backtrace-ext", + "is-terminal", + "miette-derive", + "once_cell", + "owo-colors", + "supports-color", + "supports-hyperlinks", + "supports-unicode", + "terminal_size", + "textwrap", + "thiserror", + "unicode-width", +] + +[[package]] +name = "miette-derive" +version = "5.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "49e7bc1560b95a3c4a25d03de42fe76ca718ab92d1a22a55b9b4cf67b3ae635c" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "miniz_oxide" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e7810e0be55b428ada41041c41f32c9f1a42817901b4ccf45fa3d4b6561e74c7" +dependencies = [ + "adler", +] + [[package]] name = "nu-ansi-term" version = "0.46.0" @@ -327,6 +413,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "object" +version = "0.32.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6a622008b6e321afc04970976f62ee297fdbaa6f95318ca343e3eebb9648441" +dependencies = [ + "memchr", +] + [[package]] name = "once_cell" version = "1.18.0" @@ -339,6 +434,12 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" +[[package]] +name = "owo-colors" +version = "3.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c1b04fb49957986fdce4d6ee7a65027d55d4b6d2265e5848bbb507b58ccfdb6f" + [[package]] name = "pest" version = "2.7.0" @@ -447,6 +548,12 @@ version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dbb5fb1acd8a1a18b3dd5be62d25485eb770e05afb408a9627d14d451bae12da" +[[package]] +name = "rustc-demangle" +version = "0.1.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d626bb9dae77e28219937af045c257c28bfd3f69333c512553507f5f9798cb76" + [[package]] name = "rustix" version = "0.38.4" @@ -492,12 +599,46 @@ version = "1.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "62bb4feee49fdd9f707ef802e22365a35de4b7b299de4763d44bfea899442ff9" +[[package]] +name = "smawk" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7c388c1b5e93756d0c740965c41e8822f866621d41acbdf6336a6a168f8840c" + [[package]] name = "strsim" version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" +[[package]] +name = "supports-color" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d6398cde53adc3c4557306a96ce67b302968513830a77a95b2b17305d9719a89" +dependencies = [ + "is-terminal", + "is_ci", +] + +[[package]] +name = "supports-hyperlinks" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f84231692eb0d4d41e4cdd0cabfdd2e6cd9e255e65f80c9aa7c98dd502b4233d" +dependencies = [ + "is-terminal", +] + +[[package]] +name = "supports-unicode" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b6c2cb240ab5dd21ed4906895ee23fe5a48acdbd15a3ce388e7b62a9b66baf7" +dependencies = [ + "is-terminal", +] + [[package]] name = "syn" version = "2.0.25" @@ -509,20 +650,41 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "terminal_size" +version = "0.1.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "633c1a546cee861a1a6d0dc69ebeca693bf4296661ba7852b9d21d159e0506df" +dependencies = [ + "libc", + "winapi", +] + +[[package]] +name = "textwrap" +version = "0.15.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7b3e525a49ec206798b40326a44121291b530c963cfb01018f63e135bac543d" +dependencies = [ + "smawk", + "unicode-linebreak", + "unicode-width", +] + [[package]] name = "thiserror" -version = "1.0.43" +version = "1.0.52" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a35fc5b8971143ca348fa6df4f024d4d55264f3468c71ad1c2f365b0a4d58c42" +checksum = "83a48fd946b02c0a526b2e9481c8e2a17755e47039164a86c4070446e3a4614d" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.43" +version = "1.0.52" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "463fe12d7993d3b327787537ce8dd4dfa058de32fc2b195ef3cde03dc4771e8f" +checksum = "e7fbe9b594d6568a6a1443250a7e67d80b74e1e96f6d1715e1e21cc1888291d3" dependencies = [ "proc-macro2", "quote", @@ -621,12 +783,24 @@ version = "1.0.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "22049a19f4a68748a168c0fc439f9516686aa045927ff767eca0a85101fb6e73" +[[package]] +name = "unicode-linebreak" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b09c83c3c29d37506a3e260c08c03743a6bb66a9cd432c6934ab501a190571f" + [[package]] name = "unicode-segmentation" version = "1.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1dd624098567895118886609431a7c3b8f516e41d30e0643f03d94592a147e36" +[[package]] +name = "unicode-width" +version = "0.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e51733f11c9c4f72aa0c160008246859e340b00807569a0da0e7a1079b27ba85" + [[package]] name = "utf8parse" version = "0.2.1" @@ -649,16 +823,17 @@ checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" name = "verusfmt" version = "0.1.0" dependencies = [ - "anyhow", "clap", "fs-err", "insta", "itertools", + "miette", "pest", "pest_derive", "pretty", "regex", "similar", + "thiserror", "tracing", "tracing-subscriber", ] diff --git a/Cargo.toml b/Cargo.toml index 67e070a..f134915 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,16 +5,17 @@ edition = "2021" autoexamples = false [dependencies] -anyhow = "1.0.71" clap = { version = "4.3.11", features = ["derive"] } fs-err = "2.9.0" insta = "1.30.0" itertools = "0.11.0" +miette = { version = "5.10.0", features = ["fancy"] } pest = "2.0" pest_derive = "2.0" pretty = "0.12.1" regex = "1.9.6" similar = "2.2.1" +thiserror = "1.0.52" tracing = "0.1.37" tracing-subscriber = { version = "0.3.17" } diff --git a/src/lib.rs b/src/lib.rs index 38a405e..0aaa26e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1290,11 +1290,86 @@ fn is_multiline_comment(pair: &Pair) -> bool { matches!(&pair.as_span().as_str()[..2], "/*") } -pub fn parse_and_format(s: &str) -> Result> { +#[derive(thiserror::Error, Debug)] +pub enum ParseAndFormatError { + #[error("Failed to parse")] + ParseError { pest_err: pest::error::Error }, +} + +impl From> for ParseAndFormatError { + fn from(pest_err: pest::error::Error) -> Self { + Self::ParseError { pest_err } + } +} + +impl miette::Diagnostic for ParseAndFormatError { + fn help<'a>(&'a self) -> Option> { + match self { + ParseAndFormatError::ParseError { pest_err, .. } => { + let mut help = String::new(); + match &pest_err.variant { + pest::error::ErrorVariant::ParsingError { + positives, + negatives, + } => { + if !positives.is_empty() { + help += "Expected one of: "; + for (i, p) in positives.iter().enumerate() { + if i > 0 { + help += ", "; + } + help += format!("{:?}", p).as_str(); + } + } + if !negatives.is_empty() { + if !help.is_empty() { + help += "; "; + } + help += "Negatives: "; + for (i, n) in negatives.iter().enumerate() { + if i > 0 { + help += ", "; + } + help += format!("{:?}", n).as_str(); + } + } + if help.is_empty() { + return None; + } + } + pest::error::ErrorVariant::CustomError { message } => { + help += message; + } + } + Some(Box::new(help)) + } + } + } + + fn labels(&self) -> Option + '_>> { + match self { + ParseAndFormatError::ParseError { pest_err, .. } => { + let (start, len) = match pest_err.location { + pest::error::InputLocation::Pos(start) => (start, 1), + pest::error::InputLocation::Span((start, end)) => (start, end - start), + }; + + Some(Box::new(std::iter::once(miette::LabeledSpan::new( + Some("here".to_owned()), + start, + len, + )))) + } + } + } +} + +pub fn parse_and_format(s: &str) -> miette::Result { let ctx = Context { inline_comment_lines: find_inline_comment_lines(s), }; - let parsed_file = VerusParser::parse(Rule::file, s)? + let parsed_file = VerusParser::parse(Rule::file, s) + .map_err(ParseAndFormatError::from)? .next() .expect("There will be exactly one `file` rule matched in a valid parsed file") .into_inner(); diff --git a/src/main.rs b/src/main.rs index 085dda8..0820441 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,8 +1,8 @@ use std::path::PathBuf; -use anyhow::anyhow; use clap::Parser as ClapParser; use fs_err as fs; +use miette::{miette, IntoDiagnostic, NamedSource}; use tracing::{error, info}; // debug, trace use verusfmt::{parse_and_format, rustfmt}; @@ -26,18 +26,20 @@ struct Args { debug_level: u8, } -fn format_file(file: &PathBuf, check: bool, verus_only: bool) -> anyhow::Result<()> { - let unparsed_file = fs::read_to_string(file)?; +fn format_file(file: &PathBuf, check: bool, verus_only: bool) -> miette::Result<()> { + let unparsed_file = fs::read_to_string(file).into_diagnostic()?; + + let verus_fmted = parse_and_format(&unparsed_file).map_err(|e| { + e.with_source_code(NamedSource::new( + file.to_string_lossy(), + unparsed_file.clone(), + )) + })?; + let formatted_output = if verus_only { - parse_and_format(&unparsed_file)? + verus_fmted } else { - let rustfmt_out = match rustfmt(&unparsed_file) { - None => { - return Err(anyhow!("rustfmt failed")); - } - Some(s) => s, - }; - parse_and_format(&rustfmt_out)? + rustfmt(&unparsed_file).ok_or(miette!("rustfmt failed"))? }; if check { @@ -55,19 +57,19 @@ fn format_file(file: &PathBuf, check: bool, verus_only: bool) -> anyhow::Result< Some(("original", "formatted")), ); println!("{diff}"); - return Err(anyhow!("invalid formatting")); + return Err(miette!("invalid formatting")); } } else { - fs::write(file, formatted_output)?; + fs::write(file, formatted_output).into_diagnostic()?; Ok(()) } } // TODO: Call rustfmt on the code too (maybe include an option to skip it) -fn main() -> anyhow::Result<()> { +fn main() -> miette::Result<()> { let args = Args::parse(); if args.files.len() == 0 { - return Err(anyhow!("No files specified")); + return Err(miette!("No files specified")); } tracing_subscriber::fmt() diff --git a/tests/syntax-rs-unchanged.rs b/tests/syntax-rs-unchanged.rs index 5cda8fc..048c626 100644 --- a/tests/syntax-rs-unchanged.rs +++ b/tests/syntax-rs-unchanged.rs @@ -10,5 +10,5 @@ use verusfmt::parse_and_format; #[test] fn syntax_rs_unchanged() { let syntax_rs = include_str!("../examples/syntax.rs").to_owned(); - assert_eq!(parse_and_format(&syntax_rs), Ok(syntax_rs)); + assert_eq!(parse_and_format(&syntax_rs).unwrap(), syntax_rs); }