Some final touches

10 files changed, 86 insertions, 67 deletions

diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs
index 1d5b7dfa594..bc7b5fb211d 100644
--- a/crates/hir-expand/src/lib.rs
+++ b/crates/hir-expand/src/lib.rs
@@ -663,7 +663,7 @@ impl ExpansionInfo {
         range: TextRange,
     ) -> Option<(FileRange, SyntaxContextId)> {
         debug_assert!(self.expanded.value.text_range().contains_range(range));
-        let mut spans = self.exp_map.spans_for_node_range(range);
+        let mut spans = self.exp_map.spans_for_range(range);
         let SpanData { range, anchor, ctx } = spans.next()?;
         let mut start = range.start();
         let mut end = range.end();
diff --git a/crates/hir-expand/src/quote.rs b/crates/hir-expand/src/quote.rs
index 069bcc3bd8e..0950f5d2875 100644
--- a/crates/hir-expand/src/quote.rs
+++ b/crates/hir-expand/src/quote.rs
@@ -215,10 +215,18 @@ impl_to_to_tokentrees! {
 #[cfg(test)]
 mod tests {
     use crate::tt;
-    use ::tt::Span;
+    use base_db::{
+        span::{SpanAnchor, SyntaxContextId, ROOT_ERASED_FILE_AST_ID},
+        FileId,
+    };
     use expect_test::expect;
+    use syntax::{TextRange, TextSize};
 
-    const DUMMY: tt::SpanData = tt::SpanData::DUMMY;
+    const DUMMY: tt::SpanData = tt::SpanData {
+        range: TextRange::empty(TextSize::new(0)),
+        anchor: SpanAnchor { file_id: FileId::BOGUS, ast_id: ROOT_ERASED_FILE_AST_ID },
+        ctx: SyntaxContextId::ROOT,
+    };
 
     #[test]
     fn test_quote_delimiters() {
@@ -242,10 +250,7 @@ mod tests {
     }
 
     fn mk_ident(name: &str) -> crate::tt::Ident {
-        crate::tt::Ident {
-            text: name.into(),
-            span: <crate::tt::SpanData as crate::tt::Span>::DUMMY,
-        }
+        crate::tt::Ident { text: name.into(), span: DUMMY }
     }
 
     #[test]
@@ -256,8 +261,8 @@ mod tests {
         assert_eq!(quoted.to_string(), "hello");
         let t = format!("{quoted:?}");
         expect![[r#"
-            SUBTREE $$ SpanData { range: 0..0, anchor: SpanAnchor(FileId(0), 0), ctx: SyntaxContextId(0) } SpanData { range: 0..0, anchor: SpanAnchor(FileId(0), 0), ctx: SyntaxContextId(0) }
-              IDENT   hello SpanData { range: 0..0, anchor: SpanAnchor(FileId(0), 0), ctx: SyntaxContextId(0) }"#]].assert_eq(&t);
+            SUBTREE $$ SpanData { range: 0..0, anchor: SpanAnchor(FileId(4294967295), 0), ctx: SyntaxContextId(0) } SpanData { range: 0..0, anchor: SpanAnchor(FileId(4294967295), 0), ctx: SyntaxContextId(0) }
+              IDENT   hello SpanData { range: 0..0, anchor: SpanAnchor(FileId(4294967295), 0), ctx: SyntaxContextId(0) }"#]].assert_eq(&t);
     }
 
     #[test]
@@ -290,8 +295,8 @@ mod tests {
         let list = crate::tt::Subtree {
             delimiter: crate::tt::Delimiter {
                 kind: crate::tt::DelimiterKind::Brace,
-                open: <crate::tt::SpanData as crate::tt::Span>::DUMMY,
-                close: <crate::tt::SpanData as crate::tt::Span>::DUMMY,
+                open: DUMMY,
+                close: DUMMY,
             },
             token_trees: fields.collect(),
         };
diff --git a/crates/hir-expand/src/span.rs b/crates/hir-expand/src/span.rs
index 1703923d11a..0a6c22fe42d 100644
--- a/crates/hir-expand/src/span.rs
+++ b/crates/hir-expand/src/span.rs
@@ -4,13 +4,12 @@ use base_db::{
     span::{ErasedFileAstId, SpanAnchor, SpanData, SyntaxContextId, ROOT_ERASED_FILE_AST_ID},
     FileId,
 };
-use mbe::TokenMap;
 use syntax::{ast::HasModuleItem, AstNode, TextRange, TextSize};
 use triomphe::Arc;
 
 use crate::db::ExpandDatabase;
 
-pub type ExpansionSpanMap = TokenMap<SpanData>;
+pub type ExpansionSpanMap = mbe::SpanMap<SpanData>;
 
 /// Spanmap for a macro file or a real file
 #[derive(Clone, Debug, PartialEq, Eq)]
diff --git a/crates/mbe/src/benchmark.rs b/crates/mbe/src/benchmark.rs
index 271efe1a92e..f503aecce2c 100644
--- a/crates/mbe/src/benchmark.rs
+++ b/crates/mbe/src/benchmark.rs
@@ -6,11 +6,10 @@ use syntax::{
     AstNode, SmolStr,
 };
 use test_utils::{bench, bench_fixture, skip_slow_tests};
-use tt::Span;
 
 use crate::{
     parser::{MetaVarKind, Op, RepeatKind, Separator},
-    syntax_node_to_token_tree, DeclarativeMacro, DummyTestSpanData, DummyTestSpanMap,
+    syntax_node_to_token_tree, DeclarativeMacro, DummyTestSpanData, DummyTestSpanMap, DUMMY,
 };
 
 #[test]
@@ -97,8 +96,8 @@ fn invocation_fixtures(
                 loop {
                     let mut subtree = tt::Subtree {
                         delimiter: tt::Delimiter {
-                            open: DummyTestSpanData::DUMMY,
-                            close: DummyTestSpanData::DUMMY,
+                            open: DUMMY,
+                            close: DUMMY,
                             kind: tt::DelimiterKind::Invisible,
                         },
                         token_trees: vec![],
@@ -211,34 +210,20 @@ fn invocation_fixtures(
             *seed
         }
         fn make_ident(ident: &str) -> tt::TokenTree<DummyTestSpanData> {
-            tt::Leaf::Ident(tt::Ident { span: DummyTestSpanData::DUMMY, text: SmolStr::new(ident) })
-                .into()
+            tt::Leaf::Ident(tt::Ident { span: DUMMY, text: SmolStr::new(ident) }).into()
         }
         fn make_punct(char: char) -> tt::TokenTree<DummyTestSpanData> {
-            tt::Leaf::Punct(tt::Punct {
-                span: DummyTestSpanData::DUMMY,
-                char,
-                spacing: tt::Spacing::Alone,
-            })
-            .into()
+            tt::Leaf::Punct(tt::Punct { span: DUMMY, char, spacing: tt::Spacing::Alone }).into()
         }
         fn make_literal(lit: &str) -> tt::TokenTree<DummyTestSpanData> {
-            tt::Leaf::Literal(tt::Literal {
-                span: DummyTestSpanData::DUMMY,
-                text: SmolStr::new(lit),
-            })
-            .into()
+            tt::Leaf::Literal(tt::Literal { span: DUMMY, text: SmolStr::new(lit) }).into()
         }
         fn make_subtree(
             kind: tt::DelimiterKind,
             token_trees: Option<Vec<tt::TokenTree<DummyTestSpanData>>>,
         ) -> tt::TokenTree<DummyTestSpanData> {
             tt::Subtree {
-                delimiter: tt::Delimiter {
-                    open: DummyTestSpanData::DUMMY,
-                    close: DummyTestSpanData::DUMMY,
-                    kind,
-                },
+                delimiter: tt::Delimiter { open: DUMMY, close: DUMMY, kind },
                 token_trees: token_trees.unwrap_or_default(),
             }
             .into()
diff --git a/crates/mbe/src/expander/transcriber.rs b/crates/mbe/src/expander/transcriber.rs
index e4a46c16597..7a3e8653c28 100644
--- a/crates/mbe/src/expander/transcriber.rs
+++ b/crates/mbe/src/expander/transcriber.rs
@@ -79,8 +79,8 @@ impl<S: Span> Bindings<S> {
                     }
                     MetaVarKind::Block => Fragment::Tokens(tt::TokenTree::Subtree(tt::Subtree {
                         delimiter: tt::Delimiter {
-                            open: S::DUMMY,
-                            close: S::DUMMY,
+                            open: span,
+                            close: span,
                             kind: tt::DelimiterKind::Brace,
                         },
                         token_trees: vec![],
@@ -225,6 +225,7 @@ fn expand_subtree<S: Span>(
                 arena.push(
                     tt::Leaf::Literal(tt::Literal {
                         text: index.to_string().into(),
+                        // FIXME
                         span: S::DUMMY,
                     })
                     .into(),
@@ -282,8 +283,12 @@ fn expand_subtree<S: Span>(
                     }
                 };
                 arena.push(
-                    tt::Leaf::Literal(tt::Literal { text: c.to_string().into(), span: S::DUMMY })
-                        .into(),
+                    tt::Leaf::Literal(tt::Literal {
+                        text: c.to_string().into(),
+                        // FIXME
+                        span: S::DUMMY,
+                    })
+                    .into(),
                 );
             }
         }
@@ -337,7 +342,9 @@ fn expand_var<S: Span>(
         }
         Err(e) => ExpandResult {
             value: Fragment::Tokens(tt::TokenTree::Subtree(tt::Subtree::empty(tt::DelimSpan {
+                // FIXME
                 open: S::DUMMY,
+                // FIXME
                 close: S::DUMMY,
             }))),
             err: Some(e),
@@ -479,6 +486,7 @@ fn fix_up_and_push_path_tt<S: Span>(buf: &mut Vec<tt::TokenTree<S>>, subtree: tt
                     tt::Leaf::Punct(tt::Punct {
                         char: ':',
                         spacing: tt::Spacing::Joint,
+                        // FIXME
                         span: S::DUMMY,
                     })
                     .into(),
@@ -487,6 +495,7 @@ fn fix_up_and_push_path_tt<S: Span>(buf: &mut Vec<tt::TokenTree<S>>, subtree: tt
                     tt::Leaf::Punct(tt::Punct {
                         char: ':',
                         spacing: tt::Spacing::Alone,
+                        // FIXME
                         span: S::DUMMY,
                     })
                     .into(),
diff --git a/crates/mbe/src/lib.rs b/crates/mbe/src/lib.rs
index 10eb59bb835..2b52e04b251 100644
--- a/crates/mbe/src/lib.rs
+++ b/crates/mbe/src/lib.rs
@@ -38,7 +38,7 @@ pub use crate::{
         syntax_node_to_token_tree, syntax_node_to_token_tree_modified, token_tree_to_syntax_node,
         SpanMapper,
     },
-    token_map::TokenMap,
+    token_map::SpanMap,
 };
 
 pub use crate::syntax_bridge::dummy_test_span_utils::*;
diff --git a/crates/mbe/src/syntax_bridge.rs b/crates/mbe/src/syntax_bridge.rs
index 5722a5bd8eb..1c46471a383 100644
--- a/crates/mbe/src/syntax_bridge.rs
+++ b/crates/mbe/src/syntax_bridge.rs
@@ -13,7 +13,7 @@ use tt::{
     Span, SpanData, SyntaxContext,
 };
 
-use crate::{to_parser_input::to_parser_input, tt_iter::TtIter, TokenMap};
+use crate::{to_parser_input::to_parser_input, tt_iter::TtIter, SpanMap};
 
 #[cfg(test)]
 mod tests;
@@ -22,7 +22,7 @@ pub trait SpanMapper<S: Span> {
     fn span_for(&self, range: TextRange) -> S;
 }
 
-impl<S: Span> SpanMapper<S> for TokenMap<S> {
+impl<S: Span> SpanMapper<S> for SpanMap<S> {
     fn span_for(&self, range: TextRange) -> S {
         self.span_at(range.start())
     }
@@ -34,10 +34,12 @@ impl<S: Span, SM: SpanMapper<S>> SpanMapper<S> for &SM {
     }
 }
 
+/// Dummy things for testing where spans don't matter.
 pub(crate) mod dummy_test_span_utils {
     use super::*;
 
     pub type DummyTestSpanData = tt::SpanData<DummyTestSpanAnchor, DummyTestSyntaxContext>;
+    pub const DUMMY: DummyTestSpanData = DummyTestSpanData::DUMMY;
 
     #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
     pub struct DummyTestSpanAnchor;
@@ -62,9 +64,8 @@ pub(crate) mod dummy_test_span_utils {
     }
 }
 
-/// Convert the syntax node to a `TokenTree` (what macro
-/// will consume).
-/// FIXME: Flesh out the doc comment more thoroughly
+/// Converts a syntax tree to a [`tt::Subtree`] using the provided span map to populate the
+/// subtree's spans.
 pub fn syntax_node_to_token_tree<Anchor, Ctx, SpanMap>(
     node: &SyntaxNode,
     map: SpanMap,
@@ -79,6 +80,9 @@ where
     convert_tokens(&mut c)
 }
 
+/// Converts a syntax tree to a [`tt::Subtree`] using the provided span map to populate the
+/// subtree's spans. Additionally using the append and remove parameters, the additional tokens can
+/// be injected or hidden from the output.
 pub fn syntax_node_to_token_tree_modified<Anchor, Ctx, SpanMap>(
     node: &SyntaxNode,
     map: SpanMap,
@@ -107,10 +111,12 @@ where
 // * AssocItems(SmallVec<[ast::AssocItem; 1]>)
 // * ForeignItems(SmallVec<[ast::ForeignItem; 1]>
 
+/// Converts a [`tt::Subtree`] back to a [`SyntaxNode`].
+/// The produced `SpanMap` contains a mapping from the syntax nodes offsets to the subtree's spans.
 pub fn token_tree_to_syntax_node<Anchor, Ctx>(
     tt: &tt::Subtree<SpanData<Anchor, Ctx>>,
     entry_point: parser::TopEntryPoint,
-) -> (Parse<SyntaxNode>, TokenMap<SpanData<Anchor, Ctx>>)
+) -> (Parse<SyntaxNode>, SpanMap<SpanData<Anchor, Ctx>>)
 where
     SpanData<Anchor, Ctx>: Span,
     Anchor: Copy,
@@ -142,7 +148,8 @@ where
     tree_sink.finish()
 }
 
-/// Convert a string to a `TokenTree`
+/// Convert a string to a `TokenTree`. The spans of the subtree will be anchored to the provided
+/// anchor with the given context.
 pub fn parse_to_token_tree<Anchor, Ctx>(
     anchor: Anchor,
     ctx: Ctx,
@@ -161,7 +168,7 @@ where
     Some(convert_tokens(&mut conv))
 }
 
-/// Convert a string to a `TokenTree`
+/// Convert a string to a `TokenTree`. The passed span will be used for all spans of the produced subtree.
 pub fn parse_to_token_tree_static_span<S>(span: S, text: &str) -> Option<tt::Subtree<S>>
 where
     S: Span,
@@ -798,7 +805,7 @@ where
     cursor: Cursor<'a, SpanData<Anchor, Ctx>>,
     text_pos: TextSize,
     inner: SyntaxTreeBuilder,
-    token_map: TokenMap<SpanData<Anchor, Ctx>>,
+    token_map: SpanMap<SpanData<Anchor, Ctx>>,
 }
 
 impl<'a, Anchor, Ctx> TtTreeSink<'a, Anchor, Ctx>
@@ -811,11 +818,11 @@ where
             cursor,
             text_pos: 0.into(),
             inner: SyntaxTreeBuilder::default(),
-            token_map: TokenMap::empty(),
+            token_map: SpanMap::empty(),
         }
     }
 
-    fn finish(mut self) -> (Parse<SyntaxNode>, TokenMap<SpanData<Anchor, Ctx>>) {
+    fn finish(mut self) -> (Parse<SyntaxNode>, SpanMap<SpanData<Anchor, Ctx>>) {
         self.token_map.finish();
         (self.inner.finish(), self.token_map)
     }
diff --git a/crates/mbe/src/token_map.rs b/crates/mbe/src/token_map.rs
index 5871c0f1251..b51d7575a11 100644
--- a/crates/mbe/src/token_map.rs
+++ b/crates/mbe/src/token_map.rs
@@ -8,30 +8,33 @@ use tt::Span;
 
 /// Maps absolute text ranges for the corresponding file to the relevant span data.
 #[derive(Debug, PartialEq, Eq, Clone, Hash)]
-// FIXME: Rename to SpanMap
-pub struct TokenMap<S: Span> {
-    // FIXME: This needs to be sorted by (FileId, AstId)
-    // Then we can do a binary search on the file id,
-    // then a bin search on the ast id?
+pub struct SpanMap<S: Span> {
     spans: Vec<(TextSize, S)>,
 }
 
-impl<S: Span> TokenMap<S> {
+impl<S: Span> SpanMap<S> {
+    /// Creates a new empty [`SpanMap`].
     pub fn empty() -> Self {
         Self { spans: Vec::new() }
     }
 
+    /// Finalizes the [`SpanMap`], shrinking its backing storage and validating that the offsets are
+    /// in order.
     pub fn finish(&mut self) {
         assert!(self.spans.iter().tuple_windows().all(|(a, b)| a.0 < b.0));
         self.spans.shrink_to_fit();
     }
 
+    /// Pushes a new span onto the [`SpanMap`].
     pub fn push(&mut self, offset: TextSize, span: S) {
+        debug_assert!(self.spans.last().map_or(true, |&(last_offset, _)| last_offset < offset));
         self.spans.push((offset, span));
     }
 
+    /// Returns all [`TextRange`]s that correspond to the given span.
+    ///
+    /// Note this does a linear search through the entire backing vector.
     pub fn ranges_with_span(&self, span: S) -> impl Iterator<Item = TextRange> + '_ {
-        // FIXME: linear search
         self.spans.iter().enumerate().filter_map(move |(idx, &(end, s))| {
             if s != span {
                 return None;
@@ -41,14 +44,15 @@ impl<S: Span> TokenMap<S> {
         })
     }
 
-    // FIXME: We need APIs for fetching the span of a token as well as for a whole node. The node
-    // one *is* fallible though.
+    /// Returns the span at the given position.
     pub fn span_at(&self, offset: TextSize) -> S {
         let entry = self.spans.partition_point(|&(it, _)| it <= offset);
         self.spans[entry].1
     }
 
-    pub fn spans_for_node_range(&self, range: TextRange) -> impl Iterator<Item = S> + '_ {
+    /// Returns the spans associated with the given range.
+    /// In other words, this will return all spans that correspond to all offsets within the given range.
+    pub fn spans_for_range(&self, range: TextRange) -> impl Iterator<Item = S> + '_ {
         let (start, end) = (range.start(), range.end());
         let start_entry = self.spans.partition_point(|&(it, _)| it <= start);
         let end_entry = self.spans[start_entry..].partition_point(|&(it, _)| it <= end); // FIXME: this might be wrong?
diff --git a/crates/tt/src/lib.rs b/crates/tt/src/lib.rs
index b1f21851621..10e05862189 100644
--- a/crates/tt/src/lib.rs
+++ b/crates/tt/src/lib.rs
@@ -23,6 +23,7 @@ pub struct SpanData<Anchor, Ctx> {
 }
 
 impl<Anchor: SpanAnchor, Ctx: SyntaxContext> Span for SpanData<Anchor, Ctx> {
+    #[allow(deprecated)]
     const DUMMY: Self = SpanData {
         range: TextRange::empty(TextSize::new(0)),
         anchor: Anchor::DUMMY,
@@ -30,18 +31,24 @@ impl<Anchor: SpanAnchor, Ctx: SyntaxContext> Span for SpanData<Anchor, Ctx> {
     };
 }
 
-pub trait SpanAnchor:
-    std::fmt::Debug + Copy + Sized + Eq + Copy + fmt::Debug + std::hash::Hash
-{
+pub trait Span: std::fmt::Debug + Copy + Sized + Eq {
+    // FIXME: Should not exist. Dummy spans will always be wrong if they leak somewhere. Instead,
+    // the call site or def site spans should be used in relevant places, its just that we don't
+    // expose those everywhere in the yet.
     const DUMMY: Self;
 }
 
-// FIXME: Get rid of this trait?
-pub trait Span: std::fmt::Debug + Copy + Sized + Eq {
+// FIXME: Should not exist
+pub trait SpanAnchor:
+    std::fmt::Debug + Copy + Sized + Eq + Copy + fmt::Debug + std::hash::Hash
+{
+    #[deprecated(note = "this should not exist")]
     const DUMMY: Self;
 }
 
+// FIXME: Should not exist
 pub trait SyntaxContext: std::fmt::Debug + Copy + Sized + Eq {
+    #[deprecated(note = "this should not exist")]
     const DUMMY: Self;
 }
 
@@ -128,6 +135,7 @@ pub struct DelimSpan<S> {
 }
 
 impl<S: Span> DelimSpan<S> {
+    // FIXME should not exist
     pub const DUMMY: Self = Self { open: S::DUMMY, close: S::DUMMY };
 }
 
@@ -139,9 +147,11 @@ pub struct Delimiter<S> {
 }
 
 impl<S: Span> Delimiter<S> {
+    // FIXME should not exist
     pub const DUMMY_INVISIBLE: Self =
         Self { open: S::DUMMY, close: S::DUMMY, kind: DelimiterKind::Invisible };
 
+    // FIXME should not exist
     pub const fn dummy_invisible() -> Self {
         Self::DUMMY_INVISIBLE
     }
diff --git a/crates/vfs/src/lib.rs b/crates/vfs/src/lib.rs
index c6866bbfb9b..128559727bd 100644
--- a/crates/vfs/src/lib.rs
+++ b/crates/vfs/src/lib.rs
@@ -63,7 +63,7 @@ pub use paths::{AbsPath, AbsPathBuf};
 pub struct FileId(pub u32);
 
 impl FileId {
-    /// Think twice about using this. If this ends up in a wrong place it will cause panics!
+    /// Think twice about using this outside of tests. If this ends up in a wrong place it will cause panics!
     pub const BOGUS: FileId = FileId(u32::MAX);
 }