X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Flet_if_seq.rs;h=a7dae9497db4b2e4664f78156e929104cf06cc01;hb=e5a5b0a0774625eebbe7b29c67b49dc6431544d1;hp=53d13407be3511a682714033632738d9a4621b58;hpb=af2d6a0c146a04df486007c3fe9aaf1fafbe6e99;p=rust.git diff --git a/clippy_lints/src/let_if_seq.rs b/clippy_lints/src/let_if_seq.rs index 53d13407be3..a7dae9497db 100644 --- a/clippy_lints/src/let_if_seq.rs +++ b/clippy_lints/src/let_if_seq.rs @@ -1,66 +1,59 @@ -use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; -use crate::rustc::{declare_tool_lint, lint_array}; +use crate::utils::{higher, qpath_res, snippet, span_lint_and_then}; use if_chain::if_chain; -use crate::rustc::hir; -use crate::rustc::hir::BindingAnnotation; -use crate::rustc::hir::def::Def; -use crate::syntax::ast; -use crate::utils::{snippet, span_lint_and_then}; -use crate::rustc_errors::Applicability; - -/// **What it does:** Checks for variable declarations immediately followed by a -/// conditional affectation. -/// -/// **Why is this bad?** This is not idiomatic Rust. -/// -/// **Known problems:** None. -/// -/// **Example:** -/// ```rust,ignore -/// let foo; -/// -/// if bar() { -/// foo = 42; -/// } else { -/// foo = 0; -/// } -/// -/// let mut baz = None; -/// -/// if bar() { -/// baz = Some(42); -/// } -/// ``` -/// -/// should be written -/// -/// ```rust,ignore -/// let foo = if bar() { -/// 42 -/// } else { -/// 0 -/// }; -/// -/// let baz = if bar() { -/// Some(42) -/// } else { -/// None -/// }; -/// ``` +use rustc::declare_lint_pass; +use rustc::hir; +use rustc::hir::def::Res; +use rustc::hir::BindingAnnotation; +use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; +use rustc_errors::Applicability; +use rustc_session::declare_tool_lint; + declare_clippy_lint! { + /// **What it does:** Checks for variable declarations immediately followed by a + /// conditional affectation. + /// + /// **Why is this bad?** This is not idiomatic Rust. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust,ignore + /// let foo; + /// + /// if bar() { + /// foo = 42; + /// } else { + /// foo = 0; + /// } + /// + /// let mut baz = None; + /// + /// if bar() { + /// baz = Some(42); + /// } + /// ``` + /// + /// should be written + /// + /// ```rust,ignore + /// let foo = if bar() { + /// 42 + /// } else { + /// 0 + /// }; + /// + /// let baz = if bar() { + /// Some(42) + /// } else { + /// None + /// }; + /// ``` pub USELESS_LET_IF_SEQ, style, "unidiomatic `let mut` declaration followed by initialization in `if`" } -#[derive(Copy, Clone)] -pub struct LetIfSeq; - -impl LintPass for LetIfSeq { - fn get_lints(&self) -> LintArray { - lint_array!(USELESS_LET_IF_SEQ) - } -} +declare_lint_pass!(LetIfSeq => [USELESS_LET_IF_SEQ]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetIfSeq { fn check_block(&mut self, cx: &LateContext<'a, 'tcx>, block: &'tcx hir::Block) { @@ -68,23 +61,29 @@ fn check_block(&mut self, cx: &LateContext<'a, 'tcx>, block: &'tcx hir::Block) { while let Some(stmt) = it.next() { if_chain! { if let Some(expr) = it.peek(); - if let hir::StmtKind::Decl(ref decl, _) = stmt.node; - if let hir::DeclKind::Local(ref decl) = decl.node; - if let hir::PatKind::Binding(mode, canonical_id, ident, None) = decl.pat.node; - if let hir::StmtKind::Expr(ref if_, _) = expr.node; - if let hir::ExprKind::If(ref cond, ref then, ref else_) = if_.node; + if let hir::StmtKind::Local(ref local) = stmt.kind; + if let hir::PatKind::Binding(mode, canonical_id, ident, None) = local.pat.kind; + if let hir::StmtKind::Expr(ref if_) = expr.kind; + if let Some((ref cond, ref then, ref else_)) = higher::if_block(&if_); if !used_in_expr(cx, canonical_id, cond); - if let hir::ExprKind::Block(ref then, _) = then.node; + if let hir::ExprKind::Block(ref then, _) = then.kind; if let Some(value) = check_assign(cx, canonical_id, &*then); if !used_in_expr(cx, canonical_id, value); then { let span = stmt.span.to(if_.span); + let has_interior_mutability = !cx.tables.node_type(canonical_id).is_freeze( + cx.tcx, + cx.param_env, + span + ); + if has_interior_mutability { return; } + let (default_multi_stmts, default) = if let Some(ref else_) = *else_ { - if let hir::ExprKind::Block(ref else_, _) = else_.node { + if let hir::ExprKind::Block(ref else_, _) = else_.kind { if let Some(default) = check_assign(cx, canonical_id, else_) { (else_.stmts.len() > 1, default) - } else if let Some(ref default) = decl.init { + } else if let Some(ref default) = local.init { (true, &**default) } else { continue; @@ -92,7 +91,7 @@ fn check_block(&mut self, cx: &LateContext<'a, 'tcx>, block: &'tcx hir::Block) { } else { continue; } - } else if let Some(ref default) = decl.init { + } else if let Some(ref default) = local.init { (false, &**default) } else { continue; @@ -121,7 +120,7 @@ fn check_block(&mut self, cx: &LateContext<'a, 'tcx>, block: &'tcx hir::Block) { span, "`if _ { .. } else { .. }` is an expression", |db| { - db.span_suggestion_with_applicability( + db.span_suggestion( span, "it is more idiomatic to write", sug, @@ -137,17 +136,17 @@ fn check_block(&mut self, cx: &LateContext<'a, 'tcx>, block: &'tcx hir::Block) { } } -struct UsedVisitor<'a, 'tcx: 'a> { +struct UsedVisitor<'a, 'tcx> { cx: &'a LateContext<'a, 'tcx>, - id: ast::NodeId, + id: hir::HirId, used: bool, } impl<'a, 'tcx> hir::intravisit::Visitor<'tcx> for UsedVisitor<'a, 'tcx> { fn visit_expr(&mut self, expr: &'tcx hir::Expr) { if_chain! { - if let hir::ExprKind::Path(ref qpath) = expr.node; - if let Def::Local(local_id) = self.cx.tables.qpath_def(qpath, expr.hir_id); + if let hir::ExprKind::Path(ref qpath) = expr.kind; + if let Res::Local(local_id) = qpath_res(self.cx, qpath, expr.hir_id); if self.id == local_id; then { self.used = true; @@ -163,16 +162,16 @@ fn nested_visit_map<'this>(&'this mut self) -> hir::intravisit::NestedVisitorMap fn check_assign<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, - decl: ast::NodeId, + decl: hir::HirId, block: &'tcx hir::Block, ) -> Option<&'tcx hir::Expr> { if_chain! { if block.expr.is_none(); if let Some(expr) = block.stmts.iter().last(); - if let hir::StmtKind::Semi(ref expr, _) = expr.node; - if let hir::ExprKind::Assign(ref var, ref value) = expr.node; - if let hir::ExprKind::Path(ref qpath) = var.node; - if let Def::Local(local_id) = cx.tables.qpath_def(qpath, var.hir_id); + 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) = qpath_res(cx, qpath, var.hir_id); if decl == local_id; then { let mut v = UsedVisitor { @@ -196,12 +195,8 @@ fn check_assign<'a, 'tcx>( None } -fn used_in_expr<'a, 'tcx: 'a>(cx: &LateContext<'a, 'tcx>, id: ast::NodeId, expr: &'tcx hir::Expr) -> bool { - let mut v = UsedVisitor { - cx, - id, - used: false, - }; +fn used_in_expr<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, id: hir::HirId, expr: &'tcx hir::Expr) -> bool { + let mut v = UsedVisitor { cx, id, used: false }; hir::intravisit::walk_expr(&mut v, expr); v.used }