Merge pull request #217 from rage/policies

Policies

Author: Heliozoa

.gitignore

@@ -1,3 +1,4 @@
 /target
 **/*.rs.bk
 .vscode
+/test-cache

Cargo.lock

@@ -547,7 +547,7 @@ fn login(mut cx: FunctionContext) -> JsResult<JsValue> {
         password
     };
     let token = with_client(&client_name, client_version, |client| {
-        tmc_langs::login_with_password(client, &client_name, email, decoded)
+        tmc_langs::login_with_password(client, email, decoded)
     })
     .map_err(|e| convert_err(&mut cx, e))?;
 

crates/bindings/tmc-langs-node/src/lib.rs

@@ -1,27 +1,12 @@
 //! Student file policy for the C# plugin.
 
-use std::path::Path;
+use std::{ffi::OsStr, path::Path};
 use tmc_langs_framework::{StudentFilePolicy, TmcProjectYml};
 
 pub struct CSharpStudentFilePolicy {
     project_config: TmcProjectYml,
 }
 
-impl CSharpStudentFilePolicy {
-    /// Goes up directories until a bin or obj directory is found, either indicating that the path is in a binary directory.
-    fn is_child_of_binary_dir(path: &Path) -> bool {
-        // checks each parent directory for bin or obj
-        for ancestor in path.ancestors().skip(1) {
-            if let Some(file_name) = ancestor.file_name() {
-                if file_name == "bin" || file_name == "obj" {
-                    return true;
-                }
-            }
-        }
-        false
-    }
-}
-
 impl StudentFilePolicy for CSharpStudentFilePolicy {
     fn new_with_project_config(project_config: TmcProjectYml) -> Self
     where
@@ -34,14 +19,9 @@ impl StudentFilePolicy for CSharpStudentFilePolicy {
         &self.project_config
     }
 
-    // false for .csproj files and files in bin or obj directories
-    // true for files in src except for .csproj files
+    // .cs files in src
     fn is_non_extra_student_file(&self, path: &Path) -> bool {
-        path.starts_with("src")
-            // exclude files in bin
-            && !Self::is_child_of_binary_dir(path)
-            // exclude .csproj files
-            && !path.extension().map(|ext| ext == "csproj").unwrap_or_default()
+        path.starts_with("src") && path.extension() == Some(OsStr::new("cs"))
     }
 }
 
@@ -53,14 +33,14 @@ mod test {
     fn file_in_binary_dir_is_not_student_file() {
         let policy = CSharpStudentFilePolicy::new(Path::new(".")).unwrap();
         assert!(!policy.is_student_file(Path::new("src/bin/any/file")));
-        assert!(!policy.is_student_file(Path::new("obj/any/src/file")));
+        assert!(!policy.is_student_file(Path::new("obj/any/src/file.cs")));
     }
 
     #[test]
-    fn file_in_src_is_student_file() {
+    fn cs_file_in_src_is_student_file() {
         let policy = CSharpStudentFilePolicy::new(Path::new(".")).unwrap();
-        assert!(policy.is_student_file(Path::new("src/file")));
-        assert!(policy.is_student_file(Path::new("src/any/file")));
+        assert!(policy.is_student_file(Path::new("src/file.cs")));
+        assert!(policy.is_student_file(Path::new("src/any/file.cs")));
     }
 
     #[test]

crates/plugins/csharp/src/policy.rs

@@ -1,6 +1,6 @@
 //! Ant student file policy
 
-use std::path::Path;
+use std::{ffi::OsStr, path::Path};
 use tmc_langs_framework::{StudentFilePolicy, TmcProjectYml};
 
 pub struct AntStudentFilePolicy {
@@ -20,7 +20,7 @@ impl StudentFilePolicy for AntStudentFilePolicy {
     }
 
     fn is_non_extra_student_file(&self, path: &Path) -> bool {
-        path.starts_with("src")
+        path.starts_with("src") && path.extension() == Some(OsStr::new("java"))
     }
 }
 
@@ -31,13 +31,14 @@ mod test {
     #[test]
     fn is_student_file() {
         let policy = AntStudentFilePolicy::new(Path::new(".")).unwrap();
-        assert!(policy.is_student_file(Path::new("src/file")));
-        assert!(policy.is_student_file(Path::new("src/dir/file")));
+        assert!(policy.is_student_file(Path::new("src/file.java")));
+        assert!(policy.is_student_file(Path::new("src/dir/file.java")));
     }
 
     #[test]
     fn is_not_student_source_file() {
         let policy = AntStudentFilePolicy::new(Path::new(".")).unwrap();
+        assert!(!policy.is_student_file(Path::new("src/file")));
         assert!(!policy.is_student_file(Path::new("file")));
         assert!(!policy.is_student_file(Path::new("dir/src/file")));
         assert!(!policy.is_student_file(Path::new("srca/file")));

crates/plugins/java/src/ant_policy.rs

@@ -6,7 +6,7 @@ use crate::{
 };
 use flate2::read::GzDecoder;
 use std::{
-    ffi::OsString,
+    ffi::{OsStr, OsString},
     io::{Cursor, Read, Seek},
     ops::ControlFlow::{Break, Continue},
     path::{Path, PathBuf},
@@ -17,7 +17,7 @@ use tmc_langs_framework::{
     Archive, ExerciseDesc, Language, LanguagePlugin, RunResult, StyleValidationResult, TmcCommand,
     TmcError, nom::IResult, nom_language::error::VerboseError,
 };
-use tmc_langs_util::{file_util, path_util};
+use tmc_langs_util::file_util;
 
 const MVN_ARCHIVE: &[u8] = include_bytes!("../deps/apache-maven-3.8.1-bin.tar.gz");
 const MVN_PATH_IN_ARCHIVE: &str = "apache-maven-3.8.1"; // the name of the base directory in the maven archive
@@ -132,14 +132,14 @@ impl LanguagePlugin for MavenPlugin {
         archive: &mut Archive<R>,
     ) -> Result<PathBuf, TmcError> {
         let mut iter = archive.iter()?;
+
         let project_dir = loop {
             let next = iter.with_next(|file| {
                 let file_path = file.path()?;
 
-                if file.is_file() {
-                    // check for pom.xml
-                    if let Some(parent) = path_util::get_parent_of_named(&file_path, "pom.xml") {
-                        return Ok(Break(Some(parent)));
+                if file.is_file() && file_path.file_name() == Some(OsStr::new("pom.xml")) {
+                    if let Some(pom_parent) = file_path.parent() {
+                        return Ok(Break(Some(pom_parent.to_path_buf())));
                     }
                 }
                 Ok(Continue(()))

crates/plugins/java/src/maven_plugin.rs

@@ -1,6 +1,6 @@
 //! Maven student file policy
 
-use std::path::Path;
+use std::{ffi::OsStr, path::Path};
 use tmc_langs_framework::{StudentFilePolicy, TmcProjectYml};
 
 pub struct MavenStudentFilePolicy {
@@ -20,7 +20,8 @@ impl StudentFilePolicy for MavenStudentFilePolicy {
     }
 
     fn is_non_extra_student_file(&self, path: &Path) -> bool {
-        path.starts_with("src/main")
+        // technically pom.xml would need to be included to differentiate between maven and ant projects
+        path.starts_with("src/main") && path.extension() == Some(OsStr::new("java"))
     }
 }
 
@@ -31,8 +32,8 @@ mod test {
     #[test]
     fn is_student_file() {
         let policy = MavenStudentFilePolicy::new(Path::new(".")).unwrap();
-        assert!(policy.is_student_file(Path::new("src/main/file")));
-        assert!(policy.is_student_file(Path::new("src/main/dir/file")));
+        assert!(policy.is_student_file(Path::new("src/main/file.java")));
+        assert!(policy.is_student_file(Path::new("src/main/dir/file.java")));
     }
 
     #[test]

crates/plugins/java/src/maven_policy.rs

@@ -1,6 +1,6 @@
 //! Contains the language policy for the plugin.
 
-use std::path::Path;
+use std::{ffi::OsStr, path::Path};
 use tmc_langs_framework::{StudentFilePolicy, TmcProjectYml};
 
 pub struct MakeStudentFilePolicy {
@@ -20,7 +20,8 @@ impl StudentFilePolicy for MakeStudentFilePolicy {
     }
 
     fn is_non_extra_student_file(&self, path: &Path) -> bool {
-        path.starts_with("src")
+        let ext = path.extension();
+        path.starts_with("src") && (ext == Some(OsStr::new("c")) || ext == Some(OsStr::new("h")))
     }
 }
 
@@ -31,14 +32,17 @@ mod test {
     #[test]
     fn is_student_file() {
         let policy = MakeStudentFilePolicy::new(Path::new(".")).unwrap();
-        assert!(policy.is_student_file(Path::new("src")));
-        assert!(policy.is_student_file(Path::new("src/file")));
-        assert!(policy.is_student_file(Path::new("src/dir/file")));
+        assert!(policy.is_student_file(Path::new("src/file.c")));
+        assert!(policy.is_student_file(Path::new("src/file.h")));
+        assert!(policy.is_student_file(Path::new("src/dir/file.c")));
+        assert!(policy.is_student_file(Path::new("src/dir/file.h")));
     }
 
     #[test]
     fn is_not_student_source_file() {
         let policy = MakeStudentFilePolicy::new(Path::new(".")).unwrap();
+        assert!(!policy.is_student_file(Path::new("a.c")));
+        assert!(!policy.is_student_file(Path::new("a.h")));
         assert!(!policy.is_student_file(Path::new("srcc")));
         assert!(!policy.is_student_file(Path::new("dir/src/file")));
     }

crates/plugins/make/src/policy.rs

@@ -1,5 +1,6 @@
 #include <stdio.h>
 #include "source.h"
+#include <stdlib.h>
 
 int one(void)
 {

crates/plugins/make/tests/data/valgrind-failing-exercise/src/source.c

@@ -20,33 +20,20 @@ impl StudentFilePolicy for Python3StudentFilePolicy {
     }
 
     fn is_non_extra_student_file(&self, path: &Path) -> bool {
-        // no files in tmc, test and venv subdirectories are considered student files
-        let is_cache_file = path.extension() == Some(OsStr::new("pyc"))
-            || path
-                .components()
-                .any(|c| c.as_os_str() == OsStr::new("__pycache__"));
-        let is_in_exercise_subdir = path.starts_with("test") || path.starts_with("tmc");
-        let is_in_venv = path.starts_with(".venv") || path.starts_with("venv");
-        if is_cache_file || is_in_exercise_subdir || is_in_venv {
+        // never include pyc files
+        let is_pyc = path.extension() == Some(OsStr::new("pyc"));
+        if is_pyc {
             return false;
         }
 
-        // all non-pyc or __pycache__ files in src are student source files
         let in_src = path.starts_with("src");
-        // .py files in exercise root are student source files
-        let is_in_project_root = match path.parent() {
+        let in_exercise_root = match path.parent() {
             Some(s) => s.as_os_str().is_empty(),
             None => true,
         };
-        let is_py_file = path.extension() == Some(OsStr::new("py"));
+        let is_py = path.extension() == Some(OsStr::new("py"));
         let is_ipynb = path.extension() == Some(OsStr::new("ipynb"));
-
-        // all in all, excluding cache files and the exception subdirs,
-        // we take non-cache files in src, py files in root, everything not in the root and not in src, and all ipynb files
-        in_src
-            || is_in_project_root && is_py_file
-            || !is_in_exercise_subdir && !is_in_project_root
-            || is_ipynb
+        (in_src || in_exercise_root) && (is_py || is_ipynb)
     }
 }
 
@@ -83,10 +70,10 @@ mod test {
     }
 
     #[test]
-    fn subdirs_are_student_files() {
+    fn subdirs_are_not_student_files() {
         let policy = Python3StudentFilePolicy::new(Path::new(".")).unwrap();
-        assert!(policy.is_student_file(Path::new("subdir/something")));
-        assert!(policy.is_student_file(Path::new("another/mid/else")));
+        assert!(!policy.is_student_file(Path::new("subdir/something")));
+        assert!(!policy.is_student_file(Path::new("another/mid/else")));
     }
 
     #[test]