mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #7361 - xFrednet:5394-expand-setup-command, r=flip1995

Browse Source 
Added `cargo dev setup git-hook` and updated `cargo dev setup intellij` including a `remove` command

This PR enables our dev tool to install a git hook that formats the code before each commit and also runs `update_lints` to make sure that everything is registered correctly. The script is located at `util/etc/pre-commit.sh`. I found it reasonable to locate it in the `util` folder and decided to add a `etc` in correlation to the main rust repo and to bring a bit of structure into it.

* The hook can be installed via: `cargo dev setup git-hook`
* And removed via: `cargo dev remove git-hook`

cc: #5394

The refactoring of `src/ide_setup.rs` to `src/setup/intellij.rs` is an extra commit to simplify the review.

---

Changes:
* Added `cargo dev setup git-hook` for formatting before every commit
* Added `cargo dev remove git-hook` to remove the hook again
* Added `cargo dev remove intellij` to remove rustc source path dependencies
* Changed `cargo dev ide_setup` to `cargo dev setup intellij`

changelog: none

This is only an internal change and therefore not worth an entry in the general change log.

---

Tested on:
* [x] Linux (by `@xFrednet)`
* [ ] Windows (All used commands run inside the git bash, so it's very likely to work as well `@xFrednet)`
* [ ] macOS
This commit is contained in:
bors 2021-06-25 09:22:04 +00:00
commit a03dfb9eac
10 changed files with 384 additions and 120 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CONTRIBUTING.md
View File

@ -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 <repo-path>` where `<repo-path>` is a path to the rustc repo
Run `cargo dev setup intellij --repo-path <repo-path>` where `<repo-path>` 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.

2
clippy_dev/src/fmt.rs
View File

@ -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."
);

103
clippy_dev/src/ide_setup.rs
View File

@ -1,103 +0,0 @@
use std::fs;
use std::fs::File;
use std::io::prelude::*;
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
/// # 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");
}
fn inject_deps_into_manifest(
rustc_source_dir: &Path,
manifest_path: &str,
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]") {
eprintln!(
"cargo dev ide_setup: warning: deps already found inside {}, doing nothing.",
manifest_path
);
return Ok(());
}
let extern_crates = lib_rs
.lines()
// get the deps
.filter(|line| line.starts_with("extern crate"))
// 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| {
// format the dependencies that are going to be put inside the Cargo.toml
format!(
"{dep} = {{ path = \"{source_path}/{dep}\" }}\n",
dep = dep,
source_path = rustc_source_dir.display()
)
});
// format a new [dependencies]-block with the new deps we need to inject
let mut all_deps = String::from("[target.'cfg(NOT_A_PLATFORM)'.dependencies]\n");
new_deps.for_each(|dep_line| {
all_deps.push_str(&dep_line);
});
all_deps.push_str("\n[dependencies]\n");
// replace "[dependencies]" with
// [dependencies]
// dep1 = { path = ... }
// dep2 = { path = ... }
// etc
let new_manifest = cargo_toml.replacen("[dependencies]\n", &all_deps, 1);
// println!("{}", new_manifest);
let mut file = File::create(manifest_path)?;
file.write_all(new_manifest.as_bytes())?;
println!("Dependency paths injected: {}", manifest_path);
Ok(())
}

2
clippy_dev/src/lib.rs
View File

@ -14,9 +14,9 @@ use walkdir::WalkDir;
pub mod bless;
pub mod fmt;
pub mod ide_setup;
pub mod new_lint;
pub mod serve;
pub mod setup;
pub mod stderr_length_check;
pub mod update_lints;

66
clippy_dev/src/main.rs
View File

@ -2,8 +2,8 @@
// 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 clippy_dev::{bless, fmt, ide_setup, new_lint, serve, stderr_length_check, update_lints};
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();
@ -36,7 +36,20 @@ fn main() {
("limit_stderr_length", _) => {
stderr_length_check::check();
},
("ide_setup", Some(matches)) => ide_setup::run(matches.value_of("rustc-repo-path")),
("setup", Some(sub_command)) => match sub_command.subcommand() {
("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")),
_ => {},
},
("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();
let lint = matches.value_of("lint");
@ -48,6 +61,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")
@ -140,16 +154,42 @@ 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")
.setting(AppSettings::ArgRequiredElseHelp)
.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(
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("intellij")
.about("Removes rustc source paths added via `cargo dev setup intellij`"),
),
)
.subcommand(

79
clippy_dev/src/setup/git_hook.rs Normal file
View File

@ -0,0 +1,79 @@
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_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) {
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_FILE, HOOK_TARGET_FILE) {
Ok(_) => {
println!("info: the hook can be removed with `cargo dev remove git-hook`");
println!("git hook successfully installed");
},
Err(err) => eprintln!(
"error: unable to copy `{}` to `{}` ({})",
HOOK_SOURCE_FILE, HOOK_TARGET_FILE, err
),
}
}
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() {
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 {
return delete_git_hook_file(path);
}
eprintln!("error: there is already a pre-commit hook installed");
println!("info: use the `--force-override` flag to override the existing hook");
return false;
}
true
}
pub fn remove_hook() {
let path = Path::new(HOOK_TARGET_FILE);
if path.exists() {
if delete_git_hook_file(path) {
println!("git hook successfully removed");
}
} else {
println!("no pre-commit hook was found");
}
}
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 {
true
}
}

223
clippy_dev/src/setup/intellij.rs Normal file
View File

@ -0,0 +1,223 @@
use std::fs;
use std::fs::File;
use std::io::prelude::*;
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 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]";
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;
}
}
println!("info: the source paths can be removed again with `cargo dev remove intellij`");
}
fn check_and_get_rustc_dir(rustc_path: &str) -> Result<PathBuf, ()> {
let mut path = PathBuf::from(rustc_path);
if path.is_relative() {
match path.canonicalize() {
Ok(absolute_path) => {
println!("info: the rustc path was resolved to: `{}`", absolute_path.display());
path = absolute_path;
},
Err(err) => {
eprintln!("error: unable to get the absolute path of rustc ({})", err);
return Err(());
},
};
}
let path = path.join("compiler");
println!("info: looking for compiler sources at: {}", path.display());
if !path.exists() {
eprintln!("error: the given path does not exist");
return Err(());
}
if !path.is_dir() {
eprintln!("error: the given path is 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)?;
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!(
"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) -> Result<String, ()> {
let path = Path::new(file_path);
if !path.exists() {
eprintln!("error: unable to find the file `{}`", file_path);
return Err(());
}
match fs::read_to_string(path) {
Ok(content) => Ok(content),
Err(err) => {
eprintln!("error: the file `{}` could not be read ({})", file_path, err);
Err(())
},
}
}
fn inject_deps_into_manifest(
rustc_source_dir: &Path,
manifest_path: &str,
cargo_toml: &str,
lib_rs: &str,
) -> std::io::Result<()> {
// 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",
manifest_path
);
return Ok(());
}
let extern_crates = lib_rs
.lines()
// 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| {
// format the dependencies that are going to be put inside the Cargo.toml
format!(
"{dep} = {{ path = \"{source_path}/{dep}\" }}\n",
dep = dep,
source_path = rustc_source_dir.display()
)
});
// format a new [dependencies]-block with the new deps we need to inject
let mut all_deps = String::from("[target.'cfg(NOT_A_PLATFORM)'.dependencies]\n");
new_deps.for_each(|dep_line| {
all_deps.push_str(&dep_line);
});
all_deps.push_str("\n[dependencies]\n");
// replace "[dependencies]" with
// [dependencies]
// dep1 = { path = ... }
// dep2 = { path = ... }
// etc
let new_manifest = cargo_toml.replacen("[dependencies]\n", &all_deps, 1);
// println!("{}", new_manifest);
let mut file = File::create(manifest_path)?;
file.write_all(new_manifest.as_bytes())?;
println!("info: successfully setup dependencies inside {}", manifest_path);
Ok(())
}
pub fn remove_rustc_src() {
for project in CLIPPY_PROJECTS {
remove_rustc_src_from_project(project);
}
}
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 {
println!(
"info: dependencies could not be found in `{}` for {}, skipping file",
project.cargo_file, project.name
);
return true;
};
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 false;
};
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);
true
},
Err(err) => {
eprintln!(
"error: unable to open file `{}` to remove rustc dependencies for {} ({})",
project.cargo_file, project.name, err
);
false
},
}
}

2
clippy_dev/src/setup/mod.rs Normal file
View File

@ -0,0 +1,2 @@
pub mod git_hook;
pub mod intellij;

4
doc/basics.md
View File

@ -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

21
util/etc/pre-commit.sh Executable file
View File

@ -0,0 +1,21 @@
#!/bin/sh
# 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