Skip to content

Prepare to split clippy_lints #14684

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
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 @@ -15,11 +21,13 @@ extern crate rustc_driver;
extern crate rustc_lexer;
extern crate rustc_literal_escaper;

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
37 changes: 18 additions & 19 deletions clippy_dev/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,18 @@
#![warn(rust_2018_idioms, unused_lifetimes)]

use clap::{Args, Parser, Subcommand};
use clippy_dev::{dogfood, fmt, lint, new_lint, release, serve, setup, sync, update_lints, utils};
use clippy_dev::{
deprecate_lint, dogfood, fmt, lint, new_lint, release, rename_lint, serve, setup, sync, update_lints, utils,
};
use std::convert::Infallible;
use std::env;

fn main() {
let dev = Dev::parse();
let clippy = utils::ClippyInfo::search_for_manifest();
if let Err(e) = env::set_current_dir(&clippy.path) {
panic!("error setting current directory to `{}`: {e}", clippy.path.display());
}

match dev.command {
DevCommand::Bless => {
Expand All @@ -20,22 +27,14 @@ fn main() {
allow_no_vcs,
} => dogfood::dogfood(fix, allow_dirty, allow_staged, allow_no_vcs),
DevCommand::Fmt { check, verbose } => fmt::run(check, verbose),
DevCommand::UpdateLints { print_only, check } => {
if print_only {
update_lints::print_lints();
} else if check {
update_lints::update(utils::UpdateMode::Check);
} else {
update_lints::update(utils::UpdateMode::Change);
}
},
DevCommand::UpdateLints { check } => update_lints::update(utils::UpdateMode::from_check(check)),
DevCommand::NewLint {
pass,
name,
category,
r#type,
msrv,
} => match new_lint::create(pass, &name, &category, r#type.as_deref(), msrv) {
} => match new_lint::create(clippy.version, pass, &name, &category, r#type.as_deref(), msrv) {
Ok(()) => update_lints::update(utils::UpdateMode::Change),
Err(e) => eprintln!("Unable to create lint: {e}"),
},
Expand Down Expand Up @@ -79,13 +78,18 @@ fn main() {
old_name,
new_name,
uplift,
} => update_lints::rename(&old_name, new_name.as_ref().unwrap_or(&old_name), uplift),
DevCommand::Deprecate { name, reason } => update_lints::deprecate(&name, &reason),
} => rename_lint::rename(
clippy.version,
&old_name,
new_name.as_ref().unwrap_or(&old_name),
uplift,
),
DevCommand::Deprecate { name, reason } => deprecate_lint::deprecate(clippy.version, &name, &reason),
DevCommand::Sync(SyncCommand { subcommand }) => match subcommand {
SyncSubcommand::UpdateNightly => sync::update_nightly(),
},
DevCommand::Release(ReleaseCommand { subcommand }) => match subcommand {
ReleaseSubcommand::BumpVersion => release::bump_version(),
ReleaseSubcommand::BumpVersion => release::bump_version(clippy.version),
},
}
}
Expand Down Expand Up @@ -135,11 +139,6 @@ enum DevCommand {
/// * lint modules in `clippy_lints/*` are visible in `src/lib.rs` via `pub mod` {n}
/// * all lints are registered in the lint store
UpdateLints {
#[arg(long)]
/// Print a table of lints to STDOUT
///
/// This does not include deprecated and internal lints. (Does not modify any files)
print_only: bool,
#[arg(long)]
/// Checks that `cargo dev update_lints` has been run. Used on CI.
check: bool,
Expand Down
Loading
Loading