Skip to content

Commit

Permalink
Rollup merge of #92808 - compiler-errors:wrap-struct-shorthand-field-…
Browse files Browse the repository at this point in the history
…in-variant, r=davidtwco

Fix `try wrapping expression in variant` suggestion with struct field shorthand

Fixes a broken suggestion: [playground](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=83fe2dbfe1485f8cfca1aef2a6582e77)

before:
```
error[E0308]: mismatched types
 --> src/main.rs:7:19
  |
7 |     let x = Foo { bar };
  |                   ^^^ expected enum `Option`, found integer
  |
  = note: expected enum `Option<i32>`
             found type `{integer}`
help: try wrapping the expression in `Some`
  |
7 |     let x = Foo { Some(bar) };
  |                   +++++   +
```

after:
```
error[E0308]: mismatched types
 --> src/main.rs:7:19
  |
7 |     let x = Foo { bar };
  |                   ^^^ expected enum `Option`, found integer
  |
  = note: expected enum `Option<i32>`
             found type `{integer}`
help: try wrapping the expression in `Some`
  |
7 |     let x = Foo { bar: Some(bar) };
  |                   ~~~~~~~~~~~~~~
```

r? ``@m-ou-se``
since you touched the code last in #91080
  • Loading branch information
matthiaskrgr authored Jan 17, 2022
2 parents 3de7276 + 272fb23 commit ff1b653
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 94 deletions.
184 changes: 102 additions & 82 deletions compiler/rustc_typeck/src/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_middle::ty::adjustment::AllowTwoPhase;
use rustc_middle::ty::error::{ExpectedFound, TypeError};
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::{self, AssocItem, Ty, TypeAndMut};
use rustc_span::symbol::sym;
use rustc_span::symbol::{sym, Symbol};
use rustc_span::{BytePos, Span};

use super::method::probe;
Expand All @@ -24,7 +24,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub fn emit_coerce_suggestions(
&self,
err: &mut DiagnosticBuilder<'_>,
expr: &hir::Expr<'_>,
expr: &hir::Expr<'tcx>,
expr_ty: Ty<'tcx>,
expected: Ty<'tcx>,
expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
Expand Down Expand Up @@ -109,7 +109,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

pub fn demand_coerce(
&self,
expr: &hir::Expr<'_>,
expr: &hir::Expr<'tcx>,
checked_ty: Ty<'tcx>,
expected: Ty<'tcx>,
expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
Expand All @@ -129,7 +129,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// will be permitted if the diverges flag is currently "always".
pub fn demand_coerce_diag(
&self,
expr: &hir::Expr<'_>,
expr: &hir::Expr<'tcx>,
checked_ty: Ty<'tcx>,
expected: Ty<'tcx>,
expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
Expand Down Expand Up @@ -338,31 +338,40 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
})
.collect();

if let [variant] = &compatible_variants[..] {
// Just a single matching variant.
err.multipart_suggestion(
&format!("try wrapping the expression in `{}`", variant),
vec![
(expr.span.shrink_to_lo(), format!("{}(", variant)),
(expr.span.shrink_to_hi(), ")".to_string()),
],
Applicability::MaybeIncorrect,
);
} else if compatible_variants.len() > 1 {
// More than one matching variant.
err.multipart_suggestions(
&format!(
"try wrapping the expression in a variant of `{}`",
self.tcx.def_path_str(expected_adt.did)
),
compatible_variants.into_iter().map(|variant| {
let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
Some(ident) => format!("{}: ", ident),
None => format!(""),
};

match &compatible_variants[..] {
[] => { /* No variants to format */ }
[variant] => {
// Just a single matching variant.
err.multipart_suggestion_verbose(
&format!("try wrapping the expression in `{}`", variant),
vec![
(expr.span.shrink_to_lo(), format!("{}(", variant)),
(expr.span.shrink_to_lo(), format!("{}{}(", prefix, variant)),
(expr.span.shrink_to_hi(), ")".to_string()),
]
}),
Applicability::MaybeIncorrect,
);
],
Applicability::MaybeIncorrect,
);
}
_ => {
// More than one matching variant.
err.multipart_suggestions(
&format!(
"try wrapping the expression in a variant of `{}`",
self.tcx.def_path_str(expected_adt.did)
),
compatible_variants.into_iter().map(|variant| {
vec![
(expr.span.shrink_to_lo(), format!("{}{}(", prefix, variant)),
(expr.span.shrink_to_hi(), ")".to_string()),
]
}),
Applicability::MaybeIncorrect,
);
}
}
}
}
Expand Down Expand Up @@ -483,33 +492,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

crate fn is_hir_id_from_struct_pattern_shorthand_field(
crate fn maybe_get_struct_pattern_shorthand_field(
&self,
hir_id: hir::HirId,
sp: Span,
) -> bool {
let sm = self.sess().source_map();
let parent_id = self.tcx.hir().get_parent_node(hir_id);
if let Some(parent) = self.tcx.hir().find(parent_id) {
// Account for fields
if let Node::Expr(hir::Expr { kind: hir::ExprKind::Struct(_, fields, ..), .. }) = parent
{
if let Ok(src) = sm.span_to_snippet(sp) {
for field in *fields {
if field.ident.as_str() == src && field.is_shorthand {
return true;
}
expr: &hir::Expr<'_>,
) -> Option<Symbol> {
let hir = self.tcx.hir();
let local = match expr {
hir::Expr {
kind:
hir::ExprKind::Path(hir::QPath::Resolved(
None,
hir::Path {
res: hir::def::Res::Local(_),
segments: [hir::PathSegment { ident, .. }],
..
},
)),
..
} => Some(ident),
_ => None,
}?;

match hir.find(hir.get_parent_node(expr.hir_id))? {
Node::Expr(hir::Expr { kind: hir::ExprKind::Struct(_, fields, ..), .. }) => {
for field in *fields {
if field.ident.name == local.name && field.is_shorthand {
return Some(local.name);
}
}
}
_ => {}
}
false

None
}

/// If the given `HirId` corresponds to a block with a trailing expression, return that expression
crate fn maybe_get_block_expr(&self, hir_id: hir::HirId) -> Option<&'tcx hir::Expr<'tcx>> {
match self.tcx.hir().find(hir_id)? {
Node::Expr(hir::Expr { kind: hir::ExprKind::Block(block, ..), .. }) => block.expr,
crate fn maybe_get_block_expr(&self, expr: &hir::Expr<'tcx>) -> Option<&'tcx hir::Expr<'tcx>> {
match expr {
hir::Expr { kind: hir::ExprKind::Block(block, ..), .. } => block.expr,
_ => None,
}
}
Expand Down Expand Up @@ -547,7 +568,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// `&mut`!".
pub fn check_ref(
&self,
expr: &hir::Expr<'_>,
expr: &hir::Expr<'tcx>,
checked_ty: Ty<'tcx>,
expected: Ty<'tcx>,
) -> Option<(Span, &'static str, String, Applicability, bool /* verbose */)> {
Expand All @@ -565,9 +586,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
s.strip_prefix(old).map(|stripped| new.to_string() + stripped)
};

let is_struct_pat_shorthand_field =
self.is_hir_id_from_struct_pattern_shorthand_field(expr.hir_id, sp);

// `ExprKind::DropTemps` is semantically irrelevant for these suggestions.
let expr = expr.peel_drop_temps();

Expand Down Expand Up @@ -661,11 +679,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
false,
));
}
let field_name = if is_struct_pat_shorthand_field {
format!("{}: ", sugg_expr)
} else {
String::new()

let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
Some(ident) => format!("{}: ", ident),
None => format!(""),
};

if let Some(hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::Assign(left_expr, ..),
..
Expand Down Expand Up @@ -695,14 +714,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
hir::Mutability::Mut => (
sp,
"consider mutably borrowing here",
format!("{}&mut {}", field_name, sugg_expr),
format!("{}&mut {}", prefix, sugg_expr),
Applicability::MachineApplicable,
false,
),
hir::Mutability::Not => (
sp,
"consider borrowing here",
format!("{}&{}", field_name, sugg_expr),
format!("{}&{}", prefix, sugg_expr),
Applicability::MachineApplicable,
false,
),
Expand Down Expand Up @@ -846,32 +865,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if self.infcx.type_is_copy_modulo_regions(self.param_env, expected, sp)
|| checked_ty.is_box()
{
if let Ok(code) = sm.span_to_snippet(expr.span) {
let message = if checked_ty.is_box() {
"consider unboxing the value"
} else if checked_ty.is_region_ptr() {
"consider dereferencing the borrow"
} else {
"consider dereferencing the type"
};
let (span, suggestion) = if is_struct_pat_shorthand_field {
(expr.span, format!("{}: *{}", code, code))
} else if self.is_else_if_block(expr) {
// Don't suggest nonsense like `else *if`
return None;
} else if let Some(expr) = self.maybe_get_block_expr(expr.hir_id) {
(expr.span.shrink_to_lo(), "*".to_string())
} else {
(expr.span.shrink_to_lo(), "*".to_string())
};
return Some((
span,
message,
suggestion,
Applicability::MachineApplicable,
true,
));
}
let message = if checked_ty.is_box() {
"consider unboxing the value"
} else if checked_ty.is_region_ptr() {
"consider dereferencing the borrow"
} else {
"consider dereferencing the type"
};
let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
Some(ident) => format!("{}: ", ident),
None => format!(""),
};
let (span, suggestion) = if self.is_else_if_block(expr) {
// Don't suggest nonsense like `else *if`
return None;
} else if let Some(expr) = self.maybe_get_block_expr(expr) {
// prefix should be empty here..
(expr.span.shrink_to_lo(), "*".to_string())
} else {
(expr.span.shrink_to_lo(), format!("{}*", prefix))
};
return Some((
span,
message,
suggestion,
Applicability::MachineApplicable,
true,
));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub fn suggest_deref_ref_or_into(
&self,
err: &mut DiagnosticBuilder<'_>,
expr: &hir::Expr<'_>,
expr: &hir::Expr<'tcx>,
expected: Ty<'tcx>,
found: Ty<'tcx>,
expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
Expand All @@ -231,7 +231,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
} else if !self.check_for_cast(err, expr, found, expected, expected_ty_expr) {
let is_struct_pat_shorthand_field =
self.is_hir_id_from_struct_pattern_shorthand_field(expr.hir_id, expr.span);
self.maybe_get_struct_pattern_shorthand_field(expr).is_some();
let methods = self.get_conversion_methods(expr.span, expected, found, expr.hir_id);
if !methods.is_empty() {
if let Ok(expr_text) = self.sess().source_map().span_to_snippet(expr.span) {
Expand Down
8 changes: 8 additions & 0 deletions src/test/ui/did_you_mean/compatible-variants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ enum Hey<A, B> {
B(B),
}

struct Foo {
bar: Option<i32>,
}

fn f() {}

fn a() -> Option<()> {
Expand Down Expand Up @@ -40,4 +44,8 @@ fn main() {
let _: Hey<i32, bool> = false;
//~^ ERROR mismatched types
//~| HELP try wrapping
let bar = 1i32;
let _ = Foo { bar };
//~^ ERROR mismatched types
//~| HELP try wrapping
}
Loading

0 comments on commit ff1b653

Please sign in to comment.