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

Catch stray { in let-chains #117770

Merged
merged 5 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
38 changes: 37 additions & 1 deletion compiler/rustc_parse/src/lexer/tokentrees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use super::{StringReader, UnmatchedDelim};
use rustc_ast::token::{self, Delimiter, Token};
use rustc_ast::tokenstream::{DelimSpan, Spacing, TokenStream, TokenTree};
use rustc_ast_pretty::pprust::token_to_string;
use rustc_errors::PErr;
use rustc_errors::{Applicability, PErr};
use rustc_span::symbol::kw;

pub(super) struct TokenTreesReader<'a> {
string_reader: StringReader<'a>,
Expand Down Expand Up @@ -121,9 +122,44 @@ impl<'a> TokenTreesReader<'a> {
// out instead of complaining about the unclosed delims.
let mut parser = crate::stream_to_parser(self.string_reader.sess, tts, None);
let mut diff_errs = vec![];
// Suggest removing a `{` we think appears in an `if`/`while` condition
// We want to suggest removing a `{` only if we think we're in an `if`/`while` condition, but
// we have no way of tracking this in the lexer itself, so we piggyback on the parser
let mut in_cond = false;
while parser.token != token::Eof {
if let Err(diff_err) = parser.err_diff_marker() {
diff_errs.push(diff_err);
} else if parser.is_keyword_ahead(0, &[kw::If, kw::While]) {
in_cond = true;
} else if matches!(
parser.token.kind,
token::CloseDelim(Delimiter::Brace) | token::FatArrow
) {
// end of the `if`/`while` body, or the end of a `match` guard
in_cond = false;
} else if in_cond && parser.token == token::OpenDelim(Delimiter::Brace) {
// Store the `&&` and `let` to use their spans later when creating the diagnostic
let maybe_andand = parser.look_ahead(1, |t| t.clone());
let maybe_let = parser.look_ahead(2, |t| t.clone());
if maybe_andand == token::OpenDelim(Delimiter::Brace) {
// This might be the beginning of the `if`/`while` body (i.e., the end of the condition)
in_cond = false;
} else if maybe_andand == token::AndAnd && maybe_let.is_keyword(kw::Let) {
let mut err = parser.struct_span_err(
parser.token.span,
"found a `{` in the middle of a let-chain",
);
err.span_suggestion(
parser.token.span,
"consider removing this brace to parse the `let` as part of the same chain",
"", Applicability::MachineApplicable
);
err.span_note(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
err.span_note(
err.span_label(

maybe_andand.span.to(maybe_let.span),
"you might have meant to continue the let-chain here",
);
errs.push(err);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's move this logic to a separate method to make it less difficult to read the happy path. Either the new logic or the whole parser loop.

}
parser.bump();
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1115,7 +1115,7 @@ impl<'a> Parser<'a> {
}

/// Returns whether any of the given keywords are `dist` tokens ahead of the current one.
fn is_keyword_ahead(&self, dist: usize, kws: &[Symbol]) -> bool {
pub fn is_keyword_ahead(&self, dist: usize, kws: &[Symbol]) -> bool {
self.look_ahead(dist, |t| kws.iter().any(|&kw| t.is_keyword(kw)))
}

Expand Down
58 changes: 58 additions & 0 deletions tests/ui/parser/brace-in-let-chain.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// issue #117766

#![feature(let_chains)]
fn main() {
if let () = ()
&& let () = () { //~ERROR: found a `{` in the middle of a let-chain
&& let () = ()
{
}
}

fn quux() {
while let () = ()
&& let () = () { //~ERROR: found a `{` in the middle of a let-chain
&& let () = ()
{
}
}

fn foobar() {
while false {}
{
&& let () = ()
}

fn fubar() {
while false {
{
&& let () = ()
}
}

fn qux() {
let foo = false;
match foo {
_ if foo => {
&& let () = ()
_ => {}
}
}

fn foo() {
{
&& let () = ()
}

fn bar() {
if false {}
{
&& let () = ()
}

fn baz() {
if false {
{
&& let () = ()
}
} //~ERROR: this file contains an unclosed delimiter
71 changes: 71 additions & 0 deletions tests/ui/parser/brace-in-let-chain.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
error: this file contains an unclosed delimiter
--> $DIR/brace-in-let-chain.rs:58:54
|
LL | fn main() {
| - unclosed delimiter
...
LL | fn quux() {
| - unclosed delimiter
...
LL | fn foobar() {
| - unclosed delimiter
...
LL | fn fubar() {
| - unclosed delimiter
...
LL | fn qux() {
| - unclosed delimiter
...
LL | fn foo() {
| - unclosed delimiter
...
LL | fn bar() {
| - unclosed delimiter
...
LL | fn baz() {
| - unclosed delimiter
LL | if false {
LL | {
| - this delimiter might not be properly closed...
LL | && let () = ()
LL | }
| - ...as it matches this but it has different indentation
LL | }
| ^
Comment on lines +1 to +34
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm trying to decide if it's better in general to silence this error (like we do for diff markers) or not is best for these cases... I would leave it as is for now, because I don't know if I can come up with a case where doing so would be detrimental, but suspect we can silence it.


error: found a `{` in the middle of a let-chain
--> $DIR/brace-in-let-chain.rs:14:24
|
LL | && let () = () {
| ^
|
note: you might have meant to continue the let-chain here
--> $DIR/brace-in-let-chain.rs:15:9
|
LL | && let () = ()
| ^^^^^^
help: consider removing this brace to parse the `let` as part of the same chain
|
LL - && let () = () {
LL + && let () = ()
|

error: found a `{` in the middle of a let-chain
--> $DIR/brace-in-let-chain.rs:6:24
|
LL | && let () = () {
| ^
|
note: you might have meant to continue the let-chain here
--> $DIR/brace-in-let-chain.rs:7:9
|
LL | && let () = ()
| ^^^^^^
help: consider removing this brace to parse the `let` as part of the same chain
|
LL - && let () = () {
LL + && let () = ()
|

error: aborting due to 3 previous errors