Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start making clippy easier to invoke in non-cargo contexts #3665

Merged
merged 6 commits into from
Feb 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ path = "src/main.rs"

[[bin]]
name = "clippy-driver"
test = false
path = "src/driver.rs"

[dependencies]
Expand Down
38 changes: 32 additions & 6 deletions ci/base-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
61 changes: 56 additions & 5 deletions src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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"));
Expand All @@ -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);
}
Expand All @@ -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
Expand All @@ -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()]);
Expand Down
10 changes: 2 additions & 8 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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")
Expand Down