Recover on attribute in use tree

Author: scrabsha

compiler/rustc_parse/src/errors.rs

@@ -1102,6 +1102,27 @@ pub(crate) struct ArrayBracketsInsteadOfBracesSugg {
     pub right: Span,
 }
 
+#[derive(Diagnostic)]
+#[diag("attributes are not allowed inside imports")]
+pub(crate) struct AttrInUseTree {
+    #[primary_span]
+    pub attr_span: Span,
+    #[subdiagnostic]
+    pub sub: Option<AttrInUseTreeSugg>,
+}
+
+#[derive(Subdiagnostic)]
+#[multipart_suggestion("move the import to its own item", style = "verbose")]
+pub(crate) struct AttrInUseTreeSugg {
+    #[suggestion_part(code = "{code}")]
+    pub use_lo: Span,
+    #[suggestion_part(code = "")]
+    pub attr_span: Span,
+    #[suggestion_part(code = "")]
+    pub tree_span: Span,
+    pub code: String,
+}
+
 #[derive(Diagnostic)]
 #[diag("`match` arm body without braces")]
 pub(crate) struct MatchArmBodyWithoutBraces {

compiler/rustc_parse/src/parser/attr.rs

@@ -302,11 +302,15 @@ impl<'a> Parser<'a> {
     /// Parses an inner part of an attribute (the path and following tokens).
     /// The tokens must be either a delimited token stream, or empty token stream,
     /// or the "legacy" key-value form.
+    ///
+    /// ```none
     ///     PATH `(` TOKEN_STREAM `)`
     ///     PATH `[` TOKEN_STREAM `]`
     ///     PATH `{` TOKEN_STREAM `}`
     ///     PATH
     ///     PATH `=` UNSUFFIXED_LIT
+    /// ```
+    ///
     /// The delimiters or `=` are still put into the resulting token stream.
     pub fn parse_attr_item(&mut self, force_collect: ForceCollect) -> PResult<'a, ast::AttrItem> {
         if let Some(item) = self.eat_metavar_seq_with_matcher(

compiler/rustc_parse/src/parser/item.rs

@@ -430,7 +430,8 @@ impl<'a> Parser<'a> {
     }
 
     fn parse_use_item(&mut self) -> PResult<'a, ItemKind> {
-        let tree = self.parse_use_tree()?;
+        let use_token_span = self.prev_token.span;
+        let tree = self.parse_use_tree(use_token_span, None)?;
         if let Err(mut e) = self.expect_semi() {
             match tree.kind {
                 UseTreeKind::Glob(_) => {
@@ -1291,7 +1292,11 @@ impl<'a> Parser<'a> {
     ///            PATH `::` `{` USE_TREE_LIST `}` |
     ///            PATH [`as` IDENT]
     /// ```
-    fn parse_use_tree(&mut self) -> PResult<'a, UseTree> {
+    fn parse_use_tree<'b>(
+        &mut self,
+        use_token_span: Span,
+        use_path: Option<&'b UsePathList<'b>>,
+    ) -> PResult<'a, UseTree> {
         let lo = self.token.span;
 
         let mut prefix =
@@ -1306,13 +1311,14 @@ impl<'a> Parser<'a> {
                         .push(PathSegment::path_root(lo.shrink_to_lo().with_ctxt(mod_sep_ctxt)));
                 }
 
-                self.parse_use_tree_glob_or_nested()?
+                self.parse_use_tree_glob_or_nested(use_token_span, use_path)?
             } else {
                 // `use path::*;` or `use path::{...};` or `use path;` or `use path as bar;`
                 prefix = self.parse_path(PathStyle::Mod)?;
 
                 if self.eat_path_sep() {
-                    self.parse_use_tree_glob_or_nested()?
+                    let use_path = UsePathList { element: &prefix.segments, prev: use_path };
+                    self.parse_use_tree_glob_or_nested(use_token_span, Some(&use_path))?
                 } else {
                     // Recover from using a colon as path separator.
                     while self.eat_noexpect(&token::Colon) {
@@ -1332,13 +1338,17 @@ impl<'a> Parser<'a> {
     }
 
     /// Parses `*` or `{...}`.
-    fn parse_use_tree_glob_or_nested(&mut self) -> PResult<'a, UseTreeKind> {
+    fn parse_use_tree_glob_or_nested<'b>(
+        &mut self,
+        use_token_span: Span,
+        use_path: Option<&'b UsePathList<'b>>,
+    ) -> PResult<'a, UseTreeKind> {
         Ok(if self.eat(exp!(Star)) {
             UseTreeKind::Glob(self.prev_token.span)
         } else {
             let lo = self.token.span;
             UseTreeKind::Nested {
-                items: self.parse_use_tree_list()?,
+                items: self.parse_use_tree_list(use_token_span, use_path)?,
                 span: lo.to(self.prev_token.span),
             }
         })
@@ -1349,14 +1359,119 @@ impl<'a> Parser<'a> {
     /// ```text
     /// USE_TREE_LIST = ∅ | (USE_TREE `,`)* USE_TREE [`,`]
     /// ```
-    fn parse_use_tree_list(&mut self) -> PResult<'a, ThinVec<(UseTree, ast::NodeId)>> {
+    fn parse_use_tree_list<'b>(
+        &mut self,
+        use_token_span: Span,
+        prefix: Option<&'b UsePathList<'b>>,
+    ) -> PResult<'a, ThinVec<(UseTree, ast::NodeId)>> {
         self.parse_delim_comma_seq(exp!(OpenBrace), exp!(CloseBrace), |p| {
             p.recover_vcs_conflict_marker();
-            Ok((p.parse_use_tree()?, DUMMY_NODE_ID))
+            let recovered = p.recover_attr_in_use_tree();
+            let use_tree = p.parse_use_tree(use_token_span, prefix)?;
+            if let Some(attr_span) = recovered {
+                p.emit_error_attr_in_use_tree(use_token_span, prefix, use_tree.span(), attr_span);
+            }
+
+            Ok((use_tree, DUMMY_NODE_ID))
         })
         .map(|(r, _)| r)
     }
 
+    /// Consumes the tokens that constitute the attribute and returns them.
+    ///
+    /// /!\ Always emit an error if this function returns `Some(_)`.
+    fn recover_attr_in_use_tree(&mut self) -> Option<Span> {
+        if self.check_noexpect(&TokenKind::Pound)
+            && (self.look_ahead(1, |tok| matches!(tok.kind, TokenKind::OpenBracket)) // `#[`
+                || (self.look_ahead(1, |tok| matches!(tok.kind, TokenKind::Bang))
+                    && self.look_ahead(2, |tok| matches!(tok.kind, TokenKind::OpenBracket))))
+        {
+            let start_pos = self.token.span;
+            // `#`
+            self.bump();
+
+            if self.token.kind == TokenKind::Bang {
+                // `!`
+                self.bump();
+            }
+
+            // We checked that there is a `[` right ahead. This should always return `Some(_)`.
+            let delim = self.parse_delim_args_inner().unwrap();
+
+            let end_pos = delim.dspan.close;
+            let attr_span = start_pos.to(end_pos);
+
+            Some(attr_span)
+        } else {
+            None
+        }
+    }
+
+    fn emit_error_attr_in_use_tree<'b>(
+        &self,
+        use_token_span: Span,
+        prefix: Option<&'b UsePathList<'b>>,
+        use_tree_span: Span,
+        attr_span: Span,
+    ) {
+        {
+            let mut prefix = prefix;
+            let attr = self
+                .psess
+                .source_map()
+                .span_to_source(attr_span, |s, start, end| Ok(s[start..end].to_string()))
+                .unwrap();
+
+            let prefix = {
+                let mut tmp = Vec::new();
+                while let Some(prefix_) = prefix {
+                    tmp.push(prefix_.element);
+                    prefix = prefix_.prev;
+                }
+                tmp.reverse();
+                tmp
+            };
+
+            let prefix = prefix
+                .iter()
+                .flat_map(|segments| segments.iter().map(|segment| segment.ident.name.as_str()))
+                .collect::<Vec<_>>()
+                .join("::");
+
+            let mut comma_reached = false;
+            let tree_span = self
+                .psess
+                .source_map()
+                .span_extend_while(use_tree_span, |c| {
+                    if comma_reached {
+                        return false;
+                    }
+                    comma_reached = c == ',';
+                    c.is_whitespace() || comma_reached
+                })
+                .unwrap();
+
+            let use_tree = self
+                .psess
+                .source_map()
+                .span_to_source(use_tree_span, |s, start, end| Ok(s[start..end].to_string()))
+                .unwrap();
+
+            let code = format!("{attr}\nuse {prefix}::{use_tree};\n");
+
+            let err = crate::errors::AttrInUseTree {
+                attr_span,
+                sub: Some(crate::errors::AttrInUseTreeSugg {
+                    use_lo: use_token_span.shrink_to_lo(),
+                    attr_span,
+                    tree_span,
+                    code,
+                }),
+            };
+            self.dcx().emit_err(err);
+        }
+    }
+
     fn parse_rename(&mut self) -> PResult<'a, Option<Ident>> {
         if self.eat_keyword(exp!(As)) {
             self.parse_ident_or_underscore().map(Some)
@@ -3657,3 +3772,8 @@ pub(super) enum FrontMatterParsingMode {
     /// For function pointer types, the `const` and `async` keywords are not permitted.
     FunctionPtrType,
 }
+
+struct UsePathList<'a> {
+    element: &'a [ast::PathSegment],
+    prev: Option<&'a Self>,
+}

compiler/rustc_span/src/source_map.rs

@@ -562,9 +562,13 @@ impl SourceMap {
     /// Extracts the source surrounding the given `Span` using the `extract_source` function. The
     /// extract function takes three arguments: a string slice containing the source, an index in
     /// the slice for the beginning of the span and an index in the slice for the end of the span.
-    pub fn span_to_source<F, T>(&self, sp: Span, extract_source: F) -> Result<T, SpanSnippetError>
+    pub fn span_to_source<F, T>(
+        &self,
+        sp: Span,
+        mut extract_source: F,
+    ) -> Result<T, SpanSnippetError>
     where
-        F: Fn(&str, usize, usize) -> Result<T, SpanSnippetError>,
+        F: FnMut(&str, usize, usize) -> Result<T, SpanSnippetError>,
     {
         let local_begin = self.lookup_byte_offset(sp.lo());
         let local_end = self.lookup_byte_offset(sp.hi());
@@ -719,7 +723,7 @@ impl SourceMap {
     pub fn span_extend_while(
         &self,
         span: Span,
-        f: impl Fn(char) -> bool,
+        mut f: impl FnMut(char) -> bool,
     ) -> Result<Span, SpanSnippetError> {
         self.span_to_source(span, |s, _start, end| {
             let n = s[end..].char_indices().find(|&(_, c)| !f(c)).map_or(s.len() - end, |(i, _)| i);

tests/ui/use/attr-in-use-tree.fixed

@@ -0,0 +1,22 @@
+//@ run-rustfix
+
+#[allow(unused_imports)]
+#[cfg(true)]
+use foo::bar;
+#[cfg(false)]
+use foo::baz;
+use foo::{
+    
+    //~^ ERROR attributes are not allowed inside imports
+    
+    
+    //~^ ERROR attributes are not allowed inside imports
+    
+};
+
+mod foo {
+    pub(crate) mod bar {}
+    pub(crate) mod baz {}
+}
+
+fn main() {}

tests/ui/use/attr-in-use-tree.rs

@@ -3,9 +3,10 @@
 #[allow(unused_imports)]
 use foo::{
     #[cfg(true)]
-    //~^ ERROR expected identifier, found `#`
+    //~^ ERROR attributes are not allowed inside imports
     bar,
     #[cfg(false)]
+    //~^ ERROR attributes are not allowed inside imports
     baz,
 };
 

tests/ui/use/attr-in-use-tree.stderr

@@ -1,8 +1,37 @@
-error: expected identifier, found `#`
-  --> $DIR/attr-in-use-tree.rs:4:5
+error: attributes are not allowed inside imports
+  --> $DIR/attr-in-use-tree.rs:5:5
    |
 LL |     #[cfg(true)]
-   |     ^ expected identifier
+   |     ^^^^^^^^^^^^
+   |
+help: move the import to its own item
+   |
+LL + #[cfg(true)]
+LL + use foo::bar;
+LL | use foo::{
+LL ~     
+LL |
+LL ~     
+   |
+
+error: attributes are not allowed inside imports
+  --> $DIR/attr-in-use-tree.rs:8:5
+   |
+LL |     #[cfg(false)]
+   |     ^^^^^^^^^^^^^
+   |
+help: move the import to its own item
+   |
+LL + #[cfg(false)]
+LL + use foo::baz;
+LL | use foo::{
+LL |     #[cfg(true)]
+LL |
+LL |     bar,
+LL ~     
+LL |
+LL ~     
+   |
 
-error: aborting due to 1 previous error
+error: aborting due to 2 previous errors