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

Fix is_from_proc_macro patterns #11538

Merged
merged 2 commits into from
Dec 11, 2023
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
9 changes: 4 additions & 5 deletions clippy_lints/src/as_conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,10 @@ declare_lint_pass!(AsConversions => [AS_CONVERSIONS]);

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,
Expand Down
4 changes: 3 additions & 1 deletion clippy_lints/src/borrow_deref_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ impl<'tcx> LateLintPass<'tcx> for BorrowDerefRef {
&& !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, ..))
Expand All @@ -75,6 +74,9 @@ impl<'tcx> LateLintPass<'tcx> for BorrowDerefRef {
return;
}
}
if is_from_proc_macro(cx, e) {
return;
}

span_lint_and_then(
cx,
Expand Down
4 changes: 3 additions & 1 deletion clippy_lints/src/manual_float_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,16 @@ impl<'tcx> LateLintPass<'tcx> for ManualFloatMethods {
// 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) {
(BinOpKind::Or, BinOpKind::Eq, BinOpKind::Eq) => Variant::ManualIsInfinite,
(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 {
Expand Down
6 changes: 1 addition & 5 deletions clippy_lints/src/methods/unnecessary_lazy_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/needless_if.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ impl LateLintPass<'_> for NeedlessIf {
&& 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
Expand All @@ -53,6 +52,7 @@ impl LateLintPass<'_> for NeedlessIf {
// - #[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,
Expand Down
25 changes: 14 additions & 11 deletions clippy_lints/src/operators/arithmetic_side_effects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,11 @@ impl ArithmeticSideEffects {
}

// 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);
Expand Down Expand Up @@ -160,10 +164,10 @@ impl ArithmeticSideEffects {
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;
Expand Down Expand Up @@ -236,10 +240,10 @@ impl ArithmeticSideEffects {
/// 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;
Expand All @@ -264,8 +268,8 @@ impl ArithmeticSideEffects {
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 {
Expand All @@ -287,14 +291,13 @@ impl ArithmeticSideEffects {

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;
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/single_call_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ impl<'tcx> LateLintPass<'tcx> for SingleCallFn {
) {
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;
}
Expand Down
80 changes: 41 additions & 39 deletions clippy_utils/src/check_proc_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,8 +33,6 @@ use rustc_target::spec::abi::Abi;
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.
Expand All @@ -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()),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/doc_unsafe.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/needless_pass_by_ref_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
Loading