Skip to content

Extend cargo dev rename_lint #14633

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 11 commits into
base: master
Choose a base branch
from
Open
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
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
18 changes: 16 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,19 @@ build = "build.rs"
edition = "2024"
publish = false

# [workspace]
# members = [
# "clippy_config",
# "clippy_dev",
# "clippy_dummy",
# "clippy_lints",
# "clippy_lints_internal",
# "clippy_utils",
# "lintcheck",
# "rustc_tools_util",
# ]
# resolver = "2"

[[bin]]
name = "cargo-clippy"
test = false
Expand All @@ -26,6 +39,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 +71,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
1 change: 0 additions & 1 deletion clippy_dev/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ version = "0.0.1"
edition = "2024"

[dependencies]
aho-corasick = "1.0"
chrono = { version = "0.4.38", default-features = false, features = ["clock"] }
clap = { version = "4.4", features = ["derive"] }
indoc = "1.0"
Expand Down
161 changes: 161 additions & 0 deletions clippy_dev/src/deprecate_lint.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
use crate::update_lints::{DeprecatedLint, 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) {
if let Some((prefix, _)) = name.split_once("::") {
panic!("`{name}` should not contain the `{prefix}` prefix");
}

let mut lints = find_lint_decls();
let (mut deprecated_lints, renamed_lints) = read_deprecated_lints();

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

let prefixed_name = String::from_iter(["clippy::", name]);
match deprecated_lints.binary_search_by(|x| x.name.cmp(&prefixed_name)) {
Ok(_) => {
println!("`{name}` is already deprecated");
return;
},
Err(idx) => deprecated_lints.insert(
idx,
DeprecatedLint {
name: prefixed_name,
reason: reason.into(),
version: clippy_version.rust_display().to_string(),
},
),
}

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(name, &mod_path, &mut lints).unwrap_or(false) {
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
32 changes: 11 additions & 21 deletions clippy_dev/src/fmt.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::utils::clippy_project_root;
use itertools::Itertools;
use rustc_lexer::{TokenKind, tokenize};
use shell_escape::escape;
Expand Down Expand Up @@ -104,15 +103,8 @@ fn fmt_conf(check: bool) -> Result<(), Error> {
Field,
}

let path: PathBuf = [
clippy_project_root().as_path(),
"clippy_config".as_ref(),
"src".as_ref(),
"conf.rs".as_ref(),
]
.into_iter()
.collect();
let text = fs::read_to_string(&path)?;
let path = "clippy_config/src/conf.rs";
let text = fs::read_to_string(path)?;

let (pre, conf) = text
.split_once("define_Conf! {\n")
Expand Down Expand Up @@ -203,7 +195,7 @@ fn fmt_conf(check: bool) -> Result<(), Error> {
| (State::Lints, TokenKind::Comma | TokenKind::OpenParen | TokenKind::CloseParen) => {},
_ => {
return Err(Error::Parse(
path,
PathBuf::from(path),
offset_to_line(&text, conf_offset + i),
format!("unexpected token `{}`", &conf[i..i + t.len as usize]),
));
Expand All @@ -213,7 +205,7 @@ fn fmt_conf(check: bool) -> Result<(), Error> {

if !matches!(state, State::Field) {
return Err(Error::Parse(
path,
PathBuf::from(path),
offset_to_line(&text, conf_offset + conf.len()),
"incomplete field".into(),
));
Expand Down Expand Up @@ -260,18 +252,16 @@ fn fmt_conf(check: bool) -> Result<(), Error> {
if check {
return Err(Error::CheckFailed);
}
fs::write(&path, new_text.as_bytes())?;
fs::write(path, new_text.as_bytes())?;
}
Ok(())
}

fn run_rustfmt(context: &FmtContext) -> Result<(), Error> {
let project_root = clippy_project_root();

// if we added a local rustc repo as path dependency to clippy for rust analyzer, we do NOT want to
// format because rustfmt would also format the entire rustc repo as it is a local
// dependency
if fs::read_to_string(project_root.join("Cargo.toml"))
if fs::read_to_string("Cargo.toml")
.expect("Failed to read clippy Cargo.toml")
.contains("[target.'cfg(NOT_A_PLATFORM)'.dependencies]")
{
Expand All @@ -280,12 +270,12 @@ fn run_rustfmt(context: &FmtContext) -> Result<(), Error> {

check_for_rustfmt(context)?;

cargo_fmt(context, project_root.as_path())?;
cargo_fmt(context, &project_root.join("clippy_dev"))?;
cargo_fmt(context, &project_root.join("rustc_tools_util"))?;
cargo_fmt(context, &project_root.join("lintcheck"))?;
cargo_fmt(context, ".".as_ref())?;
cargo_fmt(context, "clippy_dev".as_ref())?;
cargo_fmt(context, "rustc_tools_util".as_ref())?;
cargo_fmt(context, "lintcheck".as_ref())?;

let chunks = WalkDir::new(project_root.join("tests"))
let chunks = WalkDir::new("tests")
.into_iter()
.filter_map(|entry| {
let entry = entry.expect("failed to find tests");
Expand Down
12 changes: 10 additions & 2 deletions clippy_dev/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
#![feature(let_chains)]
#![feature(rustc_private)]
#![feature(
rustc_private,
if_let_guard,
let_chains,
os_str_slice,
os_string_truncate,
slice_split_once
)]
#![warn(
trivial_casts,
trivial_numeric_casts,
Expand All @@ -14,11 +20,13 @@
extern crate rustc_driver;
extern crate rustc_lexer;

pub mod deprecate_lint;
pub mod dogfood;
pub mod fmt;
pub mod lint;
pub mod new_lint;
pub mod release;
pub mod rename_lint;
pub mod serve;
pub mod setup;
pub mod sync;
Expand Down
Loading