From 1e2fd9c92f8cd6e389525ca1a999fca4c90b5921 Mon Sep 17 00:00:00 2001 From: Mario Reder Date: Fri, 14 Feb 2020 15:25:03 +0100 Subject: [PATCH] feat: Add clippy lints - adds a new 'clippy' category for exercises - clippy exercises should throw no warnings - install script now also installs clippy is related to https://github.com/rust-lang/rust-clippy/issues/2604 --- .gitignore | 4 +++- exercises/clippy/README.md | 8 ++++++++ exercises/clippy/clippy1.rs | 15 +++++++++++++++ exercises/clippy/clippy2.rs | 13 +++++++++++++ info.toml | 16 ++++++++++++++++ install.ps1 | 13 +++++++++---- install.sh | 21 ++++++++++++--------- src/exercise.rs | 32 +++++++++++++++++++++++++++++++- src/run.rs | 1 + src/verify.rs | 2 ++ 10 files changed, 110 insertions(+), 15 deletions(-) create mode 100644 exercises/clippy/README.md create mode 100644 exercises/clippy/clippy1.rs create mode 100644 exercises/clippy/clippy2.rs diff --git a/.gitignore b/.gitignore index 4cf87f2..de87c1e 100644 --- a/.gitignore +++ b/.gitignore @@ -2,4 +2,6 @@ target/ **/*.rs.bk .DS_Store -*.pdb \ No newline at end of file +*.pdb +exercises/clippy/Cargo.toml +exercises/clippy/Cargo.lock diff --git a/exercises/clippy/README.md b/exercises/clippy/README.md new file mode 100644 index 0000000..60a12fe --- /dev/null +++ b/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/). diff --git a/exercises/clippy/clippy1.rs b/exercises/clippy/clippy1.rs new file mode 100644 index 0000000..2b4c635 --- /dev/null +++ b/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!"); + } +} diff --git a/exercises/clippy/clippy2.rs b/exercises/clippy/clippy2.rs new file mode 100644 index 0000000..37af9ed --- /dev/null +++ b/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); +} diff --git a/info.toml b/info.toml index b1c6db5..82c22b7 100644 --- a/info.toml +++ b/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]] diff --git a/install.ps1 b/install.ps1 index f644610..04ea4a0 100644 --- a/install.ps1 +++ b/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." diff --git a/install.sh b/install.sh index 85bdad7..1075061 100755 --- a/install.sh +++ b/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." diff --git a/src/exercise.rs b/src/exercise.rs index d72eeb5..30b1864 100644 --- a/src/exercise.rs +++ b/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."); diff --git a/src/run.rs b/src/run.rs index cfde7ab..ebb0ae6 100644 --- a/src/run.rs +++ b/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(()) } diff --git a/src/verify.rs b/src/verify.rs index 3d14896..229aa6d 100644 --- a/src/verify.rs +++ b/src/verify.rs @@ -7,6 +7,7 @@ pub fn verify<'a>(start_at: impl IntoIterator) -> 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!("");