Lay the foundation for diagnostics in ty lowering, and implement a first diagnostic

The diagnostic implemented is a simple one (E0109). It serves as a test for the new foundation.

This commit only implements diagnostics for type in bodies and body-carrying signatures; the next commit will include diagnostics in the rest of the things.

Also fix one weird bug that was detected when implementing this that caused `Fn::(A, B) -> C` (which is a valid, if bizarre, alternative syntax to `Fn(A, B) -> C` to lower incorrectly.

And also fix a maybe-bug where parentheses were sneaked into a code string needlessly; this was not detected until now because the parentheses were removed (by the make-AST family API), but with a change in this commit they are now inserted. So fix that too.

Author: ChayimFriedman2

Cargo.lock

@@ -141,7 +141,7 @@ pub struct BodySourceMap {
     field_map_back: FxHashMap<ExprId, FieldSource>,
     pat_field_map_back: FxHashMap<PatId, PatFieldSource>,
 
-    types: TypesSourceMap,
+    pub types: TypesSourceMap,
 
     // FIXME: Make this a sane struct.
     template_map: Option<

crates/hir-def/src/body.rs

@@ -219,6 +219,8 @@ pub struct TypesSourceMap {
 }
 
 impl TypesSourceMap {
+    pub const EMPTY: Self = Self { types_map_back: ArenaMap::new() };
+
     pub fn type_syntax(&self, id: TypeRefId) -> Result<TypeSource, SyntheticSyntax> {
         self.types_map_back.get(id).cloned().ok_or(SyntheticSyntax)
     }

crates/hir-def/src/hir/type_ref.rs

@@ -1,5 +1,7 @@
 //! A desugared representation of paths like `crate::foo` or `<Type as Trait>::bar`.
 mod lower;
+#[cfg(test)]
+mod tests;
 
 use std::{
     fmt::{self, Display},
@@ -19,6 +21,8 @@ use syntax::ast;
 
 pub use hir_expand::mod_path::{path, ModPath, PathKind};
 
+pub use lower::hir_segment_to_ast_segment;
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum ImportAlias {
     /// Unnamed alias, as in `use Foo as _;`
@@ -230,7 +234,7 @@ impl Path {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct PathSegment<'a> {
     pub name: &'a Name,
     pub args_and_bindings: Option<&'a GenericArgs>,
@@ -274,6 +278,12 @@ impl<'a> PathSegments<'a> {
             generic_args: self.generic_args.map(|it| it.get(..len).unwrap_or(it)),
         }
     }
+    pub fn strip_last(&self) -> PathSegments<'a> {
+        PathSegments {
+            segments: self.segments.split_last().map_or(&[], |it| it.1),
+            generic_args: self.generic_args.map(|it| it.split_last().map_or(&[][..], |it| it.1)),
+        }
+    }
     pub fn iter(&self) -> impl Iterator<Item = PathSegment<'a>> {
         self.segments
             .iter()

crates/hir-def/src/path.rs

@@ -17,23 +17,58 @@ use crate::{
     type_ref::{LifetimeRef, TypeBound, TypeRef},
 };
 
+#[cfg(test)]
+thread_local! {
+    /// This is used to test `hir_segment_to_ast_segment()`. It's a hack, but it makes testing much easier.
+    pub(super) static SEGMENT_LOWERING_MAP: std::cell::RefCell<rustc_hash::FxHashMap<ast::PathSegment, usize>> = std::cell::RefCell::default();
+}
+
 /// Converts an `ast::Path` to `Path`. Works with use trees.
 /// It correctly handles `$crate` based path from macro call.
+// If you modify the logic of the lowering, make sure to check if `hir_segment_to_ast_segment()`
+// also needs an update.
 pub(super) fn lower_path(ctx: &mut LowerCtx<'_>, mut path: ast::Path) -> Option<Path> {
     let mut kind = PathKind::Plain;
     let mut type_anchor = None;
     let mut segments = Vec::new();
     let mut generic_args = Vec::new();
+    #[cfg(test)]
+    let mut ast_segments = Vec::new();
+    #[cfg(test)]
+    let mut ast_segments_offset = 0;
+    #[allow(unused_mut)]
+    let mut push_segment = |_segment: &ast::PathSegment, segments: &mut Vec<Name>, name| {
+        #[cfg(test)]
+        ast_segments.push(_segment.clone());
+        segments.push(name);
+    };
     loop {
         let segment = path.segment()?;
 
-        if segment.coloncolon_token().is_some() {
-            kind = PathKind::Abs;
+        if let Some(coloncolon) = segment.coloncolon_token() {
+            if coloncolon
+                .prev_token()
+                .is_none_or(|it| !path.syntax().text_range().contains_range(it.text_range()))
+            {
+                // A `::` inside segment (not between segments) can occur in two cases:
+                //  1. At the prefix of the path, i.e. `::foo::bar`. In this case the path is absolute.
+                //  2. In a truly bizarre syntax `Fn::(A, B) -> C`, which is another way to spell `Fn(A, B) -> C`.
+                //     In this case the path is *not* absolute.
+                // We differentiate between the two by checking if the previous token is also part of the path.
+                // If it is, it must be the second thing. If not, it's the first.
+                // (With normal generic arguments, that also can be prefixed with `::`, we don't have the same problem
+                // because the `::` is inside the `GenericArgList`).
+                kind = PathKind::Abs;
+            }
         }
 
         match segment.kind()? {
             ast::PathSegmentKind::Name(name_ref) => {
                 if name_ref.text() == "$crate" {
+                    if path.qualifier().is_some() {
+                        // FIXME: Report an error.
+                        return None;
+                    }
                     break kind = resolve_crate_root(
                         ctx.db.upcast(),
                         ctx.span_map().span_for_range(name_ref.syntax().text_range()).ctx,
@@ -56,10 +91,10 @@ pub(super) fn lower_path(ctx: &mut LowerCtx<'_>, mut path: ast::Path) -> Option<
                     generic_args.resize(segments.len(), None);
                     generic_args.push(args);
                 }
-                segments.push(name);
+                push_segment(&segment, &mut segments, name);
             }
             ast::PathSegmentKind::SelfTypeKw => {
-                segments.push(Name::new_symbol_root(sym::Self_.clone()));
+                push_segment(&segment, &mut segments, Name::new_symbol_root(sym::Self_.clone()));
             }
             ast::PathSegmentKind::Type { type_ref, trait_ref } => {
                 assert!(path.qualifier().is_none()); // this can only occur at the first segment
@@ -81,6 +116,10 @@ pub(super) fn lower_path(ctx: &mut LowerCtx<'_>, mut path: ast::Path) -> Option<
                         kind = mod_path.kind;
 
                         segments.extend(mod_path.segments().iter().cloned().rev());
+                        #[cfg(test)]
+                        {
+                            ast_segments_offset = mod_path.segments().len();
+                        }
                         if let Some(path_generic_args) = path_generic_args {
                             generic_args.resize(segments.len() - num_segments, None);
                             generic_args.extend(Vec::from(path_generic_args).into_iter().rev());
@@ -112,10 +151,18 @@ pub(super) fn lower_path(ctx: &mut LowerCtx<'_>, mut path: ast::Path) -> Option<
                 }
             }
             ast::PathSegmentKind::CrateKw => {
+                if path.qualifier().is_some() {
+                    // FIXME: Report an error.
+                    return None;
+                }
                 kind = PathKind::Crate;
                 break;
             }
             ast::PathSegmentKind::SelfKw => {
+                if path.qualifier().is_some() {
+                    // FIXME: Report an error.
+                    return None;
+                }
                 // don't break out if `self` is the last segment of a path, this mean we got a
                 // use tree like `foo::{self}` which we want to resolve as `foo`
                 if !segments.is_empty() {
@@ -162,6 +209,13 @@ pub(super) fn lower_path(ctx: &mut LowerCtx<'_>, mut path: ast::Path) -> Option<
         }
     }
 
+    #[cfg(test)]
+    {
+        ast_segments.reverse();
+        SEGMENT_LOWERING_MAP
+            .with_borrow_mut(|map| map.extend(ast_segments.into_iter().zip(ast_segments_offset..)));
+    }
+
     let mod_path = Interned::new(ModPath::from_segments(kind, segments));
     if type_anchor.is_none() && generic_args.is_empty() {
         return Some(Path::BarePath(mod_path));
@@ -181,6 +235,41 @@ pub(super) fn lower_path(ctx: &mut LowerCtx<'_>, mut path: ast::Path) -> Option<
     }
 }
 
+/// This function finds the AST segment that corresponds to the HIR segment
+/// with index `segment_idx` on the path that is lowered from `path`.
+pub fn hir_segment_to_ast_segment(path: &ast::Path, segment_idx: u32) -> Option<ast::PathSegment> {
+    // Too tightly coupled to `lower_path()`, but unfortunately we cannot decouple them,
+    // as keeping source maps for all paths segments will have a severe impact on memory usage.
+
+    let mut segments = path.segments();
+    if let Some(ast::PathSegmentKind::Type { trait_ref: Some(trait_ref), .. }) =
+        segments.clone().next().and_then(|it| it.kind())
+    {
+        segments.next();
+        return find_segment(trait_ref.path()?.segments().chain(segments), segment_idx);
+    }
+    return find_segment(segments, segment_idx);
+
+    fn find_segment(
+        segments: impl Iterator<Item = ast::PathSegment>,
+        segment_idx: u32,
+    ) -> Option<ast::PathSegment> {
+        segments
+            .filter(|segment| match segment.kind() {
+                Some(
+                    ast::PathSegmentKind::CrateKw
+                    | ast::PathSegmentKind::SelfKw
+                    | ast::PathSegmentKind::SuperKw
+                    | ast::PathSegmentKind::Type { .. },
+                )
+                | None => false,
+                Some(ast::PathSegmentKind::Name(name)) => name.text() != "$crate",
+                Some(ast::PathSegmentKind::SelfTypeKw) => true,
+            })
+            .nth(segment_idx as usize)
+    }
+}
+
 pub(super) fn lower_generic_args(
     lower_ctx: &mut LowerCtx<'_>,
     node: ast::GenericArgList,

crates/hir-def/src/path/lower.rs

@@ -0,0 +1,126 @@
+use expect_test::{expect, Expect};
+use span::Edition;
+use syntax::ast::{self, make};
+use test_fixture::WithFixture;
+
+use crate::{
+    lower::LowerCtx,
+    path::{
+        lower::{hir_segment_to_ast_segment, SEGMENT_LOWERING_MAP},
+        Path,
+    },
+    pretty,
+    test_db::TestDB,
+    type_ref::{TypesMap, TypesSourceMap},
+};
+
+fn lower_path(path: ast::Path) -> (TestDB, TypesMap, Option<Path>) {
+    let (db, file_id) = TestDB::with_single_file("");
+    let mut types_map = TypesMap::default();
+    let mut types_source_map = TypesSourceMap::default();
+    let mut ctx = LowerCtx::new(&db, file_id.into(), &mut types_map, &mut types_source_map);
+    let lowered_path = ctx.lower_path(path);
+    (db, types_map, lowered_path)
+}
+
+#[track_caller]
+fn check_hir_to_ast(path: &str, ignore_segments: &[&str]) {
+    let path = make::path_from_text(path);
+    SEGMENT_LOWERING_MAP.with_borrow_mut(|map| map.clear());
+    let _ = lower_path(path.clone()).2.expect("failed to lower path");
+    SEGMENT_LOWERING_MAP.with_borrow(|map| {
+        for (segment, segment_idx) in map {
+            if ignore_segments.contains(&&*segment.to_string()) {
+                continue;
+            }
+
+            let restored_segment = hir_segment_to_ast_segment(&path, *segment_idx as u32)
+                .unwrap_or_else(|| {
+                    panic!(
+                        "failed to map back segment `{segment}` \
+                        numbered {segment_idx} in HIR from path `{path}`"
+                    )
+                });
+            assert_eq!(
+                segment, &restored_segment,
+                "mapping back `{segment}` numbered {segment_idx} in HIR \
+                from path `{path}` produced incorrect segment `{restored_segment}`"
+            );
+        }
+    });
+}
+
+#[test]
+fn hir_to_ast_trait_ref() {
+    check_hir_to_ast("<A as B::C::D>::E::F", &["A"]);
+}
+
+#[test]
+fn hir_to_ast_plain_path() {
+    check_hir_to_ast("A::B::C::D::E::F", &[]);
+}
+
+#[test]
+fn hir_to_ast_crate_path() {
+    check_hir_to_ast("crate::A::B::C", &[]);
+    check_hir_to_ast("crate::super::super::A::B::C", &[]);
+}
+
+#[test]
+fn hir_to_ast_self_path() {
+    check_hir_to_ast("self::A::B::C", &[]);
+    check_hir_to_ast("self::super::super::A::B::C", &[]);
+}
+
+#[test]
+fn hir_to_ast_super_path() {
+    check_hir_to_ast("super::A::B::C", &[]);
+    check_hir_to_ast("super::super::super::A::B::C", &[]);
+}
+
+#[test]
+fn hir_to_ast_type_anchor_path() {
+    check_hir_to_ast("<A::B>::C::D", &["A", "B"]);
+}
+
+#[test]
+fn hir_to_ast_path_super_in_middle() {
+    check_hir_to_ast("A::super::B::super::super::C::D", &[]);
+}
+
+#[track_caller]
+fn check_fail_lowering(path: &str) {
+    let (_, _, lowered_path) = lower_path(make::path_from_text(path));
+    assert!(lowered_path.is_none(), "path `{path}` should fail lowering");
+}
+
+#[test]
+fn keywords_in_middle_fail_lowering1() {
+    check_fail_lowering("self::A::self::B::super::C::crate::D");
+}
+
+#[test]
+fn keywords_in_middle_fail_lowering2() {
+    check_fail_lowering("A::super::self::C::D");
+}
+
+#[test]
+fn keywords_in_middle_fail_lowering3() {
+    check_fail_lowering("A::crate::B::C::D");
+}
+
+#[track_caller]
+fn check_path_lowering(path: &str, expected: Expect) {
+    let (db, types_map, lowered_path) = lower_path(make::path_from_text(path));
+    let lowered_path = lowered_path.expect("failed to lower path");
+    let mut buf = String::new();
+    pretty::print_path(&db, &lowered_path, &types_map, &mut buf, Edition::CURRENT)
+        .expect("failed to pretty-print path");
+    expected.assert_eq(&buf);
+}
+
+#[test]
+fn fn_like_path_with_coloncolon() {
+    check_path_lowering("Fn::(A, B) -> C", expect![[r#"Fn::<(A, B), Output = C>"#]]);
+    check_path_lowering("Fn::(A, B)", expect![[r#"Fn::<(A, B), Output = ()>"#]]);
+}

crates/hir-def/src/path/tests.rs

@@ -180,6 +180,13 @@ impl<FileId: FileIdToSyntax, T> InFileWrapper<FileId, T> {
     }
 }
 
+#[allow(private_bounds)]
+impl<FileId: FileIdToSyntax, N: AstNode> InFileWrapper<FileId, AstPtr<N>> {
+    pub fn to_node(&self, db: &dyn ExpandDatabase) -> N {
+        self.value.to_node(&self.file_syntax(db))
+    }
+}
+
 impl<FileId: Copy, N: AstNode> InFileWrapper<FileId, N> {
     pub fn syntax(&self) -> InFileWrapper<FileId, &SyntaxNode> {
         self.with_value(self.value.syntax())