From 3983881d4e00c2b12d1b5b0319b4c61d72926917 Mon Sep 17 00:00:00 2001 From: yukang Date: Tue, 6 Jun 2023 23:51:09 +0800 Subject: [PATCH 1/3] take care module name for suggesting surround the struct literal in parentheses --- .../rustc_parse/src/parser/diagnostics.rs | 10 +++- compiler/rustc_span/src/source_map.rs | 12 +++++ tests/ui/parser/issues/issue-111692.rs | 32 +++++++++++++ tests/ui/parser/issues/issue-111692.stderr | 46 +++++++++++++++++++ 4 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 tests/ui/parser/issues/issue-111692.rs create mode 100644 tests/ui/parser/issues/issue-111692.stderr diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index c1454039685..2abd485b1be 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -751,10 +751,18 @@ impl<'a> Parser<'a> { tail.could_be_bare_literal = true; if maybe_struct_name.is_ident() && can_be_struct_literal { // Account for `if Example { a: one(), }.is_pos() {}`. + // expand `before` so that we take care of module path such as: + // `foo::Bar { ... } ` + // we expect to suggest `(foo::Bar { ... })` instead of `foo::(Bar { ... })` + let sm = self.sess.source_map(); + let before = maybe_struct_name.span.shrink_to_lo(); + let extend_before = sm.span_extend_prev_while(before, |t| { + t.is_alphanumeric() || t == ':' || t == '_' + }); Err(self.sess.create_err(StructLiteralNeedingParens { span: maybe_struct_name.span.to(expr.span), sugg: StructLiteralNeedingParensSugg { - before: maybe_struct_name.span.shrink_to_lo(), + before: extend_before.unwrap().shrink_to_lo(), after: expr.span.shrink_to_hi(), }, })) diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs index 1824510a974..f354751112f 100644 --- a/compiler/rustc_span/src/source_map.rs +++ b/compiler/rustc_span/src/source_map.rs @@ -744,6 +744,18 @@ impl SourceMap { }) } + /// Extends the given `Span` to previous character while the previous character matches the predicate + pub fn span_extend_prev_while( + &self, + span: Span, + f: impl Fn(char) -> bool, + ) -> Result { + self.span_to_source(span, |s, start, _end| { + let n = s[..start].char_indices().rfind(|&(_, c)| !f(c)).map_or(start, |(i, _)| start - i - 1); + Ok(span.with_lo(span.lo() - BytePos(n as u32))) + }) + } + /// Extends the given `Span` to just before the next occurrence of `c`. pub fn span_extend_to_next_char(&self, sp: Span, c: char, accept_newlines: bool) -> Span { if let Ok(next_source) = self.span_to_next_source(sp) { diff --git a/tests/ui/parser/issues/issue-111692.rs b/tests/ui/parser/issues/issue-111692.rs new file mode 100644 index 00000000000..56096f706a8 --- /dev/null +++ b/tests/ui/parser/issues/issue-111692.rs @@ -0,0 +1,32 @@ +mod module { + #[derive(Eq, PartialEq)] + pub struct Type { + pub x: u8, + pub y: u8, + } + + pub const C: u8 = 32u8; +} + +fn test(x: module::Type) { + if x == module::Type { x: module::C, y: 1 } { //~ ERROR invalid struct literal + } +} + +fn test2(x: module::Type) { + if x ==module::Type { x: module::C, y: 1 } { //~ ERROR invalid struct literal + } +} + + +fn test3(x: module::Type) { + if x == Type { x: module::C, y: 1 } { //~ ERROR invalid struct literal + } +} + +fn test4(x: module::Type) { + if x == demo_module::Type { x: module::C, y: 1 } { //~ ERROR invalid struct literal + } +} + +fn main() { } diff --git a/tests/ui/parser/issues/issue-111692.stderr b/tests/ui/parser/issues/issue-111692.stderr new file mode 100644 index 00000000000..7b09d47301d --- /dev/null +++ b/tests/ui/parser/issues/issue-111692.stderr @@ -0,0 +1,46 @@ +error: invalid struct literal + --> $DIR/issue-111692.rs:12:21 + | +LL | if x == module::Type { x: module::C, y: 1 } { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: you might need to surround the struct literal in parentheses + | +LL | if x == (module::Type { x: module::C, y: 1 }) { + | + + + +error: invalid struct literal + --> $DIR/issue-111692.rs:17:20 + | +LL | if x ==module::Type { x: module::C, y: 1 } { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: you might need to surround the struct literal in parentheses + | +LL | if x ==(module::Type { x: module::C, y: 1 }) { + | + + + +error: invalid struct literal + --> $DIR/issue-111692.rs:23:13 + | +LL | if x == Type { x: module::C, y: 1 } { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: you might need to surround the struct literal in parentheses + | +LL | if x == (Type { x: module::C, y: 1 }) { + | + + + +error: invalid struct literal + --> $DIR/issue-111692.rs:28:26 + | +LL | if x == demo_module::Type { x: module::C, y: 1 } { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: you might need to surround the struct literal in parentheses + | +LL | if x == (demo_module::Type { x: module::C, y: 1 }) { + | + + + +error: aborting due to 4 previous errors + From e3071eaa608301bd4106c304e3c2f433d6507500 Mon Sep 17 00:00:00 2001 From: yukang Date: Tue, 6 Jun 2023 23:56:46 +0800 Subject: [PATCH 2/3] reword the message to suggest surrounding with parentheses --- compiler/rustc_parse/messages.ftl | 2 +- compiler/rustc_span/src/source_map.rs | 5 ++++- tests/ui/parser/issues/issue-111692.stderr | 8 ++++---- .../method-call-on-struct-literal-in-if-condition.stderr | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_parse/messages.ftl b/compiler/rustc_parse/messages.ftl index 9263394508e..35eec2c8e1b 100644 --- a/compiler/rustc_parse/messages.ftl +++ b/compiler/rustc_parse/messages.ftl @@ -695,7 +695,7 @@ parse_struct_literal_body_without_path = parse_struct_literal_needing_parens = invalid struct literal - .suggestion = you might need to surround the struct literal in parentheses + .suggestion = you might need to surround the struct literal with parentheses parse_struct_literal_not_allowed_here = struct literals are not allowed here .suggestion = surround the struct literal with parentheses diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs index f354751112f..c53fe084c4d 100644 --- a/compiler/rustc_span/src/source_map.rs +++ b/compiler/rustc_span/src/source_map.rs @@ -751,7 +751,10 @@ impl SourceMap { f: impl Fn(char) -> bool, ) -> Result { self.span_to_source(span, |s, start, _end| { - let n = s[..start].char_indices().rfind(|&(_, c)| !f(c)).map_or(start, |(i, _)| start - i - 1); + let n = s[..start] + .char_indices() + .rfind(|&(_, c)| !f(c)) + .map_or(start, |(i, _)| start - i - 1); Ok(span.with_lo(span.lo() - BytePos(n as u32))) }) } diff --git a/tests/ui/parser/issues/issue-111692.stderr b/tests/ui/parser/issues/issue-111692.stderr index 7b09d47301d..068b0483b0f 100644 --- a/tests/ui/parser/issues/issue-111692.stderr +++ b/tests/ui/parser/issues/issue-111692.stderr @@ -4,7 +4,7 @@ error: invalid struct literal LL | if x == module::Type { x: module::C, y: 1 } { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -help: you might need to surround the struct literal in parentheses +help: you might need to surround the struct literal with parentheses | LL | if x == (module::Type { x: module::C, y: 1 }) { | + + @@ -15,7 +15,7 @@ error: invalid struct literal LL | if x ==module::Type { x: module::C, y: 1 } { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -help: you might need to surround the struct literal in parentheses +help: you might need to surround the struct literal with parentheses | LL | if x ==(module::Type { x: module::C, y: 1 }) { | + + @@ -26,7 +26,7 @@ error: invalid struct literal LL | if x == Type { x: module::C, y: 1 } { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -help: you might need to surround the struct literal in parentheses +help: you might need to surround the struct literal with parentheses | LL | if x == (Type { x: module::C, y: 1 }) { | + + @@ -37,7 +37,7 @@ error: invalid struct literal LL | if x == demo_module::Type { x: module::C, y: 1 } { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -help: you might need to surround the struct literal in parentheses +help: you might need to surround the struct literal with parentheses | LL | if x == (demo_module::Type { x: module::C, y: 1 }) { | + + diff --git a/tests/ui/parser/method-call-on-struct-literal-in-if-condition.stderr b/tests/ui/parser/method-call-on-struct-literal-in-if-condition.stderr index 7fd7ffc94a5..dedbad90945 100644 --- a/tests/ui/parser/method-call-on-struct-literal-in-if-condition.stderr +++ b/tests/ui/parser/method-call-on-struct-literal-in-if-condition.stderr @@ -4,7 +4,7 @@ error: invalid struct literal LL | if Example { a: one(), }.is_pos() { | ^^^^^^^^^^^^^^^^^^^^^ | -help: you might need to surround the struct literal in parentheses +help: you might need to surround the struct literal with parentheses | LL | if (Example { a: one(), }).is_pos() { | + + From f54e75730bb33d08281c91a9b77332606ec69942 Mon Sep 17 00:00:00 2001 From: yukang Date: Sat, 10 Jun 2023 10:34:19 +0800 Subject: [PATCH 3/3] remove unwrap --- .../rustc_parse/src/parser/diagnostics.rs | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 2abd485b1be..228eff1269f 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -756,16 +756,19 @@ impl<'a> Parser<'a> { // we expect to suggest `(foo::Bar { ... })` instead of `foo::(Bar { ... })` let sm = self.sess.source_map(); let before = maybe_struct_name.span.shrink_to_lo(); - let extend_before = sm.span_extend_prev_while(before, |t| { + if let Ok(extend_before) = sm.span_extend_prev_while(before, |t| { t.is_alphanumeric() || t == ':' || t == '_' - }); - Err(self.sess.create_err(StructLiteralNeedingParens { - span: maybe_struct_name.span.to(expr.span), - sugg: StructLiteralNeedingParensSugg { - before: extend_before.unwrap().shrink_to_lo(), - after: expr.span.shrink_to_hi(), - }, - })) + }) { + Err(self.sess.create_err(StructLiteralNeedingParens { + span: maybe_struct_name.span.to(expr.span), + sugg: StructLiteralNeedingParensSugg { + before: extend_before.shrink_to_lo(), + after: expr.span.shrink_to_hi(), + }, + })) + } else { + return None; + } } else { self.sess.emit_err(StructLiteralBodyWithoutPath { span: expr.span,