From cc779c8050a863328ddf5c0197ba509473a54be7 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sun, 28 Jul 2019 06:41:13 +0200 Subject: [PATCH 1/6] dev-fmt: better error handling Check if rustfmt is installed at the start and exit if it isn't. --- clippy_dev/src/fmt.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/clippy_dev/src/fmt.rs b/clippy_dev/src/fmt.rs index 5ccdbec14288..3b5d6d2dbb4c 100644 --- a/clippy_dev/src/fmt.rs +++ b/clippy_dev/src/fmt.rs @@ -10,6 +10,7 @@ pub enum CliError { CommandFailed(String), IoError(io::Error), ProjectRootNotFound, + RustfmtNotInstalled, WalkDirError(walkdir::Error), } @@ -36,6 +37,8 @@ pub fn run(check: bool, verbose: bool) { let project_root = project_root()?; + rustfmt_test(context)?; + success &= cargo_fmt(context, project_root.as_path())?; success &= cargo_fmt(context, &project_root.join("clippy_dev"))?; success &= cargo_fmt(context, &project_root.join("rustc_tools_util"))?; @@ -69,6 +72,9 @@ pub fn run(check: bool, verbose: bool) { CliError::ProjectRootNotFound => { eprintln!("error: Can't determine root of project. Please run inside a Clippy working dir."); }, + CliError::RustfmtNotInstalled => { + eprintln!("error: rustfmt nightly is not installed."); + }, CliError::WalkDirError(err) => { eprintln!("error: {}", err); }, @@ -139,6 +145,29 @@ fn cargo_fmt(context: &FmtContext, path: &Path) -> Result { Ok(success) } +fn rustfmt_test(context: &FmtContext) -> Result<(), CliError> { + let program = "rustfmt"; + let dir = std::env::current_dir()?; + let args = &["+nightly", "--version"]; + + if context.verbose { + println!("{}", format_command(&program, &dir, args)); + } + + let output = Command::new(&program).current_dir(&dir).args(args.iter()).output()?; + + if output.status.success() { + Ok(()) + } else if std::str::from_utf8(&output.stderr) + .unwrap_or("") + .starts_with("error: 'rustfmt' is not installed") + { + Err(CliError::RustfmtNotInstalled) + } else { + Err(CliError::CommandFailed(format_command(&program, &dir, args))) + } +} + fn rustfmt(context: &FmtContext, path: &Path) -> Result { let mut args = vec!["+nightly".as_ref(), path.as_os_str()]; if context.check { From dae40767601ee9aa647b442130993bd8ad664f9f Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sun, 28 Jul 2019 06:41:25 +0200 Subject: [PATCH 2/6] Skip fmt test if rustfmt is unavailble --- tests/fmt.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/fmt.rs b/tests/fmt.rs index 4df93cf5a085..65a1c50e46da 100644 --- a/tests/fmt.rs +++ b/tests/fmt.rs @@ -1,3 +1,5 @@ +use std::process::Command; + #[test] #[ignore] fn fmt() { @@ -5,9 +7,20 @@ fn fmt() { return; } + // Skip this test if rustup nightly is unavailable + let rustup_output = Command::new("rustup") + .args(&["component", "list", "--toolchain", "nightly"]) + .output() + .unwrap(); + assert!(rustup_output.status.success()); + let component_output = String::from_utf8_lossy(&rustup_output.stdout); + if !component_output.contains("rustfmt") { + return; + } + let root_dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")); let dev_dir = root_dir.join("clippy_dev"); - let output = std::process::Command::new("cargo") + let output = Command::new("cargo") .current_dir(dev_dir) .args(&["+nightly", "run", "--", "fmt", "--check"]) .output() From a9714227bde1ad9e280d087acf513b0420afd871 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sun, 28 Jul 2019 06:41:29 +0200 Subject: [PATCH 3/6] Enable rustfmt tests This reverts commit d73a953db7a8120ec00b31c57d7092aff0f2c10b. --- .travis.yml | 2 +- appveyor.yml | 2 +- tests/fmt.rs | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0b012139d7a7..0a1bd0ea2750 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,7 +22,7 @@ env: install: - | if [ -z ${INTEGRATION} ]; then - # rustup component add rustfmt || cargo install --git https://github.com/rust-lang/rustfmt/ --force + rustup component add rustfmt || cargo install --git https://github.com/rust-lang/rustfmt/ --force if [ "$TRAVIS_OS_NAME" == "linux" ]; then . $HOME/.nvm/nvm.sh nvm install stable diff --git a/appveyor.yml b/appveyor.yml index 4cea8d99ec6d..9f6cc45af81e 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -22,7 +22,7 @@ install: - del rust-toolchain - cargo install rustup-toolchain-install-master --debug || echo "rustup-toolchain-install-master already installed" - rustup-toolchain-install-master %RUSTC_HASH% -f -n master - #- rustup component add rustfmt --toolchain nightly + - rustup component add rustfmt --toolchain nightly - rustup default master - set PATH=%PATH%;C:\Users\appveyor\.rustup\toolchains\master\bin - rustc -V diff --git a/tests/fmt.rs b/tests/fmt.rs index 65a1c50e46da..bc37be84b433 100644 --- a/tests/fmt.rs +++ b/tests/fmt.rs @@ -1,7 +1,6 @@ use std::process::Command; #[test] -#[ignore] fn fmt() { if option_env!("RUSTC_TEST_SUITE").is_some() { return; From 82be293dfd532beb763c022f881bcac974d483a9 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Mon, 29 Jul 2019 07:51:44 +0200 Subject: [PATCH 4/6] Make appveyor build ignore rustfmt unavailability --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 9f6cc45af81e..23268d7e0dec 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -22,7 +22,7 @@ install: - del rust-toolchain - cargo install rustup-toolchain-install-master --debug || echo "rustup-toolchain-install-master already installed" - rustup-toolchain-install-master %RUSTC_HASH% -f -n master - - rustup component add rustfmt --toolchain nightly + - rustup component add rustfmt --toolchain nightly || echo "rustfmt night is unavailable" - rustup default master - set PATH=%PATH%;C:\Users\appveyor\.rustup\toolchains\master\bin - rustc -V From 2682bd112e14716fe128879abefff667c8a68766 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Mon, 29 Jul 2019 07:54:03 +0200 Subject: [PATCH 5/6] Correct typo --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 23268d7e0dec..092a5c80a8eb 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -22,7 +22,7 @@ install: - del rust-toolchain - cargo install rustup-toolchain-install-master --debug || echo "rustup-toolchain-install-master already installed" - rustup-toolchain-install-master %RUSTC_HASH% -f -n master - - rustup component add rustfmt --toolchain nightly || echo "rustfmt night is unavailable" + - rustup component add rustfmt --toolchain nightly || echo "rustfmt nightly is unavailable" - rustup default master - set PATH=%PATH%;C:\Users\appveyor\.rustup\toolchains\master\bin - rustc -V From be646ac0dff10f154855b9dc706294c2a4ee9059 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Thu, 1 Aug 2019 07:09:57 +0200 Subject: [PATCH 6/6] Update formatting --- clippy_lints/src/consts.rs | 4 +--- clippy_lints/src/misc_early.rs | 2 +- clippy_lints/src/open_options.rs | 8 ++++---- clippy_lints/src/redundant_static_lifetimes.rs | 3 ++- clippy_lints/src/use_self.rs | 7 +++---- rustc_tools_util/src/lib.rs | 1 - tests/ui/checked_unwrap/simple_conditionals.rs | 6 ++++-- tests/ui/drop_forget_ref.rs | 1 - tests/ui/use_self.fixed | 2 +- tests/ui/use_self.rs | 2 +- 10 files changed, 17 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/consts.rs b/clippy_lints/src/consts.rs index d18474abdcd4..9d1cc110e5d4 100644 --- a/clippy_lints/src/consts.rs +++ b/clippy_lints/src/consts.rs @@ -65,9 +65,7 @@ impl PartialEq for Constant { f64::from(l).to_bits() == f64::from(r).to_bits() }, (&Self::Bool(l), &Self::Bool(r)) => l == r, - (&Self::Vec(ref l), &Self::Vec(ref r)) | (&Self::Tuple(ref l), &Self::Tuple(ref r)) => { - l == r - }, + (&Self::Vec(ref l), &Self::Vec(ref r)) | (&Self::Tuple(ref l), &Self::Tuple(ref r)) => l == r, (&Self::Repeat(ref lv, ref ls), &Self::Repeat(ref rv, ref rs)) => ls == rs && lv == rv, // TODO: are there inter-type equalities? _ => false, diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index c2e42a0a9933..c9df436dae7e 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -305,7 +305,7 @@ impl EarlyLintPass for MiscEarlyLints { name makes code comprehension and documentation more difficult", arg_name[1..].to_owned() ), - );; + ); } } else { registered_names.insert(arg_name, arg.pat.span); diff --git a/clippy_lints/src/open_options.rs b/clippy_lints/src/open_options.rs index e115769abe8c..5a3af2c3773f 100644 --- a/clippy_lints/src/open_options.rs +++ b/clippy_lints/src/open_options.rs @@ -127,7 +127,7 @@ fn check_open_options(cx: &LateContext<'_, '_>, options: &[(OpenOption, Argument } else { create = true } - create_arg = create_arg || (arg == Argument::True);; + create_arg = create_arg || (arg == Argument::True); }, (OpenOption::Append, arg) => { if append { @@ -140,7 +140,7 @@ fn check_open_options(cx: &LateContext<'_, '_>, options: &[(OpenOption, Argument } else { append = true } - append_arg = append_arg || (arg == Argument::True);; + append_arg = append_arg || (arg == Argument::True); }, (OpenOption::Truncate, arg) => { if truncate { @@ -166,7 +166,7 @@ fn check_open_options(cx: &LateContext<'_, '_>, options: &[(OpenOption, Argument } else { read = true } - read_arg = read_arg || (arg == Argument::True);; + read_arg = read_arg || (arg == Argument::True); }, (OpenOption::Write, arg) => { if write { @@ -179,7 +179,7 @@ fn check_open_options(cx: &LateContext<'_, '_>, options: &[(OpenOption, Argument } else { write = true } - write_arg = write_arg || (arg == Argument::True);; + write_arg = write_arg || (arg == Argument::True); }, } } diff --git a/clippy_lints/src/redundant_static_lifetimes.rs b/clippy_lints/src/redundant_static_lifetimes.rs index bdd48fe86461..4d9fbbca83ed 100644 --- a/clippy_lints/src/redundant_static_lifetimes.rs +++ b/clippy_lints/src/redundant_static_lifetimes.rs @@ -81,7 +81,8 @@ impl EarlyLintPass for RedundantStaticLifetimes { if !in_macro_or_desugar(item.span) { if let ItemKind::Const(ref var_type, _) = item.node { self.visit_type(var_type, cx, "Constants have by default a `'static` lifetime"); - // Don't check associated consts because `'static` cannot be elided on those (issue #2438) + // Don't check associated consts because `'static` cannot be elided on those (issue + // #2438) } if let ItemKind::Static(ref var_type, _, _) = item.node { diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index 3f93e019c667..4e2e97ddf95b 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -222,13 +222,12 @@ impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> { let last_but_one = &path.segments[path.segments.len() - 2]; if last_but_one.ident.name != kw::SelfUpper { let enum_def_id = match path.res { - Res::Def(DefKind::Variant, variant_def_id) => - self.cx.tcx.parent(variant_def_id), + Res::Def(DefKind::Variant, variant_def_id) => self.cx.tcx.parent(variant_def_id), Res::Def(DefKind::Ctor(def::CtorOf::Variant, _), ctor_def_id) => { let variant_def_id = self.cx.tcx.parent(ctor_def_id); variant_def_id.and_then(|def_id| self.cx.tcx.parent(def_id)) - } - _ => None + }, + _ => None, }; if self.item_path.res.opt_def_id() == enum_def_id { diff --git a/rustc_tools_util/src/lib.rs b/rustc_tools_util/src/lib.rs index ec3b69219e7f..92b710614ecb 100644 --- a/rustc_tools_util/src/lib.rs +++ b/rustc_tools_util/src/lib.rs @@ -154,5 +154,4 @@ mod test { "VersionInfo { crate_name: \"rustc_tools_util\", major: 0, minor: 2, patch: 0 }" ); } - } diff --git a/tests/ui/checked_unwrap/simple_conditionals.rs b/tests/ui/checked_unwrap/simple_conditionals.rs index c20c4a7a7009..c080ae82697a 100644 --- a/tests/ui/checked_unwrap/simple_conditionals.rs +++ b/tests/ui/checked_unwrap/simple_conditionals.rs @@ -31,10 +31,12 @@ fn main() { if x.is_ok() { x = Err(()); x.unwrap(); // not unnecessary because of mutation of x - // it will always panic but the lint is not smart enough to see this (it only checks if conditions). + // it will always panic but the lint is not smart enough to see this (it only + // checks if conditions). } else { x = Ok(()); x.unwrap_err(); // not unnecessary because of mutation of x - // it will always panic but the lint is not smart enough to see this (it only checks if conditions). + // it will always panic but the lint is not smart enough to see this (it + // only checks if conditions). } } diff --git a/tests/ui/drop_forget_ref.rs b/tests/ui/drop_forget_ref.rs index b60f373e75e1..aad996c03762 100644 --- a/tests/ui/drop_forget_ref.rs +++ b/tests/ui/drop_forget_ref.rs @@ -78,7 +78,6 @@ fn test_owl_result() -> Result<(), ()> { Ok(()) } - #[allow(dead_code)] fn test_owl_result_2() -> Result { produce_half_owl_error().map_err(|_| ())?; diff --git a/tests/ui/use_self.fixed b/tests/ui/use_self.fixed index ac2a1708b65e..86ed7ca26f94 100644 --- a/tests/ui/use_self.fixed +++ b/tests/ui/use_self.fixed @@ -266,7 +266,7 @@ mod nesting { enum Enum { A, B(u64), - C { field: bool } + C { field: bool }, } impl Enum { fn method() { diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs index 21b5833e56ee..cabb9a7f82ac 100644 --- a/tests/ui/use_self.rs +++ b/tests/ui/use_self.rs @@ -266,7 +266,7 @@ mod nesting { enum Enum { A, B(u64), - C { field: bool } + C { field: bool }, } impl Enum { fn method() {