From 51cd5a866728539c1d1db81c437c474ffbef7ccf Mon Sep 17 00:00:00 2001 From: Serial <69764315+Serial-ATA@users.noreply.github.com> Date: Thu, 14 Jul 2022 15:58:38 -0400 Subject: [PATCH] Add `--type` flag to `dev new_lint` --- clippy_dev/src/main.rs | 9 +- clippy_dev/src/new_lint.rs | 294 ++++++++++++++++++++++++++---- clippy_dev/src/update_lints.rs | 10 +- clippy_lints/src/cargo/mod.rs | 10 +- clippy_lints/src/matches/mod.rs | 20 +- clippy_lints/src/operators/mod.rs | 12 +- 6 files changed, 295 insertions(+), 60 deletions(-) diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index a29ba2d0c..2008942d0 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -37,6 +37,7 @@ fn main() { matches.get_one::("pass"), matches.get_one::("name"), matches.get_one::("category"), + matches.get_one::("type"), matches.contains_id("msrv"), ) { Ok(_) => update_lints::update(update_lints::UpdateMode::Change), @@ -157,7 +158,8 @@ fn get_clap_config() -> ArgMatches { .help("Specify whether the lint runs during the early or late pass") .takes_value(true) .value_parser([PossibleValue::new("early"), PossibleValue::new("late")]) - .required(true), + .conflicts_with("type") + .required_unless_present("type"), Arg::new("name") .short('n') .long("name") @@ -183,6 +185,11 @@ fn get_clap_config() -> ArgMatches { PossibleValue::new("internal_warn"), ]) .takes_value(true), + Arg::new("type") + .long("type") + .help("What directory the lint belongs in") + .takes_value(true) + .required(false), Arg::new("msrv").long("msrv").help("Add MSRV config code to the lint"), ]), Command::new("setup") diff --git a/clippy_dev/src/new_lint.rs b/clippy_dev/src/new_lint.rs index 7d7e760ef..af22cb899 100644 --- a/clippy_dev/src/new_lint.rs +++ b/clippy_dev/src/new_lint.rs @@ -1,5 +1,5 @@ use crate::clippy_project_root; -use indoc::indoc; +use indoc::{indoc, writedoc}; use std::fmt::Write as _; use std::fs::{self, OpenOptions}; use std::io::prelude::*; @@ -10,6 +10,7 @@ struct LintData<'a> { pass: &'a str, name: &'a str, category: &'a str, + ty: Option<&'a str>, project_root: PathBuf, } @@ -38,25 +39,35 @@ pub fn create( pass: Option<&String>, lint_name: Option<&String>, category: Option<&String>, + ty: Option<&String>, msrv: bool, ) -> io::Result<()> { let lint = LintData { - pass: pass.expect("`pass` argument is validated by clap"), + pass: pass.map_or("", String::as_str), name: lint_name.expect("`name` argument is validated by clap"), category: category.expect("`category` argument is validated by clap"), + ty: ty.map(String::as_str), project_root: clippy_project_root(), }; create_lint(&lint, msrv).context("Unable to create lint implementation")?; create_test(&lint).context("Unable to create a test for the new lint")?; - add_lint(&lint, msrv).context("Unable to add lint to clippy_lints/src/lib.rs") + + if lint.ty.is_none() { + add_lint(&lint, msrv).context("Unable to add lint to clippy_lints/src/lib.rs")?; + } + + Ok(()) } fn create_lint(lint: &LintData<'_>, enable_msrv: bool) -> io::Result<()> { - let lint_contents = get_lint_file_contents(lint, enable_msrv); - - let lint_path = format!("clippy_lints/src/{}.rs", lint.name); - write_file(lint.project_root.join(&lint_path), lint_contents.as_bytes()) + if let Some(ty) = lint.ty { + generate_from_ty(lint, enable_msrv, ty) + } else { + let lint_contents = get_lint_file_contents(lint, enable_msrv); + let lint_path = format!("clippy_lints/src/{}.rs", lint.name); + write_file(lint.project_root.join(&lint_path), lint_contents.as_bytes()) + } } fn create_test(lint: &LintData<'_>) -> io::Result<()> { @@ -204,7 +215,6 @@ fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String { }, }; - let version = get_stabilization_version(); let lint_name = lint.name; let category = lint.category; let name_camel = to_camel_case(lint.name); @@ -238,32 +248,7 @@ fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String { ) }); - let _ = write!( - result, - indoc! {r#" - declare_clippy_lint! {{ - /// ### What it does - /// - /// ### Why is this bad? - /// - /// ### Example - /// ```rust - /// // example code where clippy issues a warning - /// ``` - /// Use instead: - /// ```rust - /// // example code which does not raise clippy warning - /// ``` - #[clippy::version = "{version}"] - pub {name_upper}, - {category}, - "default lint description" - }} - "#}, - version = version, - name_upper = name_upper, - category = category, - ); + let _ = write!(result, "{}", get_lint_declaration(&name_upper, category)); result.push_str(&if enable_msrv { format!( @@ -312,6 +297,247 @@ fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String { result } +fn get_lint_declaration(name_upper: &str, category: &str) -> String { + format!( + indoc! {r#" + declare_clippy_lint! {{ + /// ### What it does + /// + /// ### Why is this bad? + /// + /// ### Example + /// ```rust + /// // example code where clippy issues a warning + /// ``` + /// Use instead: + /// ```rust + /// // example code which does not raise clippy warning + /// ``` + #[clippy::version = "{version}"] + pub {name_upper}, + {category}, + "default lint description" + }} + "#}, + version = get_stabilization_version(), + name_upper = name_upper, + category = category, + ) +} + +fn generate_from_ty(lint: &LintData<'_>, enable_msrv: bool, ty: &str) -> io::Result<()> { + if ty == "cargo" { + assert_eq!( + lint.category, "cargo", + "Lints of type `cargo` must have the `cargo` category" + ); + } + + let ty_dir = lint.project_root.join(format!("clippy_lints/src/{}", ty)); + assert!( + ty_dir.exists() && ty_dir.is_dir(), + "Directory `{}` does not exist!", + ty_dir.display() + ); + + let lint_file_path = ty_dir.join(format!("{}.rs", lint.name)); + assert!( + !lint_file_path.exists(), + "File `{}` already exists", + lint_file_path.display() + ); + + let mod_file_path = ty_dir.join("mod.rs"); + let context_import = setup_mod_file(&mod_file_path, lint)?; + + let name_upper = lint.name.to_uppercase(); + let mut lint_file_contents = String::new(); + + if enable_msrv { + let _ = writedoc!( + lint_file_contents, + r#" + use clippy_utils::{{meets_msrv, msrvs}}; + use rustc_lint::{{{context_import}, LintContext}}; + use rustc_semver::RustcVersion; + + use super::{name_upper}; + + // TODO: Adjust the parameters as necessary + pub(super) fn check(cx: &{context_import}, msrv: Option) {{ + if !meets_msrv(msrv, todo!("Add a new entry in `clippy_utils/src/msrvs`")) {{ + return; + }} + todo!(); + }} + "#, + context_import = context_import, + name_upper = name_upper, + ); + } else { + let _ = writedoc!( + lint_file_contents, + r#" + use rustc_lint::{{{context_import}, LintContext}}; + + use super::{name_upper}; + + // TODO: Adjust the parameters as necessary + pub(super) fn check(cx: &{context_import}) {{ + todo!(); + }} + "#, + context_import = context_import, + name_upper = name_upper, + ); + } + + write_file(lint_file_path, lint_file_contents)?; + + Ok(()) +} + +#[allow(clippy::too_many_lines)] +fn setup_mod_file(path: &Path, lint: &LintData<'_>) -> io::Result<&'static str> { + use super::update_lints::{match_tokens, LintDeclSearchResult}; + use rustc_lexer::TokenKind; + + let lint_name_upper = lint.name.to_uppercase(); + + let mut file_contents = fs::read_to_string(path)?; + assert!( + !file_contents.contains(&lint_name_upper), + "Lint `{}` already defined in `{}`", + lint.name, + path.display() + ); + + let mut offset = 0usize; + let mut last_decl_curly_offset = None; + let mut lint_context = None; + + let mut iter = rustc_lexer::tokenize(&file_contents).map(|t| { + let range = offset..offset + t.len; + offset = range.end; + + LintDeclSearchResult { + token_kind: t.kind, + content: &file_contents[range.clone()], + range, + } + }); + + // Find both the last lint declaration (declare_clippy_lint!) and the lint pass impl + while let Some(LintDeclSearchResult { content, .. }) = iter.find(|result| result.token_kind == TokenKind::Ident) { + let mut iter = iter + .by_ref() + .filter(|t| !matches!(t.token_kind, TokenKind::Whitespace | TokenKind::LineComment { .. })); + + match content { + "declare_clippy_lint" => { + // matches `!{` + match_tokens!(iter, Bang OpenBrace); + if let Some(LintDeclSearchResult { range, .. }) = + iter.find(|result| result.token_kind == TokenKind::CloseBrace) + { + last_decl_curly_offset = Some(range.end); + } + }, + "impl" => { + let mut token = iter.next(); + match token { + // matches <'foo> + Some(LintDeclSearchResult { + token_kind: TokenKind::Lt, + .. + }) => { + match_tokens!(iter, Lifetime { .. } Gt); + token = iter.next(); + }, + None => break, + _ => {}, + } + + if let Some(LintDeclSearchResult { + token_kind: TokenKind::Ident, + content, + .. + }) = token + { + // Get the appropriate lint context struct + lint_context = match content { + "LateLintPass" => Some("LateContext"), + "EarlyLintPass" => Some("EarlyContext"), + _ => continue, + }; + } + }, + _ => {}, + } + } + + drop(iter); + + let last_decl_curly_offset = + last_decl_curly_offset.unwrap_or_else(|| panic!("No lint declarations found in `{}`", path.display())); + let lint_context = + lint_context.unwrap_or_else(|| panic!("No lint pass implementation found in `{}`", path.display())); + + // Add the lint declaration to `mod.rs` + file_contents.replace_range( + // Remove the trailing newline, which should always be present + last_decl_curly_offset..=last_decl_curly_offset, + &format!("\n\n{}", get_lint_declaration(&lint_name_upper, lint.category)), + ); + + // Add the lint to `impl_lint_pass`/`declare_lint_pass` + let impl_lint_pass_start = file_contents.find("impl_lint_pass!").unwrap_or_else(|| { + file_contents + .find("declare_lint_pass!") + .unwrap_or_else(|| panic!("failed to find `impl_lint_pass`/`declare_lint_pass`")) + }); + + let mut arr_start = file_contents[impl_lint_pass_start..].find('[').unwrap_or_else(|| { + panic!("malformed `impl_lint_pass`/`declare_lint_pass`"); + }); + + arr_start += impl_lint_pass_start; + + let mut arr_end = file_contents[arr_start..] + .find(']') + .expect("failed to find `impl_lint_pass` terminator"); + + arr_end += arr_start; + + let mut arr_content = file_contents[arr_start + 1..arr_end].to_string(); + arr_content.retain(|c| !c.is_whitespace()); + + let mut new_arr_content = String::new(); + for ident in arr_content + .split(',') + .chain(std::iter::once(&*lint_name_upper)) + .filter(|s| !s.is_empty()) + { + let _ = write!(new_arr_content, "\n {},", ident); + } + new_arr_content.push('\n'); + + file_contents.replace_range(arr_start + 1..arr_end, &new_arr_content); + + // Just add the mod declaration at the top, it'll be fixed by rustfmt + file_contents.insert_str(0, &format!("mod {};\n", &lint.name)); + + let mut file = OpenOptions::new() + .write(true) + .truncate(true) + .open(path) + .context(format!("trying to open: `{}`", path.display()))?; + file.write_all(file_contents.as_bytes()) + .context(format!("writing to file: `{}`", path.display()))?; + + Ok(lint_context) +} + #[test] fn test_camel_case() { let s = "a_lint"; diff --git a/clippy_dev/src/update_lints.rs b/clippy_dev/src/update_lints.rs index c089f4d8c..aed38bc28 100644 --- a/clippy_dev/src/update_lints.rs +++ b/clippy_dev/src/update_lints.rs @@ -824,10 +824,12 @@ macro_rules! match_tokens { } } -struct LintDeclSearchResult<'a> { - token_kind: TokenKind, - content: &'a str, - range: Range, +pub(crate) use match_tokens; + +pub(crate) struct LintDeclSearchResult<'a> { + pub token_kind: TokenKind, + pub content: &'a str, + pub range: Range, } /// Parse a source file looking for `declare_clippy_lint` macro invocations. diff --git a/clippy_lints/src/cargo/mod.rs b/clippy_lints/src/cargo/mod.rs index abe95c666..9f45db86a 100644 --- a/clippy_lints/src/cargo/mod.rs +++ b/clippy_lints/src/cargo/mod.rs @@ -1,3 +1,8 @@ +mod common_metadata; +mod feature_name; +mod multiple_crate_versions; +mod wildcard_dependencies; + use cargo_metadata::MetadataCommand; use clippy_utils::diagnostics::span_lint; use clippy_utils::is_lint_allowed; @@ -6,11 +11,6 @@ use rustc_lint::{LateContext, LateLintPass, Lint}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::DUMMY_SP; -mod common_metadata; -mod feature_name; -mod multiple_crate_versions; -mod wildcard_dependencies; - declare_clippy_lint! { /// ### What it does /// Checks to see if all common metadata is defined in diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index a3d69a4d0..b638f2716 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -1,13 +1,3 @@ -use clippy_utils::source::{snippet_opt, span_starts_with, walk_span_to_context}; -use clippy_utils::{higher, in_constant, meets_msrv, msrvs}; -use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat}; -use rustc_lexer::{tokenize, TokenKind}; -use rustc_lint::{LateContext, LateLintPass, LintContext}; -use rustc_middle::lint::in_external_macro; -use rustc_semver::RustcVersion; -use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::{Span, SpanData, SyntaxContext}; - mod collapsible_match; mod infallible_destructuring_match; mod manual_map; @@ -31,6 +21,16 @@ mod single_match; mod try_err; mod wild_in_or_pats; +use clippy_utils::source::{snippet_opt, span_starts_with, walk_span_to_context}; +use clippy_utils::{higher, in_constant, meets_msrv, msrvs}; +use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat}; +use rustc_lexer::{tokenize, TokenKind}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_semver::RustcVersion; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::{Span, SpanData, SyntaxContext}; + declare_clippy_lint! { /// ### What it does /// Checks for matches with a single arm where an `if let` diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index c688d94bb..bb6d99406 100644 --- a/clippy_lints/src/operators/mod.rs +++ b/clippy_lints/src/operators/mod.rs @@ -1,9 +1,3 @@ -use rustc_hir::{Body, Expr, ExprKind, UnOp}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_tool_lint, impl_lint_pass}; - -pub(crate) mod arithmetic; - mod absurd_extreme_comparisons; mod assign_op_pattern; mod bit_mask; @@ -27,6 +21,12 @@ mod ptr_eq; mod self_assignment; mod verbose_bit_mask; +pub(crate) mod arithmetic; + +use rustc_hir::{Body, Expr, ExprKind, UnOp}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; + declare_clippy_lint! { /// ### What it does /// Checks for comparisons where one side of the relation is