Improve unexpected close and mismatch delimiter hint in TokenTreesReader

Author: chenyukang

compiler/rustc_parse/src/lexer/diagnostics.rs

@@ -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");
+        }
+    }
+}

compiler/rustc_parse/src/lexer/mod.rs

@@ -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;

compiler/rustc_parse/src/lexer/tokentrees.rs

@@ -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> {
@@ -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.
@@ -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,
@@ -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
     }
@@ -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
@@ -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;
             }
@@ -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
@@ -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;
                 }
             }
@@ -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
     }

tests/ui/parser/deli-ident-issue-1.rs

@@ -1,4 +1,3 @@
-// error-pattern: this file contains an unclosed delimiter
 #![feature(let_chains)]
 trait Demo {}