Skip to content

Refactor and speed up cargo dev fmt #14638

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open
2 changes: 1 addition & 1 deletion .github/workflows/clippy_mq.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ jobs:
run: cargo test --features internal -- --skip dogfood

- name: Test clippy_lints
run: cargo test --features internal
run: cargo test
working-directory: clippy_lints

- name: Test clippy_utils
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/clippy_pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
run: cargo test --features internal

- name: Test clippy_lints
run: cargo test --features internal
run: cargo test
working-directory: clippy_lints

- name: Test clippy_utils
Expand Down
7 changes: 3 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
[package]
name = "clippy"
# begin autogenerated version
version = "0.1.88"
# end autogenerated version
description = "A bunch of helpful lints to avoid common pitfalls in Rust"
repository = "https://github.com/rust-lang/rust-clippy"
readme = "README.md"
Expand All @@ -26,6 +24,7 @@ path = "src/driver.rs"
clippy_config = { path = "clippy_config" }
clippy_lints = { path = "clippy_lints" }
rustc_tools_util = { path = "rustc_tools_util", version = "0.4.2" }
clippy_lints_internal = { path = "clippy_lints_internal", optional = true }
tempfile = { version = "3.3", optional = true }
termize = "0.1"
color-print = "0.3.4"
Expand Down Expand Up @@ -57,8 +56,8 @@ tokio = { version = "1", features = ["io-util"] }
rustc_tools_util = { path = "rustc_tools_util", version = "0.4.2" }

[features]
integration = ["tempfile"]
internal = ["clippy_lints/internal", "tempfile"]
integration = ["dep:tempfile"]
internal = ["dep:clippy_lints_internal", "dep:tempfile"]

[package.metadata.rust-analyzer]
# This package uses #[feature(rustc_private)]
Expand Down
3 changes: 1 addition & 2 deletions book/src/development/defining_lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ lint involves some boilerplate code.

A lint type is the category of items and expressions in which your lint focuses on.

As of the writing of this documentation update, there are 12 _types_ of lints
As of the writing of this documentation update, there are 11 _types_ of lints
besides the numerous standalone lints living under `clippy_lints/src/`:

- `cargo`
Expand All @@ -23,7 +23,6 @@ besides the numerous standalone lints living under `clippy_lints/src/`:
- `transmute`
- `types`
- `unit_types`
- `utils / internal` (Clippy internal lints)

These types group together lints that share some common behaviors. For instance,
`functions` groups together lints that deal with some aspects of functions in
Expand Down
2 changes: 0 additions & 2 deletions clippy_config/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
[package]
name = "clippy_config"
# begin autogenerated version
version = "0.1.88"
# end autogenerated version
edition = "2024"
publish = false

Expand Down
1 change: 0 additions & 1 deletion clippy_dev/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ clap = { version = "4.4", features = ["derive"] }
indoc = "1.0"
itertools = "0.12"
opener = "0.7"
shell-escape = "0.1"
walkdir = "2.3"

[package.metadata.rust-analyzer]
Expand Down
174 changes: 174 additions & 0 deletions clippy_dev/src/deprecate_lint.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
use crate::update_lints::{
DeprecatedLint, DeprecatedLints, Lint, find_lint_decls, generate_lint_files, read_deprecated_lints,
};
use crate::utils::{UpdateMode, Version};
use std::ffi::OsStr;
use std::path::{Path, PathBuf};
use std::{fs, io};

/// Runs the `deprecate` command
///
/// This does the following:
/// * Adds an entry to `deprecated_lints.rs`.
/// * Removes the lint declaration (and the entire file if applicable)
///
/// # Panics
///
/// If a file path could not read from or written to
pub fn deprecate(clippy_version: Version, name: &str, reason: &str) {
let prefixed_name = if name.starts_with("clippy::") {
name.to_owned()
} else {
format!("clippy::{name}")
};
let stripped_name = &prefixed_name[8..];

let mut lints = find_lint_decls();
let DeprecatedLints {
renamed: renamed_lints,
deprecated: mut deprecated_lints,
file: mut deprecated_file,
contents: mut deprecated_contents,
deprecated_end,
..
} = read_deprecated_lints();

let Some(lint) = lints.iter().find(|l| l.name == stripped_name) else {
eprintln!("error: failed to find lint `{name}`");
return;
};

let mod_path = {
let mut mod_path = PathBuf::from(format!("clippy_lints/src/{}", lint.module));
if mod_path.is_dir() {
mod_path = mod_path.join("mod");
}

mod_path.set_extension("rs");
mod_path
};

if remove_lint_declaration(stripped_name, &mod_path, &mut lints).unwrap_or(false) {
deprecated_contents.insert_str(
deprecated_end as usize,
&format!(
" #[clippy::version = \"{}\"]\n (\"{}\", \"{}\"),\n",
clippy_version.rust_display(),
prefixed_name,
reason,
),
);
deprecated_file.replace_contents(deprecated_contents.as_bytes());
drop(deprecated_file);

deprecated_lints.push(DeprecatedLint {
name: prefixed_name,
reason: reason.into(),
});

generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints);
println!("info: `{name}` has successfully been deprecated");
println!("note: you must run `cargo uitest` to update the test results");
} else {
eprintln!("error: lint not found");
}
}

fn remove_lint_declaration(name: &str, path: &Path, lints: &mut Vec<Lint>) -> io::Result<bool> {
fn remove_lint(name: &str, lints: &mut Vec<Lint>) {
lints.iter().position(|l| l.name == name).map(|pos| lints.remove(pos));
}

fn remove_test_assets(name: &str) {
let test_file_stem = format!("tests/ui/{name}");
let path = Path::new(&test_file_stem);

// Some lints have their own directories, delete them
if path.is_dir() {
let _ = fs::remove_dir_all(path);
return;
}

// Remove all related test files
let _ = fs::remove_file(path.with_extension("rs"));
let _ = fs::remove_file(path.with_extension("stderr"));
let _ = fs::remove_file(path.with_extension("fixed"));
}

fn remove_impl_lint_pass(lint_name_upper: &str, content: &mut String) {
let impl_lint_pass_start = content.find("impl_lint_pass!").unwrap_or_else(|| {
content
.find("declare_lint_pass!")
.unwrap_or_else(|| panic!("failed to find `impl_lint_pass`"))
});
let mut impl_lint_pass_end = content[impl_lint_pass_start..]
.find(']')
.expect("failed to find `impl_lint_pass` terminator");

impl_lint_pass_end += impl_lint_pass_start;
if let Some(lint_name_pos) = content[impl_lint_pass_start..impl_lint_pass_end].find(lint_name_upper) {
let mut lint_name_end = impl_lint_pass_start + (lint_name_pos + lint_name_upper.len());
for c in content[lint_name_end..impl_lint_pass_end].chars() {
// Remove trailing whitespace
if c == ',' || c.is_whitespace() {
lint_name_end += 1;
} else {
break;
}
}

content.replace_range(impl_lint_pass_start + lint_name_pos..lint_name_end, "");
}
}

if path.exists()
&& let Some(lint) = lints.iter().find(|l| l.name == name)
{
if lint.module == name {
// The lint name is the same as the file, we can just delete the entire file
fs::remove_file(path)?;
} else {
// We can't delete the entire file, just remove the declaration

if let Some(Some("mod.rs")) = path.file_name().map(OsStr::to_str) {
// Remove clippy_lints/src/some_mod/some_lint.rs
let mut lint_mod_path = path.to_path_buf();
lint_mod_path.set_file_name(name);
lint_mod_path.set_extension("rs");

let _ = fs::remove_file(lint_mod_path);
}

let mut content =
fs::read_to_string(path).unwrap_or_else(|_| panic!("failed to read `{}`", path.to_string_lossy()));

eprintln!(
"warn: you will have to manually remove any code related to `{name}` from `{}`",
path.display()
);

assert!(
content[lint.declaration_range.clone()].contains(&name.to_uppercase()),
"error: `{}` does not contain lint `{}`'s declaration",
path.display(),
lint.name
);

// Remove lint declaration (declare_clippy_lint!)
content.replace_range(lint.declaration_range.clone(), "");

// Remove the module declaration (mod xyz;)
let mod_decl = format!("\nmod {name};");
content = content.replacen(&mod_decl, "", 1);

remove_impl_lint_pass(&lint.name.to_uppercase(), &mut content);
fs::write(path, content).unwrap_or_else(|_| panic!("failed to write to `{}`", path.to_string_lossy()));
}

remove_test_assets(name);
remove_lint(name, lints);
return Ok(true);
}

Ok(false)
}
5 changes: 2 additions & 3 deletions clippy_dev/src/dogfood.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{clippy_project_root, exit_if_err};
use crate::utils::exit_if_err;
use std::process::Command;

/// # Panics
Expand All @@ -8,8 +8,7 @@ use std::process::Command;
pub fn dogfood(fix: bool, allow_dirty: bool, allow_staged: bool, allow_no_vcs: bool) {
let mut cmd = Command::new("cargo");

cmd.current_dir(clippy_project_root())
.args(["test", "--test", "dogfood"])
cmd.args(["test", "--test", "dogfood"])
.args(["--features", "internal"])
.args(["--", "dogfood_clippy", "--nocapture"]);

Expand Down
Loading