From 935d6acb013197f7196d195b6a21b764cdb84735 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 10 Jul 2018 11:23:41 -0700 Subject: [PATCH 1/2] Fix dropping the diagnostic server blocking Previously it would block forever, now it should exit cleanly! --- cargo-fix/src/diagnostics.rs | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/cargo-fix/src/diagnostics.rs b/cargo-fix/src/diagnostics.rs index 9a91980..6d339e2 100644 --- a/cargo-fix/src/diagnostics.rs +++ b/cargo-fix/src/diagnostics.rs @@ -4,6 +4,8 @@ use std::env; use std::io::{BufReader, Read, Write}; use std::net::{Shutdown, SocketAddr, TcpListener, TcpStream}; +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; use std::thread::{self, JoinHandle}; use atty; @@ -64,7 +66,8 @@ pub struct Server { } pub struct StartedServer { - _addr: SocketAddr, + addr: SocketAddr, + done: Arc, thread: Option>, } @@ -81,18 +84,21 @@ impl Server { where F: Fn(Message, &mut StandardStream) + Send + 'static, { - let _addr = self.listener.local_addr()?; + let addr = self.listener.local_addr()?; + let done = Arc::new(AtomicBool::new(false)); + let done2 = done.clone(); let thread = thread::spawn(move || { - self.run(on_message); + self.run(on_message, &done2); }); Ok(StartedServer { - _addr, + addr, thread: Some(thread), + done, }) } - fn run(self, on_message: F) + fn run(self, on_message: F, done: &AtomicBool) where F: Fn(Message, &mut StandardStream), { @@ -105,12 +111,21 @@ impl Server { warn!("invalid diagnostics message: {}", e); } } + + if done.load(Ordering::SeqCst) { + break + } } } } impl Drop for StartedServer { fn drop(&mut self) { + self.done.store(true, Ordering::SeqCst); + // Ignore errors here as this is largely best-effort + if TcpStream::connect(&self.addr).is_err() { + return; + } drop(self.thread.take().unwrap().join()); } } From 6eca16bf2f7231b3b6e7ed9621ee768653beee17 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 10 Jul 2018 11:24:03 -0700 Subject: [PATCH 2/2] Bolster help and add Cargo flags This commit explicitly takes many flags that Cargo takes to filter the package selection and otherwise configure what packages are being built or what targets are being built. The intention here is to take arguments but forward them all to the underlying `cargo check`, but doing so in a first-class fashion to avoid `--` if possible. --- cargo-fix/src/cli.rs | 179 +++++++++++++++++++++++++----- cargo-fix/tests/all/subtargets.rs | 6 +- 2 files changed, 152 insertions(+), 33 deletions(-) diff --git a/cargo-fix/src/cli.rs b/cargo-fix/src/cli.rs index 5dd030b..7361bad 100644 --- a/cargo-fix/src/cli.rs +++ b/cargo-fix/src/cli.rs @@ -21,39 +21,133 @@ static PLEASE_REPORT_THIS_BUG: &str = quoting the full output of this command we'd be very appreciative!\n\n\ "; +enum CargoArg { + OneFlag(&'static str, &'static str), + ManyValue(&'static str, &'static str), + OneValue(&'static str, &'static str), +} + +const CARGO_ARGS: &[CargoArg] = &[ + CargoArg::ManyValue("package", "Package(s) to fix"), + CargoArg::OneFlag("all", "Fix all packages in the workspace"), + CargoArg::ManyValue("exclude", "Exclude packages from the build"), + CargoArg::OneFlag("lib", "Fix only this package's library"), + CargoArg::ManyValue("bin", "Fix only the specified binary"), + CargoArg::OneFlag("bins", "Fix all binaries"), + CargoArg::ManyValue("example", "Fix only the specified example"), + CargoArg::OneFlag("examples", "Fix all examples"), + CargoArg::ManyValue("test", "Fix only the specified test"), + CargoArg::OneFlag("tests", "Fix all tests"), + CargoArg::ManyValue("bench", "Fix only the specified bench"), + CargoArg::OneFlag("benches", "Fix all benchess"), + CargoArg::OneFlag("all-targets", "Fix all targets (lib and bin are default)"), + CargoArg::OneFlag("release", "Fix for release mode"), + CargoArg::OneValue("features", "Space-separated list of features"), + CargoArg::OneFlag("all-features", "Activate all available features"), + CargoArg::OneFlag("no-default-features", "Don't activate `default` feature"), + CargoArg::OneValue("target", "Build for the target triple"), + CargoArg::OneValue("target-dir", "Directory for all generated artifacts"), + CargoArg::OneValue("manifest-path", "Path to Cargo.toml"), +]; + pub fn run() -> Result<(), Error> { + let mut cmd = SubCommand::with_name("fix") + .version(env!("CARGO_PKG_VERSION")) + .author("The Rust Project Developers") + .about("Automatically apply rustc's suggestions about fixing code") + .setting(AppSettings::TrailingVarArg) + .arg( + Arg::with_name("args") + .multiple(true) + .help("Arguments to forward to underlying `cargo check`") + ) + .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-nursery/rustfix +"); + + for arg in CARGO_ARGS { + match arg { + CargoArg::OneFlag(name, help) => { + cmd = cmd.arg(Arg::with_name(name).long(name).help(help)); + } + CargoArg::OneValue(name, help) => { + cmd = cmd.arg( + Arg::with_name(name) + .long(name) + .help(help) + .takes_value(true) + ); + } + CargoArg::ManyValue(name, help) => { + let mut arg = Arg::with_name(name) + .long(name) + .help(help) + .takes_value(true) + .multiple(true); + if *name == "package" { + arg = arg.short("p"); + } + cmd = cmd.arg(arg); + } + } + } let matches = App::new("Cargo Fix") .bin_name("cargo") - .subcommand( - SubCommand::with_name("fix") - .version(env!("CARGO_PKG_VERSION")) - .author("The Rust Project Developers") - .about("Automatically apply rustc's suggestions about fixing code") - .setting(AppSettings::TrailingVarArg) - .arg(Arg::with_name("args").multiple(true)) - .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"), - ), - ) + .subcommand(cmd) .get_matches(); let matches = match matches.subcommand() { @@ -87,6 +181,31 @@ pub fn run() -> Result<(), Error> { // TODO: somehow we need to force `check` to actually do something here, if // `cargo check` was previously run it won't actually do anything again. cmd.arg("check"); + + // Handle all of Cargo's arguments that we parse and forward on to Cargo + for arg in CARGO_ARGS { + match arg { + CargoArg::OneFlag(name, _) => { + if matches.is_present(name) { + cmd.arg(format!("--{}", name)); + } + } + CargoArg::OneValue(name, _) => { + if let Some(value) = matches.value_of(name) { + cmd.arg(format!("--{}", name)).arg(value); + } + } + CargoArg::ManyValue(name, _) => { + if let Some(values) = matches.values_of(name) { + for value in values { + cmd.arg(format!("--{}", name)).arg(value); + } + } + } + } + } + + // Handle the forwarded arguments after `--` if let Some(args) = matches.values_of("args") { cmd.args(args); } diff --git a/cargo-fix/tests/all/subtargets.rs b/cargo-fix/tests/all/subtargets.rs index db87211..6290d62 100644 --- a/cargo-fix/tests/all/subtargets.rs +++ b/cargo-fix/tests/all/subtargets.rs @@ -39,7 +39,7 @@ fn fixes_missing_ampersand() { ) .build(); - p.expect_cmd("cargo fix -- --all-targets") + p.expect_cmd("cargo-fix fix --all-targets") .fix_everything() .stdout("") .stderr_contains("[COMPILING] foo v0.1.0 (CWD)") @@ -84,8 +84,8 @@ fn fix_features() { ) .build(); - p.expect_cmd("cargo fix").run(); + p.expect_cmd("cargo-fix fix").run(); p.expect_cmd("cargo build").run(); - p.expect_cmd("cargo fix -- --features bar").run(); + p.expect_cmd("cargo-fix fix --features bar").run(); p.expect_cmd("cargo build --features bar").run(); }