Merge commit 'b40ea209e7f14c8193ddfc98143967b6a2f4f5c9' into clippyup

9 files changed, 142 insertions, 93 deletions

diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs
index 8d28421d70d..7ec8452bf4c 100644
--- a/clippy_utils/src/attrs.rs
+++ b/clippy_utils/src/attrs.rs
@@ -1,4 +1,4 @@
-use rustc_ast::ast;
+use rustc_ast::{ast, attr};
 use rustc_errors::Applicability;
 use rustc_session::Session;
 use rustc_span::sym;
@@ -148,3 +148,13 @@ pub fn get_unique_inner_attr(sess: &Session, attrs: &[ast::Attribute], name: &'s
 pub fn is_proc_macro(sess: &Session, attrs: &[ast::Attribute]) -> bool {
     attrs.iter().any(|attr| sess.is_proc_macro_attr(attr))
 }
+
+/// Return true if the attributes contain `#[doc(hidden)]`
+pub fn is_doc_hidden(attrs: &[ast::Attribute]) -> bool {
+    #[allow(clippy::filter_map)]
+    attrs
+        .iter()
+        .filter(|attr| attr.has_name(sym::doc))
+        .flat_map(ast::Attribute::meta_item_list)
+        .any(|l| attr::list_contains_name(&l, sym::hidden))
+}
diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs
index 8af10ebe777..2a305d8bcbe 100644
--- a/clippy_utils/src/consts.rs
+++ b/clippy_utils/src/consts.rs
@@ -140,12 +140,10 @@ impl Constant {
             (&Self::F64(l), &Self::F64(r)) => l.partial_cmp(&r),
             (&Self::F32(l), &Self::F32(r)) => l.partial_cmp(&r),
             (&Self::Bool(ref l), &Self::Bool(ref r)) => Some(l.cmp(r)),
-            (&Self::Tuple(ref l), &Self::Tuple(ref r)) | (&Self::Vec(ref l), &Self::Vec(ref r)) => {
-                iter::zip(l, r)
-                    .map(|(li, ri)| Self::partial_cmp(tcx, cmp_type, li, ri))
-                    .find(|r| r.map_or(true, |o| o != Ordering::Equal))
-                    .unwrap_or_else(|| Some(l.len().cmp(&r.len())))
-            }
+            (&Self::Tuple(ref l), &Self::Tuple(ref r)) | (&Self::Vec(ref l), &Self::Vec(ref r)) => iter::zip(l, r)
+                .map(|(li, ri)| Self::partial_cmp(tcx, cmp_type, li, ri))
+                .find(|r| r.map_or(true, |o| o != Ordering::Equal))
+                .unwrap_or_else(|| Some(l.len().cmp(&r.len()))),
             (&Self::Repeat(ref lv, ref ls), &Self::Repeat(ref rv, ref rs)) => {
                 match Self::partial_cmp(tcx, cmp_type, lv, rv) {
                     Some(Equal) => Some(ls.cmp(rs)),
diff --git a/clippy_utils/src/diagnostics.rs b/clippy_utils/src/diagnostics.rs
index e9c9cb12b9d..7f827f1759d 100644
--- a/clippy_utils/src/diagnostics.rs
+++ b/clippy_utils/src/diagnostics.rs
@@ -133,9 +133,11 @@ pub fn span_lint_and_note<'a, T: LintContext>(
 ///
 /// If you need to customize your lint output a lot, use this function.
 /// If you change the signature, remember to update the internal lint `CollapsibleCalls`
-pub fn span_lint_and_then<'a, T: LintContext, F>(cx: &'a T, lint: &'static Lint, sp: Span, msg: &str, f: F)
+pub fn span_lint_and_then<C, S, F>(cx: &C, lint: &'static Lint, sp: S, msg: &str, f: F)
 where
-    F: for<'b> FnOnce(&mut DiagnosticBuilder<'b>),
+    C: LintContext,
+    S: Into<MultiSpan>,
+    F: FnOnce(&mut DiagnosticBuilder<'_>),
 {
     cx.struct_span_lint(lint, sp, |diag| {
         let mut diag = diag.build(msg);
@@ -216,6 +218,11 @@ where
     multispan_sugg_with_applicability(diag, help_msg, Applicability::Unspecified, sugg)
 }
 
+/// Create a suggestion made from several `span → replacement`.
+///
+/// rustfix currently doesn't support the automatic application of suggestions with
+/// multiple spans. This is tracked in issue [rustfix#141](https://github.com/rust-lang/rustfix/issues/141).
+/// Suggestions with multiple spans will be silently ignored.
 pub fn multispan_sugg_with_applicability<I>(
     diag: &mut DiagnosticBuilder<'_>,
     help_msg: &str,
diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs
index b30c0b79881..f695f1a61e7 100644
--- a/clippy_utils/src/hir_utils.rs
+++ b/clippy_utils/src/hir_utils.rs
@@ -2,9 +2,9 @@ use crate::consts::{constant_context, constant_simple};
 use crate::differing_macro_contexts;
 use crate::source::snippet_opt;
 use rustc_ast::ast::InlineAsmTemplatePiece;
-use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_hir::def::Res;
+use rustc_hir::HirIdMap;
 use rustc_hir::{
     BinOpKind, Block, BlockCheckMode, BodyId, BorrowKind, CaptureBy, Expr, ExprField, ExprKind, FnRetTy, GenericArg,
     GenericArgs, Guard, HirId, InlineAsmOperand, Lifetime, LifetimeName, ParamName, Pat, PatField, PatKind, Path,
@@ -58,13 +58,14 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
 
     /// Use this method to wrap comparisons that may involve inter-expression context.
     /// See `self.locals`.
-    fn inter_expr(&mut self) -> HirEqInterExpr<'_, 'a, 'tcx> {
+    pub fn inter_expr(&mut self) -> HirEqInterExpr<'_, 'a, 'tcx> {
         HirEqInterExpr {
             inner: self,
-            locals: FxHashMap::default(),
+            locals: HirIdMap::default(),
         }
     }
 
+    #[allow(dead_code)]
     pub fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool {
         self.inter_expr().eq_block(left, right)
     }
@@ -82,22 +83,24 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
     }
 }
 
-struct HirEqInterExpr<'a, 'b, 'tcx> {
+pub struct HirEqInterExpr<'a, 'b, 'tcx> {
     inner: &'a mut SpanlessEq<'b, 'tcx>,
 
     // When binding are declared, the binding ID in the left expression is mapped to the one on the
     // right. For example, when comparing `{ let x = 1; x + 2 }` and `{ let y = 1; y + 2 }`,
     // these blocks are considered equal since `x` is mapped to `y`.
-    locals: FxHashMap<HirId, HirId>,
+    locals: HirIdMap<HirId>,
 }
 
 impl HirEqInterExpr<'_, '_, '_> {
-    fn eq_stmt(&mut self, left: &Stmt<'_>, right: &Stmt<'_>) -> bool {
+    pub fn eq_stmt(&mut self, left: &Stmt<'_>, right: &Stmt<'_>) -> bool {
         match (&left.kind, &right.kind) {
             (&StmtKind::Local(ref l), &StmtKind::Local(ref r)) => {
-                self.eq_pat(&l.pat, &r.pat)
+                // eq_pat adds the HirIds to the locals map. We therefor call it last to make sure that
+                // these only get added if the init and type is equal.
+                both(&l.init, &r.init, |l, r| self.eq_expr(l, r))
                     && both(&l.ty, &r.ty, |l, r| self.eq_ty(l, r))
-                    && both(&l.init, &r.init, |l, r| self.eq_expr(l, r))
+                    && self.eq_pat(&l.pat, &r.pat)
             },
             (&StmtKind::Expr(ref l), &StmtKind::Expr(ref r)) | (&StmtKind::Semi(ref l), &StmtKind::Semi(ref r)) => {
                 self.eq_expr(l, r)
@@ -159,7 +162,7 @@ impl HirEqInterExpr<'_, '_, '_> {
     }
 
     #[allow(clippy::similar_names)]
-    fn eq_expr(&mut self, left: &Expr<'_>, right: &Expr<'_>) -> bool {
+    pub fn eq_expr(&mut self, left: &Expr<'_>, right: &Expr<'_>) -> bool {
         if !self.inner.allow_side_effects && differing_macro_contexts(left.span, right.span) {
             return false;
         }
@@ -483,6 +486,15 @@ pub fn over<X>(left: &[X], right: &[X], mut eq_fn: impl FnMut(&X, &X) -> bool) -
     left.len() == right.len() && left.iter().zip(right).all(|(x, y)| eq_fn(x, y))
 }
 
+/// Counts how many elements of the slices are equal as per `eq_fn`.
+pub fn count_eq<X: Sized>(
+    left: &mut dyn Iterator<Item = X>,
+    right: &mut dyn Iterator<Item = X>,
+    mut eq_fn: impl FnMut(&X, &X) -> bool,
+) -> usize {
+    left.zip(right).take_while(|(l, r)| eq_fn(l, r)).count()
+}
+
 /// Checks if two expressions evaluate to the same value, and don't contain any side effects.
 pub fn eq_expr_value(cx: &LateContext<'_>, left: &Expr<'_>, right: &Expr<'_>) -> bool {
     SpanlessEq::new(cx).deny_side_effects().eq_expr(left, right)
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index 6b235875c20..c847712ec2e 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -48,7 +48,7 @@ pub mod usage;
 pub mod visitors;
 
 pub use self::attrs::*;
-pub use self::hir_utils::{both, eq_expr_value, over, SpanlessEq, SpanlessHash};
+pub use self::hir_utils::{both, count_eq, eq_expr_value, over, SpanlessEq, SpanlessHash};
 
 use std::collections::hash_map::Entry;
 use std::hash::BuildHasherDefault;
@@ -61,10 +61,9 @@ use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
 use rustc_hir::def_id::{DefId, LOCAL_CRATE};
 use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
 use rustc_hir::{
-    def, Arm, BindingAnnotation, Block, Body, Constness, Expr, ExprKind, FieldDef, FnDecl, ForeignItem, GenericArgs,
-    GenericParam, HirId, Impl, ImplItem, ImplItemKind, Item, ItemKind, LangItem, Lifetime, Local, MacroDef,
-    MatchSource, Mod, Node, Param, Pat, PatKind, Path, PathSegment, QPath, Stmt, TraitItem, TraitItemKind, TraitRef,
-    TyKind, Variant, Visibility,
+    def, Arm, BindingAnnotation, Block, Body, Constness, Expr, ExprKind, FnDecl, GenericArgs, HirId, Impl, ImplItem,
+    ImplItemKind, Item, ItemKind, LangItem, MatchSource, Node, Param, Pat, PatKind, Path, PathSegment, QPath,
+    TraitItem, TraitItemKind, TraitRef, TyKind,
 };
 use rustc_lint::{LateContext, Level, Lint, LintContext};
 use rustc_middle::hir::exports::Export;
@@ -76,7 +75,7 @@ use rustc_session::Session;
 use rustc_span::hygiene::{ExpnKind, MacroKind};
 use rustc_span::source_map::original_sp;
 use rustc_span::sym;
-use rustc_span::symbol::{kw, Ident, Symbol};
+use rustc_span::symbol::{kw, Symbol};
 use rustc_span::{Span, DUMMY_SP};
 use rustc_target::abi::Integer;
 
@@ -350,6 +349,10 @@ pub fn single_segment_path<'tcx>(path: &QPath<'tcx>) -> Option<&'tcx PathSegment
     }
 }
 
+/// THIS METHOD IS DEPRECATED and will eventually be removed since it does not match against the
+/// entire path or resolved `DefId`. Prefer using `match_def_path`. Consider getting a `DefId` from
+/// `QPath::Resolved.1.res.opt_def_id()`.
+///
 /// Matches a `QPath` against a slice of segment string literals.
 ///
 /// There is also `match_path` if you are dealing with a `rustc_hir::Path` instead of a
@@ -377,6 +380,10 @@ pub fn match_qpath(path: &QPath<'_>, segments: &[&str]) -> bool {
     }
 }
 
+/// THIS METHOD IS DEPRECATED and will eventually be removed since it does not match against the
+/// entire path or resolved `DefId`. Prefer using `match_def_path`. Consider getting a `DefId` from
+/// `QPath::Resolved.1.res.opt_def_id()`.
+///
 /// Matches a `Path` against a slice of segment string literals.
 ///
 /// There is also `match_qpath` if you are dealing with a `rustc_hir::QPath` instead of a
@@ -610,9 +617,9 @@ pub fn get_pat_name(pat: &Pat<'_>) -> Option<Symbol> {
     }
 }
 
-struct ContainsName {
-    name: Symbol,
-    result: bool,
+pub struct ContainsName {
+    pub name: Symbol,
+    pub result: bool,
 }
 
 impl<'tcx> Visitor<'tcx> for ContainsName {
@@ -713,41 +720,6 @@ fn line_span<T: LintContext>(cx: &T, span: Span) -> Span {
     Span::new(line_start, span.hi(), span.ctxt())
 }
 
-/// Gets the span of the node, if there is one.
-pub fn get_node_span(node: Node<'_>) -> Option<Span> {
-    match node {
-        Node::Param(Param { span, .. })
-        | Node::Item(Item { span, .. })
-        | Node::ForeignItem(ForeignItem { span, .. })
-        | Node::TraitItem(TraitItem { span, .. })
-        | Node::ImplItem(ImplItem { span, .. })
-        | Node::Variant(Variant { span, .. })
-        | Node::Field(FieldDef { span, .. })
-        | Node::Expr(Expr { span, .. })
-        | Node::Stmt(Stmt { span, .. })
-        | Node::PathSegment(PathSegment {
-            ident: Ident { span, .. },
-            ..
-        })
-        | Node::Ty(hir::Ty { span, .. })
-        | Node::TraitRef(TraitRef {
-            path: Path { span, .. },
-            ..
-        })
-        | Node::Binding(Pat { span, .. })
-        | Node::Pat(Pat { span, .. })
-        | Node::Arm(Arm { span, .. })
-        | Node::Block(Block { span, .. })
-        | Node::Local(Local { span, .. })
-        | Node::MacroDef(MacroDef { span, .. })
-        | Node::Lifetime(Lifetime { span, .. })
-        | Node::GenericParam(GenericParam { span, .. })
-        | Node::Visibility(Visibility { span, .. })
-        | Node::Crate(Mod { inner: span, .. }) => Some(*span),
-        Node::Ctor(_) | Node::AnonConst(_) => None,
-    }
-}
-
 /// Gets the parent node, if any.
 pub fn get_parent_node(tcx: TyCtxt<'_>, id: HirId) -> Option<Node<'_>> {
     tcx.hir().parent_iter(id).next().map(|(_, node)| node)
@@ -798,22 +770,29 @@ pub fn get_parent_as_impl(tcx: TyCtxt<'_>, id: HirId) -> Option<&Impl<'_>> {
     }
 }
 
-/// Checks if the given expression is the else clause in the expression `if let .. {} else {}`
-pub fn is_else_clause_of_if_let_else(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool {
+/// Checks if the given expression is the else clause of either an `if` or `if let` expression.
+pub fn is_else_clause(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool {
     let map = tcx.hir();
     let mut iter = map.parent_iter(expr.hir_id);
-    let arm_id = match iter.next() {
-        Some((id, Node::Arm(..))) => id,
-        _ => return false,
-    };
     match iter.next() {
+        Some((arm_id, Node::Arm(..))) => matches!(
+            iter.next(),
+            Some((
+                _,
+                Node::Expr(Expr {
+                    kind: ExprKind::Match(_, [_, else_arm], MatchSource::IfLetDesugar { .. }),
+                    ..
+                })
+            ))
+            if else_arm.hir_id == arm_id
+        ),
         Some((
             _,
             Node::Expr(Expr {
-                kind: ExprKind::Match(_, [_, else_arm], kind),
+                kind: ExprKind::If(_, _, Some(else_expr)),
                 ..
             }),
-        )) => else_arm.hir_id == arm_id && matches!(kind, MatchSource::IfLetDesugar { .. }),
+        )) => else_expr.hir_id == expr.hir_id,
         _ => false,
     }
 }
@@ -1210,6 +1189,8 @@ pub fn if_sequence<'tcx>(mut expr: &'tcx Expr<'tcx>) -> (Vec<&'tcx Expr<'tcx>>,
     (conds, blocks)
 }
 
+/// This function returns true if the given expression is the `else` or `if else` part of an if
+/// statement
 pub fn parent_node_is_if_expr(expr: &Expr<'_>, cx: &LateContext<'_>) -> bool {
     let map = cx.tcx.hir();
     let parent_id = map.get_parent_node(expr.hir_id);
@@ -1223,16 +1204,9 @@ pub fn parent_node_is_if_expr(expr: &Expr<'_>, cx: &LateContext<'_>) -> bool {
     )
 }
 
-// Finds the attribute with the given name, if any
-pub fn attr_by_name<'a>(attrs: &'a [Attribute], name: &'_ str) -> Option<&'a Attribute> {
-    attrs
-        .iter()
-        .find(|attr| attr.ident().map_or(false, |ident| ident.as_str() == name))
-}
-
 // Finds the `#[must_use]` attribute, if any
 pub fn must_use_attr(attrs: &[Attribute]) -> Option<&Attribute> {
-    attr_by_name(attrs, "must_use")
+    attrs.iter().find(|a| a.has_name(sym::must_use))
 }
 
 // check if expr is calling method or function with #[must_use] attribute
@@ -1320,6 +1294,16 @@ pub fn fn_def_id(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<DefId> {
     }
 }
 
+/// This function checks if any of the lints in the slice is enabled for the provided `HirId`.
+/// A lint counts as enabled with any of the levels: `Level::Forbid` | `Level::Deny` | `Level::Warn`
+///
+/// ```ignore
+/// #[deny(clippy::YOUR_AWESOME_LINT)]
+/// println!("Hello, World!"); // <- Clippy code: run_lints(cx, &[YOUR_AWESOME_LINT], id) == true
+///
+/// #[allow(clippy::YOUR_AWESOME_LINT)]
+/// println!("See you soon!"); // <- Clippy code: run_lints(cx, &[YOUR_AWESOME_LINT], id) == false
+/// ```
 pub fn run_lints(cx: &LateContext<'_>, lints: &[&'static Lint], id: HirId) -> bool {
     lints.iter().any(|lint| {
         matches!(
diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs
index 11a446e42a4..3b4c4070c0e 100644
--- a/clippy_utils/src/paths.rs
+++ b/clippy_utils/src/paths.rs
@@ -26,6 +26,7 @@ pub const DEFAULT_TRAIT: [&str; 3] = ["core", "default", "Default"];
 pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"];
 pub const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"];
 pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"];
+pub const DIR_BUILDER: [&str; 3] = ["std", "fs", "DirBuilder"];
 pub const DISPLAY_FMT_METHOD: [&str; 4] = ["core", "fmt", "Display", "fmt"];
 pub const DISPLAY_TRAIT: [&str; 3] = ["core", "fmt", "Display"];
 pub const DOUBLE_ENDED_ITERATOR: [&str; 4] = ["core", "iter", "traits", "DoubleEndedIterator"];
@@ -92,9 +93,10 @@ pub(super) const PANIC_ANY: [&str; 3] = ["std", "panic", "panic_any"];
 pub const PARKING_LOT_MUTEX_GUARD: [&str; 2] = ["parking_lot", "MutexGuard"];
 pub const PARKING_LOT_RWLOCK_READ_GUARD: [&str; 2] = ["parking_lot", "RwLockReadGuard"];
 pub const PARKING_LOT_RWLOCK_WRITE_GUARD: [&str; 2] = ["parking_lot", "RwLockWriteGuard"];
-pub const PATH_BUF: [&str; 3] = ["std", "path", "PathBuf"];
 pub const PATH_BUF_AS_PATH: [&str; 4] = ["std", "path", "PathBuf", "as_path"];
 pub const PATH_TO_PATH_BUF: [&str; 4] = ["std", "path", "Path", "to_path_buf"];
+pub const PERMISSIONS: [&str; 3] = ["std", "fs", "Permissions"];
+pub const PERMISSIONS_FROM_MODE: [&str; 7] = ["std", "sys", "unix", "ext", "fs", "PermissionsExt", "from_mode"];
 pub const POLL: [&str; 4] = ["core", "task", "poll", "Poll"];
 pub const POLL_PENDING: [&str; 5] = ["core", "task", "poll", "Poll", "Pending"];
 pub const POLL_READY: [&str; 5] = ["core", "task", "poll", "Poll", "Ready"];
diff --git a/clippy_utils/src/qualify_min_const_fn.rs b/clippy_utils/src/qualify_min_const_fn.rs
index dac4d93499d..b52cbf31e35 100644
--- a/clippy_utils/src/qualify_min_const_fn.rs
+++ b/clippy_utils/src/qualify_min_const_fn.rs
@@ -210,7 +210,7 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen
         StatementKind::Assign(box (place, rval)) => {
             check_place(tcx, *place, span, body)?;
             check_rvalue(tcx, body, def_id, rval, span)
-        }
+        },
 
         StatementKind::FakeRead(box (_, place)) => check_place(tcx, *place, span, body),
         // just an assignment
@@ -218,13 +218,11 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen
 
         StatementKind::LlvmInlineAsm { .. } => Err((span, "cannot use inline assembly in const fn".into())),
 
-        StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping{
-          dst, src, count,
-        }) => {
-          check_operand(tcx, dst, span, body)?;
-          check_operand(tcx, src, span, body)?;
-          check_operand(tcx, count, span, body)
-        }
+        StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { dst, src, count }) => {
+            check_operand(tcx, dst, span, body)?;
+            check_operand(tcx, src, span, body)?;
+            check_operand(tcx, count, span, body)
+        },
         // These are all NOPs
         StatementKind::StorageLive(_)
         | StatementKind::StorageDead(_)
diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs
index b2fe4317154..0633a19391f 100644
--- a/clippy_utils/src/sugg.rs
+++ b/clippy_utils/src/sugg.rs
@@ -267,17 +267,44 @@ impl<'a> Sugg<'a> {
             Sugg::NonParen(..) => self,
             // `(x)` and `(x).y()` both don't need additional parens.
             Sugg::MaybeParen(sugg) => {
-                if sugg.starts_with('(') && sugg.ends_with(')') {
+                if has_enclosing_paren(&sugg) {
                     Sugg::MaybeParen(sugg)
                 } else {
                     Sugg::NonParen(format!("({})", sugg).into())
                 }
             },
-            Sugg::BinOp(_, sugg) => Sugg::NonParen(format!("({})", sugg).into()),
+            Sugg::BinOp(_, sugg) => {
+                if has_enclosing_paren(&sugg) {
+                    Sugg::NonParen(sugg)
+                } else {
+                    Sugg::NonParen(format!("({})", sugg).into())
+                }
+            },
         }
     }
 }
 
+/// Return `true` if `sugg` is enclosed in parenthesis.
+fn has_enclosing_paren(sugg: impl AsRef<str>) -> bool {
+    let mut chars = sugg.as_ref().chars();
+    if let Some('(') = chars.next() {
+        let mut depth = 1;
+        while let Some(c) = chars.next() {
+            if c == '(' {
+                depth += 1;
+            } else if c == ')' {
+                depth -= 1;
+            }
+            if depth == 0 {
+                break;
+            }
+        }
+        chars.next().is_none()
+    } else {
+        false
+    }
+}
+
 // Copied from the rust standart library, and then edited
 macro_rules! forward_binop_impls_to_ref {
     (impl $imp:ident, $method:ident for $t:ty, type Output = $o:ty) => {
@@ -668,6 +695,8 @@ impl<T: LintContext> DiagnosticBuilderExt<T> for rustc_errors::DiagnosticBuilder
 #[cfg(test)]
 mod test {
     use super::Sugg;
+
+    use rustc_ast::util::parser::AssocOp;
     use std::borrow::Cow;
 
     const SUGGESTION: Sugg<'static> = Sugg::NonParen(Cow::Borrowed("function_call()"));
@@ -681,4 +710,13 @@ mod test {
     fn blockify_transforms_sugg_into_a_block() {
         assert_eq!("{ function_call() }", SUGGESTION.blockify().to_string());
     }
+
+    #[test]
+    fn binop_maybe_par() {
+        let sugg = Sugg::BinOp(AssocOp::Add, "(1 + 1)".into());
+        assert_eq!("(1 + 1)", sugg.maybe_par().to_string());
+
+        let sugg = Sugg::BinOp(AssocOp::Add, "(1 + 1) + (1 + 1)".into());
+        assert_eq!("((1 + 1) + (1 + 1))", sugg.maybe_par().to_string());
+    }
 }
diff --git a/clippy_utils/src/usage.rs b/clippy_utils/src/usage.rs
index 54f110988d7..650b70c63af 100644
--- a/clippy_utils/src/usage.rs
+++ b/clippy_utils/src/usage.rs
@@ -1,9 +1,9 @@
 use crate as utils;
-use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
 use rustc_hir::def::Res;
 use rustc_hir::intravisit;
 use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
+use rustc_hir::HirIdSet;
 use rustc_hir::{Expr, ExprKind, HirId, Path};
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_lint::LateContext;
@@ -13,9 +13,9 @@ use rustc_middle::ty;
 use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
 
 /// Returns a set of mutated local variable IDs, or `None` if mutations could not be determined.
-pub fn mutated_variables<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) -> Option<FxHashSet<HirId>> {
+pub fn mutated_variables<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) -> Option<HirIdSet> {
     let mut delegate = MutVarsDelegate {
-        used_mutably: FxHashSet::default(),
+        used_mutably: HirIdSet::default(),
         skip: false,
     };
     cx.tcx.infer_ctxt().enter(|infcx| {
@@ -44,7 +44,7 @@ pub fn is_potentially_mutated<'tcx>(variable: &'tcx Path<'_>, expr: &'tcx Expr<'
 }
 
 struct MutVarsDelegate {
-    used_mutably: FxHashSet<HirId>,
+    used_mutably: HirIdSet,
     skip: bool,
 }