From ef4af443a509e013e6fa4ef6f0a149cea1aba1d1 Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Tue, 10 Aug 2021 17:08:10 +1200 Subject: [PATCH] parser fixes for windows and pretty errors --- Cargo.toml | 1 + src/errors.rs | 322 ++++++++++++++++++++++++++++++++++++++++++++ src/eval.rs | 4 +- src/lex.rs | 3 +- src/lib.rs | 4 +- src/main.rs | 19 ++- src/parser_state.rs | 59 +++++++- src/tests.rs | 16 ++- 8 files changed, 410 insertions(+), 18 deletions(-) create mode 100644 src/errors.rs diff --git a/Cargo.toml b/Cargo.toml index b661d778b..7808c544c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,6 +8,7 @@ edition = "2018" [dependencies] reedline = { git = "https://github.com/jntrnr/reedline", branch = "main" } nu-ansi-term = "0.32.0" +codespan-reporting = "0.11.1" # mimalloc = { version = "*", default-features = false } [dev-dependencies] diff --git a/src/errors.rs b/src/errors.rs new file mode 100644 index 000000000..2af1c7a16 --- /dev/null +++ b/src/errors.rs @@ -0,0 +1,322 @@ +use core::ops::Range; + +use crate::{ParseError, ParserWorkingSet, ShellError, Span}; +use codespan_reporting::diagnostic::{Diagnostic, Label}; +use codespan_reporting::term::termcolor::{ColorChoice, StandardStream}; + +impl<'a> codespan_reporting::files::Files<'a> for ParserWorkingSet<'a> { + type FileId = usize; + + type Name = String; + + type Source = String; + + fn name(&'a self, id: Self::FileId) -> Result { + Ok(self.get_filename(id)) + } + + fn source( + &'a self, + id: Self::FileId, + ) -> Result { + Ok(self.get_file_source(id)) + } + + fn line_index( + &'a self, + id: Self::FileId, + byte_index: usize, + ) -> Result { + let source = self.get_file_source(id); + + let mut count = 0; + + for byte in source.bytes().enumerate() { + if byte.0 == byte_index { + // println!("count: {} for file: {} index: {}", count, id, byte_index); + return Ok(count); + } + if byte.1 == b'\n' { + count += 1; + } + } + + // println!("count: {} for file: {} index: {}", count, id, byte_index); + Ok(count) + } + + fn line_range( + &'a self, + id: Self::FileId, + line_index: usize, + ) -> Result, codespan_reporting::files::Error> { + let source = self.get_file_source(id); + + let mut count = 0; + + let mut start = Some(0); + let mut end = None; + + for byte in source.bytes().enumerate() { + #[allow(clippy::comparison_chain)] + if count > line_index { + let start = start.expect("internal error: couldn't find line"); + let end = end.expect("internal error: couldn't find line"); + + // println!( + // "Span: {}..{} for fileid: {} index: {}", + // start, end, id, line_index + // ); + return Ok(start..end); + } else if count == line_index { + end = Some(byte.0 + 1); + } + + #[allow(clippy::comparison_chain)] + if byte.1 == b'\n' { + count += 1; + if count > line_index { + break; + } else if count == line_index { + start = Some(byte.0); + } + } + } + + match (start, end) { + (Some(start), Some(end)) => { + // println!( + // "Span: {}..{} for fileid: {} index: {}", + // start, end, id, line_index + // ); + Ok(start..end) + } + _ => Err(codespan_reporting::files::Error::FileMissing), + } + } +} + +fn convert_span_to_diag( + working_set: &ParserWorkingSet, + span: &Span, +) -> Result<(usize, Range), Box> { + for (file_id, (_, start, end)) in working_set.files().enumerate() { + if span.start >= *start && span.end <= *end { + let new_start = span.start - start; + let new_end = span.end - start; + + return Ok((file_id, new_start..new_end)); + } + } + + panic!("internal error: can't find span in parser state") +} + +pub fn report_parsing_error( + working_set: &ParserWorkingSet, + error: &ParseError, +) -> Result<(), Box> { + let writer = StandardStream::stderr(ColorChoice::Always); + let config = codespan_reporting::term::Config::default(); + + let diagnostic = + match error { + ParseError::Mismatch(missing, span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Type mismatch during operation") + .with_labels(vec![Label::primary(diag_file_id, diag_range) + .with_message(format!("expected {}", missing))]) + } + ParseError::ExtraTokens(span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Extra tokens in code") + .with_labels(vec![ + Label::primary(diag_file_id, diag_range).with_message("extra tokens") + ]) + } + ParseError::ExtraPositional(span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Extra positional argument") + .with_labels(vec![Label::primary(diag_file_id, diag_range) + .with_message("extra positional argument")]) + } + ParseError::UnexpectedEof(s, span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Unexpected end of code") + .with_labels(vec![Label::primary(diag_file_id, diag_range) + .with_message(format!("expected {}", s))]) + } + ParseError::Unclosed(delim, span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Unclosed delimiter") + .with_labels(vec![Label::primary(diag_file_id, diag_range) + .with_message(format!("unclosed {}", delim))]) + } + ParseError::UnknownStatement(span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Unknown statement") + .with_labels(vec![ + Label::primary(diag_file_id, diag_range).with_message("unknown statement") + ]) + } + ParseError::MultipleRestParams(span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Multiple rest params") + .with_labels(vec![Label::primary(diag_file_id, diag_range) + .with_message("multiple rest params")]) + } + ParseError::VariableNotFound(span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Variable not found") + .with_labels(vec![ + Label::primary(diag_file_id, diag_range).with_message("variable not found") + ]) + } + ParseError::UnknownCommand(span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Unknown command") + .with_labels(vec![ + Label::primary(diag_file_id, diag_range).with_message("unknown command") + ]) + } + ParseError::UnknownFlag(span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Unknown flag") + .with_labels(vec![ + Label::primary(diag_file_id, diag_range).with_message("unknown flag") + ]) + } + ParseError::UnknownType(span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Unknown type") + .with_labels(vec![ + Label::primary(diag_file_id, diag_range).with_message("unknown type") + ]) + } + ParseError::MissingFlagParam(span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Missing flag param") + .with_labels(vec![Label::primary(diag_file_id, diag_range) + .with_message("flag missing parameter")]) + } + ParseError::ShortFlagBatchCantTakeArg(span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Batches of short flags can't take arguments") + .with_labels(vec![Label::primary(diag_file_id, diag_range) + .with_message("short flag batches can't take args")]) + } + ParseError::MissingPositional(name, span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Missing required positional arg") + .with_labels(vec![Label::primary(diag_file_id, diag_range) + .with_message(format!("missing {}", name))]) + } + ParseError::MissingType(span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Missing type") + .with_labels(vec![ + Label::primary(diag_file_id, diag_range).with_message("expected type") + ]) + } + ParseError::TypeMismatch(ty, span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Type mismatch") + .with_labels(vec![Label::primary(diag_file_id, diag_range) + .with_message(format!("expected {:?}", ty))]) + } + ParseError::MissingRequiredFlag(name, span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Missing required flag") + .with_labels(vec![Label::primary(diag_file_id, diag_range) + .with_message(format!("missing required flag {}", name))]) + } + ParseError::IncompleteMathExpression(span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Incomplete math expresssion") + .with_labels(vec![Label::primary(diag_file_id, diag_range) + .with_message("incomplete math expression")]) + } + ParseError::UnknownState(name, span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Unknown state") + .with_labels(vec![Label::primary(diag_file_id, diag_range) + .with_message(format!("unknown state {}", name))]) + } + ParseError::NonUtf8(span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Non-UTF8 code") + .with_labels(vec![ + Label::primary(diag_file_id, diag_range).with_message("non-UTF8 code") + ]) + } + }; + + // println!("DIAG"); + // println!("{:?}", diagnostic); + codespan_reporting::term::emit(&mut writer.lock(), &config, working_set, &diagnostic)?; + + Ok(()) +} + +pub fn report_shell_error( + working_set: &ParserWorkingSet, + error: &ShellError, +) -> Result<(), Box> { + let writer = StandardStream::stderr(ColorChoice::Always); + let config = codespan_reporting::term::Config::default(); + + let diagnostic = match error { + ShellError::Mismatch(missing, span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Type mismatch during operation") + .with_labels(vec![Label::primary(diag_file_id, diag_range) + .with_message(format!("expected {}", missing))]) + } + ShellError::Unsupported(span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Unsupported operation") + .with_labels(vec![ + Label::primary(diag_file_id, diag_range).with_message("unsupported operation") + ]) + } + ShellError::InternalError(s) => { + Diagnostic::error().with_message(format!("Internal error: {}", s)) + } + ShellError::VariableNotFound(span) => { + let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; + Diagnostic::error() + .with_message("Variable not found") + .with_labels(vec![ + Label::primary(diag_file_id, diag_range).with_message("variable not found") + ]) + } + }; + + // println!("DIAG"); + // println!("{:?}", diagnostic); + codespan_reporting::term::emit(&mut writer.lock(), &config, working_set, &diagnostic)?; + + Ok(()) +} diff --git a/src/eval.rs b/src/eval.rs index 9876dffe1..3ad4820c6 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -358,9 +358,7 @@ fn eval_call(state: &State, stack: Stack, call: &Call) -> Result std::io::Result<()> { @@ -130,7 +131,8 @@ fn main() -> std::io::Result<()> { let mut working_set = ParserWorkingSet::new(&*parser_state); let (output, err) = working_set.parse_file(&path, &file, false); if let Some(err) = err { - eprintln!("Parse Error: {:?}", err); + let _ = report_parsing_error(&working_set, &err); + std::process::exit(1); } (output, working_set.render()) @@ -149,7 +151,11 @@ fn main() -> std::io::Result<()> { println!("{}", value); } Err(err) => { - eprintln!("Eval Error: {:?}", err); + let parser_state = parser_state.borrow(); + let working_set = ParserWorkingSet::new(&*parser_state); + + let _ = engine_q::report_shell_error(&working_set, &err); + std::process::exit(1); } } @@ -189,7 +195,7 @@ fn main() -> std::io::Result<()> { false, ); if let Some(err) = err { - eprintln!("Parse Error: {:?}", err); + let _ = report_parsing_error(&working_set, &err); continue; } (output, working_set.render()) @@ -206,7 +212,10 @@ fn main() -> std::io::Result<()> { println!("{}", value); } Err(err) => { - eprintln!("Eval Error: {:?}", err); + let parser_state = parser_state.borrow(); + let working_set = ParserWorkingSet::new(&*parser_state); + + let _ = report_shell_error(&working_set, &err); } } } diff --git a/src/parser_state.rs b/src/parser_state.rs index 1b4874610..50e12b8e9 100644 --- a/src/parser_state.rs +++ b/src/parser_state.rs @@ -1,6 +1,6 @@ use crate::{parser::Block, Declaration, Span}; use core::panic; -use std::collections::HashMap; +use std::{collections::HashMap, slice::Iter}; #[derive(Debug)] pub struct ParserState { @@ -155,6 +155,33 @@ impl ParserState { self.file_contents.len() } + pub fn files(&self) -> Iter<(String, usize, usize)> { + self.files.iter() + } + + pub fn get_filename(&self, file_id: usize) -> String { + for file in self.files.iter().enumerate() { + if file.0 == file_id { + return file.1 .0.clone(); + } + } + + "".into() + } + + pub fn get_file_source(&self, file_id: usize) -> String { + for file in self.files.iter().enumerate() { + if file.0 == file_id { + let output = + String::from_utf8_lossy(&self.file_contents[file.1 .1..file.1 .2]).to_string(); + + return output; + } + } + + "".into() + } + #[allow(unused)] pub(crate) fn add_file(&mut self, filename: String, contents: Vec) -> usize { let next_span_start = self.next_span_start(); @@ -264,6 +291,36 @@ impl<'a> ParserWorkingSet<'a> { self.permanent_state.next_span_start() } + pub fn files(&'a self) -> impl Iterator { + self.permanent_state.files().chain(self.delta.files.iter()) + } + + pub fn get_filename(&self, file_id: usize) -> String { + for file in self.files().enumerate() { + if file.0 == file_id { + return file.1 .0.clone(); + } + } + + "".into() + } + + pub fn get_file_source(&self, file_id: usize) -> String { + for file in self.files().enumerate() { + if file.0 == file_id { + let output = String::from_utf8_lossy(self.get_span_contents(Span { + start: file.1 .1, + end: file.1 .2, + })) + .to_string(); + + return output; + } + } + + "".into() + } + pub fn add_file(&mut self, filename: String, contents: &[u8]) -> usize { let next_span_start = self.next_span_start(); diff --git a/src/tests.rs b/src/tests.rs index 7e41d5dba..43d1872dc 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -43,11 +43,13 @@ fn fail_test(input: &str, expected: &str) -> TestResult { let output = cmd.output()?; - let output = String::from_utf8_lossy(&output.stderr).to_string(); - println!("{}", output); + let stderr = String::from_utf8_lossy(&output.stderr).to_string(); + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); - assert!(output.contains("Error:")); - assert!(output.contains(expected)); + println!("stdout: {}", stdout); + println!("stderr: {}", stderr); + + assert!(stderr.contains(expected)); Ok(()) } @@ -64,7 +66,7 @@ fn add_simple2() -> TestResult { #[test] fn broken_math() -> TestResult { - fail_test("3 + ", "Incomplete") + fail_test("3 + ", "incomplete") } #[test] @@ -81,7 +83,7 @@ fn if_test2() -> TestResult { fn no_scope_leak1() -> TestResult { fail_test( "if $false { let $x = 10 } else { let $x = 20 }; $x", - "VariableNotFound", + "variable not found", ) } @@ -89,7 +91,7 @@ fn no_scope_leak1() -> TestResult { fn no_scope_leak2() -> TestResult { fail_test( "def foo [] { $x }; def bar [] { let $x = 10; foo }; bar", - "VariableNotFound", + "Variable not found", ) }