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

Check for missing space between fat arrow and range pattern #107425

Merged
merged 2 commits into from
Jan 29, 2023
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
11 changes: 11 additions & 0 deletions compiler/rustc_error_messages/locales/en-US/parse.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,17 @@ parse_match_arm_body_without_braces = `match` arm body without braces
} with a body
.suggestion_use_comma_not_semicolon = use a comma to end a `match` arm expression

parse_inclusive_range_extra_equals = unexpected `=` after inclusive range
.suggestion_remove_eq = use `..=` instead
.note = inclusive ranges end with a single equals sign (`..=`)

parse_inclusive_range_match_arrow = unexpected `=>` after open range
.suggestion_add_space = add a space between the pattern and `=>`

parse_inclusive_range_no_end = inclusive range with no end
.suggestion_open_range = use `..` instead
.note = inclusive ranges must be bounded at the end (`..=b` or `a..=b`)

parse_struct_literal_not_allowed_here = struct literals are not allowed here
.suggestion = surround the struct literal with parentheses

Expand Down
42 changes: 42 additions & 0 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,48 @@ pub(crate) struct MatchArmBodyWithoutBraces {
pub sub: MatchArmBodyWithoutBracesSugg,
}

#[derive(Diagnostic)]
#[diag(parse_inclusive_range_extra_equals)]
#[note]
pub(crate) struct InclusiveRangeExtraEquals {
#[primary_span]
#[suggestion(
suggestion_remove_eq,
style = "short",
code = "..=",
applicability = "maybe-incorrect"
)]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(parse_inclusive_range_match_arrow)]
pub(crate) struct InclusiveRangeMatchArrow {
#[primary_span]
pub span: Span,
#[suggestion(
suggestion_add_space,
style = "verbose",
code = " ",
applicability = "machine-applicable"
)]
pub after_pat: Span,
}

#[derive(Diagnostic)]
#[diag(parse_inclusive_range_no_end, code = "E0586")]
#[note]
pub(crate) struct InclusiveRangeNoEnd {
#[primary_span]
#[suggestion(
suggestion_open_range,
code = "..",
applicability = "machine-applicable",
style = "short"
)]
pub span: Span,
}

#[derive(Subdiagnostic)]
pub(crate) enum MatchArmBodyWithoutBracesSugg {
#[multipart_suggestion(suggestion_add_braces, applicability = "machine-applicable")]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3168,7 +3168,7 @@ impl<'a> Parser<'a> {
limits: RangeLimits,
) -> ExprKind {
if end.is_none() && limits == RangeLimits::Closed {
self.inclusive_range_with_incorrect_end(self.prev_token.span);
self.inclusive_range_with_incorrect_end();
ExprKind::Err
} else {
ExprKind::Range(start, end, limits)
Expand Down
53 changes: 30 additions & 23 deletions compiler/rustc_parse/src/parser/pat.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use super::{ForceCollect, Parser, PathStyle, TrailingToken};
use crate::errors::RemoveLet;
use crate::errors::{
InclusiveRangeExtraEquals, InclusiveRangeMatchArrow, InclusiveRangeNoEnd, RemoveLet,
};
use crate::{maybe_recover_from_interpolated_ty_qpath, maybe_whole};
use rustc_ast::mut_visit::{noop_visit_pat, MutVisitor};
use rustc_ast::ptr::P;
Expand All @@ -9,7 +11,7 @@ use rustc_ast::{
PatField, PatKind, Path, QSelf, RangeEnd, RangeSyntax,
};
use rustc_ast_pretty::pprust;
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder, ErrorGuaranteed, PResult};
use rustc_errors::{Applicability, DiagnosticBuilder, ErrorGuaranteed, PResult};
use rustc_session::errors::ExprParenthesesNeeded;
use rustc_span::source_map::{respan, Span, Spanned};
use rustc_span::symbol::{kw, sym, Ident};
Expand Down Expand Up @@ -746,47 +748,52 @@ impl<'a> Parser<'a> {
// Parsing e.g. `X..`.
if let RangeEnd::Included(_) = re.node {
// FIXME(Centril): Consider semantic errors instead in `ast_validation`.
self.inclusive_range_with_incorrect_end(re.span);
self.inclusive_range_with_incorrect_end();
}
None
};
Ok(PatKind::Range(Some(begin), end, re))
}

pub(super) fn inclusive_range_with_incorrect_end(&mut self, span: Span) {
pub(super) fn inclusive_range_with_incorrect_end(&mut self) {
let tok = &self.token;

let span = self.prev_token.span;
// If the user typed "..==" instead of "..=", we want to give them
// a specific error message telling them to use "..=".
// If they typed "..=>", suggest they use ".. =>".
// Otherwise, we assume that they meant to type a half open exclusive
// range and give them an error telling them to do that instead.
if matches!(tok.kind, token::Eq) && tok.span.lo() == span.hi() {
let span_with_eq = span.to(tok.span);
let no_space = tok.span.lo() == span.hi();
clubby789 marked this conversation as resolved.
Show resolved Hide resolved
match tok.kind {
token::Eq if no_space => {
let span_with_eq = span.to(tok.span);

// Ensure the user doesn't receive unhelpful unexpected token errors
self.bump();
if self.is_pat_range_end_start(0) {
let _ = self.parse_pat_range_end().map_err(|e| e.cancel());
}
// Ensure the user doesn't receive unhelpful unexpected token errors
self.bump();
if self.is_pat_range_end_start(0) {
let _ = self.parse_pat_range_end().map_err(|e| e.cancel());
}

self.error_inclusive_range_with_extra_equals(span_with_eq);
} else {
self.error_inclusive_range_with_no_end(span);
self.error_inclusive_range_with_extra_equals(span_with_eq);
}
token::Gt if no_space => {
self.error_inclusive_range_match_arrow(span);
}
_ => self.error_inclusive_range_with_no_end(span),
}
}

fn error_inclusive_range_with_extra_equals(&self, span: Span) {
self.struct_span_err(span, "unexpected `=` after inclusive range")
.span_suggestion_short(span, "use `..=` instead", "..=", Applicability::MaybeIncorrect)
.note("inclusive ranges end with a single equals sign (`..=`)")
.emit();
self.sess.emit_err(InclusiveRangeExtraEquals { span });
}

fn error_inclusive_range_match_arrow(&self, span: Span) {
let after_pat = span.with_hi(span.hi() - rustc_span::BytePos(1)).shrink_to_hi();
self.sess.emit_err(InclusiveRangeMatchArrow { span, after_pat });
}

fn error_inclusive_range_with_no_end(&self, span: Span) {
struct_span_err!(self.sess.span_diagnostic, span, E0586, "inclusive range with no end")
.span_suggestion_short(span, "use `..` instead", "..", Applicability::MachineApplicable)
.note("inclusive ranges must be bounded at the end (`..=b` or `a..=b`)")
.emit();
self.sess.emit_err(InclusiveRangeNoEnd { span });
}

/// Parse a range-to pattern, `..X` or `..=X` where `X` remains to be parsed.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fn main() {
let x = 42;
match x {
0..=73 => {},
74..=> {}, //~ ERROR unexpected `=>` after open range
//~^ ERROR expected one of `=>`, `if`, or `|`, found `>`
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error: unexpected `=>` after open range
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have preferred more elaboration in this message. Right now the suggestion helps the user, but it doesn't teach the user what the problem actually was (this was parsed as InclusiveRangeStart(74) > and not ExclusiveRangeStart(74) =>.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I agree that this message is confusing now that I look at it.

It honestly could've probably just been left as the old primary, like "unexpected > after inclusive range", perhaps with a label saying something like:

 --> $DIR/half-open-range-pats-inclusive-match-arrow.rs:5:11
   |
LL |         74..=> {},
   |           ^^^ This is parsed as an inclusive range `..=`

@clubby789, would you like to follow up on this message?

--> $DIR/half-open-range-pats-inclusive-match-arrow.rs:5:11
|
LL | 74..=> {},
| ^^^
|
help: add a space between the pattern and `=>`
|
LL | 74.. => {},
| +

error: expected one of `=>`, `if`, or `|`, found `>`
--> $DIR/half-open-range-pats-inclusive-match-arrow.rs:5:14
|
LL | 74..=> {},
| ^ expected one of `=>`, `if`, or `|`

error: aborting due to 2 previous errors