Cleanup path to local checks

This commit is contained in:
Cameron Steffen 2021-02-01 19:22:31 -06:00
parent 357c6a7e27
commit 56f7fbb4ae
14 changed files with 180 additions and 301 deletions

View File

@ -1,10 +1,10 @@
use crate::utils::visitors::LocalUsedVisitor;
use crate::utils::{span_lint_and_then, SpanlessEq};
use crate::utils::{path_to_local, span_lint_and_then, SpanlessEq};
use if_chain::if_chain;
use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, Pat, PatKind, QPath, StmtKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{DefIdTree, TyCtxt};
use rustc_middle::ty::{DefIdTree, TyCtxt, TypeckResults};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{MultiSpan, Span};
@ -72,7 +72,7 @@ fn check_arm(arm: &Arm<'_>, wild_outer_arm: &Arm<'_>, cx: &LateContext<'_>) {
if arms_inner.iter().all(|arm| arm.guard.is_none());
// match expression must be a local binding
// match <local> { .. }
if let Some(binding_id) = addr_adjusted_binding(expr_in, cx);
if let Some(binding_id) = path_to_local(strip_ref_operators(expr_in, cx.typeck_results()));
// one of the branches must be "wild-like"
if let Some(wild_inner_arm_idx) = arms_inner.iter().rposition(|arm_inner| arm_is_wild_like(arm_inner, cx.tcx));
let (wild_inner_arm, non_wild_inner_arm) =
@ -175,19 +175,15 @@ fn is_none_ctor(res: Res, tcx: TyCtxt<'_>) -> bool {
false
}
/// Retrieves a binding ID with optional `&` and/or `*` operators removed. (e.g. `&**foo`)
/// Returns `None` if a non-reference type is de-referenced.
/// For example, if `Vec` is de-referenced to a slice, `None` is returned.
fn addr_adjusted_binding(mut expr: &Expr<'_>, cx: &LateContext<'_>) -> Option<HirId> {
/// Removes `AddrOf` operators (`&`) or deref operators (`*`), but only if a reference type is
/// dereferenced. An overloaded deref such as `Vec` to slice would not be removed.
fn strip_ref_operators<'hir>(mut expr: &'hir Expr<'hir>, typeck_results: &TypeckResults<'_>) -> &'hir Expr<'hir> {
loop {
match expr.kind {
ExprKind::AddrOf(_, _, e) => expr = e,
ExprKind::Path(QPath::Resolved(None, path)) => match path.res {
Res::Local(binding_id) => break Some(binding_id),
_ => break None,
},
ExprKind::Unary(UnOp::UnDeref, e) if cx.typeck_results().expr_ty(e).is_ref() => expr = e,
_ => break None,
ExprKind::Unary(UnOp::UnDeref, e) if typeck_results.expr_ty(e).is_ref() => expr = e,
_ => break,
}
}
expr
}

View File

@ -1,7 +1,6 @@
use crate::utils::{get_parent_expr, span_lint, span_lint_and_note};
use if_chain::if_chain;
use crate::utils::{get_parent_expr, path_to_local, path_to_local_id, span_lint, span_lint_and_note};
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{def, BinOpKind, Block, Expr, ExprKind, Guard, HirId, Local, Node, QPath, Stmt, StmtKind};
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, Guard, HirId, Local, Node, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::map::Map;
use rustc_middle::ty;
@ -72,20 +71,14 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
// Find a write to a local variable.
match expr.kind {
ExprKind::Assign(ref lhs, ..) | ExprKind::AssignOp(_, ref lhs, _) => {
if let ExprKind::Path(ref qpath) = lhs.kind {
if let QPath::Resolved(_, ref path) = *qpath {
if path.segments.len() == 1 {
if let def::Res::Local(var) = cx.qpath_res(qpath, lhs.hir_id) {
let mut visitor = ReadVisitor {
cx,
var,
write_expr: expr,
last_expr: expr,
};
check_for_unsequenced_reads(&mut visitor);
}
}
}
if let Some(var) = path_to_local(lhs) {
let mut visitor = ReadVisitor {
cx,
var,
write_expr: expr,
last_expr: expr,
};
check_for_unsequenced_reads(&mut visitor);
}
},
_ => {},
@ -304,27 +297,20 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
return;
}
match expr.kind {
ExprKind::Path(ref qpath) => {
if_chain! {
if let QPath::Resolved(None, ref path) = *qpath;
if path.segments.len() == 1;
if let def::Res::Local(local_id) = self.cx.qpath_res(qpath, expr.hir_id);
if local_id == self.var;
// Check that this is a read, not a write.
if !is_in_assignment_position(self.cx, expr);
then {
span_lint_and_note(
self.cx,
EVAL_ORDER_DEPENDENCE,
expr.span,
"unsequenced read of a variable",
Some(self.write_expr.span),
"whether read occurs before this write depends on evaluation order"
);
}
}
if path_to_local_id(expr, self.var) {
// Check that this is a read, not a write.
if !is_in_assignment_position(self.cx, expr) {
span_lint_and_note(
self.cx,
EVAL_ORDER_DEPENDENCE,
expr.span,
"unsequenced read of a variable",
Some(self.write_expr.span),
"whether read occurs before this write depends on evaluation order",
);
}
}
match expr.kind {
// We're about to descend a closure. Since we don't know when (or
// if) the closure will be evaluated, any reads in it might not
// occur here (or ever). Like above, bail to avoid false positives.

View File

@ -1,7 +1,7 @@
use crate::utils::{
attr_by_name, attrs::is_proc_macro, is_must_use_ty, is_trait_impl_item, is_type_diagnostic_item, iter_input_pats,
last_path_segment, match_def_path, must_use_attr, return_ty, snippet, snippet_opt, span_lint, span_lint_and_help,
span_lint_and_then, trait_ref_of_method, type_is_unsafe_function,
last_path_segment, match_def_path, must_use_attr, path_to_local, return_ty, snippet, snippet_opt, span_lint,
span_lint_and_help, span_lint_and_then, trait_ref_of_method, type_is_unsafe_function,
};
use if_chain::if_chain;
use rustc_ast::ast::Attribute;
@ -9,7 +9,7 @@
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::intravisit;
use rustc_hir::{def::Res, def_id::DefId};
use rustc_hir::{def::Res, def_id::DefId, QPath};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::hir::map::Map;
use rustc_middle::lint::in_external_macro;
@ -658,16 +658,14 @@ fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
impl<'a, 'tcx> DerefVisitor<'a, 'tcx> {
fn check_arg(&self, ptr: &hir::Expr<'_>) {
if let hir::ExprKind::Path(ref qpath) = ptr.kind {
if let Res::Local(id) = self.cx.qpath_res(qpath, ptr.hir_id) {
if self.ptrs.contains(&id) {
span_lint(
self.cx,
NOT_UNSAFE_PTR_ARG_DEREF,
ptr.span,
"this public function dereferences a raw pointer but is not marked `unsafe`",
);
}
if let Some(id) = path_to_local(ptr) {
if self.ptrs.contains(&id) {
span_lint(
self.cx,
NOT_UNSAFE_PTR_ARG_DEREF,
ptr.span,
"this public function dereferences a raw pointer but is not marked `unsafe`",
);
}
}
}
@ -698,7 +696,7 @@ fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
arg.span,
&mut tys,
)
&& is_mutated_static(self.cx, arg)
&& is_mutated_static(arg)
{
self.mutates_static = true;
return;
@ -707,7 +705,7 @@ fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
}
},
Assign(ref target, ..) | AssignOp(_, ref target, _) | AddrOf(_, hir::Mutability::Mut, ref target) => {
self.mutates_static |= is_mutated_static(self.cx, target)
self.mutates_static |= is_mutated_static(target)
},
_ => {},
}
@ -718,12 +716,13 @@ fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
}
}
fn is_mutated_static(cx: &LateContext<'_>, e: &hir::Expr<'_>) -> bool {
fn is_mutated_static(e: &hir::Expr<'_>) -> bool {
use hir::ExprKind::{Field, Index, Path};
match e.kind {
Path(ref qpath) => !matches!(cx.qpath_res(qpath, e.hir_id), Res::Local(_)),
Field(ref inner, _) | Index(ref inner, _) => is_mutated_static(cx, inner),
Path(QPath::Resolved(_, path)) => !matches!(path.res, Res::Local(_)),
Path(_) => true,
Field(ref inner, _) | Index(ref inner, _) => is_mutated_static(inner),
_ => false,
}
}

View File

@ -1,8 +1,7 @@
use crate::utils::{snippet, span_lint_and_then, visitors::LocalUsedVisitor};
use crate::utils::{path_to_local_id, snippet, span_lint_and_then, visitors::LocalUsedVisitor};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_hir::BindingAnnotation;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
@ -66,7 +65,7 @@ fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) {
if let hir::ExprKind::If(ref cond, ref then, ref else_) = if_.kind;
if !LocalUsedVisitor::new(canonical_id).check_expr(cond);
if let hir::ExprKind::Block(ref then, _) = then.kind;
if let Some(value) = check_assign(cx, canonical_id, &*then);
if let Some(value) = check_assign(canonical_id, &*then);
if !LocalUsedVisitor::new(canonical_id).check_expr(value);
then {
let span = stmt.span.to(if_.span);
@ -79,7 +78,7 @@ fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) {
let (default_multi_stmts, default) = if let Some(ref else_) = *else_ {
if let hir::ExprKind::Block(ref else_, _) = else_.kind {
if let Some(default) = check_assign(cx, canonical_id, else_) {
if let Some(default) = check_assign(canonical_id, else_) {
(else_.stmts.len() > 1, default)
} else if let Some(ref default) = local.init {
(true, &**default)
@ -134,19 +133,13 @@ fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) {
}
}
fn check_assign<'tcx>(
cx: &LateContext<'tcx>,
decl: hir::HirId,
block: &'tcx hir::Block<'_>,
) -> Option<&'tcx hir::Expr<'tcx>> {
fn check_assign<'tcx>(decl: hir::HirId, block: &'tcx hir::Block<'_>) -> Option<&'tcx hir::Expr<'tcx>> {
if_chain! {
if block.expr.is_none();
if let Some(expr) = block.stmts.iter().last();
if let hir::StmtKind::Semi(ref expr) = expr.kind;
if let hir::ExprKind::Assign(ref var, ref value, _) = expr.kind;
if let hir::ExprKind::Path(ref qpath) = var.kind;
if let Res::Local(local_id) = cx.qpath_res(qpath, var.hir_id);
if decl == local_id;
if path_to_local_id(var, decl);
then {
let mut v = LocalUsedVisitor::new(decl);

View File

@ -1,14 +1,13 @@
use crate::consts::constant;
use crate::utils::paths;
use crate::utils::sugg::Sugg;
use crate::utils::usage::{is_unused, mutated_variables};
use crate::utils::usage::mutated_variables;
use crate::utils::visitors::LocalUsedVisitor;
use crate::utils::{
contains_name, get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
indent_of, is_in_panic_handler, is_integer_const, is_no_std_crate, is_ok_ctor, is_refutable, is_some_ctor,
is_type_diagnostic_item, last_path_segment, match_trait_method, match_type, match_var, multispan_sugg,
single_segment_path, snippet, snippet_with_applicability, snippet_with_macro_callsite, span_lint,
span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg, SpanlessEq,
is_type_diagnostic_item, last_path_segment, match_trait_method, match_type, multispan_sugg, path_to_local,
path_to_local_id, paths, single_segment_path, snippet, snippet_with_applicability, snippet_with_macro_callsite,
span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg, SpanlessEq,
};
use if_chain::if_chain;
use rustc_ast::ast;
@ -877,21 +876,6 @@ fn get_span_of_entire_for_loop(expr: &Expr<'_>) -> Span {
}
}
fn same_var<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, var: HirId) -> bool {
if_chain! {
if let ExprKind::Path(qpath) = &expr.kind;
if let QPath::Resolved(None, path) = qpath;
if path.segments.len() == 1;
if let Res::Local(local_id) = cx.qpath_res(qpath, expr.hir_id);
then {
// our variable!
local_id == var
} else {
false
}
}
}
/// a wrapper of `Sugg`. Besides what `Sugg` do, this removes unnecessary `0`;
/// and also, it avoids subtracting a variable from the same one by replacing it with `0`.
/// it exists for the convenience of the overloaded operators while normal functions can do the
@ -1044,14 +1028,9 @@ fn get_details_from_idx<'tcx>(
idx: &Expr<'_>,
starts: &[Start<'tcx>],
) -> Option<(StartKind<'tcx>, Offset)> {
fn get_start<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, starts: &[Start<'tcx>]) -> Option<StartKind<'tcx>> {
starts.iter().find_map(|start| {
if same_var(cx, e, start.id) {
Some(start.kind)
} else {
None
}
})
fn get_start<'tcx>(e: &Expr<'_>, starts: &[Start<'tcx>]) -> Option<StartKind<'tcx>> {
let id = path_to_local(e)?;
starts.iter().find(|start| start.id == id).map(|start| start.kind)
}
fn get_offset<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, starts: &[Start<'tcx>]) -> Option<Sugg<'static>> {
@ -1060,7 +1039,7 @@ fn get_offset<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, starts: &[Start<'tcx>]
ast::LitKind::Int(x, _ty) => Some(Sugg::NonParen(x.to_string().into())),
_ => None,
},
ExprKind::Path(..) if get_start(cx, e, starts).is_none() => Some(Sugg::hir(cx, e, "???")),
ExprKind::Path(..) if get_start(e, starts).is_none() => Some(Sugg::hir(cx, e, "???")),
_ => None,
}
}
@ -1068,18 +1047,18 @@ fn get_offset<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, starts: &[Start<'tcx>]
match idx.kind {
ExprKind::Binary(op, lhs, rhs) => match op.node {
BinOpKind::Add => {
let offset_opt = get_start(cx, lhs, starts)
let offset_opt = get_start(lhs, starts)
.and_then(|s| get_offset(cx, rhs, starts).map(|o| (s, o)))
.or_else(|| get_start(cx, rhs, starts).and_then(|s| get_offset(cx, lhs, starts).map(|o| (s, o))));
.or_else(|| get_start(rhs, starts).and_then(|s| get_offset(cx, lhs, starts).map(|o| (s, o))));
offset_opt.map(|(s, o)| (s, Offset::positive(o)))
},
BinOpKind::Sub => {
get_start(cx, lhs, starts).and_then(|s| get_offset(cx, rhs, starts).map(|o| (s, Offset::negative(o))))
get_start(lhs, starts).and_then(|s| get_offset(cx, rhs, starts).map(|o| (s, Offset::negative(o))))
},
_ => None,
},
ExprKind::Path(..) => get_start(cx, idx, starts).map(|s| (s, Offset::empty())),
ExprKind::Path(..) => get_start(idx, starts).map(|s| (s, Offset::empty())),
_ => None,
}
}
@ -1096,11 +1075,10 @@ fn get_assignment<'tcx>(e: &'tcx Expr<'tcx>) -> Option<(&'tcx Expr<'tcx>, &'tcx
/// The returned iterator yields `None` if no assignment expressions are there,
/// filtering out the increments of the given whitelisted loop counters;
/// because its job is to make sure there's nothing other than assignments and the increments.
fn get_assignments<'a: 'c, 'tcx: 'c, 'c>(
cx: &'a LateContext<'tcx>,
fn get_assignments<'a, 'tcx>(
Block { stmts, expr, .. }: &'tcx Block<'tcx>,
loop_counters: &'c [Start<'tcx>],
) -> impl Iterator<Item = Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>> + 'c {
loop_counters: &'a [Start<'tcx>],
) -> impl Iterator<Item = Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>> + 'a {
// As the `filter` and `map` below do different things, I think putting together
// just increases complexity. (cc #3188 and #4193)
stmts
@ -1112,12 +1090,14 @@ fn get_assignments<'a: 'c, 'tcx: 'c, 'c>(
.chain((*expr).into_iter())
.filter(move |e| {
if let ExprKind::AssignOp(_, place, _) = e.kind {
!loop_counters
.iter()
// skip the first item which should be `StartKind::Range`
// this makes it possible to use the slice with `StartKind::Range` in the same iterator loop.
.skip(1)
.any(|counter| same_var(cx, place, counter.id))
path_to_local(place).map_or(false, |id| {
!loop_counters
.iter()
// skip the first item which should be `StartKind::Range`
// this makes it possible to use the slice with `StartKind::Range` in the same iterator loop.
.skip(1)
.any(|counter| counter.id == id)
})
} else {
true
}
@ -1174,7 +1154,7 @@ fn print_offset(offset: MinifyingSugg<'static>) -> MinifyingSugg<'static> {
if method.ident.name == sym!(len);
if len_args.len() == 1;
if let Some(arg) = len_args.get(0);
if var_def_id(cx, arg) == var_def_id(cx, base);
if path_to_local(arg) == path_to_local(base);
then {
if sugg.as_str() == end_str {
sugg::EMPTY.into()
@ -1279,7 +1259,7 @@ fn detect_manual_memcpy<'tcx>(
if let Some(loop_counters) = get_loop_counters(cx, block, expr) {
starts.extend(loop_counters);
}
iter_a = Some(get_assignments(cx, block, &starts));
iter_a = Some(get_assignments(block, &starts));
} else {
iter_b = Some(get_assignment(body));
}
@ -1301,7 +1281,7 @@ fn detect_manual_memcpy<'tcx>(
if let Some((start_right, offset_right)) = get_details_from_idx(cx, &idx_right, &starts);
// Source and destination must be different
if var_def_id(cx, base_left) != var_def_id(cx, base_right);
if path_to_local(base_left) != path_to_local(base_right);
then {
Some((IndexExpr { base: base_left, idx: start_left, idx_offset: offset_left },
IndexExpr { base: base_right, idx: start_right, idx_offset: offset_right }))
@ -2018,9 +1998,7 @@ fn check_manual_flatten<'tcx>(
) = inner_expr.kind;
// Ensure match_expr in `if let` statement is the same as the pat from the for-loop
if let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind;
if let ExprKind::Path(QPath::Resolved(None, match_expr_path)) = match_expr.kind;
if let Res::Local(match_expr_path_id) = match_expr_path.res;
if pat_hir_id == match_expr_path_id;
if path_to_local_id(match_expr, pat_hir_id);
// Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result`
if let PatKind::TupleStruct(QPath::Resolved(None, path), _, _) = match_arms[0].pat.kind;
let some_ctor = is_some_ctor(cx, path.res);
@ -2131,20 +2109,11 @@ fn mut_warn_with_span(cx: &LateContext<'_>, span: Option<Span>) {
fn check_for_mutability(cx: &LateContext<'_>, bound: &Expr<'_>) -> Option<HirId> {
if_chain! {
if let ExprKind::Path(ref qpath) = bound.kind;
if let QPath::Resolved(None, _) = *qpath;
if let Some(hir_id) = path_to_local(bound);
if let Node::Binding(pat) = cx.tcx.hir().get(hir_id);
if let PatKind::Binding(BindingAnnotation::Mutable, ..) = pat.kind;
then {
let res = cx.qpath_res(qpath, bound.hir_id);
if let Res::Local(hir_id) = res {
let node_str = cx.tcx.hir().get(hir_id);
if_chain! {
if let Node::Binding(pat) = node_str;
if let PatKind::Binding(BindingAnnotation::Mutable, ..) = pat.kind;
then {
return Some(hir_id);
}
}
}
return Some(hir_id);
}
}
None
@ -2179,7 +2148,9 @@ fn check_for_mutation<'tcx>(
fn pat_is_wild<'tcx>(pat: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool {
match *pat {
PatKind::Wild => true,
PatKind::Binding(.., ident, None) if ident.as_str().starts_with('_') => is_unused(&ident, body),
PatKind::Binding(_, id, ident, None) if ident.as_str().starts_with('_') => {
!LocalUsedVisitor::new(id).check_expr(body)
},
_ => false,
}
}
@ -2215,7 +2186,7 @@ fn check(&mut self, idx: &'tcx Expr<'_>, seqexpr: &'tcx Expr<'_>, expr: &'tcx Ex
if let QPath::Resolved(None, ref seqvar) = *seqpath;
if seqvar.segments.len() == 1;
then {
let index_used_directly = same_var(self.cx, idx, self.var);
let index_used_directly = path_to_local_id(idx, self.var);
let indexed_indirectly = {
let mut used_visitor = LocalUsedVisitor::new(self.var);
walk_expr(&mut used_visitor, idx);
@ -2286,17 +2257,14 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if_chain! {
// directly using a variable
if let ExprKind::Path(ref qpath) = expr.kind;
if let QPath::Resolved(None, ref path) = *qpath;
if path.segments.len() == 1;
if let ExprKind::Path(QPath::Resolved(None, path)) = expr.kind;
if let Res::Local(local_id) = path.res;
then {
if let Res::Local(local_id) = self.cx.qpath_res(qpath, expr.hir_id) {
if local_id == self.var {
self.nonindex = true;
} else {
// not the correct variable, but still a variable
self.referenced.insert(path.segments[0].ident.name);
}
if local_id == self.var {
self.nonindex = true;
} else {
// not the correct variable, but still a variable
self.referenced.insert(path.segments[0].ident.name);
}
}
}
@ -2354,7 +2322,7 @@ fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
}
fn is_used_inside<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, container: &'tcx Expr<'_>) -> bool {
let def_id = match var_def_id(cx, expr) {
let def_id = match path_to_local(expr) {
Some(id) => id,
None => return false,
};
@ -2367,12 +2335,11 @@ fn is_used_inside<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, container:
}
fn is_iterator_used_after_while_let<'tcx>(cx: &LateContext<'tcx>, iter_expr: &'tcx Expr<'_>) -> bool {
let def_id = match var_def_id(cx, iter_expr) {
let def_id = match path_to_local(iter_expr) {
Some(id) => id,
None => return false,
};
let mut visitor = VarUsedAfterLoopVisitor {
cx,
def_id,
iter_expr_id: iter_expr.hir_id,
past_while_let: false,
@ -2384,20 +2351,19 @@ fn is_iterator_used_after_while_let<'tcx>(cx: &LateContext<'tcx>, iter_expr: &'t
visitor.var_used_after_while_let
}
struct VarUsedAfterLoopVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
struct VarUsedAfterLoopVisitor {
def_id: HirId,
iter_expr_id: HirId,
past_while_let: bool,
var_used_after_while_let: bool,
}
impl<'a, 'tcx> Visitor<'tcx> for VarUsedAfterLoopVisitor<'a, 'tcx> {
impl<'tcx> Visitor<'tcx> for VarUsedAfterLoopVisitor {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if self.past_while_let {
if Some(self.def_id) == var_def_id(self.cx, expr) {
if path_to_local_id(expr, self.def_id) {
self.var_used_after_while_let = true;
}
} else if self.iter_expr_id == expr.hir_id {
@ -2519,7 +2485,7 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
}
// If node is a variable
if let Some(def_id) = var_def_id(self.cx, expr) {
if let Some(def_id) = path_to_local(expr) {
if let Some(parent) = get_parent_expr(self.cx, expr) {
let state = self.states.entry(def_id).or_insert(IncrementVisitorVarState::Initial);
if *state == IncrementVisitorVarState::IncrOnce {
@ -2646,7 +2612,7 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
}
// If node is the desired variable, see how it's used
if var_def_id(self.cx, expr) == Some(self.var_id) {
if path_to_local_id(expr, self.var_id) {
if self.past_loop {
self.state = InitializeVisitorState::DontWarn;
return;
@ -2693,16 +2659,6 @@ fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
}
}
fn var_def_id(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<HirId> {
if let ExprKind::Path(ref qpath) = expr.kind {
let path_res = cx.qpath_res(qpath, expr.hir_id);
if let Res::Local(hir_id) = path_res {
return Some(hir_id);
}
}
None
}
fn is_loop(expr: &Expr<'_>) -> bool {
matches!(expr.kind, ExprKind::Loop(..))
}
@ -2725,8 +2681,8 @@ fn is_nested(cx: &LateContext<'_>, match_expr: &Expr<'_>, iter_expr: &Expr<'_>)
fn is_loop_nested(cx: &LateContext<'_>, loop_expr: &Expr<'_>, iter_expr: &Expr<'_>) -> bool {
let mut id = loop_expr.hir_id;
let iter_name = if let Some(name) = path_name(iter_expr) {
name
let iter_id = if let Some(id) = path_to_local(iter_expr) {
id
} else {
return true;
};
@ -2744,7 +2700,7 @@ fn is_loop_nested(cx: &LateContext<'_>, loop_expr: &Expr<'_>, iter_expr: &Expr<'
Some(Node::Block(block)) => {
let mut block_visitor = LoopNestVisitor {
hir_id: id,
iterator: iter_name,
iterator: iter_id,
nesting: Unknown,
};
walk_block(&mut block_visitor, block);
@ -2772,7 +2728,7 @@ enum Nesting {
struct LoopNestVisitor {
hir_id: HirId,
iterator: Symbol,
iterator: HirId,
nesting: Nesting,
}
@ -2797,7 +2753,7 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
}
match expr.kind {
ExprKind::Assign(ref path, _, _) | ExprKind::AssignOp(_, ref path, _) => {
if match_var(path, self.iterator) {
if path_to_local_id(path, self.iterator) {
self.nesting = RuledOut;
}
},
@ -2809,8 +2765,8 @@ fn visit_pat(&mut self, pat: &'tcx Pat<'_>) {
if self.nesting != Unknown {
return;
}
if let PatKind::Binding(.., span_name, _) = pat.kind {
if self.iterator == span_name.name {
if let PatKind::Binding(_, id, ..) = pat.kind {
if id == self.iterator {
self.nesting = RuledOut;
return;
}
@ -2823,16 +2779,6 @@ fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
}
}
fn path_name(e: &Expr<'_>) -> Option<Symbol> {
if let ExprKind::Path(QPath::Resolved(_, ref path)) = e.kind {
let segments = &path.segments;
if segments.len() == 1 {
return Some(segments[0].ident.name);
}
};
None
}
fn check_infinite_loop<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) {
if constant(cx, cx.typeck_results(), cond).is_some() {
// A pure constant condition (e.g., `while false`) is not linted.
@ -3194,7 +3140,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UsedCountVisitor<'a, 'tcx> {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if same_var(self.cx, expr, self.id) {
if path_to_local_id(expr, self.id) {
self.count += 1;
} else {
walk_expr(self, expr);

View File

@ -1,9 +1,10 @@
use crate::utils::{
indent_of, is_type_diagnostic_item, match_qpath, paths, reindent_multiline, snippet_opt, span_lint_and_sugg,
indent_of, is_type_diagnostic_item, match_qpath, path_to_local_id, paths, reindent_multiline, snippet_opt,
span_lint_and_sugg,
};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{def, Expr, ExprKind, PatKind, QPath};
use rustc_hir::{Expr, ExprKind, PatKind};
use rustc_lint::LintContext;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::lint::in_external_macro;
@ -90,8 +91,6 @@ fn is_ok_wrapping(cx: &LateContext<'_>, map_expr: &Expr<'_>) -> bool {
if let PatKind::Binding(_, param_id, ..) = body.params[0].pat.kind;
if let ExprKind::Call(Expr { kind: ExprKind::Path(ok_path), .. }, &[ref ok_arg]) = body.value.kind;
if match_qpath(ok_path, &paths::RESULT_OK);
if let ExprKind::Path(QPath::Resolved(_, ok_arg_path)) = ok_arg.kind;
if let def::Res::Local(ok_arg_path_id) = ok_arg_path.res;
then { param_id == ok_arg_path_id } else { false }
then { path_to_local_id(ok_arg, param_id) } else { false }
}
}

View File

@ -1,9 +1,9 @@
use crate::consts::constant_simple;
use crate::utils;
use crate::utils::sugg;
use crate::utils::{path_to_local_id, sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{def, Arm, Expr, ExprKind, Pat, PatKind, QPath};
use rustc_hir::{Arm, Expr, ExprKind, Pat, PatKind};
use rustc_lint::LintContext;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::lint::in_external_macro;
@ -83,9 +83,7 @@ fn applicable_or_arm<'a>(arms: &'a [Arm<'a>]) -> Option<&'a Arm<'a>> {
if utils::match_qpath(unwrap_qpath, &utils::paths::OPTION_SOME)
|| utils::match_qpath(unwrap_qpath, &utils::paths::RESULT_OK);
if let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind;
if let ExprKind::Path(QPath::Resolved(_, body_path)) = unwrap_arm.body.kind;
if let def::Res::Local(body_path_hir_id) = body_path.res;
if body_path_hir_id == binding_hir_id;
if path_to_local_id(unwrap_arm.body, binding_hir_id);
if !utils::usage::contains_return_break_continue_macro(or_arm.body);
then {
Some(or_arm)

View File

@ -1,11 +1,12 @@
use crate::consts::{constant, miri_to_const, Constant};
use crate::utils::sugg::Sugg;
use crate::utils::usage::is_unused;
use crate::utils::visitors::LocalUsedVisitor;
use crate::utils::{
expr_block, get_arg_name, get_parent_expr, implements_trait, in_macro, indent_of, is_allowed, is_expn_of,
is_refutable, is_type_diagnostic_item, is_wild, match_qpath, match_type, match_var, meets_msrv, multispan_sugg,
expr_block, get_parent_expr, implements_trait, in_macro, indent_of, is_allowed, is_expn_of, is_refutable,
is_type_diagnostic_item, is_wild, match_qpath, match_type, meets_msrv, multispan_sugg, path_to_local_id,
peel_hir_pat_refs, peel_mid_ty_refs, peel_n_hir_expr_refs, remove_blocks, snippet, snippet_block, snippet_opt,
snippet_with_applicability, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then,
strip_pat_refs,
};
use crate::utils::{paths, search_same, SpanlessEq, SpanlessHash};
use if_chain::if_chain;
@ -616,9 +617,9 @@ fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'_>) {
if let PatKind::TupleStruct(
QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pat.kind;
if args.len() == 1;
if let Some(arg) = get_arg_name(&args[0]);
if let PatKind::Binding(_, arg, ..) = strip_pat_refs(&args[0]).kind;
let body = remove_blocks(&arms[0].body);
if match_var(body, arg);
if path_to_local_id(body, arg);
then {
let mut applicability = Applicability::MachineApplicable;
@ -922,8 +923,8 @@ fn check_wild_err_arm(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
if !matching_wild {
// Looking for unused bindings (i.e.: `_e`)
inner.iter().for_each(|pat| {
if let PatKind::Binding(.., ident, None) = &pat.kind {
if ident.as_str().starts_with('_') && is_unused(ident, arm.body) {
if let PatKind::Binding(_, id, ident, None) = pat.kind {
if ident.as_str().starts_with('_') && !LocalUsedVisitor::new(id).check_expr(arm.body) {
ident_bind_name = (&ident.name.as_str()).to_string();
matching_wild = true;
}

View File

@ -15,8 +15,7 @@
use rustc_ast::ast;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_hir::{Expr, ExprKind, PatKind, QPath, TraitItem, TraitItemKind, UnOp};
use rustc_hir::{Expr, ExprKind, PatKind, TraitItem, TraitItemKind, UnOp};
use rustc_lint::{LateContext, LateLintPass, Lint, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::{self, TraitRef, Ty, TyS};
@ -30,12 +29,12 @@
use crate::utils::eager_or_lazy::is_lazyness_candidate;
use crate::utils::usage::mutated_variables;
use crate::utils::{
contains_return, contains_ty, get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, higher,
implements_trait, in_macro, is_copy, is_expn_of, is_type_diagnostic_item, iter_input_pats, last_path_segment,
match_def_path, match_qpath, match_trait_method, match_type, match_var, meets_msrv, method_calls,
method_chain_args, paths, remove_blocks, return_ty, single_segment_path, snippet, snippet_with_applicability,
snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg,
walk_ptrs_ty_depth, SpanlessEq,
contains_return, contains_ty, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
in_macro, is_copy, is_expn_of, is_type_diagnostic_item, iter_input_pats, last_path_segment, match_def_path,
match_qpath, match_trait_method, match_type, meets_msrv, method_calls, method_chain_args, path_to_local_id, paths,
remove_blocks, return_ty, single_segment_path, snippet, snippet_with_applicability, snippet_with_macro_callsite,
span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, strip_pat_refs, sugg, walk_ptrs_ty_depth,
SpanlessEq,
};
declare_clippy_lint! {
@ -2396,11 +2395,12 @@ fn check_fold_with_op(
if bin_op.node == op;
// Extract the names of the two arguments to the closure
if let Some(first_arg_ident) = get_arg_name(&closure_body.params[0].pat);
if let Some(second_arg_ident) = get_arg_name(&closure_body.params[1].pat);
if let [param_a, param_b] = closure_body.params;
if let PatKind::Binding(_, first_arg_id, ..) = strip_pat_refs(&param_a.pat).kind;
if let PatKind::Binding(_, second_arg_id, second_arg_ident, _) = strip_pat_refs(&param_b.pat).kind;
if match_var(&*left_expr, first_arg_ident);
if replacement_has_args || match_var(&*right_expr, second_arg_ident);
if path_to_local_id(left_expr, first_arg_id);
if replacement_has_args || path_to_local_id(right_expr, second_arg_id);
then {
let mut applicability = Applicability::MachineApplicable;
@ -3068,10 +3068,8 @@ fn lint_filter_map<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, is_f
};
// let the filter closure arg and the map closure arg be equal
if_chain! {
if let ExprKind::Path(QPath::Resolved(None, a_path)) = a_path.kind;
if let ExprKind::Path(QPath::Resolved(None, b_path)) = b.kind;
if a_path.res == Res::Local(filter_param_id);
if b_path.res == Res::Local(map_param_id);
if path_to_local_id(a_path, filter_param_id);
if path_to_local_id(b, map_param_id);
if TyS::same_type(cx.typeck_results().expr_ty_adjusted(a), cx.typeck_results().expr_ty_adjusted(b));
then {
return true;
@ -3255,8 +3253,9 @@ fn lint_search_is_some<'tcx>(
then {
if let hir::PatKind::Ref(..) = closure_arg.pat.kind {
Some(search_snippet.replacen('&', "", 1))
} else if let Some(name) = get_arg_name(&closure_arg.pat) {
Some(search_snippet.replace(&format!("*{}", name), &name.as_str()))
} else if let PatKind::Binding(_, _, ident, _) = strip_pat_refs(&closure_arg.pat).kind {
let name = &*ident.name.as_str();
Some(search_snippet.replace(&format!("*{}", name), name))
} else {
None
}
@ -3688,9 +3687,7 @@ fn lint_option_as_ref_deref<'tcx>(
hir::ExprKind::MethodCall(_, _, args, _) => {
if_chain! {
if args.len() == 1;
if let hir::ExprKind::Path(qpath) = &args[0].kind;
if let hir::def::Res::Local(local_id) = cx.qpath_res(qpath, args[0].hir_id);
if closure_body.params[0].pat.hir_id == local_id;
if path_to_local_id(&args[0], closure_body.params[0].pat.hir_id);
let adj = cx
.typeck_results()
.expr_adjustments(&args[0])
@ -3710,10 +3707,8 @@ fn lint_option_as_ref_deref<'tcx>(
if_chain! {
if let hir::ExprKind::Unary(hir::UnOp::UnDeref, ref inner1) = inner.kind;
if let hir::ExprKind::Unary(hir::UnOp::UnDeref, ref inner2) = inner1.kind;
if let hir::ExprKind::Path(ref qpath) = inner2.kind;
if let hir::def::Res::Local(local_id) = cx.qpath_res(qpath, inner2.hir_id);
then {
closure_body.params[0].pat.hir_id == local_id
path_to_local_id(inner2, closure_body.params[0].pat.hir_id)
} else {
false
}

View File

@ -1,8 +1,6 @@
use crate::utils::paths;
use crate::utils::usage::mutated_variables;
use crate::utils::{match_qpath, match_trait_method, span_lint};
use crate::utils::{match_qpath, match_trait_method, path_to_local_id, paths, span_lint};
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
@ -59,14 +57,8 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc
if let hir::ExprKind::Path(ref path) = func.kind;
then {
if match_qpath(path, &paths::OPTION_SOME) {
if_chain! {
if let hir::ExprKind::Path(path) = &args[0].kind;
if let Res::Local(ref local) = cx.qpath_res(path, args[0].hir_id);
then {
if arg_id == *local {
return (false, false)
}
}
if path_to_local_id(&args[0], arg_id) {
return (false, false)
}
return (true, false);
}

View File

@ -1,6 +1,5 @@
use crate::utils::{match_def_path, match_trait_method, paths, span_lint};
use crate::utils::{match_def_path, match_trait_method, path_to_local_id, paths, span_lint};
use if_chain::if_chain;
use rustc_hir::def::Res;
use rustc_hir::{Expr, ExprKind, HirId, Impl, ImplItem, ImplItemKind, Item, ItemKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
@ -89,14 +88,12 @@ fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
if_chain! {
if self.in_display_impl;
if let Some(self_hir_id) = self.self_hir_id;
if let ExprKind::MethodCall(ref path, _, args, _) = expr.kind;
if path.ident.name == sym!(to_string);
if match_trait_method(cx, expr, &paths::TO_STRING);
if self.in_display_impl;
if let ExprKind::Path(ref qpath) = args[0].kind;
if let Res::Local(hir_id) = cx.qpath_res(qpath, args[0].hir_id);
if let Some(self_hir_id) = self.self_hir_id;
if hir_id == self_hir_id;
if path_to_local_id(&args[0], self_hir_id);
then {
span_lint(
cx,

View File

@ -307,6 +307,22 @@ pub fn match_path_ast(path: &ast::Path, segments: &[&str]) -> bool {
.all(|(a, b)| a.ident.name.as_str() == *b)
}
/// If the expression is a path to a local, returns the canonical `HirId` of the local.
pub fn path_to_local(expr: &Expr<'_>) -> Option<HirId> {
if let ExprKind::Path(QPath::Resolved(None, ref path)) = expr.kind {
if let Res::Local(id) = path.res {
return Some(id);
}
}
None
}
/// Returns true if the expression is a path to a local with the specified `HirId`.
/// Use this function to see if an expression matches a function argument or a match binding.
pub fn path_to_local_id(expr: &Expr<'_>, id: HirId) -> bool {
path_to_local(expr) == Some(id)
}
/// Gets the definition associated to a path.
#[allow(clippy::shadow_unrelated)] // false positive #6563
pub fn path_to_res(cx: &LateContext<'_>, path: &[&str]) -> Res {
@ -1135,9 +1151,7 @@ fn is_ok(arm: &Arm<'_>) -> bool {
if let PatKind::TupleStruct(ref path, ref pat, None) = arm.pat.kind;
if match_qpath(path, &paths::RESULT_OK[1..]);
if let PatKind::Binding(_, hir_id, _, None) = pat[0].kind;
if let ExprKind::Path(QPath::Resolved(None, ref path)) = arm.body.kind;
if let Res::Local(lid) = path.res;
if lid == hir_id;
if path_to_local_id(arm.body, hir_id);
then {
return true;
}
@ -1181,12 +1195,11 @@ pub fn is_allowed(cx: &LateContext<'_>, lint: &'static Lint, id: HirId) -> bool
cx.tcx.lint_level_at_node(lint, id).0 == Level::Allow
}
pub fn get_arg_name(pat: &Pat<'_>) -> Option<Symbol> {
match pat.kind {
PatKind::Binding(.., ident, None) => Some(ident.name),
PatKind::Ref(ref subpat, _) => get_arg_name(subpat),
_ => None,
pub fn strip_pat_refs<'hir>(mut pat: &'hir Pat<'hir>) -> &'hir Pat<'hir> {
while let PatKind::Ref(subpat, _) = pat.kind {
pat = subpat;
}
pat
}
pub fn int_bits(tcx: TyCtxt<'_>, ity: ty::IntTy) -> u64 {

View File

@ -1,16 +1,14 @@
use crate::utils;
use crate::utils::match_var;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_hir::intravisit;
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
use rustc_hir::{Expr, ExprKind, HirId, Path};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
use rustc_middle::ty;
use rustc_span::symbol::{Ident, Symbol};
use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
/// Returns a set of mutated local variable IDs, or `None` if mutations could not be determined.
@ -81,36 +79,6 @@ fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId) {
}
}
pub struct UsedVisitor {
pub var: Symbol, // var to look for
pub used: bool, // has the var been used otherwise?
}
impl<'tcx> Visitor<'tcx> for UsedVisitor {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if match_var(expr, self.var) {
self.used = true;
} else {
walk_expr(self, expr);
}
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
}
pub fn is_unused<'tcx>(ident: &'tcx Ident, body: &'tcx Expr<'_>) -> bool {
let mut visitor = UsedVisitor {
var: ident.name,
used: false,
};
walk_expr(&mut visitor, body);
!visitor.used
}
pub struct ParamBindingIdCollector {
binding_hir_ids: Vec<hir::HirId>,
}

View File

@ -1,7 +1,7 @@
use crate::utils::path_to_local_id;
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_hir::intravisit::{self, walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{Arm, Expr, ExprKind, HirId, QPath, Stmt};
use rustc_hir::{Arm, Expr, HirId, Stmt};
use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
@ -168,15 +168,11 @@ impl<'v> Visitor<'v> for LocalUsedVisitor {
type Map = Map<'v>;
fn visit_expr(&mut self, expr: &'v Expr<'v>) {
if let ExprKind::Path(QPath::Resolved(None, path)) = expr.kind {
if let Res::Local(id) = path.res {
if id == self.local_hir_id {
self.used = true;
return;
}
}
if path_to_local_id(expr, self.local_hir_id) {
self.used = true;
} else {
walk_expr(self, expr);
}
walk_expr(self, expr);
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {