Auto merge of #42896 - llogiq:clippy_compiletest, r=alexcrichton

fixed some clippy warnings in compiletest

This is mainly readability stuff. Whenever the `clone_ref` lint asked me to clone the dereferenced object, I removed the `.clone()` instead, relying on the fact that it has worked so far and the immutable borrow ensures that the value won't change.

src/tools/compiletest/src/errors.rs

@@ -35,7 +35,7 @@ fn from_str(s: &str) -> Result<Self, Self::Err> {
             "ERROR" => Ok(ErrorKind::Error),
             "NOTE" => Ok(ErrorKind::Note),
             "SUGGESTION" => Ok(ErrorKind::Suggestion),
-            "WARN" => Ok(ErrorKind::Warning),
+            "WARN" |
             "WARNING" => Ok(ErrorKind::Warning),
             _ => Err(()),
         }
@@ -95,7 +95,7 @@ pub fn load_errors(testfile: &Path, cfg: Option<&str>) -> Vec<Error> {
 
     let tag = match cfg {
         Some(rev) => format!("//[{}]~", rev),
-        None => format!("//~"),
+        None => "//~".to_string(),
     };
 
     rdr.lines()
@@ -153,7 +153,7 @@ fn parse_expected(last_nonfollow_error: Option<usize>,
     let msg = msg.trim().to_owned();
 
     let (which, line_num) = if follow {
-        assert!(adjusts == 0, "use either //~| or //~^, not both.");
+        assert_eq!(adjusts, 0, "use either //~| or //~^, not both.");
         let line_num = last_nonfollow_error.expect("encountered //~| without \
                                                     preceding //~^ line.");
         (FollowPrevious(line_num), line_num)

src/tools/compiletest/src/header.rs

@@ -258,7 +258,7 @@ pub fn new() -> Self {
             check_stdout: false,
             no_prefer_dynamic: false,
             pretty_expanded: false,
-            pretty_mode: format!("normal"),
+            pretty_mode: "normal".to_string(),
             pretty_compare_only: false,
             forbid_output: vec![],
             incremental_dir: None,
@@ -381,14 +381,11 @@ pub fn load_from(&mut self,
             }
         });
 
-        for key in vec!["RUST_TEST_NOCAPTURE", "RUST_TEST_THREADS"] {
+        for key in &["RUST_TEST_NOCAPTURE", "RUST_TEST_THREADS"] {
-            match env::var(key) {
+            if let Ok(val) = env::var(key) {
-                Ok(val) => {
+                if self.exec_env.iter().find(|&&(ref x, _)| x == key).is_none() {
-                    if self.exec_env.iter().find(|&&(ref x, _)| *x == key).is_none() {
+                    self.exec_env.push(((*key).to_owned(), val))
-                        self.exec_env.push((key.to_owned(), val))
-                    }
                 }
-                Err(..) => {}
             }
         }
     }
@@ -409,7 +406,7 @@ fn iter_header(testfile: &Path, cfg: Option<&str>, it: &mut FnMut(&str)) {
             return;
         } else if ln.starts_with("//[") {
             // A comment like `//[foo]` is specific to revision `foo`
-            if let Some(close_brace) = ln.find("]") {
+            if let Some(close_brace) = ln.find(']') {
                 let lncfg = &ln[3..close_brace];
                 let matches = match cfg {
                     Some(s) => s == &lncfg[..],
@@ -521,12 +518,10 @@ fn parse_env(&self, line: &str, name: &str) -> Option<(String, String)> {
     fn parse_pp_exact(&self, line: &str, testfile: &Path) -> Option<PathBuf> {
         if let Some(s) = self.parse_name_value_directive(line, "pp-exact") {
             Some(PathBuf::from(&s))
+        } else if self.parse_name_directive(line, "pp-exact") {
+            testfile.file_name().map(PathBuf::from)
         } else {
-            if self.parse_name_directive(line, "pp-exact") {
+            None
-                testfile.file_name().map(PathBuf::from)
-            } else {
-                None
-            }
         }
     }
 
@@ -554,9 +549,7 @@ pub fn parse_name_value_directive(&self, line: &str, directive: &str) -> Option<
 pub fn lldb_version_to_int(version_string: &str) -> isize {
     let error_string = format!("Encountered LLDB version string with unexpected format: {}",
                                version_string);
-    let error_string = error_string;
+    version_string.parse().expect(&error_string)
-    let major: isize = version_string.parse().ok().expect(&error_string);
-    return major;
 }
 
 fn expand_variables(mut value: String, config: &Config) -> String {

src/tools/compiletest/src/json.rs

@@ -65,7 +65,7 @@ pub fn parse_output(file_name: &str, output: &str, proc_res: &ProcRes) -> Vec<Er
 fn parse_line(file_name: &str, line: &str, output: &str, proc_res: &ProcRes) -> Vec<Error> {
     // The compiler sometimes intermingles non-JSON stuff into the
     // output.  This hack just skips over such lines. Yuck.
-    if line.chars().next() == Some('{') {
+    if line.starts_with('{') {
         match json::decode::<Diagnostic>(line) {
             Ok(diagnostic) => {
                 let mut expected_errors = vec![];

src/tools/compiletest/src/main.rs

@@ -168,7 +168,7 @@ fn make_absolute(path: PathBuf) -> PathBuf {
         src_base: opt_path(matches, "src-base"),
         build_base: opt_path(matches, "build-base"),
         stage_id: matches.opt_str("stage-id").unwrap(),
-        mode: matches.opt_str("mode").unwrap().parse().ok().expect("invalid mode"),
+        mode: matches.opt_str("mode").unwrap().parse().expect("invalid mode"),
         run_ignored: matches.opt_present("ignored"),
         filter: matches.free.first().cloned(),
         filter_exact: matches.opt_present("exact"),
@@ -208,7 +208,7 @@ fn make_absolute(path: PathBuf) -> PathBuf {
 
 pub fn log_config(config: &Config) {
     let c = config;
-    logv(c, format!("configuration:"));
+    logv(c, "configuration:".to_string());
     logv(c, format!("compile_lib_path: {:?}", config.compile_lib_path));
     logv(c, format!("run_lib_path: {:?}", config.run_lib_path));
     logv(c, format!("rustc_path: {:?}", config.rustc_path.display()));
@@ -238,10 +238,10 @@ pub fn log_config(config: &Config) {
                     config.adb_device_status));
     logv(c, format!("verbose: {}", config.verbose));
     logv(c, format!("quiet: {}", config.quiet));
-    logv(c, format!("\n"));
+    logv(c, "\n".to_string());
 }
 
-pub fn opt_str<'a>(maybestr: &'a Option<String>) -> &'a str {
+pub fn opt_str(maybestr: &Option<String>) -> &str {
     match *maybestr {
         None => "(none)",
         Some(ref s) => s,
@@ -465,11 +465,9 @@ pub fn make_test(config: &Config, testpaths: &TestPaths) -> test::TestDescAndFn
     };
 
     // Debugging emscripten code doesn't make sense today
-    let mut ignore = early_props.ignore || !up_to_date(config, testpaths, &early_props);
+    let ignore = early_props.ignore || !up_to_date(config, testpaths, &early_props) ||
-    if (config.mode == DebugInfoGdb || config.mode == DebugInfoLldb) &&
+                 (config.mode == DebugInfoGdb || config.mode == DebugInfoLldb) &&
-        config.target.contains("emscripten") {
+                  config.target.contains("emscripten");
-        ignore = true;
-    }
 
     test::TestDescAndFn {
         desc: test::TestDesc {
@@ -488,7 +486,7 @@ fn stamp(config: &Config, testpaths: &TestPaths) -> PathBuf {
                                            .to_str().unwrap(),
                              config.stage_id);
     config.build_base.canonicalize()
-          .unwrap_or(config.build_base.clone())
+          .unwrap_or_else(|_| config.build_base.clone())
           .join(stamp_name)
 }
 
@@ -513,7 +511,7 @@ fn up_to_date(config: &Config, testpaths: &TestPaths, props: &EarlyProps) -> boo
 fn mtime(path: &Path) -> FileTime {
     fs::metadata(path).map(|f| {
         FileTime::from_last_modification_time(&f)
-    }).unwrap_or(FileTime::zero())
+    }).unwrap_or_else(|_| FileTime::zero())
 }
 
 pub fn make_test_name(config: &Config, testpaths: &TestPaths) -> test::TestName {
@@ -561,7 +559,7 @@ fn analyze_gdb(gdb: Option<String>) -> (Option<String>, Option<u32>, bool) {
 
     let gdb_native_rust = version.map_or(false, |v| v >= MIN_GDB_WITH_RUST);
 
-    return (Some(gdb.to_owned()), version, gdb_native_rust);
+    (Some(gdb.to_owned()), version, gdb_native_rust)
 }
 
 fn extract_gdb_version(full_version_line: &str) -> Option<u32> {
@@ -601,7 +599,8 @@ fn extract_gdb_version(full_version_line: &str) -> Option<u32> {
             Some(idx) => if line.as_bytes()[idx] == b'.' {
                 let patch = &line[idx + 1..];
 
-                let patch_len = patch.find(|c: char| !c.is_digit(10)).unwrap_or(patch.len());
+                let patch_len = patch.find(|c: char| !c.is_digit(10))
+                                                       .unwrap_or_else(|| patch.len());
                 let patch = &patch[..patch_len];
                 let patch = if patch_len > 3 || patch_len == 0 { None } else { Some(patch) };
 

src/tools/compiletest/src/procsrv.rs

@@ -9,7 +9,6 @@
 // except according to those terms.
 
 use std::env;
-use std::ffi::OsString;
 use std::io::prelude::*;
 use std::io;
 use std::path::PathBuf;
@@ -31,7 +30,7 @@ fn add_target_env(cmd: &mut Command, lib_path: &str, aux_path: Option<&str>) {
     // Need to be sure to put both the lib_path and the aux path in the dylib
     // search path for the child.
     let var = dylib_env_var();
-    let mut path = env::split_paths(&env::var_os(var).unwrap_or(OsString::new()))
+    let mut path = env::split_paths(&env::var_os(var).unwrap_or_default())
         .collect::<Vec<_>>();
     if let Some(p) = aux_path {
         path.insert(0, PathBuf::from(p))

src/tools/compiletest/src/runtest.rs

@@ -68,7 +68,7 @@ pub fn run(config: Config, testpaths: &TestPaths) {
     } else {
         for revision in &base_props.revisions {
             let mut revision_props = base_props.clone();
-            revision_props.load_from(&testpaths.file, Some(&revision), &config);
+            revision_props.load_from(&testpaths.file, Some(revision), &config);
             let rev_cx = TestCx {
                 config: &config,
                 props: &revision_props,
@@ -81,7 +81,7 @@ pub fn run(config: Config, testpaths: &TestPaths) {
 
     base_cx.complete_all();
 
-    File::create(::stamp(&config, &testpaths)).unwrap();
+    File::create(::stamp(&config, testpaths)).unwrap();
 }
 
 struct TestCx<'test> {
@@ -101,9 +101,8 @@ impl<'test> TestCx<'test> {
     /// invoked once before any revisions have been processed
     fn init_all(&self) {
         assert!(self.revision.is_none(), "init_all invoked for a revision");
-        match self.config.mode {
+        if let Incremental = self.config.mode {
-            Incremental => self.init_incremental_test(),
+            self.init_incremental_test()
-            _ => { }
         }
     }
 
@@ -111,7 +110,7 @@ fn init_all(&self) {
     /// revisions, exactly once, with revision == None).
     fn run_revision(&self) {
         match self.config.mode {
-            CompileFail => self.run_cfail_test(),
+            CompileFail |
             ParseFail => self.run_cfail_test(),
             RunFail => self.run_rfail_test(),
             RunPass => self.run_rpass_test(),
@@ -352,10 +351,10 @@ fn make_pp_args(&self,
                             aux_dir.to_str().unwrap().to_owned()];
         args.extend(self.split_maybe_args(&self.config.target_rustcflags));
         args.extend(self.props.compile_flags.iter().cloned());
-        return ProcArgs {
+        ProcArgs {
             prog: self.config.rustc_path.to_str().unwrap().to_owned(),
             args: args,
-        };
+        }
     }
 
     fn compare_source(&self,
@@ -407,17 +406,17 @@ fn make_typecheck_args(&self) -> ProcArgs {
                             aux_dir.to_str().unwrap().to_owned()];
         if let Some(revision) = self.revision {
             args.extend(vec![
-                format!("--cfg"),
+                "--cfg".to_string(),
-                format!("{}", revision),
+                revision.to_string(),
             ]);
         }
         args.extend(self.split_maybe_args(&self.config.target_rustcflags));
         args.extend(self.props.compile_flags.iter().cloned());
         // FIXME (#9639): This needs to handle non-utf8 paths
-        return ProcArgs {
+        ProcArgs {
             prog: self.config.rustc_path.to_str().unwrap().to_owned(),
             args: args,
-        };
+        }
     }
 
     fn run_debuginfo_gdb_test(&self) {
@@ -708,7 +707,7 @@ fn find_rust_src_root(&self) -> Option<PathBuf> {
             }
         }
 
-        return None;
+        None
     }
 
     fn run_debuginfo_lldb_test(&self) {
@@ -875,13 +874,13 @@ fn parse_debugger_commands(&self, debugger_prefixes: &[&str]) -> DebuggerCommand
                     for &(ref command_directive, ref check_directive) in &directives {
                         self.config.parse_name_value_directive(
                             &line,
-                            &command_directive).map(|cmd| {
+                            command_directive).map(|cmd| {
                                 commands.push(cmd)
                             });
 
                         self.config.parse_name_value_directive(
                             &line,
-                            &check_directive).map(|cmd| {
+                            check_directive).map(|cmd| {
                                 check_lines.push(cmd)
                             });
                     }
@@ -962,8 +961,7 @@ fn check_single_line(line: &str, check_line: &str) -> bool {
                 (line, 0)
             };
 
-            for fragment_index in first_fragment .. check_fragments.len() {
+            for current_fragment in &check_fragments[first_fragment..] {
-                let current_fragment = check_fragments[fragment_index];
                 match rest.find(current_fragment) {
                     Some(pos) => {
                         rest = &rest[pos + current_fragment.len() .. ];
@@ -976,7 +974,7 @@ fn check_single_line(line: &str, check_line: &str) -> bool {
                 return false;
             }
 
-            return true;
+            true
         }
     }
 
@@ -1059,7 +1057,7 @@ fn check_expected_errors(&self,
         let expect_note = expected_errors.iter().any(|ee| ee.kind == Some(ErrorKind::Note));
 
         // Parse the JSON output from the compiler and extract out the messages.
-        let actual_errors = json::parse_output(&file_name, &proc_res.stderr, &proc_res);
+        let actual_errors = json::parse_output(&file_name, &proc_res.stderr, proc_res);
         let mut unexpected = Vec::new();
         let mut found = vec![false; expected_errors.len()];
         for actual_error in &actual_errors {
@@ -1092,7 +1090,7 @@ fn check_expected_errors(&self,
                                      .map_or(String::from("message"),
                                              |k| k.to_string()),
                                      actual_error.msg));
-                        unexpected.push(actual_error.clone());
+                        unexpected.push(actual_error);
                     }
                 }
             }
@@ -1110,20 +1108,20 @@ fn check_expected_errors(&self,
                              .map_or("message".into(),
                                      |k| k.to_string()),
                              expected_error.msg));
-                not_found.push(expected_error.clone());
+                not_found.push(expected_error);
             }
         }
 
-        if unexpected.len() > 0 || not_found.len() > 0 {
+        if !unexpected.is_empty() || !not_found.is_empty() {
             self.error(
                 &format!("{} unexpected errors found, {} expected errors not found",
                          unexpected.len(), not_found.len()));
-            print!("status: {}\ncommand: {}\n",
+            println!("status: {}\ncommand: {}",
                    proc_res.status, proc_res.cmdline);
-            if unexpected.len() > 0 {
+            if !unexpected.is_empty() {
                 println!("unexpected errors (from JSON output): {:#?}\n", unexpected);
             }
-            if not_found.len() > 0 {
+            if !not_found.is_empty() {
                 println!("not found errors (from test file): {:#?}\n", not_found);
             }
             panic!();
@@ -1142,9 +1140,9 @@ fn is_unexpected_compiler_message(&self,
         match actual_error.kind {
             Some(ErrorKind::Help) => expect_help,
             Some(ErrorKind::Note) => expect_note,
-            Some(ErrorKind::Error) => true,
+            Some(ErrorKind::Error) |
             Some(ErrorKind::Warning) => true,
-            Some(ErrorKind::Suggestion) => false,
+            Some(ErrorKind::Suggestion) |
             None => false
         }
     }
@@ -1287,7 +1285,8 @@ fn compose_and_run_compiler(&self, args: ProcArgs, input: Option<String>) -> Pro
                                                      self.config);
             let mut crate_type = if aux_props.no_prefer_dynamic {
                 Vec::new()
-            } else {
+            } else if (self.config.target.contains("musl") && !aux_props.force_host) ||
+                      self.config.target.contains("emscripten") {
                 // We primarily compile all auxiliary libraries as dynamic libraries
                 // to avoid code size bloat and large binaries as much as possible
                 // for the test suite (otherwise including libstd statically in all
@@ -1297,13 +1296,9 @@ fn compose_and_run_compiler(&self, args: ProcArgs, input: Option<String>) -> Pro
                 // dynamic libraries so we just go back to building a normal library. Note,
                 // however, that for MUSL if the library is built with `force_host` then
                 // it's ok to be a dylib as the host should always support dylibs.
-                if (self.config.target.contains("musl") && !aux_props.force_host) ||
+                vec!["--crate-type=lib".to_owned()]
-                    self.config.target.contains("emscripten")
+            } else {
-                {
+                vec!["--crate-type=dylib".to_owned()]
-                    vec!["--crate-type=lib".to_owned()]
-                } else {
-                    vec!["--crate-type=dylib".to_owned()]
-                }
             };
             crate_type.extend(extra_link_args.clone());
             let aux_output = {
@@ -1344,7 +1339,7 @@ fn compose_and_run(&self,
                        lib_path: &str,
                        aux_path: Option<&str>,
                        input: Option<String>) -> ProcRes {
-        return self.program_output(lib_path, prog, aux_path, args, procenv, input);
+        self.program_output(lib_path, prog, aux_path, args, procenv, input)
     }
 
     fn make_compile_args(&self,
@@ -1367,7 +1362,7 @@ fn make_compile_args(&self,
         // Optionally prevent default --target if specified in test compile-flags.
         let custom_target = self.props.compile_flags
             .iter()
-            .fold(false, |acc, ref x| acc || x.starts_with("--target"));
+            .fold(false, |acc, x| acc || x.starts_with("--target"));
 
         if !custom_target {
             args.extend(vec![
@@ -1377,14 +1372,14 @@ fn make_compile_args(&self,
 
         if let Some(revision) = self.revision {
             args.extend(vec![
-                format!("--cfg"),
+                "--cfg".to_string(),
-                format!("{}", revision),
+                revision.to_string(),
             ]);
         }
 
         if let Some(ref incremental_dir) = self.props.incremental_dir {
             args.extend(vec![
-                format!("-Z"),
+                "-Z".to_string(),
                 format!("incremental={}", incremental_dir.display()),
             ]);
         }
@@ -1457,10 +1452,10 @@ fn make_compile_args(&self,
             args.extend(self.split_maybe_args(&self.config.target_rustcflags));
         }
         args.extend(self.props.compile_flags.iter().cloned());
-        return ProcArgs {
+        ProcArgs {
             prog: self.config.rustc_path.to_str().unwrap().to_owned(),
             args: args,
-        };
+        }
     }
 
     fn make_lib_name(&self, auxfile: &Path) -> PathBuf {
@@ -1508,10 +1503,10 @@ fn make_run_args(&self) -> ProcArgs {
         args.extend(self.split_maybe_args(&self.props.run_flags));
 
         let prog = args.remove(0);
-        return ProcArgs {
+         ProcArgs {
             prog: prog,
             args: args,
-        };
+        }
     }
 
     fn split_maybe_args(&self, argstr: &Option<String>) -> Vec<String> {
@@ -1558,12 +1553,12 @@ fn program_output(&self,
                          env,
                          input).expect(&format!("failed to exec `{}`", prog));
         self.dump_output(&out, &err);
-        return ProcRes {
+        ProcRes {
             status: status,
             stdout: out,
             stderr: err,
             cmdline: cmdline,
-        };
+        }
     }
 
     fn make_cmdline(&self, libpath: &str, prog: &str, args: &[String]) -> String {
@@ -1764,7 +1759,7 @@ fn run_rustdoc_test(&self) {
             self.fatal_proc_rec("rustdoc failed!", &proc_res);
         }
 
-        if self.props.check_test_line_numbers_match == true {
+        if self.props.check_test_line_numbers_match {
             self.check_rustdoc_test_option(proc_res);
         } else {
             let root = self.find_rust_src_root().unwrap();
@@ -1791,7 +1786,7 @@ fn get_lines<P: AsRef<Path>>(&self, path: &P,
                .filter_map(|(line_nb, line)| {
                    if (line.trim_left().starts_with("pub mod ") ||
                        line.trim_left().starts_with("mod ")) &&
-                      line.ends_with(";") {
+                      line.ends_with(';') {
                        if let Some(ref mut other_files) = other_files {
                            other_files.push(line.rsplit("mod ")
                                       .next()
@@ -1840,7 +1835,7 @@ fn check_rustdoc_test_option(&self, res: ProcRes) {
         }
 
         let mut tested = 0;
-        for _ in res.stdout.split("\n")
+        for _ in res.stdout.split('\n')
                            .filter(|s| s.starts_with("test "))
                            .inspect(|s| {
                                let tmp: Vec<&str> = s.split(" - ").collect();
@@ -1853,7 +1848,7 @@ fn check_rustdoc_test_option(&self, res: ProcRes) {
                                        iter.next();
                                        let line = iter.next()
                                                       .unwrap_or(")")
-                                                      .split(")")
+                                                      .split(')')
                                                       .next()
                                                       .unwrap_or("0")
                                                       .parse()
@@ -1873,7 +1868,7 @@ fn check_rustdoc_test_option(&self, res: ProcRes) {
             self.fatal_proc_rec(&format!("No test has been found... {:?}", files), &res);
         } else {
             for (entry, v) in &files {
-                if v.len() != 0 {
+                if !v.is_empty() {
                     self.fatal_proc_rec(&format!("Not found test at line{} \"{}\":{:?}",
                                                  if v.len() > 1 { "s" } else { "" }, entry, v),
                                         &res);
@@ -1916,11 +1911,10 @@ fn run_codegen_units_test(&self) {
                                                    .find(|ti| ti.name == expected_item.name);
 
             if let Some(actual_item) = actual_item_with_same_name {
-                if !expected_item.codegen_units.is_empty() {
+                if !expected_item.codegen_units.is_empty() &&
-                    // Also check for codegen units
+                   // Also check for codegen units
-                    if expected_item.codegen_units != actual_item.codegen_units {
+                   expected_item.codegen_units != actual_item.codegen_units {
-                        wrong_cgus.push((expected_item.clone(), actual_item.clone()));
+                    wrong_cgus.push((expected_item.clone(), actual_item.clone()));
-                    }
                 }
             } else {
                 missing.push(expected_item.string.clone());
@@ -2005,7 +1999,7 @@ fn str_to_trans_item(s: &str) -> TransItem {
             let cgus = if parts.len() > 1 {
                 let cgus_str = parts[1];
 
-                cgus_str.split(" ")
+                cgus_str.split(' ')
                         .map(str::trim)
                         .filter(|s| !s.is_empty())
                         .map(str::to_owned)
@@ -2323,7 +2317,7 @@ fn check_mir_test_timestamp(&self, test_name: &str, output_file: &Path) {
         }
     }
 
-    fn compare_mir_test_output(&self, test_name: &str, expected_content: &Vec<&str>) {
+    fn compare_mir_test_output(&self, test_name: &str, expected_content: &[&str]) {
         let mut output_file = PathBuf::new();
         output_file.push(self.get_mir_dump_dir());
         output_file.push(test_name);