Skip to content

Commit

Permalink
Auto merge of #5723 - alexcrichton:cargo-fix, r=ehuss
Browse files Browse the repository at this point in the history
Import `cargo fix` directly in to Cargo

This commit imports the `cargo fix` subcommand in rust-lang-nursery/rustfix
directly into Cargo as a subcommand. This should allow us to ease our
distribution story of `cargo fix` as we prepare for the upcoming 2018 edition
release.

It's been attempted here to make the code as idiomatic as possible for Cargo's
own codebase. Additionally all tests from cargo-fix were imported into Cargo's
test suite as well. After this lands and is published in nightly the `cargo-fix`
command in rust-lang-nursery/rustfix will likely be removed.

cc rust-lang/rust#52272
  • Loading branch information
bors committed Jul 17, 2018
2 parents 558eee4 + b02ba37 commit 506eea7
Show file tree
Hide file tree
Showing 20 changed files with 1,984 additions and 40 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ libc = "0.2"
libgit2-sys = "0.7.1"
log = "0.4"
num_cpus = "1.0"
rustfix = "0.4"
same-file = "1"
semver = { version = "0.9.0", features = ["serde"] }
serde = "1.0"
Expand Down
117 changes: 117 additions & 0 deletions src/bin/cargo/commands/fix.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
use command_prelude::*;

use cargo::ops;

pub fn cli() -> App {
subcommand("fix")
.about("Automatically fix lint warnings reported by rustc")
.arg_package_spec(
"Package(s) to fix",
"Fix all packages in the workspace",
"Exclude packages from the fixes",
)
.arg_jobs()
.arg_targets_all(
"Fix only this package's library",
"Fix only the specified binary",
"Fix all binaries",
"Fix only the specified example",
"Fix all examples",
"Fix only the specified test target",
"Fix all tests",
"Fix only the specified bench target",
"Fix all benches",
"Fix all targets (lib and bin targets by default)",
)
.arg_release("Fix artifacts in release mode, with optimizations")
.arg(opt("profile", "Profile to build the selected target for").value_name("PROFILE"))
.arg_features()
.arg_target_triple("Fix for the target triple")
.arg_target_dir()
.arg_manifest_path()
.arg_message_format()
.arg(
Arg::with_name("broken-code")
.long("broken-code")
.help("Fix code even if it already has compiler errors"),
)
.arg(
Arg::with_name("edition")
.long("prepare-for")
.help("Fix warnings in preparation of an edition upgrade")
.takes_value(true)
.possible_values(&["2018"]),
)
.arg(
Arg::with_name("allow-no-vcs")
.long("allow-no-vcs")
.help("Fix code even if a VCS was not detected"),
)
.arg(
Arg::with_name("allow-dirty")
.long("allow-dirty")
.help("Fix code even if the working directory is dirty"),
)
.after_help(
"\
This Cargo subcommmand will automatically take rustc's suggestions from
diagnostics like warnings and apply them to your source code. This is intended
to help automate tasks that rustc itself already knows how to tell you to fix!
The `cargo fix` subcommand is also being developed for the Rust 2018 edition
to provide code the ability to easily opt-in to the new edition without having
to worry about any breakage.
Executing `cargo fix` will under the hood execute `cargo check`. Any warnings
applicable to your crate will be automatically fixed (if possible) and all
remaining warnings will be displayed when the check process is finished. For
example if you'd like to prepare for the 2018 edition, you can do so by
executing:
cargo fix --prepare-for 2018
Note that this is not guaranteed to fix all your code as it only fixes code that
`cargo check` would otherwise compile. For example unit tests are left out
from this command, but they can be checked with:
cargo fix --prepare-for 2018 --all-targets
which behaves the same as `cargo check --all-targets`. Similarly if you'd like
to fix code for different platforms you can do:
cargo fix --prepare-for 2018 --target x86_64-pc-windows-gnu
or if your crate has optional features:
cargo fix --prepare-for 2018 --no-default-features --features foo
If you encounter any problems with `cargo fix` or otherwise have any questions
or feature requests please don't hesitate to file an issue at
https://github.com/rust-lang/cargo
",
)
}

pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
let ws = args.workspace(config)?;
let test = match args.value_of("profile") {
Some("test") => true,
None => false,
Some(profile) => {
let err = format_err!(
"unknown profile: `{}`, only `test` is \
currently supported",
profile
);
return Err(CliError::new(err, 101));
}
};
let mode = CompileMode::Check { test };
ops::fix(&ws, &mut ops::FixOptions {
edition: args.value_of("edition"),
compile_opts: args.compile_options(config, mode)?,
allow_dirty: args.is_present("allow-dirty"),
allow_no_vcs: args.is_present("allow-no-vcs"),
broken_code: args.is_present("broken-code"),
})?;
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 @@ -8,6 +8,7 @@ pub fn builtin() -> Vec<App> {
clean::cli(),
doc::cli(),
fetch::cli(),
fix::cli(),
generate_lockfile::cli(),
git_checkout::cli(),
init::cli(),
Expand Down Expand Up @@ -42,6 +43,7 @@ pub fn builtin_exec(cmd: &str) -> Option<fn(&mut Config, &ArgMatches) -> CliResu
"clean" => clean::exec,
"doc" => doc::exec,
"fetch" => fetch::exec,
"fix" => fix::exec,
"generate-lockfile" => generate_lockfile::exec,
"git-checkout" => git_checkout::exec,
"init" => init::exec,
Expand Down Expand Up @@ -76,6 +78,7 @@ pub mod check;
pub mod clean;
pub mod doc;
pub mod fetch;
pub mod fix;
pub mod generate_lockfile;
pub mod git_checkout;
pub mod init;
Expand Down
12 changes: 8 additions & 4 deletions src/bin/cargo/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,14 @@ fn main() {
}
};

let result = {
init_git_transports(&mut config);
let _token = cargo::util::job::setup();
cli::main(&mut config)
let result = match cargo::ops::fix_maybe_exec_rustc() {
Ok(true) => Ok(()),
Ok(false) => {
init_git_transports(&mut config);
let _token = cargo::util::job::setup();
cli::main(&mut config)
}
Err(e) => Err(CliError::from(e)),
};

match result {
Expand Down
15 changes: 14 additions & 1 deletion src/cargo/core/compiler/build_config.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::path::Path;
use util::{CargoResult, CargoResultExt, Config};
use std::cell::RefCell;

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

/// Configuration information for a rustc build.
#[derive(Debug)]
Expand All @@ -16,6 +18,13 @@ pub struct BuildConfig {
pub message_format: MessageFormat,
/// 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>,
pub rustfix_diagnostic_server: RefCell<Option<RustfixDiagnosticServer>>,
}

impl BuildConfig {
Expand Down Expand Up @@ -71,6 +80,10 @@ impl BuildConfig {
mode,
message_format: MessageFormat::Human,
build_plan: false,
cargo_as_rustc_wrapper: false,
extra_rustc_env: Vec::new(),
extra_rustc_args: Vec::new(),
rustfix_diagnostic_server: RefCell::new(None),
})
}

Expand Down
25 changes: 21 additions & 4 deletions src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::collections::{BTreeSet, HashMap, HashSet};
use std::env;
use std::ffi::OsStr;
use std::path::PathBuf;

Expand Down Expand Up @@ -80,8 +81,24 @@ pub struct Compilation<'cfg> {
}

impl<'cfg> Compilation<'cfg> {
pub fn new<'a>(bcx: &BuildContext<'a, 'cfg>) -> Compilation<'cfg> {
Compilation {
pub fn new<'a>(bcx: &BuildContext<'a, 'cfg>) -> CargoResult<Compilation<'cfg>> {
let mut rustc = bcx.rustc.process();
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);
}
if bcx.build_config.cargo_as_rustc_wrapper {
let prog = rustc.get_program().to_owned();
rustc.env("RUSTC", prog);
rustc.program(env::current_exe()?);
}
let srv = bcx.build_config.rustfix_diagnostic_server.borrow();
if let Some(server) = &*srv {
server.configure(&mut rustc);
}
Ok(Compilation {
libraries: HashMap::new(),
native_dirs: BTreeSet::new(), // TODO: deprecated, remove
root_output: PathBuf::from("/"),
Expand All @@ -96,11 +113,11 @@ impl<'cfg> Compilation<'cfg> {
cfgs: HashMap::new(),
rustdocflags: HashMap::new(),
config: bcx.config,
rustc_process: bcx.rustc.process(),
rustc_process: rustc,
host: bcx.host_triple().to_string(),
target: bcx.target_triple().to_string(),
target_runner: LazyCell::new(),
}
})
}

/// See `process`.
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {

Ok(Self {
bcx,
compilation: Compilation::new(bcx),
compilation: Compilation::new(bcx)?,
build_state: Arc::new(BuildState::new(&bcx.host_config, &bcx.target_config)),
fingerprints: HashMap::new(),
compiled: HashSet::new(),
Expand Down
19 changes: 16 additions & 3 deletions src/cargo/core/compiler/job_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use handle_error;
use util::{internal, profile, CargoResult, CargoResultExt, ProcessBuilder};
use util::{Config, DependencyQueue, Dirty, Fresh, Freshness};
use util::{Progress, ProgressStyle};
use util::diagnostic_server;

use super::job::Job;
use super::{BuildContext, BuildPlan, CompileMode, Context, Kind, Unit};
Expand Down Expand Up @@ -64,6 +65,7 @@ enum Message<'a> {
BuildPlanMsg(String, ProcessBuilder, Arc<Vec<OutputFile>>),
Stdout(String),
Stderr(String),
FixDiagnostic(diagnostic_server::Message),
Token(io::Result<Acquired>),
Finish(Key<'a>, CargoResult<()>),
}
Expand Down Expand Up @@ -134,9 +136,9 @@ impl<'a> JobQueue<'a> {
self.queue.queue_finished();

// We need to give a handle to the send half of our message queue to the
// jobserver helper thread. Unfortunately though we need the handle to be
// `'static` as that's typically what's required when spawning a
// thread!
// jobserver and (optionally) diagnostic helper thread. Unfortunately
// though we need the handle to be `'static` as that's typically what's
// required when spawning a thread!
//
// To work around this we transmute the `Sender` to a static lifetime.
// we're only sending "longer living" messages and we should also
Expand All @@ -148,12 +150,20 @@ impl<'a> JobQueue<'a> {
// practice.
let tx = self.tx.clone();
let tx = unsafe { mem::transmute::<Sender<Message<'a>>, Sender<Message<'static>>>(tx) };
let tx2 = tx.clone();
let helper = cx.jobserver
.clone()
.into_helper_thread(move |token| {
drop(tx.send(Message::Token(token)));
})
.chain_err(|| "failed to create helper thread for jobserver management")?;
let _diagnostic_server = cx.bcx.build_config
.rustfix_diagnostic_server
.borrow_mut()
.take()
.map(move |srv| {
srv.start(move |msg| drop(tx2.send(Message::FixDiagnostic(msg))))
});

crossbeam::scope(|scope| self.drain_the_queue(cx, plan, scope, &helper))
}
Expand Down Expand Up @@ -279,6 +289,9 @@ impl<'a> JobQueue<'a> {
writeln!(cx.bcx.config.shell().err(), "{}", err)?;
}
}
Message::FixDiagnostic(msg) => {
msg.print_to(cx.bcx.config)?;
}
Message::Finish(key, result) => {
info!("end: {:?}", key);

Expand Down
1 change: 1 addition & 0 deletions src/cargo/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ extern crate libgit2_sys;
#[macro_use]
extern crate log;
extern crate num_cpus;
extern crate rustfix;
extern crate same_file;
extern crate semver;
#[macro_use]
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> {
Ok(())
}

fn existing_vcs_repo(path: &Path, cwd: &Path) -> bool {
pub fn existing_vcs_repo(path: &Path, cwd: &Path) -> bool {
GitRepo::discover(path, cwd).is_ok() || HgRepo::discover(path, cwd).is_ok()
}

Expand Down
Loading

0 comments on commit 506eea7

Please sign in to comment.