Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make spans for tuple patterns in E0023 more precise #88123

Merged
merged 7 commits into from
Aug 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3182,6 +3182,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())
/// ```
camelid marked this conversation as resolved.
Show resolved Hide resolved
pub fn ident(&self) -> Option<Ident> {
match self {
Node::TraitItem(TraitItem { ident, .. })
Expand All @@ -3190,8 +3204,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()),
estebank marked this conversation as resolved.
Show resolved Hide resolved
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
}
camelid marked this conversation as resolved.
Show resolved Hide resolved

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