From 1b2f2be085d9723fd72bd033b6fe81d8aa31c071 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sun, 14 Apr 2019 13:09:17 -0700 Subject: [PATCH 1/4] Remove now-unnecessary calls to node_to_hir_id --- clippy_lints/src/functions.rs | 2 +- clippy_lints/src/let_if_seq.rs | 4 ++-- clippy_lints/src/loops.rs | 16 +++++++--------- .../src/methods/unnecessary_filter_map.rs | 2 +- clippy_lints/src/utils/mod.rs | 2 +- clippy_lints/src/utils/usage.rs | 2 +- 6 files changed, 13 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index 307f03c69280..d657bb6a0b4e 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -327,7 +327,7 @@ impl<'a, 'tcx: 'a> DerefVisitor<'a, 'tcx> { fn check_arg(&self, ptr: &hir::Expr) { if let hir::ExprKind::Path(ref qpath) = ptr.node { if let Def::Local(id) = self.cx.tables.qpath_def(qpath, ptr.hir_id) { - if self.ptrs.contains(&self.cx.tcx.hir().node_to_hir_id(id)) { + if self.ptrs.contains(&id) { span_lint( self.cx, NOT_UNSAFE_PTR_ARG_DEREF, diff --git a/clippy_lints/src/let_if_seq.rs b/clippy_lints/src/let_if_seq.rs index 5ef3d8dd4abf..1f454c4983a9 100644 --- a/clippy_lints/src/let_if_seq.rs +++ b/clippy_lints/src/let_if_seq.rs @@ -150,7 +150,7 @@ impl<'a, 'tcx> hir::intravisit::Visitor<'tcx> for UsedVisitor<'a, 'tcx> { 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 self.id == self.cx.tcx.hir().node_to_hir_id(local_id); + if self.id == local_id; then { self.used = true; return; @@ -175,7 +175,7 @@ fn check_assign<'a, 'tcx>( 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 decl == cx.tcx.hir().node_to_hir_id(local_id); + if decl == local_id; then { let mut v = UsedVisitor { cx, diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 11b4d237c560..ccd8be38868c 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -790,7 +790,7 @@ fn same_var<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &Expr, var: HirId) -> bo if path.segments.len() == 1; if let Def::Local(local_id) = cx.tables.qpath_def(qpath, expr.hir_id); // our variable! - if cx.tcx.hir().node_to_hir_id(local_id) == var; + if local_id == var; then { return true; } @@ -1663,7 +1663,7 @@ fn check_for_mutability(cx: &LateContext<'_, '_>, bound: &Expr) -> Option if let PatKind::Binding(bind_ann, ..) = pat.node; if let BindingAnnotation::Mutable = bind_ann; then { - return Some(cx.tcx.hir().node_to_hir_id(node_id)); + return Some(node_id); } } } @@ -1792,9 +1792,7 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> { } let def = self.cx.tables.qpath_def(seqpath, seqexpr.hir_id); match def { - Def::Local(node_id) | Def::Upvar(node_id, ..) => { - let hir_id = self.cx.tcx.hir().node_to_hir_id(node_id); - + Def::Local(hir_id) | Def::Upvar(hir_id, ..) => { let parent_id = self.cx.tcx.hir().get_parent_item(expr.hir_id); let parent_def_id = self.cx.tcx.hir().local_def_id_from_hir_id(parent_id); let extent = self.cx.tcx.region_scope_tree(parent_def_id).var_scope(hir_id.local_id); @@ -1856,7 +1854,7 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> { then { match self.cx.tables.qpath_def(qpath, expr.hir_id) { Def::Upvar(local_id, ..) => { - if self.cx.tcx.hir().node_to_hir_id(local_id) == self.var { + if local_id == self.var { // we are not indexing anything, record that self.nonindex = true; } @@ -1864,7 +1862,7 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> { Def::Local(local_id) => { - if self.cx.tcx.hir().node_to_hir_id(local_id) == self.var { + if local_id == self.var { self.nonindex = true; } else { // not the correct variable, but still a variable @@ -2209,7 +2207,7 @@ fn var_def_id(cx: &LateContext<'_, '_>, expr: &Expr) -> Option { if let ExprKind::Path(ref qpath) = expr.node { let path_res = cx.tables.qpath_def(qpath, expr.hir_id); if let Def::Local(node_id) = path_res { - return Some(cx.tcx.hir().node_to_hir_id(node_id)); + return Some(node_id); } } None @@ -2404,7 +2402,7 @@ impl<'a, 'tcx> VarCollectorVisitor<'a, 'tcx> { then { match def { Def::Local(node_id) | Def::Upvar(node_id, ..) => { - self.ids.insert(self.cx.tcx.hir().node_to_hir_id(node_id)); + self.ids.insert(node_id); }, Def::Static(def_id, mutable) => { self.def_ids.insert(def_id, mutable); diff --git a/clippy_lints/src/methods/unnecessary_filter_map.rs b/clippy_lints/src/methods/unnecessary_filter_map.rs index c081384db4b8..c751de865bee 100644 --- a/clippy_lints/src/methods/unnecessary_filter_map.rs +++ b/clippy_lints/src/methods/unnecessary_filter_map.rs @@ -68,7 +68,7 @@ fn check_expression<'a, 'tcx: 'a>( if let hir::ExprKind::Path(path) = &args[0].node; if let Def::Local(ref local) = cx.tables.qpath_def(path, args[0].hir_id); then { - if arg_id == cx.tcx.hir().node_to_hir_id(*local) { + if arg_id == *local { return (false, false) } } diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 3383e47ac776..e5ce17cf1927 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -958,7 +958,7 @@ pub fn is_try<'a>(cx: &'_ LateContext<'_, '_>, expr: &'a Expr) -> Option<&'a Exp if let PatKind::Binding(_, hir_id, _, None) = pat[0].node; if let ExprKind::Path(QPath::Resolved(None, ref path)) = arm.body.node; if let Def::Local(lid) = path.def; - if cx.tcx.hir().node_to_hir_id(lid) == hir_id; + if lid == hir_id; then { return true; } diff --git a/clippy_lints/src/utils/usage.rs b/clippy_lints/src/utils/usage.rs index 521ddbc08656..14711b7fe1b4 100644 --- a/clippy_lints/src/utils/usage.rs +++ b/clippy_lints/src/utils/usage.rs @@ -33,7 +33,7 @@ pub fn is_potentially_mutated<'a, 'tcx: 'a>( Def::Local(id) | Def::Upvar(id, ..) => id, _ => return true, }; - mutated_variables(expr, cx).map_or(true, |mutated| mutated.contains(&cx.tcx.hir().node_to_hir_id(id))) + mutated_variables(expr, cx).map_or(true, |mutated| mutated.contains(&id)) } struct MutVarsDelegate { From 0c6956f8ce9e35b96b20d5cdc40bacf9f37322ff Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sun, 14 Apr 2019 13:14:17 -0700 Subject: [PATCH 2/4] Use _from_hir_id APIs --- clippy_lints/src/loops.rs | 2 +- clippy_lints/src/misc.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index ccd8be38868c..a070e824cc0d 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -1657,7 +1657,7 @@ fn check_for_mutability(cx: &LateContext<'_, '_>, bound: &Expr) -> Option then { let def = cx.tables.qpath_def(qpath, bound.hir_id); if let Def::Local(node_id) = def { - let node_str = cx.tcx.hir().get(node_id); + let node_str = cx.tcx.hir().get_by_hir_id(node_id); if_chain! { if let Node::Binding(pat) = node_str; if let PatKind::Binding(bind_ann, ..) = pat.node; diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index a1e0f9d8345a..bd0b4bd4515a 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -612,7 +612,7 @@ fn in_attributes_expansion(expr: &Expr) -> bool { /// Tests whether `def` is a variable defined outside a macro. fn non_macro_local(cx: &LateContext<'_, '_>, def: &def::Def) -> bool { match *def { - def::Def::Local(id) | def::Def::Upvar(id, _, _) => !in_macro(cx.tcx.hir().span(id)), + def::Def::Local(id) | def::Def::Upvar(id, _, _) => !in_macro(cx.tcx.hir().span_by_hir_id(id)), _ => false, } } From 3c93a95b1f72c5a247ac8f20d9aeecc597eb4f60 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sun, 14 Apr 2019 13:18:34 -0700 Subject: [PATCH 3/4] HirIdify ReadVisitor --- clippy_lints/src/eval_order_dependence.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clippy_lints/src/eval_order_dependence.rs b/clippy_lints/src/eval_order_dependence.rs index 90e6102dd844..2a4d404ccc7f 100644 --- a/clippy_lints/src/eval_order_dependence.rs +++ b/clippy_lints/src/eval_order_dependence.rs @@ -5,7 +5,6 @@ use rustc::hir::*; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::ty; use rustc::{declare_tool_lint, lint_array}; -use syntax::ast; declare_clippy_lint! { /// **What it does:** Checks for a read and a write to the same variable where @@ -287,7 +286,7 @@ fn check_stmt<'a, 'tcx>(vis: &mut ReadVisitor<'a, 'tcx>, stmt: &'tcx Stmt) -> St struct ReadVisitor<'a, 'tcx: 'a> { cx: &'a LateContext<'a, 'tcx>, /// The ID of the variable we're looking for. - var: ast::NodeId, + var: HirId, /// The expressions where the write to the variable occurred (for reporting /// in the lint). write_expr: &'tcx Expr, From 2156f6733e9bee83be265d35447bb25091d7e9b2 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sun, 14 Apr 2019 13:19:33 -0700 Subject: [PATCH 4/4] Clean up unused cx parameters --- clippy_lints/src/unused_io_amount.rs | 2 +- clippy_lints/src/utils/mod.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/unused_io_amount.rs b/clippy_lints/src/unused_io_amount.rs index bf31b774bba2..89f6873565a5 100644 --- a/clippy_lints/src/unused_io_amount.rs +++ b/clippy_lints/src/unused_io_amount.rs @@ -50,7 +50,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedIoAmount { }; match expr.node { - hir::ExprKind::Match(ref res, _, _) if is_try(cx, expr).is_some() => { + hir::ExprKind::Match(ref res, _, _) if is_try(expr).is_some() => { if let hir::ExprKind::Call(ref func, ref args) = res.node { if let hir::ExprKind::Path(ref path) = func.node { if match_qpath(path, &paths::TRY_INTO_RESULT) && args.len() == 1 { diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index e5ce17cf1927..46aec1fa343f 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -950,8 +950,8 @@ pub fn iter_input_pats<'tcx>(decl: &FnDecl, body: &'tcx Body) -> impl Iterator(cx: &'_ LateContext<'_, '_>, expr: &'a Expr) -> Option<&'a Expr> { - fn is_ok(cx: &'_ LateContext<'_, '_>, arm: &Arm) -> bool { +pub fn is_try(expr: &Expr) -> Option<&Expr> { + fn is_ok(arm: &Arm) -> bool { if_chain! { if let PatKind::TupleStruct(ref path, ref pat, None) = arm.pats[0].node; if match_qpath(path, &paths::RESULT_OK[1..]); @@ -984,8 +984,8 @@ pub fn is_try<'a>(cx: &'_ LateContext<'_, '_>, expr: &'a Expr) -> Option<&'a Exp if arms.len() == 2; if arms[0].pats.len() == 1 && arms[0].guard.is_none(); if arms[1].pats.len() == 1 && arms[1].guard.is_none(); - if (is_ok(cx, &arms[0]) && is_err(&arms[1])) || - (is_ok(cx, &arms[1]) && is_err(&arms[0])); + if (is_ok(&arms[0]) && is_err(&arms[1])) || + (is_ok(&arms[1]) && is_err(&arms[0])); then { return Some(expr); }