Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add clippy command to xtask #128

Merged
merged 12 commits into from
Feb 8, 2021
63 changes: 63 additions & 0 deletions xtask/src/clippy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
use std::env;
use std::process::Command;

use anyhow::{bail, Result};

pub fn run(package: Option<String>, target: Option<String>) -> Result<()> {
let package = package.unwrap_or_else(|| {
let path = env::current_dir().unwrap();
let manifest_path = path.join("Cargo.toml");
let contents = std::fs::read(manifest_path).unwrap();
let toml: toml::Value = toml::from_slice(&contents).unwrap();

// someday, try blocks will be stable...
toml.get("package")
.and_then(|v| v.get("name"))
.and_then(|v| v.as_str())
.expect("Couldn't find [package.name]; pass -p <package> to run clippy on a specific package or --all to check all packages")
.to_string()
});

println!("Running clippy on: {}", package);

let mut cmd = Command::new("cargo");
cmd.arg("clippy");
// TODO: Remove this argument once resolved on stable:
//
// https://github.com/rust-lang/rust-clippy/issues/4612
//
// TL;DR: Switching back and forth between "clippy" and "check"
// caches the results from one execution. This means a succeeding
// "check" execution may hide a failing "clippy" execution.
cmd.arg("-Zunstable-options");
cmd.arg("-p");
cmd.arg(&package);

if let Some(target) = target {
cmd.arg("--target");
cmd.arg(target);
}

// Clippy arguments
cmd.arg("--");
cmd.arg("-D");
cmd.arg("clippy::all");
cmd.arg("-A");
cmd.arg("clippy::missing_safety_doc");
cmd.arg("-A");
cmd.arg("clippy::identity_op");
cmd.arg("-A");
cmd.arg("clippy::too_many_arguments");

// this is only actually used for demo-stm32h7 but is harmless to include, so let's do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm. This information comes from the app.toml normally. It would be nice to be able to run clippy on an app. That doesn't need to happen in this change though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, full transparency, this was basically lifted exactly from xtask/src/check.rs. I also see it being set in xtask/src/dist.rs when "HUBRIS_BOARD" is set before running rustc - how is this extracted from app.toml files?

// it unconditionally
cmd.env("HUBRIS_BOARD", "nucleo-h743zi2");

let status = cmd.status()?;

if !status.success() {
bail!("Could not build {}", package);
}

Ok(())
}
132 changes: 96 additions & 36 deletions xtask/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use serde::Deserialize;
use indexmap::IndexMap;

mod check;
mod clippy;
mod dist;
mod flash;
mod gdb;
Expand Down Expand Up @@ -85,6 +86,21 @@ enum Xtask {
#[structopt(long)]
all: bool,
},

/// Runs `cargo clippy` on a specified task
Clippy {
/// the target to build for, uses [package.metadata.build.target] if not passed
#[structopt(long)]
target: Option<String>,

/// the package you're trying to build, uses current directory if not passed
#[structopt(short)]
package: Option<String>,

/// check all packages, not only one
#[structopt(long)]
all: bool,
},
}

#[derive(Clone, Debug, Deserialize)]
Expand Down Expand Up @@ -176,6 +192,76 @@ struct LoadSegment {
data: Vec<u8>,
}

// For commands which may execute on specific packages, this enum
// identifies the set of packages that should be operated upon.
enum RequestedPackages {
// Specifies a single specific (Package, Target) pair.
Specific(Option<String>, Option<String>),
// Specifies the command should operate on all packages.
All,
}

impl RequestedPackages {
fn new(package: Option<String>, target: Option<String>, all: bool) -> Self {
if all {
RequestedPackages::All
} else {
RequestedPackages::Specific(package, target)
}
}
}

// Runs a function on the a requested set of packages.
//
// # Arguments
//
// * `requested` - The requested packages to operate upon.
// * `func` - The function to execute for requested packages,
// acting on a (Package, Target) pair.
fn run_for_packages<F>(requested: RequestedPackages, func: F) -> Result<()>
where
F: Fn(Option<String>, Option<String>) -> Result<()>,
{
match requested {
RequestedPackages::Specific(package, target) => func(package, target)?,
RequestedPackages::All => {
use cargo_metadata::MetadataCommand;

let metadata = MetadataCommand::new()
.manifest_path("./Cargo.toml")
.exec()
.unwrap();

#[derive(Debug, Deserialize)]
struct CustomMetadata {
build: Option<BuildMetadata>,
}

#[derive(Debug, Deserialize)]
struct BuildMetadata {
target: Option<String>,
}

for id in &metadata.workspace_members {
let package = metadata
.packages
.iter()
.find(|p| &p.id == id)
.unwrap()
.clone();

let m: Option<CustomMetadata> =
serde_json::from_value(package.metadata)?;

let target = (|| m?.build?.target)();

func(Some(package.name), target)?;
}
}
}
Ok(())
}

fn main() -> Result<()> {
let xtask = Xtask::from_args();

Expand Down Expand Up @@ -211,42 +297,16 @@ fn main() -> Result<()> {
target,
all,
} => {
if !all {
check::run(package, target)?;
} else {
use cargo_metadata::MetadataCommand;

let metadata = MetadataCommand::new()
.manifest_path("./Cargo.toml")
.exec()
.unwrap();

#[derive(Debug, Deserialize)]
struct CustomMetadata {
build: Option<BuildMetadata>,
}

#[derive(Debug, Deserialize)]
struct BuildMetadata {
target: Option<String>,
}

for id in &metadata.workspace_members {
let package = metadata
.packages
.iter()
.find(|p| &p.id == id)
.unwrap()
.clone();

let m: Option<CustomMetadata> =
serde_json::from_value(package.metadata)?;

let target = (|| m?.build?.target)();

check::run(Some(package.name), target)?;
}
}
let requested = RequestedPackages::new(package, target, all);
run_for_packages(requested, check::run)?;
}
Xtask::Clippy {
package,
target,
all,
} => {
let requested = RequestedPackages::new(package, target, all);
run_for_packages(requested, clippy::run)?;
}
}

Expand Down