Skip to content

Commit

Permalink
Move find_all_ret_expressions into utils
Browse files Browse the repository at this point in the history
  • Loading branch information
hkmatsumoto committed Oct 6, 2020
1 parent 5ae1e97 commit c4e24c8
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 250 deletions.
125 changes: 1 addition & 124 deletions clippy_lints/src/methods/bind_instead_of_map.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
use super::{contains_return, BIND_INSTEAD_OF_MAP};
use crate::utils::{
in_macro, match_qpath, match_type, method_calls, multispan_sugg_with_applicability, paths, remove_blocks, snippet,
snippet_with_macro_callsite, span_lint_and_sugg, span_lint_and_then,
snippet_with_macro_callsite, span_lint_and_sugg, span_lint_and_then, visitors::find_all_ret_expressions,
};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::intravisit::{self, Visitor};
use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
use rustc_span::Span;

pub(crate) struct OptionAndThenSome;
Expand Down Expand Up @@ -186,124 +184,3 @@ pub(crate) trait BindInsteadOfMap {
}
}
}

/// 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
}

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
}

fn find_all_ret_expressions<'hir, F>(_cx: &LateContext<'_>, expr: &'hir hir::Expr<'hir>, callback: F) -> bool
where
F: FnMut(&'hir hir::Expr<'hir>) -> bool,
{
struct RetFinder<F> {
in_stmt: bool,
failed: bool,
cb: F,
}

struct WithStmtGuarg<'a, F> {
val: &'a mut RetFinder<F>,
prev_in_stmt: bool,
}

impl<F> RetFinder<F> {
fn inside_stmt(&mut self, in_stmt: bool) -> WithStmtGuarg<'_, F> {
let prev_in_stmt = std::mem::replace(&mut self.in_stmt, in_stmt);
WithStmtGuarg {
val: self,
prev_in_stmt,
}
}
}

impl<F> std::ops::Deref for WithStmtGuarg<'_, F> {
type Target = RetFinder<F>;

fn deref(&self) -> &Self::Target {
self.val
}
}

impl<F> std::ops::DerefMut for WithStmtGuarg<'_, F> {
fn deref_mut(&mut self) -> &mut Self::Target {
self.val
}
}

impl<F> Drop for WithStmtGuarg<'_, F> {
fn drop(&mut self) {
self.val.in_stmt = self.prev_in_stmt;
}
}

impl<'hir, F: FnMut(&'hir hir::Expr<'hir>) -> bool> intravisit::Visitor<'hir> for RetFinder<F> {
type Map = Map<'hir>;

fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
intravisit::NestedVisitorMap::None
}

fn visit_stmt(&mut self, stmt: &'hir hir::Stmt<'_>) {
intravisit::walk_stmt(&mut *self.inside_stmt(true), stmt)
}

fn visit_expr(&mut self, expr: &'hir hir::Expr<'_>) {
if self.failed {
return;
}
if self.in_stmt {
match expr.kind {
hir::ExprKind::Ret(Some(expr)) => self.inside_stmt(false).visit_expr(expr),
_ => intravisit::walk_expr(self, expr),
}
} else {
match expr.kind {
hir::ExprKind::Match(cond, arms, _) => {
self.inside_stmt(true).visit_expr(cond);
for arm in arms {
self.visit_expr(arm.body);
}
},
hir::ExprKind::Block(..) => intravisit::walk_expr(self, expr),
hir::ExprKind::Ret(Some(expr)) => self.visit_expr(expr),
_ => self.failed |= !(self.cb)(expr),
}
}
}
}

!contains_try(expr) && {
let mut ret_finder = RetFinder {
in_stmt: false,
failed: false,
cb: callback,
};
ret_finder.visit_expr(expr);
!ret_finder.failed
}
}
132 changes: 6 additions & 126 deletions clippy_lints/src/unnecessary_wrap.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
use crate::utils::{is_type_diagnostic_item, match_qpath, paths, return_ty, snippet, span_lint_and_then};
use crate::utils::{
is_type_diagnostic_item, match_qpath, paths, return_ty, snippet, span_lint_and_then,
visitors::find_all_ret_expressions,
};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{FnKind, Visitor};
use rustc_hir::intravisit::FnKind;
use rustc_hir::*;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::{hir::map::Map, ty::subst::GenericArgKind};
use rustc_middle::ty::subst::GenericArgKind;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::Span;

Expand Down Expand Up @@ -125,126 +128,3 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap {
}
}
}

// code below is copied from `bind_instead_of_map`

fn find_all_ret_expressions<'hir, F>(_cx: &LateContext<'_>, expr: &'hir Expr<'hir>, callback: F) -> bool
where
F: FnMut(&'hir Expr<'hir>) -> bool,
{
struct RetFinder<F> {
in_stmt: bool,
failed: bool,
cb: F,
}

struct WithStmtGuarg<'a, F> {
val: &'a mut RetFinder<F>,
prev_in_stmt: bool,
}

impl<F> RetFinder<F> {
fn inside_stmt(&mut self, in_stmt: bool) -> WithStmtGuarg<'_, F> {
let prev_in_stmt = std::mem::replace(&mut self.in_stmt, in_stmt);
WithStmtGuarg {
val: self,
prev_in_stmt,
}
}
}

impl<F> std::ops::Deref for WithStmtGuarg<'_, F> {
type Target = RetFinder<F>;

fn deref(&self) -> &Self::Target {
self.val
}
}

impl<F> std::ops::DerefMut for WithStmtGuarg<'_, F> {
fn deref_mut(&mut self) -> &mut Self::Target {
self.val
}
}

impl<F> Drop for WithStmtGuarg<'_, F> {
fn drop(&mut self) {
self.val.in_stmt = self.prev_in_stmt;
}
}

impl<'hir, F: FnMut(&'hir Expr<'hir>) -> bool> intravisit::Visitor<'hir> for RetFinder<F> {
type Map = Map<'hir>;

fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
intravisit::NestedVisitorMap::None
}

fn visit_stmt(&mut self, stmt: &'hir Stmt<'_>) {
intravisit::walk_stmt(&mut *self.inside_stmt(true), stmt)
}

fn visit_expr(&mut self, expr: &'hir Expr<'_>) {
if self.failed {
return;
}
if self.in_stmt {
match expr.kind {
ExprKind::Ret(Some(expr)) => self.inside_stmt(false).visit_expr(expr),
_ => intravisit::walk_expr(self, expr),
}
} else {
match expr.kind {
ExprKind::Match(cond, arms, _) => {
self.inside_stmt(true).visit_expr(cond);
for arm in arms {
self.visit_expr(arm.body);
}
},
ExprKind::Block(..) => intravisit::walk_expr(self, expr),
ExprKind::Ret(Some(expr)) => self.visit_expr(expr),
_ => self.failed |= !(self.cb)(expr),
}
}
}
}

!contains_try(expr) && {
let mut ret_finder = RetFinder {
in_stmt: false,
failed: false,
cb: callback,
};
ret_finder.visit_expr(expr);
!ret_finder.failed
}
}

/// returns `true` if expr contains match expr desugared from try
fn contains_try(expr: &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
}

fn visit_expr(&mut self, expr: &'hir Expr<'hir>) {
if self.found {
return;
}
match expr.kind {
ExprKind::Match(_, _, MatchSource::TryDesugar) => self.found = true,
_ => intravisit::walk_expr(self, expr),
}
}
}

let mut visitor = TryFinder { found: false };
visitor.visit_expr(expr);
visitor.found
}
1 change: 1 addition & 0 deletions clippy_lints/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub mod paths;
pub mod ptr;
pub mod sugg;
pub mod usage;
pub mod visitors;

pub use self::attrs::*;
pub use self::diagnostics::*;
Expand Down
Loading

0 comments on commit c4e24c8

Please sign in to comment.