Auto merge of #143 - cjpearce:fix-exercise-path-matching, r=komaeda

Canonicalize paths to fix path matching

This PR should fix #126. The main solution to the issue was using `canonicalize()` on the paths we create for the exercises from `info.toml` and any user-specified paths, so that path `ends_with` matching will work correctly.

As adding calls to the canonicalize function everywhere requires unwrapping, I also decided to extract a struct representing an exercise and use serde to deserialize the paths from the `info.toml` file up front. I also tried to move the path handling out into the `exercise.rs` file and down into `main.rs` so that it doesn't create as much clutter. There was already a lot of unwrapping and path handling in the other files and I felt like it was getting a bit too repetitive.

If the approach is going too far (too many changes etc.) I'm happy to try to produce a smaller PR that fixes the bug without any refactoring.

Cargo.toml

@@ -11,6 +11,7 @@ console = "0.6.2"
 syntect = "3.0.2"
 notify = "4.0.0"
 toml = "0.4.10"
+serde = {version = "1.0.10", features = ["derive"]}
 
 [[bin]]
 name = "rustlings"

src/exercise.rs

@@ -0,0 +1,79 @@
+use serde::Deserialize;
+use std::fmt::{self, Display, Formatter};
+use std::fs::{remove_file};
+use std::path::{PathBuf};
+use std::process::{self, Command, Output};
+
+const RUSTC_COLOR_ARGS: &[&str] = &["--color", "always"];
+
+fn temp_file() -> String {
+    format!("./temp_{}", process::id())
+}
+
+#[derive(Deserialize)]
+#[serde(rename_all = "lowercase")]
+pub enum Mode {
+    Compile,
+    Test,
+}
+
+#[derive(Deserialize)]
+pub struct ExerciseList {
+    pub exercises: Vec<Exercise>,
+}
+
+#[derive(Deserialize)]
+pub struct Exercise {
+    pub path: PathBuf,
+    pub mode: Mode,
+}
+
+impl Exercise {
+    pub fn compile(&self) -> Output {
+        match self.mode {
+            Mode::Compile => Command::new("rustc")
+                .args(&[self.path.to_str().unwrap(), "-o", &temp_file()])
+                .args(RUSTC_COLOR_ARGS)
+                .output(),
+            Mode::Test => Command::new("rustc")
+                .args(&["--test", self.path.to_str().unwrap(), "-o", &temp_file()])
+                .args(RUSTC_COLOR_ARGS)
+                .output(),
+        }
+        .expect("Failed to run 'compile' command.")
+    }
+
+    pub fn run(&self) -> Output {
+        Command::new(&temp_file())
+            .output()
+            .expect("Failed to run 'run' command")
+    }
+
+    pub fn clean(&self) {
+        let _ignored = remove_file(&temp_file());
+    }
+}
+
+impl Display for Exercise {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+        write!(f, "{}", self.path.to_str().unwrap())
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use std::path::Path;
+    use std::fs::File;
+
+    #[test]
+    fn test_clean() {
+        File::create(&temp_file()).unwrap();
+        let exercise = Exercise {
+            path: PathBuf::from("example.rs"),
+            mode: Mode::Test,
+        };
+        exercise.clean();
+        assert!(!Path::new(&temp_file()).exists());
+    }
+}

src/main.rs

@@ -1,9 +1,11 @@
+use crate::exercise::{Exercise, ExerciseList};
 use crate::run::run;
 use crate::verify::verify;
 use clap::{crate_version, App, Arg, SubCommand};
 use notify::DebouncedEvent;
 use notify::{RecommendedWatcher, RecursiveMode, Watcher};
 use std::ffi::OsStr;
+use std::fs;
 use std::io::BufRead;
 use std::path::Path;
 use std::sync::mpsc::channel;
@@ -13,8 +15,8 @@ use syntect::highlighting::{Style, ThemeSet};
 use syntect::parsing::SyntaxSet;
 use syntect::util::as_24_bit_terminal_escaped;
 
+mod exercise;
 mod run;
-mod util;
 mod verify;
 
 fn main() {
@@ -56,16 +58,36 @@ fn main() {
         std::process::exit(1);
     }
 
-    if let Some(matches) = matches.subcommand_matches("run") {
+    let toml_str = &fs::read_to_string("info.toml").unwrap();
-        run(matches.clone()).unwrap_or_else(|_| std::process::exit(1));
+    let exercises = toml::from_str::<ExerciseList>(toml_str).unwrap().exercises;
+
+    if let Some(ref matches) = matches.subcommand_matches("run") {
+        let filename = matches.value_of("file").unwrap_or_else(|| {
+            println!("Please supply a file name!");
+            std::process::exit(1);
+        });
+
+        let matching_exercise = |e: &&Exercise| {
+            Path::new(filename)
+                .canonicalize()
+                .map(|p| p.ends_with(&e.path))
+                .unwrap_or(false)
+        };
+
+        let exercise = exercises.iter().find(matching_exercise).unwrap_or_else(|| {
+            println!("No exercise found for your file name!");
+            std::process::exit(1)
+        });
+
+        run(&exercise).unwrap_or_else(|_| std::process::exit(1));
     }
 
     if matches.subcommand_matches("verify").is_some() {
-        verify(None).unwrap_or_else(|_| std::process::exit(1));
+        verify(&exercises).unwrap_or_else(|_| std::process::exit(1));
     }
 
     if matches.subcommand_matches("watch").is_some() {
-        watch().unwrap();
+        watch(&exercises).unwrap();
     }
 
     if matches.subcommand_name().is_none() {
@@ -81,13 +103,13 @@ fn main() {
     println!("\x1b[0m");
 }
 
-fn watch() -> notify::Result<()> {
+fn watch(exercises: &[Exercise]) -> notify::Result<()> {
     let (tx, rx) = channel();
 
     let mut watcher: RecommendedWatcher = Watcher::new(tx, Duration::from_secs(2))?;
     watcher.watch(Path::new("./exercises"), RecursiveMode::Recursive)?;
 
-    let _ignored = verify(None);
+    let _ignored = verify(exercises.iter());
 
     loop {
         match rx.recv() {
@@ -95,7 +117,11 @@ fn watch() -> notify::Result<()> {
                 DebouncedEvent::Create(b) | DebouncedEvent::Chmod(b) | DebouncedEvent::Write(b) => {
                     if b.extension() == Some(OsStr::new("rs")) {
                         println!("----------**********----------\n");
-                        let _ignored = verify(Some(b.as_path().to_str().unwrap()));
+                        let filepath = b.as_path().canonicalize().unwrap();
+                        let exercise = exercises
+                            .iter()
+                            .skip_while(|e| !filepath.ends_with(&e.path));
+                        let _ignored = verify(exercise);
                     }
                 }
                 _ => {}

src/run.rs

@@ -1,57 +1,40 @@
-use crate::util;
+use crate::exercise::{Mode, Exercise};
 use crate::verify::test;
 use console::{style, Emoji};
 use indicatif::ProgressBar;
-use std::fs;
-use toml::Value;
 
-pub fn run(matches: clap::ArgMatches) -> Result<(), ()> {
+pub fn run(exercise: &Exercise) -> Result<(), ()> {
-    if let Some(filename) = matches.value_of("file") {
+    match exercise.mode {
-        let toml: Value = fs::read_to_string("info.toml").unwrap().parse().unwrap();
+        Mode::Test => test(exercise)?,
-        let tomlvec: &Vec<Value> = toml.get("exercises").unwrap().as_array().unwrap();
+        Mode::Compile => compile_and_run(exercise)?,
-        let mut exercises = tomlvec.clone();
-        exercises.retain(|i| i.get("path").unwrap().as_str().unwrap() == filename);
-        if exercises.is_empty() {
-            println!("No exercise found for your filename!");
-            std::process::exit(1);
-        }
-
-        let exercise: &Value = &exercises[0];
-        match exercise.get("mode").unwrap().as_str().unwrap() {
-            "test" => test(exercise.get("path").unwrap().as_str().unwrap())?,
-            "compile" => compile_and_run(exercise.get("path").unwrap().as_str().unwrap())?,
-            _ => (),
     }
     Ok(())
-    } else {
-        panic!("Please supply a filename!");
-    }
 }
 
-pub fn compile_and_run(filename: &str) -> Result<(), ()> {
+pub fn compile_and_run(exercise: &Exercise) -> Result<(), ()> {
     let progress_bar = ProgressBar::new_spinner();
-    progress_bar.set_message(format!("Compiling {}...", filename).as_str());
+    progress_bar.set_message(format!("Compiling {}...", exercise).as_str());
     progress_bar.enable_steady_tick(100);
 
-    let compilecmd = util::compile_cmd(filename);
+    let compilecmd = exercise.compile();
-    progress_bar.set_message(format!("Running {}...", filename).as_str());
+    progress_bar.set_message(format!("Running {}...", exercise).as_str());
     if compilecmd.status.success() {
-        let runcmd = util::run_cmd();
+        let runcmd = exercise.run();
         progress_bar.finish_and_clear();
 
         if runcmd.status.success() {
             println!("{}", String::from_utf8_lossy(&runcmd.stdout));
-            let formatstr = format!("{} Successfully ran {}", Emoji("✅", "✓"), filename);
+            let formatstr = format!("{} Successfully ran {}", Emoji("✅", "✓"), exercise);
             println!("{}", style(formatstr).green());
-            util::clean();
+            exercise.clean();
             Ok(())
         } else {
             println!("{}", String::from_utf8_lossy(&runcmd.stdout));
             println!("{}", String::from_utf8_lossy(&runcmd.stderr));
 
-            let formatstr = format!("{} Ran {} with errors", Emoji("⚠️ ", "!"), filename);
+            let formatstr = format!("{} Ran {} with errors", Emoji("⚠️ ", "!"), exercise);
             println!("{}", style(formatstr).red());
-            util::clean();
+            exercise.clean();
             Err(())
         }
     } else {
@@ -59,11 +42,11 @@ pub fn compile_and_run(filename: &str) -> Result<(), ()> {
         let formatstr = format!(
             "{} Compilation of {} failed! Compiler error message:\n",
             Emoji("⚠️ ", "!"),
-            filename
+            exercise
         );
         println!("{}", style(formatstr).red());
         println!("{}", String::from_utf8_lossy(&compilecmd.stderr));
-        util::clean();
+        exercise.clean();
         Err(())
     }
 }

src/util.rs

@@ -1,41 +0,0 @@
-use std::fs::remove_file;
-use std::process::{self, Command, Output};
-
-const RUSTC_COLOR_ARGS: &[&str] = &["--color", "always"];
-
-fn temp_file() -> String {
-    format!("./temp_{}", process::id())
-}
-
-pub fn compile_test_cmd(filename: &str) -> Output {
-    Command::new("rustc")
-        .args(&["--test", filename, "-o", &temp_file()])
-        .args(RUSTC_COLOR_ARGS)
-        .output()
-        .expect("failed to compile exercise")
-}
-
-pub fn compile_cmd(filename: &str) -> Output {
-    Command::new("rustc")
-        .args(&[filename, "-o", &temp_file()])
-        .args(RUSTC_COLOR_ARGS)
-        .output()
-        .expect("failed to compile exercise")
-}
-
-pub fn run_cmd() -> Output {
-    Command::new(&temp_file())
-        .output()
-        .expect("failed to run exercise")
-}
-
-pub fn clean() {
-    let _ignored = remove_file(&temp_file());
-}
-
-#[test]
-fn test_clean() {
-    std::fs::File::create(&temp_file()).unwrap();
-    clean();
-    assert!(!std::path::Path::new(&temp_file()).exists());
-}

src/verify.rs

@@ -1,86 +1,71 @@
-use crate::util;
+use crate::exercise::{Exercise, Mode};
 use console::{style, Emoji};
 use indicatif::ProgressBar;
-use std::fs;
-use toml::Value;
 
-pub fn verify(start_at: Option<&str>) -> Result<(), ()> {
+pub fn verify<'a>(start_at: impl IntoIterator<Item=&'a Exercise>) -> Result<(), ()> {
-    let toml: Value = fs::read_to_string("info.toml").unwrap().parse().unwrap();
+    for exercise in start_at {
-    let tomlvec: &Vec<Value> = toml.get("exercises").unwrap().as_array().unwrap();
+        match exercise.mode {
-    let mut hit_start_at = false;
+            Mode::Test => test(&exercise)?,
-
+            Mode::Compile => compile_only(&exercise)?,
-    for i in tomlvec {
-        let path = i.get("path").unwrap().as_str().unwrap();
-
-        if let Some(start_at) = start_at {
-            if start_at.ends_with(path) {
-                hit_start_at = true;
-            } else if !hit_start_at {
-                continue;
-            }
-        }
-
-        match i.get("mode").unwrap().as_str().unwrap() {
-            "test" => test(path)?,
-            "compile" => compile_only(path)?,
-            _ => (),
         }
     }
     Ok(())
 }
 
-fn compile_only(filename: &str) -> Result<(), ()> {
+fn compile_only(exercise: &Exercise) -> Result<(), ()> {
     let progress_bar = ProgressBar::new_spinner();
-    progress_bar.set_message(format!("Compiling {}...", filename).as_str());
+    progress_bar.set_message(format!("Compiling {}...", exercise).as_str());
     progress_bar.enable_steady_tick(100);
-    let compilecmd = util::compile_cmd(filename);
+    let compile_output = exercise.compile();
     progress_bar.finish_and_clear();
-    if compilecmd.status.success() {
+    if compile_output.status.success() {
         let formatstr = format!(
             "{} Successfully compiled {}!",
             Emoji("✅", "✓"),
-            filename
+            exercise
         );
         println!("{}", style(formatstr).green());
-        util::clean();
+        exercise.clean();
         Ok(())
     } else {
         let formatstr = format!(
             "{} Compilation of {} failed! Compiler error message:\n",
             Emoji("⚠️ ", "!"),
-            filename
+            exercise
         );
         println!("{}", style(formatstr).red());
-        println!("{}", String::from_utf8_lossy(&compilecmd.stderr));
+        println!("{}", String::from_utf8_lossy(&compile_output.stderr));
-        util::clean();
+        exercise.clean();
         Err(())
     }
 }
 
-pub fn test(filename: &str) -> Result<(), ()> {
+pub fn test(exercise: &Exercise) -> Result<(), ()> {
     let progress_bar = ProgressBar::new_spinner();
-    progress_bar.set_message(format!("Testing {}...", filename).as_str());
+    progress_bar.set_message(format!("Testing {}...", exercise).as_str());
     progress_bar.enable_steady_tick(100);
-    let testcmd = util::compile_test_cmd(filename);
+
-    if testcmd.status.success() {
+    let compile_output = exercise.compile();
-        progress_bar.set_message(format!("Running {}...", filename).as_str());
+    if compile_output.status.success() {
-        let runcmd = util::run_cmd();
+        progress_bar.set_message(format!("Running {}...", exercise).as_str());
+
+        let runcmd = exercise.run();
         progress_bar.finish_and_clear();
 
         if runcmd.status.success() {
-            let formatstr = format!("{} Successfully tested {}!", Emoji("✅", "✓"), filename);
+            let formatstr = format!("{} Successfully tested {}!", Emoji("✅", "✓"), exercise);
             println!("{}", style(formatstr).green());
-            util::clean();
+            exercise.clean();
             Ok(())
         } else {
             let formatstr = format!(
                 "{} Testing of {} failed! Please try again. Here's the output:",
                 Emoji("⚠️ ", "!"),
-                filename
+                exercise
             );
             println!("{}", style(formatstr).red());
             println!("{}", String::from_utf8_lossy(&runcmd.stdout));
-            util::clean();
+            exercise.clean();
             Err(())
         }
     } else {
@@ -88,11 +73,11 @@ pub fn test(filename: &str) -> Result<(), ()> {
         let formatstr = format!(
             "{} Compiling of {} failed! Please try again. Here's the output:",
             Emoji("⚠️ ", "!"),
-            filename
+            exercise
         );
         println!("{}", style(formatstr).red());
-        println!("{}", String::from_utf8_lossy(&testcmd.stderr));
+        println!("{}", String::from_utf8_lossy(&compile_output.stderr));
-        util::clean();
+        exercise.clean();
         Err(())
     }
 }