From 0941d9f14d9c446f9d7a465485652a3dc131de1e Mon Sep 17 00:00:00 2001 From: xFrednet Date: Thu, 10 Jun 2021 20:45:03 +0200 Subject: [PATCH 1/8] Moved dev `ide_setup` to `setup/intellij.rs` --- clippy_dev/src/lib.rs | 2 +- clippy_dev/src/main.rs | 4 ++-- clippy_dev/src/{ide_setup.rs => setup/intellij.rs} | 0 clippy_dev/src/setup/mod.rs | 1 + 4 files changed, 4 insertions(+), 3 deletions(-) rename clippy_dev/src/{ide_setup.rs => setup/intellij.rs} (100%) create mode 100644 clippy_dev/src/setup/mod.rs diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 69f42aca8b69..a9098695df9e 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -14,7 +14,7 @@ use walkdir::WalkDir; pub mod bless; pub mod fmt; -pub mod ide_setup; +pub mod setup; pub mod new_lint; pub mod serve; pub mod stderr_length_check; diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index 7040c257c831..bd4c137ec6d0 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -3,7 +3,7 @@ #![warn(rust_2018_idioms, unused_lifetimes)] use clap::{App, Arg, ArgMatches, SubCommand}; -use clippy_dev::{bless, fmt, ide_setup, new_lint, serve, stderr_length_check, update_lints}; +use clippy_dev::{bless, fmt, setup, new_lint, serve, stderr_length_check, update_lints}; fn main() { let matches = get_clap_config(); @@ -36,7 +36,7 @@ fn main() { ("limit_stderr_length", _) => { stderr_length_check::check(); }, - ("ide_setup", Some(matches)) => ide_setup::run(matches.value_of("rustc-repo-path")), + ("ide_setup", Some(matches)) => setup::intellij::run(matches.value_of("rustc-repo-path")), ("serve", Some(matches)) => { let port = matches.value_of("port").unwrap().parse().unwrap(); let lint = matches.value_of("lint"); diff --git a/clippy_dev/src/ide_setup.rs b/clippy_dev/src/setup/intellij.rs similarity index 100% rename from clippy_dev/src/ide_setup.rs rename to clippy_dev/src/setup/intellij.rs diff --git a/clippy_dev/src/setup/mod.rs b/clippy_dev/src/setup/mod.rs new file mode 100644 index 000000000000..cab4e386fc21 --- /dev/null +++ b/clippy_dev/src/setup/mod.rs @@ -0,0 +1 @@ +pub mod intellij; \ No newline at end of file From 0a5f28c4b0c78c030956afe77b7a5c0c3e33ef5b Mon Sep 17 00:00:00 2001 From: xFrednet Date: Fri, 11 Jun 2021 01:05:51 +0200 Subject: [PATCH 2/8] Added `cargo dev setup git-hook` --- clippy_dev/src/lib.rs | 2 +- clippy_dev/src/main.rs | 43 +++++++++++++++------- clippy_dev/src/setup/git_hook.rs | 61 ++++++++++++++++++++++++++++++++ clippy_dev/src/setup/mod.rs | 31 +++++++++++++++- util/etc/pre-commit.sh | 3 ++ 5 files changed, 126 insertions(+), 14 deletions(-) create mode 100644 clippy_dev/src/setup/git_hook.rs create mode 100755 util/etc/pre-commit.sh diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index a9098695df9e..72bdaf8d5928 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -14,9 +14,9 @@ use walkdir::WalkDir; pub mod bless; pub mod fmt; -pub mod setup; pub mod new_lint; pub mod serve; +pub mod setup; pub mod stderr_length_check; pub mod update_lints; diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index bd4c137ec6d0..b20c40bc556f 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -3,7 +3,7 @@ #![warn(rust_2018_idioms, unused_lifetimes)] use clap::{App, Arg, ArgMatches, SubCommand}; -use clippy_dev::{bless, fmt, setup, new_lint, serve, stderr_length_check, update_lints}; +use clippy_dev::{bless, fmt, new_lint, serve, setup, stderr_length_check, update_lints}; fn main() { let matches = get_clap_config(); @@ -36,7 +36,11 @@ fn main() { ("limit_stderr_length", _) => { stderr_length_check::check(); }, - ("ide_setup", Some(matches)) => setup::intellij::run(matches.value_of("rustc-repo-path")), + ("setup", Some(sub_command)) => match sub_command.subcommand() { + ("intellij", Some(matches)) => setup::intellij::run(matches.value_of("rustc-repo-path")), + ("git-hook", Some(matches)) => setup::git_hook::run(matches.is_present("force-override")), + _ => {}, + }, ("serve", Some(matches)) => { let port = matches.value_of("port").unwrap().parse().unwrap(); let lint = matches.value_of("lint"); @@ -140,16 +144,31 @@ fn get_clap_config<'a>() -> ArgMatches<'a> { .about("Ensures that stderr files do not grow longer than a certain amount of lines."), ) .subcommand( - SubCommand::with_name("ide_setup") - .about("Alter dependencies so Intellij Rust can find rustc internals") - .arg( - Arg::with_name("rustc-repo-path") - .long("repo-path") - .short("r") - .help("The path to a rustc repo that will be used for setting the dependencies") - .takes_value(true) - .value_name("path") - .required(true), + SubCommand::with_name("setup") + .about("Support for setting up your personal development environment") + .subcommand( + SubCommand::with_name("intellij") + .about("Alter dependencies so Intellij Rust can find rustc internals") + .arg( + Arg::with_name("rustc-repo-path") + .long("repo-path") + .short("r") + .help("The path to a rustc repo that will be used for setting the dependencies") + .takes_value(true) + .value_name("path") + .required(true), + ), + ) + .subcommand( + SubCommand::with_name("git-hook") + .about("Add a pre-commit git hook that formats your code to make it look pretty") + .arg( + Arg::with_name("force-override") + .long("force-override") + .short("f") + .help("Forces the override of an existing git pre-commit hook") + .required(false), + ), ), ) .subcommand( diff --git a/clippy_dev/src/setup/git_hook.rs b/clippy_dev/src/setup/git_hook.rs new file mode 100644 index 000000000000..741738e37fb8 --- /dev/null +++ b/clippy_dev/src/setup/git_hook.rs @@ -0,0 +1,61 @@ +use std::fs; +use std::path::Path; + +/// Rusts setup uses `git rev-parse --git-common-dir` to get the root directory of the repo. +/// I've decided against this for the sake of simplicity and to make sure that it doesn't install +/// the hook if `clippy_dev` would be used in the rust tree. The hook also references this tool +/// for formatting and should therefor only be used in a normal clone of clippy +const REPO_GIT_DIR: &str = ".git"; +const HOOK_SOURCE_PATH: &str = "util/etc/pre-commit.sh"; +const HOOK_TARGET_PATH: &str = ".git/hooks/pre-commit"; + +pub fn run(force_override: bool) { + if let Err(_) = check_precondition(force_override) { + return; + } + + // So a little bit of a funny story. Git on unix requires the pre-commit file + // to have the `execute` permission to be set. The Rust functions for modifying + // these flags doesn't seem to work when executed with normal user permissions. + // + // However, there is a little hack that is also being used by Rust itself in their + // setup script. Git saves the `execute` flag when syncing files. This means + // that we can check in a file with execution permissions and the sync it to create + // a file with the flag set. We then copy this file here. The copy function will also + // include the `execute` permission. + match fs::copy(HOOK_SOURCE_PATH, HOOK_TARGET_PATH) { + Ok(_) => println!("Git hook successfully installed :)"), + Err(err) => println!( + "error: unable to copy `{}` to `{}` ({})", + HOOK_SOURCE_PATH, HOOK_TARGET_PATH, err + ), + } +} + +fn check_precondition(force_override: bool) -> Result<(), ()> { + // Make sure that we can find the git repository + let git_path = Path::new(REPO_GIT_DIR); + if !git_path.exists() || !git_path.is_dir() { + println!("error: clippy_dev was unable to find the `.git` directory"); + return Err(()); + } + + // Make sure that we don't override an existing hook by accident + let path = Path::new(HOOK_TARGET_PATH); + if path.exists() { + if !force_override { + println!("warn: The found `.git` directory already has a commit hook"); + } + + if force_override || super::ask_yes_no_question("Do you want to override it?") { + if fs::remove_file(path).is_err() { + println!("error: unable to delete existing pre-commit git hook"); + return Err(()); + } + } else { + return Err(()); + } + } + + Ok(()) +} diff --git a/clippy_dev/src/setup/mod.rs b/clippy_dev/src/setup/mod.rs index cab4e386fc21..5db545c0ff13 100644 --- a/clippy_dev/src/setup/mod.rs +++ b/clippy_dev/src/setup/mod.rs @@ -1 +1,30 @@ -pub mod intellij; \ No newline at end of file +use std::io::{self, Write}; +pub mod git_hook; +pub mod intellij; + +/// This function will asked the user the given question and wait for user input +/// either `true` for yes and `false` for no. +fn ask_yes_no_question(question: &str) -> bool { + // This code was proudly stolen from rusts bootstrapping tool. + + fn ask_with_result(question: &str) -> io::Result { + let mut input = String::new(); + Ok(loop { + print!("{}: [y/N] ", question); + io::stdout().flush()?; + input.clear(); + io::stdin().read_line(&mut input)?; + break match input.trim().to_lowercase().as_str() { + "y" | "yes" => true, + "n" | "no" | "" => false, + _ => { + println!("error: unrecognized option '{}'", input.trim()); + println!("note: press Ctrl+C to exit"); + continue; + }, + }; + }) + } + + ask_with_result(question).unwrap_or_default() +} diff --git a/util/etc/pre-commit.sh b/util/etc/pre-commit.sh new file mode 100755 index 000000000000..3c76e924b347 --- /dev/null +++ b/util/etc/pre-commit.sh @@ -0,0 +1,3 @@ +#!/bin/sh + +cargo dev fmt From 41bc0f4d4d3cdacc97de6c803ff7bdf1c120f315 Mon Sep 17 00:00:00 2001 From: xFrednet Date: Wed, 16 Jun 2021 00:04:50 +0200 Subject: [PATCH 3/8] Adjust pre-commit script to readd files after formatting --- util/etc/pre-commit.sh | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/util/etc/pre-commit.sh b/util/etc/pre-commit.sh index 3c76e924b347..528f8953b25d 100755 --- a/util/etc/pre-commit.sh +++ b/util/etc/pre-commit.sh @@ -1,3 +1,21 @@ #!/bin/sh -cargo dev fmt +# hide output +set -e + +# Update lints +cargo dev update_lints +git add clippy_lints/src/lib.rs + +# Formatting: +# Git will not automatically add the formatted code to the staged changes once +# fmt was executed. This collects all staged files rs files that are currently staged. +# They will later be added back. +# +# This was proudly stolen and adjusted from here: +# https://medium.com/@harshitbangar/automatic-code-formatting-with-git-66c3c5c26798 +files=$( (git diff --cached --name-only --diff-filter=ACMR | grep -Ei "\.rs$") || true) +if [ ! -z "${files}" ]; then + cargo dev fmt + git add $(echo "$files" | paste -s -d " " -) +fi From 3d0984975e555e122499e58d3fbc20e99b7be589 Mon Sep 17 00:00:00 2001 From: xFrednet Date: Wed, 16 Jun 2021 00:21:13 +0200 Subject: [PATCH 4/8] Print cargo dev help on missing arg and updated setup documentation --- CONTRIBUTING.md | 2 +- clippy_dev/src/fmt.rs | 2 +- clippy_dev/src/main.rs | 4 +++- clippy_dev/src/setup/intellij.rs | 2 +- doc/basics.md | 4 +++- 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7265d1b83237..b202fc4f281c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -115,7 +115,7 @@ To work around this, you need to have a copy of the [rustc-repo][rustc_repo] ava `git clone https://github.com/rust-lang/rust/`. Then you can run a `cargo dev` command to automatically make Clippy use the rustc-repo via path-dependencies which `IntelliJ Rust` will be able to understand. -Run `cargo dev ide_setup --repo-path ` where `` is a path to the rustc repo +Run `cargo dev setup intellij --repo-path ` where `` is a path to the rustc repo you just cloned. The command will add path-dependencies pointing towards rustc-crates inside the rustc repo to Clippys `Cargo.toml`s and should allow `IntelliJ Rust` to understand most of the types that Clippy uses. diff --git a/clippy_dev/src/fmt.rs b/clippy_dev/src/fmt.rs index edc8e6a629ce..c81eb40d52f3 100644 --- a/clippy_dev/src/fmt.rs +++ b/clippy_dev/src/fmt.rs @@ -86,7 +86,7 @@ pub fn run(check: bool, verbose: bool) { }, CliError::RaSetupActive => { eprintln!( - "error: a local rustc repo is enabled as path dependency via `cargo dev ide_setup`. + "error: a local rustc repo is enabled as path dependency via `cargo dev setup intellij`. Not formatting because that would format the local repo as well! Please revert the changes to Cargo.tomls first." ); diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index b20c40bc556f..faf8700f55ab 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -2,7 +2,7 @@ // warn on lints, that are included in `rust-lang/rust`s bootstrap #![warn(rust_2018_idioms, unused_lifetimes)] -use clap::{App, Arg, ArgMatches, SubCommand}; +use clap::{App, AppSettings, Arg, ArgMatches, SubCommand}; use clippy_dev::{bless, fmt, new_lint, serve, setup, stderr_length_check, update_lints}; fn main() { let matches = get_clap_config(); @@ -52,6 +52,7 @@ fn main() { fn get_clap_config<'a>() -> ArgMatches<'a> { App::new("Clippy developer tooling") + .setting(AppSettings::ArgRequiredElseHelp) .subcommand( SubCommand::with_name("bless") .about("bless the test output changes") @@ -146,6 +147,7 @@ fn get_clap_config<'a>() -> ArgMatches<'a> { .subcommand( SubCommand::with_name("setup") .about("Support for setting up your personal development environment") + .setting(AppSettings::ArgRequiredElseHelp) .subcommand( SubCommand::with_name("intellij") .about("Alter dependencies so Intellij Rust can find rustc internals") diff --git a/clippy_dev/src/setup/intellij.rs b/clippy_dev/src/setup/intellij.rs index defb1133e44e..9b084f52466e 100644 --- a/clippy_dev/src/setup/intellij.rs +++ b/clippy_dev/src/setup/intellij.rs @@ -55,7 +55,7 @@ fn inject_deps_into_manifest( // do not inject deps if we have aleady done so if cargo_toml.contains("[target.'cfg(NOT_A_PLATFORM)'.dependencies]") { eprintln!( - "cargo dev ide_setup: warning: deps already found inside {}, doing nothing.", + "cargo dev setup intellij: warning: deps already found inside {}, doing nothing.", manifest_path ); return Ok(()); diff --git a/doc/basics.md b/doc/basics.md index e2e307ce4f6c..89d572ad9312 100644 --- a/doc/basics.md +++ b/doc/basics.md @@ -90,8 +90,10 @@ cargo dev fmt cargo dev update_lints # create a new lint and register it cargo dev new_lint +# automatically formatting all code before each commit +cargo dev setup git-hook # (experimental) Setup Clippy to work with IntelliJ-Rust -cargo dev ide_setup +cargo dev setup intellij ``` ## lintcheck From b48f041befdca6ad14acfda2db68c280ee3fbc85 Mon Sep 17 00:00:00 2001 From: xFrednet Date: Wed, 16 Jun 2021 18:59:28 +0200 Subject: [PATCH 5/8] Added the `cargo dev remove` command for convenience --- clippy_dev/src/main.rs | 13 +++++++- clippy_dev/src/setup/git_hook.rs | 51 +++++++++++++++++++++----------- 2 files changed, 45 insertions(+), 19 deletions(-) diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index faf8700f55ab..70c3d93ed473 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -38,9 +38,14 @@ fn main() { }, ("setup", Some(sub_command)) => match sub_command.subcommand() { ("intellij", Some(matches)) => setup::intellij::run(matches.value_of("rustc-repo-path")), - ("git-hook", Some(matches)) => setup::git_hook::run(matches.is_present("force-override")), + ("git-hook", Some(matches)) => setup::git_hook::install_hook(matches.is_present("force-override")), _ => {}, }, + ("remove", Some(sub_command)) => { + if let ("git-hook", Some(_)) = sub_command.subcommand() { + setup::git_hook::remove_hook(); + } + }, ("serve", Some(matches)) => { let port = matches.value_of("port").unwrap().parse().unwrap(); let lint = matches.value_of("lint"); @@ -173,6 +178,12 @@ fn get_clap_config<'a>() -> ArgMatches<'a> { ), ), ) + .subcommand( + SubCommand::with_name("remove") + .about("Support for undoing changes done by the setup command") + .setting(AppSettings::ArgRequiredElseHelp) + .subcommand(SubCommand::with_name("git-hook").about("Remove any existing pre-commit git hook")), + ) .subcommand( SubCommand::with_name("serve") .about("Launch a local 'ALL the Clippy Lints' website in a browser") diff --git a/clippy_dev/src/setup/git_hook.rs b/clippy_dev/src/setup/git_hook.rs index 741738e37fb8..beb07a073fe5 100644 --- a/clippy_dev/src/setup/git_hook.rs +++ b/clippy_dev/src/setup/git_hook.rs @@ -6,11 +6,11 @@ use std::path::Path; /// the hook if `clippy_dev` would be used in the rust tree. The hook also references this tool /// for formatting and should therefor only be used in a normal clone of clippy const REPO_GIT_DIR: &str = ".git"; -const HOOK_SOURCE_PATH: &str = "util/etc/pre-commit.sh"; -const HOOK_TARGET_PATH: &str = ".git/hooks/pre-commit"; +const HOOK_SOURCE_FILE: &str = "util/etc/pre-commit.sh"; +const HOOK_TARGET_FILE: &str = ".git/hooks/pre-commit"; -pub fn run(force_override: bool) { - if let Err(_) = check_precondition(force_override) { +pub fn install_hook(force_override: bool) { + if check_precondition(force_override).is_err() { return; } @@ -23,11 +23,14 @@ pub fn run(force_override: bool) { // that we can check in a file with execution permissions and the sync it to create // a file with the flag set. We then copy this file here. The copy function will also // include the `execute` permission. - match fs::copy(HOOK_SOURCE_PATH, HOOK_TARGET_PATH) { - Ok(_) => println!("Git hook successfully installed :)"), + match fs::copy(HOOK_SOURCE_FILE, HOOK_TARGET_FILE) { + Ok(_) => { + println!("note: the hook can be removed with `cargo dev remove git-hook`"); + println!("Git hook successfully installed :)"); + }, Err(err) => println!( "error: unable to copy `{}` to `{}` ({})", - HOOK_SOURCE_PATH, HOOK_TARGET_PATH, err + HOOK_SOURCE_FILE, HOOK_TARGET_FILE, err ), } } @@ -41,21 +44,33 @@ fn check_precondition(force_override: bool) -> Result<(), ()> { } // Make sure that we don't override an existing hook by accident - let path = Path::new(HOOK_TARGET_PATH); + let path = Path::new(HOOK_TARGET_FILE); if path.exists() { - if !force_override { - println!("warn: The found `.git` directory already has a commit hook"); + if force_override || super::ask_yes_no_question("Do you want to override the existing pre-commit hook it?") { + return delete_git_hook_file(path); } + return Err(()); + } + + Ok(()) +} - if force_override || super::ask_yes_no_question("Do you want to override it?") { - if fs::remove_file(path).is_err() { - println!("error: unable to delete existing pre-commit git hook"); - return Err(()); - } - } else { - return Err(()); +pub fn remove_hook() { + let path = Path::new(HOOK_TARGET_FILE); + if path.exists() { + if delete_git_hook_file(path).is_ok() { + println!("Git hook successfully removed :)"); } + } else { + println!("No pre-commit hook was found. You're good to go :)"); } +} - Ok(()) +fn delete_git_hook_file(path: &Path) -> Result<(), ()> { + if fs::remove_file(path).is_err() { + println!("error: unable to delete existing pre-commit git hook"); + Err(()) + } else { + Ok(()) + } } From 8fdf2897da4ba476018090f3fb8f637a0de00c73 Mon Sep 17 00:00:00 2001 From: xFrednet Date: Tue, 22 Jun 2021 00:25:10 +0200 Subject: [PATCH 6/8] Updated `cargo dev setup intellij` for cleaner user messages --- clippy_dev/src/main.rs | 6 +- clippy_dev/src/setup/intellij.rs | 147 +++++++++++++++++++++++-------- 2 files changed, 114 insertions(+), 39 deletions(-) diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index 70c3d93ed473..8b5c499cd5e9 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -37,7 +37,11 @@ fn main() { stderr_length_check::check(); }, ("setup", Some(sub_command)) => match sub_command.subcommand() { - ("intellij", Some(matches)) => setup::intellij::run(matches.value_of("rustc-repo-path")), + ("intellij", Some(matches)) => setup::intellij::setup_rustc_src( + matches + .value_of("rustc-repo-path") + .expect("this field is mandatory and therefore always valid"), + ), ("git-hook", Some(matches)) => setup::git_hook::install_hook(matches.is_present("force-override")), _ => {}, }, diff --git a/clippy_dev/src/setup/intellij.rs b/clippy_dev/src/setup/intellij.rs index 9b084f52466e..3685e1b6def4 100644 --- a/clippy_dev/src/setup/intellij.rs +++ b/clippy_dev/src/setup/intellij.rs @@ -8,42 +8,113 @@ use std::path::{Path, PathBuf}; // This allows rust analyzer to analyze rustc internals and show proper information inside clippy // code. See https://github.com/rust-analyzer/rust-analyzer/issues/3517 and https://github.com/rust-lang/rust-clippy/issues/5514 for details -/// # Panics -/// -/// Panics if `rustc_path` does not lead to a rustc repo or the files could not be read -pub fn run(rustc_path: Option<&str>) { - // we can unwrap here because the arg is required by clap - let rustc_path = PathBuf::from(rustc_path.unwrap()) - .canonicalize() - .expect("failed to get the absolute repo path"); - assert!(rustc_path.is_dir(), "path is not a directory"); - let rustc_source_basedir = rustc_path.join("compiler"); - assert!( - rustc_source_basedir.is_dir(), - "are you sure the path leads to a rustc repo?" - ); - - let clippy_root_manifest = fs::read_to_string("Cargo.toml").expect("failed to read ./Cargo.toml"); - let clippy_root_lib_rs = fs::read_to_string("src/driver.rs").expect("failed to read ./src/driver.rs"); - inject_deps_into_manifest( - &rustc_source_basedir, - "Cargo.toml", - &clippy_root_manifest, - &clippy_root_lib_rs, - ) - .expect("Failed to inject deps into ./Cargo.toml"); - - let clippy_lints_manifest = - fs::read_to_string("clippy_lints/Cargo.toml").expect("failed to read ./clippy_lints/Cargo.toml"); - let clippy_lints_lib_rs = - fs::read_to_string("clippy_lints/src/lib.rs").expect("failed to read ./clippy_lints/src/lib.rs"); - inject_deps_into_manifest( - &rustc_source_basedir, - "clippy_lints/Cargo.toml", - &clippy_lints_manifest, - &clippy_lints_lib_rs, - ) - .expect("Failed to inject deps into ./clippy_lints/Cargo.toml"); +const CLIPPY_PROJECTS: &[ClippyProjectInfo] = &[ + ClippyProjectInfo::new("root", "Cargo.toml", "src/driver.rs"), + ClippyProjectInfo::new("clippy_lints", "clippy_lints/Cargo.toml", "clippy_lints/src/lib.rs"), + ClippyProjectInfo::new("clippy_utils", "clippy_utils/Cargo.toml", "clippy_utils/src/lib.rs"), +]; + +/// Used to store clippy project information to later inject the dependency into. +struct ClippyProjectInfo { + /// Only used to display information to the user + name: &'static str, + cargo_file: &'static str, + lib_rs_file: &'static str, +} + +impl ClippyProjectInfo { + const fn new(name: &'static str, cargo_file: &'static str, lib_rs_file: &'static str) -> Self { + Self { + name, + cargo_file, + lib_rs_file, + } + } +} + +pub fn setup_rustc_src(rustc_path: &str) { + let rustc_source_dir = match check_and_get_rustc_dir(rustc_path) { + Ok(path) => path, + Err(_) => return, + }; + + for project in CLIPPY_PROJECTS { + if inject_deps_into_project(&rustc_source_dir, project).is_err() { + return; + } + } +} + +fn check_and_get_rustc_dir(rustc_path: &str) -> Result { + let mut path = PathBuf::from(rustc_path); + + if path.is_relative() { + match path.canonicalize() { + Ok(absolute_path) => { + println!("note: the rustc path was resolved to: `{}`", absolute_path.display()); + path = absolute_path; + }, + Err(err) => { + println!("error: unable to get the absolute path of rustc ({})", err); + return Err(()); + }, + }; + } + + let path = path.join("compiler"); + println!("note: looking for compiler sources at: {}", path.display()); + + if !path.exists() { + println!("error: the given path does not exist"); + return Err(()); + } + + if !path.is_dir() { + println!("error: the given path is a file and not a directory"); + return Err(()); + } + + Ok(path) +} + +fn inject_deps_into_project(rustc_source_dir: &Path, project: &ClippyProjectInfo) -> Result<(), ()> { + let cargo_content = read_project_file(project.cargo_file, "Cargo.toml", project.name)?; + let lib_content = read_project_file(project.lib_rs_file, "lib.rs", project.name)?; + + if inject_deps_into_manifest(rustc_source_dir, project.cargo_file, &cargo_content, &lib_content).is_err() { + println!( + "error: unable to inject dependencies into {} with the Cargo file {}", + project.name, project.cargo_file + ); + Err(()) + } else { + Ok(()) + } +} + +/// `clippy_dev` expects to be executed in the root directory of Clippy. This function +/// loads the given file or returns an error. Having it in this extra function ensures +/// that the error message looks nice. +fn read_project_file(file_path: &str, file_name: &str, project: &str) -> Result { + let path = Path::new(file_path); + if !path.exists() { + println!( + "error: unable to find the `{}` file for the project {}", + file_name, project + ); + return Err(()); + } + + match fs::read_to_string(path) { + Ok(content) => Ok(content), + Err(err) => { + println!( + "error: the `{}` file for the project {} could not be read ({})", + file_name, project, err + ); + Err(()) + }, + } } fn inject_deps_into_manifest( @@ -55,7 +126,7 @@ fn inject_deps_into_manifest( // do not inject deps if we have aleady done so if cargo_toml.contains("[target.'cfg(NOT_A_PLATFORM)'.dependencies]") { eprintln!( - "cargo dev setup intellij: warning: deps already found inside {}, doing nothing.", + "warn: dependencies are already setup inside {}, skipping file.", manifest_path ); return Ok(()); @@ -97,7 +168,7 @@ fn inject_deps_into_manifest( let mut file = File::create(manifest_path)?; file.write_all(new_manifest.as_bytes())?; - println!("Dependency paths injected: {}", manifest_path); + println!("note: successfully setup dependencies inside {}", manifest_path); Ok(()) } From f0fa3636536f3843fee2315fc062aa022479fdee Mon Sep 17 00:00:00 2001 From: xFrednet Date: Tue, 22 Jun 2021 20:13:05 +0200 Subject: [PATCH 7/8] Added `cargo dev remove intellij` --- clippy_dev/src/main.rs | 14 ++++-- clippy_dev/src/setup/intellij.rs | 80 ++++++++++++++++++++++++++------ 2 files changed, 76 insertions(+), 18 deletions(-) diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index 8b5c499cd5e9..f5bd08657ea8 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -45,10 +45,10 @@ fn main() { ("git-hook", Some(matches)) => setup::git_hook::install_hook(matches.is_present("force-override")), _ => {}, }, - ("remove", Some(sub_command)) => { - if let ("git-hook", Some(_)) = sub_command.subcommand() { - setup::git_hook::remove_hook(); - } + ("remove", Some(sub_command)) => match sub_command.subcommand() { + ("git-hook", Some(_)) => setup::git_hook::remove_hook(), + ("intellij", Some(_)) => setup::intellij::remove_rustc_src(), + _ => {}, }, ("serve", Some(matches)) => { let port = matches.value_of("port").unwrap().parse().unwrap(); @@ -186,7 +186,11 @@ fn get_clap_config<'a>() -> ArgMatches<'a> { SubCommand::with_name("remove") .about("Support for undoing changes done by the setup command") .setting(AppSettings::ArgRequiredElseHelp) - .subcommand(SubCommand::with_name("git-hook").about("Remove any existing pre-commit git hook")), + .subcommand(SubCommand::with_name("git-hook").about("Remove any existing pre-commit git hook")) + .subcommand( + SubCommand::with_name("intellij") + .about("Removes rustc source paths added via `cargo dev setup intellij`"), + ), ) .subcommand( SubCommand::with_name("serve") diff --git a/clippy_dev/src/setup/intellij.rs b/clippy_dev/src/setup/intellij.rs index 3685e1b6def4..249804240dfd 100644 --- a/clippy_dev/src/setup/intellij.rs +++ b/clippy_dev/src/setup/intellij.rs @@ -8,6 +8,9 @@ use std::path::{Path, PathBuf}; // This allows rust analyzer to analyze rustc internals and show proper information inside clippy // code. See https://github.com/rust-analyzer/rust-analyzer/issues/3517 and https://github.com/rust-lang/rust-clippy/issues/5514 for details +const RUSTC_PATH_SECTION: &str = "[target.'cfg(NOT_A_PLATFORM)'.dependencies]"; +const DEPENDENCIES_SECTION: &str = "[dependencies]"; + const CLIPPY_PROJECTS: &[ClippyProjectInfo] = &[ ClippyProjectInfo::new("root", "Cargo.toml", "src/driver.rs"), ClippyProjectInfo::new("clippy_lints", "clippy_lints/Cargo.toml", "clippy_lints/src/lib.rs"), @@ -43,6 +46,8 @@ pub fn setup_rustc_src(rustc_path: &str) { return; } } + + println!("info: the source paths can be removed again with `cargo dev remove intellij`"); } fn check_and_get_rustc_dir(rustc_path: &str) -> Result { @@ -51,26 +56,26 @@ fn check_and_get_rustc_dir(rustc_path: &str) -> Result { if path.is_relative() { match path.canonicalize() { Ok(absolute_path) => { - println!("note: the rustc path was resolved to: `{}`", absolute_path.display()); + println!("info: the rustc path was resolved to: `{}`", absolute_path.display()); path = absolute_path; }, Err(err) => { - println!("error: unable to get the absolute path of rustc ({})", err); + eprintln!("error: unable to get the absolute path of rustc ({})", err); return Err(()); }, }; } let path = path.join("compiler"); - println!("note: looking for compiler sources at: {}", path.display()); + println!("info: looking for compiler sources at: {}", path.display()); if !path.exists() { - println!("error: the given path does not exist"); + eprintln!("error: the given path does not exist"); return Err(()); } if !path.is_dir() { - println!("error: the given path is a file and not a directory"); + eprintln!("error: the given path is a file and not a directory"); return Err(()); } @@ -82,7 +87,7 @@ fn inject_deps_into_project(rustc_source_dir: &Path, project: &ClippyProjectInfo let lib_content = read_project_file(project.lib_rs_file, "lib.rs", project.name)?; if inject_deps_into_manifest(rustc_source_dir, project.cargo_file, &cargo_content, &lib_content).is_err() { - println!( + eprintln!( "error: unable to inject dependencies into {} with the Cargo file {}", project.name, project.cargo_file ); @@ -98,7 +103,7 @@ fn inject_deps_into_project(rustc_source_dir: &Path, project: &ClippyProjectInfo fn read_project_file(file_path: &str, file_name: &str, project: &str) -> Result { let path = Path::new(file_path); if !path.exists() { - println!( + eprintln!( "error: unable to find the `{}` file for the project {}", file_name, project ); @@ -123,10 +128,10 @@ fn inject_deps_into_manifest( cargo_toml: &str, lib_rs: &str, ) -> std::io::Result<()> { - // do not inject deps if we have aleady done so - if cargo_toml.contains("[target.'cfg(NOT_A_PLATFORM)'.dependencies]") { + // do not inject deps if we have already done so + if cargo_toml.contains(RUSTC_PATH_SECTION) { eprintln!( - "warn: dependencies are already setup inside {}, skipping file.", + "warn: dependencies are already setup inside {}, skipping file", manifest_path ); return Ok(()); @@ -134,8 +139,8 @@ fn inject_deps_into_manifest( let extern_crates = lib_rs .lines() - // get the deps - .filter(|line| line.starts_with("extern crate")) + // only take dependencies starting with `rustc_` + .filter(|line| line.starts_with("extern crate rustc_")) // we have something like "extern crate foo;", we only care about the "foo" // ↓ ↓ // extern crate rustc_middle; @@ -168,7 +173,56 @@ fn inject_deps_into_manifest( let mut file = File::create(manifest_path)?; file.write_all(new_manifest.as_bytes())?; - println!("note: successfully setup dependencies inside {}", manifest_path); + println!("info: successfully setup dependencies inside {}", manifest_path); Ok(()) } + +pub fn remove_rustc_src() { + for project in CLIPPY_PROJECTS { + // We don't care about the result here as we want to go through all + // dependencies either way. Any info and error message will be issued by + // the removal code itself. + let _ = remove_rustc_src_from_project(project); + } +} + +fn remove_rustc_src_from_project(project: &ClippyProjectInfo) -> Result<(), ()> { + let mut cargo_content = read_project_file(project.cargo_file, "Cargo.toml", project.name)?; + let section_start = if let Some(section_start) = cargo_content.find(RUSTC_PATH_SECTION) { + section_start + } else { + println!( + "info: dependencies could not be found in `{}` for {}, skipping file", + project.cargo_file, project.name + ); + return Ok(()); + }; + + let end_point = if let Some(end_point) = cargo_content.find(DEPENDENCIES_SECTION) { + end_point + } else { + eprintln!( + "error: the end of the rustc dependencies section could not be found in `{}`", + project.cargo_file + ); + return Err(()); + }; + + cargo_content.replace_range(section_start..end_point, ""); + + match File::create(project.cargo_file) { + Ok(mut file) => { + file.write_all(cargo_content.as_bytes()).unwrap(); + println!("info: successfully removed dependencies inside {}", project.cargo_file); + Ok(()) + }, + Err(err) => { + eprintln!( + "error: unable to open file `{}` to remove rustc dependencies for {} ({})", + project.cargo_file, project.name, err + ); + Err(()) + }, + } +} From 8e969cdcefefe6792537dac11855bc5f91904f0b Mon Sep 17 00:00:00 2001 From: xFrednet Date: Wed, 23 Jun 2021 19:04:09 +0200 Subject: [PATCH 8/8] Updated several clippy_dev messages and types (PR suggestions) Co-authored-by: Philipp Krones --- clippy_dev/src/setup/git_hook.rs | 39 ++++++++++++++------------- clippy_dev/src/setup/intellij.rs | 45 ++++++++++++++------------------ clippy_dev/src/setup/mod.rs | 28 -------------------- 3 files changed, 41 insertions(+), 71 deletions(-) diff --git a/clippy_dev/src/setup/git_hook.rs b/clippy_dev/src/setup/git_hook.rs index beb07a073fe5..f27b69a195b4 100644 --- a/clippy_dev/src/setup/git_hook.rs +++ b/clippy_dev/src/setup/git_hook.rs @@ -10,7 +10,7 @@ const HOOK_SOURCE_FILE: &str = "util/etc/pre-commit.sh"; const HOOK_TARGET_FILE: &str = ".git/hooks/pre-commit"; pub fn install_hook(force_override: bool) { - if check_precondition(force_override).is_err() { + if !check_precondition(force_override) { return; } @@ -25,52 +25,55 @@ pub fn install_hook(force_override: bool) { // include the `execute` permission. match fs::copy(HOOK_SOURCE_FILE, HOOK_TARGET_FILE) { Ok(_) => { - println!("note: the hook can be removed with `cargo dev remove git-hook`"); - println!("Git hook successfully installed :)"); + println!("info: the hook can be removed with `cargo dev remove git-hook`"); + println!("git hook successfully installed"); }, - Err(err) => println!( + Err(err) => eprintln!( "error: unable to copy `{}` to `{}` ({})", HOOK_SOURCE_FILE, HOOK_TARGET_FILE, err ), } } -fn check_precondition(force_override: bool) -> Result<(), ()> { +fn check_precondition(force_override: bool) -> bool { // Make sure that we can find the git repository let git_path = Path::new(REPO_GIT_DIR); if !git_path.exists() || !git_path.is_dir() { - println!("error: clippy_dev was unable to find the `.git` directory"); - return Err(()); + eprintln!("error: clippy_dev was unable to find the `.git` directory"); + return false; } // Make sure that we don't override an existing hook by accident let path = Path::new(HOOK_TARGET_FILE); if path.exists() { - if force_override || super::ask_yes_no_question("Do you want to override the existing pre-commit hook it?") { + if force_override { return delete_git_hook_file(path); } - return Err(()); + + eprintln!("error: there is already a pre-commit hook installed"); + println!("info: use the `--force-override` flag to override the existing hook"); + return false; } - Ok(()) + true } pub fn remove_hook() { let path = Path::new(HOOK_TARGET_FILE); if path.exists() { - if delete_git_hook_file(path).is_ok() { - println!("Git hook successfully removed :)"); + if delete_git_hook_file(path) { + println!("git hook successfully removed"); } } else { - println!("No pre-commit hook was found. You're good to go :)"); + println!("no pre-commit hook was found"); } } -fn delete_git_hook_file(path: &Path) -> Result<(), ()> { - if fs::remove_file(path).is_err() { - println!("error: unable to delete existing pre-commit git hook"); - Err(()) +fn delete_git_hook_file(path: &Path) -> bool { + if let Err(err) = fs::remove_file(path) { + eprintln!("error: unable to delete existing pre-commit git hook ({})", err); + false } else { - Ok(()) + true } } diff --git a/clippy_dev/src/setup/intellij.rs b/clippy_dev/src/setup/intellij.rs index 249804240dfd..bf741e6d1217 100644 --- a/clippy_dev/src/setup/intellij.rs +++ b/clippy_dev/src/setup/intellij.rs @@ -5,8 +5,8 @@ use std::path::{Path, PathBuf}; // This module takes an absolute path to a rustc repo and alters the dependencies to point towards // the respective rustc subcrates instead of using extern crate xyz. -// This allows rust analyzer to analyze rustc internals and show proper information inside clippy -// code. See https://github.com/rust-analyzer/rust-analyzer/issues/3517 and https://github.com/rust-lang/rust-clippy/issues/5514 for details +// This allows IntelliJ to analyze rustc internals and show proper information inside Clippy +// code. See https://github.com/rust-lang/rust-clippy/issues/5514 for details const RUSTC_PATH_SECTION: &str = "[target.'cfg(NOT_A_PLATFORM)'.dependencies]"; const DEPENDENCIES_SECTION: &str = "[dependencies]"; @@ -75,7 +75,7 @@ fn check_and_get_rustc_dir(rustc_path: &str) -> Result { } if !path.is_dir() { - eprintln!("error: the given path is a file and not a directory"); + eprintln!("error: the given path is not a directory"); return Err(()); } @@ -83,8 +83,8 @@ fn check_and_get_rustc_dir(rustc_path: &str) -> Result { } fn inject_deps_into_project(rustc_source_dir: &Path, project: &ClippyProjectInfo) -> Result<(), ()> { - let cargo_content = read_project_file(project.cargo_file, "Cargo.toml", project.name)?; - let lib_content = read_project_file(project.lib_rs_file, "lib.rs", project.name)?; + let cargo_content = read_project_file(project.cargo_file)?; + let lib_content = read_project_file(project.lib_rs_file)?; if inject_deps_into_manifest(rustc_source_dir, project.cargo_file, &cargo_content, &lib_content).is_err() { eprintln!( @@ -100,23 +100,17 @@ fn inject_deps_into_project(rustc_source_dir: &Path, project: &ClippyProjectInfo /// `clippy_dev` expects to be executed in the root directory of Clippy. This function /// loads the given file or returns an error. Having it in this extra function ensures /// that the error message looks nice. -fn read_project_file(file_path: &str, file_name: &str, project: &str) -> Result { +fn read_project_file(file_path: &str) -> Result { let path = Path::new(file_path); if !path.exists() { - eprintln!( - "error: unable to find the `{}` file for the project {}", - file_name, project - ); + eprintln!("error: unable to find the file `{}`", file_path); return Err(()); } match fs::read_to_string(path) { Ok(content) => Ok(content), Err(err) => { - println!( - "error: the `{}` file for the project {} could not be read ({})", - file_name, project, err - ); + eprintln!("error: the file `{}` could not be read ({})", file_path, err); Err(()) }, } @@ -142,8 +136,8 @@ fn inject_deps_into_manifest( // only take dependencies starting with `rustc_` .filter(|line| line.starts_with("extern crate rustc_")) // we have something like "extern crate foo;", we only care about the "foo" - // ↓ ↓ // extern crate rustc_middle; + // ^^^^^^^^^^^^ .map(|s| &s[13..(s.len() - 1)]); let new_deps = extern_crates.map(|dep| { @@ -180,15 +174,16 @@ fn inject_deps_into_manifest( pub fn remove_rustc_src() { for project in CLIPPY_PROJECTS { - // We don't care about the result here as we want to go through all - // dependencies either way. Any info and error message will be issued by - // the removal code itself. - let _ = remove_rustc_src_from_project(project); + remove_rustc_src_from_project(project); } } -fn remove_rustc_src_from_project(project: &ClippyProjectInfo) -> Result<(), ()> { - let mut cargo_content = read_project_file(project.cargo_file, "Cargo.toml", project.name)?; +fn remove_rustc_src_from_project(project: &ClippyProjectInfo) -> bool { + let mut cargo_content = if let Ok(content) = read_project_file(project.cargo_file) { + content + } else { + return false; + }; let section_start = if let Some(section_start) = cargo_content.find(RUSTC_PATH_SECTION) { section_start } else { @@ -196,7 +191,7 @@ fn remove_rustc_src_from_project(project: &ClippyProjectInfo) -> Result<(), ()> "info: dependencies could not be found in `{}` for {}, skipping file", project.cargo_file, project.name ); - return Ok(()); + return true; }; let end_point = if let Some(end_point) = cargo_content.find(DEPENDENCIES_SECTION) { @@ -206,7 +201,7 @@ fn remove_rustc_src_from_project(project: &ClippyProjectInfo) -> Result<(), ()> "error: the end of the rustc dependencies section could not be found in `{}`", project.cargo_file ); - return Err(()); + return false; }; cargo_content.replace_range(section_start..end_point, ""); @@ -215,14 +210,14 @@ fn remove_rustc_src_from_project(project: &ClippyProjectInfo) -> Result<(), ()> Ok(mut file) => { file.write_all(cargo_content.as_bytes()).unwrap(); println!("info: successfully removed dependencies inside {}", project.cargo_file); - Ok(()) + true }, Err(err) => { eprintln!( "error: unable to open file `{}` to remove rustc dependencies for {} ({})", project.cargo_file, project.name, err ); - Err(()) + false }, } } diff --git a/clippy_dev/src/setup/mod.rs b/clippy_dev/src/setup/mod.rs index 5db545c0ff13..3834f5a18421 100644 --- a/clippy_dev/src/setup/mod.rs +++ b/clippy_dev/src/setup/mod.rs @@ -1,30 +1,2 @@ -use std::io::{self, Write}; pub mod git_hook; pub mod intellij; - -/// This function will asked the user the given question and wait for user input -/// either `true` for yes and `false` for no. -fn ask_yes_no_question(question: &str) -> bool { - // This code was proudly stolen from rusts bootstrapping tool. - - fn ask_with_result(question: &str) -> io::Result { - let mut input = String::new(); - Ok(loop { - print!("{}: [y/N] ", question); - io::stdout().flush()?; - input.clear(); - io::stdin().read_line(&mut input)?; - break match input.trim().to_lowercase().as_str() { - "y" | "yes" => true, - "n" | "no" | "" => false, - _ => { - println!("error: unrecognized option '{}'", input.trim()); - println!("note: press Ctrl+C to exit"); - continue; - }, - }; - }) - } - - ask_with_result(question).unwrap_or_default() -}