From bdbb1b33fdac8491bc9a517be3807d97e17d69c9 Mon Sep 17 00:00:00 2001 From: Akshay Date: Sat, 12 Oct 2024 19:03:22 +0530 Subject: trace errors in tests, add warnings to tbsp --- src/ast.rs | 4 +-- src/eval/analysis.rs | 76 ++++++++++++++++++++++++++++++++++++++++++++-------- src/eval/builtins.rs | 4 +-- src/eval/mod.rs | 34 ++++++++++++++++------- src/eval/test.rs | 61 +++++++++++++++++++++++++++-------------- 5 files changed, 133 insertions(+), 46 deletions(-) (limited to 'src') diff --git a/src/ast.rs b/src/ast.rs index 7fc71a6..c8445ab 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -63,7 +63,7 @@ impl std::fmt::Display for Pattern { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::Begin => write!(f, "BEGIN"), - Self::End=> write!(f, "END"), + Self::End => write!(f, "END"), Self::Tree { modifier, matcher } => write!(f, "{modifier} {matcher}"), } } @@ -383,7 +383,7 @@ pub enum Type { } impl std::fmt::Display for Type { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::Unit => write!(f, "unit"), Self::Integer => write!(f, "int"), diff --git a/src/eval/analysis.rs b/src/eval/analysis.rs index b3a0083..d5a7165 100644 --- a/src/eval/analysis.rs +++ b/src/eval/analysis.rs @@ -6,10 +6,17 @@ pub struct Analysis { } impl Analysis { - pub fn print(&self) { + pub fn contains_error(&self) -> bool { + self.diagnostics.iter().any(|d| d.level == Level::Error) + } +} + +impl std::fmt::Display for Analysis { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { for Diagnostic { level, kind } in &self.diagnostics { - eprintln!("{level} {kind}"); + writeln!(f, "{level} {kind}")?; } + Ok(()) } } @@ -36,16 +43,20 @@ impl std::fmt::Display for Level { #[derive(Debug, PartialEq, Eq)] pub enum Kind { - Pattern { + InvalidPattern { invalid_node_kind: String, full_pattern: ast::Pattern, }, + SimplifiablePattern { + pattern: ast::TreePattern, + simplified: ast::TreePattern, + }, } impl std::fmt::Display for Kind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Pattern { + Self::InvalidPattern { invalid_node_kind, full_pattern, } => { @@ -54,12 +65,21 @@ impl std::fmt::Display for Kind { "invalid node kind `{invalid_node_kind}` in pattern `{full_pattern}`" ) } + Self::SimplifiablePattern { + pattern, + simplified, + } => { + write!( + f, + "the pattern `{pattern}` can be reduced to just `{simplified}`" + ) + } } } } pub fn run(ctx: &super::Context) -> Analysis { - [validate_patterns(ctx)] + [validate_patterns(ctx), redundant_list_pattern(ctx)] .into_iter() .flatten() .collect::>() @@ -67,11 +87,9 @@ pub fn run(ctx: &super::Context) -> Analysis { } fn validate_patterns(ctx: &super::Context) -> Vec { - fn validate_pattern( - pattern: &ast::TreePattern, - language: &tree_sitter::Language, - ) -> Option { + fn check(pattern: &ast::TreePattern, language: &tree_sitter::Language) -> Option { match pattern { + // base case ast::TreePattern::Atom(a) => { if language.id_for_node_kind(a, true) == 0 { Some(a.to_owned()) @@ -79,9 +97,10 @@ fn validate_patterns(ctx: &super::Context) -> Vec { None } } + // recursive case ast::TreePattern::List(items) => { for item in items { - validate_pattern(item, language)?; + check(item, language)?; } None } @@ -94,7 +113,7 @@ fn validate_patterns(ctx: &super::Context) -> Vec { .flat_map(|s| match &s.pattern { ast::Pattern::Begin | ast::Pattern::End => None, ast::Pattern::Tree { matcher, .. } => { - validate_pattern(matcher, &ctx.language).map(|invalid_node_kind| Kind::Pattern { + check(matcher, &ctx.language).map(|invalid_node_kind| Kind::InvalidPattern { invalid_node_kind, full_pattern: s.pattern.clone(), }) @@ -106,3 +125,38 @@ fn validate_patterns(ctx: &super::Context) -> Vec { }) .collect() } + +fn redundant_list_pattern(ctx: &super::Context) -> Vec { + fn simplify(pattern: &ast::TreePattern) -> ast::TreePattern { + match pattern { + ast::TreePattern::Atom(a) => ast::TreePattern::Atom(a.to_owned()), + ast::TreePattern::List(l) => match l.as_slice() { + [a] => simplify(a), + items => ast::TreePattern::List(items.iter().map(simplify).collect::>()), + }, + } + } + + ctx.program + .stanzas + .iter() + .flat_map(|s| match &s.pattern { + ast::Pattern::Begin | ast::Pattern::End => None, + ast::Pattern::Tree { matcher, .. } => { + let simplified = simplify(matcher); + if &simplified != matcher { + Some(Kind::SimplifiablePattern { + pattern: matcher.clone(), + simplified, + }) + } else { + None + } + } + }) + .map(|kind| Diagnostic { + level: Level::Warning, + kind, + }) + .collect() +} diff --git a/src/eval/builtins.rs b/src/eval/builtins.rs index c91f7ba..e1fa10b 100644 --- a/src/eval/builtins.rs +++ b/src/eval/builtins.rs @@ -50,9 +50,7 @@ builtins! { fn print(ctx: &mut Context, args: &[ast::Expr]) -> Result { for arg in args { let val = ctx.eval_expr(arg)?; - let mut default_stream = Box::new(std::io::stdout()) as Box; - let stream = ctx.output_stream.as_mut().unwrap_or(&mut default_stream); - write!(stream, "{val}").unwrap(); + write!(ctx.output_stream, "{val}").unwrap(); } Ok(Value::Unit) } diff --git a/src/eval/mod.rs b/src/eval/mod.rs index 589be37..9e0d033 100644 --- a/src/eval/mod.rs +++ b/src/eval/mod.rs @@ -3,8 +3,8 @@ use crate::{ast, Wrap}; use std::{collections::HashMap, fmt, io}; -mod builtins; mod analysis; +mod builtins; #[cfg(test)] mod test; @@ -453,7 +453,8 @@ pub struct Context<'s> { cursor: Option>, tree: Option<&'static tree_sitter::Tree>, cache: HashMap>, - output_stream: Option>, + output_stream: Box, + error_stream: Box, } impl<'s> fmt::Debug for Context<'s> { @@ -484,7 +485,8 @@ impl<'s> Context<'s> { cursor: None, tree: None, cache: HashMap::default(), - output_stream: Some(Box::new(io::stdout()) as Box), + output_stream: Box::new(io::stdout()) as Box, + error_stream: Box::new(io::stderr()) as Box, } } @@ -539,7 +541,12 @@ impl<'s> Context<'s> { } pub fn with_output_stream(mut self, stream: Box) -> Self { - self.output_stream = Some(stream); + self.output_stream = stream; + self + } + + pub fn with_error_stream(mut self, stream: Box) -> Self { + self.error_stream = stream; self } @@ -793,13 +800,11 @@ impl<'s> Context<'s> { .unwrap_or_default() } - pub fn analyze(&mut self) -> Result { - let analysis = analysis::run(&*self); - analysis.print(); - Err(Error::Analysis) + pub fn analyze(&mut self) -> analysis::Analysis { + analysis::run(&*self) } - pub fn eval(&mut self) -> Result { + fn eval_program(&mut self) -> Result { let program = std::mem::take(&mut self.program); let mut has_next = true; let mut postorder = Vec::new(); @@ -847,6 +852,15 @@ impl<'s> Context<'s> { Ok(Value::Unit) } + + pub fn eval(&mut self) -> Result { + let analysis = self.analyze(); + write!(self.error_stream, "{analysis}").unwrap(); + if analysis.contains_error() { + return Err(Error::Analysis); + } + self.eval_program() + } } pub fn evaluate(file: &str, program: &str, language: tree_sitter::Language) -> Result { @@ -860,8 +874,8 @@ pub fn evaluate(file: &str, program: &str, language: tree_sitter::Language) -> R .with_input(file.to_owned()) .with_tree(tree) .with_output_stream(Box::new(io::stdout())) + .with_error_stream(Box::new(io::stderr())) .with_program(program); - ctx.analyze()?; ctx.eval() } diff --git a/src/eval/test.rs b/src/eval/test.rs index 83749e0..c9e7630 100644 --- a/src/eval/test.rs +++ b/src/eval/test.rs @@ -1,32 +1,27 @@ use super::*; use crate::ast::*; -use std::io::Write; use expect_test::{expect, Expect}; -fn gen_test(file: &str, program: &str, expected: Expect) { +fn gen_test(file: &str, program: &str, stdout: Expect, stderr: Expect) { let language = tree_sitter_python::language(); let mut parser = tree_sitter::Parser::new(); let _ = parser.set_language(&language); let tree = parser.parse(file, None).unwrap(); let program = ast::Program::new().with_file(program).unwrap(); - let mut output = Vec::new(); - let result; + let mut out = Vec::new(); + let mut err = Vec::new(); - { - let mut ctx = Context::new(language) - .with_input(file.to_owned()) - .with_tree(tree) - .with_program(program) - .with_output_stream(Box::new(&mut output) as Box); + Context::new(language) + .with_input(file.to_owned()) + .with_tree(tree) + .with_program(program) + .with_output_stream(Box::new(&mut out) as Box) + .with_error_stream(Box::new(&mut err) as Box) + .eval(); - result = ctx.eval(); - } - - if let Err(e) = result { - writeln!(output, "{e:?}").unwrap(); - } - expected.assert_eq(&String::from_utf8(output).unwrap()) + stdout.assert_eq(&String::from_utf8(out).unwrap()); + stderr.assert_eq(&String::from_utf8(err).unwrap()); } #[test] @@ -201,6 +196,7 @@ fn list_1() { } ", expect!["[5]"], + expect![], ); } @@ -214,6 +210,7 @@ fn list_2() { } ", expect!["3"], + expect![], ); } @@ -229,6 +226,7 @@ fn list_3() { } "#, expect!["true, false"], + expect![], ); } @@ -256,6 +254,7 @@ fn list_4() { [5, 4] [5] "#]], + expect![], ); } @@ -274,6 +273,7 @@ fn list_5() { false true "#]], + expect![], ); } @@ -291,6 +291,7 @@ fn string_1() { FOO foo "#]], + expect![], ); } @@ -320,6 +321,7 @@ fn string_2() { FOO is upper? true FOO is lower? false "#]], + expect![], ); } @@ -337,6 +339,7 @@ fn string_3() { a[3:5]: ` b` a[2:9]: `o bar b` "#]], + expect![], ); } @@ -349,9 +352,8 @@ fn string_4() { println("a[9:20]: `", substr(a, 9, 20), "`"); } "#, - expect![[r#" - a[9:20]: `InvalidStringSlice { length: 11, start: 9, end: 20 } - "#]], + expect!["a[9:20]: `"], + expect![], ); } @@ -367,5 +369,24 @@ fn node_1() { def foo(a, b): hello() foo "#]], + expect![], + ); +} + +#[test] +fn analysis_1() { + gen_test( + "def foo(a, b): hello()", + r#" + enter (function) { } + enter ((function (name))) { } + enter ((function (name args))) { } + "#, + expect![], + expect![[r#" + [warning] the pattern `(function)` can be reduced to just `function` + [warning] the pattern `((function (name)))` can be reduced to just `(function name)` + [warning] the pattern `((function (name args)))` can be reduced to just `(function (name args))` + "#]], ); } -- cgit v1.2.3