Add lint to check for invalid `clippy:version` attributes

7 files changed, 184 insertions, 22 deletions

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<Symbol>,
 }
 
-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<Symbol> {
+    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<Symbol>,
     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<String> {
 }
 
 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
+