Skip to content

Add MSRV to deprecated_cfg_attr #7944

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 9, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
//! checks for attributes
use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
use clippy_utils::match_panic_def_id;
use clippy_utils::msrvs;
use clippy_utils::source::{first_line_of_span, is_present_in_source, snippet_opt, without_block_comments};
use clippy_utils::{extract_msrv_attr, match_panic_def_id, meets_msrv};
use if_chain::if_chain;
use rustc_ast::{AttrKind, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
use rustc_errors::Applicability;
@@ -12,7 +13,8 @@ use rustc_hir::{
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_semver::RustcVersion;
use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::Span;
use rustc_span::sym;
use rustc_span::symbol::{Symbol, SymbolStr};
@@ -497,7 +499,11 @@ fn is_word(nmi: &NestedMetaItem, expected: Symbol) -> bool {
}
}

declare_lint_pass!(EarlyAttributes => [
pub struct EarlyAttributes {
pub msrv: Option<RustcVersion>,
}

impl_lint_pass!(EarlyAttributes => [
DEPRECATED_CFG_ATTR,
MISMATCHED_TARGET_OS,
EMPTY_LINE_AFTER_OUTER_ATTR,
@@ -509,9 +515,11 @@ impl EarlyLintPass for EarlyAttributes {
}

fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
check_deprecated_cfg_attr(cx, attr);
check_deprecated_cfg_attr(cx, attr, self.msrv);
check_mismatched_target_os(cx, attr);
}

extract_msrv_attr!(EarlyContext);
}

fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
@@ -548,8 +556,9 @@ fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::It
}
}

fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute) {
fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: Option<RustcVersion>) {
if_chain! {
if meets_msrv(msrv.as_ref(), &msrvs::TOOL_ATTRIBUTES);
// check cfg_attr
if attr.has_name(sym::cfg_attr);
if let Some(items) = attr.meta_item_list();
15 changes: 13 additions & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
@@ -404,10 +404,21 @@ use crate::utils::conf::TryConf;
/// level (i.e `#![cfg_attr(...)]`) will still be expanded even when using a pre-expansion pass.
///
/// Used in `./src/driver.rs`.
pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore) {
pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore, sess: &Session, conf: &Conf) {
// NOTE: Do not add any more pre-expansion passes. These should be removed eventually.

let msrv = conf.msrv.as_ref().and_then(|s| {
parse_msrv(s, None, None).or_else(|| {
sess.err(&format!(
"error reading Clippy's configuration file. `{}` is not a valid Rust version",
s
));
None
})
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like copying these lines. Is there a reason these 3 lints can't just be a part of register_plugins?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be no specific reason. IMHO, as the comments says, It might be easier to keep them separate as separate functions as they are now, since it is desirable that they are eventually removed.


store.register_pre_expansion_pass(|| Box::new(write::Write::default()));
store.register_pre_expansion_pass(|| Box::new(attrs::EarlyAttributes));
store.register_pre_expansion_pass(move || Box::new(attrs::EarlyAttributes { msrv }));
store.register_pre_expansion_pass(|| Box::new(dbg_macro::DbgMacro));
}

2 changes: 1 addition & 1 deletion clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
@@ -148,7 +148,7 @@ define_Conf! {
///
/// Suppress lints whenever the suggested change would cause breakage for other crates.
(avoid_breaking_exported_api: bool = true),
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT.
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR.
///
/// The minimum rust version that the project supports
(msrv: Option<String> = None),
2 changes: 1 addition & 1 deletion clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
@@ -27,7 +27,7 @@ msrv_aliases! {
1,36,0 { ITERATOR_COPIED }
1,35,0 { OPTION_COPIED, RANGE_CONTAINS }
1,34,0 { TRY_FROM }
1,30,0 { ITERATOR_FIND_MAP }
1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES }
1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST }
1,16,0 { STR_REPEAT }
}
2 changes: 1 addition & 1 deletion src/driver.rs
Original file line number Diff line number Diff line change
@@ -108,7 +108,7 @@ impl rustc_driver::Callbacks for ClippyCallbacks {

let conf = clippy_lints::read_conf(sess);
clippy_lints::register_plugins(lint_store, sess, &conf);
clippy_lints::register_pre_expansion_lints(lint_store);
clippy_lints::register_pre_expansion_lints(lint_store, sess, &conf);
clippy_lints::register_renamed(lint_store);
}));

13 changes: 8 additions & 5 deletions tests/ui/min_rust_version_attr.rs
Original file line number Diff line number Diff line change
@@ -137,6 +137,9 @@ fn unnest_or_patterns() {
if let TS(0, x) | TS(1, x) = TS(0, 0) {}
}

#[cfg_attr(rustfmt, rustfmt_skip)]
fn deprecated_cfg_attr() {}

fn main() {
filter_map_next();
checked_conversion();
@@ -155,9 +158,9 @@ fn main() {
unnest_or_patterns();
}

mod meets_msrv {
mod just_under_msrv {
#![feature(custom_inner_attributes)]
#![clippy::msrv = "1.45.0"]
#![clippy::msrv = "1.44.0"]

fn main() {
let s = "hello, world!";
@@ -167,9 +170,9 @@ mod meets_msrv {
}
}

mod just_under_msrv {
mod meets_msrv {
#![feature(custom_inner_attributes)]
#![clippy::msrv = "1.46.0"]
#![clippy::msrv = "1.45.0"]

fn main() {
let s = "hello, world!";
@@ -181,7 +184,7 @@ mod just_under_msrv {

mod just_above_msrv {
#![feature(custom_inner_attributes)]
#![clippy::msrv = "1.44.0"]
#![clippy::msrv = "1.46.0"]

fn main() {
let s = "hello, world!";
8 changes: 4 additions & 4 deletions tests/ui/min_rust_version_attr.stderr
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
error: stripping a prefix manually
--> $DIR/min_rust_version_attr.rs:165:24
--> $DIR/min_rust_version_attr.rs:180:24
|
LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!");
| ^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::manual-strip` implied by `-D warnings`
note: the prefix was tested here
--> $DIR/min_rust_version_attr.rs:164:9
--> $DIR/min_rust_version_attr.rs:179:9
|
LL | if s.starts_with("hello, ") {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -17,13 +17,13 @@ LL ~ assert_eq!(<stripped>.to_uppercase(), "WORLD!");
|

error: stripping a prefix manually
--> $DIR/min_rust_version_attr.rs:177:24
--> $DIR/min_rust_version_attr.rs:192:24
|
LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!");
| ^^^^^^^^^^^^^^^^^^^^
|
note: the prefix was tested here
--> $DIR/min_rust_version_attr.rs:176:9
--> $DIR/min_rust_version_attr.rs:191:9
|
LL | if s.starts_with("hello, ") {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^