Auto merge of #11538 - Jarcho:proc_mac, r=dswij

Fix `is_from_proc_macro` patterns

fixes #11533

changelog: none
This commit is contained in:
bors 2023-12-11 07:49:53 +00:00
commit 3813a7b27e
12 changed files with 144 additions and 67 deletions

View File

@ -48,11 +48,10 @@
impl<'tcx> LateLintPass<'tcx> for AsConversions {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
if in_external_macro(cx.sess(), expr.span) || is_from_proc_macro(cx, expr) {
return;
}
if let ExprKind::Cast(_, _) = expr.kind {
if let ExprKind::Cast(_, _) = expr.kind
&& !in_external_macro(cx.sess(), expr.span)
&& !is_from_proc_macro(cx, expr)
{
span_lint_and_help(
cx,
AS_CONVERSIONS,

View File

@ -57,7 +57,6 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &rustc_hir::Expr<'tcx>) {
&& !matches!(deref_target.kind, ExprKind::Unary(UnOp::Deref, ..))
&& let ref_ty = cx.typeck_results().expr_ty(deref_target)
&& let ty::Ref(_, inner_ty, Mutability::Not) = ref_ty.kind()
&& !is_from_proc_macro(cx, e)
{
if let Some(parent_expr) = get_parent_expr(cx, e) {
if matches!(parent_expr.kind, ExprKind::Unary(UnOp::Deref, ..))
@ -75,6 +74,9 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &rustc_hir::Expr<'tcx>) {
return;
}
}
if is_from_proc_macro(cx, e) {
return;
}
span_lint_and_then(
cx,

View File

@ -105,7 +105,6 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
// case somebody does that for some reason
&& (is_infinity(const_1) && is_neg_infinity(const_2)
|| is_neg_infinity(const_1) && is_infinity(const_2))
&& !is_from_proc_macro(cx, expr)
&& let Some(local_snippet) = snippet_opt(cx, first.span)
{
let variant = match (kind.node, lhs_kind.node, rhs_kind.node) {
@ -113,6 +112,9 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
(BinOpKind::And, BinOpKind::Ne, BinOpKind::Ne) => Variant::ManualIsFinite,
_ => return,
};
if is_from_proc_macro(cx, expr) {
return;
}
span_lint_and_then(cx, variant.lint(), expr.span, variant.msg(), |diag| {
match variant {

View File

@ -19,10 +19,6 @@ pub(super) fn check<'tcx>(
arg: &'tcx hir::Expr<'_>,
simplify_using: &str,
) {
if is_from_proc_macro(cx, expr) {
return;
}
let is_option = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Option);
let is_result = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Result);
let is_bool = cx.typeck_results().expr_ty(recv).is_bool();
@ -32,7 +28,7 @@ pub(super) fn check<'tcx>(
let body = cx.tcx.hir().body(body);
let body_expr = &body.value;
if usage::BindingUsageFinder::are_params_used(cx, body) {
if usage::BindingUsageFinder::are_params_used(cx, body) || is_from_proc_macro(cx, expr) {
return;
}

View File

@ -44,7 +44,6 @@ fn check_stmt<'tcx>(&mut self, cx: &LateContext<'tcx>, stmt: &Stmt<'tcx>) {
&& block.stmts.is_empty()
&& block.expr.is_none()
&& !in_external_macro(cx.sess(), expr.span)
&& !is_from_proc_macro(cx, expr)
&& let Some(then_snippet) = snippet_opt(cx, then.span)
// Ignore
// - empty macro expansions
@ -53,6 +52,7 @@ fn check_stmt<'tcx>(&mut self, cx: &LateContext<'tcx>, stmt: &Stmt<'tcx>) {
// - #[cfg]'d out code
&& then_snippet.chars().all(|ch| matches!(ch, '{' | '}') || ch.is_ascii_whitespace())
&& let Some(cond_snippet) = snippet_opt(cx, cond.span)
&& !is_from_proc_macro(cx, expr)
{
span_lint_and_sugg(
cx,

View File

@ -132,7 +132,11 @@ fn is_integral(ty: Ty<'_>) -> bool {
}
// Common entry-point to avoid code duplication.
fn issue_lint(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
fn issue_lint<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
if is_from_proc_macro(cx, expr) {
return;
}
let msg = "arithmetic operation that can potentially result in unexpected side-effects";
span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, msg);
self.expr_span = Some(expr.span);
@ -160,10 +164,10 @@ fn literal_integer(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<u128> {
fn manage_bin_ops<'tcx>(
&mut self,
cx: &LateContext<'tcx>,
expr: &hir::Expr<'tcx>,
expr: &'tcx hir::Expr<'_>,
op: &Spanned<hir::BinOpKind>,
lhs: &hir::Expr<'tcx>,
rhs: &hir::Expr<'tcx>,
lhs: &'tcx hir::Expr<'_>,
rhs: &'tcx hir::Expr<'_>,
) {
if constant_simple(cx, cx.typeck_results(), expr).is_some() {
return;
@ -236,10 +240,10 @@ fn manage_bin_ops<'tcx>(
/// provided input.
fn manage_method_call<'tcx>(
&mut self,
args: &[hir::Expr<'tcx>],
args: &'tcx [hir::Expr<'_>],
cx: &LateContext<'tcx>,
ps: &hir::PathSegment<'tcx>,
receiver: &hir::Expr<'tcx>,
ps: &'tcx hir::PathSegment<'_>,
receiver: &'tcx hir::Expr<'_>,
) {
let Some(arg) = args.first() else {
return;
@ -264,8 +268,8 @@ fn manage_method_call<'tcx>(
fn manage_unary_ops<'tcx>(
&mut self,
cx: &LateContext<'tcx>,
expr: &hir::Expr<'tcx>,
un_expr: &hir::Expr<'tcx>,
expr: &'tcx hir::Expr<'_>,
un_expr: &'tcx hir::Expr<'_>,
un_op: hir::UnOp,
) {
let hir::UnOp::Neg = un_op else {
@ -287,14 +291,13 @@ fn manage_unary_ops<'tcx>(
fn should_skip_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &hir::Expr<'tcx>) -> bool {
is_lint_allowed(cx, ARITHMETIC_SIDE_EFFECTS, expr.hir_id)
|| is_from_proc_macro(cx, expr)
|| self.expr_span.is_some()
|| self.const_span.map_or(false, |sp| sp.contains(expr.span))
}
}
impl<'tcx> LateLintPass<'tcx> for ArithmeticSideEffects {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &hir::Expr<'tcx>) {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
if self.should_skip_expr(cx, expr) {
return;
}

View File

@ -72,8 +72,8 @@ fn check_fn(
) {
if self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(def_id)
|| in_external_macro(cx.sess(), span)
|| is_from_proc_macro(cx, &(&kind, body, cx.tcx.local_def_id_to_hir_id(def_id), span))
|| is_in_test_function(cx.tcx, body.value.hir_id)
|| is_from_proc_macro(cx, &(&kind, body, cx.tcx.local_def_id_to_hir_id(def_id), span))
{
return;
}

View File

@ -12,14 +12,14 @@
//! code was written, and check if the span contains that text. Note this will only work correctly
//! if the span is not from a `macro_rules` based macro.
use rustc_ast::ast::{AttrKind, Attribute, IntTy, LitIntType, LitKind, StrStyle, UintTy};
use rustc_ast::ast::{AttrKind, Attribute, IntTy, LitIntType, LitKind, StrStyle, TraitObjectSyntax, UintTy};
use rustc_ast::token::CommentKind;
use rustc_ast::AttrStyle;
use rustc_hir::intravisit::FnKind;
use rustc_hir::{
Block, BlockCheckMode, Body, Closure, Destination, Expr, ExprKind, FieldDef, FnHeader, HirId, Impl, ImplItem,
ImplItemKind, IsAuto, Item, ItemKind, LoopSource, MatchSource, MutTy, Node, QPath, TraitItem, TraitItemKind, Ty,
TyKind, UnOp, UnsafeSource, Unsafety, Variant, VariantData, YieldSource,
Block, BlockCheckMode, Body, Closure, Destination, Expr, ExprKind, FieldDef, FnHeader, FnRetTy, HirId, Impl,
ImplItem, ImplItemKind, IsAuto, Item, ItemKind, LoopSource, MatchSource, MutTy, Node, QPath, TraitItem,
TraitItemKind, Ty, TyKind, UnOp, UnsafeSource, Unsafety, Variant, VariantData, YieldSource,
};
use rustc_lint::{LateContext, LintContext};
use rustc_middle::ty::TyCtxt;
@ -33,8 +33,6 @@
pub enum Pat {
/// A single string.
Str(&'static str),
/// A single string.
OwnedStr(String),
/// Any of the given strings.
MultiStr(&'static [&'static str]),
/// Any of the given strings.
@ -59,14 +57,12 @@ fn span_matches_pat(sess: &Session, span: Span, start_pat: Pat, end_pat: Pat) ->
let end_str = s.trim_end_matches(|c: char| c.is_whitespace() || c == ')' || c == ',');
(match start_pat {
Pat::Str(text) => start_str.starts_with(text),
Pat::OwnedStr(text) => start_str.starts_with(&text),
Pat::MultiStr(texts) => texts.iter().any(|s| start_str.starts_with(s)),
Pat::OwnedMultiStr(texts) => texts.iter().any(|s| start_str.starts_with(s)),
Pat::Sym(sym) => start_str.starts_with(sym.as_str()),
Pat::Num => start_str.as_bytes().first().map_or(false, u8::is_ascii_digit),
} && match end_pat {
Pat::Str(text) => end_str.ends_with(text),
Pat::OwnedStr(text) => end_str.starts_with(&text),
Pat::MultiStr(texts) => texts.iter().any(|s| start_str.ends_with(s)),
Pat::OwnedMultiStr(texts) => texts.iter().any(|s| start_str.starts_with(s)),
Pat::Sym(sym) => end_str.ends_with(sym.as_str()),
@ -125,6 +121,8 @@ fn qpath_search_pat(path: &QPath<'_>) -> (Pat, Pat) {
fn expr_search_pat(tcx: TyCtxt<'_>, e: &Expr<'_>) -> (Pat, Pat) {
match e.kind {
ExprKind::ConstBlock(_) => (Pat::Str("const"), Pat::Str("}")),
// Parenthesis are trimmed from the text before the search patterns are matched.
// See: `span_matches_pat`
ExprKind::Tup([]) => (Pat::Str(")"), Pat::Str("(")),
ExprKind::Unary(UnOp::Deref, e) => (Pat::Str("*"), expr_search_pat(tcx, e).1),
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
fn attr_search_pat(attr: &Attribute) -> (Pat, Pat) {
match attr.kind {
AttrKind::Normal(..) => {
let mut pat = if matches!(attr.style, AttrStyle::Outer) {
(Pat::Str("#["), Pat::Str("]"))
} else {
(Pat::Str("#!["), Pat::Str("]"))
};
if let Some(ident) = attr.ident()
&& let Pat::Str(old_pat) = pat.0
{
if let Some(ident) = attr.ident() {
// TODO: I feel like it's likely we can use `Cow` instead but this will require quite a bit of
// refactoring
// NOTE: This will likely have false positives, like `allow = 1`
pat.0 = Pat::OwnedMultiStr(vec![ident.to_string(), old_pat.to_owned()]);
pat.1 = Pat::Str("");
(
Pat::OwnedMultiStr(vec![ident.to_string(), "#".to_owned()]),
Pat::Str(""),
)
} else {
(Pat::Str("#"), Pat::Str("]"))
}
pat
},
AttrKind::DocComment(_kind @ CommentKind::Line, ..) => {
if matches!(attr.style, AttrStyle::Outer) {
@ -324,32 +316,42 @@ fn attr_search_pat(attr: &Attribute) -> (Pat, Pat) {
fn ty_search_pat(ty: &Ty<'_>) -> (Pat, Pat) {
match ty.kind {
TyKind::Slice(..) | TyKind::Array(..) => (Pat::Str("["), Pat::Str("]")),
TyKind::Ptr(MutTy { mutbl, ty }) => (
if mutbl.is_mut() {
Pat::Str("*const")
} else {
Pat::Str("*mut")
},
ty_search_pat(ty).1,
),
TyKind::Ptr(MutTy { ty, .. }) => (Pat::Str("*"), ty_search_pat(ty).1),
TyKind::Ref(_, MutTy { ty, .. }) => (Pat::Str("&"), ty_search_pat(ty).1),
TyKind::BareFn(bare_fn) => (
Pat::OwnedStr(format!("{}{} fn", bare_fn.unsafety.prefix_str(), bare_fn.abi.name())),
ty_search_pat(ty).1,
if bare_fn.unsafety == Unsafety::Unsafe {
Pat::Str("unsafe")
} else if bare_fn.abi != Abi::Rust {
Pat::Str("extern")
} else {
Pat::MultiStr(&["fn", "extern"])
},
match bare_fn.decl.output {
FnRetTy::DefaultReturn(_) => {
if let [.., ty] = bare_fn.decl.inputs {
ty_search_pat(ty).1
} else {
Pat::Str("(")
}
},
FnRetTy::Return(ty) => ty_search_pat(ty).1,
},
),
TyKind::Never => (Pat::Str("!"), Pat::Str("")),
TyKind::Tup(..) => (Pat::Str("("), Pat::Str(")")),
TyKind::Never => (Pat::Str("!"), Pat::Str("!")),
// Parenthesis are trimmed from the text before the search patterns are matched.
// See: `span_matches_pat`
TyKind::Tup([]) => (Pat::Str(")"), Pat::Str("(")),
TyKind::Tup([ty]) => ty_search_pat(ty),
TyKind::Tup([head, .., tail]) => (ty_search_pat(head).0, ty_search_pat(tail).1),
TyKind::OpaqueDef(..) => (Pat::Str("impl"), Pat::Str("")),
TyKind::Path(qpath) => qpath_search_pat(&qpath),
// NOTE: This is missing `TraitObject`. It will always return true then.
TyKind::Infer => (Pat::Str("_"), Pat::Str("_")),
TyKind::TraitObject(_, _, TraitObjectSyntax::Dyn) => (Pat::Str("dyn"), Pat::Str("")),
// NOTE: `TraitObject` is incomplete. It will always return true then.
_ => (Pat::Str(""), Pat::Str("")),
}
}
fn ident_search_pat(ident: Ident) -> (Pat, Pat) {
(Pat::OwnedStr(ident.name.as_str().to_owned()), Pat::Str(""))
}
pub trait WithSearchPat<'cx> {
type Context: LintContext;
fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat);
@ -408,7 +410,7 @@ impl<'cx> WithSearchPat<'cx> for Ident {
type Context = LateContext<'cx>;
fn search_pat(&self, _cx: &Self::Context) -> (Pat, Pat) {
ident_search_pat(*self)
(Pat::Sym(self.name), Pat::Sym(self.name))
}
fn span(&self) -> Span {

View File

@ -1,6 +1,6 @@
//@aux-build:proc_macros.rs
#![allow(clippy::let_unit_value)]
#![allow(clippy::let_unit_value, clippy::needless_pass_by_ref_mut)]
extern crate proc_macros;
use proc_macros::external;

View File

@ -307,6 +307,19 @@ fn filter_copy<T: Copy>(predicate: &mut impl FnMut(T) -> bool) -> impl FnMut(&T)
move |&item| predicate(item)
}
// `is_from_proc_macro` stress tests
fn _empty_tup(x: &mut (())) {}
fn _single_tup(x: &mut ((i32,))) {}
fn _multi_tup(x: &mut ((i32, u32))) {}
fn _fn(x: &mut (fn())) {}
#[rustfmt::skip]
fn _extern_rust_fn(x: &mut extern "Rust" fn()) {}
fn _extern_c_fn(x: &mut extern "C" fn()) {}
fn _unsafe_fn(x: &mut unsafe fn()) {}
fn _unsafe_extern_fn(x: &mut unsafe extern "C" fn()) {}
fn _fn_with_arg(x: &mut unsafe extern "C" fn(i32)) {}
fn _fn_with_ret(x: &mut unsafe extern "C" fn() -> (i32)) {}
fn main() {
let mut u = 0;
let mut v = vec![0];

View File

@ -139,5 +139,65 @@ LL | pub async fn closure4(n: &mut usize) {
|
= warning: changing this function will impact semver compatibility
error: aborting due to 21 previous errors
error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:311:18
|
LL | fn _empty_tup(x: &mut (())) {}
| ^^^^^^^^^ help: consider changing to: `&()`
error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:312:19
|
LL | fn _single_tup(x: &mut ((i32,))) {}
| ^^^^^^^^^^^^^ help: consider changing to: `&(i32,)`
error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:313:18
|
LL | fn _multi_tup(x: &mut ((i32, u32))) {}
| ^^^^^^^^^^^^^^^^^ help: consider changing to: `&(i32, u32)`
error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:314:11
|
LL | fn _fn(x: &mut (fn())) {}
| ^^^^^^^^^^^ help: consider changing to: `&fn()`
error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:316:23
|
LL | fn _extern_rust_fn(x: &mut extern "Rust" fn()) {}
| ^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&extern "Rust" fn()`
error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:317:20
|
LL | fn _extern_c_fn(x: &mut extern "C" fn()) {}
| ^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&extern "C" fn()`
error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:318:18
|
LL | fn _unsafe_fn(x: &mut unsafe fn()) {}
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe fn()`
error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:319:25
|
LL | fn _unsafe_extern_fn(x: &mut unsafe extern "C" fn()) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe extern "C" fn()`
error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:320:20
|
LL | fn _fn_with_arg(x: &mut unsafe extern "C" fn(i32)) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe extern "C" fn(i32)`
error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:321:20
|
LL | fn _fn_with_ret(x: &mut unsafe extern "C" fn() -> (i32)) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe extern "C" fn() -> (i32)`
error: aborting due to 31 previous errors

View File

@ -1,6 +1,6 @@
//@aux-build:proc_macros.rs
#![allow(clippy::let_unit_value)]
#![allow(clippy::let_unit_value, clippy::needless_pass_by_ref_mut)]
#![warn(clippy::unnecessary_safety_doc)]
extern crate proc_macros;