Skip to content

Commit

Permalink
feat: Add clippy lints
Browse files Browse the repository at this point in the history
- adds a new 'clippy' category for exercises
- clippy exercises should throw no warnings
- install script now also installs clippy

is related to rust-lang/rust-clippy#2604
  • Loading branch information
Tarnadas committed Feb 26, 2020
1 parent 7e8530b commit 1e2fd9c
Show file tree
Hide file tree
Showing 10 changed files with 110 additions and 15 deletions.
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@
target/
**/*.rs.bk
.DS_Store
*.pdb
*.pdb
exercises/clippy/Cargo.toml
exercises/clippy/Cargo.lock
8 changes: 8 additions & 0 deletions exercises/clippy/README.md
Original file line number Diff line number Diff line change
@@ -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/).
15 changes: 15 additions & 0 deletions exercises/clippy/clippy1.rs
Original file line number Diff line number Diff line change
@@ -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!");
}
}

This comment has been minimized.

Copy link
@sl4m

sl4m Mar 5, 2020

Contributor

@Tarnadas What's the intent of this exercise? I agree we should not be comparing against two floats, but I'm not quite following what the solution should be.

This comment has been minimized.

Copy link
@Tarnadas

Tarnadas Mar 11, 2020

Author Contributor

hey, sorry for the late reply.
The intent of the exercise is to show what static code analysis can do and that it can show you obvious logical errors in your code.

Comparing floats should always be done in a range rather than with an exact value.

13 changes: 13 additions & 0 deletions exercises/clippy/clippy2.rs
Original file line number Diff line number Diff line change
@@ -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);
}
16 changes: 16 additions & 0 deletions info.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand Down
13 changes: 9 additions & 4 deletions install.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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."
21 changes: 12 additions & 9 deletions install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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."
32 changes: 31 additions & 1 deletion src/exercise.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
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};

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())
Expand All @@ -19,6 +20,7 @@ fn temp_file() -> String {
pub enum Mode {
Compile,
Test,
Clippy,
}

#[derive(Deserialize)]
Expand Down Expand Up @@ -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.");

Expand Down
1 change: 1 addition & 0 deletions src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand Down
2 changes: 2 additions & 0 deletions src/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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!("");
Expand Down

0 comments on commit 1e2fd9c

Please sign in to comment.