Display forbid(clippy::undocumented_unsafe_blocks) crates differently

[fee1-dead]

For crates that require unsafe usage, it is possible to ensure all usages of unsafe are documented and justified as sound by adding #![forbid(clippy::undocumented_unsafe_blocks)] to the top of a crate. The lint was recently added so it is not currently very popular, but I think there is still potential.

[pinkforest]

Thanks for raising this up -

So this is clippy dependant thing whilst not guaranteeing build failure like #![forbid(unsafe_code)] does which we look for.

We would have to radd, run and parse clippy - bringing another run dependency

We could perhaps add it - #226

cargo geiger --documented ignore | count | warn | error [default count] subcommand

Where

 ignore - ignores silently
 count - counts them and includes them in the analysis
 warn - counts, warns about them in STDOUT
 error - counts, errors about them in STDERR and exits(signal)

There is some work at #213 ideating over how we would be able to track changes and do more validations.

Also WG has rust-secure-code/wg#19 (comment)

[pinkforest]

Re: Outputs - Requires nightly which we would have to check and guarantee being used - or stable that has received this.

Clippy I remember didn't have a flag where we can say require a lint capability or error out - if we would use this as a runner dependency.

Nightly

rustup show
Default host: x86_64-unknown-linux-gnu

installed toolchains
--------------------

nightly-x86_64-unknown-linux-gnu (default)

active toolchain
----------------

nightly-x86_64-unknown-linux-gnu (default)

foobar@foobar-MS-7C77:~/rust-thing/unsafe-test$ cargo build
   Compiling unsafe-test v0.1.0 (/home/foobar/rust-thing/unsafe-test)
warning: unnecessary `unsafe` block
 --> src/main.rs:4:3
  |
4 |   unsafe { dbg!("wee.."); }
  |   ^^^^^^ unnecessary `unsafe` block
  |
  = note: `#[warn(unused_unsafe)]` on by default

warning: `unsafe-test` (bin "unsafe-test") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.39s
foobar@foobar-MS-7C77:~/rust-thing/unsafe-test$ cargo clippy
    Checking unsafe-test v0.1.0 (/home/foobar/rust-thing/unsafe-test)
warning: unnecessary `unsafe` block
 --> src/main.rs:4:3
  |
4 |   unsafe { dbg!("wee.."); }
  |   ^^^^^^ unnecessary `unsafe` block
  |
  = note: `#[warn(unused_unsafe)]` on by default

error: unsafe block missing a safety comment
 --> src/main.rs:4:3
  |
4 |   unsafe { dbg!("wee.."); }
  |   ^^^^^^^^^^^^^^^^^^^^^^^^^
  |
note: the lint level is defined here
 --> src/main.rs:1:11
  |
1 | #![forbid(clippy::undocumented_unsafe_blocks)]
  |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks
help: consider adding a safety comment
  |
4 ~   // Safety: ...
5 +   unsafe { dbg!("wee.."); }
  |

warning: `unsafe-test` (bin "unsafe-test") generated 1 warning

Stable

rustup default stable
info: using existing install for 'stable-x86_64-unknown-linux-gnu'
info: default toolchain set to 'stable-x86_64-unknown-linux-gnu'

  stable-x86_64-unknown-linux-gnu unchanged - rustc 1.57.0 (f1edd0429 2021-11-29)

foobar@foobar-MS-7C77:~/rust-thing/unsafe-test$ cargo clippy
    Checking unsafe-test v0.1.0 (/home/foobar/rust-thing/unsafe-test)
warning: unknown lint: `clippy::undocumented_unsafe_blocks`
 --> src/main.rs:1:11
  |
1 | #![forbid(clippy::undocumented_unsafe_blocks)]
  |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unknown_lints)]` on by default

warning: unnecessary `unsafe` block
 --> src/main.rs:4:3
  |
4 |   unsafe { dbg!("wee.."); }
  |   ^^^^^^ unnecessary `unsafe` block
  |
  = note: `#[warn(unused_unsafe)]` on by default

warning: `unsafe-test` (bin "unsafe-test") generated 2 warnings
    Finished dev [unoptimized + debuginfo] target(s) in 0.30s

[fee1-dead]

@pinkforest It's been added in rust 1.58, which is not released yet.

[aewag]

Clippy I remember didn't have a flag where we can say require a lint capability or error out - if we would use this as a runner dependency.

The list of available/supported clippy lints can be get with clippy-driver -Whelp
(see rust-lang/rust-clippy#6122 (comment) and rust-lang/rust#77671)

Using that, the runner could grep for the required lints and return an error if not available.

[aewag]

I would suggest to pass the required lints from within cargo-geiger to the clippy runner as &mut [Lint].

Below is an example, which simply checks, if undocumented-unsafe-blocks and unsafe-removed-from-name is available.

use std::process::Command;

#[derive(Debug)]
enum LintLevel {
    Allow,
    Warn,
    Deny,
    None,
}

#[derive(Debug)]
struct Lint {
    pub name: String,
    pub level: LintLevel,
    pub available: bool,
}

impl Lint {
    pub fn new(name: String, level: LintLevel) -> Self {
        Lint {
            name,
            level,
            available: false,
        }
    }
}

pub fn main() {
    let mut required_lints = [
        Lint::new("undocumented-unsafe-blocks".to_string(), LintLevel::Allow),
        Lint::new("unsafe-removed-from-name".to_string(), LintLevel::Deny),
    ];

    let output = Command::new("clippy-driver")
        .arg("-Whelp")
        .output()
        .expect("failed to execute");

    for lint in String::from_utf8(output.stdout)
        .unwrap()
        .split(" ")
        .filter(|l| l.contains("clippy::"))
        .map(|l| l.replace("clippy::", "").replace(",", "").replace("\n", ""))
    {
        required_lints.iter_mut().filter(|rl| rl.name == lint).for_each(|rl| rl.available = true);
    }
    for rl in required_lints {
        println!("{:?}", rl);
    }

}

The clippy runner could first check whether all required lints are available and if this is the case, trigger clippy to run.

If not it could return an error and cargo-geiger could, if wanted, check which lints are not available and return this to the user or maybe restart without the unavailable lints.