Auto merge of #4303 - mikerite:dev-fmt-20190728, r=flip1995

Make fmt test and command more robust

changelog: none

Author: bors

.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

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 nightly is unavailable"
     - rustup default master
     - set PATH=%PATH%;C:\Users\appveyor\.rustup\toolchains\master\bin
     - rustc -V

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<bool, CliError> {
     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<bool, CliError> {
     let mut args = vec!["+nightly".as_ref(), path.as_os_str()];
     if context.check {

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,

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

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);
             },
         }
     }

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 {

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 {

rustc_tools_util/src/lib.rs

@@ -154,5 +154,4 @@ mod test {
             "VersionInfo { crate_name: \"rustc_tools_util\", major: 0, minor: 2, patch: 0 }"
         );
     }
-
 }

tests/fmt.rs

@@ -1,13 +1,25 @@
+use std::process::Command;
+
 #[test]
-#[ignore]
 fn fmt() {
     if option_env!("RUSTC_TEST_SUITE").is_some() {
         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()

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

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<u8, ()> {
     produce_half_owl_error().map_err(|_| ())?;

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() {

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() {