Skip to content

Commit

Permalink
Use for_each_expr in place of some visitors
Browse files Browse the repository at this point in the history
  • Loading branch information
@Jarcho
Jarcho committed Aug 20, 2022
1 parent 231f099 commit 9cfba1a
Show file tree
Hide file tree
Showing 9 changed files with 197 additions and 325 deletions.
68 changes: 28 additions & 40 deletions clippy_lints/src/blocks_in_if_conditions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ use clippy_utils::get_parent_expr;
use clippy_utils::higher;
use clippy_utils::source::snippet_block_with_applicability;
use clippy_utils::ty::implements_trait;
use clippy_utils::visitors::{for_each_expr, Descend};
use core::ops::ControlFlow;
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_expr, Visitor};
use rustc_hir::{BlockCheckMode, Closure, Expr, ExprKind};
use rustc_hir::{BlockCheckMode, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -44,39 +45,6 @@ declare_clippy_lint! {

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 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`";
Expand Down Expand Up @@ -144,11 +112,31 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInIfConditions {
}
}
} else {
let mut visitor = ExVisitor { found_block: None, cx };
walk_expr(&mut visitor, cond);
if let Some(block) = visitor.found_block {
span_lint(cx, BLOCKS_IN_IF_CONDITIONS, block.span, COMPLEX_BLOCK_MESSAGE);
}
let _: Option<!> = for_each_expr(cond, |e| {
if let ExprKind::Closure(closure) = e.kind {
// do not lint if the closure is called using an iterator (see #1141)
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)
});
}
}
}
Expand Down
61 changes: 27 additions & 34 deletions clippy_lints/src/cognitive_complexity.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::visitors::for_each_expr;
use clippy_utils::LimitStack;
use core::ops::ControlFlow;
use rustc_ast::ast::Attribute;
use rustc_hir::intravisit::{walk_expr, FnKind, Visitor};
use rustc_hir::{Body, Expr, ExprKind, FnDecl, HirId};
use rustc_hir::intravisit::FnKind;
use rustc_hir::{Body, ExprKind, FnDecl, HirId};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::Span;
Expand Down Expand Up @@ -63,9 +65,25 @@ impl CognitiveComplexity {

let expr = &body.value;

let mut helper = CcHelper { cc: 1, returns: 0 };
helper.visit_expr(expr);
let CcHelper { cc, returns } = helper;
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 ret_ty = cx.typeck_results().node_type(expr.hir_id);
let ret_adjust = if is_type_diagnostic_item(cx, ret_ty, sym::Result) {
returns
Expand All @@ -74,13 +92,12 @@ impl CognitiveComplexity {
(returns / 2)
};

let mut rust_cc = cc;
// prevent degenerate cases where unreachable code contains `return` statements
if rust_cc >= ret_adjust {
rust_cc -= ret_adjust;
if cc >= ret_adjust {
cc -= ret_adjust;
}

if rust_cc > self.limit.limit() {
if cc > self.limit.limit() {
let fn_span = match kind {
FnKind::ItemFn(ident, _, _) | FnKind::Method(ident, _) => ident.span,
FnKind::Closure => {
Expand Down Expand Up @@ -108,7 +125,7 @@ impl CognitiveComplexity {
fn_span,
&format!(
"the function has a cognitive complexity of ({}/{})",
rust_cc,
cc,
self.limit.limit()
),
None,
Expand Down Expand Up @@ -141,27 +158,3 @@ impl<'tcx> LateLintPass<'tcx> for CognitiveComplexity {
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,
_ => {},
}
}
}
70 changes: 27 additions & 43 deletions clippy_lints/src/functions/must_use.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use rustc_ast::ast::Attribute;
use rustc_errors::Applicability;
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_middle::{
lint::in_external_macro,
Expand All @@ -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::source::snippet_opt;
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 core::ops::ControlFlow;

use super::{DOUBLE_MUST_USE, MUST_USE_CANDIDATE, MUST_USE_UNIT};

pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
Expand Down Expand Up @@ -199,61 +202,42 @@ 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,
fn is_mutated_static(e: &hir::Expr<'_>) -> bool {
use hir::ExprKind::{Field, Index, Path};

match e.kind {
Path(QPath::Resolved(_, path)) => !matches!(path.res, Res::Local(_)),
Path(_) => true,
Field(inner, _) | Index(inner, _) => is_mutated_static(inner),
_ => false,
}
}

impl<'a, 'tcx> intravisit::Visitor<'tcx> for StaticMutVisitor<'a, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
fn mutates_static<'tcx>(cx: &LateContext<'tcx>, body: &'tcx hir::Body<'_>) -> bool {
for_each_expr(&body.value, |e| {
use hir::ExprKind::{AddrOf, Assign, AssignOp, Call, MethodCall};

if self.mutates_static {
return;
}
match expr.kind {
match e.kind {
Call(_, args) | MethodCall(_, 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).expr_ty(arg),
arg.span,
&mut tys,
)
if cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id())
&& is_mutable_ty(cx, cx.tcx.typeck(arg.hir_id.owner).expr_ty(arg), arg.span, &mut tys)
&& is_mutated_static(arg)
{
self.mutates_static = true;
return;
return ControlFlow::Break(());
}
tys.clear();
}
ControlFlow::Continue(())
},
Assign(target, ..) | AssignOp(_, target, _) | AddrOf(_, hir::Mutability::Mut, target) => {
self.mutates_static |= is_mutated_static(target);
Assign(target, ..) | AssignOp(_, target, _) | AddrOf(_, hir::Mutability::Mut, target)
if is_mutated_static(target) =>
{
ControlFlow::Break(())
},
_ => {},
_ => ControlFlow::Continue(()),
}
}
}

fn is_mutated_static(e: &hir::Expr<'_>) -> bool {
use hir::ExprKind::{Field, Index, Path};

match e.kind {
Path(QPath::Resolved(_, path)) => !matches!(path.res, Res::Local(_)),
Path(_) => true,
Field(inner, _) | Index(inner, _) => is_mutated_static(inner),
_ => false,
}
}

fn mutates_static<'tcx>(cx: &LateContext<'tcx>, body: &'tcx hir::Body<'_>) -> bool {
let mut v = StaticMutVisitor {
cx,
mutates_static: false,
};
intravisit::walk_expr(&mut v, &body.value);
v.mutates_static
})
.is_some()
}
Loading

0 comments on commit 9cfba1a

Please sign in to comment.