Use for_each_expr in place of some visitors

This commit is contained in:
Jason Newcomb 2022-04-29 00:09:55 -04:00
parent 649d443646
commit 38236a7135
9 changed files with 226 additions and 349 deletions

View file

@ -3,10 +3,11 @@ use clippy_utils::get_parent_expr;
use clippy_utils::higher; use clippy_utils::higher;
use clippy_utils::source::snippet_block_with_applicability; use clippy_utils::source::snippet_block_with_applicability;
use clippy_utils::ty::implements_trait; use clippy_utils::ty::implements_trait;
use clippy_utils::visitors::{for_each_expr, Descend};
use core::ops::ControlFlow;
use if_chain::if_chain; use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_hir::{BlockCheckMode, Expr, ExprKind};
use rustc_hir::{BlockCheckMode, Closure, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro; use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
@ -44,39 +45,6 @@ declare_clippy_lint! {
declare_lint_pass!(BlocksInIfConditions => [BLOCKS_IN_IF_CONDITIONS]); declare_lint_pass!(BlocksInIfConditions => [BLOCKS_IN_IF_CONDITIONS]);
struct ExVisitor<'a, 'tcx> {
found_block: Option<&'tcx Expr<'tcx>>,
cx: &'a LateContext<'tcx>,
}
impl<'a, 'tcx> Visitor<'tcx> for ExVisitor<'a, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
if let ExprKind::Closure(&Closure { body, .. }) = expr.kind {
// do not lint if the closure is called using an iterator (see #1141)
if_chain! {
if let Some(parent) = get_parent_expr(self.cx, expr);
if let ExprKind::MethodCall(_, self_arg, ..) = &parent.kind;
let caller = self.cx.typeck_results().expr_ty(self_arg);
if let Some(iter_id) = self.cx.tcx.get_diagnostic_item(sym::Iterator);
if implements_trait(self.cx, caller, iter_id, &[]);
then {
return;
}
}
let body = self.cx.tcx.hir().body(body);
let ex = &body.value;
if let ExprKind::Block(block, _) = ex.kind {
if !body.value.span.from_expansion() && !block.stmts.is_empty() {
self.found_block = Some(ex);
return;
}
}
}
walk_expr(self, expr);
}
}
const BRACED_EXPR_MESSAGE: &str = "omit braces around single expression condition"; const BRACED_EXPR_MESSAGE: &str = "omit braces around single expression condition";
const COMPLEX_BLOCK_MESSAGE: &str = "in an `if` condition, avoid complex blocks or closures with blocks; \ const COMPLEX_BLOCK_MESSAGE: &str = "in an `if` condition, avoid complex blocks or closures with blocks; \
instead, move the block or closure higher and bind it with a `let`"; instead, move the block or closure higher and bind it with a `let`";
@ -144,11 +112,31 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInIfConditions {
} }
} }
} else { } else {
let mut visitor = ExVisitor { found_block: None, cx }; let _: Option<!> = for_each_expr(cond, |e| {
walk_expr(&mut visitor, cond); if let ExprKind::Closure(closure) = e.kind {
if let Some(block) = visitor.found_block { // do not lint if the closure is called using an iterator (see #1141)
span_lint(cx, BLOCKS_IN_IF_CONDITIONS, block.span, COMPLEX_BLOCK_MESSAGE); if_chain! {
} if let Some(parent) = get_parent_expr(cx, e);
if let ExprKind::MethodCall(_, self_arg, _, _) = &parent.kind;
let caller = cx.typeck_results().expr_ty(self_arg);
if let Some(iter_id) = cx.tcx.get_diagnostic_item(sym::Iterator);
if implements_trait(cx, caller, iter_id, &[]);
then {
return ControlFlow::Continue(Descend::No);
}
}
let body = cx.tcx.hir().body(closure.body);
let ex = &body.value;
if let ExprKind::Block(block, _) = ex.kind {
if !body.value.span.from_expansion() && !block.stmts.is_empty() {
span_lint(cx, BLOCKS_IN_IF_CONDITIONS, ex.span, COMPLEX_BLOCK_MESSAGE);
return ControlFlow::Continue(Descend::No);
}
}
}
ControlFlow::Continue(Descend::Yes)
});
} }
} }
} }

View file

@ -3,10 +3,12 @@
use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::source::snippet_opt; use clippy_utils::source::snippet_opt;
use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::visitors::for_each_expr;
use clippy_utils::LimitStack; use clippy_utils::LimitStack;
use core::ops::ControlFlow;
use rustc_ast::ast::Attribute; use rustc_ast::ast::Attribute;
use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; use rustc_hir::intravisit::FnKind;
use rustc_hir::{Body, Expr, ExprKind, FnDecl, HirId}; use rustc_hir::{Body, ExprKind, FnDecl, HirId};
use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::Span; use rustc_span::source_map::Span;
@ -61,11 +63,27 @@ impl CognitiveComplexity {
return; return;
} }
let expr = &body.value; let expr = body.value;
let mut cc = 1u64;
let mut returns = 0u64;
let _: Option<!> = for_each_expr(expr, |e| {
match e.kind {
ExprKind::If(_, _, _) => {
cc += 1;
},
ExprKind::Match(_, arms, _) => {
if arms.len() > 1 {
cc += 1;
}
cc += arms.iter().filter(|arm| arm.guard.is_some()).count() as u64;
},
ExprKind::Ret(_) => returns += 1,
_ => {},
}
ControlFlow::Continue(())
});
let mut helper = CcHelper { cc: 1, returns: 0 };
helper.visit_expr(expr);
let CcHelper { cc, returns } = helper;
let ret_ty = cx.typeck_results().node_type(expr.hir_id); let ret_ty = cx.typeck_results().node_type(expr.hir_id);
let ret_adjust = if is_type_diagnostic_item(cx, ret_ty, sym::Result) { let ret_adjust = if is_type_diagnostic_item(cx, ret_ty, sym::Result) {
returns returns
@ -74,13 +92,12 @@ impl CognitiveComplexity {
(returns / 2) (returns / 2)
}; };
let mut rust_cc = cc;
// prevent degenerate cases where unreachable code contains `return` statements // prevent degenerate cases where unreachable code contains `return` statements
if rust_cc >= ret_adjust { if cc >= ret_adjust {
rust_cc -= ret_adjust; cc -= ret_adjust;
} }
if rust_cc > self.limit.limit() { if cc > self.limit.limit() {
let fn_span = match kind { let fn_span = match kind {
FnKind::ItemFn(ident, _, _) | FnKind::Method(ident, _) => ident.span, FnKind::ItemFn(ident, _, _) | FnKind::Method(ident, _) => ident.span,
FnKind::Closure => { FnKind::Closure => {
@ -107,7 +124,7 @@ impl CognitiveComplexity {
COGNITIVE_COMPLEXITY, COGNITIVE_COMPLEXITY,
fn_span, fn_span,
&format!( &format!(
"the function has a cognitive complexity of ({rust_cc}/{})", "the function has a cognitive complexity of ({cc}/{})",
self.limit.limit() self.limit.limit()
), ),
None, None,
@ -140,27 +157,3 @@ impl<'tcx> LateLintPass<'tcx> for CognitiveComplexity {
self.limit.pop_attrs(cx.sess(), attrs, "cognitive_complexity"); self.limit.pop_attrs(cx.sess(), attrs, "cognitive_complexity");
} }
} }
struct CcHelper {
cc: u64,
returns: u64,
}
impl<'tcx> Visitor<'tcx> for CcHelper {
fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
walk_expr(self, e);
match e.kind {
ExprKind::If(_, _, _) => {
self.cc += 1;
},
ExprKind::Match(_, arms, _) => {
if arms.len() > 1 {
self.cc += 1;
}
self.cc += arms.iter().filter(|arm| arm.guard.is_some()).count() as u64;
},
ExprKind::Ret(_) => self.returns += 1,
_ => {},
}
}
}

View file

@ -1,7 +1,7 @@
use rustc_ast::ast::Attribute; use rustc_ast::ast::Attribute;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::def_id::{DefIdSet, LocalDefId}; use rustc_hir::def_id::{DefIdSet, LocalDefId};
use rustc_hir::{self as hir, def::Res, intravisit, QPath}; use rustc_hir::{self as hir, def::Res, QPath};
use rustc_lint::{LateContext, LintContext}; use rustc_lint::{LateContext, LintContext};
use rustc_middle::{ use rustc_middle::{
lint::in_external_macro, lint::in_external_macro,
@ -13,8 +13,11 @@ use clippy_utils::attrs::is_proc_macro;
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then}; use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then};
use clippy_utils::source::snippet_opt; use clippy_utils::source::snippet_opt;
use clippy_utils::ty::is_must_use_ty; use clippy_utils::ty::is_must_use_ty;
use clippy_utils::visitors::for_each_expr;
use clippy_utils::{match_def_path, return_ty, trait_ref_of_method}; use clippy_utils::{match_def_path, return_ty, trait_ref_of_method};
use core::ops::ControlFlow;
use super::{DOUBLE_MUST_USE, MUST_USE_CANDIDATE, MUST_USE_UNIT}; use super::{DOUBLE_MUST_USE, MUST_USE_CANDIDATE, MUST_USE_UNIT};
pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) { pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
@ -200,63 +203,6 @@ fn is_mutable_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, span: Span, tys: &m
} }
} }
struct StaticMutVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
mutates_static: bool,
}
impl<'a, 'tcx> intravisit::Visitor<'tcx> for StaticMutVisitor<'a, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
use hir::ExprKind::{AddrOf, Assign, AssignOp, Call, MethodCall};
if self.mutates_static {
return;
}
match expr.kind {
Call(_, args) => {
let mut tys = DefIdSet::default();
for arg in args {
if self.cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id())
&& is_mutable_ty(
self.cx,
self.cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg),
arg.span,
&mut tys,
)
&& is_mutated_static(arg)
{
self.mutates_static = true;
return;
}
tys.clear();
}
},
MethodCall(_, receiver, args, _) => {
let mut tys = DefIdSet::default();
for arg in std::iter::once(receiver).chain(args.iter()) {
if self.cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id())
&& is_mutable_ty(
self.cx,
self.cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg),
arg.span,
&mut tys,
)
&& is_mutated_static(arg)
{
self.mutates_static = true;
return;
}
tys.clear();
}
},
Assign(target, ..) | AssignOp(_, target, _) | AddrOf(_, hir::Mutability::Mut, target) => {
self.mutates_static |= is_mutated_static(target);
},
_ => {},
}
}
}
fn is_mutated_static(e: &hir::Expr<'_>) -> bool { fn is_mutated_static(e: &hir::Expr<'_>) -> bool {
use hir::ExprKind::{Field, Index, Path}; use hir::ExprKind::{Field, Index, Path};
@ -269,10 +215,53 @@ fn is_mutated_static(e: &hir::Expr<'_>) -> bool {
} }
fn mutates_static<'tcx>(cx: &LateContext<'tcx>, body: &'tcx hir::Body<'_>) -> bool { fn mutates_static<'tcx>(cx: &LateContext<'tcx>, body: &'tcx hir::Body<'_>) -> bool {
let mut v = StaticMutVisitor { for_each_expr(body.value, |e| {
cx, use hir::ExprKind::{AddrOf, Assign, AssignOp, Call, MethodCall};
mutates_static: false,
}; match e.kind {
intravisit::walk_expr(&mut v, body.value); Call(_, args) => {
v.mutates_static let mut tys = DefIdSet::default();
for arg in args {
if cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id())
&& is_mutable_ty(
cx,
cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg),
arg.span,
&mut tys,
)
&& is_mutated_static(arg)
{
return ControlFlow::Break(());
}
tys.clear();
}
ControlFlow::Continue(())
},
MethodCall(_, receiver, args, _) => {
let mut tys = DefIdSet::default();
for arg in std::iter::once(receiver).chain(args.iter()) {
if cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id())
&& is_mutable_ty(
cx,
cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg),
arg.span,
&mut tys,
)
&& is_mutated_static(arg)
{
return ControlFlow::Break(());
}
tys.clear();
}
ControlFlow::Continue(())
},
Assign(target, ..) | AssignOp(_, target, _) | AddrOf(_, hir::Mutability::Mut, target)
if is_mutated_static(target) =>
{
ControlFlow::Break(())
},
_ => ControlFlow::Continue(()),
}
})
.is_some()
} }

View file

@ -5,8 +5,11 @@ use rustc_span::def_id::LocalDefId;
use clippy_utils::diagnostics::span_lint; use clippy_utils::diagnostics::span_lint;
use clippy_utils::ty::type_is_unsafe_function; use clippy_utils::ty::type_is_unsafe_function;
use clippy_utils::visitors::for_each_expr_with_closures;
use clippy_utils::{iter_input_pats, path_to_local}; use clippy_utils::{iter_input_pats, path_to_local};
use core::ops::ControlFlow;
use super::NOT_UNSAFE_PTR_ARG_DEREF; use super::NOT_UNSAFE_PTR_ARG_DEREF;
pub(super) fn check_fn<'tcx>( pub(super) fn check_fn<'tcx>(
@ -39,21 +42,34 @@ fn check_raw_ptr<'tcx>(
body: &'tcx hir::Body<'tcx>, body: &'tcx hir::Body<'tcx>,
def_id: LocalDefId, def_id: LocalDefId,
) { ) {
let expr = &body.value;
if unsafety == hir::Unsafety::Normal && cx.access_levels.is_exported(def_id) { if unsafety == hir::Unsafety::Normal && cx.access_levels.is_exported(def_id) {
let raw_ptrs = iter_input_pats(decl, body) let raw_ptrs = iter_input_pats(decl, body)
.filter_map(|arg| raw_ptr_arg(cx, arg)) .filter_map(|arg| raw_ptr_arg(cx, arg))
.collect::<HirIdSet>(); .collect::<HirIdSet>();
if !raw_ptrs.is_empty() { if !raw_ptrs.is_empty() {
let typeck_results = cx.tcx.typeck_body(body.id()); let typeck = cx.tcx.typeck_body(body.id());
let mut v = DerefVisitor { let _: Option<!> = for_each_expr_with_closures(cx, body.value, |e| {
cx, match e.kind {
ptrs: raw_ptrs, hir::ExprKind::Call(f, args) if type_is_unsafe_function(cx, typeck.expr_ty(f)) => {
typeck_results, for arg in args {
}; check_arg(cx, &raw_ptrs, arg);
}
intravisit::walk_expr(&mut v, expr); },
hir::ExprKind::MethodCall(_, recv, args, _) => {
let def_id = typeck.type_dependent_def_id(e.hir_id).unwrap();
if cx.tcx.fn_sig(def_id).skip_binder().unsafety == hir::Unsafety::Unsafe {
check_arg(cx, &raw_ptrs, recv);
for arg in args {
check_arg(cx, &raw_ptrs, arg);
}
}
},
hir::ExprKind::Unary(hir::UnOp::Deref, ptr) => check_arg(cx, &raw_ptrs, ptr),
_ => (),
}
ControlFlow::Continue(())
});
} }
} }
} }
@ -70,54 +86,13 @@ fn raw_ptr_arg(cx: &LateContext<'_>, arg: &hir::Param<'_>) -> Option<hir::HirId>
} }
} }
struct DerefVisitor<'a, 'tcx> { fn check_arg(cx: &LateContext<'_>, raw_ptrs: &HirIdSet, arg: &hir::Expr<'_>) {
cx: &'a LateContext<'tcx>, if path_to_local(arg).map_or(false, |id| raw_ptrs.contains(&id)) {
ptrs: HirIdSet, span_lint(
typeck_results: &'a ty::TypeckResults<'tcx>, cx,
} NOT_UNSAFE_PTR_ARG_DEREF,
arg.span,
impl<'a, 'tcx> intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> { "this public function might dereference a raw pointer but is not marked `unsafe`",
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) { );
match expr.kind {
hir::ExprKind::Call(f, args) => {
let ty = self.typeck_results.expr_ty(f);
if type_is_unsafe_function(self.cx, ty) {
for arg in args {
self.check_arg(arg);
}
}
},
hir::ExprKind::MethodCall(_, receiver, args, _) => {
let def_id = self.typeck_results.type_dependent_def_id(expr.hir_id).unwrap();
let base_type = self.cx.tcx.type_of(def_id);
if type_is_unsafe_function(self.cx, base_type) {
self.check_arg(receiver);
for arg in args {
self.check_arg(arg);
}
}
},
hir::ExprKind::Unary(hir::UnOp::Deref, ptr) => self.check_arg(ptr),
_ => (),
}
intravisit::walk_expr(self, expr);
}
}
impl<'a, 'tcx> DerefVisitor<'a, 'tcx> {
fn check_arg(&self, ptr: &hir::Expr<'_>) {
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 might dereference a raw pointer but is not marked `unsafe`",
);
}
}
} }
} }

View file

@ -2,9 +2,10 @@ use super::utils::clone_or_copy_needed;
use clippy_utils::diagnostics::span_lint; use clippy_utils::diagnostics::span_lint;
use clippy_utils::ty::is_copy; use clippy_utils::ty::is_copy;
use clippy_utils::usage::mutated_variables; use clippy_utils::usage::mutated_variables;
use clippy_utils::visitors::{for_each_expr, Descend};
use clippy_utils::{is_res_lang_ctor, is_trait_method, path_res, path_to_local_id}; use clippy_utils::{is_res_lang_ctor, is_trait_method, path_res, path_to_local_id};
use core::ops::ControlFlow;
use rustc_hir as hir; use rustc_hir as hir;
use rustc_hir::intravisit::{walk_expr, Visitor};
use rustc_hir::LangItem::{OptionNone, OptionSome}; use rustc_hir::LangItem::{OptionNone, OptionSome};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::ty; use rustc_middle::ty;
@ -13,7 +14,7 @@ use rustc_span::sym;
use super::UNNECESSARY_FILTER_MAP; use super::UNNECESSARY_FILTER_MAP;
use super::UNNECESSARY_FIND_MAP; use super::UNNECESSARY_FIND_MAP;
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>, name: &str) { pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, arg: &'tcx hir::Expr<'tcx>, name: &str) {
if !is_trait_method(cx, expr, sym::Iterator) { if !is_trait_method(cx, expr, sym::Iterator) {
return; return;
} }
@ -26,10 +27,16 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<
let (mut found_mapping, mut found_filtering) = check_expression(cx, arg_id, body.value); let (mut found_mapping, mut found_filtering) = check_expression(cx, arg_id, body.value);
let mut return_visitor = ReturnVisitor::new(cx, arg_id); let _: Option<!> = for_each_expr(body.value, |e| {
return_visitor.visit_expr(body.value); if let hir::ExprKind::Ret(Some(e)) = &e.kind {
found_mapping |= return_visitor.found_mapping; let (found_mapping_res, found_filtering_res) = check_expression(cx, arg_id, e);
found_filtering |= return_visitor.found_filtering; found_mapping |= found_mapping_res;
found_filtering |= found_filtering_res;
ControlFlow::Continue(Descend::No)
} else {
ControlFlow::Continue(Descend::Yes)
}
});
let in_ty = cx.typeck_results().node_type(body.params[0].hir_id); let in_ty = cx.typeck_results().node_type(body.params[0].hir_id);
let sugg = if !found_filtering { let sugg = if !found_filtering {
@ -97,35 +104,3 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc
_ => (true, true), _ => (true, true),
} }
} }
struct ReturnVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
arg_id: hir::HirId,
// Found a non-None return that isn't Some(input)
found_mapping: bool,
// Found a return that isn't Some
found_filtering: bool,
}
impl<'a, 'tcx> ReturnVisitor<'a, 'tcx> {
fn new(cx: &'a LateContext<'tcx>, arg_id: hir::HirId) -> ReturnVisitor<'a, 'tcx> {
ReturnVisitor {
cx,
arg_id,
found_mapping: false,
found_filtering: false,
}
}
}
impl<'a, 'tcx> Visitor<'tcx> for ReturnVisitor<'a, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
if let hir::ExprKind::Ret(Some(expr)) = &expr.kind {
let (found_mapping, found_filtering) = check_expression(self.cx, self.arg_id, expr);
self.found_mapping |= found_mapping;
self.found_filtering |= found_filtering;
} else {
walk_expr(self, expr);
}
}
}

View file

@ -2,11 +2,12 @@ use clippy_utils::binop_traits;
use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet_opt; use clippy_utils::source::snippet_opt;
use clippy_utils::ty::implements_trait; use clippy_utils::ty::implements_trait;
use clippy_utils::visitors::for_each_expr;
use clippy_utils::{eq_expr_value, trait_ref_of_method}; use clippy_utils::{eq_expr_value, trait_ref_of_method};
use core::ops::ControlFlow;
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;
use rustc_hir::intravisit::{walk_expr, Visitor};
use rustc_hir_analysis::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; use rustc_hir_analysis::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::mir::FakeReadCause; use rustc_middle::mir::FakeReadCause;
@ -65,15 +66,19 @@ pub(super) fn check<'tcx>(
} }
}; };
let mut visitor = ExprVisitor { let mut found = false;
assignee, let found_multiple = for_each_expr(e, |e| {
counter: 0, if eq_expr_value(cx, assignee, e) {
cx, if found {
}; return ControlFlow::Break(());
}
found = true;
}
ControlFlow::Continue(())
})
.is_some();
walk_expr(&mut visitor, e); if found && !found_multiple {
if visitor.counter == 1 {
// a = a op b // a = a op b
if eq_expr_value(cx, assignee, l) { if eq_expr_value(cx, assignee, l) {
lint(assignee, r); lint(assignee, r);
@ -98,22 +103,6 @@ pub(super) fn check<'tcx>(
} }
} }
struct ExprVisitor<'a, 'tcx> {
assignee: &'a hir::Expr<'a>,
counter: u8,
cx: &'a LateContext<'tcx>,
}
impl<'a, 'tcx> Visitor<'tcx> for ExprVisitor<'a, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
if eq_expr_value(self.cx, self.assignee, expr) {
self.counter += 1;
}
walk_expr(self, expr);
}
}
fn imm_borrows_in_expr(cx: &LateContext<'_>, e: &hir::Expr<'_>) -> hir::HirIdSet { fn imm_borrows_in_expr(cx: &LateContext<'_>, e: &hir::Expr<'_>) -> hir::HirIdSet {
struct S(hir::HirIdSet); struct S(hir::HirIdSet);
impl Delegate<'_> for S { impl Delegate<'_> for S {

View file

@ -1,9 +1,11 @@
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then}; use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::source::{snippet_opt, snippet_with_context}; use clippy_utils::source::{snippet_opt, snippet_with_context};
use clippy_utils::visitors::{for_each_expr, Descend};
use clippy_utils::{fn_def_id, path_to_local_id}; use clippy_utils::{fn_def_id, path_to_local_id};
use core::ops::ControlFlow;
use if_chain::if_chain; use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; use rustc_hir::intravisit::FnKind;
use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, HirId, MatchSource, PatKind, StmtKind}; use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, HirId, MatchSource, PatKind, StmtKind};
use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro; use rustc_middle::lint::in_external_macro;
@ -270,33 +272,20 @@ fn emit_return_lint(
} }
fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
let mut visitor = BorrowVisitor { cx, borrows: false }; for_each_expr(expr, |e| {
walk_expr(&mut visitor, expr); if let Some(def_id) = fn_def_id(cx, e)
visitor.borrows && cx
}
struct BorrowVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
borrows: bool,
}
impl<'tcx> Visitor<'tcx> for BorrowVisitor<'_, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if self.borrows || expr.span.from_expansion() {
return;
}
if let Some(def_id) = fn_def_id(self.cx, expr) {
self.borrows = self
.cx
.tcx .tcx
.fn_sig(def_id) .fn_sig(def_id)
.output()
.skip_binder() .skip_binder()
.output()
.walk() .walk()
.any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_))); .any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_)))
{
ControlFlow::Break(())
} else {
ControlFlow::Continue(Descend::from(!expr.span.from_expansion()))
} }
})
walk_expr(self, expr); .is_some()
}
} }

View file

@ -1,8 +1,9 @@
use clippy_utils::diagnostics::span_lint; use clippy_utils::diagnostics::span_lint;
use clippy_utils::visitors::for_each_expr;
use clippy_utils::{binop_traits, trait_ref_of_method, BINOP_TRAITS, OP_ASSIGN_TRAITS}; use clippy_utils::{binop_traits, trait_ref_of_method, BINOP_TRAITS, OP_ASSIGN_TRAITS};
use core::ops::ControlFlow;
use if_chain::if_chain; use if_chain::if_chain;
use rustc_hir as hir; use rustc_hir as hir;
use rustc_hir::intravisit::{walk_expr, Visitor};
use rustc_lint::{LateContext, LateLintPass}; use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
@ -92,25 +93,17 @@ impl<'tcx> LateLintPass<'tcx> for SuspiciousImpl {
} }
fn count_binops(expr: &hir::Expr<'_>) -> u32 { fn count_binops(expr: &hir::Expr<'_>) -> u32 {
let mut visitor = BinaryExprVisitor::default(); let mut count = 0u32;
visitor.visit_expr(expr); let _: Option<!> = for_each_expr(expr, |e| {
visitor.nb_binops if matches!(
} e.kind,
#[derive(Default)]
struct BinaryExprVisitor {
nb_binops: u32,
}
impl<'tcx> Visitor<'tcx> for BinaryExprVisitor {
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
match expr.kind {
hir::ExprKind::Binary(..) hir::ExprKind::Binary(..)
| hir::ExprKind::Unary(hir::UnOp::Not | hir::UnOp::Neg, _) | hir::ExprKind::Unary(hir::UnOp::Not | hir::UnOp::Neg, _)
| hir::ExprKind::AssignOp(..) => self.nb_binops += 1, | hir::ExprKind::AssignOp(..)
_ => {}, ) {
count += 1;
} }
ControlFlow::Continue(())
walk_expr(self, expr); });
} count
} }

View file

@ -1,12 +1,12 @@
use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::visitors::for_each_expr;
use clippy_utils::{method_chain_args, return_ty}; use clippy_utils::{method_chain_args, return_ty};
use core::ops::ControlFlow;
use if_chain::if_chain; use if_chain::if_chain;
use rustc_hir as hir; use rustc_hir as hir;
use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::ImplItemKind;
use rustc_hir::{Expr, ImplItemKind};
use rustc_lint::{LateContext, LateLintPass}; use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{sym, Span}; use rustc_span::{sym, Span};
@ -73,51 +73,37 @@ impl<'tcx> LateLintPass<'tcx> for UnwrapInResult {
} }
} }
struct FindExpectUnwrap<'a, 'tcx> {
lcx: &'a LateContext<'tcx>,
typeck_results: &'tcx ty::TypeckResults<'tcx>,
result: Vec<Span>,
}
impl<'a, 'tcx> Visitor<'tcx> for FindExpectUnwrap<'a, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
// check for `expect`
if let Some(arglists) = method_chain_args(expr, &["expect"]) {
let receiver_ty = self.typeck_results.expr_ty(arglists[0].0).peel_refs();
if is_type_diagnostic_item(self.lcx, receiver_ty, sym::Option)
|| is_type_diagnostic_item(self.lcx, receiver_ty, sym::Result)
{
self.result.push(expr.span);
}
}
// check for `unwrap`
if let Some(arglists) = method_chain_args(expr, &["unwrap"]) {
let receiver_ty = self.typeck_results.expr_ty(arglists[0].0).peel_refs();
if is_type_diagnostic_item(self.lcx, receiver_ty, sym::Option)
|| is_type_diagnostic_item(self.lcx, receiver_ty, sym::Result)
{
self.result.push(expr.span);
}
}
// and check sub-expressions
intravisit::walk_expr(self, expr);
}
}
fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_item: &'tcx hir::ImplItem<'_>) { fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_item: &'tcx hir::ImplItem<'_>) {
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);
let mut fpu = FindExpectUnwrap { let typeck = cx.tcx.typeck(impl_item.def_id.def_id);
lcx: cx, let mut result = Vec::new();
typeck_results: cx.tcx.typeck(impl_item.def_id.def_id), let _: Option<!> = for_each_expr(body.value, |e| {
result: Vec::new(), // check for `expect`
}; if let Some(arglists) = method_chain_args(e, &["expect"]) {
fpu.visit_expr(body.value); let receiver_ty = typeck.expr_ty(arglists[0].0).peel_refs();
if is_type_diagnostic_item(cx, receiver_ty, sym::Option)
|| is_type_diagnostic_item(cx, receiver_ty, sym::Result)
{
result.push(e.span);
}
}
// check for `unwrap`
if let Some(arglists) = method_chain_args(e, &["unwrap"]) {
let receiver_ty = typeck.expr_ty(arglists[0].0).peel_refs();
if is_type_diagnostic_item(cx, receiver_ty, sym::Option)
|| is_type_diagnostic_item(cx, receiver_ty, sym::Result)
{
result.push(e.span);
}
}
ControlFlow::Continue(())
});
// if we've found one, lint // if we've found one, lint
if !fpu.result.is_empty() { if !result.is_empty() {
span_lint_and_then( span_lint_and_then(
cx, cx,
UNWRAP_IN_RESULT, UNWRAP_IN_RESULT,
@ -125,7 +111,7 @@ fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_item: &'tc
"used unwrap or expect in a function that returns result or option", "used unwrap or expect in a function that returns result or option",
move |diag| { move |diag| {
diag.help("unwrap and expect should not be used in a function that returns result or option"); diag.help("unwrap and expect should not be used in a function that returns result or option");
diag.span_note(fpu.result, "potential non-recoverable error(s)"); diag.span_note(result, "potential non-recoverable error(s)");
}, },
); );
} }