Skip to content

Commit

Permalink
Rollup merge of rust-lang#88123 - camelid:tup-pat-precise-spans, r=es…
Browse files Browse the repository at this point in the history
…tebank

Make spans for tuple patterns in E0023 more precise

As suggested in rust-lang#86307. Closes rust-lang#86307.

r? ``@estebank``
  • Loading branch information
GuillaumeGomez authored Aug 26, 2021
2 parents e948f31 + 8a6501d commit b0d21ba
Show file tree
Hide file tree
Showing 19 changed files with 1,165 additions and 77 deletions.
35 changes: 33 additions & 2 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, .. })
Expand All @@ -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,
}
}

Expand Down
13 changes: 12 additions & 1 deletion compiler/rustc_ty_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
39 changes: 33 additions & 6 deletions compiler/rustc_typeck/src/check/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand All @@ -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
|
Expand Down
26 changes: 13 additions & 13 deletions src/test/ui/error-codes/E0023.stderr
Original file line number Diff line number Diff line change
@@ -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
|
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/issues/issue-72574-2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/match/match-pattern-field-mismatch.stderr
Original file line number Diff line number Diff line change
@@ -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
|
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
),
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/pattern/issue-74539.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down
57 changes: 57 additions & 0 deletions src/test/ui/pattern/pat-tuple-field-count-cross.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// aux-build:declarations-for-tuple-field-count-errors.rs

extern crate declarations_for_tuple_field_count_errors;

use declarations_for_tuple_field_count_errors::*;

fn main() {
match Z0 {
Z0() => {} //~ ERROR expected tuple struct or tuple variant, found unit struct `Z0`
Z0(x) => {} //~ ERROR expected tuple struct or tuple variant, found unit struct `Z0`
}
match Z1() {
Z1 => {} //~ ERROR match bindings cannot shadow tuple structs
Z1(x) => {} //~ ERROR this pattern has 1 field, but the corresponding tuple struct has 0 fields
}

match S(1, 2, 3) {
S() => {} //~ ERROR this pattern has 0 fields, but the corresponding tuple struct has 3 fields
S(1) => {} //~ ERROR this pattern has 1 field, but the corresponding tuple struct has 3 fields
S(xyz, abc) => {} //~ ERROR this pattern has 2 fields, but the corresponding tuple struct has 3 fields
S(1, 2, 3, 4) => {} //~ ERROR this pattern has 4 fields, but the corresponding tuple struct has 3 fields
}
match M(1, 2, 3) {
M() => {} //~ ERROR this pattern has 0 fields, but the corresponding tuple struct has 3 fields
M(1) => {} //~ ERROR this pattern has 1 field, but the corresponding tuple struct has 3 fields
M(xyz, abc) => {} //~ ERROR this pattern has 2 fields, but the corresponding tuple struct has 3 fields
M(1, 2, 3, 4) => {} //~ ERROR this pattern has 4 fields, but the corresponding tuple struct has 3 fields
}

match E1::Z0 {
E1::Z0() => {} //~ ERROR expected tuple struct or tuple variant, found unit variant `E1::Z0`
E1::Z0(x) => {} //~ ERROR expected tuple struct or tuple variant, found unit variant `E1::Z0`
}
match E1::Z1() {
E1::Z1 => {} //~ ERROR expected unit struct, unit variant or constant, found tuple variant `E1::Z1`
E1::Z1(x) => {} //~ ERROR this pattern has 1 field, but the corresponding tuple variant has 0 fields
}
match E1::S(1, 2, 3) {
E1::S() => {} //~ ERROR this pattern has 0 fields, but the corresponding tuple variant has 3 fields
E1::S(1) => {} //~ ERROR this pattern has 1 field, but the corresponding tuple variant has 3 fields
E1::S(xyz, abc) => {} //~ ERROR this pattern has 2 fields, but the corresponding tuple variant has 3 fields
E1::S(1, 2, 3, 4) => {} //~ ERROR this pattern has 4 fields, but the corresponding tuple variant has 3 fields
}

match E2::S(1, 2, 3) {
E2::S() => {} //~ ERROR this pattern has 0 fields, but the corresponding tuple variant has 3 fields
E2::S(1) => {} //~ ERROR this pattern has 1 field, but the corresponding tuple variant has 3 fields
E2::S(xyz, abc) => {} //~ ERROR this pattern has 2 fields, but the corresponding tuple variant has 3 fields
E2::S(1, 2, 3, 4) => {} //~ ERROR this pattern has 4 fields, but the corresponding tuple variant has 3 fields
}
match E2::M(1, 2, 3) {
E2::M() => {} //~ ERROR this pattern has 0 fields, but the corresponding tuple variant has 3 fields
E2::M(1) => {} //~ ERROR this pattern has 1 field, but the corresponding tuple variant has 3 fields
E2::M(xyz, abc) => {} //~ ERROR this pattern has 2 fields, but the corresponding tuple variant has 3 fields
E2::M(1, 2, 3, 4) => {} //~ ERROR this pattern has 4 fields, but the corresponding tuple variant has 3 fields
}
}
Loading

0 comments on commit b0d21ba

Please sign in to comment.