Skip to content

Commit 9f9a822

Browse files
authored
Check for MSRV attributes in late passes using the HIR (rust-lang#13821)
Closes rust-lang/rust-clippy#13169 Late lints now use a parent iter to check for `#[clippy::msrv]` attributes instead of keeping track with `extract_msrv_attr`. This is required for incremental lints since they run per module instead of per crate so don't visit all the necessary attributes As a basic optimisation if no `#[clippy::msrv]` attributes are discovered in early passes the HIR access is skipped completely and just the configured MSRV is used, for most code bases this will be the case changelog: none
2 parents 6f66a60 + 5b0004c commit 9f9a822

File tree

112 files changed

+672
-850
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

112 files changed

+672
-850
lines changed

Diff for: book/src/development/adding_lints.md

+14-17
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@ pub struct ManualStrip {
460460

461461
impl ManualStrip {
462462
pub fn new(conf: &'static Conf) -> Self {
463-
Self { msrv: conf.msrv.clone() }
463+
Self { msrv: conf.msrv }
464464
}
465465
}
466466
```
@@ -469,24 +469,13 @@ The project's MSRV can then be matched against the feature MSRV in the LintPass
469469
using the `Msrv::meets` method.
470470

471471
``` rust
472-
if !self.msrv.meets(msrvs::STR_STRIP_PREFIX) {
472+
if !self.msrv.meets(cx, msrvs::STR_STRIP_PREFIX) {
473473
return;
474474
}
475475
```
476476

477-
The project's MSRV can also be specified as an attribute, which overrides
478-
the value from `clippy.toml`. This can be accounted for using the
479-
`extract_msrv_attr!(LintContext)` macro and passing
480-
`LateContext`/`EarlyContext`.
481-
482-
```rust,ignore
483-
impl<'tcx> LateLintPass<'tcx> for ManualStrip {
484-
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
485-
...
486-
}
487-
extract_msrv_attr!(LateContext);
488-
}
489-
```
477+
Early lint passes should instead use `MsrvStack` coupled with
478+
`extract_msrv_attr!()`
490479

491480
Once the `msrv` is added to the lint, a relevant test case should be added to
492481
the lint's test file, `tests/ui/manual_strip.rs` in this example. It should
@@ -512,8 +501,16 @@ in `clippy_config/src/conf.rs`:
512501

513502
```rust
514503
define_Conf! {
515-
/// Lint: LIST, OF, LINTS, <THE_NEWLY_ADDED_LINT>. The minimum rust version that the project supports
516-
(msrv: Option<String> = None),
504+
#[lints(
505+
allow_attributes,
506+
allow_attributes_without_reason,
507+
..
508+
<the newly added lint name>,
509+
..
510+
unused_trait_names,
511+
use_self,
512+
)]
513+
msrv: Msrv = Msrv::default(),
517514
...
518515
}
519516
```

Diff for: clippy_config/src/conf.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -670,7 +670,7 @@ define_Conf! {
670670
unused_trait_names,
671671
use_self,
672672
)]
673-
msrv: Msrv = Msrv::empty(),
673+
msrv: Msrv = Msrv::default(),
674674
/// The minimum size (in bytes) to consider a type for passing by reference instead of by value.
675675
#[lints(large_types_passed_by_value)]
676676
pass_by_value_size_limit: u64 = 256,

Diff for: clippy_dev/src/main.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ fn main() {
3535
category,
3636
r#type,
3737
msrv,
38-
} => match new_lint::create(&pass, &name, &category, r#type.as_deref(), msrv) {
38+
} => match new_lint::create(pass, &name, &category, r#type.as_deref(), msrv) {
3939
Ok(()) => update_lints::update(utils::UpdateMode::Change),
4040
Err(e) => eprintln!("Unable to create lint: {e}"),
4141
},
@@ -147,9 +147,9 @@ enum DevCommand {
147147
#[command(name = "new_lint")]
148148
/// Create a new lint and run `cargo dev update_lints`
149149
NewLint {
150-
#[arg(short, long, value_parser = ["early", "late"], conflicts_with = "type", default_value = "late")]
150+
#[arg(short, long, conflicts_with = "type", default_value = "late")]
151151
/// Specify whether the lint runs during the early or late pass
152-
pass: String,
152+
pass: new_lint::Pass,
153153
#[arg(
154154
short,
155155
long,

Diff for: clippy_dev/src/new_lint.rs

+47-35
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,28 @@
11
use crate::utils::{clippy_project_root, clippy_version};
2+
use clap::ValueEnum;
23
use indoc::{formatdoc, writedoc};
3-
use std::fmt::Write as _;
4+
use std::fmt::{self, Write as _};
45
use std::fs::{self, OpenOptions};
5-
use std::io::prelude::*;
6+
use std::io::{self, Write as _};
67
use std::path::{Path, PathBuf};
7-
use std::{fmt, io};
8+
9+
#[derive(Clone, Copy, PartialEq, ValueEnum)]
10+
pub enum Pass {
11+
Early,
12+
Late,
13+
}
14+
15+
impl fmt::Display for Pass {
16+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
17+
f.write_str(match self {
18+
Pass::Early => "early",
19+
Pass::Late => "late",
20+
})
21+
}
22+
}
823

924
struct LintData<'a> {
10-
pass: &'a str,
25+
pass: Pass,
1126
name: &'a str,
1227
category: &'a str,
1328
ty: Option<&'a str>,
@@ -35,7 +50,7 @@ impl<T> Context for io::Result<T> {
3550
/// # Errors
3651
///
3752
/// This function errors out if the files couldn't be created or written to.
38-
pub fn create(pass: &str, name: &str, category: &str, mut ty: Option<&str>, msrv: bool) -> io::Result<()> {
53+
pub fn create(pass: Pass, name: &str, category: &str, mut ty: Option<&str>, msrv: bool) -> io::Result<()> {
3954
if category == "cargo" && ty.is_none() {
4055
// `cargo` is a special category, these lints should always be in `clippy_lints/src/cargo`
4156
ty = Some("cargo");
@@ -56,7 +71,7 @@ pub fn create(pass: &str, name: &str, category: &str, mut ty: Option<&str>, msrv
5671
add_lint(&lint, msrv).context("Unable to add lint to clippy_lints/src/lib.rs")?;
5772
}
5873

59-
if pass == "early" {
74+
if pass == Pass::Early {
6075
println!(
6176
"\n\
6277
NOTE: Use a late pass unless you need something specific from\n\
@@ -136,23 +151,17 @@ fn add_lint(lint: &LintData<'_>, enable_msrv: bool) -> io::Result<()> {
136151
let mut lib_rs = fs::read_to_string(path).context("reading")?;
137152

138153
let comment_start = lib_rs.find("// add lints here,").expect("Couldn't find comment");
154+
let ctor_arg = if lint.pass == Pass::Late { "_" } else { "" };
155+
let lint_pass = lint.pass;
156+
let module_name = lint.name;
157+
let camel_name = to_camel_case(lint.name);
139158

140159
let new_lint = if enable_msrv {
141160
format!(
142161
"store.register_{lint_pass}_pass(move |{ctor_arg}| Box::new({module_name}::{camel_name}::new(conf)));\n ",
143-
lint_pass = lint.pass,
144-
ctor_arg = if lint.pass == "late" { "_" } else { "" },
145-
module_name = lint.name,
146-
camel_name = to_camel_case(lint.name),
147162
)
148163
} else {
149-
format!(
150-
"store.register_{lint_pass}_pass(|{ctor_arg}| Box::new({module_name}::{camel_name}));\n ",
151-
lint_pass = lint.pass,
152-
ctor_arg = if lint.pass == "late" { "_" } else { "" },
153-
module_name = lint.name,
154-
camel_name = to_camel_case(lint.name),
155-
)
164+
format!("store.register_{lint_pass}_pass(|{ctor_arg}| Box::new({module_name}::{camel_name}));\n ",)
156165
};
157166

158167
lib_rs.insert_str(comment_start, &new_lint);
@@ -242,11 +251,16 @@ fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String {
242251
let mut result = String::new();
243252

244253
let (pass_type, pass_lifetimes, pass_import, context_import) = match lint.pass {
245-
"early" => ("EarlyLintPass", "", "use rustc_ast::ast::*;", "EarlyContext"),
246-
"late" => ("LateLintPass", "<'_>", "use rustc_hir::*;", "LateContext"),
247-
_ => {
248-
unreachable!("`pass_type` should only ever be `early` or `late`!");
249-
},
254+
Pass::Early => ("EarlyLintPass", "", "use rustc_ast::ast::*;", "EarlyContext"),
255+
Pass::Late => ("LateLintPass", "<'_>", "use rustc_hir::*;", "LateContext"),
256+
};
257+
let (msrv_ty, msrv_ctor, extract_msrv) = match lint.pass {
258+
Pass::Early => (
259+
"MsrvStack",
260+
"MsrvStack::new(conf.msrv)",
261+
"\n extract_msrv_attr!();\n",
262+
),
263+
Pass::Late => ("Msrv", "conf.msrv", ""),
250264
};
251265

252266
let lint_name = lint.name;
@@ -258,10 +272,10 @@ fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String {
258272
let _: fmt::Result = writedoc!(
259273
result,
260274
r"
261-
use clippy_utils::msrvs::{{self, Msrv}};
275+
use clippy_utils::msrvs::{{self, {msrv_ty}}};
262276
use clippy_config::Conf;
263277
{pass_import}
264-
use rustc_lint::{{{context_import}, {pass_type}, LintContext}};
278+
use rustc_lint::{{{context_import}, {pass_type}}};
265279
use rustc_session::impl_lint_pass;
266280
267281
"
@@ -285,20 +299,18 @@ fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String {
285299
result,
286300
r"
287301
pub struct {name_camel} {{
288-
msrv: Msrv,
302+
msrv: {msrv_ty},
289303
}}
290304
291305
impl {name_camel} {{
292306
pub fn new(conf: &'static Conf) -> Self {{
293-
Self {{ msrv: conf.msrv.clone() }}
307+
Self {{ msrv: {msrv_ctor} }}
294308
}}
295309
}}
296310
297311
impl_lint_pass!({name_camel} => [{name_upper}]);
298312
299-
impl {pass_type}{pass_lifetimes} for {name_camel} {{
300-
extract_msrv_attr!({context_import});
301-
}}
313+
impl {pass_type}{pass_lifetimes} for {name_camel} {{{extract_msrv}}}
302314
303315
// TODO: Add MSRV level to `clippy_config/src/msrvs.rs` if needed.
304316
// TODO: Update msrv config comment in `clippy_config/src/conf.rs`
@@ -375,9 +387,9 @@ fn create_lint_for_ty(lint: &LintData<'_>, enable_msrv: bool, ty: &str) -> io::R
375387

376388
let mod_file_path = ty_dir.join("mod.rs");
377389
let context_import = setup_mod_file(&mod_file_path, lint)?;
378-
let pass_lifetimes = match context_import {
379-
"LateContext" => "<'_>",
380-
_ => "",
390+
let (pass_lifetimes, msrv_ty, msrv_ref, msrv_cx) = match context_import {
391+
"LateContext" => ("<'_>", "Msrv", "", "cx, "),
392+
_ => ("", "MsrvStack", "&", ""),
381393
};
382394

383395
let name_upper = lint.name.to_uppercase();
@@ -387,14 +399,14 @@ fn create_lint_for_ty(lint: &LintData<'_>, enable_msrv: bool, ty: &str) -> io::R
387399
let _: fmt::Result = writedoc!(
388400
lint_file_contents,
389401
r#"
390-
use clippy_utils::msrvs::{{self, Msrv}};
402+
use clippy_utils::msrvs::{{self, {msrv_ty}}};
391403
use rustc_lint::{{{context_import}, LintContext}};
392404
393405
use super::{name_upper};
394406
395407
// TODO: Adjust the parameters as necessary
396-
pub(super) fn check(cx: &{context_import}{pass_lifetimes}, msrv: &Msrv) {{
397-
if !msrv.meets(todo!("Add a new entry in `clippy_utils/src/msrvs`")) {{
408+
pub(super) fn check(cx: &{context_import}{pass_lifetimes}, msrv: {msrv_ref}{msrv_ty}) {{
409+
if !msrv.meets({msrv_cx}todo!("Add a new entry in `clippy_utils/src/msrvs`")) {{
398410
return;
399411
}}
400412
todo!();

Diff for: clippy_lints/src/almost_complete_range.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_config::Conf;
22
use clippy_utils::diagnostics::span_lint_and_then;
3-
use clippy_utils::msrvs::{self, Msrv};
3+
use clippy_utils::msrvs::{self, MsrvStack};
44
use clippy_utils::source::{trim_span, walk_span_to_context};
55
use rustc_ast::ast::{Expr, ExprKind, LitKind, Pat, PatKind, RangeEnd, RangeLimits};
66
use rustc_errors::Applicability;
@@ -31,12 +31,12 @@ declare_clippy_lint! {
3131
impl_lint_pass!(AlmostCompleteRange => [ALMOST_COMPLETE_RANGE]);
3232

3333
pub struct AlmostCompleteRange {
34-
msrv: Msrv,
34+
msrv: MsrvStack,
3535
}
3636
impl AlmostCompleteRange {
3737
pub fn new(conf: &'static Conf) -> Self {
3838
Self {
39-
msrv: conf.msrv.clone(),
39+
msrv: MsrvStack::new(conf.msrv),
4040
}
4141
}
4242
}
@@ -96,7 +96,7 @@ impl EarlyLintPass for AlmostCompleteRange {
9696
}
9797
}
9898

99-
extract_msrv_attr!(EarlyContext);
99+
extract_msrv_attr!();
100100
}
101101

102102
fn is_incomplete_range(start: &Expr, end: &Expr) -> bool {

Diff for: clippy_lints/src/approx_const.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,7 @@ pub struct ApproxConstant {
6969

7070
impl ApproxConstant {
7171
pub fn new(conf: &'static Conf) -> Self {
72-
Self {
73-
msrv: conf.msrv.clone(),
74-
}
72+
Self { msrv: conf.msrv }
7573
}
7674
}
7775

@@ -89,16 +87,14 @@ impl LateLintPass<'_> for ApproxConstant {
8987
_ => (),
9088
}
9189
}
92-
93-
extract_msrv_attr!(LateContext);
9490
}
9591

9692
impl ApproxConstant {
9793
fn check_known_consts(&self, cx: &LateContext<'_>, span: Span, s: symbol::Symbol, module: &str) {
9894
let s = s.as_str();
9995
if s.parse::<f64>().is_ok() {
10096
for &(constant, name, min_digits, msrv) in &KNOWN_CONSTS {
101-
if is_approx_const(constant, s, min_digits) && msrv.is_none_or(|msrv| self.msrv.meets(msrv)) {
97+
if is_approx_const(constant, s, min_digits) && msrv.is_none_or(|msrv| self.msrv.meets(cx, msrv)) {
10298
span_lint_and_help(
10399
cx,
104100
APPROX_CONSTANT,

Diff for: clippy_lints/src/assigning_clones.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,7 @@ pub struct AssigningClones {
5959

6060
impl AssigningClones {
6161
pub fn new(conf: &'static Conf) -> Self {
62-
Self {
63-
msrv: conf.msrv.clone(),
64-
}
62+
Self { msrv: conf.msrv }
6563
}
6664
}
6765

@@ -90,7 +88,7 @@ impl<'tcx> LateLintPass<'tcx> for AssigningClones {
9088
sym::clone if is_diag_trait_item(cx, fn_id, sym::Clone) => CloneTrait::Clone,
9189
_ if fn_name.as_str() == "to_owned"
9290
&& is_diag_trait_item(cx, fn_id, sym::ToOwned)
93-
&& self.msrv.meets(msrvs::CLONE_INTO) =>
91+
&& self.msrv.meets(cx, msrvs::CLONE_INTO) =>
9492
{
9593
CloneTrait::ToOwned
9694
},
@@ -143,8 +141,6 @@ impl<'tcx> LateLintPass<'tcx> for AssigningClones {
143141
);
144142
}
145143
}
146-
147-
extract_msrv_attr!(LateContext);
148144
}
149145

150146
/// Checks if the data being cloned borrows from the place that is being assigned to:

Diff for: clippy_lints/src/attrs/deprecated_cfg_attr.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
use super::{Attribute, DEPRECATED_CFG_ATTR, DEPRECATED_CLIPPY_CFG_ATTR, unnecessary_clippy_cfg};
22
use clippy_utils::diagnostics::span_lint_and_sugg;
3-
use clippy_utils::msrvs::{self, Msrv};
3+
use clippy_utils::msrvs::{self, MsrvStack};
44
use rustc_ast::AttrStyle;
55
use rustc_errors::Applicability;
66
use rustc_lint::EarlyContext;
77
use rustc_span::sym;
88

9-
pub(super) fn check(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msrv) {
9+
pub(super) fn check(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &MsrvStack) {
1010
// check cfg_attr
1111
if attr.has_name(sym::cfg_attr)
1212
&& let Some(items) = attr.meta_item_list()

0 commit comments

Comments
 (0)