From d3837bcc23f45a2bc3aa59df18f22a0142170598 Mon Sep 17 00:00:00 2001 From: Daniel Martinez Date: Sat, 20 Feb 2021 12:57:33 +0200 Subject: [PATCH 1/3] fixes to the parser in framework and notests plugin --- plugins/csharp/src/plugin.rs | 4 +- plugins/java/src/ant_plugin.rs | 4 +- plugins/java/src/java_plugin.rs | 6 +- plugins/java/src/maven_plugin.rs | 4 +- plugins/make/src/plugin.rs | 4 +- plugins/notests/src/plugin.rs | 29 +++++++++- plugins/python3/src/plugin.rs | 4 +- plugins/r/src/plugin.rs | 23 +++++++- tmc-langs-framework/Cargo.toml | 2 +- tmc-langs-framework/src/error.rs | 4 +- tmc-langs-framework/src/plugin.rs | 96 ++++++++++++++++++++++++------- 11 files changed, 137 insertions(+), 43 deletions(-) diff --git a/plugins/csharp/src/plugin.rs b/plugins/csharp/src/plugin.rs index 1066fe3f8b8..c95de33ee8b 100644 --- a/plugins/csharp/src/plugin.rs +++ b/plugins/csharp/src/plugin.rs @@ -17,7 +17,7 @@ use tmc_langs_framework::{ }, error::{CommandError, FileIo}, file_util, - nom::{bytes, character, combinator, sequence, IResult}, + nom::{bytes, character, combinator, error::VerboseError, sequence, IResult}, plugin::Language, zip::ZipArchive, LanguagePlugin, TmcError, @@ -353,7 +353,7 @@ impl LanguagePlugin for CSharpPlugin { vec![PathBuf::from("test")] } - fn points_parser(i: &str) -> IResult<&str, &str> { + fn points_parser(i: &str) -> IResult<&str, &str, VerboseError<&str>> { combinator::map( sequence::delimited( sequence::tuple(( diff --git a/plugins/java/src/ant_plugin.rs b/plugins/java/src/ant_plugin.rs index 08b8340e594..c2cd5f7cb05 100644 --- a/plugins/java/src/ant_plugin.rs +++ b/plugins/java/src/ant_plugin.rs @@ -14,7 +14,7 @@ use tmc_langs_framework::{ command::TmcCommand, domain::{ExerciseDesc, RunResult, StyleValidationResult}, file_util, - nom::IResult, + nom::{error::VerboseError, IResult}, plugin::{Language, LanguagePlugin}, TmcError, }; @@ -126,7 +126,7 @@ impl LanguagePlugin for AntPlugin { Ok(()) } - fn points_parser(i: &str) -> IResult<&str, &str> { + fn points_parser(i: &str) -> IResult<&str, &str, VerboseError<&str>> { Self::java_points_parser(i) } diff --git a/plugins/java/src/java_plugin.rs b/plugins/java/src/java_plugin.rs index f0934eb5d66..1a24145acd3 100644 --- a/plugins/java/src/java_plugin.rs +++ b/plugins/java/src/java_plugin.rs @@ -11,7 +11,7 @@ use tmc_langs_framework::{ command::TmcCommand, domain::{ExerciseDesc, RunResult, RunStatus, StyleValidationResult, TestDesc, TestResult}, file_util, - nom::{bytes, character, combinator, sequence, IResult}, + nom::{bytes, character, combinator, error::VerboseError, sequence, IResult}, plugin::{Language, LanguagePlugin}, }; use walkdir::WalkDir; @@ -254,7 +254,7 @@ pub(crate) trait JavaPlugin: LanguagePlugin { } /// Parses @Points("1.1") point annotations. - fn java_points_parser(i: &str) -> IResult<&str, &str> { + fn java_points_parser(i: &str) -> IResult<&str, &str, VerboseError<&str>> { combinator::map( sequence::delimited( sequence::tuple(( @@ -363,7 +363,7 @@ mod test { unimplemented!() } - fn points_parser<'a>(i: &'a str) -> IResult<&'a str, &'a str> { + fn points_parser(i: &str) -> IResult<&str, &str, nom::error::VerboseError<&str>> { Self::java_points_parser(i) } } diff --git a/plugins/java/src/maven_plugin.rs b/plugins/java/src/maven_plugin.rs index b15de790683..b046159d4cf 100644 --- a/plugins/java/src/maven_plugin.rs +++ b/plugins/java/src/maven_plugin.rs @@ -16,7 +16,7 @@ use tmc_langs_framework::{ command::TmcCommand, domain::{ExerciseDesc, RunResult, StyleValidationResult}, file_util, - nom::IResult, + nom::{error::VerboseError, IResult}, plugin::{Language, LanguagePlugin}, TmcError, }; @@ -133,7 +133,7 @@ impl LanguagePlugin for MavenPlugin { vec![PathBuf::from("src/test")] } - fn points_parser(i: &str) -> IResult<&str, &str> { + fn points_parser(i: &str) -> IResult<&str, &str, VerboseError<&str>> { Self::java_points_parser(i) } } diff --git a/plugins/make/src/plugin.rs b/plugins/make/src/plugin.rs index de24f329a40..5982ecd82dc 100644 --- a/plugins/make/src/plugin.rs +++ b/plugins/make/src/plugin.rs @@ -16,7 +16,7 @@ use tmc_langs_framework::{ domain::{ExerciseDesc, RunResult, RunStatus, TestDesc}, error::{CommandError, FileIo}, file_util, - nom::{bytes, character, combinator, sequence, IResult}, + nom::{bytes, character, combinator, error::VerboseError, sequence, IResult}, plugin::LanguagePlugin, subprocess::PopenError, zip::ZipArchive, @@ -348,7 +348,7 @@ impl LanguagePlugin for MakePlugin { vec![PathBuf::from("test")] } - fn points_parser(i: &str) -> IResult<&str, &str> { + fn points_parser(i: &str) -> IResult<&str, &str, VerboseError<&str>> { combinator::map( sequence::delimited( sequence::tuple(( diff --git a/plugins/notests/src/plugin.rs b/plugins/notests/src/plugin.rs index 251e21558d2..a5657644e51 100644 --- a/plugins/notests/src/plugin.rs +++ b/plugins/notests/src/plugin.rs @@ -8,7 +8,7 @@ use std::time::Duration; use tmc_langs_framework::{ anyhow, domain::{ExerciseDesc, RunResult, RunStatus, TestDesc, TestResult}, - nom::IResult, + nom::{self, error::VerboseError, IResult}, zip::ZipArchive, LanguagePlugin, StudentFilePolicy, TmcError, }; @@ -102,8 +102,9 @@ impl LanguagePlugin for NoTestsPlugin { vec![PathBuf::from("test")] } - fn points_parser(_: &str) -> IResult<&str, &str> { - Ok(("", "")) + fn points_parser(i: &str) -> IResult<&str, &str, VerboseError<&str>> { + // does not match any characters + nom::combinator::value("", nom::character::complete::one_of(""))(i) } } @@ -228,4 +229,26 @@ no-tests: false ); assert!(!NoTestsPlugin::is_exercise_type_correct(temp_dir.path())); } + + #[test] + fn parses_empty() { + init(); + + let temp = tempfile::tempdir().unwrap(); + file_to(&temp, "test/.keep", r#""#); + + let points = NoTestsPlugin::get_available_points(temp.path()).unwrap(); + assert!(points.is_empty()); + + let temp = tempfile::tempdir().unwrap(); + file_to( + &temp, + "test/.keep", + r#" +"#, + ); + + let points = NoTestsPlugin::get_available_points(temp.path()).unwrap(); + assert!(points.is_empty()); + } } diff --git a/plugins/python3/src/plugin.rs b/plugins/python3/src/plugin.rs index b65c7e1c8ee..b52b4fac3eb 100644 --- a/plugins/python3/src/plugin.rs +++ b/plugins/python3/src/plugin.rs @@ -15,7 +15,7 @@ use tmc_langs_framework::{ domain::{ExerciseDesc, RunResult, RunStatus, TestDesc, TestResult}, error::CommandError, file_util, - nom::{branch, bytes, character, combinator, sequence, IResult}, + nom::{branch, bytes, character, combinator, error::VerboseError, sequence, IResult}, plugin::LanguagePlugin, TmcError, TmcProjectYml, }; @@ -334,7 +334,7 @@ impl LanguagePlugin for Python3Plugin { vec![PathBuf::from("test"), PathBuf::from("tmc")] } - fn points_parser(i: &str) -> IResult<&str, &str> { + fn points_parser(i: &str) -> IResult<&str, &str, VerboseError<&str>> { combinator::map( sequence::delimited( sequence::tuple(( diff --git a/plugins/r/src/plugin.rs b/plugins/r/src/plugin.rs index 24abbd9e64e..9a8906e737d 100644 --- a/plugins/r/src/plugin.rs +++ b/plugins/r/src/plugin.rs @@ -13,7 +13,7 @@ use tmc_langs_framework::{ command::TmcCommand, domain::{ExerciseDesc, RunResult, TestDesc}, file_util, - nom::{bytes, character, combinator, sequence, IResult}, + nom::{bytes, character, combinator, error::VerboseError, sequence, IResult}, zip::ZipArchive, LanguagePlugin, TmcError, }; @@ -151,7 +151,7 @@ impl LanguagePlugin for RPlugin { vec![PathBuf::from("tests")] } - fn points_parser(i: &str) -> IResult<&str, &str> { + fn points_parser(i: &str) -> IResult<&str, &str, VerboseError<&str>> { combinator::map( sequence::delimited( sequence::tuple(( @@ -475,4 +475,23 @@ test("sample", c("r1.1"), { "#; assert_eq!(RPlugin::points_parser(target).unwrap().1, "W1A.1.2"); } + + #[test] + fn parsing_regression_test() { + init(); + + let temp = tempfile::tempdir().unwrap(); + file_to( + &temp, + "tests/testthat/testExercise.R", + r#"library('testthat') +"#, + ); + + let res = RPlugin::get_available_points(temp.path()); + if let Err(TmcError::PointParse(_, e)) = res { + log::info!("{:#}", e); + } + panic!() + } } diff --git a/tmc-langs-framework/Cargo.toml b/tmc-langs-framework/Cargo.toml index ecb1ce07110..ad2e2194535 100644 --- a/tmc-langs-framework/Cargo.toml +++ b/tmc-langs-framework/Cargo.toml @@ -17,7 +17,7 @@ serde_yaml = "0.8" zip = "0.5" schemars = "0.8" once_cell = "1" -nom = "6" +nom = { version = "6", features = ["alloc"] } funty = "=1.1.0" # temporary workaround, remove later subprocess = "0.2" tempfile = "3" diff --git a/tmc-langs-framework/src/error.rs b/tmc-langs-framework/src/error.rs index 86fc97f10a3..e2b953166d1 100644 --- a/tmc-langs-framework/src/error.rs +++ b/tmc-langs-framework/src/error.rs @@ -27,8 +27,8 @@ pub enum TmcError { Canonicalize(PathBuf, #[source] std::io::Error), #[error("File {0} not in given project root {1}")] FileNotInProject(PathBuf, PathBuf), - #[error("Error while parsing available points: {0}")] - PointParse(String), + #[error("Error while parsing available points from {0}")] + PointParse(PathBuf, #[source] nom::error::VerboseError), #[error("Path {0} contained no file name")] NoFileName(PathBuf), diff --git a/tmc-langs-framework/src/plugin.rs b/tmc-langs-framework/src/plugin.rs index 7e0eb12c9f4..63a6ac1a780 100644 --- a/tmc-langs-framework/src/plugin.rs +++ b/tmc-langs-framework/src/plugin.rs @@ -10,7 +10,7 @@ use crate::error::TmcError; use crate::file_util; use crate::policy::StudentFilePolicy; use crate::TmcProjectYml; -use nom::{branch, bytes, combinator, multi, sequence, IResult}; +use nom::{branch, bytes, character, combinator, error::VerboseError, multi, sequence, IResult}; use std::collections::HashSet; use std::io::{Read, Seek, Write}; use std::path::{Path, PathBuf}; @@ -436,43 +436,73 @@ pub trait LanguagePlugin { log::trace!("parsing points from {}", entry.path().display()); let file_contents = file_util::read_file_to_string(entry.path())?; - let etc_parser = combinator::value(Parse::Other, bytes::complete::take(1usize)); + // reads any character + let etc_parser = combinator::value(Parse::Other, character::complete::anychar); + + // reads a single line comment let line_comment_parser = combinator::value( Parse::LineComment, - sequence::pair( + sequence::delimited( bytes::complete::tag(Self::LINE_COMMENT), - bytes::complete::is_not("\n"), + bytes::complete::take_until("\n"), + character::complete::newline, ), ); + + // reads a single block comment let block_comment_parser: Box _> = - if let Some(block_comment) = Self::BLOCK_COMMENT { + if let Some((block_start, block_end)) = Self::BLOCK_COMMENT { Box::new(combinator::value( Parse::BlockComment, - sequence::pair( - bytes::complete::tag(block_comment.0), - bytes::complete::is_not(block_comment.1), + sequence::delimited( + bytes::complete::tag(block_start), + bytes::complete::take_until(block_end), + bytes::complete::tag(block_end), ), )) } else { Box::new(combinator::value( - Parse::Other, - bytes::complete::take_while(|_| false), + Parse::BlockComment, + character::complete::one_of(""), )) }; + + // reads a points annotation let points_parser = combinator::map(Self::points_parser, |p| Parse::Points(p.to_string())); - let mut parser = multi::many0(multi::many_till( + // try to apply the interesting parsers, else read a character with the etc parser. repeat until the input ends + let mut parser = multi::many0(branch::alt(( + line_comment_parser, + block_comment_parser, + points_parser, etc_parser, - branch::alt((line_comment_parser, block_comment_parser, points_parser)), - )); - let res: IResult<_, _> = parser(&file_contents); - let (_, parsed) = res.map_err(|e| TmcError::PointParse(e.to_string()))?; - for (_, parse) in parsed { - if let Parse::Points(parsed) = parse { - // a single points annotation can contain multiple whitespace separated points - let split_points = parsed.split_whitespace().map(str::to_string); - points.extend(split_points); + ))); + + let res: IResult<_, _, _> = parser(&file_contents); + match res { + Ok((_, parsed)) => { + for parse in parsed { + if let Parse::Points(parsed) = parse { + // a single points annotation can contain multiple whitespace separated points + let split_points = + parsed.split_whitespace().map(str::to_string); + points.extend(split_points); + } + } + } + Err(nom::Err::Incomplete(_)) => unreachable!("this should never happen"), + Err(nom::Err::Error(e)) | Err(nom::Err::Failure(e)) => { + return Err(TmcError::PointParse( + entry.path().to_path_buf(), + VerboseError { + errors: e + .errors + .into_iter() + .map(|(s, k)| (s.to_string(), k)) + .collect(), + }, + )); } } } @@ -484,7 +514,7 @@ pub trait LanguagePlugin { /// A nom parser that recognizes a points annotation and returns the inner points value. /// /// For example implementations, see the existing language plugins. - fn points_parser(i: &str) -> IResult<&str, &str>; + fn points_parser(i: &str) -> IResult<&str, &str, nom::error::VerboseError<&str>>; } #[derive(Debug, Clone)] @@ -616,7 +646,7 @@ mod test { Ok(()) } - fn points_parser(i: &str) -> IResult<&str, &str> { + fn points_parser(i: &str) -> IResult<&str, &str, nom::error::VerboseError<&str>> { combinator::map( sequence::delimited( sequence::tuple(( @@ -1077,4 +1107,26 @@ force_update: let points = MockPlugin::get_available_points(temp.path()).unwrap(); assert_eq!(points, &["1", "2", "3", "4", "5", "6", "7", "8"]); } + + #[test] + fn parses_empty() { + init(); + + let temp = tempfile::tempdir().unwrap(); + file_to(&temp, "test/file", r#""#); + + let points = MockPlugin::get_available_points(temp.path()).unwrap(); + assert!(points.is_empty()); + + let temp = tempfile::tempdir().unwrap(); + file_to( + &temp, + "test/file", + r#" +"#, + ); + + let points = MockPlugin::get_available_points(temp.path()).unwrap(); + assert!(points.is_empty()); + } } From ce564ab73cf881b7e675d88df98d7217940d45df Mon Sep 17 00:00:00 2001 From: Daniel Martinez Date: Sat, 20 Feb 2021 13:41:53 +0200 Subject: [PATCH 2/3] added points_for_all_tests parsing to R plugin --- plugins/r/src/plugin.rs | 80 +++++++++++++++++++++++++++++------------ 1 file changed, 57 insertions(+), 23 deletions(-) diff --git a/plugins/r/src/plugin.rs b/plugins/r/src/plugin.rs index 9a8906e737d..3a419b685ac 100644 --- a/plugins/r/src/plugin.rs +++ b/plugins/r/src/plugin.rs @@ -13,7 +13,7 @@ use tmc_langs_framework::{ command::TmcCommand, domain::{ExerciseDesc, RunResult, TestDesc}, file_util, - nom::{bytes, character, combinator, error::VerboseError, sequence, IResult}, + nom::{branch, bytes, character, combinator, error::VerboseError, sequence, IResult}, zip::ZipArchive, LanguagePlugin, TmcError, }; @@ -152,25 +152,43 @@ impl LanguagePlugin for RPlugin { } fn points_parser(i: &str) -> IResult<&str, &str, VerboseError<&str>> { - combinator::map( + let test_parser = sequence::delimited( + sequence::tuple(( + bytes::complete::tag("test"), + character::complete::multispace0, + character::complete::char('('), + bytes::complete::take_until(","), + bytes::complete::take_until("\""), + )), + sequence::delimited( + character::complete::char('"'), + bytes::complete::is_not("\""), + character::complete::char('"'), + ), + sequence::tuple(( + character::complete::multispace0, + character::complete::char(')'), + )), + ); + let points_for_all_tests_parser = sequence::delimited( + sequence::tuple(( + bytes::complete::tag("points_for_all_tests"), + character::complete::multispace0, + character::complete::char('('), + bytes::complete::take_until("\""), + )), sequence::delimited( - sequence::tuple(( - bytes::complete::tag("test"), - character::complete::multispace0, - character::complete::char('('), - bytes::complete::take_until(","), - bytes::complete::take_until("\""), - )), - sequence::delimited( - character::complete::char('"'), - bytes::complete::is_not("\""), - character::complete::char('"'), - ), - sequence::tuple(( - character::complete::multispace0, - character::complete::char(')'), - )), + character::complete::char('"'), + bytes::complete::is_not("\""), + character::complete::char('"'), ), + sequence::tuple(( + character::complete::multispace0, + character::complete::char(')'), + )), + ); + combinator::map( + branch::alt((test_parser, points_for_all_tests_parser)), str::trim, )(i) } @@ -481,6 +499,7 @@ test("sample", c("r1.1"), { init(); let temp = tempfile::tempdir().unwrap(); + // a file like this used to cause an error before for some reason... file_to( &temp, "tests/testthat/testExercise.R", @@ -488,10 +507,25 @@ test("sample", c("r1.1"), { "#, ); - let res = RPlugin::get_available_points(temp.path()); - if let Err(TmcError::PointParse(_, e)) = res { - log::info!("{:#}", e); - } - panic!() + let _points = RPlugin::get_available_points(temp.path()).unwrap(); + } + + #[test] + fn parses_points_for_all_tests() { + init(); + + let temp = tempfile::tempdir().unwrap(); + file_to( + &temp, + "tests/testthat/testExercise.R", + r#" +something +points_for_all_tests(c("r1")) +etc +"#, + ); + + let points = RPlugin::get_available_points(temp.path()).unwrap(); + assert_eq!(points, &["r1"]); } } From 28177827f81c4fa40f647167aea8e025b01a4d37 Mon Sep 17 00:00:00 2001 From: Daniel Martinez Date: Sat, 20 Feb 2021 13:48:18 +0200 Subject: [PATCH 3/3] test compile fix --- plugins/java/src/java_plugin.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/java/src/java_plugin.rs b/plugins/java/src/java_plugin.rs index 1a24145acd3..3ae609346d2 100644 --- a/plugins/java/src/java_plugin.rs +++ b/plugins/java/src/java_plugin.rs @@ -285,7 +285,7 @@ mod test { use crate::SEPARATOR; use super::*; - use tmc_langs_framework::{anyhow, TmcError}; + use tmc_langs_framework::{anyhow, nom, TmcError}; fn init() { use log::*;