fixes to the parser in framework and notests plugin

Heliozoa · Heliozoa · commit d3837bcc23f4 · 2021-02-20T12:57:33.000+02:00
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<String>),
 
     #[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
@@ -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<dyn FnMut(_) -> _> =
-                        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());
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ use tmc_langs_framework::{`
`11`	`11`	`command::TmcCommand,`
`12`	`12`	`domain::{ExerciseDesc, RunResult, RunStatus, StyleValidationResult, TestDesc, TestResult},`
`13`	`13`	`file_util,`
`14`		`- nom::{bytes, character, combinator, sequence, IResult},`
	`14`	`+ nom::{bytes, character, combinator, error::VerboseError, sequence, IResult},`
`15`	`15`	`plugin::{Language, LanguagePlugin},`
`16`	`16`	`};`
`17`	`17`	`use walkdir::WalkDir;`
`@@ -254,7 +254,7 @@ pub(crate) trait JavaPlugin: LanguagePlugin {`
`254`	`254`	`}`
`255`	`255`
`256`	`256`	`/// Parses @Points("1.1") point annotations.`
`257`		`- fn java_points_parser(i: &str) -> IResult<&str, &str> {`
	`257`	`+ fn java_points_parser(i: &str) -> IResult<&str, &str, VerboseError<&str>> {`
`258`	`258`	`combinator::map(`
`259`	`259`	`sequence::delimited(`
`260`	`260`	`sequence::tuple((`
`@@ -363,7 +363,7 @@ mod test {`
`363`	`363`	`unimplemented!()`
`364`	`364`	`}`
`365`	`365`
`366`		`- fn points_parser<'a>(i: &'a str) -> IResult<&'a str, &'a str> {`
	`366`	`+ fn points_parser(i: &str) -> IResult<&str, &str, nom::error::VerboseError<&str>> {`
`367`	`367`	`Self::java_points_parser(i)`
`368`	`368`	`}`
`369`	`369`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ use tmc_langs_framework::{`
`16`	`16`	`command::TmcCommand,`
`17`	`17`	`domain::{ExerciseDesc, RunResult, StyleValidationResult},`
`18`	`18`	`file_util,`
`19`		`- nom::IResult,`
	`19`	`+ nom::{error::VerboseError, IResult},`
`20`	`20`	`plugin::{Language, LanguagePlugin},`
`21`	`21`	`TmcError,`
`22`	`22`	`};`
`@@ -133,7 +133,7 @@ impl LanguagePlugin for MavenPlugin {`
`133`	`133`	`vec![PathBuf::from("src/test")]`
`134`	`134`	`}`
`135`	`135`
`136`		`- fn points_parser(i: &str) -> IResult<&str, &str> {`
	`136`	`+ fn points_parser(i: &str) -> IResult<&str, &str, VerboseError<&str>> {`
`137`	`137`	`Self::java_points_parser(i)`
`138`	`138`	`}`
`139`	`139`	`}`