1
Fork 0

Don't use source-map when detecting struct field shorthand

This commit is contained in:
Michael Goulet 2022-01-13 14:03:56 -08:00
parent dae6dc6b97
commit 272fb2395c
4 changed files with 104 additions and 121 deletions

View file

@ -13,7 +13,7 @@ use rustc_middle::ty::adjustment::AllowTwoPhase;
use rustc_middle::ty::error::{ExpectedFound, TypeError}; use rustc_middle::ty::error::{ExpectedFound, TypeError};
use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::{self, AssocItem, Ty, TypeAndMut}; 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 rustc_span::{BytePos, Span};
use super::method::probe; use super::method::probe;
@ -24,7 +24,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub fn emit_coerce_suggestions( pub fn emit_coerce_suggestions(
&self, &self,
err: &mut DiagnosticBuilder<'_>, err: &mut DiagnosticBuilder<'_>,
expr: &hir::Expr<'_>, expr: &hir::Expr<'tcx>,
expr_ty: Ty<'tcx>, expr_ty: Ty<'tcx>,
expected: Ty<'tcx>, expected: Ty<'tcx>,
expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>, expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
@ -109,7 +109,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub fn demand_coerce( pub fn demand_coerce(
&self, &self,
expr: &hir::Expr<'_>, expr: &hir::Expr<'tcx>,
checked_ty: Ty<'tcx>, checked_ty: Ty<'tcx>,
expected: Ty<'tcx>, expected: Ty<'tcx>,
expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>, expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
@ -129,7 +129,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// will be permitted if the diverges flag is currently "always". /// will be permitted if the diverges flag is currently "always".
pub fn demand_coerce_diag( pub fn demand_coerce_diag(
&self, &self,
expr: &hir::Expr<'_>, expr: &hir::Expr<'tcx>,
checked_ty: Ty<'tcx>, checked_ty: Ty<'tcx>,
expected: Ty<'tcx>, expected: Ty<'tcx>,
expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>, expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
@ -338,67 +338,39 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}) })
.collect(); .collect();
if self.is_hir_id_from_struct_pattern_shorthand_field(expr.hir_id, expr.span) { let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
if let Ok(code) = self.tcx.sess.source_map().span_to_snippet(expr.span) { Some(ident) => format!("{}: ", ident),
match &compatible_variants[..] { None => format!(""),
[] => { /* No variants to format */ } };
[variant] => {
// Just a single matching variant. match &compatible_variants[..] {
err.span_suggestion_verbose( [] => { /* No variants to format */ }
expr.span, [variant] => {
&format!("try wrapping the expression in `{}`", variant), // Just a single matching variant.
format!("{}: {}({})", code, variant, code), err.multipart_suggestion_verbose(
Applicability::MaybeIncorrect, &format!("try wrapping the expression in `{}`", variant),
); vec![
} (expr.span.shrink_to_lo(), format!("{}{}(", prefix, variant)),
_ => { (expr.span.shrink_to_hi(), ")".to_string()),
// More than one matching variant. ],
err.span_suggestions( Applicability::MaybeIncorrect,
expr.span, );
&format!(
"try wrapping the expression in a variant of `{}`",
self.tcx.def_path_str(expected_adt.did)
),
compatible_variants
.into_iter()
.map(|variant| format!("{}: {}({})", code, variant, code)),
Applicability::MaybeIncorrect,
);
}
}
} else {
/* Can't format this without a snippet */
} }
} else { _ => {
match &compatible_variants[..] { // More than one matching variant.
[] => { /* No variants to format */ } err.multipart_suggestions(
[variant] => { &format!(
// Just a single matching variant. "try wrapping the expression in a variant of `{}`",
err.multipart_suggestion_verbose( self.tcx.def_path_str(expected_adt.did)
&format!("try wrapping the expression in `{}`", variant), ),
compatible_variants.into_iter().map(|variant| {
vec![ vec![
(expr.span.shrink_to_lo(), format!("{}(", variant)), (expr.span.shrink_to_lo(), format!("{}{}(", prefix, variant)),
(expr.span.shrink_to_hi(), ")".to_string()), (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!("{}(", variant)),
(expr.span.shrink_to_hi(), ")".to_string()),
]
}),
Applicability::MaybeIncorrect,
);
}
} }
} }
} }
@ -520,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, &self,
hir_id: hir::HirId, expr: &hir::Expr<'_>,
sp: Span, ) -> Option<Symbol> {
) -> bool { let hir = self.tcx.hir();
let sm = self.sess().source_map(); let local = match expr {
let parent_id = self.tcx.hir().get_parent_node(hir_id); hir::Expr {
if let Some(parent) = self.tcx.hir().find(parent_id) { kind:
// Account for fields hir::ExprKind::Path(hir::QPath::Resolved(
if let Node::Expr(hir::Expr { kind: hir::ExprKind::Struct(_, fields, ..), .. }) = parent None,
{ hir::Path {
if let Ok(src) = sm.span_to_snippet(sp) { res: hir::def::Res::Local(_),
for field in *fields { segments: [hir::PathSegment { ident, .. }],
if field.ident.as_str() == src && field.is_shorthand { ..
return true; },
} )),
..
} => 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 /// 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>> { crate fn maybe_get_block_expr(&self, expr: &hir::Expr<'tcx>) -> Option<&'tcx hir::Expr<'tcx>> {
match self.tcx.hir().find(hir_id)? { match expr {
Node::Expr(hir::Expr { kind: hir::ExprKind::Block(block, ..), .. }) => block.expr, hir::Expr { kind: hir::ExprKind::Block(block, ..), .. } => block.expr,
_ => None, _ => None,
} }
} }
@ -584,7 +568,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// `&mut`!". /// `&mut`!".
pub fn check_ref( pub fn check_ref(
&self, &self,
expr: &hir::Expr<'_>, expr: &hir::Expr<'tcx>,
checked_ty: Ty<'tcx>, checked_ty: Ty<'tcx>,
expected: Ty<'tcx>, expected: Ty<'tcx>,
) -> Option<(Span, &'static str, String, Applicability, bool /* verbose */)> { ) -> Option<(Span, &'static str, String, Applicability, bool /* verbose */)> {
@ -602,9 +586,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
s.strip_prefix(old).map(|stripped| new.to_string() + stripped) 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. // `ExprKind::DropTemps` is semantically irrelevant for these suggestions.
let expr = expr.peel_drop_temps(); let expr = expr.peel_drop_temps();
@ -698,11 +679,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
false, false,
)); ));
} }
let field_name = if is_struct_pat_shorthand_field {
format!("{}: ", sugg_expr) let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
} else { Some(ident) => format!("{}: ", ident),
String::new() None => format!(""),
}; };
if let Some(hir::Node::Expr(hir::Expr { if let Some(hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::Assign(left_expr, ..), kind: hir::ExprKind::Assign(left_expr, ..),
.. ..
@ -732,14 +714,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
hir::Mutability::Mut => ( hir::Mutability::Mut => (
sp, sp,
"consider mutably borrowing here", "consider mutably borrowing here",
format!("{}&mut {}", field_name, sugg_expr), format!("{}&mut {}", prefix, sugg_expr),
Applicability::MachineApplicable, Applicability::MachineApplicable,
false, false,
), ),
hir::Mutability::Not => ( hir::Mutability::Not => (
sp, sp,
"consider borrowing here", "consider borrowing here",
format!("{}&{}", field_name, sugg_expr), format!("{}&{}", prefix, sugg_expr),
Applicability::MachineApplicable, Applicability::MachineApplicable,
false, false,
), ),
@ -883,32 +865,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if self.infcx.type_is_copy_modulo_regions(self.param_env, expected, sp) if self.infcx.type_is_copy_modulo_regions(self.param_env, expected, sp)
|| checked_ty.is_box() || checked_ty.is_box()
{ {
if let Ok(code) = sm.span_to_snippet(expr.span) { let message = if checked_ty.is_box() {
let message = if checked_ty.is_box() { "consider unboxing the value"
"consider unboxing the value" } else if checked_ty.is_region_ptr() {
} else if checked_ty.is_region_ptr() { "consider dereferencing the borrow"
"consider dereferencing the borrow" } else {
} else { "consider dereferencing the type"
"consider dereferencing the type" };
}; let prefix = match self.maybe_get_struct_pattern_shorthand_field(expr) {
let (span, suggestion) = if is_struct_pat_shorthand_field { Some(ident) => format!("{}: ", ident),
(expr.span, format!("{}: *{}", code, code)) None => format!(""),
} else if self.is_else_if_block(expr) { };
// Don't suggest nonsense like `else *if` let (span, suggestion) = if self.is_else_if_block(expr) {
return None; // Don't suggest nonsense like `else *if`
} else if let Some(expr) = self.maybe_get_block_expr(expr.hir_id) { return None;
(expr.span.shrink_to_lo(), "*".to_string()) } else if let Some(expr) = self.maybe_get_block_expr(expr) {
} else { // prefix should be empty here..
(expr.span.shrink_to_lo(), "*".to_string()) (expr.span.shrink_to_lo(), "*".to_string())
}; } else {
return Some(( (expr.span.shrink_to_lo(), format!("{}*", prefix))
span, };
message, return Some((
suggestion, span,
Applicability::MachineApplicable, message,
true, suggestion,
)); Applicability::MachineApplicable,
} true,
));
} }
} }
} }

View file

@ -208,7 +208,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub fn suggest_deref_ref_or_into( pub fn suggest_deref_ref_or_into(
&self, &self,
err: &mut DiagnosticBuilder<'_>, err: &mut DiagnosticBuilder<'_>,
expr: &hir::Expr<'_>, expr: &hir::Expr<'tcx>,
expected: Ty<'tcx>, expected: Ty<'tcx>,
found: Ty<'tcx>, found: Ty<'tcx>,
expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>, expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
@ -231,7 +231,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
} }
} else if !self.check_for_cast(err, expr, found, expected, expected_ty_expr) { } else if !self.check_for_cast(err, expr, found, expected, expected_ty_expr) {
let is_struct_pat_shorthand_field = 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); let methods = self.get_conversion_methods(expr.span, expected, found, expr.hir_id);
if !methods.is_empty() { if !methods.is_empty() {
if let Ok(expr_text) = self.sess().source_map().span_to_snippet(expr.span) { if let Ok(expr_text) = self.sess().source_map().span_to_snippet(expr.span) {

View file

@ -143,7 +143,7 @@ LL | let _ = Foo { bar };
help: try wrapping the expression in `Some` help: try wrapping the expression in `Some`
| |
LL | let _ = Foo { bar: Some(bar) }; LL | let _ = Foo { bar: Some(bar) };
| ~~~~~~~~~~~~~~ | ++++++++++ +
error: aborting due to 9 previous errors error: aborting due to 9 previous errors

View file

@ -87,7 +87,7 @@ LL | let r = R { i };
help: consider dereferencing the borrow help: consider dereferencing the borrow
| |
LL | let r = R { i: *i }; LL | let r = R { i: *i };
| ~~~~~ | ++++
error[E0308]: mismatched types error[E0308]: mismatched types
--> $DIR/deref-suggestion.rs:46:20 --> $DIR/deref-suggestion.rs:46:20