Skip to content

Commit

Permalink
Auto merge of #8313 - flip1995:msrv-internal-lint, r=xFrednet
Browse files Browse the repository at this point in the history
Implement internal lint for MSRV lints

This internal lint checks if the `extract_msrv_attrs!` macro is used if
a lint has a MSRV. If not, it suggests to add this attribute to the lint
pass implementation.

Following up #8280 (comment). This currently doesn't implement the documentation check. But since this is just an extension of this lint, I think this is a good MVP of this lint.

r? `@camsteffen`

cc `@xFrednet`

changelog: none
  • Loading branch information
bors committed Mar 1, 2022
2 parents e511476 + bca4ee7 commit 28b1fe5
Show file tree
Hide file tree
Showing 10 changed files with 177 additions and 1 deletion.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ syn = { version = "1.0", features = ["full"] }
futures = "0.3"
parking_lot = "0.11.2"
tokio = { version = "1", features = ["io-util"] }
rustc-semver = "1.1"
num_cpus = "1.13"

[build-dependencies]
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ store.register_group(true, "clippy::internal", Some("clippy_internal"), vec![
LintId::of(utils::internal_lints::LINT_WITHOUT_LINT_PASS),
LintId::of(utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM),
LintId::of(utils::internal_lints::MISSING_CLIPPY_VERSION_ATTRIBUTE),
LintId::of(utils::internal_lints::MISSING_MSRV_ATTR_IMPL),
LintId::of(utils::internal_lints::OUTER_EXPN_EXPN_DATA),
LintId::of(utils::internal_lints::PRODUCE_ICE),
LintId::of(utils::internal_lints::UNNECESSARY_SYMBOL_STR),
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ store.register_lints(&[
#[cfg(feature = "internal")]
utils::internal_lints::MISSING_CLIPPY_VERSION_ATTRIBUTE,
#[cfg(feature = "internal")]
utils::internal_lints::MISSING_MSRV_ATTR_IMPL,
#[cfg(feature = "internal")]
utils::internal_lints::OUTER_EXPN_EXPN_DATA,
#[cfg(feature = "internal")]
utils::internal_lints::PRODUCE_ICE,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(utils::internal_lints::LintWithoutLintPass::default()));
store.register_late_pass(|| Box::new(utils::internal_lints::MatchTypeOnDiagItem));
store.register_late_pass(|| Box::new(utils::internal_lints::OuterExpnDataPass));
store.register_late_pass(|| Box::new(utils::internal_lints::MsrvAttrImpl));
}

store.register_late_pass(|| Box::new(utils::author::Author));
Expand Down
54 changes: 53 additions & 1 deletion clippy_lints/src/utils/internal_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use rustc_hir::{
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
use rustc_middle::hir::nested_filter;
use rustc_middle::mir::interpret::ConstValue;
use rustc_middle::ty;
use rustc_middle::ty::{self, subst::GenericArgKind};
use rustc_semver::RustcVersion;
use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::Spanned;
Expand Down Expand Up @@ -337,6 +337,15 @@ declare_clippy_lint! {
"found clippy lint without `clippy::version` attribute"
}

declare_clippy_lint! {
/// ### What it does
/// Check that the `extract_msrv_attr!` macro is used, when a lint has a MSRV.
///
pub MISSING_MSRV_ATTR_IMPL,
internal,
"checking if all necessary steps were taken when adding a MSRV to a lint"
}

declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]);

impl EarlyLintPass for ClippyLintsInternal {
Expand Down Expand Up @@ -1314,3 +1323,46 @@ fn if_chain_local_span(cx: &LateContext<'_>, local: &Local<'_>, if_chain_span: S
span.parent(),
)
}

declare_lint_pass!(MsrvAttrImpl => [MISSING_MSRV_ATTR_IMPL]);

impl LateLintPass<'_> for MsrvAttrImpl {
fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) {
if_chain! {
if let hir::ItemKind::Impl(hir::Impl {
of_trait: Some(lint_pass_trait_ref),
self_ty,
items,
..
}) = &item.kind;
if let Some(lint_pass_trait_def_id) = lint_pass_trait_ref.trait_def_id();
let is_late_pass = match_def_path(cx, lint_pass_trait_def_id, &paths::LATE_LINT_PASS);
if is_late_pass || match_def_path(cx, lint_pass_trait_def_id, &paths::EARLY_LINT_PASS);
let self_ty = hir_ty_to_ty(cx.tcx, self_ty);
if let ty::Adt(self_ty_def, _) = self_ty.kind();
if self_ty_def.is_struct();
if self_ty_def.all_fields().any(|f| {
cx.tcx
.type_of(f.did)
.walk()
.filter(|t| matches!(t.unpack(), GenericArgKind::Type(_)))
.any(|t| match_type(cx, t.expect_ty(), &paths::RUSTC_VERSION))
});
if !items.iter().any(|item| item.ident.name == sym!(enter_lint_attrs));
then {
let context = if is_late_pass { "LateContext" } else { "EarlyContext" };
let lint_pass = if is_late_pass { "LateLintPass" } else { "EarlyLintPass" };
let span = cx.sess().source_map().span_through_char(item.span, '{');
span_lint_and_sugg(
cx,
MISSING_MSRV_ATTR_IMPL,
span,
&format!("`extract_msrv_attr!` macro missing from `{lint_pass}` implementation"),
&format!("add `extract_msrv_attr!({context})` to the `{lint_pass}` implementation"),
format!("{}\n extract_msrv_attr!({context});", snippet(cx, span, "..")),
Applicability::MachineApplicable,
);
}
}
}
}
6 changes: 6 additions & 0 deletions clippy_utils/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ pub const DIR_BUILDER: [&str; 3] = ["std", "fs", "DirBuilder"];
pub const DISPLAY_TRAIT: [&str; 3] = ["core", "fmt", "Display"];
#[cfg(feature = "internal")]
pub const EARLY_CONTEXT: [&str; 2] = ["rustc_lint", "EarlyContext"];
#[cfg(feature = "internal")]
pub const EARLY_LINT_PASS: [&str; 3] = ["rustc_lint", "passes", "EarlyLintPass"];
pub const EXIT: [&str; 3] = ["std", "process", "exit"];
pub const F32_EPSILON: [&str; 4] = ["core", "f32", "<impl f32>", "EPSILON"];
pub const F64_EPSILON: [&str; 4] = ["core", "f64", "<impl f64>", "EPSILON"];
Expand Down Expand Up @@ -67,6 +69,8 @@ pub const KW_MODULE: [&str; 3] = ["rustc_span", "symbol", "kw"];
#[cfg(feature = "internal")]
pub const LATE_CONTEXT: [&str; 2] = ["rustc_lint", "LateContext"];
#[cfg(feature = "internal")]
pub const LATE_LINT_PASS: [&str; 3] = ["rustc_lint", "passes", "LateLintPass"];
#[cfg(feature = "internal")]
pub const LINT: [&str; 2] = ["rustc_lint_defs", "Lint"];
pub const MUTEX_GUARD: [&str; 4] = ["std", "sync", "mutex", "MutexGuard"];
pub const OPEN_OPTIONS: [&str; 3] = ["std", "fs", "OpenOptions"];
Expand Down Expand Up @@ -126,6 +130,8 @@ pub const REGEX_SET_NEW: [&str; 5] = ["regex", "re_set", "unicode", "RegexSet",
pub const RESULT: [&str; 3] = ["core", "result", "Result"];
pub const RESULT_ERR: [&str; 4] = ["core", "result", "Result", "Err"];
pub const RESULT_OK: [&str; 4] = ["core", "result", "Result", "Ok"];
#[cfg(feature = "internal")]
pub const RUSTC_VERSION: [&str; 2] = ["rustc_semver", "RustcVersion"];
pub const RWLOCK_READ_GUARD: [&str; 4] = ["std", "sync", "rwlock", "RwLockReadGuard"];
pub const RWLOCK_WRITE_GUARD: [&str; 4] = ["std", "sync", "rwlock", "RwLockWriteGuard"];
pub const SERDE_DESERIALIZE: [&str; 3] = ["serde", "de", "Deserialize"];
Expand Down
3 changes: 3 additions & 0 deletions tests/compile-test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ static TEST_DEPENDENCIES: &[&str] = &[
"syn",
"tokio",
"parking_lot",
"rustc_semver",
];

// Test dependencies may need an `extern crate` here to ensure that they show up
Expand All @@ -53,6 +54,8 @@ extern crate parking_lot;
#[allow(unused_extern_crates)]
extern crate quote;
#[allow(unused_extern_crates)]
extern crate rustc_semver;
#[allow(unused_extern_crates)]
extern crate syn;
#[allow(unused_extern_crates)]
extern crate tokio;
Expand Down
40 changes: 40 additions & 0 deletions tests/ui-internal/invalid_msrv_attr_impl.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// run-rustfix

#![deny(clippy::internal)]
#![allow(clippy::missing_clippy_version_attribute)]
#![feature(rustc_private)]

extern crate rustc_ast;
extern crate rustc_hir;
extern crate rustc_lint;
extern crate rustc_middle;
#[macro_use]
extern crate rustc_session;
use clippy_utils::extract_msrv_attr;
use rustc_hir::Expr;
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass};
use rustc_semver::RustcVersion;

declare_lint! {
pub TEST_LINT,
Warn,
""
}

struct Pass {
msrv: Option<RustcVersion>,
}

impl_lint_pass!(Pass => [TEST_LINT]);

impl LateLintPass<'_> for Pass {
extract_msrv_attr!(LateContext);
fn check_expr(&mut self, _: &LateContext<'_>, _: &Expr<'_>) {}
}

impl EarlyLintPass for Pass {
extract_msrv_attr!(EarlyContext);
fn check_expr(&mut self, _: &EarlyContext<'_>, _: &rustc_ast::Expr) {}
}

fn main() {}
38 changes: 38 additions & 0 deletions tests/ui-internal/invalid_msrv_attr_impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// run-rustfix

#![deny(clippy::internal)]
#![allow(clippy::missing_clippy_version_attribute)]
#![feature(rustc_private)]

extern crate rustc_ast;
extern crate rustc_hir;
extern crate rustc_lint;
extern crate rustc_middle;
#[macro_use]
extern crate rustc_session;
use clippy_utils::extract_msrv_attr;
use rustc_hir::Expr;
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass};
use rustc_semver::RustcVersion;

declare_lint! {
pub TEST_LINT,
Warn,
""
}

struct Pass {
msrv: Option<RustcVersion>,
}

impl_lint_pass!(Pass => [TEST_LINT]);

impl LateLintPass<'_> for Pass {
fn check_expr(&mut self, _: &LateContext<'_>, _: &Expr<'_>) {}
}

impl EarlyLintPass for Pass {
fn check_expr(&mut self, _: &EarlyContext<'_>, _: &rustc_ast::Expr) {}
}

fn main() {}
32 changes: 32 additions & 0 deletions tests/ui-internal/invalid_msrv_attr_impl.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
error: `extract_msrv_attr!` macro missing from `LateLintPass` implementation
--> $DIR/invalid_msrv_attr_impl.rs:30:1
|
LL | impl LateLintPass<'_> for Pass {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/invalid_msrv_attr_impl.rs:3:9
|
LL | #![deny(clippy::internal)]
| ^^^^^^^^^^^^^^^^
= note: `#[deny(clippy::missing_msrv_attr_impl)]` implied by `#[deny(clippy::internal)]`
help: add `extract_msrv_attr!(LateContext)` to the `LateLintPass` implementation
|
LL + impl LateLintPass<'_> for Pass {
LL + extract_msrv_attr!(LateContext);
|

error: `extract_msrv_attr!` macro missing from `EarlyLintPass` implementation
--> $DIR/invalid_msrv_attr_impl.rs:34:1
|
LL | impl EarlyLintPass for Pass {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: add `extract_msrv_attr!(EarlyContext)` to the `EarlyLintPass` implementation
|
LL + impl EarlyLintPass for Pass {
LL + extract_msrv_attr!(EarlyContext);
|

error: aborting due to 2 previous errors

0 comments on commit 28b1fe5

Please sign in to comment.