Auto merge of #269 - Tarnadas:master, r=fmoko

feat: Add clippy lints

This is a feature PR which adds the possiblity to create clippy exercises.

Clippy has many awesome linting rules, which can give a deeper understanding about the Rust programming language, therefor I made this PR.

.gitignore

@@ -2,4 +2,6 @@
 target/
 **/*.rs.bk
 .DS_Store
-*.pdb
+*.pdb
+exercises/clippy/Cargo.toml
+exercises/clippy/Cargo.lock

exercises/clippy/README.md

@@ -0,0 +1,8 @@
+### Clippy
+
+The Clippy tool is a collection of lints to analyze your code so you can catch common mistakes and improve your Rust code.
+
+If you used the installation script for Rustlings, Clippy should be already installed.
+If not you can install it manually via `rustup component add clippy`.
+
+For more information about Clippy lints, please see [their documentation page](https://rust-lang.github.io/rust-clippy/master/).

exercises/clippy/clippy1.rs

@@ -0,0 +1,15 @@
+// clippy1.rs
+// The Clippy tool is a collection of lints to analyze your code
+// so you can catch common mistakes and improve your Rust code.
+//
+// Execute `rustlings hint clippy1` for hints :)
+
+// I AM NOT DONE
+
+fn main() {
+    let x = 1.2331f64;
+    let y = 1.2332f64;
+    if y != x {
+        println!("Success!");
+    }
+}

exercises/clippy/clippy2.rs

@@ -0,0 +1,13 @@
+// clippy2.rs
+// Make me compile! Execute `rustlings hint clippy2` for hints :)
+
+// I AM NOT DONE
+
+fn main() {
+    let mut res = 42;
+    let option = Some(12);
+    for x in option {
+        res += x;
+    }
+    println!("{}", res);
+}

info.toml

@@ -529,6 +529,22 @@ hint = """
 It should be doing some checking, returning an `Err` result if those checks fail, and only
 returning an `Ok` result if those checks determine that everything is... okay :)"""
 
+#  CLIPPY
+
+[[exercises]]
+name = "clippy1"
+path = "exercises/clippy/clippy1.rs"
+mode = "clippy"
+hint = """
+Floating point calculations are usually imprecise, so asking if two values are exactly equal is asking for trouble"""
+
+[[exercises]]
+name = "clippy2"
+path = "exercises/clippy/clippy2.rs"
+mode = "clippy"
+hint = """
+`for` loops over Option values are more clearly expressed as an `if let`"""
+
 # STANDARD LIBRARY TYPES
 
 [[exercises]]

install.ps1

@@ -72,14 +72,19 @@ if (!($LASTEXITCODE -eq 0)) {
 # but anyone running pwsh 5 will have to pass the argument.
 $version = Invoke-WebRequest -UseBasicParsing https://api.github.com/repos/rust-lang/rustlings/releases/latest `
     | ConvertFrom-Json | Select-Object -ExpandProperty tag_name
-Write-Host "Checking out version $version..."
-Set-Location $path
-git checkout -q tags/$version
 
 Write-Host "Installing the 'rustlings' executable..."
-cargo install --force --path .
+cargo install --force --git https://github.com/rust-lang/rustlings --tag $version
 if (!(Get-Command rustlings -ErrorAction SilentlyContinue)) {
     Write-Host "WARNING: Please check that you have '~/.cargo/bin' in your PATH environment variable!"
 }
 
+# Checking whether Clippy is installed.
+# Due to a bug in Cargo, this must be done with Rustup: https://github.com/rust-lang/rustup/issues/1514
+$clippy = (rustup component list | Select-String "clippy" | Select-String "installed") | Out-String
+if (!$clippy) {
+    Write-Host "Installing the 'cargo-clippy' executable..."
+    rustup component add clippy
+}
+
 Write-Host "All done! Run 'rustlings' to get started."

install.sh

@@ -82,21 +82,24 @@ else
     echo "SUCCESS: Rust is up to date"
 fi
 
-Path=${1:-rustlings/}
-echo "Cloning Rustlings at $Path..."
-git clone -q https://github.com/rust-lang/rustlings $Path
-
 Version=$(curl -s https://api.github.com/repos/rust-lang/rustlings/releases/latest | python -c "import json,sys;obj=json.load(sys.stdin);print(obj['tag_name']);")
-echo "Checking out version $Version..."
-cd $Path
-git checkout -q tags/$Version
+CargoBin="${CARGO_HOME:-$HOME/.cargo}/bin"
 
 echo "Installing the 'rustlings' executable..."
-cargo install --force --path .
+cargo install --force --git https://github.com/rust-lang/rustlings --tag $Version
 
 if ! [ -x "$(command -v rustlings)" ]
 then
-    echo "WARNING: Please check that you have '~/.cargo/bin' in your PATH environment variable!"
+    echo "WARNING: Please check that you have '$CargoBin' in your PATH environment variable!"
+fi
+
+# Checking whether Clippy is installed.
+# Due to a bug in Cargo, this must be done with Rustup: https://github.com/rust-lang/rustup/issues/1514
+Clippy=$(rustup component list | grep "clippy" | grep "installed")
+if [ -z "$Clippy" ]
+then
+    echo "Installing the 'cargo-clippy' executable..."
+    rustup component add clippy
 fi
 
 echo "All done! Run 'rustlings' to get started."

src/exercise.rs

@@ -1,7 +1,7 @@
 use regex::Regex;
 use serde::Deserialize;
 use std::fmt::{self, Display, Formatter};
-use std::fs::{remove_file, File};
+use std::fs::{self, remove_file, File};
 use std::io::Read;
 use std::path::PathBuf;
 use std::process::{self, Command};
@@ -9,6 +9,7 @@ use std::process::{self, Command};
 const RUSTC_COLOR_ARGS: &[&str] = &["--color", "always"];
 const I_AM_DONE_REGEX: &str = r"(?m)^\s*///?\s*I\s+AM\s+NOT\s+DONE";
 const CONTEXT: usize = 2;
+const CLIPPY_CARGO_TOML_PATH: &str = "./exercises/clippy/Cargo.toml";
 
 fn temp_file() -> String {
     format!("./temp_{}", process::id())
@@ -19,6 +20,7 @@ fn temp_file() -> String {
 pub enum Mode {
     Compile,
     Test,
+    Clippy,
 }
 
 #[derive(Deserialize)]
@@ -83,6 +85,34 @@ impl Exercise {
                 .args(&["--test", self.path.to_str().unwrap(), "-o", &temp_file()])
                 .args(RUSTC_COLOR_ARGS)
                 .output(),
+            Mode::Clippy => {
+                let cargo_toml = format!(
+                    r#"[package]
+name = "{}"
+version = "0.0.1"
+edition = "2018"
+[[bin]]
+name = "{}"
+path = "{}.rs""#,
+                    self.name, self.name, self.name
+                );
+                fs::write(CLIPPY_CARGO_TOML_PATH, cargo_toml)
+                    .expect("Failed to write 📎 Clippy 📎 Cargo.toml file.");
+                // Due to an issue with Clippy, a cargo clean is required to catch all lints.
+                // See https://github.com/rust-lang/rust-clippy/issues/2604
+                // This is already fixed on master branch. See this issue to track merging into Cargo:
+                // https://github.com/rust-lang/rust-clippy/issues/3837
+                Command::new("cargo")
+                    .args(&["clean", "--manifest-path", CLIPPY_CARGO_TOML_PATH])
+                    .args(RUSTC_COLOR_ARGS)
+                    .output()
+                    .expect("Failed to run 'cargo clean'");
+                Command::new("cargo")
+                    .args(&["clippy", "--manifest-path", CLIPPY_CARGO_TOML_PATH])
+                    .args(RUSTC_COLOR_ARGS)
+                    .args(&["--", "-D", "warnings"])
+                    .output()
+            }
         }
         .expect("Failed to run 'compile' command.");
 

src/run.rs

@@ -6,6 +6,7 @@ pub fn run(exercise: &Exercise) -> Result<(), ()> {
     match exercise.mode {
         Mode::Test => test(exercise)?,
         Mode::Compile => compile_and_run(exercise)?,
+        Mode::Clippy => compile_and_run(exercise)?,
     }
     Ok(())
 }

src/verify.rs

@@ -7,6 +7,7 @@ pub fn verify<'a>(start_at: impl IntoIterator<Item = &'a Exercise>) -> Result<()
         let compile_result = match exercise.mode {
             Mode::Test => compile_and_test(&exercise, RunMode::Interactive),
             Mode::Compile => compile_only(&exercise),
+            Mode::Clippy => compile_only(&exercise),
         };
         if !compile_result.unwrap_or(false) {
             return Err(exercise);
@@ -99,6 +100,7 @@ fn prompt_for_completion(exercise: &Exercise) -> bool {
     let success_msg = match exercise.mode {
         Mode::Compile => "The code is compiling!",
         Mode::Test => "The code is compiling, and the tests pass!",
+        Mode::Clippy => "The code is compiling, and 📎 Clippy 📎 is happy!",
     };
 
     println!("");