Add for_each_local_usage. Switch LocalUsedVisitor to a function.

This commit is contained in:
Jason Newcomb 2021-06-06 20:01:14 -04:00
parent 983e5b877e
commit 8cf6dae0ca
No known key found for this signature in database
GPG Key ID: DA59E8643A37ED06
10 changed files with 91 additions and 104 deletions

View File

@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability; use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::match_type; use clippy_utils::ty::match_type;
use clippy_utils::visitors::LocalUsedVisitor; use clippy_utils::visitors::is_local_used;
use clippy_utils::{path_to_local_id, paths, peel_ref_operators, remove_blocks, strip_pat_refs}; use clippy_utils::{path_to_local_id, paths, peel_ref_operators, remove_blocks, strip_pat_refs};
use if_chain::if_chain; use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
@ -65,7 +65,7 @@ impl<'tcx> LateLintPass<'tcx> for ByteCount {
return; return;
}; };
if ty::Uint(UintTy::U8) == *cx.typeck_results().expr_ty(needle).peel_refs().kind(); if ty::Uint(UintTy::U8) == *cx.typeck_results().expr_ty(needle).peel_refs().kind();
if !LocalUsedVisitor::new(cx, arg_id).check_expr(needle); if !is_local_used(cx, needle, arg_id);
then { then {
let haystack = if let ExprKind::MethodCall(path, _, args, _) = let haystack = if let ExprKind::MethodCall(path, _, args, _) =
filter_recv.kind { filter_recv.kind {

View File

@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::visitors::LocalUsedVisitor; use clippy_utils::visitors::is_local_used;
use clippy_utils::{is_lang_ctor, path_to_local, peel_ref_operators, SpanlessEq}; use clippy_utils::{is_lang_ctor, path_to_local, peel_ref_operators, SpanlessEq};
use if_chain::if_chain; use if_chain::if_chain;
use rustc_hir::LangItem::OptionNone; use rustc_hir::LangItem::OptionNone;
@ -83,13 +83,12 @@ fn check_arm<'tcx>(arm: &Arm<'tcx>, wild_outer_arm: &Arm<'tcx>, cx: &LateContext
// the "wild-like" branches must be equal // the "wild-like" branches must be equal
if SpanlessEq::new(cx).eq_expr(wild_inner_arm.body, wild_outer_arm.body); if SpanlessEq::new(cx).eq_expr(wild_inner_arm.body, wild_outer_arm.body);
// the binding must not be used in the if guard // the binding must not be used in the if guard
let mut used_visitor = LocalUsedVisitor::new(cx, binding_id);
if match arm.guard { if match arm.guard {
None => true, None => true,
Some(Guard::If(expr) | Guard::IfLet(_, expr)) => !used_visitor.check_expr(expr), Some(Guard::If(expr) | Guard::IfLet(_, expr)) => !is_local_used(cx, expr, binding_id),
}; };
// ...or anywhere in the inner match // ...or anywhere in the inner match
if !arms_inner.iter().any(|arm| used_visitor.check_arm(arm)); if !arms_inner.iter().any(|arm| is_local_used(cx, arm, binding_id));
then { then {
span_lint_and_then( span_lint_and_then(
cx, cx,

View File

@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet; use clippy_utils::source::snippet;
use clippy_utils::{path_to_local_id, visitors::LocalUsedVisitor}; use clippy_utils::{path_to_local_id, visitors::is_local_used};
use if_chain::if_chain; use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir as hir; use rustc_hir as hir;
@ -65,11 +65,10 @@ impl<'tcx> LateLintPass<'tcx> for LetIfSeq {
if let hir::PatKind::Binding(mode, canonical_id, ident, None) = local.pat.kind; if let hir::PatKind::Binding(mode, canonical_id, ident, None) = local.pat.kind;
if let hir::StmtKind::Expr(if_) = expr.kind; if let hir::StmtKind::Expr(if_) = expr.kind;
if let hir::ExprKind::If(cond, then, ref else_) = if_.kind; if let hir::ExprKind::If(cond, then, ref else_) = if_.kind;
let mut used_visitor = LocalUsedVisitor::new(cx, canonical_id); if !is_local_used(cx, cond, canonical_id);
if !used_visitor.check_expr(cond);
if let hir::ExprKind::Block(then, _) = then.kind; if let hir::ExprKind::Block(then, _) = then.kind;
if let Some(value) = check_assign(cx, canonical_id, &*then); if let Some(value) = check_assign(cx, canonical_id, &*then);
if !used_visitor.check_expr(value); if !is_local_used(cx, value, canonical_id);
then { then {
let span = stmt.span.to(if_.span); let span = stmt.span.to(if_.span);
@ -148,15 +147,13 @@ fn check_assign<'tcx>(
if let hir::ExprKind::Assign(var, value, _) = expr.kind; if let hir::ExprKind::Assign(var, value, _) = expr.kind;
if path_to_local_id(var, decl); if path_to_local_id(var, decl);
then { then {
let mut v = LocalUsedVisitor::new(cx, decl); if block.stmts.iter().take(block.stmts.len()-1).any(|stmt| is_local_used(cx, stmt, decl)) {
None
if block.stmts.iter().take(block.stmts.len()-1).any(|stmt| v.check_stmt(stmt)) { } else {
return None; Some(value)
} }
} else {
return Some(value); None
} }
} }
None
} }

View File

@ -3,7 +3,7 @@ use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then};
use clippy_utils::source::snippet; use clippy_utils::source::snippet;
use clippy_utils::sugg; use clippy_utils::sugg;
use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::visitors::LocalUsedVisitor; use clippy_utils::visitors::is_local_used;
use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, Pat, PatKind}; use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, Pat, PatKind};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::ty; use rustc_middle::ty;
@ -66,9 +66,7 @@ pub(super) fn check<'tcx>(
fn pat_is_wild<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool { fn pat_is_wild<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool {
match *pat { match *pat {
PatKind::Wild => true, PatKind::Wild => true,
PatKind::Binding(_, id, ident, None) if ident.as_str().starts_with('_') => { PatKind::Binding(_, id, ident, None) if ident.as_str().starts_with('_') => !is_local_used(cx, body, id),
!LocalUsedVisitor::new(cx, id).check_expr(body)
},
_ => false, _ => false,
} }
} }

View File

@ -1,7 +1,7 @@
use super::utils::make_iterator_snippet; use super::utils::make_iterator_snippet;
use super::MANUAL_FLATTEN; use super::MANUAL_FLATTEN;
use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::visitors::LocalUsedVisitor; use clippy_utils::visitors::is_local_used;
use clippy_utils::{is_lang_ctor, path_to_local_id}; use clippy_utils::{is_lang_ctor, path_to_local_id};
use if_chain::if_chain; use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
@ -49,7 +49,7 @@ pub(super) fn check<'tcx>(
let ok_ctor = is_lang_ctor(cx, qpath, ResultOk); let ok_ctor = is_lang_ctor(cx, qpath, ResultOk);
if some_ctor || ok_ctor; if some_ctor || ok_ctor;
// Ensure epxr in `if let` is not used afterwards // Ensure epxr in `if let` is not used afterwards
if !LocalUsedVisitor::new(cx, pat_hir_id).check_arm(true_arm); if !is_local_used(cx, true_arm, pat_hir_id);
then { then {
let if_let_type = if some_ctor { "Some" } else { "Ok" }; let if_let_type = if some_ctor { "Some" } else { "Ok" };
// Prepare the error message // Prepare the error message

View File

@ -2,10 +2,8 @@ use super::NEEDLESS_RANGE_LOOP;
use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then}; use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then};
use clippy_utils::source::snippet; use clippy_utils::source::snippet;
use clippy_utils::ty::has_iter_method; use clippy_utils::ty::has_iter_method;
use clippy_utils::visitors::LocalUsedVisitor; use clippy_utils::visitors::is_local_used;
use clippy_utils::{ use clippy_utils::{contains_name, higher, is_integer_const, match_trait_method, paths, sugg, SpanlessEq};
contains_name, higher, is_integer_const, match_trait_method, path_to_local_id, paths, sugg, SpanlessEq,
};
use if_chain::if_chain; use if_chain::if_chain;
use rustc_ast::ast; use rustc_ast::ast;
use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::fx::{FxHashMap, FxHashSet};
@ -256,43 +254,36 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> {
if let ExprKind::Path(ref seqpath) = seqexpr.kind; if let ExprKind::Path(ref seqpath) = seqexpr.kind;
if let QPath::Resolved(None, seqvar) = *seqpath; if let QPath::Resolved(None, seqvar) = *seqpath;
if seqvar.segments.len() == 1; if seqvar.segments.len() == 1;
let index_used_directly = path_to_local_id(idx, self.var); if is_local_used(self.cx, idx, self.var);
let indexed_indirectly = {
let mut used_visitor = LocalUsedVisitor::new(self.cx, self.var);
walk_expr(&mut used_visitor, idx);
used_visitor.used
};
if indexed_indirectly || index_used_directly;
then { then {
if self.prefer_mutable { if self.prefer_mutable {
self.indexed_mut.insert(seqvar.segments[0].ident.name); self.indexed_mut.insert(seqvar.segments[0].ident.name);
} }
let index_used_directly = matches!(idx.kind, ExprKind::Path(_));
let res = self.cx.qpath_res(seqpath, seqexpr.hir_id); let res = self.cx.qpath_res(seqpath, seqexpr.hir_id);
match res { match res {
Res::Local(hir_id) => { Res::Local(hir_id) => {
let parent_id = self.cx.tcx.hir().get_parent_item(expr.hir_id); let parent_id = self.cx.tcx.hir().get_parent_item(expr.hir_id);
let parent_def_id = self.cx.tcx.hir().local_def_id(parent_id); let parent_def_id = self.cx.tcx.hir().local_def_id(parent_id);
let extent = self.cx.tcx.region_scope_tree(parent_def_id).var_scope(hir_id.local_id); let extent = self.cx.tcx.region_scope_tree(parent_def_id).var_scope(hir_id.local_id);
if indexed_indirectly {
self.indexed_indirectly.insert(seqvar.segments[0].ident.name, Some(extent));
}
if index_used_directly { if index_used_directly {
self.indexed_directly.insert( self.indexed_directly.insert(
seqvar.segments[0].ident.name, seqvar.segments[0].ident.name,
(Some(extent), self.cx.typeck_results().node_type(seqexpr.hir_id)), (Some(extent), self.cx.typeck_results().node_type(seqexpr.hir_id)),
); );
} else {
self.indexed_indirectly.insert(seqvar.segments[0].ident.name, Some(extent));
} }
return false; // no need to walk further *on the variable* return false; // no need to walk further *on the variable*
} }
Res::Def(DefKind::Static | DefKind::Const, ..) => { Res::Def(DefKind::Static | DefKind::Const, ..) => {
if indexed_indirectly {
self.indexed_indirectly.insert(seqvar.segments[0].ident.name, None);
}
if index_used_directly { if index_used_directly {
self.indexed_directly.insert( self.indexed_directly.insert(
seqvar.segments[0].ident.name, seqvar.segments[0].ident.name,
(None, self.cx.typeck_results().node_type(seqexpr.hir_id)), (None, self.cx.typeck_results().node_type(seqexpr.hir_id)),
); );
} else {
self.indexed_indirectly.insert(seqvar.segments[0].ident.name, None);
} }
return false; // no need to walk further *on the variable* return false; // no need to walk further *on the variable*
} }

View File

@ -5,7 +5,7 @@ use clippy_utils::diagnostics::{
use clippy_utils::source::{expr_block, indent_of, snippet, snippet_block, snippet_opt, snippet_with_applicability}; use clippy_utils::source::{expr_block, indent_of, snippet, snippet_block, snippet_opt, snippet_with_applicability};
use clippy_utils::sugg::Sugg; use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, match_type, peel_mid_ty_refs}; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, match_type, peel_mid_ty_refs};
use clippy_utils::visitors::LocalUsedVisitor; use clippy_utils::visitors::is_local_used;
use clippy_utils::{ use clippy_utils::{
get_parent_expr, in_macro, is_expn_of, is_lang_ctor, is_lint_allowed, is_refutable, is_wild, meets_msrv, msrvs, get_parent_expr, in_macro, is_expn_of, is_lang_ctor, is_lint_allowed, is_refutable, is_wild, meets_msrv, msrvs,
path_to_local, path_to_local_id, peel_hir_pat_refs, peel_n_hir_expr_refs, recurse_or_patterns, remove_blocks, path_to_local, path_to_local_id, peel_hir_pat_refs, peel_n_hir_expr_refs, recurse_or_patterns, remove_blocks,
@ -953,9 +953,7 @@ fn check_wild_err_arm<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm
// Looking for unused bindings (i.e.: `_e`) // Looking for unused bindings (i.e.: `_e`)
for pat in inner.iter() { for pat in inner.iter() {
if let PatKind::Binding(_, id, ident, None) = pat.kind { if let PatKind::Binding(_, id, ident, None) = pat.kind {
if ident.as_str().starts_with('_') if ident.as_str().starts_with('_') && !is_local_used(cx, arm.body, id) {
&& !LocalUsedVisitor::new(cx, id).check_expr(arm.body)
{
ident_bind_name = (&ident.name.as_str()).to_string(); ident_bind_name = (&ident.name.as_str()).to_string();
matching_wild = true; matching_wild = true;
} }

View File

@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::visitors::LocalUsedVisitor; use clippy_utils::visitors::is_local_used;
use if_chain::if_chain; use if_chain::if_chain;
use rustc_hir::{Impl, ImplItem, ImplItemKind, ItemKind}; use rustc_hir::{Impl, ImplItem, ImplItemKind, ItemKind};
use rustc_lint::{LateContext, LateLintPass}; use rustc_lint::{LateContext, LateLintPass};
@ -50,8 +50,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedSelf {
if let ImplItemKind::Fn(.., body_id) = &impl_item.kind; if let ImplItemKind::Fn(.., body_id) = &impl_item.kind;
let body = cx.tcx.hir().body(*body_id); let body = cx.tcx.hir().body(*body_id);
if let [self_param, ..] = body.params; if let [self_param, ..] = body.params;
let self_hir_id = self_param.pat.hir_id; if !is_local_used(cx, body, self_param.pat.hir_id);
if !LocalUsedVisitor::new(cx, self_hir_id).check_body(body);
then { then {
span_lint_and_help( span_lint_and_help(
cx, cx,

View File

@ -2,6 +2,7 @@
#![feature(in_band_lifetimes)] #![feature(in_band_lifetimes)]
#![feature(iter_zip)] #![feature(iter_zip)]
#![feature(rustc_private)] #![feature(rustc_private)]
#![feature(control_flow_enum)]
#![recursion_limit = "512"] #![recursion_limit = "512"]
#![cfg_attr(feature = "deny-warnings", deny(warnings))] #![cfg_attr(feature = "deny-warnings", deny(warnings))]
#![allow(clippy::missing_errors_doc, clippy::missing_panics_doc, clippy::must_use_candidate)] #![allow(clippy::missing_errors_doc, clippy::missing_panics_doc, clippy::must_use_candidate)]

View File

@ -4,6 +4,7 @@ use rustc_hir::intravisit::{self, walk_expr, ErasedMap, NestedVisitorMap, Visito
use rustc_hir::{def::Res, Arm, Block, Body, BodyId, Destination, Expr, ExprKind, HirId, Stmt}; use rustc_hir::{def::Res, Arm, Block, Body, BodyId, Destination, Expr, ExprKind, HirId, Stmt};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::hir::map::Map; use rustc_middle::hir::map::Map;
use std::ops::ControlFlow;
/// returns `true` if expr contains match expr desugared from try /// returns `true` if expr contains match expr desugared from try
fn contains_try(expr: &hir::Expr<'_>) -> bool { fn contains_try(expr: &hir::Expr<'_>) -> bool {
@ -133,62 +134,6 @@ where
} }
} }
pub struct LocalUsedVisitor<'hir> {
hir: Map<'hir>,
pub local_hir_id: HirId,
pub used: bool,
}
impl<'hir> LocalUsedVisitor<'hir> {
pub fn new(cx: &LateContext<'hir>, local_hir_id: HirId) -> Self {
Self {
hir: cx.tcx.hir(),
local_hir_id,
used: false,
}
}
fn check<T>(&mut self, t: T, visit: fn(&mut Self, T)) -> bool {
visit(self, t);
std::mem::replace(&mut self.used, false)
}
pub fn check_arm(&mut self, arm: &'hir Arm<'_>) -> bool {
self.check(arm, Self::visit_arm)
}
pub fn check_body(&mut self, body: &'hir Body<'_>) -> bool {
self.check(body, Self::visit_body)
}
pub fn check_expr(&mut self, expr: &'hir Expr<'_>) -> bool {
self.check(expr, Self::visit_expr)
}
pub fn check_stmt(&mut self, stmt: &'hir Stmt<'_>) -> bool {
self.check(stmt, Self::visit_stmt)
}
}
impl<'v> Visitor<'v> for LocalUsedVisitor<'v> {
type Map = Map<'v>;
fn visit_expr(&mut self, expr: &'v Expr<'v>) {
if self.used {
return;
}
if path_to_local_id(expr, self.local_hir_id) {
self.used = true;
} else {
walk_expr(self, expr);
}
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::OnlyBodies(self.hir)
}
}
/// A type which can be visited. /// A type which can be visited.
pub trait Visitable<'tcx> { pub trait Visitable<'tcx> {
/// Calls the corresponding `visit_*` function on the visitor. /// Calls the corresponding `visit_*` function on the visitor.
@ -202,8 +147,22 @@ macro_rules! visitable_ref {
} }
} }
}; };
([$t:ident], $f:ident) => {
impl Visitable<'tcx> for &'tcx [$t<'tcx>] {
fn visit<V: Visitor<'tcx>>(self, visitor: &mut V) {
for x in self {
visitor.$f(x);
}
}
}
};
} }
visitable_ref!(Arm, visit_arm);
visitable_ref!(Block, visit_block); visitable_ref!(Block, visit_block);
visitable_ref!(Body, visit_body);
visitable_ref!(Expr, visit_expr);
visitable_ref!(Stmt, visit_stmt);
visitable_ref!([Stmt], visit_stmt);
/// Calls the given function for each break expression. /// Calls the given function for each break expression.
pub fn visit_break_exprs<'tcx>( pub fn visit_break_exprs<'tcx>(
@ -260,3 +219,48 @@ pub fn is_res_used(cx: &LateContext<'_>, res: Res, body: BodyId) -> bool {
v.visit_expr(&cx.tcx.hir().body(body).value); v.visit_expr(&cx.tcx.hir().body(body).value);
v.found v.found
} }
/// Calls the given function for each usage of the given local.
pub fn for_each_local_usage<'tcx, B>(
cx: &LateContext<'tcx>,
visitable: impl Visitable<'tcx>,
id: HirId,
f: impl FnMut(&'tcx Expr<'tcx>) -> ControlFlow<B>,
) -> ControlFlow<B> {
struct V<'tcx, B, F> {
map: Map<'tcx>,
id: HirId,
f: F,
res: ControlFlow<B>,
}
impl<'tcx, B, F: FnMut(&'tcx Expr<'tcx>) -> ControlFlow<B>> Visitor<'tcx> for V<'tcx, B, F> {
type Map = Map<'tcx>;
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::OnlyBodies(self.map)
}
fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
if self.res.is_continue() {
if path_to_local_id(e, self.id) {
self.res = (self.f)(e);
} else {
walk_expr(self, e);
}
}
}
}
let mut v = V {
map: cx.tcx.hir(),
id,
f,
res: ControlFlow::CONTINUE,
};
visitable.visit(&mut v);
v.res
}
/// Checks if the given local is used.
pub fn is_local_used(cx: &LateContext<'tcx>, visitable: impl Visitable<'tcx>, id: HirId) -> bool {
for_each_local_usage(cx, visitable, id, |_| ControlFlow::BREAK).is_break()
}