Skip to content

Commit

Permalink
Auto merge of #6759 - yaahallo:cargo-clippy, r=alexcrichton
Browse files Browse the repository at this point in the history
Cargo clippy

resolves rust-lang/rust-clippy#3837
  • Loading branch information
bors committed Apr 1, 2019
2 parents d04d75b + 842da62 commit 025b01e
Show file tree
Hide file tree
Showing 13 changed files with 129 additions and 80 deletions.
2 changes: 1 addition & 1 deletion src/bin/cargo/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ Run with 'cargo -Z [FLAG] [SUBCOMMAND]'"
}

if let Some(code) = args.value_of("explain") {
let mut procss = config.rustc(None)?.process();
let mut procss = config.load_global_rustc(None)?.process();
procss.arg("--explain").arg(code).exec()?;
return Ok(());
}
Expand Down
77 changes: 77 additions & 0 deletions src/bin/cargo/commands/clippy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
use crate::command_prelude::*;

use cargo::ops;
use cargo::util;

pub fn cli() -> App {
subcommand("clippy-preview")
.about("Checks a package to catch common mistakes and improve your Rust code.")
.arg_package_spec(
"Package(s) to check",
"Check all packages in the workspace",
"Exclude packages from the check",
)
.arg_jobs()
.arg_targets_all(
"Check only this package's library",
"Check only the specified binary",
"Check all binaries",
"Check only the specified example",
"Check all examples",
"Check only the specified test target",
"Check all tests",
"Check only the specified bench target",
"Check all benches",
"Check all targets",
)
.arg_release("Check artifacts in release mode, with optimizations")
.arg_features()
.arg_target_triple("Check for the target triple")
.arg_target_dir()
.arg_manifest_path()
.arg_message_format()
.after_help(
"\
If the `--package` argument is given, then SPEC is a package ID specification
which indicates which package should be built. If it is not given, then the
current package is built. For more information on SPEC and its format, see the
`cargo help pkgid` command.
All packages in the workspace are checked if the `--all` flag is supplied. The
`--all` flag is automatically assumed for a virtual manifest.
Note that `--exclude` has to be specified in conjunction with the `--all` flag.
To allow or deny a lint from the command line you can use `cargo clippy --`
with:
-W --warn OPT Set lint warnings
-A --allow OPT Set lint allowed
-D --deny OPT Set lint denied
-F --forbid OPT Set lint forbidden
You can use tool lints to allow or deny lints from your code, eg.:
#[allow(clippy::needless_lifetimes)]
",
)
}

pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
let ws = args.workspace(config)?;

let mode = CompileMode::Check { test: false };
let mut compile_opts = args.compile_options(config, mode, Some(&ws))?;

if !config.cli_unstable().unstable_options {
return Err(failure::format_err!(
"`clippy-preview` is unstable, pass `-Z unstable-options` to enable it"
)
.into());
}

let wrapper = util::process("clippy-driver");
compile_opts.build_config.rustc_wrapper = Some(wrapper);

ops::compile(&ws, &compile_opts)?;
Ok(())
}
3 changes: 3 additions & 0 deletions src/bin/cargo/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pub fn builtin() -> Vec<App> {
build::cli(),
check::cli(),
clean::cli(),
clippy::cli(),
doc::cli(),
fetch::cli(),
fix::cli(),
Expand Down Expand Up @@ -41,6 +42,7 @@ pub fn builtin_exec(cmd: &str) -> Option<fn(&mut Config, &ArgMatches<'_>) -> Cli
"build" => build::exec,
"check" => check::exec,
"clean" => clean::exec,
"clippy-preview" => clippy::exec,
"doc" => doc::exec,
"fetch" => fetch::exec,
"fix" => fix::exec,
Expand Down Expand Up @@ -76,6 +78,7 @@ pub mod bench;
pub mod build;
pub mod check;
pub mod clean;
pub mod clippy;
pub mod doc;
pub mod fetch;
pub mod fix;
Expand Down
13 changes: 4 additions & 9 deletions src/cargo/core/compiler/build_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::path::Path;

use serde::ser;

use crate::util::ProcessBuilder;
use crate::util::{CargoResult, CargoResultExt, Config, RustfixDiagnosticServer};

/// Configuration information for a rustc build.
Expand All @@ -23,12 +24,8 @@ pub struct BuildConfig {
pub force_rebuild: bool,
/// Output a build plan to stdout instead of actually compiling.
pub build_plan: bool,
/// Use Cargo itself as the wrapper around rustc, only used for `cargo fix`.
pub cargo_as_rustc_wrapper: bool,
/// Extra env vars to inject into rustc commands.
pub extra_rustc_env: Vec<(String, String)>,
/// Extra args to inject into rustc commands.
pub extra_rustc_args: Vec<String>,
/// An optional wrapper, if any, used to wrap rustc invocations
pub rustc_wrapper: Option<ProcessBuilder>,
pub rustfix_diagnostic_server: RefCell<Option<RustfixDiagnosticServer>>,
}

Expand Down Expand Up @@ -98,9 +95,7 @@ impl BuildConfig {
message_format: MessageFormat::Human,
force_rebuild: false,
build_plan: false,
cargo_as_rustc_wrapper: false,
extra_rustc_env: Vec::new(),
extra_rustc_args: Vec::new(),
rustc_wrapper: None,
rustfix_diagnostic_server: RefCell::new(None),
})
}
Expand Down
6 changes: 5 additions & 1 deletion src/cargo/core/compiler/build_context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
profiles: &'a Profiles,
extra_compiler_args: HashMap<Unit<'a>, Vec<String>>,
) -> CargoResult<BuildContext<'a, 'cfg>> {
let rustc = config.rustc(Some(ws))?;
let mut rustc = config.load_global_rustc(Some(ws))?;
if let Some(wrapper) = &build_config.rustc_wrapper {
rustc.set_wrapper(wrapper.clone());
}

let host_config = TargetConfig::new(config, &rustc.host)?;
let target_config = match build_config.requested_target.as_ref() {
Some(triple) => TargetConfig::new(config, triple)?,
Expand Down
22 changes: 2 additions & 20 deletions src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,29 +75,11 @@ pub struct Compilation<'cfg> {

impl<'cfg> Compilation<'cfg> {
pub fn new<'a>(bcx: &BuildContext<'a, 'cfg>) -> CargoResult<Compilation<'cfg>> {
// If we're using cargo as a rustc wrapper then we're in a situation
// like `cargo fix`. For now just disregard the `RUSTC_WRAPPER` env var
// (which is typically set to `sccache` for now). Eventually we'll
// probably want to implement `RUSTC_WRAPPER` for `cargo fix`, but we'll
// leave that open as a bug for now.
let mut rustc = if bcx.build_config.cargo_as_rustc_wrapper {
let mut rustc = bcx.rustc.process_no_wrapper();
let prog = rustc.get_program().to_owned();
rustc.env("RUSTC", prog);
rustc.program(env::current_exe()?);
rustc
} else {
bcx.rustc.process()
};
let mut rustc = bcx.rustc.process();

if bcx.config.extra_verbose() {
rustc.display_env_vars();
}
for (k, v) in bcx.build_config.extra_rustc_env.iter() {
rustc.env(k, v);
}
for arg in bcx.build_config.extra_rustc_args.iter() {
rustc.arg(arg);
}
let srv = bcx.build_config.rustfix_diagnostic_server.borrow();
if let Some(server) = &*srv {
server.configure(&mut rustc);
Expand Down
14 changes: 7 additions & 7 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,6 @@ use log::debug;
use same_file::is_same_file;
use serde::Serialize;

use crate::core::manifest::TargetSourcePath;
use crate::core::profiles::{Lto, PanicStrategy, Profile};
use crate::core::{PackageId, Target};
use crate::util::errors::{CargoResult, CargoResultExt, Internal, ProcessError};
use crate::util::paths;
use crate::util::{self, machine_message, process, Freshness, ProcessBuilder};
use crate::util::{internal, join_paths, profile};
pub use self::build_config::{BuildConfig, CompileMode, MessageFormat};
pub use self::build_context::{BuildContext, FileFlavor, TargetConfig, TargetInfo};
use self::build_plan::BuildPlan;
Expand All @@ -39,6 +32,13 @@ use self::job::{Job, Work};
use self::job_queue::JobQueue;
pub use self::layout::is_bad_artifact_name;
use self::output_depinfo::output_depinfo;
use crate::core::manifest::TargetSourcePath;
use crate::core::profiles::{Lto, PanicStrategy, Profile};
use crate::core::{PackageId, Target};
use crate::util::errors::{CargoResult, CargoResultExt, Internal, ProcessError};
use crate::util::paths;
use crate::util::{self, machine_message, process, Freshness, ProcessBuilder};
use crate::util::{internal, join_paths, profile};

/// Indicates whether an object is for the host architcture or the target architecture.
///
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub fn fetch<'a>(
let jobs = Some(1);
let config = ws.config();
let build_config = BuildConfig::new(config, jobs, &options.target, CompileMode::Build)?;
let rustc = config.rustc(Some(ws))?;
let rustc = config.load_global_rustc(Some(ws))?;
let target_info =
TargetInfo::new(config, &build_config.requested_target, &rustc, Kind::Target)?;
{
Expand Down
39 changes: 13 additions & 26 deletions src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ use crate::core::Workspace;
use crate::ops::{self, CompileOptions};
use crate::util::diagnostic_server::{Message, RustfixDiagnosticServer};
use crate::util::errors::CargoResult;
use crate::util::paths;
use crate::util::{self, paths};
use crate::util::{existing_vcs_repo, LockServer, LockServerClient};

const FIX_ENV: &str = "__CARGO_FIX_PLZ";
Expand All @@ -81,46 +81,31 @@ pub fn fix(ws: &Workspace<'_>, opts: &mut FixOptions<'_>) -> CargoResult<()> {

// Spin up our lock server, which our subprocesses will use to synchronize fixes.
let lock_server = LockServer::new()?;
opts.compile_opts
.build_config
.extra_rustc_env
.push((FIX_ENV.to_string(), lock_server.addr().to_string()));
let mut wrapper = util::process(env::current_exe()?);
wrapper.env(FIX_ENV, lock_server.addr().to_string());
let _started = lock_server.start()?;

opts.compile_opts.build_config.force_rebuild = true;

if opts.broken_code {
let key = BROKEN_CODE_ENV.to_string();
opts.compile_opts
.build_config
.extra_rustc_env
.push((key, "1".to_string()));
wrapper.env(BROKEN_CODE_ENV, "1");
}

if opts.edition {
let key = EDITION_ENV.to_string();
opts.compile_opts
.build_config
.extra_rustc_env
.push((key, "1".to_string()));
wrapper.env(EDITION_ENV, "1");
} else if let Some(edition) = opts.prepare_for {
opts.compile_opts
.build_config
.extra_rustc_env
.push((PREPARE_FOR_ENV.to_string(), edition.to_string()));
wrapper.env(PREPARE_FOR_ENV, edition);
}
if opts.idioms {
opts.compile_opts
.build_config
.extra_rustc_env
.push((IDIOMS_ENV.to_string(), "1".to_string()));
wrapper.env(IDIOMS_ENV, "1");
}
opts.compile_opts.build_config.cargo_as_rustc_wrapper = true;

*opts
.compile_opts
.build_config
.rustfix_diagnostic_server
.borrow_mut() = Some(RustfixDiagnosticServer::new()?);
opts.compile_opts.build_config.rustc_wrapper = Some(wrapper);

ops::compile(ws, &opts.compile_opts)?;
Ok(())
Expand Down Expand Up @@ -207,7 +192,7 @@ pub fn fix_maybe_exec_rustc() -> CargoResult<bool> {

let args = FixArgs::get();
trace!("cargo-fix as rustc got file {:?}", args.file);
let rustc = env::var_os("RUSTC").expect("failed to find RUSTC env var");
let rustc = args.rustc.as_ref().expect("fix wrapper rustc was not set");

// Our goal is to fix only the crates that the end user is interested in.
// That's very likely to only mean the crates in the workspace the user is
Expand Down Expand Up @@ -576,6 +561,7 @@ struct FixArgs {
enabled_edition: Option<String>,
other: Vec<OsString>,
primary_package: bool,
rustc: Option<PathBuf>,
}

enum PrepareFor {
Expand All @@ -593,7 +579,8 @@ impl Default for PrepareFor {
impl FixArgs {
fn get() -> FixArgs {
let mut ret = FixArgs::default();
for arg in env::args_os().skip(1) {
ret.rustc = env::args_os().nth(1).map(PathBuf::from);
for arg in env::args_os().skip(2) {
let path = PathBuf::from(arg);
if path.extension().and_then(|s| s.to_str()) == Some("rs") && path.exists() {
ret.file = Some(path);
Expand Down
12 changes: 6 additions & 6 deletions src/cargo/util/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use serde::Deserialize;
use serde::{de, de::IntoDeserializer};
use url::Url;

use self::ConfigValue as CV;
use crate::core::profiles::ConfigProfiles;
use crate::core::shell::Verbosity;
use crate::core::{CliUnstable, Shell, SourceId, Workspace};
Expand All @@ -30,7 +31,6 @@ use crate::util::Filesystem;
use crate::util::Rustc;
use crate::util::ToUrl;
use crate::util::{paths, validate_package_name};
use self::ConfigValue as CV;

/// Configuration information for cargo. This is not specific to a build, it is information
/// relating to cargo itself.
Expand Down Expand Up @@ -191,15 +191,16 @@ impl Config {
}

/// Gets the path to the `rustc` executable.
pub fn rustc(&self, ws: Option<&Workspace<'_>>) -> CargoResult<Rustc> {
pub fn load_global_rustc(&self, ws: Option<&Workspace<'_>>) -> CargoResult<Rustc> {
let cache_location = ws.map(|ws| {
ws.target_dir()
.join(".rustc_info.json")
.into_path_unlocked()
});
let wrapper = self.maybe_get_tool("rustc_wrapper")?;
Rustc::new(
self.get_tool("rustc")?,
self.maybe_get_tool("rustc_wrapper")?,
wrapper,
&self
.home()
.join("bin")
Expand Down Expand Up @@ -755,7 +756,7 @@ impl Config {

/// Looks for a path for `tool` in an environment variable or config path, defaulting to `tool`
/// as a path.
fn get_tool(&self, tool: &str) -> CargoResult<PathBuf> {
pub fn get_tool(&self, tool: &str) -> CargoResult<PathBuf> {
self.maybe_get_tool(tool)
.map(|t| t.unwrap_or_else(|| PathBuf::from(tool)))
}
Expand Down Expand Up @@ -923,8 +924,7 @@ impl ConfigError {
}
}

impl std::error::Error for ConfigError {
}
impl std::error::Error for ConfigError {}

// Future note: currently, we cannot override `Fail::cause` (due to
// specialization) so we have no way to return the underlying causes. In the
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub mod process_builder;
pub mod profile;
mod progress;
mod read2;
mod rustc;
pub mod rustc;
mod sha256;
pub mod to_semver;
pub mod to_url;
Expand Down
5 changes: 1 addition & 4 deletions src/cargo/util/process_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,7 @@ impl ProcessBuilder {

/// (chainable) Replaces the args list with the given `args`.
pub fn args_replace<T: AsRef<OsStr>>(&mut self, args: &[T]) -> &mut ProcessBuilder {
self.args = args
.iter()
.map(|t| t.as_ref().to_os_string())
.collect();
self.args = args.iter().map(|t| t.as_ref().to_os_string()).collect();
self
}

Expand Down
Loading

0 comments on commit 025b01e

Please sign in to comment.