fix: incorrect suggestion for nonexistent enum variant and applying generic on variant of type-aliased enum

Author: NKID00

compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs

@@ -1625,7 +1625,7 @@ pub(crate) fn fn_trait_to_string(
     }
 }
 
-/// Used for generics args error extend.
+/// Extra information for diagnostics of generics args error.
 pub enum GenericsArgsErrExtend<'tcx> {
     EnumVariant {
         qself: &'tcx hir::Ty<'tcx>,
@@ -1652,87 +1652,65 @@ fn generics_args_err_extend<'a>(
 ) {
     match err_extend {
         GenericsArgsErrExtend::EnumVariant { qself, assoc_segment, adt_def } => {
-            err.note("enum variants can't have type parameters");
+            err.note("variants of type-aliased enum can't have type parameters");
             let type_name = tcx.item_name(adt_def.did());
-            let msg = format!(
-                "you might have meant to specify type parameters on enum \
-                `{type_name}`"
-            );
-            let Some(args) = assoc_segment.args else {
+            let Some(variant_args) = assoc_segment.args else {
                 return;
             };
             // Get the span of the generics args *including* the leading `::`.
             // We do so by stretching args.span_ext to the left by 2. Earlier
             // it was done based on the end of assoc segment but that sometimes
             // led to impossible spans and caused issues like #116473
-            let args_span = args.span_ext.with_lo(args.span_ext.lo() - BytePos(2));
+            let variant_args_span =
+                variant_args.span_ext.with_lo(variant_args.span_ext.lo() - BytePos(2));
             if tcx.generics_of(adt_def.did()).is_empty() {
                 // FIXME(estebank): we could also verify that the arguments being
                 // work for the `enum`, instead of just looking if it takes *any*.
                 err.span_suggestion_verbose(
-                    args_span,
+                    variant_args_span,
                     format!("{type_name} doesn't have generic parameters"),
                     "",
                     Applicability::MachineApplicable,
                 );
                 return;
             }
-            let Ok(snippet) = tcx.sess.source_map().span_to_snippet(args_span) else {
+            let msg = "you might have meant to specify type parameters on the enum";
+            let Ok(snippet) = tcx.sess.source_map().span_to_snippet(variant_args_span) else {
+                err.note(msg);
+                return;
+            };
+            // If the path segment already has type params, we want to overwrite them.
+            let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = &qself.kind else {
+                err.note(msg);
+                return;
+            };
+            // The last element of `path.segments` is the previous to last element
+            // on the path and would normally be the enum alias itself.
+            let Some(hir::PathSegment {
+                ident,
+                args: enum_args,
+                res: Res::SelfTyAlias { .. } | Res::Def(DefKind::TyAlias, _),
+                ..
+            }) = &path.segments.last()
+            else {
                 err.note(msg);
                 return;
             };
-            let (qself_sugg_span, is_self) =
-                if let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = &qself.kind {
-                    // If the path segment already has type params, we want to overwrite
-                    // them.
-                    match &path.segments {
-                        // `segment` is the previous to last element on the path,
-                        // which would normally be the `enum` itself, while the last
-                        // `_` `PathSegment` corresponds to the variant.
-                        [
-                            ..,
-                            hir::PathSegment {
-                                ident, args, res: Res::Def(DefKind::Enum, _), ..
-                            },
-                            _,
-                        ] => (
-                            // We need to include the `::` in `Type::Variant::<Args>`
-                            // to point the span to `::<Args>`, not just `<Args>`.
-                            ident
-                                .span
-                                .shrink_to_hi()
-                                .to(args.map_or(ident.span.shrink_to_hi(), |a| a.span_ext)),
-                            false,
-                        ),
-                        [segment] => {
-                            (
-                                // We need to include the `::` in `Type::Variant::<Args>`
-                                // to point the span to `::<Args>`, not just `<Args>`.
-                                segment.ident.span.shrink_to_hi().to(segment
-                                    .args
-                                    .map_or(segment.ident.span.shrink_to_hi(), |a| a.span_ext)),
-                                kw::SelfUpper == segment.ident.name,
-                            )
-                        }
-                        _ => {
-                            err.note(msg);
-                            return;
-                        }
-                    }
-                } else {
-                    err.note(msg);
-                    return;
-                };
             let suggestion = vec![
-                if is_self {
+                if path.segments.len() == 1 && kw::SelfUpper == ident.name {
                     // Account for people writing `Self::Variant::<Args>`, where
                     // `Self` is the enum, and suggest replacing `Self` with the
                     // appropriate type: `Type::<Args>::Variant`.
                     (qself.span, format!("{type_name}{snippet}"))
                 } else {
-                    (qself_sugg_span, snippet)
+                    // We need to include the `::` in `Type::<Args>::Variants`
+                    // to point the span to `::<Args>`, not just `<Args>`.
+                    let enum_args_span = ident.span.shrink_to_hi().to(enum_args
+                        .and_then(|a| a.span_ext())
+                        .unwrap_or(ident.span.shrink_to_hi()));
+                    (enum_args_span, snippet)
                 },
-                (args_span, String::new()),
+                (variant_args_span, String::new()),
             ];
             err.multipart_suggestion_verbose(msg, suggestion, Applicability::MaybeIncorrect);
         }

compiler/rustc_hir_typeck/src/lib.rs

@@ -422,10 +422,10 @@ fn report_unexpected_variant_res(
             let mut suggestion = vec![];
             match tcx.parent_hir_node(expr.hir_id) {
                 hir::Node::Expr(hir::Expr {
-                    kind: hir::ExprKind::Call(..),
+                    kind: hir::ExprKind::Call(callee, ..),
                     span: call_span,
                     ..
-                }) => {
+                }) if callee.hir_id == expr.hir_id => {
                     suggestion.push((span.shrink_to_hi().with_hi(call_span.hi()), sugg));
                 }
                 hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Binary(..), hir_id, .. }) => {

compiler/rustc_hir_typeck/src/method/suggest.rs

@@ -1592,13 +1592,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     && let hir::Node::Stmt(&hir::Stmt { kind: hir::StmtKind::Semi(parent), .. })
                     | hir::Node::Expr(parent) = self.tcx.parent_hir_node(path_expr.hir_id)
                 {
-                    let replacement_span =
-                        if let hir::ExprKind::Call(..) | hir::ExprKind::Struct(..) = parent.kind {
-                            // We want to replace the parts that need to go, like `()` and `{}`.
-                            span.with_hi(parent.span.hi())
-                        } else {
-                            span
-                        };
+                    // We want to also replace variant constructor part like `()` and `{}`.
+                    let replacement_span = match parent.kind {
+                        hir::ExprKind::Call(callee, ..) if callee.hir_id == path_expr.hir_id => {
+                            if span.hi() == callee.span.hi() {
+                                span.with_hi(parent.span.hi())
+                            } else {
+                                // Bail if there are parens around the variant like `(A::B)()`
+                                span
+                            }
+                        }
+                        hir::ExprKind::Struct(..) => span.with_hi(parent.span.hi()),
+                        _ => span,
+                    };
                     match (variant.ctor, parent.kind) {
                         (None, hir::ExprKind::Struct(..)) => {
                             // We want a struct and we have a struct. We won't suggest changing
@@ -1631,7 +1637,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         (
                             Some((hir::def::CtorKind::Fn, def_id)),
                             hir::ExprKind::Call(rcvr, args),
-                        ) => {
+                        ) if rcvr.hir_id == path_expr.hir_id => {
                             let fn_sig = self.tcx.fn_sig(def_id).instantiate_identity();
                             let inputs = fn_sig.inputs().skip_binder();
                             // FIXME: reuse the logic for "change args" suggestion to account for types

tests/crashes/130395.rs

@@ -0,0 +1,57 @@
+// Regression test for #146586
+
+//@ only-linux
+//@ compile-flags: --error-format=human --color=always
+
+enum Enum {
+    Unit,
+    Tuple(i32),
+    Struct { x: i32 },
+}
+
+fn foo(_: Enum) {}
+
+fn main() {
+    foo(Enum::Unit);
+    foo(Enum::Tuple);
+    foo(Enum::Struct); // Suggestion was malformed
+    foo(Enum::Unit());
+    foo(Enum::Tuple());
+    foo(Enum::Struct());
+    foo(Enum::Unit {});
+    foo(Enum::Tuple {});
+    foo(Enum::Struct {});
+    foo(Enum::Unit(0));
+    foo(Enum::Tuple(0));
+    foo(Enum::Struct(0));
+    foo(Enum::Unit { x: 0 });
+    foo(Enum::Tuple { x: 0 });
+    foo(Enum::Struct { x: 0 });
+    foo(Enum::Unit(0, 0));
+    foo(Enum::Tuple(0, 0));
+    foo(Enum::Struct(0, 0));
+    foo(Enum::Unit { x: 0, y: 0 });
+    foo(Enum::Tuple { x: 0, y: 0 });
+    foo(Enum::Struct { x: 0, y: 0 });
+    foo(Enum::unit); // Suggestion was malformed
+    foo(Enum::tuple); // Suggestion is enhanced
+    foo(Enum::r#struct); // Suggestion was malformed
+    foo(Enum::unit());
+    foo(Enum::tuple());
+    foo(Enum::r#struct());
+    foo(Enum::unit {}); // Suggestion could be enhanced
+    foo(Enum::tuple {}); // Suggestion could be enhanced
+    foo(Enum::r#struct {}); // Suggestion could be enhanced
+    foo(Enum::unit(0));
+    foo(Enum::tuple(0));
+    foo(Enum::r#struct(0));
+    foo(Enum::unit { x: 0 }); // Suggestion could be enhanced
+    foo(Enum::tuple { x: 0 }); // Suggestion could be enhanced
+    foo(Enum::r#struct { x: 0 });
+    foo(Enum::unit(0, 0));
+    foo(Enum::tuple(0, 0));
+    foo(Enum::r#struct(0, 0));
+    foo(Enum::unit { x: 0, y: 0 }); // Suggestion could be enhanced
+    foo(Enum::tuple { x: 0, y: 0 }); // Suggestion could be enhanced
+    foo(Enum::r#struct { x: 0, y: 0 }); // Suggestion could be enhanced
+}