Skip to content

Commit

Permalink
Auto merge of rust-lang#12394 - CBSpeir:issue-12390-refactor, r=Alexe…
Browse files Browse the repository at this point in the history
…ndoo

Refactor lints in clippy_lints::attrs into separate submodules/files

This pull request contains the changes requested in issue rust-lang#12390.

changelog: none
  • Loading branch information
bors committed Mar 2, 2024
2 parents 346b094 + 1580af6 commit 5e0afee
Show file tree
Hide file tree
Showing 16 changed files with 1,361 additions and 1,265 deletions.
1,265 changes: 0 additions & 1,265 deletions clippy_lints/src/attrs.rs

This file was deleted.

37 changes: 37 additions & 0 deletions clippy_lints/src/attrs/allow_attributes_without_reason.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
use super::{Attribute, ALLOW_ATTRIBUTES_WITHOUT_REASON};
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::is_from_proc_macro;
use rustc_ast::{MetaItemKind, NestedMetaItem};
use rustc_lint::{LateContext, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_span::sym;
use rustc_span::symbol::Symbol;

pub(super) fn check<'cx>(cx: &LateContext<'cx>, name: Symbol, items: &[NestedMetaItem], attr: &'cx Attribute) {
// Check for the feature
if !cx.tcx.features().lint_reasons {
return;
}

// Check if the reason is present
if let Some(item) = items.last().and_then(NestedMetaItem::meta_item)
&& let MetaItemKind::NameValue(_) = &item.kind
&& item.path == sym::reason
{
return;
}

// Check if the attribute is in an external macro and therefore out of the developer's control
if in_external_macro(cx.sess(), attr.span) || is_from_proc_macro(cx, &attr) {
return;
}

span_lint_and_help(
cx,
ALLOW_ATTRIBUTES_WITHOUT_REASON,
attr.span,
&format!("`{}` attribute without specifying a reason", name.as_str()),
None,
"try adding a reason at the end with `, reason = \"..\"`",
);
}
44 changes: 44 additions & 0 deletions clippy_lints/src/attrs/blanket_clippy_restriction_lints.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
use super::utils::extract_clippy_lint;
use super::BLANKET_CLIPPY_RESTRICTION_LINTS;
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then};
use rustc_ast::NestedMetaItem;
use rustc_lint::{LateContext, Level, LintContext};
use rustc_span::symbol::Symbol;
use rustc_span::{sym, DUMMY_SP};

pub(super) fn check(cx: &LateContext<'_>, name: Symbol, items: &[NestedMetaItem]) {
for lint in items {
if let Some(lint_name) = extract_clippy_lint(lint) {
if lint_name.as_str() == "restriction" && name != sym::allow {
span_lint_and_help(
cx,
BLANKET_CLIPPY_RESTRICTION_LINTS,
lint.span(),
"`clippy::restriction` is not meant to be enabled as a group",
None,
"enable the restriction lints you need individually",
);
}
}
}
}

pub(super) fn check_command_line(cx: &LateContext<'_>) {
for (name, level) in &cx.sess().opts.lint_opts {
if name == "clippy::restriction" && *level > Level::Allow {
span_lint_and_then(
cx,
BLANKET_CLIPPY_RESTRICTION_LINTS,
DUMMY_SP,
"`clippy::restriction` is not meant to be enabled as a group",
|diag| {
diag.note(format!(
"because of the command line `--{} clippy::restriction`",
level.as_str()
));
diag.help("enable the restriction lints you need individually");
},
);
}
}
}
87 changes: 87 additions & 0 deletions clippy_lints/src/attrs/deprecated_cfg_attr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
use super::{unnecessary_clippy_cfg, Attribute, DEPRECATED_CFG_ATTR, DEPRECATED_CLIPPY_CFG_ATTR};
use clippy_config::msrvs::{self, Msrv};
use clippy_utils::diagnostics::span_lint_and_sugg;
use rustc_ast::AttrStyle;
use rustc_errors::Applicability;
use rustc_lint::EarlyContext;
use rustc_span::sym;

pub(super) fn check(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msrv) {
// check cfg_attr
if attr.has_name(sym::cfg_attr)
&& let Some(items) = attr.meta_item_list()
&& items.len() == 2
&& let Some(feature_item) = items[0].meta_item()
{
// check for `rustfmt`
if feature_item.has_name(sym::rustfmt)
&& msrv.meets(msrvs::TOOL_ATTRIBUTES)
// check for `rustfmt_skip` and `rustfmt::skip`
&& let Some(skip_item) = &items[1].meta_item()
&& (skip_item.has_name(sym!(rustfmt_skip))
|| skip_item
.path
.segments
.last()
.expect("empty path in attribute")
.ident
.name
== sym::skip)
// Only lint outer attributes, because custom inner attributes are unstable
// Tracking issue: https://github.com/rust-lang/rust/issues/54726
&& attr.style == AttrStyle::Outer
{
span_lint_and_sugg(
cx,
DEPRECATED_CFG_ATTR,
attr.span,
"`cfg_attr` is deprecated for rustfmt and got replaced by tool attributes",
"use",
"#[rustfmt::skip]".to_string(),
Applicability::MachineApplicable,
);
} else {
check_deprecated_cfg_recursively(cx, feature_item);
if let Some(behind_cfg_attr) = items[1].meta_item() {
unnecessary_clippy_cfg::check(cx, feature_item, behind_cfg_attr, attr);
}
}
}
}

pub(super) fn check_clippy(cx: &EarlyContext<'_>, attr: &Attribute) {
if attr.has_name(sym::cfg)
&& let Some(list) = attr.meta_item_list()
{
for item in list.iter().filter_map(|item| item.meta_item()) {
check_deprecated_cfg_recursively(cx, item);
}
}
}

fn check_deprecated_cfg_recursively(cx: &EarlyContext<'_>, attr: &rustc_ast::MetaItem) {
if let Some(ident) = attr.ident() {
if ["any", "all", "not"].contains(&ident.name.as_str()) {
let Some(list) = attr.meta_item_list() else { return };
for item in list.iter().filter_map(|item| item.meta_item()) {
check_deprecated_cfg_recursively(cx, item);
}
} else {
check_cargo_clippy_attr(cx, attr);
}
}
}

fn check_cargo_clippy_attr(cx: &EarlyContext<'_>, item: &rustc_ast::MetaItem) {
if item.has_name(sym::feature) && item.value_str().is_some_and(|v| v.as_str() == "cargo-clippy") {
span_lint_and_sugg(
cx,
DEPRECATED_CLIPPY_CFG_ATTR,
item.span,
"`feature = \"cargo-clippy\"` was replaced by `clippy`",
"replace with",
"clippy".to_string(),
Applicability::MachineApplicable,
);
}
}
20 changes: 20 additions & 0 deletions clippy_lints/src/attrs/deprecated_semver.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
use super::DEPRECATED_SEMVER;
use clippy_utils::diagnostics::span_lint;
use rustc_ast::{LitKind, MetaItemLit};
use rustc_lint::LateContext;
use rustc_span::Span;
use semver::Version;

pub(super) fn check(cx: &LateContext<'_>, span: Span, lit: &MetaItemLit) {
if let LitKind::Str(is, _) = lit.kind {
if is.as_str() == "TBD" || Version::parse(is.as_str()).is_ok() {
return;
}
}
span_lint(
cx,
DEPRECATED_SEMVER,
span,
"the since field must contain a semver-compliant version",
);
}
52 changes: 52 additions & 0 deletions clippy_lints/src/attrs/empty_line_after.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
use super::{EMPTY_LINE_AFTER_DOC_COMMENTS, EMPTY_LINE_AFTER_OUTER_ATTR};
use clippy_utils::diagnostics::span_lint;
use clippy_utils::source::{is_present_in_source, snippet_opt, without_block_comments};
use rustc_ast::{AttrKind, AttrStyle};
use rustc_lint::EarlyContext;
use rustc_span::Span;

/// Check for empty lines after outer attributes.
///
/// Attributes and documentation comments are both considered outer attributes
/// by the AST. However, the average user likely considers them to be different.
/// Checking for empty lines after each of these attributes is split into two different
/// lints but can share the same logic.
pub(super) fn check(cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
let mut iter = item.attrs.iter().peekable();
while let Some(attr) = iter.next() {
if (matches!(attr.kind, AttrKind::Normal(..)) || matches!(attr.kind, AttrKind::DocComment(..)))
&& attr.style == AttrStyle::Outer
&& is_present_in_source(cx, attr.span)
{
let begin_of_attr_to_item = Span::new(attr.span.lo(), item.span.lo(), item.span.ctxt(), item.span.parent());
let end_of_attr_to_next_attr_or_item = Span::new(
attr.span.hi(),
iter.peek().map_or(item.span.lo(), |next_attr| next_attr.span.lo()),
item.span.ctxt(),
item.span.parent(),
);

if let Some(snippet) = snippet_opt(cx, end_of_attr_to_next_attr_or_item) {
let lines = snippet.split('\n').collect::<Vec<_>>();
let lines = without_block_comments(lines);

if lines.iter().filter(|l| l.trim().is_empty()).count() > 2 {
let (lint_msg, lint_type) = match attr.kind {
AttrKind::DocComment(..) => (
"found an empty line after a doc comment. \
Perhaps you need to use `//!` to make a comment on a module, remove the empty line, or make a regular comment with `//`?",
EMPTY_LINE_AFTER_DOC_COMMENTS,
),
AttrKind::Normal(..) => (
"found an empty line after an outer attribute. \
Perhaps you forgot to add a `!` to make it an inner attribute?",
EMPTY_LINE_AFTER_OUTER_ATTR,
),
};

span_lint(cx, lint_type, begin_of_attr_to_item, lint_msg);
}
}
}
}
}
29 changes: 29 additions & 0 deletions clippy_lints/src/attrs/inline_always.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
use super::utils::is_word;
use super::INLINE_ALWAYS;
use clippy_utils::diagnostics::span_lint;
use rustc_ast::Attribute;
use rustc_lint::LateContext;
use rustc_span::symbol::Symbol;
use rustc_span::{sym, Span};

pub(super) fn check(cx: &LateContext<'_>, span: Span, name: Symbol, attrs: &[Attribute]) {
if span.from_expansion() {
return;
}

for attr in attrs {
if let Some(values) = attr.meta_item_list() {
if values.len() != 1 || !attr.has_name(sym::inline) {
continue;
}
if is_word(&values[0], sym::always) {
span_lint(
cx,
INLINE_ALWAYS,
attr.span,
&format!("you have declared `#[inline(always)]` on `{name}`. This is usually a bad idea"),
);
}
}
}
}
51 changes: 51 additions & 0 deletions clippy_lints/src/attrs/maybe_misused_cfg.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
use super::{Attribute, MAYBE_MISUSED_CFG};
use clippy_utils::diagnostics::span_lint_and_sugg;
use rustc_ast::{MetaItemKind, NestedMetaItem};
use rustc_errors::Applicability;
use rustc_lint::EarlyContext;
use rustc_span::sym;

pub(super) fn check(cx: &EarlyContext<'_>, attr: &Attribute) {
if attr.has_name(sym::cfg)
&& let Some(items) = attr.meta_item_list()
{
check_nested_misused_cfg(cx, &items);
}
}

fn check_nested_misused_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) {
for item in items {
if let NestedMetaItem::MetaItem(meta) = item {
if let Some(ident) = meta.ident()
&& ident.name.as_str() == "features"
&& let Some(val) = meta.value_str()
{
span_lint_and_sugg(
cx,
MAYBE_MISUSED_CFG,
meta.span,
"'feature' may be misspelled as 'features'",
"did you mean",
format!("feature = \"{val}\""),
Applicability::MaybeIncorrect,
);
}
if let MetaItemKind::List(list) = &meta.kind {
check_nested_misused_cfg(cx, list);
// If this is not a list, then we check for `cfg(test)`.
} else if let Some(ident) = meta.ident()
&& matches!(ident.name.as_str(), "tests" | "Test")
{
span_lint_and_sugg(
cx,
MAYBE_MISUSED_CFG,
meta.span,
&format!("'test' may be misspelled as '{}'", ident.name.as_str()),
"did you mean",
"test".to_string(),
Applicability::MaybeIncorrect,
);
}
}
}
}
Loading

0 comments on commit 5e0afee

Please sign in to comment.