Skip to content

Commit e176324

Browse files
committed
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.
2 parents 450cacc + f0131fb commit e176324

File tree

5 files changed

+97
-22
lines changed

5 files changed

+97
-22
lines changed

Cargo.toml

-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ path = "src/main.rs"
3434

3535
[[bin]]
3636
name = "clippy-driver"
37-
test = false
3837
path = "src/driver.rs"
3938

4039
[dependencies]

ci/base-tests.sh

+32-6
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,45 @@ cargo build --features debugging
1111
cargo test --features debugging
1212
# for faster build, share target dir between subcrates
1313
export CARGO_TARGET_DIR=`pwd`/target/
14-
cd clippy_lints && cargo test && cd ..
15-
cd rustc_tools_util && cargo test && cd ..
16-
cd clippy_dev && cargo test && cd ..
14+
(cd clippy_lints && cargo test)
15+
(cd rustc_tools_util && cargo test)
16+
(cd clippy_dev && cargo test)
1717

1818
# make sure clippy can be called via ./path/to/cargo-clippy
19-
cd clippy_workspace_tests
20-
../target/debug/cargo-clippy
21-
cd ..
19+
(
20+
cd clippy_workspace_tests
21+
../target/debug/cargo-clippy
22+
)
2223

2324
# Perform various checks for lint registration
2425
./util/dev update_lints --check
2526
cargo +nightly fmt --all -- --check
2627

28+
# Check running clippy-driver without cargo
29+
(
30+
export LD_LIBRARY_PATH=$(rustc --print sysroot)/lib
31+
32+
# Check sysroot handling
33+
sysroot=$(./target/debug/clippy-driver --print sysroot)
34+
test $sysroot = $(rustc --print sysroot)
35+
36+
sysroot=$(./target/debug/clippy-driver --sysroot /tmp --print sysroot)
37+
test $sysroot = /tmp
38+
39+
sysroot=$(SYSROOT=/tmp ./target/debug/clippy-driver --print sysroot)
40+
test $sysroot = /tmp
41+
42+
# Make sure this isn't set - clippy-driver should cope without it
43+
unset CARGO_MANIFEST_DIR
44+
45+
# Run a lint and make sure it produces the expected output. It's also expected to exit with code 1
46+
# XXX How to match the clippy invocation in compile-test.rs?
47+
! ./target/debug/clippy-driver -Dwarnings -Aunused -Zui-testing --emit metadata --crate-type bin tests/ui/cstring.rs 2> cstring.stderr
48+
diff <(sed -e 's,tests/ui,$DIR,' -e '/= help/d' cstring.stderr) tests/ui/cstring.stderr
49+
50+
# TODO: CLIPPY_CONF_DIR / CARGO_MANIFEST_DIR
51+
)
52+
2753
# make sure tests are formatted
2854

2955
# some lints are sensitive to formatting, exclude some files

clippy_lints/src/utils/conf.rs

+7-2
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,13 @@ pub fn lookup_conf_file() -> io::Result<Option<path::PathBuf>> {
163163
/// Possible filename to search for.
164164
const CONFIG_FILE_NAMES: [&str; 2] = [".clippy.toml", "clippy.toml"];
165165

166-
let mut current = path::PathBuf::from(env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set"));
167-
166+
// Start looking for a config file in CLIPPY_CONF_DIR, or failing that, CARGO_MANIFEST_DIR.
167+
// If neither of those exist, use ".".
168+
let mut current = path::PathBuf::from(
169+
env::var("CLIPPY_CONF_DIR")
170+
.or_else(|_| env::var("CARGO_MANIFEST_DIR"))
171+
.unwrap_or_else(|_| ".".to_string()),
172+
);
168173
loop {
169174
for config_file_name in &CONFIG_FILE_NAMES {
170175
let config_file = current.join(config_file_name);

src/driver.rs

+56-5
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,46 @@ fn show_version() {
2020
println!(env!("CARGO_PKG_VERSION"));
2121
}
2222

23+
/// If a command-line option matches `find_arg`, then apply the predicate `pred` on its value. If
24+
/// true, then return it. The parameter is assumed to be either `--arg=value` or `--arg value`.
25+
fn arg_value<'a>(
26+
args: impl IntoIterator<Item = &'a String>,
27+
find_arg: &str,
28+
pred: impl Fn(&str) -> bool,
29+
) -> Option<&'a str> {
30+
let mut args = args.into_iter().map(String::as_str);
31+
32+
while let Some(arg) = args.next() {
33+
let arg: Vec<_> = arg.splitn(2, '=').collect();
34+
if arg.get(0) != Some(&find_arg) {
35+
continue;
36+
}
37+
38+
let value = arg.get(1).cloned().or_else(|| args.next());
39+
if value.as_ref().map_or(false, |p| pred(p)) {
40+
return value;
41+
}
42+
}
43+
None
44+
}
45+
46+
#[test]
47+
fn test_arg_value() {
48+
let args: Vec<_> = ["--bar=bar", "--foobar", "123", "--foo"]
49+
.iter()
50+
.map(|s| s.to_string())
51+
.collect();
52+
53+
assert_eq!(arg_value(None, "--foobar", |_| true), None);
54+
assert_eq!(arg_value(&args, "--bar", |_| false), None);
55+
assert_eq!(arg_value(&args, "--bar", |_| true), Some("bar"));
56+
assert_eq!(arg_value(&args, "--bar", |p| p == "bar"), Some("bar"));
57+
assert_eq!(arg_value(&args, "--bar", |p| p == "foo"), None);
58+
assert_eq!(arg_value(&args, "--foobar", |p| p == "foo"), None);
59+
assert_eq!(arg_value(&args, "--foobar", |p| p == "123"), Some("123"));
60+
assert_eq!(arg_value(&args, "--foo", |_| true), None);
61+
}
62+
2363
#[allow(clippy::too_many_lines)]
2464
pub fn main() {
2565
rustc_driver::init_rustc_env_logger();
@@ -32,8 +72,19 @@ pub fn main() {
3272
exit(0);
3373
}
3474

35-
let sys_root = option_env!("SYSROOT")
36-
.map(String::from)
75+
let mut orig_args: Vec<String> = env::args().collect();
76+
77+
// Get the sysroot, looking from most specific to this invocation to the least:
78+
// - command line
79+
// - runtime environment
80+
// - SYSROOT
81+
// - RUSTUP_HOME, MULTIRUST_HOME, RUSTUP_TOOLCHAIN, MULTIRUST_TOOLCHAIN
82+
// - sysroot from rustc in the path
83+
// - compile-time environment
84+
let sys_root_arg = arg_value(&orig_args, "--sysroot", |_| true);
85+
let have_sys_root_arg = sys_root_arg.is_some();
86+
let sys_root = sys_root_arg
87+
.map(|s| s.to_string())
3788
.or_else(|| std::env::var("SYSROOT").ok())
3889
.or_else(|| {
3990
let home = option_env!("RUSTUP_HOME").or(option_env!("MULTIRUST_HOME"));
@@ -49,11 +100,11 @@ pub fn main() {
49100
.and_then(|out| String::from_utf8(out.stdout).ok())
50101
.map(|s| s.trim().to_owned())
51102
})
103+
.or_else(|| option_env!("SYSROOT").map(String::from))
52104
.expect("need to specify SYSROOT env var during clippy compilation, or use rustup or multirust");
53105

54106
// Setting RUSTC_WRAPPER causes Cargo to pass 'rustc' as the first argument.
55107
// We're invoking the compiler programmatically, so we ignore this/
56-
let mut orig_args: Vec<String> = env::args().collect();
57108
if orig_args.len() <= 1 {
58109
std::process::exit(1);
59110
}
@@ -64,7 +115,7 @@ pub fn main() {
64115
// this conditional check for the --sysroot flag is there so users can call
65116
// `clippy_driver` directly
66117
// without having to pass --sysroot or anything
67-
let mut args: Vec<String> = if orig_args.iter().any(|s| s == "--sysroot") {
118+
let mut args: Vec<String> = if have_sys_root_arg {
68119
orig_args.clone()
69120
} else {
70121
orig_args
@@ -79,7 +130,7 @@ pub fn main() {
79130
// crate is
80131
// linted but not built
81132
let clippy_enabled = env::var("CLIPPY_TESTS").ok().map_or(false, |val| val == "true")
82-
|| orig_args.iter().any(|s| s == "--emit=dep-info,metadata");
133+
|| arg_value(&orig_args, "--emit", |val| val.split(',').any(|e| e == "metadata")).is_some();
83134

84135
if clippy_enabled {
85136
args.extend_from_slice(&["--cfg".to_owned(), r#"feature="cargo-clippy""#.to_owned()]);

src/main.rs

+2-8
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,8 @@ where
6161
{
6262
let mut args = vec!["check".to_owned()];
6363

64-
let mut found_dashes = false;
6564
for arg in old_args.by_ref() {
66-
found_dashes |= arg == "--";
67-
if found_dashes {
65+
if arg == "--" {
6866
break;
6967
}
7068
args.push(arg);
@@ -82,11 +80,7 @@ where
8280
let target_dir = std::env::var_os("CLIPPY_DOGFOOD")
8381
.map(|_| {
8482
std::env::var_os("CARGO_MANIFEST_DIR").map_or_else(
85-
|| {
86-
let mut fallback = std::ffi::OsString::new();
87-
fallback.push("clippy_dogfood");
88-
fallback
89-
},
83+
|| std::ffi::OsString::from("clippy_dogfood"),
9084
|d| {
9185
std::path::PathBuf::from(d)
9286
.join("target")

0 commit comments

Comments
 (0)