Auto merge of rust-lang#6759 - yaahallo:cargo-clippy, r=alexcrichton

Cargo clippy

resolves rust-lang/rust-clippy#3837

src/bin/cargo/cli.rs

@@ -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(());
     }

src/bin/cargo/commands/clippy.rs

@@ -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(())
+}

src/bin/cargo/commands/mod.rs

@@ -6,6 +6,7 @@ pub fn builtin() -> Vec<App> {
         build::cli(),
         check::cli(),
         clean::cli(),
+        clippy::cli(),
         doc::cli(),
         fetch::cli(),
         fix::cli(),
@@ -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,
@@ -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;

src/cargo/core/compiler/build_config.rs

@@ -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.
@@ -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>>,
 }
 
@@ -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),
         })
     }

src/cargo/core/compiler/build_context/mod.rs

@@ -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)?,

src/cargo/core/compiler/compilation.rs

@@ -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);

src/cargo/core/compiler/mod.rs

@@ -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;
@@ -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.
 ///

src/cargo/ops/cargo_fetch.rs

@@ -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)?;
     {

src/cargo/ops/fix.rs

@@ -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";
@@ -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(())
@@ -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
@@ -576,6 +561,7 @@ struct FixArgs {
     enabled_edition: Option<String>,
     other: Vec<OsString>,
     primary_package: bool,
+    rustc: Option<PathBuf>,
 }
 
 enum PrepareFor {
@@ -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);

src/cargo/util/config.rs

@@ -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};
@@ -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.
@@ -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")
@@ -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)))
     }
@@ -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

src/cargo/util/mod.rs

@@ -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;

src/cargo/util/process_builder.rs

@@ -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
     }