empty_enum_variants_with_brackets: Do not lint pub enums and enums used as functions

CHANGELOG.md

@@ -5917,6 +5917,7 @@ Released 2018-09-13
 [`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
 [`unnecessary_literal_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_literal_unwrap
 [`unnecessary_map_on_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_map_on_constructor
+[`unnecessary_min_or_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_min_or_max
 [`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed
 [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation
 [`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings

clippy_lints/src/allow_attributes.rs

@@ -57,7 +57,7 @@ impl LateLintPass<'_> for AllowAttribute {
             && let AttrStyle::Outer = attr.style
             && let Some(ident) = attr.ident()
             && ident.name == rustc_span::symbol::sym::allow
-            && !is_from_proc_macro(cx, &attr)
+            && !is_from_proc_macro(cx, attr)
         {
             span_lint_and_sugg(
                 cx,

clippy_lints/src/attrs/allow_attributes_without_reason.rs

@@ -22,7 +22,7 @@ pub(super) fn check<'cx>(cx: &LateContext<'cx>, name: Symbol, items: &[NestedMet
     }
 
     // Check if the attribute is in an external macro and therefore out of the developer's control
-    if in_external_macro(cx.sess(), attr.span) || is_from_proc_macro(cx, &attr) {
+    if in_external_macro(cx.sess(), attr.span) || is_from_proc_macro(cx, attr) {
         return;
     }
 

clippy_lints/src/declared_lints.rs

@@ -471,6 +471,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::methods::UNNECESSARY_JOIN_INFO,
     crate::methods::UNNECESSARY_LAZY_EVALUATIONS_INFO,
     crate::methods::UNNECESSARY_LITERAL_UNWRAP_INFO,
+    crate::methods::UNNECESSARY_MIN_OR_MAX_INFO,
     crate::methods::UNNECESSARY_RESULT_MAP_OR_ELSE_INFO,
     crate::methods::UNNECESSARY_SORT_BY_INFO,
     crate::methods::UNNECESSARY_TO_OWNED_INFO,

clippy_lints/src/empty_with_brackets.rs

@@ -1,10 +1,15 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::snippet_opt;
-use rustc_ast::ast::{Item, ItemKind, Variant, VariantData};
+use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::Applicability;
+use rustc_hir::def::CtorOf;
+use rustc_hir::def::DefKind::Ctor;
+use rustc_hir::def::Res::Def;
+use rustc_hir::def_id::DefId;
+use rustc_hir::{Expr, ExprKind, Item, ItemKind, Path, QPath, Variant, VariantData};
 use rustc_lexer::TokenKind;
-use rustc_lint::{EarlyContext, EarlyLintPass};
-use rustc_session::declare_lint_pass;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::impl_lint_pass;
 use rustc_span::Span;
 
 declare_clippy_lint! {
@@ -70,10 +75,16 @@ declare_clippy_lint! {
     "finds enum variants with empty brackets"
 }
 
-declare_lint_pass!(EmptyWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS, EMPTY_ENUM_VARIANTS_WITH_BRACKETS]);
+#[derive(Default)]
+pub struct EmptyWithBrackets {
+    empty_tuple_enum_variants: FxHashSet<(DefId, Span)>,
+    enum_variants_used_as_functions: FxHashSet<DefId>,
+}
+
+impl_lint_pass!(EmptyWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS, EMPTY_ENUM_VARIANTS_WITH_BRACKETS]);
 
-impl EarlyLintPass for EmptyWithBrackets {
-    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
+impl LateLintPass<'_> for EmptyWithBrackets {
+    fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
         let span_after_ident = item.span.with_lo(item.ident.span.hi());
 
         if let ItemKind::Struct(var_data, _) = &item.kind
@@ -97,22 +108,61 @@ impl EarlyLintPass for EmptyWithBrackets {
         }
     }
 
-    fn check_variant(&mut self, cx: &EarlyContext<'_>, variant: &Variant) {
+    fn check_variant(&mut self, cx: &LateContext<'_>, variant: &Variant<'_>) {
+        // Don't lint pub enums
+        if cx.effective_visibilities.is_reachable(variant.def_id) {
+            return;
+        }
+
         let span_after_ident = variant.span.with_lo(variant.ident.span.hi());
 
-        if has_brackets(&variant.data) && has_no_fields(cx, &variant.data, span_after_ident) {
+        if has_no_fields(cx, &variant.data, span_after_ident) {
+            match variant.data {
+                VariantData::Struct { .. } => {
+                    span_lint_and_then(
+                        cx,
+                        EMPTY_ENUM_VARIANTS_WITH_BRACKETS,
+                        span_after_ident,
+                        "enum variant has empty brackets",
+                        |diagnostic| {
+                            diagnostic.span_suggestion_hidden(
+                                span_after_ident,
+                                "remove the brackets",
+                                "",
+                                Applicability::MaybeIncorrect,
+                            );
+                        },
+                    );
+                },
+                VariantData::Tuple(..) => {
+                    if let Some(x) = variant.data.ctor_def_id() {
+                        self.empty_tuple_enum_variants.insert((x.to_def_id(), span_after_ident));
+                    }
+                },
+                VariantData::Unit(..) => {},
+            }
+        }
+    }
+
+    fn check_expr(&mut self, _cx: &LateContext<'_>, expr: &Expr<'_>) {
+        if let Some(def_id) = check_expr_for_enum_as_function(expr) {
+            self.enum_variants_used_as_functions.insert(def_id);
+        }
+    }
+
+    fn check_crate_post(&mut self, cx: &LateContext<'_>) {
+        for &(_, span) in self
+            .empty_tuple_enum_variants
+            .iter()
+            .filter(|(variant, _)| !self.enum_variants_used_as_functions.contains(variant))
+        {
             span_lint_and_then(
                 cx,
                 EMPTY_ENUM_VARIANTS_WITH_BRACKETS,
-                span_after_ident,
+                span,
                 "enum variant has empty brackets",
                 |diagnostic| {
-                    diagnostic.span_suggestion_hidden(
-                        span_after_ident,
-                        "remove the brackets",
-                        "",
-                        Applicability::MaybeIncorrect,
-                    );
+                    diagnostic.span_suggestion_hidden(span, "remove the brackets", "", Applicability::MaybeIncorrect);
                 },
             );
         }
@@ -123,11 +173,11 @@ fn has_no_ident_token(braces_span_str: &str) -> bool {
     !rustc_lexer::tokenize(braces_span_str).any(|t| t.kind == TokenKind::Ident)
 }
 
-fn has_brackets(var_data: &VariantData) -> bool {
-    !matches!(var_data, VariantData::Unit(_))
+fn has_brackets(var_data: &VariantData<'_>) -> bool {
+    !matches!(var_data, VariantData::Unit(..))
 }
 
-fn has_no_fields(cx: &EarlyContext<'_>, var_data: &VariantData, braces_span: Span) -> bool {
+fn has_no_fields(cx: &LateContext<'_>, var_data: &VariantData<'_>, braces_span: Span) -> bool {
     if !var_data.fields().is_empty() {
         return false;
     }
@@ -142,6 +192,20 @@ fn has_no_fields(cx: &EarlyContext<'_>, var_data: &VariantData, braces_span: Spa
     has_no_ident_token(braces_span_str.as_ref())
 }
 
+fn check_expr_for_enum_as_function(expr: &Expr<'_>) -> Option<DefId> {
+    let ExprKind::Path(QPath::Resolved(
+        _,
+        Path {
+            res: Def(Ctor(CtorOf::Variant, _), def_id),
+            ..
+        },
+    )) = expr.kind
+    else {
+        return None;
+    };
+    Some(*def_id)
+}
+
 #[cfg(test)]
 mod unit_test {
     use super::*;

clippy_lints/src/implicit_return.rs

@@ -1,7 +1,7 @@
 use clippy_utils::diagnostics::span_lint_hir_and_then;
 use clippy_utils::source::{snippet_with_applicability, snippet_with_context, walk_span_to_context};
 use clippy_utils::visitors::for_each_expr_without_closures;
-use clippy_utils::{get_async_fn_body, is_async_fn};
+use clippy_utils::{get_async_fn_body, is_async_fn, is_from_proc_macro};
 use core::ops::ControlFlow;
 use rustc_errors::Applicability;
 use rustc_hir::intravisit::FnKind;
@@ -245,6 +245,10 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitReturn {
         } else {
             body.value
         };
+
+        if is_from_proc_macro(cx, expr) {
+            return;
+        }
         lint_implicit_returns(cx, expr, expr.span.ctxt(), None);
     }
 }

clippy_lints/src/lib.rs

@@ -1007,7 +1007,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
         })
     });
     store.register_early_pass(|| Box::new(crate_in_macro_def::CrateInMacroDef));
-    store.register_early_pass(|| Box::new(empty_with_brackets::EmptyWithBrackets));
+    store.register_late_pass(|_| Box::new(empty_with_brackets::EmptyWithBrackets::default()));
     store.register_late_pass(|_| Box::new(unnecessary_owned_empty_strings::UnnecessaryOwnedEmptyStrings));
     store.register_early_pass(|| Box::new(pub_use::PubUse));
     store.register_late_pass(|_| Box::new(format_push_string::FormatPushString));

clippy_lints/src/matches/manual_unwrap_or.rs

@@ -3,46 +3,78 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::{indent_of, reindent_multiline, snippet_opt};
 use clippy_utils::ty::is_type_diagnostic_item;
 use clippy_utils::usage::contains_return_break_continue_macro;
-use clippy_utils::{is_res_lang_ctor, path_to_local_id, sugg};
+use clippy_utils::{is_res_lang_ctor, path_to_local_id, peel_blocks, sugg};
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::LangItem::{OptionNone, ResultErr};
-use rustc_hir::{Arm, Expr, PatKind};
+use rustc_hir::{Arm, Expr, Pat, PatKind};
 use rustc_lint::LateContext;
+use rustc_middle::ty::Ty;
 use rustc_span::sym;
 
 use super::MANUAL_UNWRAP_OR;
 
-pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, scrutinee: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>]) {
+pub(super) fn check_match<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'tcx>,
+    scrutinee: &'tcx Expr<'_>,
+    arms: &'tcx [Arm<'_>],
+) {
     let ty = cx.typeck_results().expr_ty(scrutinee);
-    if let Some(ty_name) = if is_type_diagnostic_item(cx, ty, sym::Option) {
+    if let Some((or_arm, unwrap_arm)) = applicable_or_arm(cx, arms) {
+        check_and_lint(cx, expr, unwrap_arm.pat, scrutinee, unwrap_arm.body, or_arm.body, ty);
+    }
+}
+
+pub(super) fn check_if_let<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    let_pat: &'tcx Pat<'_>,
+    let_expr: &'tcx Expr<'_>,
+    then_expr: &'tcx Expr<'_>,
+    else_expr: &'tcx Expr<'_>,
+) {
+    let ty = cx.typeck_results().expr_ty(let_expr);
+    check_and_lint(cx, expr, let_pat, let_expr, then_expr, peel_blocks(else_expr), ty);
+}
+
+fn check_and_lint<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    let_pat: &'tcx Pat<'_>,
+    let_expr: &'tcx Expr<'_>,
+    then_expr: &'tcx Expr<'_>,
+    else_expr: &'tcx Expr<'_>,
+    ty: Ty<'tcx>,
+) {
+    if let PatKind::TupleStruct(ref qpath, [unwrap_pat], _) = let_pat.kind
+        && let Res::Def(DefKind::Ctor(..), ctor_id) = cx.qpath_res(qpath, let_pat.hir_id)
+        && let Some(variant_id) = cx.tcx.opt_parent(ctor_id)
+        && (cx.tcx.lang_items().option_some_variant() == Some(variant_id)
+            || cx.tcx.lang_items().result_ok_variant() == Some(variant_id))
+        && let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind
+        && path_to_local_id(peel_blocks(then_expr), binding_hir_id)
+        && cx.typeck_results().expr_adjustments(then_expr).is_empty()
+        && let Some(ty_name) = find_type_name(cx, ty)
+        && let Some(or_body_snippet) = snippet_opt(cx, else_expr.span)
+        && let Some(indent) = indent_of(cx, expr.span)
+        && constant_simple(cx, cx.typeck_results(), else_expr).is_some()
+    {
+        lint(cx, expr, let_expr, ty_name, or_body_snippet, indent);
+    }
+}
+
+fn find_type_name<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<&'static str> {
+    if is_type_diagnostic_item(cx, ty, sym::Option) {
         Some("Option")
     } else if is_type_diagnostic_item(cx, ty, sym::Result) {
         Some("Result")
     } else {
         None
-    } && let Some(or_arm) = applicable_or_arm(cx, arms)
-        && let Some(or_body_snippet) = snippet_opt(cx, or_arm.body.span)
-        && let Some(indent) = indent_of(cx, expr.span)
-        && constant_simple(cx, cx.typeck_results(), or_arm.body).is_some()
-    {
-        let reindented_or_body = reindent_multiline(or_body_snippet.into(), true, Some(indent));
-
-        let mut app = Applicability::MachineApplicable;
-        let suggestion = sugg::Sugg::hir_with_context(cx, scrutinee, expr.span.ctxt(), "..", &mut app).maybe_par();
-        span_lint_and_sugg(
-            cx,
-            MANUAL_UNWRAP_OR,
-            expr.span,
-            format!("this pattern reimplements `{ty_name}::unwrap_or`"),
-            "replace with",
-            format!("{suggestion}.unwrap_or({reindented_or_body})",),
-            app,
-        );
     }
 }
 
-fn applicable_or_arm<'a>(cx: &LateContext<'_>, arms: &'a [Arm<'a>]) -> Option<&'a Arm<'a>> {
+fn applicable_or_arm<'a>(cx: &LateContext<'_>, arms: &'a [Arm<'a>]) -> Option<(&'a Arm<'a>, &'a Arm<'a>)> {
     if arms.len() == 2
         && arms.iter().all(|arm| arm.guard.is_none())
         && let Some((idx, or_arm)) = arms.iter().enumerate().find(|(_, arm)| match arm.pat.kind {
@@ -54,18 +86,33 @@ fn applicable_or_arm<'a>(cx: &LateContext<'_>, arms: &'a [Arm<'a>]) -> Option<&'
             _ => false,
         })
         && let unwrap_arm = &arms[1 - idx]
-        && let PatKind::TupleStruct(ref qpath, [unwrap_pat], _) = unwrap_arm.pat.kind
-        && let Res::Def(DefKind::Ctor(..), ctor_id) = cx.qpath_res(qpath, unwrap_arm.pat.hir_id)
-        && let Some(variant_id) = cx.tcx.opt_parent(ctor_id)
-        && (cx.tcx.lang_items().option_some_variant() == Some(variant_id)
-            || cx.tcx.lang_items().result_ok_variant() == Some(variant_id))
-        && let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind
-        && path_to_local_id(unwrap_arm.body, binding_hir_id)
-        && cx.typeck_results().expr_adjustments(unwrap_arm.body).is_empty()
         && !contains_return_break_continue_macro(or_arm.body)
     {
-        Some(or_arm)
+        Some((or_arm, unwrap_arm))
     } else {
         None
     }
 }
+
+fn lint<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &Expr<'tcx>,
+    scrutinee: &'tcx Expr<'_>,
+    ty_name: &str,
+    or_body_snippet: String,
+    indent: usize,
+) {
+    let reindented_or_body = reindent_multiline(or_body_snippet.into(), true, Some(indent));
+
+    let mut app = Applicability::MachineApplicable;
+    let suggestion = sugg::Sugg::hir_with_context(cx, scrutinee, expr.span.ctxt(), "..", &mut app).maybe_par();
+    span_lint_and_sugg(
+        cx,
+        MANUAL_UNWRAP_OR,
+        expr.span,
+        format!("this pattern reimplements `{ty_name}::unwrap_or`"),
+        "replace with",
+        format!("{suggestion}.unwrap_or({reindented_or_body})",),
+        app,
+    );
+}

clippy_lints/src/matches/mod.rs

@@ -1069,7 +1069,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
                     redundant_guards::check(cx, arms, &self.msrv);
 
                     if !in_constant(cx, expr.hir_id) {
-                        manual_unwrap_or::check(cx, expr, ex, arms);
+                        manual_unwrap_or::check_match(cx, expr, ex, arms);
                         manual_map::check_match(cx, expr, ex, arms);
                         manual_filter::check_match(cx, ex, arms, expr);
                     }
@@ -1097,6 +1097,14 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
                         );
                     }
                     if !in_constant(cx, expr.hir_id) {
+                        manual_unwrap_or::check_if_let(
+                            cx,
+                            expr,
+                            if_let.let_pat,
+                            if_let.let_expr,
+                            if_let.if_then,
+                            else_expr,
+                        );
                         manual_map::check_if_let(cx, expr, if_let.let_pat, if_let.let_expr, if_let.if_then, else_expr);
                         manual_filter::check_if_let(
                             cx,