Skip to content

Commit 184845f

Browse files
JarchoJarcho
committed
Fix is_from_proc_macro patterns
1 parent 87aed03 commit 184845f

5 files changed

+117
-42
lines changed

‎clippy_utils/src/check_proc_macro.rs

+41-39
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@
1212
//! code was written, and check if the span contains that text. Note this will only work correctly
1313
//! if the span is not from a `macro_rules` based macro.
1414
15-
use rustc_ast::ast::{AttrKind, Attribute, IntTy, LitIntType, LitKind, StrStyle, UintTy};
15+
use rustc_ast::ast::{AttrKind, Attribute, IntTy, LitIntType, LitKind, StrStyle, TraitObjectSyntax, UintTy};
1616
use rustc_ast::token::CommentKind;
1717
use rustc_ast::AttrStyle;
1818
use rustc_hir::intravisit::FnKind;
1919
use rustc_hir::{
20-
Block, BlockCheckMode, Body, Closure, Destination, Expr, ExprKind, FieldDef, FnHeader, HirId, Impl, ImplItem,
21-
ImplItemKind, IsAuto, Item, ItemKind, LoopSource, MatchSource, MutTy, Node, QPath, TraitItem, TraitItemKind, Ty,
22-
TyKind, UnOp, UnsafeSource, Unsafety, Variant, VariantData, YieldSource,
20+
Block, BlockCheckMode, Body, Closure, Destination, Expr, ExprKind, FieldDef, FnHeader, FnRetTy, HirId, Impl,
21+
ImplItem, ImplItemKind, IsAuto, Item, ItemKind, LoopSource, MatchSource, MutTy, Node, QPath, TraitItem,
22+
TraitItemKind, Ty, TyKind, UnOp, UnsafeSource, Unsafety, Variant, VariantData, YieldSource,
2323
};
2424
use rustc_lint::{LateContext, LintContext};
2525
use rustc_middle::ty::TyCtxt;
@@ -33,8 +33,6 @@ use rustc_target::spec::abi::Abi;
3333
pub enum Pat {
3434
/// A single string.
3535
Str(&'static str),
36-
/// A single string.
37-
OwnedStr(String),
3836
/// Any of the given strings.
3937
MultiStr(&'static [&'static str]),
4038
/// Any of the given strings.
@@ -59,14 +57,12 @@ fn span_matches_pat(sess: &Session, span: Span, start_pat: Pat, end_pat: Pat) ->
5957
let end_str = s.trim_end_matches(|c: char| c.is_whitespace() || c == ')' || c == ',');
6058
(match start_pat {
6159
Pat::Str(text) => start_str.starts_with(text),
62-
Pat::OwnedStr(text) => start_str.starts_with(&text),
6360
Pat::MultiStr(texts) => texts.iter().any(|s| start_str.starts_with(s)),
6461
Pat::OwnedMultiStr(texts) => texts.iter().any(|s| start_str.starts_with(s)),
6562
Pat::Sym(sym) => start_str.starts_with(sym.as_str()),
6663
Pat::Num => start_str.as_bytes().first().map_or(false, u8::is_ascii_digit),
6764
} && match end_pat {
6865
Pat::Str(text) => end_str.ends_with(text),
69-
Pat::OwnedStr(text) => end_str.starts_with(&text),
7066
Pat::MultiStr(texts) => texts.iter().any(|s| start_str.ends_with(s)),
7167
Pat::OwnedMultiStr(texts) => texts.iter().any(|s| start_str.starts_with(s)),
7268
Pat::Sym(sym) => end_str.ends_with(sym.as_str()),
@@ -125,6 +121,8 @@ fn qpath_search_pat(path: &QPath<'_>) -> (Pat, Pat) {
125121
fn expr_search_pat(tcx: TyCtxt<'_>, e: &Expr<'_>) -> (Pat, Pat) {
126122
match e.kind {
127123
ExprKind::ConstBlock(_) => (Pat::Str("const"), Pat::Str("}")),
124+
// Parenthesis are trimmed from the text before the search patterns are matched.
125+
// See: `span_matches_pat`
128126
ExprKind::Tup([]) => (Pat::Str(")"), Pat::Str("(")),
129127
ExprKind::Unary(UnOp::Deref, e) => (Pat::Str("*"), expr_search_pat(tcx, e).1),
130128
ExprKind::Unary(UnOp::Not, e) => (Pat::Str("!"), expr_search_pat(tcx, e).1),
@@ -286,23 +284,17 @@ fn fn_kind_pat(tcx: TyCtxt<'_>, kind: &FnKind<'_>, body: &Body<'_>, hir_id: HirI
286284
fn attr_search_pat(attr: &Attribute) -> (Pat, Pat) {
287285
match attr.kind {
288286
AttrKind::Normal(..) => {
289-
let mut pat = if matches!(attr.style, AttrStyle::Outer) {
290-
(Pat::Str("#["), Pat::Str("]"))
291-
} else {
292-
(Pat::Str("#!["), Pat::Str("]"))
293-
};
294-
295-
if let Some(ident) = attr.ident()
296-
&& let Pat::Str(old_pat) = pat.0
297-
{
287+
if let Some(ident) = attr.ident() {
298288
// TODO: I feel like it's likely we can use `Cow` instead but this will require quite a bit of
299289
// refactoring
300290
// NOTE: This will likely have false positives, like `allow = 1`
301-
pat.0 = Pat::OwnedMultiStr(vec![ident.to_string(), old_pat.to_owned()]);
302-
pat.1 = Pat::Str("");
291+
(
292+
Pat::OwnedMultiStr(vec![ident.to_string(), "#".to_owned()]),
293+
Pat::Str(""),
294+
)
295+
} else {
296+
(Pat::Str("#"), Pat::Str("]"))
303297
}
304-
305-
pat
306298
},
307299
AttrKind::DocComment(_kind @ CommentKind::Line, ..) => {
308300
if matches!(attr.style, AttrStyle::Outer) {
@@ -324,32 +316,42 @@ fn attr_search_pat(attr: &Attribute) -> (Pat, Pat) {
324316
fn ty_search_pat(ty: &Ty<'_>) -> (Pat, Pat) {
325317
match ty.kind {
326318
TyKind::Slice(..) | TyKind::Array(..) => (Pat::Str("["), Pat::Str("]")),
327-
TyKind::Ptr(MutTy { mutbl, ty }) => (
328-
if mutbl.is_mut() {
329-
Pat::Str("*const")
330-
} else {
331-
Pat::Str("*mut")
332-
},
333-
ty_search_pat(ty).1,
334-
),
319+
TyKind::Ptr(MutTy { ty, .. }) => (Pat::Str("*"), ty_search_pat(ty).1),
335320
TyKind::Ref(_, MutTy { ty, .. }) => (Pat::Str("&"), ty_search_pat(ty).1),
336321
TyKind::BareFn(bare_fn) => (
337-
Pat::OwnedStr(format!("{}{} fn", bare_fn.unsafety.prefix_str(), bare_fn.abi.name())),
338-
ty_search_pat(ty).1,
322+
if bare_fn.unsafety == Unsafety::Unsafe {
323+
Pat::Str("unsafe")
324+
} else if bare_fn.abi != Abi::Rust {
325+
Pat::Str("extern")
326+
} else {
327+
Pat::MultiStr(&["fn", "extern"])
328+
},
329+
match bare_fn.decl.output {
330+
FnRetTy::DefaultReturn(_) => {
331+
if let [.., ty] = bare_fn.decl.inputs {
332+
ty_search_pat(ty).1
333+
} else {
334+
Pat::Str("(")
335+
}
336+
},
337+
FnRetTy::Return(ty) => ty_search_pat(ty).1,
338+
},
339339
),
340-
TyKind::Never => (Pat::Str("!"), Pat::Str("")),
341-
TyKind::Tup(..) => (Pat::Str("("), Pat::Str(")")),
340+
TyKind::Never => (Pat::Str("!"), Pat::Str("!")),
341+
// Parenthesis are trimmed from the text before the search patterns are matched.
342+
// See: `span_matches_pat`
343+
TyKind::Tup([]) => (Pat::Str(")"), Pat::Str("(")),
344+
TyKind::Tup([ty]) => ty_search_pat(ty),
345+
TyKind::Tup([head, .., tail]) => (ty_search_pat(head).0, ty_search_pat(tail).1),
342346
TyKind::OpaqueDef(..) => (Pat::Str("impl"), Pat::Str("")),
343347
TyKind::Path(qpath) => qpath_search_pat(&qpath),
344-
// NOTE: This is missing `TraitObject`. It will always return true then.
348+
TyKind::Infer => (Pat::Str("_"), Pat::Str("_")),
349+
TyKind::TraitObject(_, _, TraitObjectSyntax::Dyn) => (Pat::Str("dyn"), Pat::Str("")),
350+
// NOTE: `TraitObject` is incomplete. It will always return true then.
345351
_ => (Pat::Str(""), Pat::Str("")),
346352
}
347353
}
348354

349-
fn ident_search_pat(ident: Ident) -> (Pat, Pat) {
350-
(Pat::OwnedStr(ident.name.as_str().to_owned()), Pat::Str(""))
351-
}
352-
353355
pub trait WithSearchPat<'cx> {
354356
type Context: LintContext;
355357
fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat);
@@ -408,7 +410,7 @@ impl<'cx> WithSearchPat<'cx> for Ident {
408410
type Context = LateContext<'cx>;
409411

410412
fn search_pat(&self, _cx: &Self::Context) -> (Pat, Pat) {
411-
ident_search_pat(*self)
413+
(Pat::Sym(self.name), Pat::Sym(self.name))
412414
}
413415

414416
fn span(&self) -> Span {

‎tests/ui/doc_unsafe.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@aux-build:proc_macros.rs
22

3-
#![allow(clippy::let_unit_value)]
3+
#![allow(clippy::let_unit_value, clippy::needless_pass_by_ref_mut)]
44

55
extern crate proc_macros;
66
use proc_macros::external;

‎tests/ui/needless_pass_by_ref_mut.rs

+13
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,19 @@ fn filter_copy<T: Copy>(predicate: &mut impl FnMut(T) -> bool) -> impl FnMut(&T)
307307
move |&item| predicate(item)
308308
}
309309

310+
// `is_from_proc_macro` stress tests
311+
fn _empty_tup(x: &mut (())) {}
312+
fn _single_tup(x: &mut ((i32,))) {}
313+
fn _multi_tup(x: &mut ((i32, u32))) {}
314+
fn _fn(x: &mut (fn())) {}
315+
#[rustfmt::skip]
316+
fn _extern_rust_fn(x: &mut extern "Rust" fn()) {}
317+
fn _extern_c_fn(x: &mut extern "C" fn()) {}
318+
fn _unsafe_fn(x: &mut unsafe fn()) {}
319+
fn _unsafe_extern_fn(x: &mut unsafe extern "C" fn()) {}
320+
fn _fn_with_arg(x: &mut unsafe extern "C" fn(i32)) {}
321+
fn _fn_with_ret(x: &mut unsafe extern "C" fn() -> (i32)) {}
322+
310323
fn main() {
311324
let mut u = 0;
312325
let mut v = vec![0];

‎tests/ui/needless_pass_by_ref_mut.stderr

+61-1
Original file line numberDiff line numberDiff line change
@@ -139,5 +139,65 @@ LL | pub async fn closure4(n: &mut usize) {
139139
|
140140
= warning: changing this function will impact semver compatibility
141141

142-
error: aborting due to 21 previous errors
142+
error: this argument is a mutable reference, but not used mutably
143+
--> $DIR/needless_pass_by_ref_mut.rs:311:18
144+
|
145+
LL | fn _empty_tup(x: &mut (())) {}
146+
| ^^^^^^^^^ help: consider changing to: `&()`
147+
148+
error: this argument is a mutable reference, but not used mutably
149+
--> $DIR/needless_pass_by_ref_mut.rs:312:19
150+
|
151+
LL | fn _single_tup(x: &mut ((i32,))) {}
152+
| ^^^^^^^^^^^^^ help: consider changing to: `&(i32,)`
153+
154+
error: this argument is a mutable reference, but not used mutably
155+
--> $DIR/needless_pass_by_ref_mut.rs:313:18
156+
|
157+
LL | fn _multi_tup(x: &mut ((i32, u32))) {}
158+
| ^^^^^^^^^^^^^^^^^ help: consider changing to: `&(i32, u32)`
159+
160+
error: this argument is a mutable reference, but not used mutably
161+
--> $DIR/needless_pass_by_ref_mut.rs:314:11
162+
|
163+
LL | fn _fn(x: &mut (fn())) {}
164+
| ^^^^^^^^^^^ help: consider changing to: `&fn()`
165+
166+
error: this argument is a mutable reference, but not used mutably
167+
--> $DIR/needless_pass_by_ref_mut.rs:316:23
168+
|
169+
LL | fn _extern_rust_fn(x: &mut extern "Rust" fn()) {}
170+
| ^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&extern "Rust" fn()`
171+
172+
error: this argument is a mutable reference, but not used mutably
173+
--> $DIR/needless_pass_by_ref_mut.rs:317:20
174+
|
175+
LL | fn _extern_c_fn(x: &mut extern "C" fn()) {}
176+
| ^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&extern "C" fn()`
177+
178+
error: this argument is a mutable reference, but not used mutably
179+
--> $DIR/needless_pass_by_ref_mut.rs:318:18
180+
|
181+
LL | fn _unsafe_fn(x: &mut unsafe fn()) {}
182+
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe fn()`
183+
184+
error: this argument is a mutable reference, but not used mutably
185+
--> $DIR/needless_pass_by_ref_mut.rs:319:25
186+
|
187+
LL | fn _unsafe_extern_fn(x: &mut unsafe extern "C" fn()) {}
188+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe extern "C" fn()`
189+
190+
error: this argument is a mutable reference, but not used mutably
191+
--> $DIR/needless_pass_by_ref_mut.rs:320:20
192+
|
193+
LL | fn _fn_with_arg(x: &mut unsafe extern "C" fn(i32)) {}
194+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe extern "C" fn(i32)`
195+
196+
error: this argument is a mutable reference, but not used mutably
197+
--> $DIR/needless_pass_by_ref_mut.rs:321:20
198+
|
199+
LL | fn _fn_with_ret(x: &mut unsafe extern "C" fn() -> (i32)) {}
200+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe extern "C" fn() -> (i32)`
201+
202+
error: aborting due to 31 previous errors
143203

‎tests/ui/unnecessary_unsafety_doc.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@aux-build:proc_macros.rs
22

3-
#![allow(clippy::let_unit_value)]
3+
#![allow(clippy::let_unit_value, clippy::needless_pass_by_ref_mut)]
44
#![warn(clippy::unnecessary_safety_doc)]
55

66
extern crate proc_macros;

0 commit comments

Comments
 (0)