Replace local variables signifying "done" or "loop break", use ControlFlow #12830
This commit is contained in:
parent
615284aa5a
commit
b21a91ae3a
@ -1,6 +1,7 @@
|
||||
#![feature(array_windows)]
|
||||
#![feature(binary_heap_into_iter_sorted)]
|
||||
#![feature(box_patterns)]
|
||||
#![feature(control_flow_enum)]
|
||||
#![feature(f128)]
|
||||
#![feature(f16)]
|
||||
#![feature(if_let_guard)]
|
||||
|
@ -22,6 +22,7 @@
|
||||
use rustc_span::def_id::LocalDefId;
|
||||
use rustc_span::symbol::{kw, Ident, Symbol};
|
||||
use rustc_span::Span;
|
||||
use std::ops::ControlFlow;
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
@ -380,11 +381,8 @@ fn could_use_elision<'tcx>(
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut checker = BodyLifetimeChecker {
|
||||
lifetimes_used_in_body: false,
|
||||
};
|
||||
checker.visit_expr(body.value);
|
||||
if checker.lifetimes_used_in_body {
|
||||
let mut checker = BodyLifetimeChecker;
|
||||
if checker.visit_expr(body.value).is_break() {
|
||||
return None;
|
||||
}
|
||||
}
|
||||
@ -694,15 +692,15 @@ fn report_extra_impl_lifetimes<'tcx>(cx: &LateContext<'tcx>, impl_: &'tcx Impl<'
|
||||
}
|
||||
}
|
||||
|
||||
struct BodyLifetimeChecker {
|
||||
lifetimes_used_in_body: bool,
|
||||
}
|
||||
struct BodyLifetimeChecker;
|
||||
|
||||
impl<'tcx> Visitor<'tcx> for BodyLifetimeChecker {
|
||||
type Result = ControlFlow<()>;
|
||||
// for lifetimes as parameters of generics
|
||||
fn visit_lifetime(&mut self, lifetime: &'tcx Lifetime) {
|
||||
fn visit_lifetime(&mut self, lifetime: &'tcx Lifetime) -> ControlFlow<()> {
|
||||
if !lifetime.is_anonymous() && lifetime.ident.name != kw::StaticLifetime {
|
||||
self.lifetimes_used_in_body = true;
|
||||
return ControlFlow::Break(());
|
||||
}
|
||||
ControlFlow::Continue(())
|
||||
}
|
||||
}
|
||||
|
@ -8,6 +8,7 @@
|
||||
use rustc_middle::mir::FakeReadCause;
|
||||
use rustc_middle::ty;
|
||||
use rustc_span::Span;
|
||||
use std::ops::ControlFlow;
|
||||
|
||||
pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>, body: &Expr<'_>) {
|
||||
if let Some(higher::Range {
|
||||
@ -114,7 +115,6 @@ fn mutation_span(&self) -> (Option<Span>, Option<Span>) {
|
||||
struct BreakAfterExprVisitor {
|
||||
hir_id: HirId,
|
||||
past_expr: bool,
|
||||
past_candidate: bool,
|
||||
break_after_expr: bool,
|
||||
}
|
||||
|
||||
@ -123,7 +123,6 @@ pub fn is_found(cx: &LateContext<'_>, hir_id: HirId) -> bool {
|
||||
let mut visitor = BreakAfterExprVisitor {
|
||||
hir_id,
|
||||
past_expr: false,
|
||||
past_candidate: false,
|
||||
break_after_expr: false,
|
||||
};
|
||||
|
||||
@ -135,21 +134,19 @@ pub fn is_found(cx: &LateContext<'_>, hir_id: HirId) -> bool {
|
||||
}
|
||||
|
||||
impl<'tcx> Visitor<'tcx> for BreakAfterExprVisitor {
|
||||
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
|
||||
if self.past_candidate {
|
||||
return;
|
||||
}
|
||||
|
||||
type Result = ControlFlow<()>;
|
||||
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) -> ControlFlow<()> {
|
||||
if expr.hir_id == self.hir_id {
|
||||
self.past_expr = true;
|
||||
ControlFlow::Continue(())
|
||||
} else if self.past_expr {
|
||||
if matches!(&expr.kind, ExprKind::Break(..)) {
|
||||
self.break_after_expr = true;
|
||||
}
|
||||
|
||||
self.past_candidate = true;
|
||||
ControlFlow::Break(())
|
||||
} else {
|
||||
intravisit::walk_expr(self, expr);
|
||||
intravisit::walk_expr(self, expr)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -7,6 +7,7 @@
|
||||
use rustc_hir::intravisit::{walk_expr, Visitor};
|
||||
use rustc_hir::{Expr, ExprKind, HirIdSet, QPath};
|
||||
use rustc_lint::LateContext;
|
||||
use std::ops::ControlFlow;
|
||||
|
||||
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) {
|
||||
if constant(cx, cx.typeck_results(), cond).is_some() {
|
||||
@ -35,11 +36,8 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &'
|
||||
};
|
||||
let mutable_static_in_cond = var_visitor.def_ids.items().any(|(_, v)| *v);
|
||||
|
||||
let mut has_break_or_return_visitor = HasBreakOrReturnVisitor {
|
||||
has_break_or_return: false,
|
||||
};
|
||||
has_break_or_return_visitor.visit_expr(expr);
|
||||
let has_break_or_return = has_break_or_return_visitor.has_break_or_return;
|
||||
let mut has_break_or_return_visitor = HasBreakOrReturnVisitor;
|
||||
let has_break_or_return = has_break_or_return_visitor.visit_expr(expr).is_break();
|
||||
|
||||
if no_cond_variable_mutated && !mutable_static_in_cond {
|
||||
span_lint_and_then(
|
||||
@ -59,25 +57,19 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &'
|
||||
}
|
||||
}
|
||||
|
||||
struct HasBreakOrReturnVisitor {
|
||||
has_break_or_return: bool,
|
||||
}
|
||||
struct HasBreakOrReturnVisitor;
|
||||
|
||||
impl<'tcx> Visitor<'tcx> for HasBreakOrReturnVisitor {
|
||||
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
|
||||
if self.has_break_or_return {
|
||||
return;
|
||||
}
|
||||
|
||||
type Result = ControlFlow<()>;
|
||||
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) -> ControlFlow<()> {
|
||||
match expr.kind {
|
||||
ExprKind::Ret(_) | ExprKind::Break(_, _) => {
|
||||
self.has_break_or_return = true;
|
||||
return;
|
||||
return ControlFlow::Break(());
|
||||
},
|
||||
_ => {},
|
||||
}
|
||||
|
||||
walk_expr(self, expr);
|
||||
walk_expr(self, expr)
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -10,6 +10,7 @@
|
||||
use rustc_lint::LateContext;
|
||||
use rustc_middle::hir::nested_filter;
|
||||
use rustc_span::{sym, Span};
|
||||
use std::ops::ControlFlow;
|
||||
|
||||
use super::MAP_UNWRAP_OR;
|
||||
|
||||
@ -54,15 +55,14 @@ pub(super) fn check<'tcx>(
|
||||
let mut reference_visitor = ReferenceVisitor {
|
||||
cx,
|
||||
identifiers: unwrap_visitor.identifiers,
|
||||
found_reference: false,
|
||||
unwrap_or_span: unwrap_arg.span,
|
||||
};
|
||||
|
||||
let map = cx.tcx.hir();
|
||||
let body = map.body_owned_by(map.enclosing_body_owner(expr.hir_id));
|
||||
reference_visitor.visit_body(body);
|
||||
|
||||
if reference_visitor.found_reference {
|
||||
// Visit the body, and return if we've found a reference
|
||||
if reference_visitor.visit_body(body).is_break() {
|
||||
return;
|
||||
}
|
||||
}
|
||||
@ -151,17 +151,16 @@ fn nested_visit_map(&mut self) -> Self::Map {
|
||||
struct ReferenceVisitor<'a, 'tcx> {
|
||||
cx: &'a LateContext<'tcx>,
|
||||
identifiers: FxHashSet<HirId>,
|
||||
found_reference: bool,
|
||||
unwrap_or_span: Span,
|
||||
}
|
||||
|
||||
impl<'a, 'tcx> Visitor<'tcx> for ReferenceVisitor<'a, 'tcx> {
|
||||
type NestedFilter = nested_filter::All;
|
||||
fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'_>) {
|
||||
type Result = ControlFlow<()>;
|
||||
fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'_>) -> ControlFlow<()> {
|
||||
// If we haven't found a reference yet, check if this references
|
||||
// one of the locals that was moved in the `unwrap_or` argument.
|
||||
// We are only interested in exprs that appear before the `unwrap_or` call.
|
||||
if !self.found_reference {
|
||||
if expr.span < self.unwrap_or_span
|
||||
&& let ExprKind::Path(ref path) = expr.kind
|
||||
&& let QPath::Resolved(_, path) = path
|
||||
@ -170,10 +169,9 @@ fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'_>) {
|
||||
&& let PatKind::Binding(_, local_id, ..) = pat.kind
|
||||
&& self.identifiers.contains(&local_id)
|
||||
{
|
||||
self.found_reference = true;
|
||||
}
|
||||
rustc_hir::intravisit::walk_expr(self, expr);
|
||||
return ControlFlow::Break(());
|
||||
}
|
||||
rustc_hir::intravisit::walk_expr(self, expr)
|
||||
}
|
||||
|
||||
fn nested_visit_map(&mut self) -> Self::Map {
|
||||
|
@ -14,6 +14,7 @@
|
||||
use rustc_middle::ty;
|
||||
use rustc_session::declare_lint_pass;
|
||||
use rustc_span::ExpnKind;
|
||||
use std::ops::ControlFlow;
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
@ -42,24 +43,15 @@
|
||||
declare_lint_pass!(RedundantClosureCall => [REDUNDANT_CLOSURE_CALL]);
|
||||
|
||||
// Used to find `return` statements or equivalents e.g., `?`
|
||||
struct ReturnVisitor {
|
||||
found_return: bool,
|
||||
}
|
||||
|
||||
impl ReturnVisitor {
|
||||
#[must_use]
|
||||
fn new() -> Self {
|
||||
Self { found_return: false }
|
||||
}
|
||||
}
|
||||
struct ReturnVisitor;
|
||||
|
||||
impl<'tcx> Visitor<'tcx> for ReturnVisitor {
|
||||
fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) {
|
||||
type Result = ControlFlow<()>;
|
||||
fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) -> ControlFlow<()> {
|
||||
if let ExprKind::Ret(_) | ExprKind::Match(.., hir::MatchSource::TryDesugar(_)) = ex.kind {
|
||||
self.found_return = true;
|
||||
} else {
|
||||
hir_visit::walk_expr(self, ex);
|
||||
return ControlFlow::Break(());
|
||||
}
|
||||
hir_visit::walk_expr(self, ex)
|
||||
}
|
||||
}
|
||||
|
||||
@ -101,9 +93,8 @@ fn find_innermost_closure<'tcx>(
|
||||
while let ExprKind::Closure(closure) = expr.kind
|
||||
&& let body = cx.tcx.hir().body(closure.body)
|
||||
&& {
|
||||
let mut visitor = ReturnVisitor::new();
|
||||
visitor.visit_expr(body.value);
|
||||
!visitor.found_return
|
||||
let mut visitor = ReturnVisitor;
|
||||
!visitor.visit_expr(body.value).is_break()
|
||||
}
|
||||
&& steps > 0
|
||||
{
|
||||
|
@ -16,6 +16,7 @@
|
||||
use rustc_span::symbol::{kw, Ident};
|
||||
use rustc_span::{sym, Span};
|
||||
use rustc_trait_selection::error_reporting::traits::suggestions::ReturnsVisitor;
|
||||
use std::ops::ControlFlow;
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
@ -276,7 +277,6 @@ struct CheckCalls<'a, 'tcx> {
|
||||
cx: &'a LateContext<'tcx>,
|
||||
map: Map<'tcx>,
|
||||
implemented_ty_id: DefId,
|
||||
found_default_call: bool,
|
||||
method_span: Span,
|
||||
}
|
||||
|
||||
@ -285,16 +285,14 @@ impl<'a, 'tcx> Visitor<'tcx> for CheckCalls<'a, 'tcx>
|
||||
'tcx: 'a,
|
||||
{
|
||||
type NestedFilter = nested_filter::OnlyBodies;
|
||||
type Result = ControlFlow<()>;
|
||||
|
||||
fn nested_visit_map(&mut self) -> Self::Map {
|
||||
self.map
|
||||
}
|
||||
|
||||
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
|
||||
if self.found_default_call {
|
||||
return;
|
||||
}
|
||||
walk_expr(self, expr);
|
||||
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) -> ControlFlow<()> {
|
||||
walk_expr(self, expr)?;
|
||||
|
||||
if let ExprKind::Call(f, _) = expr.kind
|
||||
&& let ExprKind::Path(qpath) = f.kind
|
||||
@ -303,9 +301,10 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
|
||||
&& let Some(trait_def_id) = self.cx.tcx.trait_of_item(method_def_id)
|
||||
&& self.cx.tcx.is_diagnostic_item(sym::Default, trait_def_id)
|
||||
{
|
||||
self.found_default_call = true;
|
||||
span_error(self.cx, self.method_span, expr);
|
||||
return ControlFlow::Break(());
|
||||
}
|
||||
ControlFlow::Continue(())
|
||||
}
|
||||
}
|
||||
|
||||
@ -383,7 +382,6 @@ fn check_default_new<'tcx>(
|
||||
cx,
|
||||
map: cx.tcx.hir(),
|
||||
implemented_ty_id,
|
||||
found_default_call: false,
|
||||
method_span,
|
||||
};
|
||||
walk_body(&mut c, body);
|
||||
|
@ -8,6 +8,7 @@
|
||||
use rustc_middle::hir::nested_filter::OnlyBodies;
|
||||
use rustc_session::declare_lint_pass;
|
||||
use rustc_span::sym;
|
||||
use std::ops::ControlFlow;
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
@ -70,15 +71,16 @@ fn check_block(&mut self, cx: &LateContext<'tcx>, block: &Block<'tcx>) {
|
||||
return;
|
||||
}
|
||||
|
||||
for stmt in &block.stmts[idx..] {
|
||||
vis.visit_stmt(stmt);
|
||||
let mut found_peek_call = block.stmts[idx..].iter().any(|stmt| vis.visit_stmt(stmt).is_break());
|
||||
|
||||
if !found_peek_call
|
||||
&& let Some(expr) = block.expr
|
||||
&& vis.visit_expr(expr).is_break()
|
||||
{
|
||||
found_peek_call = true;
|
||||
}
|
||||
|
||||
if let Some(expr) = block.expr {
|
||||
vis.visit_expr(expr);
|
||||
}
|
||||
|
||||
if !vis.found_peek_call {
|
||||
if !found_peek_call {
|
||||
span_lint_hir_and_then(
|
||||
cx,
|
||||
UNUSED_PEEKABLE,
|
||||
@ -98,31 +100,23 @@ fn check_block(&mut self, cx: &LateContext<'tcx>, block: &Block<'tcx>) {
|
||||
struct PeekableVisitor<'a, 'tcx> {
|
||||
cx: &'a LateContext<'tcx>,
|
||||
expected_hir_id: HirId,
|
||||
found_peek_call: bool,
|
||||
}
|
||||
|
||||
impl<'a, 'tcx> PeekableVisitor<'a, 'tcx> {
|
||||
fn new(cx: &'a LateContext<'tcx>, expected_hir_id: HirId) -> Self {
|
||||
Self {
|
||||
cx,
|
||||
expected_hir_id,
|
||||
found_peek_call: false,
|
||||
}
|
||||
Self { cx, expected_hir_id }
|
||||
}
|
||||
}
|
||||
|
||||
impl<'tcx> Visitor<'tcx> for PeekableVisitor<'_, 'tcx> {
|
||||
type NestedFilter = OnlyBodies;
|
||||
type Result = ControlFlow<()>;
|
||||
|
||||
fn nested_visit_map(&mut self) -> Self::Map {
|
||||
self.cx.tcx.hir()
|
||||
}
|
||||
|
||||
fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
|
||||
if self.found_peek_call {
|
||||
return;
|
||||
}
|
||||
|
||||
fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) -> ControlFlow<()> {
|
||||
if path_to_local_id(ex, self.expected_hir_id) {
|
||||
for (_, node) in self.cx.tcx.hir().parent_iter(ex.hir_id) {
|
||||
match node {
|
||||
@ -137,14 +131,14 @@ fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
|
||||
&& func_did == into_iter_did
|
||||
{
|
||||
// Probably a for loop desugar, stop searching
|
||||
return;
|
||||
return ControlFlow::Continue(());
|
||||
}
|
||||
|
||||
if args.iter().any(|arg| arg_is_mut_peekable(self.cx, arg)) {
|
||||
self.found_peek_call = true;
|
||||
return ControlFlow::Break(());
|
||||
}
|
||||
|
||||
return;
|
||||
return ControlFlow::Continue(());
|
||||
},
|
||||
// Catch anything taking a Peekable mutably
|
||||
ExprKind::MethodCall(
|
||||
@ -162,16 +156,14 @@ fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
|
||||
if matches!(method_name, "peek" | "peek_mut" | "next_if" | "next_if_eq")
|
||||
&& arg_is_mut_peekable(self.cx, self_arg)
|
||||
{
|
||||
self.found_peek_call = true;
|
||||
return;
|
||||
return ControlFlow::Break(());
|
||||
}
|
||||
|
||||
// foo.some_method() excluding Iterator methods
|
||||
if remaining_args.iter().any(|arg| arg_is_mut_peekable(self.cx, arg))
|
||||
&& !is_trait_method(self.cx, expr, sym::Iterator)
|
||||
{
|
||||
self.found_peek_call = true;
|
||||
return;
|
||||
return ControlFlow::Break(());
|
||||
}
|
||||
|
||||
// foo.by_ref(), keep checking for `peek`
|
||||
@ -179,41 +171,42 @@ fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
|
||||
continue;
|
||||
}
|
||||
|
||||
return;
|
||||
return ControlFlow::Continue(());
|
||||
},
|
||||
ExprKind::AddrOf(_, Mutability::Mut, _) | ExprKind::Unary(..) | ExprKind::DropTemps(_) => {
|
||||
},
|
||||
ExprKind::AddrOf(_, Mutability::Not, _) => return,
|
||||
ExprKind::AddrOf(_, Mutability::Not, _) => return ControlFlow::Continue(()),
|
||||
_ => {
|
||||
self.found_peek_call = true;
|
||||
return;
|
||||
return ControlFlow::Break(());
|
||||
},
|
||||
}
|
||||
},
|
||||
Node::LetStmt(LetStmt { init: Some(init), .. }) => {
|
||||
if arg_is_mut_peekable(self.cx, init) {
|
||||
self.found_peek_call = true;
|
||||
return ControlFlow::Break(());
|
||||
}
|
||||
|
||||
return;
|
||||
return ControlFlow::Continue(());
|
||||
},
|
||||
Node::Stmt(stmt) => {
|
||||
match stmt.kind {
|
||||
StmtKind::Let(_) | StmtKind::Item(_) => self.found_peek_call = true,
|
||||
StmtKind::Let(_) | StmtKind::Item(_) => {
|
||||
return ControlFlow::Break(());
|
||||
},
|
||||
StmtKind::Expr(_) | StmtKind::Semi(_) => {},
|
||||
}
|
||||
|
||||
return;
|
||||
return ControlFlow::Continue(());
|
||||
},
|
||||
Node::Block(_) | Node::ExprField(_) => {},
|
||||
_ => {
|
||||
return;
|
||||
return ControlFlow::Continue(());
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
walk_expr(self, ex);
|
||||
walk_expr(self, ex)
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -109,23 +109,36 @@ struct V<B, F> {
|
||||
res: Option<B>,
|
||||
}
|
||||
impl<'tcx, B, C: Continue, F: FnMut(&'tcx Expr<'tcx>) -> ControlFlow<B, C>> Visitor<'tcx> for V<B, F> {
|
||||
fn visit_expr(&mut self, e: &'tcx Expr<'tcx>) {
|
||||
type Result = ControlFlow<()>;
|
||||
|
||||
fn visit_expr(&mut self, e: &'tcx Expr<'tcx>) -> ControlFlow<()> {
|
||||
if self.res.is_some() {
|
||||
return;
|
||||
return ControlFlow::Break(());
|
||||
}
|
||||
match (self.f)(e) {
|
||||
ControlFlow::Continue(c) if c.descend() => walk_expr(self, e),
|
||||
ControlFlow::Break(b) => self.res = Some(b),
|
||||
ControlFlow::Continue(_) => (),
|
||||
ControlFlow::Break(b) => {
|
||||
self.res = Some(b);
|
||||
ControlFlow::Break(())
|
||||
},
|
||||
ControlFlow::Continue(_) => ControlFlow::Continue(()),
|
||||
}
|
||||
}
|
||||
|
||||
// Avoid unnecessary `walk_*` calls.
|
||||
fn visit_ty(&mut self, _: &'tcx hir::Ty<'tcx>) {}
|
||||
fn visit_pat(&mut self, _: &'tcx Pat<'tcx>) {}
|
||||
fn visit_qpath(&mut self, _: &'tcx QPath<'tcx>, _: HirId, _: Span) {}
|
||||
fn visit_ty(&mut self, _: &'tcx hir::Ty<'tcx>) -> ControlFlow<()> {
|
||||
ControlFlow::Continue(())
|
||||
}
|
||||
fn visit_pat(&mut self, _: &'tcx Pat<'tcx>) -> ControlFlow<()> {
|
||||
ControlFlow::Continue(())
|
||||
}
|
||||
fn visit_qpath(&mut self, _: &'tcx QPath<'tcx>, _: HirId, _: Span) -> ControlFlow<()> {
|
||||
ControlFlow::Continue(())
|
||||
}
|
||||
// Avoid monomorphising all `visit_*` functions.
|
||||
fn visit_nested_item(&mut self, _: ItemId) {}
|
||||
fn visit_nested_item(&mut self, _: ItemId) -> ControlFlow<()> {
|
||||
ControlFlow::Continue(())
|
||||
}
|
||||
}
|
||||
let mut v = V { f, res: None };
|
||||
node.visit(&mut v);
|
||||
|
Loading…
Reference in New Issue
Block a user