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

Deprecate util/dev in favor of cargo alias #5109

Merged
merged 4 commits into from
Jan 31, 2020
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: 1 addition & 0 deletions .cargo/config
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[alias]
uitest = "test --test compile-test"
dev = "run --package clippy_dev --bin clippy_dev --manifest-path clippy_dev/Cargo.toml --"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use --target-dir flag I believe


[build]
rustflags = ["-Zunstable-options"]
4 changes: 2 additions & 2 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ checked during review or continuous integration.
- [ ] Followed [lint naming conventions][lint_naming]
- [ ] Added passing UI tests (including committed `.stderr` file)
- [ ] `cargo test` passes locally
- [ ] Executed `./util/dev update_lints`
- [ ] Executed `cargo dev update_lints`
- [ ] Added lint documentation
- [ ] Run `./util/dev fmt`
- [ ] Run `cargo dev fmt`

[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints

Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry) {

The [`plugin::PluginRegistry`][plugin_registry] provides two methods to register lints: [register_early_lint_pass][reg_early_lint_pass] and [register_late_lint_pass][reg_late_lint_pass].
Both take an object that implements an [`EarlyLintPass`][early_lint_pass] or [`LateLintPass`][late_lint_pass] respectively. This is done in every single lint.
It's worth noting that the majority of `clippy_lints/src/lib.rs` is autogenerated by `util/dev update_lints` and you don't have to add anything by hand. When you are writing your own lint, you can use that script to save you some time.
It's worth noting that the majority of `clippy_lints/src/lib.rs` is autogenerated by `cargo dev update_lints` and you don't have to add anything by hand. When you are writing your own lint, you can use that script to save you some time.

```rust
// ./clippy_lints/src/else_if_without_else.rs
Expand Down
4 changes: 2 additions & 2 deletions ci/base-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ cargo test --features deny-warnings
)

# Perform various checks for lint registration
./util/dev update_lints --check
./util/dev --limit-stderr-length
cargo dev update_lints --check
cargo dev --limit-stderr-length

# Check running clippy-driver without cargo
(
Expand Down
30 changes: 4 additions & 26 deletions clippy_dev/src/fmt.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use clippy_dev::clippy_project_root;
use shell_escape::escape;
use std::ffi::OsStr;
use std::io;
use std::path::{Path, PathBuf};
use std::path::Path;
use std::process::{self, Command};
use walkdir::WalkDir;

#[derive(Debug)]
pub enum CliError {
CommandFailed(String),
IoError(io::Error),
ProjectRootNotFound,
RustfmtNotInstalled,
WalkDirError(walkdir::Error),
}
Expand All @@ -35,7 +35,7 @@ pub fn run(check: bool, verbose: bool) {
fn try_run(context: &FmtContext) -> Result<bool, CliError> {
let mut success = true;

let project_root = project_root()?;
let project_root = clippy_project_root();

rustfmt_test(context)?;

Expand Down Expand Up @@ -69,9 +69,6 @@ pub fn run(check: bool, verbose: bool) {
CliError::IoError(err) => {
eprintln!("error: {}", err);
},
CliError::ProjectRootNotFound => {
eprintln!("error: Can't determine root of project. Please run inside a Clippy working dir.");
},
CliError::RustfmtNotInstalled => {
eprintln!("error: rustfmt nightly is not installed.");
},
Expand All @@ -88,7 +85,7 @@ pub fn run(check: bool, verbose: bool) {
Ok(false) => {
eprintln!();
eprintln!("Formatting check failed.");
eprintln!("Run `./util/dev fmt` to update formatting.");
eprintln!("Run `cargo dev fmt` to update formatting.");
1
},
Err(err) => {
Expand Down Expand Up @@ -176,22 +173,3 @@ fn rustfmt(context: &FmtContext, path: &Path) -> Result<bool, CliError> {
}
Ok(success)
}

fn project_root() -> Result<PathBuf, CliError> {
phansch marked this conversation as resolved.
Show resolved Hide resolved
let current_dir = std::env::current_dir()?;
for path in current_dir.ancestors() {
let result = std::fs::read_to_string(path.join("Cargo.toml"));
if let Err(err) = &result {
if err.kind() == io::ErrorKind::NotFound {
continue;
}
}

let content = result?;
if content.contains("[package]\nname = \"clippy\"") {
return Ok(path.to_path_buf());
}
}

Err(CliError::ProjectRootNotFound)
}
31 changes: 27 additions & 4 deletions clippy_dev/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::collections::HashMap;
use std::ffi::OsStr;
use std::fs;
use std::io::prelude::*;
use std::path::{Path, PathBuf};
use walkdir::WalkDir;

lazy_static! {
Expand Down Expand Up @@ -205,7 +206,8 @@ fn parse_contents(content: &str, filename: &str) -> impl Iterator<Item = Lint> {
fn lint_files() -> impl Iterator<Item = walkdir::DirEntry> {
// We use `WalkDir` instead of `fs::read_dir` here in order to recurse into subdirectories.
// Otherwise we would not collect all the lints, for example in `clippy_lints/src/methods/`.
WalkDir::new("../clippy_lints/src")
let path = clippy_project_root().join("clippy_lints/src");
WalkDir::new(path)
.into_iter()
.filter_map(std::result::Result::ok)
.filter(|f| f.path().extension() == Some(OsStr::new("rs")))
Expand All @@ -225,7 +227,7 @@ pub struct FileChange {
/// See `replace_region_in_text` for documentation of the other options.
#[allow(clippy::expect_fun_call)]
pub fn replace_region_in_file<F>(
path: &str,
path: &Path,
start: &str,
end: &str,
replace_start: bool,
Expand All @@ -235,14 +237,15 @@ pub fn replace_region_in_file<F>(
where
F: Fn() -> Vec<String>,
{
let mut f = fs::File::open(path).expect(&format!("File not found: {}", path));
let path = clippy_project_root().join(path);
let mut f = fs::File::open(&path).expect(&format!("File not found: {}", path.to_string_lossy()));
let mut contents = String::new();
f.read_to_string(&mut contents)
.expect("Something went wrong reading the file");
let file_change = replace_region_in_text(&contents, start, end, replace_start, replacements);

if write_back {
let mut f = fs::File::create(path).expect(&format!("File not found: {}", path));
let mut f = fs::File::create(&path).expect(&format!("File not found: {}", path.to_string_lossy()));
f.write_all(file_change.new_lines.as_bytes())
.expect("Unable to write file");
// Ensure we write the changes with a trailing newline so that
Expand Down Expand Up @@ -318,6 +321,26 @@ where
}
}

/// Returns the path to the Clippy project directory
#[must_use]
pub fn clippy_project_root() -> PathBuf {
phansch marked this conversation as resolved.
Show resolved Hide resolved
let current_dir = std::env::current_dir().unwrap();
for path in current_dir.ancestors() {
let result = std::fs::read_to_string(path.join("Cargo.toml"));
if let Err(err) = &result {
if err.kind() == std::io::ErrorKind::NotFound {
continue;
}
}

let content = result.unwrap();
if content.contains("[package]\nname = \"clippy\"") {
return path.to_path_buf();
}
}
panic!("error: Can't determine root of project. Please run inside a Clippy working dir.");
}

#[test]
fn test_parse_contents() {
let result: Vec<Lint> = parse_contents(
Expand Down
23 changes: 12 additions & 11 deletions clippy_dev/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use clap::{App, Arg, SubCommand};
use clippy_dev::*;
use std::path::Path;

mod fmt;
mod new_lint;
Expand Down Expand Up @@ -49,12 +50,12 @@ fn main() {
.arg(
Arg::with_name("check")
.long("check")
.help("Checks that util/dev update_lints has been run. Used on CI."),
.help("Checks that `cargo dev update_lints` has been run. Used on CI."),
),
)
.subcommand(
SubCommand::with_name("new_lint")
.about("Create new lint and run util/dev update_lints")
.about("Create new lint and run `cargo dev update_lints`")
.arg(
Arg::with_name("pass")
.short("p")
Expand Down Expand Up @@ -170,7 +171,7 @@ fn update_lints(update_mode: &UpdateMode) {
sorted_usable_lints.sort_by_key(|lint| lint.name.clone());

let mut file_change = replace_region_in_file(
"../src/lintlist/mod.rs",
Path::new("src/lintlist/mod.rs"),
"begin lint list",
"end lint list",
false,
Expand All @@ -189,7 +190,7 @@ fn update_lints(update_mode: &UpdateMode) {
.changed;

file_change |= replace_region_in_file(
"../README.md",
Path::new("README.md"),
r#"\[There are \d+ lints included in this crate!\]\(https://rust-lang.github.io/rust-clippy/master/index.html\)"#,
"",
true,
Expand All @@ -202,7 +203,7 @@ fn update_lints(update_mode: &UpdateMode) {
).changed;

file_change |= replace_region_in_file(
"../CHANGELOG.md",
Path::new("CHANGELOG.md"),
"<!-- begin autogenerated links to lint list -->",
"<!-- end autogenerated links to lint list -->",
false,
Expand All @@ -212,7 +213,7 @@ fn update_lints(update_mode: &UpdateMode) {
.changed;

file_change |= replace_region_in_file(
"../clippy_lints/src/lib.rs",
Path::new("clippy_lints/src/lib.rs"),
"begin deprecated lints",
"end deprecated lints",
false,
Expand All @@ -222,7 +223,7 @@ fn update_lints(update_mode: &UpdateMode) {
.changed;

file_change |= replace_region_in_file(
"../clippy_lints/src/lib.rs",
Path::new("clippy_lints/src/lib.rs"),
"begin register lints",
"end register lints",
false,
Expand All @@ -232,7 +233,7 @@ fn update_lints(update_mode: &UpdateMode) {
.changed;

file_change |= replace_region_in_file(
"../clippy_lints/src/lib.rs",
Path::new("clippy_lints/src/lib.rs"),
"begin lints modules",
"end lints modules",
false,
Expand All @@ -243,7 +244,7 @@ fn update_lints(update_mode: &UpdateMode) {

// Generate lists of lints in the clippy::all lint group
file_change |= replace_region_in_file(
"../clippy_lints/src/lib.rs",
Path::new("clippy_lints/src/lib.rs"),
r#"store.register_group\(true, "clippy::all""#,
r#"\]\);"#,
false,
Expand All @@ -266,7 +267,7 @@ fn update_lints(update_mode: &UpdateMode) {
// Generate the list of lints for all other lint groups
for (lint_group, lints) in Lint::by_lint_group(&usable_lints) {
file_change |= replace_region_in_file(
"../clippy_lints/src/lib.rs",
Path::new("clippy_lints/src/lib.rs"),
&format!("store.register_group\\(true, \"clippy::{}\"", lint_group),
r#"\]\);"#,
false,
Expand All @@ -279,7 +280,7 @@ fn update_lints(update_mode: &UpdateMode) {
if update_mode == &UpdateMode::Check && file_change {
println!(
"Not all lints defined properly. \
Please run `util/dev update_lints` to make sure all lints are defined properly."
Please run `cargo dev update_lints` to make sure all lints are defined properly."
);
std::process::exit(1);
}
Expand Down
23 changes: 3 additions & 20 deletions clippy_dev/src/new_lint.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use clippy_dev::clippy_project_root;
use std::fs::{File, OpenOptions};
use std::io;
use std::io::prelude::*;
use std::io::ErrorKind;
use std::path::{Path, PathBuf};
use std::path::Path;

pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str>) -> Result<(), io::Error> {
let pass = pass.expect("`pass` argument is validated by clap");
Expand Down Expand Up @@ -55,7 +56,7 @@ pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str
}

fn open_files(lint_name: &str) -> Result<(File, File), io::Error> {
let project_root = project_root()?;
let project_root = clippy_project_root();

let test_file_path = project_root.join("tests").join("ui").join(format!("{}.rs", lint_name));
let lint_file_path = project_root
Expand All @@ -82,24 +83,6 @@ fn open_files(lint_name: &str) -> Result<(File, File), io::Error> {
Ok((test_file, lint_file))
}

fn project_root() -> Result<PathBuf, io::Error> {
let current_dir = std::env::current_dir()?;
for path in current_dir.ancestors() {
let result = std::fs::read_to_string(path.join("Cargo.toml"));
if let Err(err) = &result {
if err.kind() == io::ErrorKind::NotFound {
continue;
}
}

let content = result?;
if content.contains("[package]\nname = \"clippy\"") {
return Ok(path.to_path_buf());
}
}
Err(io::Error::new(ErrorKind::Other, "Unable to find project root"))
}

fn to_camel_case(name: &str) -> String {
name.split('_')
.map(|s| {
Expand Down
10 changes: 5 additions & 5 deletions doc/adding_lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ lint. Fortunately, you can use the clippy dev tools to handle this for you. We
are naming our new lint `foo_functions` (lints are generally written in snake
case), and we don't need type information so it will have an early pass type
(more on this later on). To get started on this lint you can run
`./util/dev new_lint --name=foo_functions --pass=early --category=pedantic`
`cargo dev new_lint --name=foo_functions --pass=early --category=pedantic`
(category will default to nursery if not provided). This command will create
two files: `tests/ui/foo_functions.rs` and `clippy_lints/src/foo_functions.rs`,
as well as run `./util/dev update_lints` to register the new lint. Next, we'll
as well as run `cargo dev update_lints` to register the new lint. Next, we'll
open up these files and add our lint!

### Testing
Expand Down Expand Up @@ -386,7 +386,7 @@ It can be installed via `rustup`:
rustup component add rustfmt --toolchain=nightly
```

Use `./util/dev fmt` to format the whole codebase. Make sure that `rustfmt` is
Use `cargo dev fmt` to format the whole codebase. Make sure that `rustfmt` is
installed for the nightly toolchain.

### Debugging
Expand All @@ -404,9 +404,9 @@ Before submitting your PR make sure you followed all of the basic requirements:
- [ ] Followed [lint naming conventions][lint_naming]
- [ ] Added passing UI tests (including committed `.stderr` file)
- [ ] `cargo test` passes locally
- [ ] Executed `./util/dev update_lints`
- [ ] Executed `cargo dev update_lints`
- [ ] Added lint documentation
- [ ] Run `./util/dev fmt`
- [ ] Run `cargo dev fmt`

### Cheatsheet

Expand Down
2 changes: 1 addition & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! This file is managed by `util/dev update_lints`. Do not edit.
//! This file is managed by `cargo dev update_lints`. Do not edit.

pub mod lint;
pub use lint::Level;
Expand Down
2 changes: 1 addition & 1 deletion tests/fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ fn fmt() {

assert!(
output.status.success(),
"Formatting check failed. Run `./util/dev fmt` to update formatting."
"Formatting check failed. Run `cargo dev fmt` to update formatting."
);
}
2 changes: 2 additions & 0 deletions util/dev
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@
CARGO_TARGET_DIR=$(pwd)/target/
export CARGO_TARGET_DIR

echo 'Deprecated! `util/dev` usage is deprecated, please use `cargo dev` instead.'

cd clippy_dev && cargo run -- "$@"