diff --git a/clippy_lints/src/lib.register_internal.rs b/clippy_lints/src/lib.register_internal.rs index c8c1e0262ab..d8f5db65c09 100644 --- a/clippy_lints/src/lib.register_internal.rs +++ b/clippy_lints/src/lib.register_internal.rs @@ -9,6 +9,7 @@ store.register_group(true, "clippy::internal", Some("clippy_internal"), vec![ LintId::of(utils::internal_lints::DEFAULT_LINT), LintId::of(utils::internal_lints::IF_CHAIN_STYLE), LintId::of(utils::internal_lints::INTERNING_DEFINED_SYMBOL), + LintId::of(utils::internal_lints::INVALID_CLIPPY_VERSION_ATTRIBUTE), LintId::of(utils::internal_lints::INVALID_PATHS), LintId::of(utils::internal_lints::LINT_WITHOUT_LINT_PASS), LintId::of(utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 2cb86418e3c..588debd26c6 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -16,6 +16,8 @@ store.register_lints(&[ #[cfg(feature = "internal-lints")] utils::internal_lints::INTERNING_DEFINED_SYMBOL, #[cfg(feature = "internal-lints")] + utils::internal_lints::INVALID_CLIPPY_VERSION_ATTRIBUTE, + #[cfg(feature = "internal-lints")] utils::internal_lints::INVALID_PATHS, #[cfg(feature = "internal-lints")] utils::internal_lints::LINT_WITHOUT_LINT_PASS, diff --git a/clippy_lints/src/misc_early/mod.rs b/clippy_lints/src/misc_early/mod.rs index 4b56eaebaea..6e09e25109f 100644 --- a/clippy_lints/src/misc_early/mod.rs +++ b/clippy_lints/src/misc_early/mod.rs @@ -156,6 +156,7 @@ declare_clippy_lint! { /// // Good /// let y = 123832i32; /// ``` + #[clippy::version = "1.58.0"] pub SEPARATED_LITERAL_SUFFIX, restriction, "literals whose suffix is separated by an underscore" diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 824ec53ab9c..aa722758859 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -8,6 +8,7 @@ use clippy_utils::{ paths, SpanlessEq, }; use if_chain::if_chain; +use rustc_ast as ast; use rustc_ast::ast::{Crate, ItemKind, LitKind, ModKind, NodeId}; use rustc_ast::visit::FnKind; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -25,10 +26,11 @@ use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintCon use rustc_middle::hir::map::Map; use rustc_middle::mir::interpret::ConstValue; use rustc_middle::ty; +use rustc_semver::RustcVersion; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Spanned; use rustc_span::symbol::{Symbol, SymbolStr}; -use rustc_span::{BytePos, Span}; +use rustc_span::{sym, BytePos, Span}; use rustc_typeck::hir_ty_to_ty; use std::borrow::{Borrow, Cow}; @@ -314,6 +316,26 @@ declare_clippy_lint! { "non-idiomatic `if_chain!` usage" } +declare_clippy_lint! { + /// ### What it does + /// Checks for invalid `clippy::version` attributes + /// + /// ```txt + /// +-------------------------------+ + /// | Valid values are: | + /// | * "pre 1.29.0" | + /// | * any valid semantic version | + /// +-------------------------------+ + /// \ + /// \ (^v^) + /// <( )> + /// w w + /// ``` + pub INVALID_CLIPPY_VERSION_ATTRIBUTE, + internal, + "found an invalid `clippy::version` attribute" +} + declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]); impl EarlyLintPass for ClippyLintsInternal { @@ -351,7 +373,7 @@ pub struct LintWithoutLintPass { registered_lints: FxHashSet, } -impl_lint_pass!(LintWithoutLintPass => [DEFAULT_LINT, LINT_WITHOUT_LINT_PASS]); +impl_lint_pass!(LintWithoutLintPass => [DEFAULT_LINT, LINT_WITHOUT_LINT_PASS, INVALID_CLIPPY_VERSION_ATTRIBUTE]); impl<'tcx> LateLintPass<'tcx> for LintWithoutLintPass { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { @@ -361,6 +383,8 @@ impl<'tcx> LateLintPass<'tcx> for LintWithoutLintPass { if let hir::ItemKind::Static(ty, Mutability::Not, body_id) = item.kind { if is_lint_ref_type(cx, ty) { + check_invalid_clippy_version_attribute(cx, item); + let expr = &cx.tcx.hir().body(body_id).value; if_chain! { if let ExprKind::AddrOf(_, _, inner_exp) = expr.kind; @@ -458,6 +482,48 @@ fn is_lint_ref_type<'tcx>(cx: &LateContext<'tcx>, ty: &Ty<'_>) -> bool { false } +fn check_invalid_clippy_version_attribute(cx: &LateContext<'_>, item: &'_ Item<'_>) { + if let Some(value) = extract_clippy_version_value(cx, item) { + // The `sym!` macro doesn't work as it only expects a single token. I think + // It's better to keep it this way and have a direct `Symbol::intern` call here :) + if value == Symbol::intern("pre 1.29.0") { + return; + } + + if RustcVersion::parse(&*value.as_str()).is_err() { + span_lint_and_help( + cx, + INVALID_CLIPPY_VERSION_ATTRIBUTE, + item.span, + "this item has an invalid `clippy::version` attribute", + None, + "please use a valid sematic version, see `doc/adding_lints.md`", + ); + } + } +} + +/// This function extracts the version value of a `clippy::version` attribute if the given value has +/// one +fn extract_clippy_version_value(cx: &LateContext<'_>, item: &'_ Item<'_>) -> Option { + let attrs = cx.tcx.hir().attrs(item.hir_id()); + attrs.iter().find_map(|attr| { + if_chain! { + // Identify attribute + if let ast::AttrKind::Normal(ref attr_kind, _) = &attr.kind; + if let [tool_name, attr_name] = &attr_kind.path.segments[..]; + if tool_name.ident.name == sym::clippy; + if attr_name.ident.name == sym::version; + if let Some(version) = attr.value_str(); + then { + Some(version) + } else { + None + } + } + }) +} + struct LintCollector<'a, 'tcx> { output: &'a mut FxHashSet, cx: &'a LateContext<'tcx>, diff --git a/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/clippy_lints/src/utils/internal_lints/metadata_collector.rs index 70c3169c78e..8051c58bad7 100644 --- a/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -25,7 +25,7 @@ use std::fs::{self, OpenOptions}; use std::io::prelude::*; use std::path::Path; -use crate::utils::internal_lints::is_lint_ref_type; +use crate::utils::internal_lints::{extract_clippy_version_value, is_lint_ref_type}; use clippy_utils::{ diagnostics::span_lint, last_path_segment, match_def_path, match_function_call, match_path, paths, ty::match_type, ty::walk_ptrs_ty_depth, @@ -570,25 +570,10 @@ fn extract_attr_docs(cx: &LateContext<'_>, item: &Item<'_>) -> Option { } fn get_lint_version(cx: &LateContext<'_>, item: &Item<'_>) -> String { - let attrs = cx.tcx.hir().attrs(item.hir_id()); - attrs - .iter() - .find_map(|attr| { - if_chain! { - // Identify attribute - if let ast::AttrKind::Normal(ref attr_kind, _) = &attr.kind; - if let [tool_name, attr_name] = &attr_kind.path.segments[..]; - if tool_name.ident.name == sym::clippy; - if attr_name.ident.name == sym::version; - if let Some(version) = attr.value_str(); - then { - Some(version.as_str().to_string()) - } else { - None - } - } - }) - .unwrap_or_else(|| VERION_DEFAULT_STR.to_string()) + extract_clippy_version_value(cx, item).map_or_else( + || VERION_DEFAULT_STR.to_string(), + |version| version.as_str().to_string(), + ) } fn get_lint_group_and_level_or_lint( diff --git a/tests/ui-internal/check_clippy_version_attribute.rs b/tests/ui-internal/check_clippy_version_attribute.rs new file mode 100644 index 00000000000..e1b37d42174 --- /dev/null +++ b/tests/ui-internal/check_clippy_version_attribute.rs @@ -0,0 +1,69 @@ +#![deny(clippy::internal)] +#![feature(rustc_private)] + +#[macro_use] +extern crate rustc_middle; +#[macro_use] +extern crate rustc_session; +extern crate rustc_lint; + +/////////////////////// +// Valid descriptions +/////////////////////// +declare_tool_lint! { + #[clippy::version = "pre 1.29.0"] + pub clippy::VALID_ONE, + Warn, + "One", + report_in_external_macro: true +} + +declare_tool_lint! { + #[clippy::version = "1.29.0"] + pub clippy::VALID_TWO, + Warn, + "Two", + report_in_external_macro: true +} + +declare_tool_lint! { + #[clippy::version = "1.59.0"] + pub clippy::VALID_THREE, + Warn, + "Three", + report_in_external_macro: true +} + +/////////////////////// +// Invalid attributes +/////////////////////// +declare_tool_lint! { + #[clippy::version = "1.2.3.4.5.6"] + pub clippy::INVALID_ONE, + Warn, + "One", + report_in_external_macro: true +} + +declare_tool_lint! { + #[clippy::version = "I'm a string"] + pub clippy::INVALID_TWO, + Warn, + "Two", + report_in_external_macro: true +} + +/////////////////////// +// Ignored attributes +/////////////////////// +declare_tool_lint! { + #[clippy::version] + pub clippy::IGNORED_ONE, + Warn, + "ONE", + report_in_external_macro: true +} + +declare_lint_pass!(Pass2 => [VALID_ONE, VALID_TWO, VALID_THREE, INVALID_ONE, INVALID_TWO, IGNORED_ONE]); + +fn main() {} diff --git a/tests/ui-internal/check_clippy_version_attribute.stderr b/tests/ui-internal/check_clippy_version_attribute.stderr new file mode 100644 index 00000000000..fa697ab7425 --- /dev/null +++ b/tests/ui-internal/check_clippy_version_attribute.stderr @@ -0,0 +1,38 @@ +error: this item has an invalid `clippy::version` attribute + --> $DIR/check_clippy_version_attribute.rs:40:1 + | +LL | / declare_tool_lint! { +LL | | #[clippy::version = "1.2.3.4.5.6"] +LL | | pub clippy::INVALID_ONE, +LL | | Warn, +LL | | "One", +LL | | report_in_external_macro: true +LL | | } + | |_^ + | +note: the lint level is defined here + --> $DIR/check_clippy_version_attribute.rs:1:9 + | +LL | #![deny(clippy::internal)] + | ^^^^^^^^^^^^^^^^ + = note: `#[deny(clippy::invalid_clippy_version_attribute)]` implied by `#[deny(clippy::internal)]` + = help: please use a valid sematic version, see `doc/adding_lints.md` + = note: this error originates in the macro `$crate::declare_tool_lint` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: this item has an invalid `clippy::version` attribute + --> $DIR/check_clippy_version_attribute.rs:48:1 + | +LL | / declare_tool_lint! { +LL | | #[clippy::version = "I'm a string"] +LL | | pub clippy::INVALID_TWO, +LL | | Warn, +LL | | "Two", +LL | | report_in_external_macro: true +LL | | } + | |_^ + | + = help: please use a valid sematic version, see `doc/adding_lints.md` + = note: this error originates in the macro `$crate::declare_tool_lint` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 2 previous errors +