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.

Author: bors

src/tools/compiletest/src/errors.rs

@@ -35,7 +35,7 @@ impl FromStr for ErrorKind {
             "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 @@ impl TestProps {
             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 @@ impl TestProps {
             }
         });
 
-        for key in vec!["RUST_TEST_NOCAPTURE", "RUST_TEST_THREADS"] {
-            match env::var(key) {
-                Ok(val) => {
-                    if self.exec_env.iter().find(|&&(ref x, _)| *x == key).is_none() {
-                        self.exec_env.push((key.to_owned(), val))
-                    }
+        for key in &["RUST_TEST_NOCAPTURE", "RUST_TEST_THREADS"] {
+            if let Ok(val) = env::var(key) {
+                if self.exec_env.iter().find(|&&(ref x, _)| x == key).is_none() {
+                    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 @@ impl Config {
     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") {
-                testfile.file_name().map(PathBuf::from)
-            } else {
-                None
-            }
+            None
         }
     }
 
@@ -554,9 +549,7 @@ impl Config {
 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;
-    let major: isize = version_string.parse().ok().expect(&error_string);
-    return major;
+    version_string.parse().expect(&error_string)
 }
 
 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 @@ pub fn parse_config(args: Vec<String> ) -> Config {
         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 @@ pub fn parse_config(args: Vec<String> ) -> Config {
 
 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);
-    if (config.mode == DebugInfoGdb || config.mode == DebugInfoLldb) &&
-        config.target.contains("emscripten") {
-        ignore = true;
-    }
+    let ignore = early_props.ignore || !up_to_date(config, testpaths, &early_props) ||
+                 (config.mode == DebugInfoGdb || config.mode == DebugInfoLldb) &&
+                  config.target.contains("emscripten");
 
     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))