From bb0cb9ae9fb2dbebc62627a3f1f4326771338535 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 16 Nov 2022 19:10:38 +0000 Subject: [PATCH] Add a detailed note for missing comma in FRU syntax typo --- .../locales/en-US/hir_typeck.ftl | 8 +++ compiler/rustc_error_messages/src/lib.rs | 1 + compiler/rustc_errors/src/lib.rs | 3 ++ compiler/rustc_hir_typeck/src/errors.rs | 39 ++++++++++++++ compiler/rustc_hir_typeck/src/expr.rs | 51 ++++++++++++++----- .../ui/structs/multi-line-fru-suggestion.rs | 22 ++++++++ .../structs/multi-line-fru-suggestion.stderr | 25 +++++++++ .../ui/structs/struct-record-suggestion.fixed | 7 ++- .../ui/structs/struct-record-suggestion.rs | 3 +- .../structs/struct-record-suggestion.stderr | 27 +++++----- 10 files changed, 155 insertions(+), 31 deletions(-) create mode 100644 compiler/rustc_error_messages/locales/en-US/hir_typeck.ftl create mode 100644 src/test/ui/structs/multi-line-fru-suggestion.rs create mode 100644 src/test/ui/structs/multi-line-fru-suggestion.stderr diff --git a/compiler/rustc_error_messages/locales/en-US/hir_typeck.ftl b/compiler/rustc_error_messages/locales/en-US/hir_typeck.ftl new file mode 100644 index 00000000000..2ce417a8c78 --- /dev/null +++ b/compiler/rustc_error_messages/locales/en-US/hir_typeck.ftl @@ -0,0 +1,8 @@ +hir_typeck_fru_note = this expression may have been misinterpreted as a `..` range expression +hir_typeck_fru_expr = this expression does not end in a comma... +hir_typeck_fru_expr2 = ... so this is interpreted as a `..` range expression, instead of functional record update syntax +hir_typeck_fru_suggestion = + to set the remaining fields{$expr -> + [NONE]{""} + *[other] {" "}from `{$expr}` + }, separate the last named field with a comma diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs index 0b1b75471a6..1dbea77ef70 100644 --- a/compiler/rustc_error_messages/src/lib.rs +++ b/compiler/rustc_error_messages/src/lib.rs @@ -51,6 +51,7 @@ fluent_messages! { errors => "../locales/en-US/errors.ftl", expand => "../locales/en-US/expand.ftl", hir_analysis => "../locales/en-US/hir_analysis.ftl", + hir_typeck => "../locales/en-US/hir_typeck.ftl", infer => "../locales/en-US/infer.ftl", interface => "../locales/en-US/interface.ftl", lint => "../locales/en-US/lint.ftl", diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 170d4341ae7..ab518137342 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -466,6 +466,9 @@ pub enum StashKey { /// When an invalid lifetime e.g. `'2` should be reinterpreted /// as a char literal in the parser LifetimeIsChar, + /// Maybe there was a typo where a comma was forgotten before + /// FRU syntax + MaybeFruTypo, } fn default_track_diagnostic(_: &Diagnostic) {} diff --git a/compiler/rustc_hir_typeck/src/errors.rs b/compiler/rustc_hir_typeck/src/errors.rs index afac6e7d94a..32265bcca45 100644 --- a/compiler/rustc_hir_typeck/src/errors.rs +++ b/compiler/rustc_hir_typeck/src/errors.rs @@ -1,4 +1,5 @@ //! Errors emitted by `rustc_hir_analysis`. +use rustc_errors::{AddToDiagnostic, Applicability, Diagnostic, MultiSpan, SubdiagnosticMessage}; use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_middle::ty::Ty; use rustc_span::{symbol::Ident, Span}; @@ -133,3 +134,41 @@ pub struct OpMethodGenericParams { pub span: Span, pub method_name: String, } + +pub struct TypeMismatchFruTypo { + /// Span of the LHS of the range + pub expr_span: Span, + /// Span of the `..RHS` part of the range + pub fru_span: Span, + /// Rendered expression of the RHS of the range + pub expr: Option, +} + +impl AddToDiagnostic for TypeMismatchFruTypo { + fn add_to_diagnostic_with(self, diag: &mut Diagnostic, _: F) + where + F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage, + { + diag.set_arg("expr", self.expr.as_deref().unwrap_or("NONE")); + + // Only explain that `a ..b` is a range if it's split up + if self.expr_span.between(self.fru_span).is_empty() { + diag.span_note( + self.expr_span.to(self.fru_span), + rustc_errors::fluent::hir_typeck_fru_note, + ); + } else { + let mut multispan: MultiSpan = vec![self.expr_span, self.fru_span].into(); + multispan.push_span_label(self.expr_span, rustc_errors::fluent::hir_typeck_fru_expr); + multispan.push_span_label(self.fru_span, rustc_errors::fluent::hir_typeck_fru_expr2); + diag.span_note(multispan, rustc_errors::fluent::hir_typeck_fru_note); + } + + diag.span_suggestion( + self.expr_span.shrink_to_hi(), + rustc_errors::fluent::hir_typeck_fru_suggestion, + ", ", + Applicability::MaybeIncorrect, + ); + } +} diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 13a03b33de8..24c38ecc190 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -5,6 +5,7 @@ use crate::cast; use crate::coercion::CoerceMany; use crate::coercion::DynamicCoerceMany; +use crate::errors::TypeMismatchFruTypo; use crate::errors::{AddressOfTemporaryTaken, ReturnStmtOutsideOfFnBody, StructExprNonExhaustive}; use crate::errors::{ FieldMultiplySpecifiedInInitializer, FunctionalRecordUpdateOnNonStruct, @@ -1614,10 +1615,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.demand_coerce_diag(&field.expr, ty, field_type, None, AllowTwoPhase::No); if let Some(mut diag) = diag { - if idx == ast_fields.len() - 1 && remaining_fields.is_empty() { - self.suggest_fru_from_range(field, variant, substs, &mut diag); + if idx == ast_fields.len() - 1 { + if remaining_fields.is_empty() { + self.suggest_fru_from_range(field, variant, substs, &mut diag); + diag.emit(); + } else { + diag.stash(field.span, StashKey::MaybeFruTypo); + } + } else { + diag.emit(); } - diag.emit(); } } @@ -1875,19 +1882,39 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .map(|adt| adt.did()) != range_def_id { - let instead = self + // Suppress any range expr type mismatches + if let Some(mut diag) = self + .tcx + .sess + .diagnostic() + .steal_diagnostic(last_expr_field.span, StashKey::MaybeFruTypo) + { + diag.delay_as_bug(); + } + + // Use a (somewhat arbitrary) filtering heuristic to avoid printing + // expressions that are either too long, or have control character + //such as newlines in them. + let expr = self .tcx .sess .source_map() .span_to_snippet(range_end.expr.span) - .map(|s| format!(" from `{s}`")) - .unwrap_or_default(); - err.span_suggestion( - range_start.span.shrink_to_hi(), - &format!("to set the remaining fields{instead}, separate the last named field with a comma"), - ",", - Applicability::MaybeIncorrect, - ); + .ok() + .filter(|s| s.len() < 25 && !s.contains(|c: char| c.is_control())); + + let fru_span = self + .tcx + .sess + .source_map() + .span_extend_while(range_start.span, |c| c.is_whitespace()) + .unwrap_or(range_start.span).shrink_to_hi().to(range_end.span); + + err.subdiagnostic(TypeMismatchFruTypo { + expr_span: range_start.span, + fru_span, + expr, + }); } } diff --git a/src/test/ui/structs/multi-line-fru-suggestion.rs b/src/test/ui/structs/multi-line-fru-suggestion.rs new file mode 100644 index 00000000000..7b2b139142e --- /dev/null +++ b/src/test/ui/structs/multi-line-fru-suggestion.rs @@ -0,0 +1,22 @@ +#[derive(Default)] +struct Inner { + a: u8, + b: u8, +} + +#[derive(Default)] +struct Outer { + inner: Inner, + defaulted: u8, +} + +fn main(){ + Outer { + //~^ ERROR missing field `defaulted` in initializer of `Outer` + inner: Inner { + a: 1, + b: 2, + } + ..Default::default() + }; +} diff --git a/src/test/ui/structs/multi-line-fru-suggestion.stderr b/src/test/ui/structs/multi-line-fru-suggestion.stderr new file mode 100644 index 00000000000..8bbd3ace7d2 --- /dev/null +++ b/src/test/ui/structs/multi-line-fru-suggestion.stderr @@ -0,0 +1,25 @@ +error[E0063]: missing field `defaulted` in initializer of `Outer` + --> $DIR/multi-line-fru-suggestion.rs:14:5 + | +LL | Outer { + | ^^^^^ missing `defaulted` + | +note: this expression may have been misinterpreted as a `..` range expression + --> $DIR/multi-line-fru-suggestion.rs:16:16 + | +LL | inner: Inner { + | ________________^ +LL | | a: 1, +LL | | b: 2, +LL | | } + | |_________^ this expression does not end in a comma... +LL | ..Default::default() + | ^^^^^^^^^^^^^^^^^^^^ ... so this is interpreted as a `..` range expression, instead of functional record update syntax +help: to set the remaining fields from `Default::default()`, separate the last named field with a comma + | +LL | }, + | + + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0063`. diff --git a/src/test/ui/structs/struct-record-suggestion.fixed b/src/test/ui/structs/struct-record-suggestion.fixed index 49e38b196de..d93a62185b3 100644 --- a/src/test/ui/structs/struct-record-suggestion.fixed +++ b/src/test/ui/structs/struct-record-suggestion.fixed @@ -7,9 +7,8 @@ struct A { } fn a() { - let q = A { c: 5,..Default::default() }; - //~^ ERROR mismatched types - //~| ERROR missing fields + let q = A { c: 5, ..Default::default() }; + //~^ ERROR missing fields //~| HELP separate the last named field with a comma let r = A { c: 5, ..Default::default() }; assert_eq!(q, r); @@ -21,7 +20,7 @@ struct B { } fn b() { - let q = B { b: 1,..Default::default() }; + let q = B { b: 1, ..Default::default() }; //~^ ERROR mismatched types //~| HELP separate the last named field with a comma let r = B { b: 1 }; diff --git a/src/test/ui/structs/struct-record-suggestion.rs b/src/test/ui/structs/struct-record-suggestion.rs index 901f310c8bd..f0fd1c94e9a 100644 --- a/src/test/ui/structs/struct-record-suggestion.rs +++ b/src/test/ui/structs/struct-record-suggestion.rs @@ -8,8 +8,7 @@ struct A { fn a() { let q = A { c: 5..Default::default() }; - //~^ ERROR mismatched types - //~| ERROR missing fields + //~^ ERROR missing fields //~| HELP separate the last named field with a comma let r = A { c: 5, ..Default::default() }; assert_eq!(q, r); diff --git a/src/test/ui/structs/struct-record-suggestion.stderr b/src/test/ui/structs/struct-record-suggestion.stderr index 66e9f021ed6..f4fd655e698 100644 --- a/src/test/ui/structs/struct-record-suggestion.stderr +++ b/src/test/ui/structs/struct-record-suggestion.stderr @@ -1,37 +1,38 @@ -error[E0308]: mismatched types - --> $DIR/struct-record-suggestion.rs:10:20 - | -LL | let q = A { c: 5..Default::default() }; - | ^^^^^^^^^^^^^^^^^^^^^ expected `u64`, found struct `std::ops::Range` - | - = note: expected type `u64` - found struct `std::ops::Range<{integer}>` - error[E0063]: missing fields `b` and `d` in initializer of `A` --> $DIR/struct-record-suggestion.rs:10:13 | LL | let q = A { c: 5..Default::default() }; | ^ missing `b` and `d` | +note: this expression may have been misinterpreted as a `..` range expression + --> $DIR/struct-record-suggestion.rs:10:20 + | +LL | let q = A { c: 5..Default::default() }; + | ^^^^^^^^^^^^^^^^^^^^^ help: to set the remaining fields from `Default::default()`, separate the last named field with a comma | -LL | let q = A { c: 5,..Default::default() }; +LL | let q = A { c: 5, ..Default::default() }; | + error[E0308]: mismatched types - --> $DIR/struct-record-suggestion.rs:24:20 + --> $DIR/struct-record-suggestion.rs:23:20 | LL | let q = B { b: 1..Default::default() }; | ^^^^^^^^^^^^^^^^^^^^^ expected `u32`, found struct `std::ops::Range` | = note: expected type `u32` found struct `std::ops::Range<{integer}>` +note: this expression may have been misinterpreted as a `..` range expression + --> $DIR/struct-record-suggestion.rs:23:20 + | +LL | let q = B { b: 1..Default::default() }; + | ^^^^^^^^^^^^^^^^^^^^^ help: to set the remaining fields from `Default::default()`, separate the last named field with a comma | -LL | let q = B { b: 1,..Default::default() }; +LL | let q = B { b: 1, ..Default::default() }; | + -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors Some errors have detailed explanations: E0063, E0308. For more information about an error, try `rustc --explain E0063`.