Skip to content

Commit

Permalink
Improve unexpected close and mismatch delimiter hint in TokenTreesReader
Browse files Browse the repository at this point in the history
  • Loading branch information
chenyukang committed Jan 27, 2023
1 parent c4d00d7 commit cd23323
Show file tree
Hide file tree
Showing 18 changed files with 225 additions and 139 deletions.
119 changes: 119 additions & 0 deletions compiler/rustc_parse/src/lexer/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
use super::UnmatchedBrace;
use rustc_ast::token::Delimiter;
use rustc_errors::Diagnostic;
use rustc_span::source_map::SourceMap;
use rustc_span::Span;

#[derive(Default)]
pub struct TokenTreeDiagInfo {
/// Stack of open delimiters and their spans. Used for error message.
pub open_braces: Vec<(Delimiter, Span)>,
pub unmatched_braces: Vec<UnmatchedBrace>,

/// Used only for error recovery when arriving to EOF with mismatched braces.
pub last_unclosed_found_span: Option<Span>,

/// Collect empty block spans that might have been auto-inserted by editors.
pub empty_block_spans: Vec<Span>,

/// Collect the spans of braces (Open, Close). Used only
/// for detecting if blocks are empty and only braces.
pub matching_block_spans: Vec<(Span, Span)>,
}

pub fn same_identation_level(sm: &SourceMap, open_sp: Span, close_sp: Span) -> bool {
match (sm.span_to_margin(open_sp), sm.span_to_margin(close_sp)) {
(Some(open_padding), Some(close_padding)) => open_padding == close_padding,
_ => false,
}
}

// When we get a `)` or `]` for `{`, we should emit help message here
// it's more friendly compared to report `unmatched error` in later phase
pub fn report_missing_open_delim(
err: &mut Diagnostic,
unmatched_braces: &[UnmatchedBrace],
) -> bool {
let mut reported_missing_open = false;
for unmatch_brace in unmatched_braces.iter() {
if let Some(delim) = unmatch_brace.found_delim
&& matches!(delim, Delimiter::Parenthesis | Delimiter::Bracket)
{
let missed_open = match delim {
Delimiter::Parenthesis => "(",
Delimiter::Bracket => "[",
_ => unreachable!(),
};
err.span_label(
unmatch_brace.found_span.shrink_to_lo(),
format!("missing open `{}` for this delimiter", missed_open),
);
reported_missing_open = true;
}
}
reported_missing_open
}

pub fn report_suspicious_mismatch_block(
err: &mut Diagnostic,
diag_info: &TokenTreeDiagInfo,
sm: &SourceMap,
delim: Delimiter,
) {
if report_missing_open_delim(err, &diag_info.unmatched_braces) {
return;
}

let mut matched_spans: Vec<(Span, bool)> = diag_info
.matching_block_spans
.iter()
.map(|&(open, close)| (open.with_hi(close.lo()), same_identation_level(sm, open, close)))
.collect();

// sort by `lo`, so the large block spans in the front
matched_spans.sort_by(|a, b| a.0.lo().cmp(&b.0.lo()));

// We use larger block whose identation is well to cover those inner mismatched blocks
// O(N^2) here, but we are on error reporting path, so it is fine
for i in 0..matched_spans.len() {
let (block_span, same_ident) = matched_spans[i];
if same_ident {
for j in i + 1..matched_spans.len() {
let (inner_block, inner_same_ident) = matched_spans[j];
if block_span.contains(inner_block) && !inner_same_ident {
matched_spans[j] = (inner_block, true);
}
}
}
}

// Find the inner-most span candidate for final report
let candidate_span =
matched_spans.into_iter().rev().find(|&(_, same_ident)| !same_ident).map(|(span, _)| span);

if let Some(block_span) = candidate_span {
err.span_label(block_span.shrink_to_lo(), "this delimiter might not be properly closed...");
err.span_label(
block_span.shrink_to_hi(),
"...as it matches this but it has different indentation",
);

// If there is a empty block in the mismatched span, note it
if delim == Delimiter::Brace {
for span in diag_info.empty_block_spans.iter() {
if block_span.contains(*span) {
err.span_label(*span, "block is empty, you might have not meant to close it");
break;
}
}
}
} else {
// If there is no suspicious span, give the last properly closed block may help
if let Some(parent) = diag_info.matching_block_spans.last()
&& diag_info.open_braces.last().is_none()
&& diag_info.empty_block_spans.iter().all(|&sp| sp != parent.0.to(parent.1)) {
err.span_label(parent.0, "this opening brace...");
err.span_label(parent.1, "...matches this closing brace");
}
}
}
1 change: 1 addition & 0 deletions compiler/rustc_parse/src/lexer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use rustc_session::parse::ParseSess;
use rustc_span::symbol::{sym, Symbol};
use rustc_span::{edition::Edition, BytePos, Pos, Span};

mod diagnostics;
mod tokentrees;
mod unescape_error_reporting;
mod unicode_chars;
Expand Down
129 changes: 42 additions & 87 deletions compiler/rustc_parse/src/lexer/tokentrees.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,18 @@
use super::diagnostics::report_suspicious_mismatch_block;
use super::diagnostics::same_identation_level;
use super::diagnostics::TokenTreeDiagInfo;
use super::{StringReader, UnmatchedBrace};
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_data_structures::fx::FxHashMap;
use rustc_errors::{PErr, PResult};
use rustc_span::Span;

pub(super) struct TokenTreesReader<'a> {
string_reader: StringReader<'a>,
/// The "next" token, which has been obtained from the `StringReader` but
/// not yet handled by the `TokenTreesReader`.
token: Token,
/// Stack of open delimiters and their spans. Used for error message.
open_braces: Vec<(Delimiter, Span)>,
unmatched_braces: Vec<UnmatchedBrace>,
/// The type and spans for all braces
///
/// Used only for error recovery when arriving to EOF with mismatched braces.
matching_delim_spans: Vec<(Delimiter, Span, Span)>,
last_unclosed_found_span: Option<Span>,
/// Collect empty block spans that might have been auto-inserted by editors.
last_delim_empty_block_spans: FxHashMap<Delimiter, Span>,
/// Collect the spans of braces (Open, Close). Used only
/// for detecting if blocks are empty and only braces.
matching_block_spans: Vec<(Span, Span)>,
diag_info: TokenTreeDiagInfo,
}

impl<'a> TokenTreesReader<'a> {
Expand All @@ -33,15 +22,10 @@ impl<'a> TokenTreesReader<'a> {
let mut tt_reader = TokenTreesReader {
string_reader,
token: Token::dummy(),
open_braces: Vec::new(),
unmatched_braces: Vec::new(),
matching_delim_spans: Vec::new(),
last_unclosed_found_span: None,
last_delim_empty_block_spans: FxHashMap::default(),
matching_block_spans: Vec::new(),
diag_info: TokenTreeDiagInfo::default(),
};
let res = tt_reader.parse_token_trees(/* is_delimited */ false);
(res, tt_reader.unmatched_braces)
(res, tt_reader.diag_info.unmatched_braces)
}

// Parse a stream of tokens into a list of `TokenTree`s.
Expand Down Expand Up @@ -92,9 +76,9 @@ impl<'a> TokenTreesReader<'a> {
fn eof_err(&mut self) -> PErr<'a> {
let msg = "this file contains an unclosed delimiter";
let mut err = self.string_reader.sess.span_diagnostic.struct_span_err(self.token.span, msg);
for &(_, sp) in &self.open_braces {
for &(_, sp) in &self.diag_info.open_braces {
err.span_label(sp, "unclosed delimiter");
self.unmatched_braces.push(UnmatchedBrace {
self.diag_info.unmatched_braces.push(UnmatchedBrace {
expected_delim: Delimiter::Brace,
found_delim: None,
found_span: self.token.span,
Expand All @@ -103,23 +87,13 @@ impl<'a> TokenTreesReader<'a> {
});
}

if let Some((delim, _)) = self.open_braces.last() {
if let Some((_, open_sp, close_sp)) =
self.matching_delim_spans.iter().find(|(d, open_sp, close_sp)| {
let sm = self.string_reader.sess.source_map();
if let Some(close_padding) = sm.span_to_margin(*close_sp) {
if let Some(open_padding) = sm.span_to_margin(*open_sp) {
return delim == d && close_padding != open_padding;
}
}
false
})
// these are in reverse order as they get inserted on close, but
{
// we want the last open/first close
err.span_label(*open_sp, "this delimiter might not be properly closed...");
err.span_label(*close_sp, "...as it matches this but it has different indentation");
}
if let Some((delim, _)) = self.diag_info.open_braces.last() {
report_suspicious_mismatch_block(
&mut err,
&self.diag_info,
&self.string_reader.sess.source_map(),
*delim,
)
}
err
}
Expand All @@ -128,7 +102,7 @@ impl<'a> TokenTreesReader<'a> {
// The span for beginning of the delimited section
let pre_span = self.token.span;

self.open_braces.push((open_delim, self.token.span));
self.diag_info.open_braces.push((open_delim, self.token.span));

// Parse the token trees within the delimiters.
// We stop at any delimiter so we can try to recover if the user
Expand All @@ -137,35 +111,29 @@ impl<'a> TokenTreesReader<'a> {

// Expand to cover the entire delimited token tree
let delim_span = DelimSpan::from_pair(pre_span, self.token.span);
let sm = self.string_reader.sess.source_map();

match self.token.kind {
// Correct delimiter.
token::CloseDelim(close_delim) if close_delim == open_delim => {
let (open_brace, open_brace_span) = self.open_braces.pop().unwrap();
let (open_brace, open_brace_span) = self.diag_info.open_braces.pop().unwrap();
let close_brace_span = self.token.span;

if tts.is_empty() {
if tts.is_empty() && close_delim == Delimiter::Brace {
let empty_block_span = open_brace_span.to(close_brace_span);
let sm = self.string_reader.sess.source_map();
if !sm.is_multiline(empty_block_span) {
// Only track if the block is in the form of `{}`, otherwise it is
// likely that it was written on purpose.
self.last_delim_empty_block_spans.insert(open_delim, empty_block_span);
self.diag_info.empty_block_spans.push(empty_block_span);
}
}

//only add braces
// only add braces
if let (Delimiter::Brace, Delimiter::Brace) = (open_brace, open_delim) {
self.matching_block_spans.push((open_brace_span, close_brace_span));
// Add all the matching spans, we will sort by span later
self.diag_info.matching_block_spans.push((open_brace_span, close_brace_span));
}

if self.open_braces.is_empty() {
// Clear up these spans to avoid suggesting them as we've found
// properly matched delimiters so far for an entire block.
self.matching_delim_spans.clear();
} else {
self.matching_delim_spans.push((open_brace, open_brace_span, close_brace_span));
}
// Move past the closing delimiter.
self.token = self.string_reader.next_token().0;
}
Expand All @@ -174,36 +142,33 @@ impl<'a> TokenTreesReader<'a> {
let mut unclosed_delimiter = None;
let mut candidate = None;

if self.last_unclosed_found_span != Some(self.token.span) {
if self.diag_info.last_unclosed_found_span != Some(self.token.span) {
// do not complain about the same unclosed delimiter multiple times
self.last_unclosed_found_span = Some(self.token.span);
self.diag_info.last_unclosed_found_span = Some(self.token.span);
// This is a conservative error: only report the last unclosed
// delimiter. The previous unclosed delimiters could actually be
// closed! The parser just hasn't gotten to them yet.
if let Some(&(_, sp)) = self.open_braces.last() {
if let Some(&(_, sp)) = self.diag_info.open_braces.last() {
unclosed_delimiter = Some(sp);
};
let sm = self.string_reader.sess.source_map();
if let Some(current_padding) = sm.span_to_margin(self.token.span) {
for (brace, brace_span) in &self.open_braces {
if let Some(padding) = sm.span_to_margin(*brace_span) {
// high likelihood of these two corresponding
if current_padding == padding && brace == &close_delim {
candidate = Some(*brace_span);
}
}
for (brace, brace_span) in &self.diag_info.open_braces {
if same_identation_level(&sm, self.token.span, *brace_span)
&& brace == &close_delim
{
// high likelihood of these two corresponding
candidate = Some(*brace_span);
}
}
let (tok, _) = self.open_braces.pop().unwrap();
self.unmatched_braces.push(UnmatchedBrace {
let (tok, _) = self.diag_info.open_braces.pop().unwrap();
self.diag_info.unmatched_braces.push(UnmatchedBrace {
expected_delim: tok,
found_delim: Some(close_delim),
found_span: self.token.span,
unclosed_span: unclosed_delimiter,
candidate_span: candidate,
});
} else {
self.open_braces.pop();
self.diag_info.open_braces.pop();
}

// If the incorrect delimiter matches an earlier opening
Expand All @@ -213,7 +178,7 @@ impl<'a> TokenTreesReader<'a> {
// fn foo() {
// bar(baz(
// } // Incorrect delimiter but matches the earlier `{`
if !self.open_braces.iter().any(|&(b, _)| b == close_delim) {
if !self.diag_info.open_braces.iter().any(|&(b, _)| b == close_delim) {
self.token = self.string_reader.next_token().0;
}
}
Expand All @@ -236,22 +201,12 @@ impl<'a> TokenTreesReader<'a> {
let mut err =
self.string_reader.sess.span_diagnostic.struct_span_err(self.token.span, &msg);

// Braces are added at the end, so the last element is the biggest block
if let Some(parent) = self.matching_block_spans.last() {
if let Some(span) = self.last_delim_empty_block_spans.remove(&delim) {
// Check if the (empty block) is in the last properly closed block
if (parent.0.to(parent.1)).contains(span) {
err.span_label(span, "block is empty, you might have not meant to close it");
} else {
err.span_label(parent.0, "this opening brace...");
err.span_label(parent.1, "...matches this closing brace");
}
} else {
err.span_label(parent.0, "this opening brace...");
err.span_label(parent.1, "...matches this closing brace");
}
}

report_suspicious_mismatch_block(
&mut err,
&self.diag_info,
&self.string_reader.sess.source_map(),
delim,
);
err.span_label(self.token.span, "unexpected closing delimiter");
err
}
Expand Down
1 change: 0 additions & 1 deletion tests/ui/parser/deli-ident-issue-1.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// error-pattern: this file contains an unclosed delimiter
#![feature(let_chains)]
trait Demo {}

Expand Down
Loading

0 comments on commit cd23323

Please sign in to comment.