Skip to content

Commit 0972c3b

Browse files
committed
Check for MSRV attributes in late passes using the HIR
1 parent 2cdb90d commit 0972c3b

File tree

12 files changed

+244
-212
lines changed

12 files changed

+244
-212
lines changed

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
```

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,

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,

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!();

clippy_utils/src/lib.rs

+3-14
Original file line numberDiff line numberDiff line change
@@ -137,24 +137,13 @@ use rustc_middle::hir::nested_filter;
137137

138138
#[macro_export]
139139
macro_rules! extract_msrv_attr {
140-
(LateContext) => {
141-
fn check_attributes(&mut self, cx: &rustc_lint::LateContext<'_>, attrs: &[rustc_hir::Attribute]) {
140+
() => {
141+
fn check_attributes(&mut self, cx: &rustc_lint::EarlyContext<'_>, attrs: &[rustc_ast::ast::Attribute]) {
142142
let sess = rustc_lint::LintContext::sess(cx);
143143
self.msrv.check_attributes(sess, attrs);
144144
}
145145

146-
fn check_attributes_post(&mut self, cx: &rustc_lint::LateContext<'_>, attrs: &[rustc_hir::Attribute]) {
147-
let sess = rustc_lint::LintContext::sess(cx);
148-
self.msrv.check_attributes_post(sess, attrs);
149-
}
150-
};
151-
(EarlyContext) => {
152-
fn check_attributes(&mut self, cx: &rustc_lint::EarlyContext<'_>, attrs: &[rustc_ast::Attribute]) {
153-
let sess = rustc_lint::LintContext::sess(cx);
154-
self.msrv.check_attributes(sess, attrs);
155-
}
156-
157-
fn check_attributes_post(&mut self, cx: &rustc_lint::EarlyContext<'_>, attrs: &[rustc_ast::Attribute]) {
146+
fn check_attributes_post(&mut self, cx: &rustc_lint::EarlyContext<'_>, attrs: &[rustc_ast::ast::Attribute]) {
158147
let sess = rustc_lint::LintContext::sess(cx);
159148
self.msrv.check_attributes_post(sess, attrs);
160149
}

0 commit comments

Comments
 (0)