Rollup merge of #88123 - camelid:tup-pat-precise-spans, r=estebank

Make spans for tuple patterns in E0023 more precise

As suggested in #86307. Closes #86307.

r? ````@estebank````

Author: Manishearth

compiler/rustc_hir/src/hir.rs

@@ -3183,6 +3183,20 @@ pub enum Node<'hir> {
 }
 
 impl<'hir> Node<'hir> {
+    /// Get the identifier of this `Node`, if applicable.
+    ///
+    /// # Edge cases
+    ///
+    /// Calling `.ident()` on a [`Node::Ctor`] will return `None`
+    /// because `Ctor`s do not have identifiers themselves.
+    /// Instead, call `.ident()` on the parent struct/variant, like so:
+    ///
+    /// ```ignore (illustrative)
+    /// ctor
+    ///     .ctor_hir_id()
+    ///     .and_then(|ctor_id| tcx.hir().find(tcx.hir().get_parent_node(ctor_id)))
+    ///     .and_then(|parent| parent.ident())
+    /// ```
     pub fn ident(&self) -> Option<Ident> {
         match self {
             Node::TraitItem(TraitItem { ident, .. })
@@ -3191,8 +3205,25 @@ impl<'hir> Node<'hir> {
             | Node::Field(FieldDef { ident, .. })
             | Node::Variant(Variant { ident, .. })
             | Node::MacroDef(MacroDef { ident, .. })
-            | Node::Item(Item { ident, .. }) => Some(*ident),
-            _ => None,
+            | Node::Item(Item { ident, .. })
+            | Node::PathSegment(PathSegment { ident, .. }) => Some(*ident),
+            Node::Lifetime(lt) => Some(lt.name.ident()),
+            Node::GenericParam(p) => Some(p.name.ident()),
+            Node::Param(..)
+            | Node::AnonConst(..)
+            | Node::Expr(..)
+            | Node::Stmt(..)
+            | Node::Block(..)
+            | Node::Ctor(..)
+            | Node::Pat(..)
+            | Node::Binding(..)
+            | Node::Arm(..)
+            | Node::Local(..)
+            | Node::Visibility(..)
+            | Node::Crate(..)
+            | Node::Ty(..)
+            | Node::TraitRef(..)
+            | Node::Infer(..) => None,
         }
     }
 

compiler/rustc_ty_utils/src/ty.rs

@@ -223,7 +223,18 @@ fn associated_items(tcx: TyCtxt<'_>, def_id: DefId) -> ty::AssocItems<'_> {
 }
 
 fn def_ident_span(tcx: TyCtxt<'_>, def_id: DefId) -> Option<Span> {
-    tcx.hir().get_if_local(def_id).and_then(|node| node.ident()).map(|ident| ident.span)
+    tcx.hir()
+        .get_if_local(def_id)
+        .and_then(|node| match node {
+            // A `Ctor` doesn't have an identifier itself, but its parent
+            // struct/variant does. Compare with `hir::Map::opt_span`.
+            hir::Node::Ctor(ctor) => ctor
+                .ctor_hir_id()
+                .and_then(|ctor_id| tcx.hir().find(tcx.hir().get_parent_node(ctor_id)))
+                .and_then(|parent| parent.ident()),
+            _ => node.ident(),
+        })
+        .map(|ident| ident.span)
 }
 
 /// If the given `DefId` describes an item belonging to a trait,

compiler/rustc_typeck/src/check/pat.rs

@@ -15,7 +15,7 @@ use rustc_span::hygiene::DesugaringKind;
 use rustc_span::lev_distance::find_best_match_for_name;
 use rustc_span::source_map::{Span, Spanned};
 use rustc_span::symbol::Ident;
-use rustc_span::{BytePos, DUMMY_SP};
+use rustc_span::{BytePos, MultiSpan, DUMMY_SP};
 use rustc_trait_selection::autoderef::Autoderef;
 use rustc_trait_selection::traits::{ObligationCause, Pattern};
 use ty::VariantDef;
@@ -990,10 +990,25 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     ) {
         let subpats_ending = pluralize!(subpats.len());
         let fields_ending = pluralize!(fields.len());
+
+        let subpat_spans = if subpats.is_empty() {
+            vec![pat_span]
+        } else {
+            subpats.iter().map(|p| p.span).collect()
+        };
+        let last_subpat_span = *subpat_spans.last().unwrap();
         let res_span = self.tcx.def_span(res.def_id());
+        let def_ident_span = self.tcx.def_ident_span(res.def_id()).unwrap_or(res_span);
+        let field_def_spans = if fields.is_empty() {
+            vec![res_span]
+        } else {
+            fields.iter().map(|f| f.ident.span).collect()
+        };
+        let last_field_def_span = *field_def_spans.last().unwrap();
+
         let mut err = struct_span_err!(
             self.tcx.sess,
-            pat_span,
+            MultiSpan::from_spans(subpat_spans.clone()),
             E0023,
             "this pattern has {} field{}, but the corresponding {} has {} field{}",
             subpats.len(),
@@ -1003,10 +1018,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             fields_ending,
         );
         err.span_label(
-            pat_span,
-            format!("expected {} field{}, found {}", fields.len(), fields_ending, subpats.len(),),
-        )
-        .span_label(res_span, format!("{} defined here", res.descr()));
+            last_subpat_span,
+            &format!("expected {} field{}, found {}", fields.len(), fields_ending, subpats.len()),
+        );
+        if self.tcx.sess.source_map().is_multiline(qpath.span().between(last_subpat_span)) {
+            err.span_label(qpath.span(), "");
+        }
+        if self.tcx.sess.source_map().is_multiline(def_ident_span.between(last_field_def_span)) {
+            err.span_label(def_ident_span, format!("{} defined here", res.descr()));
+        }
+        for span in &field_def_spans[..field_def_spans.len() - 1] {
+            err.span_label(*span, "");
+        }
+        err.span_label(
+            last_field_def_span,
+            &format!("{} has {} field{}", res.descr(), fields.len(), fields_ending),
+        );
 
         // Identify the case `Some(x, y)` where the expected type is e.g. `Option<(T, U)>`.
         // More generally, the expected type wants a tuple variant with one field of an

src/test/ui/destructuring-assignment/tuple_struct_destructure_fail.stderr

@@ -15,22 +15,22 @@ LL |     Enum::SingleVariant(a, .., b, ..) = Enum::SingleVariant(0, 1);
    |                            previously used here
 
 error[E0023]: this pattern has 3 fields, but the corresponding tuple struct has 2 fields
-  --> $DIR/tuple_struct_destructure_fail.rs:30:5
+  --> $DIR/tuple_struct_destructure_fail.rs:30:17
    |
 LL | struct TupleStruct<S, T>(S, T);
-   | ------------------------------- tuple struct defined here
+   |                          -  - tuple struct has 2 fields
 ...
 LL |     TupleStruct(a, a, b) = TupleStruct(1, 2);
-   |     ^^^^^^^^^^^^^^^^^^^^ expected 2 fields, found 3
+   |                 ^  ^  ^ expected 2 fields, found 3
 
 error[E0023]: this pattern has 1 field, but the corresponding tuple struct has 2 fields
-  --> $DIR/tuple_struct_destructure_fail.rs:32:5
+  --> $DIR/tuple_struct_destructure_fail.rs:32:17
    |
 LL | struct TupleStruct<S, T>(S, T);
-   | ------------------------------- tuple struct defined here
+   |                          -  - tuple struct has 2 fields
 ...
 LL |     TupleStruct(_) = TupleStruct(1, 2);
-   |     ^^^^^^^^^^^^^^ expected 2 fields, found 1
+   |                 ^ expected 2 fields, found 1
    |
 help: use `_` to explicitly ignore each field
    |
@@ -42,22 +42,22 @@ LL |     TupleStruct(..) = TupleStruct(1, 2);
    |                 ~~
 
 error[E0023]: this pattern has 3 fields, but the corresponding tuple variant has 2 fields
-  --> $DIR/tuple_struct_destructure_fail.rs:34:5
+  --> $DIR/tuple_struct_destructure_fail.rs:34:25
    |
 LL |     SingleVariant(S, T)
-   |     ------------------- tuple variant defined here
+   |                   -  - tuple variant has 2 fields
 ...
 LL |     Enum::SingleVariant(a, a, b) = Enum::SingleVariant(1, 2);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected 2 fields, found 3
+   |                         ^  ^  ^ expected 2 fields, found 3
 
 error[E0023]: this pattern has 1 field, but the corresponding tuple variant has 2 fields
-  --> $DIR/tuple_struct_destructure_fail.rs:36:5
+  --> $DIR/tuple_struct_destructure_fail.rs:36:25
    |
 LL |     SingleVariant(S, T)
-   |     ------------------- tuple variant defined here
+   |                   -  - tuple variant has 2 fields
 ...
 LL |     Enum::SingleVariant(_) = Enum::SingleVariant(1, 2);
-   |     ^^^^^^^^^^^^^^^^^^^^^^ expected 2 fields, found 1
+   |                         ^ expected 2 fields, found 1
    |
 help: use `_` to explicitly ignore each field
    |

src/test/ui/error-codes/E0023.stderr

@@ -1,43 +1,43 @@
 error[E0023]: this pattern has 1 field, but the corresponding tuple variant has 2 fields
-  --> $DIR/E0023.rs:11:9
+  --> $DIR/E0023.rs:11:22
    |
 LL |     Apple(String, String),
-   |     --------------------- tuple variant defined here
+   |           ------  ------ tuple variant has 2 fields
 ...
 LL |         Fruit::Apple(a) => {},
-   |         ^^^^^^^^^^^^^^^ expected 2 fields, found 1
+   |                      ^ expected 2 fields, found 1
    |
 help: use `_` to explicitly ignore each field
    |
 LL |         Fruit::Apple(a, _) => {},
    |                       +++
 
 error[E0023]: this pattern has 3 fields, but the corresponding tuple variant has 2 fields
-  --> $DIR/E0023.rs:12:9
+  --> $DIR/E0023.rs:12:22
    |
 LL |     Apple(String, String),
-   |     --------------------- tuple variant defined here
+   |           ------  ------ tuple variant has 2 fields
 ...
 LL |         Fruit::Apple(a, b, c) => {},
-   |         ^^^^^^^^^^^^^^^^^^^^^ expected 2 fields, found 3
+   |                      ^  ^  ^ expected 2 fields, found 3
 
 error[E0023]: this pattern has 2 fields, but the corresponding tuple variant has 1 field
-  --> $DIR/E0023.rs:13:9
+  --> $DIR/E0023.rs:13:21
    |
 LL |     Pear(u32),
-   |     --------- tuple variant defined here
+   |          --- tuple variant has 1 field
 ...
 LL |         Fruit::Pear(1, 2) => {},
-   |         ^^^^^^^^^^^^^^^^^ expected 1 field, found 2
+   |                     ^  ^ expected 1 field, found 2
 
 error[E0023]: this pattern has 2 fields, but the corresponding tuple variant has 1 field
-  --> $DIR/E0023.rs:14:9
+  --> $DIR/E0023.rs:14:23
    |
 LL |     Orange((String, String)),
-   |     ------------------------ tuple variant defined here
+   |            ---------------- tuple variant has 1 field
 ...
 LL |         Fruit::Orange(a, b) => {},
-   |         ^^^^^^^^^^^^^^^^^^^ expected 1 field, found 2
+   |                       ^  ^ expected 1 field, found 2
    |
 help: missing parentheses
    |
@@ -48,7 +48,7 @@ error[E0023]: this pattern has 0 fields, but the corresponding tuple variant has
   --> $DIR/E0023.rs:15:9
    |
 LL |     Banana(()),
-   |     ---------- tuple variant defined here
+   |            -- tuple variant has 1 field
 ...
 LL |         Fruit::Banana() => {},
    |         ^^^^^^^^^^^^^^^ expected 1 field, found 0

src/test/ui/issues/issue-72574-2.stderr

@@ -19,13 +19,13 @@ LL |         Binder(_a, _x @ ..) => {}
    = note: only allowed in tuple, tuple struct, and slice patterns
 
 error[E0023]: this pattern has 2 fields, but the corresponding tuple struct has 3 fields
-  --> $DIR/issue-72574-2.rs:6:9
+  --> $DIR/issue-72574-2.rs:6:16
    |
 LL | struct Binder(i32, i32, i32);
-   | ----------------------------- tuple struct defined here
+   |               ---  ---  --- tuple struct has 3 fields
 ...
 LL |         Binder(_a, _x @ ..) => {}
-   |         ^^^^^^^^^^^^^^^^^^^ expected 3 fields, found 2
+   |                ^^  ^^^^^^^ expected 3 fields, found 2
    |
 help: use `_` to explicitly ignore each field
    |

src/test/ui/match/match-pattern-field-mismatch.stderr

@@ -1,11 +1,11 @@
 error[E0023]: this pattern has 2 fields, but the corresponding tuple variant has 3 fields
-  --> $DIR/match-pattern-field-mismatch.rs:10:11
+  --> $DIR/match-pattern-field-mismatch.rs:10:22
    |
 LL |         Rgb(usize, usize, usize),
-   |         ------------------------ tuple variant defined here
+   |             -----  -----  ----- tuple variant has 3 fields
 ...
 LL |           Color::Rgb(_, _) => { }
-   |           ^^^^^^^^^^^^^^^^ expected 3 fields, found 2
+   |                      ^  ^ expected 3 fields, found 2
    |
 help: use `_` to explicitly ignore each field
    |

src/test/ui/pattern/auxiliary/declarations-for-tuple-field-count-errors.rs

@@ -0,0 +1,20 @@
+pub struct Z0;
+pub struct Z1();
+
+pub struct S(pub u8, pub u8, pub u8);
+pub struct M(
+    pub u8,
+    pub u8,
+    pub u8,
+);
+
+pub enum E1 { Z0, Z1(), S(u8, u8, u8) }
+
+pub enum E2 {
+    S(u8, u8, u8),
+    M(
+        u8,
+        u8,
+        u8,
+    ),
+}

src/test/ui/pattern/issue-67037-pat-tup-scrut-ty-diff-less-fields.stderr

@@ -13,7 +13,7 @@ error[E0023]: this pattern has 0 fields, but the corresponding tuple struct has
   --> $DIR/issue-67037-pat-tup-scrut-ty-diff-less-fields.rs:19:9
    |
 LL | struct P<T>(T); // 1 type parameter wanted
-   | --------------- tuple struct defined here
+   |             - tuple struct has 1 field
 ...
 LL |     let P() = U {};
    |         ^^^ expected 1 field, found 0

src/test/ui/pattern/issue-74539.stderr

@@ -19,13 +19,13 @@ LL |         E::A(x @ ..) => {
    = note: only allowed in tuple, tuple struct, and slice patterns
 
 error[E0023]: this pattern has 1 field, but the corresponding tuple variant has 2 fields
-  --> $DIR/issue-74539.rs:8:9
+  --> $DIR/issue-74539.rs:8:14
    |
 LL |     A(u8, u8),
-   |     --------- tuple variant defined here
+   |       --  -- tuple variant has 2 fields
 ...
 LL |         E::A(x @ ..) => {
-   |         ^^^^^^^^^^^^ expected 2 fields, found 1
+   |              ^^^^^^ expected 2 fields, found 1
    |
 help: use `_` to explicitly ignore each field
    |