From 4997ee7afcda865406352f755e9e3b44958560bd Mon Sep 17 00:00:00 2001 From: blyxyas Date: Fri, 6 Sep 2024 00:26:02 +0200 Subject: [PATCH 1/4] Turn declare_clippy_lint into a declarative macro --- clippy_lints/Cargo.toml | 1 - clippy_lints/src/declare_clippy_lint.rs | 162 +++++++++++++++++++++ clippy_lints/src/lib.rs | 6 +- clippy_utils/src/lib.rs | 2 +- declare_clippy_lint/Cargo.toml | 13 -- declare_clippy_lint/src/lib.rs | 182 ------------------------ tests/versioncheck.rs | 1 - 7 files changed, 167 insertions(+), 200 deletions(-) create mode 100644 clippy_lints/src/declare_clippy_lint.rs delete mode 100644 declare_clippy_lint/Cargo.toml delete mode 100644 declare_clippy_lint/src/lib.rs diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index d1188940b46a8..55f1d31b4addc 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -13,7 +13,6 @@ arrayvec = { version = "0.7", default-features = false } cargo_metadata = "0.18" clippy_config = { path = "../clippy_config" } clippy_utils = { path = "../clippy_utils" } -declare_clippy_lint = { path = "../declare_clippy_lint" } itertools = "0.12" quine-mc_cluskey = "0.2" regex-syntax = "0.8" diff --git a/clippy_lints/src/declare_clippy_lint.rs b/clippy_lints/src/declare_clippy_lint.rs new file mode 100644 index 0000000000000..b1e39c70baa2b --- /dev/null +++ b/clippy_lints/src/declare_clippy_lint.rs @@ -0,0 +1,162 @@ +#[macro_export] +#[allow(clippy::crate_in_macro_def)] +macro_rules! declare_clippy_lint { + (@ + $(#[doc = $lit:literal])* + pub $lint_name:ident, + $category:ident, + $lintcategory:expr, + $desc:literal, + $version_expr:expr, + $version_lit:literal + ) => { + rustc_session::declare_tool_lint! { + $(#[doc = $lit])* + #[clippy::version = $version_lit] + pub clippy::$lint_name, + $category, + $desc, + report_in_external_macro:true + } + + pub(crate) static ${concat($lint_name, _INFO)}: &'static crate::LintInfo = &crate::LintInfo { + lint: &$lint_name, + category: $lintcategory, + explanation: concat!($($lit,"\n",)*), + location: concat!(file!(), "#L", line!()), + version: $version_expr + }; + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + restriction, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Allow, crate::LintCategory::Restriction, $desc, + Some($version), $version + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + style, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Warn, crate::LintCategory::Style, $desc, + Some($version), $version + + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + correctness, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Deny, crate::LintCategory::Correctness, $desc, + Some($version), $version + + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + perf, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Warn, crate::LintCategory::Perf, $desc, + Some($version), $version + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + complexity, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Warn, crate::LintCategory::Complexity, $desc, + Some($version), $version + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + suspicious, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Warn, crate::LintCategory::Suspicious, $desc, + Some($version), $version + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + nursery, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Allow, crate::LintCategory::Nursery, $desc, + Some($version), $version + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + pedantic, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Allow, crate::LintCategory::Pedantic, $desc, + Some($version), $version + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + cargo, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Allow, crate::LintCategory::Cargo, $desc, + Some($version), $version + } + }; + + ( + $(#[doc = $lit:literal])* + pub $lint_name:ident, + internal, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Allow, crate::LintCategory::Internal, $desc, + None, "0.0.0" + } + }; +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1d41f568f3785..efc9fe6f3157d 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1,6 +1,7 @@ #![feature(array_windows)] #![feature(binary_heap_into_iter_sorted)] #![feature(box_patterns)] +#![feature(macro_metavar_expr_concat)] #![feature(control_flow_enum)] #![feature(f128)] #![feature(f16)] @@ -59,9 +60,10 @@ extern crate rustc_trait_selection; extern crate thin_vec; #[macro_use] -extern crate clippy_utils; +mod declare_clippy_lint; + #[macro_use] -extern crate declare_clippy_lint; +extern crate clippy_utils; #[cfg_attr(feature = "internal", allow(clippy::missing_clippy_version_attribute))] mod utils; diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index a925549b0bff0..bf7484f7902bb 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -4,6 +4,7 @@ #![feature(f128)] #![feature(f16)] #![feature(if_let_guard)] +#![feature(macro_metavar_expr_concat)] #![feature(let_chains)] #![feature(never_type)] #![feature(rustc_private)] @@ -129,7 +130,6 @@ use crate::consts::{ConstEvalCtxt, Constant, mir_to_const}; use crate::higher::Range; use crate::ty::{adt_and_variant_of_res, can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type}; use crate::visitors::for_each_expr_without_closures; - use rustc_middle::hir::nested_filter; #[macro_export] diff --git a/declare_clippy_lint/Cargo.toml b/declare_clippy_lint/Cargo.toml deleted file mode 100644 index 67a1f7cc72c54..0000000000000 --- a/declare_clippy_lint/Cargo.toml +++ /dev/null @@ -1,13 +0,0 @@ -[package] -name = "declare_clippy_lint" -version = "0.1.83" -edition = "2021" -publish = false - -[lib] -proc-macro = true - -[dependencies] -itertools = "0.12" -quote = "1.0.21" -syn = "2.0" diff --git a/declare_clippy_lint/src/lib.rs b/declare_clippy_lint/src/lib.rs deleted file mode 100644 index fefc1a0a6c402..0000000000000 --- a/declare_clippy_lint/src/lib.rs +++ /dev/null @@ -1,182 +0,0 @@ -#![feature(let_chains, proc_macro_span)] -// warn on lints, that are included in `rust-lang/rust`s bootstrap -#![warn(rust_2018_idioms, unused_lifetimes)] - -use proc_macro::TokenStream; -use quote::{format_ident, quote}; -use syn::parse::{Parse, ParseStream}; -use syn::{Attribute, Error, Expr, ExprLit, Ident, Lit, LitStr, Meta, Result, Token, parse_macro_input}; - -fn parse_attr(path: [&'static str; LEN], attr: &Attribute) -> Option { - if let Meta::NameValue(name_value) = &attr.meta { - let path_idents = name_value.path.segments.iter().map(|segment| &segment.ident); - - if itertools::equal(path_idents, path) - && let Expr::Lit(ExprLit { lit: Lit::Str(s), .. }) = &name_value.value - { - return Some(s.clone()); - } - } - - None -} - -struct ClippyLint { - attrs: Vec, - version: Option, - explanation: String, - name: Ident, - category: Ident, - description: LitStr, -} - -impl Parse for ClippyLint { - fn parse(input: ParseStream<'_>) -> Result { - let attrs = input.call(Attribute::parse_outer)?; - - let mut in_code = false; - let mut explanation = String::new(); - let mut version = None; - for attr in &attrs { - if let Some(lit) = parse_attr(["doc"], attr) { - let value = lit.value(); - let line = value.strip_prefix(' ').unwrap_or(&value); - - if let Some(lang) = line.strip_prefix("```") { - let tag = lang.split_once(',').map_or(lang, |(left, _)| left); - if !in_code && matches!(tag, "" | "rust" | "ignore" | "should_panic" | "no_run" | "compile_fail") { - explanation += "```rust\n"; - } else { - explanation += line; - explanation.push('\n'); - } - in_code = !in_code; - } else if !(in_code && line.starts_with("# ")) { - explanation += line; - explanation.push('\n'); - } - } else if let Some(lit) = parse_attr(["clippy", "version"], attr) { - if let Some(duplicate) = version.replace(lit) { - return Err(Error::new_spanned(duplicate, "duplicate clippy::version")); - } - } else { - return Err(Error::new_spanned(attr, "unexpected attribute")); - } - } - - input.parse::()?; - let name = input.parse()?; - input.parse::()?; - - let category = input.parse()?; - input.parse::()?; - - let description = input.parse()?; - - Ok(Self { - attrs, - version, - explanation, - name, - category, - description, - }) - } -} - -/// Macro used to declare a Clippy lint. -/// -/// Every lint declaration consists of 4 parts: -/// -/// 1. The documentation, which is used for the website and `cargo clippy --explain` -/// 2. The `LINT_NAME`. See [lint naming][lint_naming] on lint naming conventions. -/// 3. The `lint_level`, which is a mapping from *one* of our lint groups to `Allow`, `Warn` or -/// `Deny`. The lint level here has nothing to do with what lint groups the lint is a part of. -/// 4. The `description` that contains a short explanation on what's wrong with code where the lint -/// is triggered. -/// -/// Currently the categories `style`, `correctness`, `suspicious`, `complexity` and `perf` are -/// enabled by default. As said in the README.md of this repository, if the lint level mapping -/// changes, please update README.md. -/// -/// # Example -/// -/// ```ignore -/// use rustc_session::declare_tool_lint; -/// -/// declare_clippy_lint! { -/// /// ### What it does -/// /// Checks for ... (describe what the lint matches). -/// /// -/// /// ### Why is this bad? -/// /// Supply the reason for linting the code. -/// /// -/// /// ### Example -/// /// ```rust -/// /// Insert a short example of code that triggers the lint -/// /// ``` -/// /// -/// /// Use instead: -/// /// ```rust -/// /// Insert a short example of improved code that doesn't trigger the lint -/// /// ``` -/// #[clippy::version = "1.65.0"] -/// pub LINT_NAME, -/// pedantic, -/// "description" -/// } -/// ``` -/// [lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints -#[proc_macro] -pub fn declare_clippy_lint(input: TokenStream) -> TokenStream { - let ClippyLint { - attrs, - version, - explanation, - name, - category, - description, - } = parse_macro_input!(input as ClippyLint); - - let mut category = category.to_string(); - - let level = format_ident!("{}", match category.as_str() { - "correctness" => "Deny", - "style" | "suspicious" | "complexity" | "perf" => "Warn", - "pedantic" | "restriction" | "cargo" | "nursery" | "internal" => "Allow", - _ => panic!("unknown category {category}"), - },); - - let info_name = format_ident!("{name}_INFO"); - - (&mut category[0..1]).make_ascii_uppercase(); - let category_variant = format_ident!("{category}"); - - let name_span = name.span().unwrap(); - let location = format!("{}#L{}", name_span.source_file().path().display(), name_span.line()); - - let version = match version { - Some(version) => quote!(Some(#version)), - None => quote!(None), - }; - - let output = quote! { - rustc_session::declare_tool_lint! { - #(#attrs)* - pub clippy::#name, - #level, - #description, - report_in_external_macro: true - } - - pub(crate) static #info_name: &'static crate::LintInfo = &crate::LintInfo { - lint: &#name, - category: crate::LintCategory::#category_variant, - explanation: #explanation, - location: #location, - version: #version, - }; - }; - - TokenStream::from(output) -} diff --git a/tests/versioncheck.rs b/tests/versioncheck.rs index 6832833393766..e29898f068d37 100644 --- a/tests/versioncheck.rs +++ b/tests/versioncheck.rs @@ -24,7 +24,6 @@ fn consistent_clippy_crate_versions() { let clippy_version = read_version("Cargo.toml"); let paths = [ - "declare_clippy_lint/Cargo.toml", "clippy_config/Cargo.toml", "clippy_lints/Cargo.toml", "clippy_utils/Cargo.toml", From 0735031be94cd4f6a102e83e72630fae04a18090 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Wed, 25 Sep 2024 13:42:21 +0200 Subject: [PATCH 2/4] Move tag removing to --explain --- clippy_lints/src/lib.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index efc9fe6f3157d..e927d9a2d5bff 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -524,8 +524,28 @@ impl LintInfo { pub fn explain(name: &str) -> i32 { let target = format!("clippy::{}", name.to_ascii_uppercase()); + if let Some(info) = declared_lints::LINTS.iter().find(|info| info.lint.name == target) { - println!("{}", info.explanation); + // Remove tags and hidden code: + let mut explanation = String::with_capacity(128); + let mut in_code = false; + for line in info.explanation.lines().map(|line| line.trim()) { + if let Some(lang) = line.strip_prefix("```") { + let tag = lang.split_once(',').map_or(lang, |(left, _)| left); + if !in_code && matches!(tag, "" | "rust" | "ignore" | "should_panic" | "no_run" | "compile_fail") { + explanation += "```rust\n"; + } else { + explanation += line; + explanation.push('\n'); + } + in_code = !in_code; + } else if !(in_code && line.starts_with("# ")) { + explanation += line; + explanation.push('\n'); + } + } + + println!("{}", explanation); // Check if the lint has configuration let mut mdconf = get_configuration_metadata(); let name = name.to_ascii_lowercase(); From daf730caedb638696ee704e22942617cf5848e5f Mon Sep 17 00:00:00 2001 From: blyxyas Date: Thu, 26 Sep 2024 00:23:38 +0200 Subject: [PATCH 3/4] Add `sanitize_explanation` --- clippy_lints/src/lib.rs | 43 +++++++++++++++++++++++------------------ tests/compile-test.rs | 7 ++++++- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index e927d9a2d5bff..0bfbb92dfda55 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -522,30 +522,35 @@ impl LintInfo { } } -pub fn explain(name: &str) -> i32 { - let target = format!("clippy::{}", name.to_ascii_uppercase()); - - if let Some(info) = declared_lints::LINTS.iter().find(|info| info.lint.name == target) { - // Remove tags and hidden code: - let mut explanation = String::with_capacity(128); - let mut in_code = false; - for line in info.explanation.lines().map(|line| line.trim()) { - if let Some(lang) = line.strip_prefix("```") { - let tag = lang.split_once(',').map_or(lang, |(left, _)| left); - if !in_code && matches!(tag, "" | "rust" | "ignore" | "should_panic" | "no_run" | "compile_fail") { - explanation += "```rust\n"; - } else { - explanation += line; - explanation.push('\n'); - } - in_code = !in_code; - } else if !(in_code && line.starts_with("# ")) { +// Remove code tags and code behind '# 's, as they are not needed for the lint docs and --explain +pub fn sanitize_explanation(raw_docs: &str) -> String { + // Remove tags and hidden code: + let mut explanation = String::with_capacity(128); + let mut in_code = false; + for line in raw_docs.lines().map(|line| line.trim()) { + if let Some(lang) = line.strip_prefix("```") { + let tag = lang.split_once(',').map_or(lang, |(left, _)| left); + if !in_code && matches!(tag, "" | "rust" | "ignore" | "should_panic" | "no_run" | "compile_fail") { + explanation += "```rust\n"; + } else { explanation += line; explanation.push('\n'); } + in_code = !in_code; + } else if !(in_code && line.starts_with("# ")) { + explanation += line; + explanation.push('\n'); } + } - println!("{}", explanation); + explanation +} + +pub fn explain(name: &str) -> i32 { + let target = format!("clippy::{}", name.to_ascii_uppercase()); + + if let Some(info) = declared_lints::LINTS.iter().find(|info| info.lint.name == target) { + println!("{}", sanitize_explanation(info.explanation)); // Check if the lint has configuration let mut mdconf = get_configuration_metadata(); let name = name.to_ascii_lowercase(); diff --git a/tests/compile-test.rs b/tests/compile-test.rs index af2aa51925777..23dd41235bdbd 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -5,9 +5,9 @@ use cargo_metadata::Message; use cargo_metadata::diagnostic::{Applicability, Diagnostic}; use clippy_config::ClippyConfiguration; -use clippy_lints::LintInfo; use clippy_lints::declared_lints::LINTS; use clippy_lints::deprecated_lints::{DEPRECATED, DEPRECATED_VERSION, RENAMED}; +use clippy_lints::{LintInfo, sanitize_explanation}; use serde::{Deserialize, Serialize}; use test_utils::IS_RUSTC_TEST_SUITE; use ui_test::custom_flags::Flag; @@ -444,7 +444,12 @@ impl DiagnosticCollector { iter::zip(DEPRECATED, DEPRECATED_VERSION) .map(|((lint, reason), version)| LintMetadata::new_deprecated(lint, reason, version)), ) + .map(|mut metadata| { + metadata.docs = sanitize_explanation(&metadata.docs); + metadata + }) .collect(); + metadata.sort_unstable_by(|a, b| a.id.cmp(&b.id)); let json = serde_json::to_string_pretty(&metadata).unwrap(); From 13e2633f193838eadcddd5bf5842b9ce0a1f17dd Mon Sep 17 00:00:00 2001 From: blyxyas Date: Tue, 1 Oct 2024 19:39:18 +0200 Subject: [PATCH 4/4] Also sanitize configuration --- clippy_config/src/conf.rs | 24 ++++++++++++++++++++++++ clippy_config/src/lib.rs | 2 +- clippy_lints/src/lib.rs | 26 +------------------------- tests/compile-test.rs | 6 +----- 4 files changed, 27 insertions(+), 31 deletions(-) diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 757620341ccc1..df13be8009880 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -97,6 +97,30 @@ impl ConfError { } } +// Remove code tags and code behind '# 's, as they are not needed for the lint docs and --explain +pub fn sanitize_explanation(raw_docs: &str) -> String { + // Remove tags and hidden code: + let mut explanation = String::with_capacity(128); + let mut in_code = false; + for line in raw_docs.lines().map(str::trim) { + if let Some(lang) = line.strip_prefix("```") { + let tag = lang.split_once(',').map_or(lang, |(left, _)| left); + if !in_code && matches!(tag, "" | "rust" | "ignore" | "should_panic" | "no_run" | "compile_fail") { + explanation += "```rust\n"; + } else { + explanation += line; + explanation.push('\n'); + } + in_code = !in_code; + } else if !(in_code && line.starts_with("# ")) { + explanation += line; + explanation.push('\n'); + } + } + + explanation +} + macro_rules! wrap_option { () => { None diff --git a/clippy_config/src/lib.rs b/clippy_config/src/lib.rs index c63d98a0a13f9..42651521f8d7a 100644 --- a/clippy_config/src/lib.rs +++ b/clippy_config/src/lib.rs @@ -26,5 +26,5 @@ mod metadata; pub mod msrvs; pub mod types; -pub use conf::{Conf, get_configuration_metadata, lookup_conf_file}; +pub use conf::{Conf, get_configuration_metadata, lookup_conf_file, sanitize_explanation}; pub use metadata::ClippyConfiguration; diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 0bfbb92dfda55..71e7b56408e2f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -396,7 +396,7 @@ mod zero_sized_map_values; mod zombie_processes; // end lints modules, do not remove this comment, it’s used in `update_lints` -use clippy_config::{Conf, get_configuration_metadata}; +use clippy_config::{Conf, get_configuration_metadata, sanitize_explanation}; use clippy_utils::macros::FormatArgsStorage; use rustc_data_structures::fx::FxHashSet; use rustc_lint::{Lint, LintId}; @@ -522,30 +522,6 @@ impl LintInfo { } } -// Remove code tags and code behind '# 's, as they are not needed for the lint docs and --explain -pub fn sanitize_explanation(raw_docs: &str) -> String { - // Remove tags and hidden code: - let mut explanation = String::with_capacity(128); - let mut in_code = false; - for line in raw_docs.lines().map(|line| line.trim()) { - if let Some(lang) = line.strip_prefix("```") { - let tag = lang.split_once(',').map_or(lang, |(left, _)| left); - if !in_code && matches!(tag, "" | "rust" | "ignore" | "should_panic" | "no_run" | "compile_fail") { - explanation += "```rust\n"; - } else { - explanation += line; - explanation.push('\n'); - } - in_code = !in_code; - } else if !(in_code && line.starts_with("# ")) { - explanation += line; - explanation.push('\n'); - } - } - - explanation -} - pub fn explain(name: &str) -> i32 { let target = format!("clippy::{}", name.to_ascii_uppercase()); diff --git a/tests/compile-test.rs b/tests/compile-test.rs index 23dd41235bdbd..8734bd4136492 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -5,9 +5,9 @@ use cargo_metadata::Message; use cargo_metadata::diagnostic::{Applicability, Diagnostic}; use clippy_config::ClippyConfiguration; +use clippy_lints::LintInfo; use clippy_lints::declared_lints::LINTS; use clippy_lints::deprecated_lints::{DEPRECATED, DEPRECATED_VERSION, RENAMED}; -use clippy_lints::{LintInfo, sanitize_explanation}; use serde::{Deserialize, Serialize}; use test_utils::IS_RUSTC_TEST_SUITE; use ui_test::custom_flags::Flag; @@ -444,10 +444,6 @@ impl DiagnosticCollector { iter::zip(DEPRECATED, DEPRECATED_VERSION) .map(|((lint, reason), version)| LintMetadata::new_deprecated(lint, reason, version)), ) - .map(|mut metadata| { - metadata.docs = sanitize_explanation(&metadata.docs); - metadata - }) .collect(); metadata.sort_unstable_by(|a, b| a.id.cmp(&b.id));