user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Do not lint if used as a fn-like argument

Browse source
This commit is contained in:
Catherine Flores 2023-07-21 09:14:13 -05:00
parent 867e0ec024
commit ef482d17f2
8 changed files with 187 additions and 100 deletions
Download patch file Download diff file Expand all files Collapse all files

113
clippy_lints/src/needless_pass_by_ref_mut.rs
View file

@ -1,16 +1,16 @@
use super::needless_pass_by_value::requires_exact_signature;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::diagnostics::span_lint_hir_and_then;
use clippy_utils::source::snippet;
use clippy_utils::{is_from_proc_macro, is_self};
use if_chain::if_chain;
use rustc_data_structures::fx::FxHashSet;
use clippy_utils::{get_parent_node, is_from_proc_macro, is_self};
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_errors::Applicability;
use rustc_hir::intravisit::FnKind;
use rustc_hir::{Body, FnDecl, HirId, HirIdMap, HirIdSet, Impl, ItemKind, Mutability, Node, PatKind};
use rustc_hir::intravisit::{walk_qpath, FnKind, Visitor};
use rustc_hir::{Body, ExprKind, FnDecl, HirId, HirIdMap, HirIdSet, Impl, ItemKind, Mutability, Node, PatKind, QPath};
use rustc_hir_typeck::expr_use_visitor as euv;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::map::associated_body;
use rustc_middle::hir::nested_filter::OnlyBodies;
use rustc_middle::mir::FakeReadCause;
use rustc_middle::ty::{self, Ty, UpvarId, UpvarPath};
use rustc_session::{declare_tool_lint, impl_lint_pass};
@ -48,20 +48,24 @@ declare_clippy_lint! {
"using a `&mut` argument when it's not mutated"
}
#[derive(Copy, Clone)]
pub struct NeedlessPassByRefMut {
#[derive(Clone)]
pub struct NeedlessPassByRefMut<'tcx> {
avoid_breaking_exported_api: bool,
used_fn_def_ids: FxHashSet<LocalDefId>,
fn_def_ids_to_maybe_unused_mut: FxIndexMap<LocalDefId, Vec<rustc_hir::Ty<'tcx>>>,
}
impl NeedlessPassByRefMut {
impl NeedlessPassByRefMut<'_> {
pub fn new(avoid_breaking_exported_api: bool) -> Self {
Self {
avoid_breaking_exported_api,
used_fn_def_ids: FxHashSet::default(),
fn_def_ids_to_maybe_unused_mut: FxIndexMap::default(),
}
}
}
impl_lint_pass!(NeedlessPassByRefMut => [NEEDLESS_PASS_BY_REF_MUT]);
impl_lint_pass!(NeedlessPassByRefMut<'_> => [NEEDLESS_PASS_BY_REF_MUT]);
fn should_skip<'tcx>(
cx: &LateContext<'tcx>,
@ -89,12 +93,12 @@ fn should_skip<'tcx>(
is_from_proc_macro(cx, &input)
}
impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut {
impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> {
fn check_fn(
&mut self,
cx: &LateContext<'tcx>,
kind: FnKind<'tcx>,
decl: &'tcx FnDecl<'_>,
decl: &'tcx FnDecl<'tcx>,
body: &'tcx Body<'_>,
span: Span,
fn_def_id: LocalDefId,
@ -140,7 +144,6 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut {
if it.peek().is_none() {
return;
}
// Collect variables mutably used and spans which will need dereferencings from the
// function body.
let MutablyUsedVariablesCtxt { mutably_used_vars, .. } = {
@ -165,30 +168,45 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut {
}
ctx
};
let show_semver_warning = self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(fn_def_id);
for ((&input, &_), arg) in it {
// Only take `&mut` arguments.
if_chain! {
if let PatKind::Binding(_, canonical_id, ..) = arg.pat.kind;
if !mutably_used_vars.contains(&canonical_id);
if let rustc_hir::TyKind::Ref(_, inner_ty) = input.kind;
then {
// If the argument is never used mutably, we emit the warning.
let sp = input.span;
span_lint_and_then(
if let PatKind::Binding(_, canonical_id, ..) = arg.pat.kind
&& !mutably_used_vars.contains(&canonical_id)
{
self.fn_def_ids_to_maybe_unused_mut.entry(fn_def_id).or_default().push(input);
}
}
}
fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
cx.tcx.hir().visit_all_item_likes_in_crate(&mut FnNeedsMutVisitor {
cx,
used_fn_def_ids: &mut self.used_fn_def_ids,
});
for (fn_def_id, unused) in self
.fn_def_ids_to_maybe_unused_mut
.iter()
.filter(|(def_id, _)| !self.used_fn_def_ids.contains(def_id))
{
let show_semver_warning =
self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(*fn_def_id);
for input in unused {
// If the argument is never used mutably, we emit the warning.
let sp = input.span;
if let rustc_hir::TyKind::Ref(_, inner_ty) = input.kind {
span_lint_hir_and_then(
cx,
NEEDLESS_PASS_BY_REF_MUT,
cx.tcx.hir().local_def_id_to_hir_id(*fn_def_id),
sp,
"this argument is a mutable reference, but not used mutably",
|diag| {
diag.span_suggestion(
sp,
"consider changing to".to_string(),
format!(
"&{}",
snippet(cx, cx.tcx.hir().span(inner_ty.ty.hir_id), "_"),
),
format!("&{}", snippet(cx, cx.tcx.hir().span(inner_ty.ty.hir_id), "_"),),
Applicability::Unspecified,
);
if show_semver_warning {
@ -316,3 +334,44 @@ impl<'tcx> euv::Delegate<'tcx> for MutablyUsedVariablesCtxt {
self.prev_bind = Some(id);
}
}
/// A final pass to check for paths referencing this function that require the argument to be
/// `&mut`, basically if the function is ever used as a `fn`-like argument.
struct FnNeedsMutVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
used_fn_def_ids: &'a mut FxHashSet<LocalDefId>,
}
impl<'tcx> Visitor<'tcx> for FnNeedsMutVisitor<'_, 'tcx> {
type NestedFilter = OnlyBodies;
fn nested_visit_map(&mut self) -> Self::Map {
self.cx.tcx.hir()
}
fn visit_qpath(&mut self, qpath: &'tcx QPath<'tcx>, hir_id: HirId, _: Span) {
walk_qpath(self, qpath, hir_id);
let Self { cx, used_fn_def_ids } = self;
// #11182; do not lint if mutability is required elsewhere
if let Node::Expr(expr) = cx.tcx.hir().get(hir_id)
&& let Some(parent) = get_parent_node(cx.tcx, expr.hir_id)
&& let ty::FnDef(def_id, _) = cx.tcx.typeck(cx.tcx.hir().enclosing_body_owner(hir_id)).expr_ty(expr).kind()
&& let Some(def_id) = def_id.as_local()
{
if let Node::Expr(e) = parent
&& let ExprKind::Call(call, _) = e.kind
&& call.hir_id == expr.hir_id
{
return;
}
// We don't need to check each argument individually as you cannot coerce a function
// taking `&mut` -> `&`, for some reason, so if we've gotten this far we know it's
// passed as a `fn`-like argument (or is unified) and should ignore every "unused"
// argument entirely
used_fn_def_ids.insert(def_id);
}
}
}