Skip to content
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

Add MSRV to deprecated_cfg_attr #7944

Merged
merged 1 commit into from
Nov 9, 2021
Merged
Show file tree
Hide file tree
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;
Expand All @@ -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};
Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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();
Expand Down
15 changes: 13 additions & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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);
}));

Expand Down
13 changes: 8 additions & 5 deletions tests/ui/min_rust_version_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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!";
Expand All @@ -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!";
Expand All @@ -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!";
Expand Down
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, ") {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -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, ") {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down