Auto merge of rust-lang#16094 - HKalbasi:rustc-tests-fixup, r=HKalbasi

bors · bors · commit 8c3e28e3e2d4 · 2023-12-11T19:28:32.000Z
Replace `doc_comments_and_attrs` with `collect_attrs`, 2nd round

I didn't understand why the one in the semantics is different. Please tell me if it needs special action.

r? `@Veykril`
diff --git a/crates/hir-def/src/child_by_source.rs b/crates/hir-def/src/child_by_source.rs
@@ -5,8 +5,7 @@
 //! node for a *child*, and get its hir.
 
 use either::Either;
-use hir_expand::HirFileId;
-use syntax::ast::HasDocComments;
+use hir_expand::{attrs::collect_attrs, HirFileId};
 
 use crate::{
     db::DefDatabase,
@@ -118,8 +117,8 @@ impl ChildBySource for ItemScope {
             |(ast_id, calls)| {
                 let adt = ast_id.to_node(db.upcast());
                 calls.for_each(|(attr_id, call_id, calls)| {
-                    if let Some(Either::Left(attr)) =
-                        adt.doc_comments_and_attrs().nth(attr_id.ast_index())
+                    if let Some((_, Either::Left(attr))) =
+                        collect_attrs(&adt).nth(attr_id.ast_index())
                     {
                         res[keys::DERIVE_MACRO_CALL].insert(attr, (attr_id, call_id, calls.into()));
                     }
diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs
@@ -10,14 +10,14 @@ use limit::Limit;
 use mbe::{syntax_node_to_token_tree, ValueResult};
 use rustc_hash::FxHashSet;
 use syntax::{
-    ast::{self, HasAttrs, HasDocComments},
+    ast::{self, HasAttrs},
     AstNode, Parse, SyntaxError, SyntaxNode, SyntaxToken, T,
 };
 use triomphe::Arc;
 
 use crate::{
     ast_id_map::AstIdMap,
-    attrs::RawAttrs,
+    attrs::{collect_attrs, RawAttrs},
     builtin_attr_macro::pseudo_derive_attr_expansion,
     builtin_fn_macro::EagerExpander,
     fixup::{self, SyntaxFixupUndoInfo},
@@ -216,9 +216,9 @@ pub fn expand_speculative(
                 // Attributes may have an input token tree, build the subtree and map for this as well
                 // then try finding a token id for our token if it is inside this input subtree.
                 let item = ast::Item::cast(speculative_args.clone())?;
-                item.doc_comments_and_attrs()
+                collect_attrs(&item)
                     .nth(invoc_attr_index.ast_index())
-                    .and_then(Either::left)
+                    .and_then(|x| Either::left(x.1))
             }?;
             match attr.token_tree() {
                 Some(token_tree) => {
@@ -479,10 +479,9 @@ fn censor_for_macro_input(loc: &MacroCallLoc, node: &SyntaxNode) -> FxHashSet<Sy
             MacroCallKind::Attr { .. } if loc.def.is_attribute_derive() => return None,
             MacroCallKind::Attr { invoc_attr_index, .. } => {
                 cov_mark::hit!(attribute_macro_attr_censoring);
-                ast::Item::cast(node.clone())?
-                    .doc_comments_and_attrs()
+                collect_attrs(&ast::Item::cast(node.clone())?)
                     .nth(invoc_attr_index.ast_index())
-                    .and_then(Either::left)
+                    .and_then(|x| Either::left(x.1))
                     .map(|attr| attr.syntax().clone())
                     .into_iter()
                     .collect()
diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs
@@ -22,6 +22,7 @@ pub mod span;
 pub mod files;
 mod fixup;
 
+use attrs::collect_attrs;
 use triomphe::Arc;
 
 use std::{fmt, hash::Hash};
@@ -32,7 +33,7 @@ use base_db::{
 };
 use either::Either;
 use syntax::{
-    ast::{self, AstNode, HasDocComments},
+    ast::{self, AstNode},
     SyntaxNode, SyntaxToken, TextRange, TextSize,
 };
 
@@ -438,9 +439,9 @@ impl MacroCallLoc {
             MacroCallKind::Derive { ast_id, derive_attr_index, .. } => {
                 // FIXME: handle `cfg_attr`
                 ast_id.with_value(ast_id.to_node(db)).map(|it| {
-                    it.doc_comments_and_attrs()
+                    collect_attrs(&it)
                         .nth(derive_attr_index.ast_index())
-                        .and_then(|it| match it {
+                        .and_then(|it| match it.1 {
                             Either::Left(attr) => Some(attr.syntax().clone()),
                             Either::Right(_) => None,
                         })
@@ -451,9 +452,9 @@ impl MacroCallLoc {
                 if self.def.is_attribute_derive() {
                     // FIXME: handle `cfg_attr`
                     ast_id.with_value(ast_id.to_node(db)).map(|it| {
-                        it.doc_comments_and_attrs()
+                        collect_attrs(&it)
                             .nth(invoc_attr_index.ast_index())
-                            .and_then(|it| match it {
+                            .and_then(|it| match it.1 {
                                 Either::Left(attr) => Some(attr.syntax().clone()),
                                 Either::Right(_) => None,
                             })
@@ -549,24 +550,24 @@ impl MacroCallKind {
             MacroCallKind::Derive { ast_id, derive_attr_index, .. } => {
                 // FIXME: should be the range of the macro name, not the whole derive
                 // FIXME: handle `cfg_attr`
-                ast_id
-                    .to_node(db)
-                    .doc_comments_and_attrs()
+                collect_attrs(&ast_id.to_node(db))
                     .nth(derive_attr_index.ast_index())
                     .expect("missing derive")
+                    .1
                     .expect_left("derive is a doc comment?")
                     .syntax()
                     .text_range()
             }
             // FIXME: handle `cfg_attr`
-            MacroCallKind::Attr { ast_id, invoc_attr_index, .. } => ast_id
-                .to_node(db)
-                .doc_comments_and_attrs()
-                .nth(invoc_attr_index.ast_index())
-                .expect("missing attribute")
-                .expect_left("attribute macro is a doc comment?")
-                .syntax()
-                .text_range(),
+            MacroCallKind::Attr { ast_id, invoc_attr_index, .. } => {
+                collect_attrs(&ast_id.to_node(db))
+                    .nth(invoc_attr_index.ast_index())
+                    .expect("missing attribute")
+                    .1
+                    .expect_left("attribute macro is a doc comment?")
+                    .syntax()
+                    .text_range()
+            }
         };
 
         FileRange { range, file_id }
@@ -737,11 +738,9 @@ impl ExpansionInfo {
         let attr_input_or_mac_def = def.or_else(|| match loc.kind {
             MacroCallKind::Attr { ast_id, invoc_attr_index, .. } => {
                 // FIXME: handle `cfg_attr`
-                let tt = ast_id
-                    .to_node(db)
-                    .doc_comments_and_attrs()
+                let tt = collect_attrs(&ast_id.to_node(db))
                     .nth(invoc_attr_index.ast_index())
-                    .and_then(Either::left)?
+                    .and_then(|x| Either::left(x.1))?
                     .token_tree()?;
                 Some(InFile::new(ast_id.file_id, tt))
             }
diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs
@@ -20,16 +20,16 @@ use hir_def::{
     AsMacroCall, DefWithBodyId, FunctionId, MacroId, TraitId, VariantId,
 };
 use hir_expand::{
-    db::ExpandDatabase, files::InRealFile, name::AsName, ExpansionInfo, InMacroFile, MacroCallId,
-    MacroFileId, MacroFileIdExt,
+    attrs::collect_attrs, db::ExpandDatabase, files::InRealFile, name::AsName, ExpansionInfo,
+    InMacroFile, MacroCallId, MacroFileId, MacroFileIdExt,
 };
 use itertools::Itertools;
 use rustc_hash::{FxHashMap, FxHashSet};
 use smallvec::{smallvec, SmallVec};
 use stdx::TupleExt;
 use syntax::{
     algo::skip_trivia_token,
-    ast::{self, HasAttrs as _, HasDocComments, HasGenericParams, HasLoopBody, IsString as _},
+    ast::{self, HasAttrs as _, HasGenericParams, HasLoopBody, IsString as _},
     match_ast, AstNode, AstToken, Direction, SyntaxKind, SyntaxNode, SyntaxNodePtr, SyntaxToken,
     TextRange, TextSize,
 };
@@ -673,11 +673,22 @@ impl<'db> SemanticsImpl<'db> {
                             }
                             _ => 0,
                         };
+                        // FIXME: here, the attribute's text range is used to strip away all
+                        // entries from the start of the attribute "list" up the the invoking
+                        // attribute. But in
+                        // ```
+                        // mod foo {
+                        //     #![inner]
+                        // }
+                        // ```
+                        // we don't wanna strip away stuff in the `mod foo {` range, that is
+                        // here if the id corresponds to an inner attribute we got strip all
+                        // text ranges of the outer ones, and then all of the inner ones up
+                        // to the invoking attribute so that the inbetween is ignored.
                         let text_range = item.syntax().text_range();
-                        let start = item
-                            .doc_comments_and_attrs()
+                        let start = collect_attrs(&item)
                             .nth(attr_id)
-                            .map(|attr| match attr {
+                            .map(|attr| match attr.1 {
                                 Either::Left(it) => it.syntax().text_range().start(),
                                 Either::Right(it) => it.syntax().text_range().start(),
                             })
diff --git a/crates/ide-diagnostics/src/handlers/unresolved_extern_crate.rs b/crates/ide-diagnostics/src/handlers/unresolved_extern_crate.rs
@@ -44,21 +44,6 @@ extern crate core;
 extern crate self as foo;
 struct Foo;
 use foo::Foo as Bar;
-"#,
-        );
-    }
-
-    #[test]
-    fn regression_panic_with_inner_attribute_in_presence_of_unresolved_crate() {
-        check_diagnostics(
-            r#"
-//- /lib.rs
-  #[macro_use] extern crate doesnotexist;
-//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unresolved extern crate
-    mod _test_inner {
-        #![empty_attr]
-      //^^^^^^^^^^^^^^ error: unresolved macro `empty_attr`
-    }
 "#,
         );
     }
diff --git a/crates/ide-diagnostics/src/handlers/unresolved_macro_call.rs b/crates/ide-diagnostics/src/handlers/unresolved_macro_call.rs
@@ -67,6 +67,18 @@ macro_rules! m { () => {} } }
 
 self::m!(); self::m2!();
                 //^^ error: unresolved macro `self::m2!`
+"#,
+        );
+    }
+
+    #[test]
+    fn regression_panic_with_inner_attribute_in_presence_of_unresolved_crate() {
+        check_diagnostics(
+            r#"
+    mod _test_inner {
+        #![empty_attr]
+      //^^^^^^^^^^^^^^ error: unresolved macro `empty_attr`
+    }
 "#,
         );
     }
diff --git a/crates/syntax/src/ast/traits.rs b/crates/syntax/src/ast/traits.rs
@@ -76,9 +76,6 @@ pub trait HasDocComments: HasAttrs {
     fn doc_comments(&self) -> DocCommentIter {
         DocCommentIter { iter: self.syntax().children_with_tokens() }
     }
-    fn doc_comments_and_attrs(&self) -> AttrDocCommentIter {
-        AttrDocCommentIter { iter: self.syntax().children_with_tokens() }
-    }
 }
 
 impl DocCommentIter {

Original file line number	Diff line number	Diff line change
`@@ -76,9 +76,6 @@ pub trait HasDocComments: HasAttrs {`
`76`	`76`	`fn doc_comments(&self) -> DocCommentIter {`
`77`	`77`	`DocCommentIter { iter: self.syntax().children_with_tokens() }`
`78`	`78`	`}`
`79`		`- fn doc_comments_and_attrs(&self) -> AttrDocCommentIter {`
`80`		`- AttrDocCommentIter { iter: self.syntax().children_with_tokens() }`
`81`		`- }`
`82`	`79`	`}`
`83`	`80`
`84`	`81`	`impl DocCommentIter {`