Auto merge of #3665 - jsgf:master, r=oli-obk

Start making clippy easier to invoke in non-cargo contexts

Clippy (clippy-driver) currently has a couple of strong but unnecessary couplings with cargo. This series:
1. makes detection of check builds more robust, and
2. make clippy-driver use the --sysroot specified on the command line as its internal sysroot.

5 files changed, 97 insertions, 22 deletions

diff --git a/Cargo.toml b/Cargo.toml
index 868f21c1a49..c3710485027 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -34,7 +34,6 @@ path = "src/main.rs"
 
 [[bin]]
 name = "clippy-driver"
-test = false
 path = "src/driver.rs"
 
 [dependencies]
diff --git a/ci/base-tests.sh b/ci/base-tests.sh
index 3ac6bbe6725..20b93276017 100755
--- a/ci/base-tests.sh
+++ b/ci/base-tests.sh
@@ -11,19 +11,45 @@ cargo build --features debugging
 cargo test --features debugging
 # for faster build, share target dir between subcrates
 export CARGO_TARGET_DIR=`pwd`/target/
-cd clippy_lints && cargo test && cd ..
-cd rustc_tools_util && cargo test && cd ..
-cd clippy_dev && cargo test && cd ..
+(cd clippy_lints && cargo test)
+(cd rustc_tools_util && cargo test)
+(cd clippy_dev && cargo test)
 
 # make sure clippy can be called via ./path/to/cargo-clippy
-cd clippy_workspace_tests
-../target/debug/cargo-clippy
-cd ..
+(
+  cd clippy_workspace_tests
+  ../target/debug/cargo-clippy
+)
 
 # Perform various checks for lint registration
 ./util/dev update_lints --check
 cargo +nightly fmt --all -- --check
 
+# Check running clippy-driver without cargo
+(
+  export LD_LIBRARY_PATH=$(rustc --print sysroot)/lib
+
+  # Check sysroot handling
+  sysroot=$(./target/debug/clippy-driver --print sysroot)
+  test $sysroot = $(rustc --print sysroot)
+
+  sysroot=$(./target/debug/clippy-driver --sysroot /tmp --print sysroot)
+  test $sysroot = /tmp
+
+  sysroot=$(SYSROOT=/tmp ./target/debug/clippy-driver --print sysroot)
+  test $sysroot = /tmp
+
+  # Make sure this isn't set - clippy-driver should cope without it
+  unset CARGO_MANIFEST_DIR
+
+  # Run a lint and make sure it produces the expected output. It's also expected to exit with code 1
+  # XXX How to match the clippy invocation in compile-test.rs?
+  ! ./target/debug/clippy-driver -Dwarnings -Aunused -Zui-testing --emit metadata --crate-type bin tests/ui/cstring.rs 2> cstring.stderr
+  diff <(sed -e 's,tests/ui,$DIR,' -e '/= help/d' cstring.stderr) tests/ui/cstring.stderr
+
+  # TODO: CLIPPY_CONF_DIR / CARGO_MANIFEST_DIR
+)
+
 # make sure tests are formatted
 
 # some lints are sensitive to formatting, exclude some files
diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs
index 09d204a562c..b0b4394ebb8 100644
--- a/clippy_lints/src/utils/conf.rs
+++ b/clippy_lints/src/utils/conf.rs
@@ -163,8 +163,13 @@ pub fn lookup_conf_file() -> io::Result<Option<path::PathBuf>> {
     /// Possible filename to search for.
     const CONFIG_FILE_NAMES: [&str; 2] = [".clippy.toml", "clippy.toml"];
 
-    let mut current = path::PathBuf::from(env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set"));
-
+    // Start looking for a config file in CLIPPY_CONF_DIR, or failing that, CARGO_MANIFEST_DIR.
+    // If neither of those exist, use ".".
+    let mut current = path::PathBuf::from(
+        env::var("CLIPPY_CONF_DIR")
+            .or_else(|_| env::var("CARGO_MANIFEST_DIR"))
+            .unwrap_or_else(|_| ".".to_string()),
+    );
     loop {
         for config_file_name in &CONFIG_FILE_NAMES {
             let config_file = current.join(config_file_name);
diff --git a/src/driver.rs b/src/driver.rs
index b41895f3c2b..fbff693f887 100644
--- a/src/driver.rs
+++ b/src/driver.rs
@@ -20,6 +20,46 @@ fn show_version() {
     println!(env!("CARGO_PKG_VERSION"));
 }
 
+/// If a command-line option matches `find_arg`, then apply the predicate `pred` on its value. If
+/// true, then return it. The parameter is assumed to be either `--arg=value` or `--arg value`.
+fn arg_value<'a>(
+    args: impl IntoIterator<Item = &'a String>,
+    find_arg: &str,
+    pred: impl Fn(&str) -> bool,
+) -> Option<&'a str> {
+    let mut args = args.into_iter().map(String::as_str);
+
+    while let Some(arg) = args.next() {
+        let arg: Vec<_> = arg.splitn(2, '=').collect();
+        if arg.get(0) != Some(&find_arg) {
+            continue;
+        }
+
+        let value = arg.get(1).cloned().or_else(|| args.next());
+        if value.as_ref().map_or(false, |p| pred(p)) {
+            return value;
+        }
+    }
+    None
+}
+
+#[test]
+fn test_arg_value() {
+    let args: Vec<_> = ["--bar=bar", "--foobar", "123", "--foo"]
+        .iter()
+        .map(|s| s.to_string())
+        .collect();
+
+    assert_eq!(arg_value(None, "--foobar", |_| true), None);
+    assert_eq!(arg_value(&args, "--bar", |_| false), None);
+    assert_eq!(arg_value(&args, "--bar", |_| true), Some("bar"));
+    assert_eq!(arg_value(&args, "--bar", |p| p == "bar"), Some("bar"));
+    assert_eq!(arg_value(&args, "--bar", |p| p == "foo"), None);
+    assert_eq!(arg_value(&args, "--foobar", |p| p == "foo"), None);
+    assert_eq!(arg_value(&args, "--foobar", |p| p == "123"), Some("123"));
+    assert_eq!(arg_value(&args, "--foo", |_| true), None);
+}
+
 #[allow(clippy::too_many_lines)]
 pub fn main() {
     rustc_driver::init_rustc_env_logger();
@@ -32,8 +72,19 @@ pub fn main() {
                 exit(0);
             }
 
-            let sys_root = option_env!("SYSROOT")
-                .map(String::from)
+            let mut orig_args: Vec<String> = env::args().collect();
+
+            // Get the sysroot, looking from most specific to this invocation to the least:
+            // - command line
+            // - runtime environment
+            //    - SYSROOT
+            //    - RUSTUP_HOME, MULTIRUST_HOME, RUSTUP_TOOLCHAIN, MULTIRUST_TOOLCHAIN
+            // - sysroot from rustc in the path
+            // - compile-time environment
+            let sys_root_arg = arg_value(&orig_args, "--sysroot", |_| true);
+            let have_sys_root_arg = sys_root_arg.is_some();
+            let sys_root = sys_root_arg
+                .map(|s| s.to_string())
                 .or_else(|| std::env::var("SYSROOT").ok())
                 .or_else(|| {
                     let home = option_env!("RUSTUP_HOME").or(option_env!("MULTIRUST_HOME"));
@@ -49,11 +100,11 @@ pub fn main() {
                         .and_then(|out| String::from_utf8(out.stdout).ok())
                         .map(|s| s.trim().to_owned())
                 })
+                .or_else(|| option_env!("SYSROOT").map(String::from))
                 .expect("need to specify SYSROOT env var during clippy compilation, or use rustup or multirust");
 
             // Setting RUSTC_WRAPPER causes Cargo to pass 'rustc' as the first argument.
             // We're invoking the compiler programmatically, so we ignore this/
-            let mut orig_args: Vec<String> = env::args().collect();
             if orig_args.len() <= 1 {
                 std::process::exit(1);
             }
@@ -64,7 +115,7 @@ pub fn main() {
             // this conditional check for the --sysroot flag is there so users can call
             // `clippy_driver` directly
             // without having to pass --sysroot or anything
-            let mut args: Vec<String> = if orig_args.iter().any(|s| s == "--sysroot") {
+            let mut args: Vec<String> = if have_sys_root_arg {
                 orig_args.clone()
             } else {
                 orig_args
@@ -79,7 +130,7 @@ pub fn main() {
             // crate is
             // linted but not built
             let clippy_enabled = env::var("CLIPPY_TESTS").ok().map_or(false, |val| val == "true")
-                || orig_args.iter().any(|s| s == "--emit=dep-info,metadata");
+                || arg_value(&orig_args, "--emit", |val| val.split(',').any(|e| e == "metadata")).is_some();
 
             if clippy_enabled {
                 args.extend_from_slice(&["--cfg".to_owned(), r#"feature="cargo-clippy""#.to_owned()]);
diff --git a/src/main.rs b/src/main.rs
index 20466fc567d..208262ca30f 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -61,10 +61,8 @@ where
 {
     let mut args = vec!["check".to_owned()];
 
-    let mut found_dashes = false;
     for arg in old_args.by_ref() {
-        found_dashes |= arg == "--";
-        if found_dashes {
+        if arg == "--" {
             break;
         }
         args.push(arg);
@@ -82,11 +80,7 @@ where
     let target_dir = std::env::var_os("CLIPPY_DOGFOOD")
         .map(|_| {
             std::env::var_os("CARGO_MANIFEST_DIR").map_or_else(
-                || {
-                    let mut fallback = std::ffi::OsString::new();
-                    fallback.push("clippy_dogfood");
-                    fallback
-                },
+                || std::ffi::OsString::from("clippy_dogfood"),
                 |d| {
                     std::path::PathBuf::from(d)
                         .join("target")