Auto merge of #7938 - camsteffen:visitors, r=xFrednet

Introduce `expr_visitor` and `expr_visitor_no_bodies`

changelog: none

A couple utils that satisfy a *lot* of visitor use cases. Factoring in every possible usage would be really big so I just focused on cleaning clippy_utils.
This commit is contained in:
bors 2021-11-08 13:39:58 +00:00
commit 94517d397c
8 changed files with 164 additions and 312 deletions

View File

@ -1,8 +1,8 @@
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::higher::VecArgs;
use clippy_utils::source::snippet_opt;
use clippy_utils::usage::UsedAfterExprVisitor;
use clippy_utils::{get_enclosing_loop_or_closure, higher, path_to_local_id};
use clippy_utils::usage::local_used_after_expr;
use clippy_utils::{get_enclosing_loop_or_closure, higher, path_to_local, path_to_local_id};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::def_id::DefId;
@ -118,7 +118,7 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction {
if let ty::Closure(_, substs) = callee_ty.peel_refs().kind();
if substs.as_closure().kind() == ClosureKind::FnMut;
if get_enclosing_loop_or_closure(cx.tcx, expr).is_some()
|| UsedAfterExprVisitor::is_found(cx, callee);
|| path_to_local(callee).map_or(false, |l| local_used_after_expr(cx, l, callee));
then {
// Mutable closure is used after current expr; we cannot consume it.

View File

@ -10,7 +10,7 @@ use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::adjustment::{Adjust, Adjustment};
use rustc_middle::ty::Ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{sym, BytePos, ExpnData, ExpnKind, Span, Symbol};
use rustc_span::{sym, ExpnData, ExpnKind, Span, Symbol};
declare_clippy_lint! {
/// ### What it does
@ -128,7 +128,7 @@ fn check_format_in_format_args(cx: &LateContext<'_>, call_site: Span, name: Symb
span_lint_and_then(
cx,
FORMAT_IN_FORMAT_ARGS,
trim_semicolon(cx, call_site),
call_site,
&format!("`format!` in `{}!` args", name),
|diag| {
diag.help(&format!(
@ -192,13 +192,6 @@ fn is_aliased(args: &[FormatArgsArg<'_>], i: usize) -> bool {
.any(|(j, arg)| i != j && std::ptr::eq(value, arg.value))
}
fn trim_semicolon(cx: &LateContext<'_>, span: Span) -> Span {
snippet_opt(cx, span).map_or(span, |snippet| {
let snippet = snippet.trim_end_matches(';');
span.with_hi(span.lo() + BytePos(u32::try_from(snippet.len()).unwrap()))
})
}
fn count_needed_derefs<'tcx, I>(mut ty: Ty<'tcx>, mut iter: I) -> (usize, Ty<'tcx>)
where
I: Iterator<Item = &'tcx Adjustment<'tcx>>,

View File

@ -2,10 +2,10 @@ use clippy_utils::{
diagnostics::span_lint_and_sugg,
get_async_fn_body, is_async_fn,
source::{snippet_with_applicability, snippet_with_context, walk_span_to_context},
visitors::visit_break_exprs,
visitors::expr_visitor_no_bodies,
};
use rustc_errors::Applicability;
use rustc_hir::intravisit::FnKind;
use rustc_hir::intravisit::{FnKind, Visitor};
use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, FnRetTy, HirId};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
@ -144,20 +144,24 @@ fn lint_implicit_returns(
ExprKind::Loop(block, ..) => {
let mut add_return = false;
visit_break_exprs(block, |break_expr, dest, sub_expr| {
if dest.target_id.ok() == Some(expr.hir_id) {
if call_site_span.is_none() && break_expr.span.ctxt() == ctxt {
// At this point sub_expr can be `None` in async functions which either diverge, or return the
// unit type.
if let Some(sub_expr) = sub_expr {
lint_break(cx, break_expr.span, sub_expr.span);
expr_visitor_no_bodies(|e| {
if let ExprKind::Break(dest, sub_expr) = e.kind {
if dest.target_id.ok() == Some(expr.hir_id) {
if call_site_span.is_none() && e.span.ctxt() == ctxt {
// At this point sub_expr can be `None` in async functions which either diverge, or return
// the unit type.
if let Some(sub_expr) = sub_expr {
lint_break(cx, e.span, sub_expr.span);
}
} else {
// the break expression is from a macro call, add a return to the loop
add_return = true;
}
} else {
// the break expression is from a macro call, add a return to the loop
add_return = true;
}
}
});
true
})
.visit_block(block);
if add_return {
#[allow(clippy::option_if_let_else)]
if let Some(span) = call_site_span {

View File

@ -632,9 +632,6 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
if let ExprKind::Match(ex, arms, _) = expr.kind {
check_match_ref_pats(cx, ex, arms.iter().map(|el| el.pat), expr);
}
if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(cx, expr) {
check_match_ref_pats(cx, let_expr, once(let_pat), expr);
}
}
fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'_>) {

View File

@ -1,6 +1,7 @@
#![feature(box_patterns)]
#![feature(in_band_lifetimes)]
#![feature(iter_zip)]
#![feature(let_else)]
#![feature(rustc_private)]
#![feature(control_flow_enum)]
#![recursion_limit = "512"]
@ -68,7 +69,7 @@ use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::hir_id::{HirIdMap, HirIdSet};
use rustc_hir::intravisit::{self, walk_expr, ErasedMap, FnKind, NestedVisitorMap, Visitor};
use rustc_hir::intravisit::{walk_expr, ErasedMap, FnKind, NestedVisitorMap, Visitor};
use rustc_hir::itemlikevisit::ItemLikeVisitor;
use rustc_hir::LangItem::{OptionNone, ResultErr, ResultOk};
use rustc_hir::{
@ -96,6 +97,7 @@ use rustc_target::abi::Integer;
use crate::consts::{constant, Constant};
use crate::ty::{can_partially_move_ty, is_copy, is_recursively_primitive_type};
use crate::visitors::expr_visitor_no_bodies;
pub fn parse_msrv(msrv: &str, sess: Option<&Session>, span: Option<Span>) -> Option<RustcVersion> {
if let Ok(version) = RustcVersion::parse(msrv) {
@ -1107,63 +1109,30 @@ pub fn contains_name(name: Symbol, expr: &Expr<'_>) -> bool {
/// Returns `true` if `expr` contains a return expression
pub fn contains_return(expr: &hir::Expr<'_>) -> bool {
struct RetCallFinder {
found: bool,
}
impl<'tcx> hir::intravisit::Visitor<'tcx> for RetCallFinder {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
if self.found {
return;
}
let mut found = false;
expr_visitor_no_bodies(|expr| {
if !found {
if let hir::ExprKind::Ret(..) = &expr.kind {
self.found = true;
} else {
hir::intravisit::walk_expr(self, expr);
found = true;
}
}
fn nested_visit_map(&mut self) -> hir::intravisit::NestedVisitorMap<Self::Map> {
hir::intravisit::NestedVisitorMap::None
}
}
let mut visitor = RetCallFinder { found: false };
visitor.visit_expr(expr);
visitor.found
}
struct FindMacroCalls<'a, 'b> {
names: &'a [&'b str],
result: Vec<Span>,
}
impl<'a, 'b, 'tcx> Visitor<'tcx> for FindMacroCalls<'a, 'b> {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if self.names.iter().any(|fun| is_expn_of(expr.span, fun).is_some()) {
self.result.push(expr.span);
}
// and check sub-expressions
intravisit::walk_expr(self, expr);
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
!found
})
.visit_expr(expr);
found
}
/// Finds calls of the specified macros in a function body.
pub fn find_macro_calls(names: &[&str], body: &Body<'_>) -> Vec<Span> {
let mut fmc = FindMacroCalls {
names,
result: Vec::new(),
};
fmc.visit_expr(&body.value);
fmc.result
let mut result = Vec::new();
expr_visitor_no_bodies(|expr| {
if names.iter().any(|fun| is_expn_of(expr.span, fun).is_some()) {
result.push(expr.span);
}
true
})
.visit_expr(&body.value);
result
}
/// Extends the span to the beginning of the spans line, incl. whitespaces.

View File

@ -1,9 +1,9 @@
use crate::source::snippet;
use crate::visitors::expr_visitor_no_bodies;
use crate::{path_to_local_id, strip_pat_refs};
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{Body, BodyId, Expr, ExprKind, HirId, PatKind};
use rustc_hir::intravisit::Visitor;
use rustc_hir::{Body, BodyId, ExprKind, HirId, PatKind};
use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
use rustc_span::Span;
use std::borrow::Cow;
@ -30,50 +30,28 @@ fn extract_clone_suggestions<'tcx>(
replace: &[(&'static str, &'static str)],
body: &'tcx Body<'_>,
) -> Option<Vec<(Span, Cow<'static, str>)>> {
let mut visitor = PtrCloneVisitor {
cx,
id,
replace,
spans: vec![],
abort: false,
};
visitor.visit_body(body);
if visitor.abort { None } else { Some(visitor.spans) }
}
struct PtrCloneVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
id: HirId,
replace: &'a [(&'static str, &'static str)],
spans: Vec<(Span, Cow<'static, str>)>,
abort: bool,
}
impl<'a, 'tcx> Visitor<'tcx> for PtrCloneVisitor<'a, 'tcx> {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if self.abort {
return;
let mut abort = false;
let mut spans = Vec::new();
expr_visitor_no_bodies(|expr| {
if abort {
return false;
}
if let ExprKind::MethodCall(seg, _, [recv], _) = expr.kind {
if path_to_local_id(recv, self.id) {
if path_to_local_id(recv, id) {
if seg.ident.name.as_str() == "capacity" {
self.abort = true;
return;
abort = true;
return false;
}
for &(fn_name, suffix) in self.replace {
for &(fn_name, suffix) in replace {
if seg.ident.name.as_str() == fn_name {
self.spans.push((expr.span, snippet(self.cx, recv.span, "_") + suffix));
return;
spans.push((expr.span, snippet(cx, recv.span, "_") + suffix));
return false;
}
}
}
}
walk_expr(self, expr);
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
!abort
})
.visit_body(body);
if abort { None } else { Some(spans) }
}

View File

@ -1,7 +1,7 @@
use crate as utils;
use crate::visitors::{expr_visitor, expr_visitor_no_bodies};
use rustc_hir as hir;
use rustc_hir::intravisit;
use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::HirIdSet;
use rustc_hir::{Expr, ExprKind, HirId};
use rustc_infer::infer::TyCtxtInferExt;
@ -148,96 +148,47 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for BindingUsageFinder<'a, 'tcx> {
}
}
struct ReturnBreakContinueMacroVisitor {
seen_return_break_continue: bool,
}
impl ReturnBreakContinueMacroVisitor {
fn new() -> ReturnBreakContinueMacroVisitor {
ReturnBreakContinueMacroVisitor {
seen_return_break_continue: false,
}
}
}
impl<'tcx> Visitor<'tcx> for ReturnBreakContinueMacroVisitor {
type Map = Map<'tcx>;
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
if self.seen_return_break_continue {
// No need to look farther if we've already seen one of them
return;
pub fn contains_return_break_continue_macro(expression: &Expr<'_>) -> bool {
let mut seen_return_break_continue = false;
expr_visitor_no_bodies(|ex| {
if seen_return_break_continue {
return false;
}
match &ex.kind {
ExprKind::Ret(..) | ExprKind::Break(..) | ExprKind::Continue(..) => {
self.seen_return_break_continue = true;
seen_return_break_continue = true;
},
// Something special could be done here to handle while or for loop
// desugaring, as this will detect a break if there's a while loop
// or a for loop inside the expression.
_ => {
if ex.span.from_expansion() {
self.seen_return_break_continue = true;
} else {
rustc_hir::intravisit::walk_expr(self, ex);
seen_return_break_continue = true;
}
},
}
}
!seen_return_break_continue
})
.visit_expr(expression);
seen_return_break_continue
}
pub fn contains_return_break_continue_macro(expression: &Expr<'_>) -> bool {
let mut recursive_visitor = ReturnBreakContinueMacroVisitor::new();
recursive_visitor.visit_expr(expression);
recursive_visitor.seen_return_break_continue
}
pub struct UsedAfterExprVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
expr: &'tcx Expr<'tcx>,
definition: HirId,
past_expr: bool,
used_after_expr: bool,
}
impl<'a, 'tcx> UsedAfterExprVisitor<'a, 'tcx> {
pub fn is_found(cx: &'a LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
utils::path_to_local(expr).map_or(false, |definition| {
let mut visitor = UsedAfterExprVisitor {
cx,
expr,
definition,
past_expr: false,
used_after_expr: false,
};
utils::get_enclosing_block(cx, definition).map_or(false, |block| {
visitor.visit_block(block);
visitor.used_after_expr
})
})
}
}
impl<'a, 'tcx> intravisit::Visitor<'tcx> for UsedAfterExprVisitor<'a, 'tcx> {
type Map = Map<'tcx>;
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::OnlyBodies(self.cx.tcx.hir())
}
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
if self.used_after_expr {
return;
pub fn local_used_after_expr(cx: &LateContext<'_>, local_id: HirId, after: &Expr<'_>) -> bool {
let Some(block) = utils::get_enclosing_block(cx, local_id) else { return false };
let mut used_after_expr = false;
let mut past_expr = false;
expr_visitor(cx, |expr| {
if used_after_expr {
return false;
}
if expr.hir_id == self.expr.hir_id {
self.past_expr = true;
} else if self.past_expr && utils::path_to_local_id(expr, self.definition) {
self.used_after_expr = true;
} else {
intravisit::walk_expr(self, expr);
if expr.hir_id == after.hir_id {
past_expr = true;
} else if past_expr && utils::path_to_local_id(expr, local_id) {
used_after_expr = true;
}
}
!used_after_expr
})
.visit_block(block);
used_after_expr
}

View File

@ -1,38 +1,66 @@
use crate::path_to_local_id;
use rustc_hir as hir;
use rustc_hir::intravisit::{self, walk_expr, ErasedMap, NestedVisitorMap, Visitor};
use rustc_hir::{def::Res, Arm, Block, Body, BodyId, Destination, Expr, ExprKind, HirId, Stmt};
use rustc_hir::intravisit::{self, walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{def::Res, Arm, Block, Body, BodyId, Expr, ExprKind, HirId, Stmt};
use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
use std::ops::ControlFlow;
/// Convenience method for creating a `Visitor` with just `visit_expr` overridden and nested
/// bodies (i.e. closures) are visited.
/// If the callback returns `true`, the expr just provided to the callback is walked.
#[must_use]
pub fn expr_visitor<'tcx>(cx: &LateContext<'tcx>, f: impl FnMut(&'tcx Expr<'tcx>) -> bool) -> impl Visitor<'tcx> {
struct V<'tcx, F> {
hir: Map<'tcx>,
f: F,
}
impl<'tcx, F: FnMut(&'tcx Expr<'tcx>) -> bool> Visitor<'tcx> for V<'tcx, F> {
type Map = Map<'tcx>;
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::OnlyBodies(self.hir)
}
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
if (self.f)(expr) {
walk_expr(self, expr);
}
}
}
V { hir: cx.tcx.hir(), f }
}
/// Convenience method for creating a `Visitor` with just `visit_expr` overridden and nested
/// bodies (i.e. closures) are not visited.
/// If the callback returns `true`, the expr just provided to the callback is walked.
#[must_use]
pub fn expr_visitor_no_bodies<'tcx>(f: impl FnMut(&'tcx Expr<'tcx>) -> bool) -> impl Visitor<'tcx> {
struct V<F>(F);
impl<'tcx, F: FnMut(&'tcx Expr<'tcx>) -> bool> Visitor<'tcx> for V<F> {
type Map = intravisit::ErasedMap<'tcx>;
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
if (self.0)(e) {
walk_expr(self, e);
}
}
}
V(f)
}
/// returns `true` if expr contains match expr desugared from try
fn contains_try(expr: &hir::Expr<'_>) -> bool {
struct TryFinder {
found: bool,
}
impl<'hir> intravisit::Visitor<'hir> for TryFinder {
type Map = Map<'hir>;
fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
intravisit::NestedVisitorMap::None
let mut found = false;
expr_visitor_no_bodies(|e| {
if !found {
found = matches!(e.kind, hir::ExprKind::Match(_, _, hir::MatchSource::TryDesugar));
}
fn visit_expr(&mut self, expr: &'hir hir::Expr<'hir>) {
if self.found {
return;
}
match expr.kind {
hir::ExprKind::Match(_, _, hir::MatchSource::TryDesugar) => self.found = true,
_ => intravisit::walk_expr(self, expr),
}
}
}
let mut visitor = TryFinder { found: false };
visitor.visit_expr(expr);
visitor.found
!found
})
.visit_expr(expr);
found
}
pub fn find_all_ret_expressions<'hir, F>(_cx: &LateContext<'_>, expr: &'hir hir::Expr<'hir>, callback: F) -> bool
@ -165,103 +193,35 @@ visitable_ref!(Stmt, visit_stmt);
// }
// }
/// Calls the given function for each break expression.
pub fn visit_break_exprs<'tcx>(
node: impl Visitable<'tcx>,
f: impl FnMut(&'tcx Expr<'tcx>, Destination, Option<&'tcx Expr<'tcx>>),
) {
struct V<F>(F);
impl<'tcx, F: FnMut(&'tcx Expr<'tcx>, Destination, Option<&'tcx Expr<'tcx>>)> Visitor<'tcx> for V<F> {
type Map = ErasedMap<'tcx>;
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
if let ExprKind::Break(dest, sub_expr) = e.kind {
self.0(e, dest, sub_expr);
}
walk_expr(self, e);
}
}
node.visit(&mut V(f));
}
/// Checks if the given resolved path is used in the given body.
pub fn is_res_used(cx: &LateContext<'_>, res: Res, body: BodyId) -> bool {
struct V<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
res: Res,
found: bool,
}
impl Visitor<'tcx> for V<'_, 'tcx> {
type Map = Map<'tcx>;
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::OnlyBodies(self.cx.tcx.hir())
let mut found = false;
expr_visitor(cx, |e| {
if found {
return false;
}
fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
if self.found {
return;
}
if let ExprKind::Path(p) = &e.kind {
if self.cx.qpath_res(p, e.hir_id) == self.res {
self.found = true;
}
} else {
walk_expr(self, e);
if let ExprKind::Path(p) = &e.kind {
if cx.qpath_res(p, e.hir_id) == res {
found = true;
}
}
}
let mut v = V { cx, res, found: false };
v.visit_expr(&cx.tcx.hir().body(body).value);
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
!found
})
.visit_expr(&cx.tcx.hir().body(body).value);
found
}
/// 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()
let mut is_used = false;
let mut visitor = expr_visitor(cx, |expr| {
if !is_used {
is_used = path_to_local_id(expr, id);
}
!is_used
});
visitable.visit(&mut visitor);
drop(visitor);
is_used
}