Unify handling of path diagnostics in hir-ty

Because it was a mess.

Previously, pretty much you had to handle all path diagnostics manually: remember to check for them and handle them. Now, we wrap the resolver in `TyLoweringContext` and ensure proper error reporting.

This means that you don't have to worry about them: most of the things are handled automatically, and things that cannot will create a compile-time error (forcing you top `drop(ty_lowering_context);`) if forgotten, instead of silently dropping the diagnostics.

The real place for error reporting is in the hir-def resolver, because there are other things resolving, both in hir-ty and in hir-def, and they all need to ensure proper diagnostics. But this is a good start, and future compatible.

This commit also ensures proper path diagnostics for value/pattern paths, which is why it's marked "feat".

Author: ChayimFriedman2

src/tools/rust-analyzer/crates/hir-def/src/nameres.rs

@@ -85,6 +85,8 @@ use crate::{
     FxIndexMap, LocalModuleId, Lookup, MacroExpander, MacroId, ModuleId, ProcMacroId, UseId,
 };
 
+pub use self::path_resolution::ResolvePathResultPrefixInfo;
+
 const PREDEFINED_TOOLS: &[SmolStr] = &[
     SmolStr::new_static("clippy"),
     SmolStr::new_static("rustfmt"),
@@ -615,13 +617,15 @@ impl DefMap {
         (res.resolved_def, res.segment_index)
     }
 
+    /// The first `Option<usize>` points at the `Enum` segment in case of `Enum::Variant`, the second
+    /// points at the unresolved segments.
     pub(crate) fn resolve_path_locally(
         &self,
         db: &dyn DefDatabase,
         original_module: LocalModuleId,
         path: &ModPath,
         shadow: BuiltinShadowMode,
-    ) -> (PerNs, Option<usize>) {
+    ) -> (PerNs, Option<usize>, ResolvePathResultPrefixInfo) {
         let res = self.resolve_path_fp_with_macro_single(
             db,
             ResolveMode::Other,
@@ -630,7 +634,7 @@ impl DefMap {
             shadow,
             None, // Currently this function isn't used for macro resolution.
         );
-        (res.resolved_def, res.segment_index)
+        (res.resolved_def, res.segment_index, res.prefix_info)
     }
 
     /// Ascends the `DefMap` hierarchy and calls `f` with every `DefMap` and containing module.

src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs

@@ -38,7 +38,7 @@ use crate::{
         attr_resolution::{attr_macro_as_call_id, derive_macro_as_call_id},
         diagnostics::DefDiagnostic,
         mod_resolution::ModDir,
-        path_resolution::ReachedFixedPoint,
+        path_resolution::{ReachedFixedPoint, ResolvePathResultPrefixInfo},
         proc_macro::{parse_macro_name_and_helper_attrs, ProcMacroDef, ProcMacroKind},
         sub_namespace_match, BuiltinShadowMode, DefMap, MacroSubNs, ModuleData, ModuleOrigin,
         ResolveMode,
@@ -797,7 +797,7 @@ impl DefCollector<'_> {
             return PartialResolvedImport::Unresolved;
         }
 
-        if res.from_differing_crate {
+        if let ResolvePathResultPrefixInfo::DifferingCrate = res.prefix_info {
             return PartialResolvedImport::Resolved(
                 def.filter_visibility(|v| matches!(v, Visibility::Public)),
             );

src/tools/rust-analyzer/crates/hir-def/src/nameres/path_resolution.rs

@@ -43,21 +43,34 @@ pub(super) struct ResolvePathResult {
     pub(super) resolved_def: PerNs,
     pub(super) segment_index: Option<usize>,
     pub(super) reached_fixedpoint: ReachedFixedPoint,
-    pub(super) from_differing_crate: bool,
+    pub(super) prefix_info: ResolvePathResultPrefixInfo,
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum ResolvePathResultPrefixInfo {
+    None,
+    DifferingCrate,
+    /// Path of the form `Enum::Variant` (and not `Variant` alone).
+    Enum,
 }
 
 impl ResolvePathResult {
     fn empty(reached_fixedpoint: ReachedFixedPoint) -> ResolvePathResult {
-        ResolvePathResult::new(PerNs::none(), reached_fixedpoint, None, false)
+        ResolvePathResult::new(
+            PerNs::none(),
+            reached_fixedpoint,
+            None,
+            ResolvePathResultPrefixInfo::None,
+        )
     }
 
     fn new(
         resolved_def: PerNs,
         reached_fixedpoint: ReachedFixedPoint,
         segment_index: Option<usize>,
-        from_differing_crate: bool,
+        prefix_info: ResolvePathResultPrefixInfo,
     ) -> ResolvePathResult {
-        ResolvePathResult { resolved_def, segment_index, reached_fixedpoint, from_differing_crate }
+        ResolvePathResult { resolved_def, segment_index, reached_fixedpoint, prefix_info }
     }
 }
 
@@ -157,7 +170,17 @@ impl DefMap {
             if result.reached_fixedpoint == ReachedFixedPoint::No {
                 result.reached_fixedpoint = new.reached_fixedpoint;
             }
-            result.from_differing_crate |= new.from_differing_crate;
+            result.prefix_info = match (result.prefix_info, new.prefix_info) {
+                (ResolvePathResultPrefixInfo::None, it) => it,
+                (ResolvePathResultPrefixInfo::DifferingCrate, _) => {
+                    ResolvePathResultPrefixInfo::DifferingCrate
+                }
+                (
+                    ResolvePathResultPrefixInfo::Enum,
+                    ResolvePathResultPrefixInfo::DifferingCrate,
+                ) => ResolvePathResultPrefixInfo::DifferingCrate,
+                (ResolvePathResultPrefixInfo::Enum, _) => ResolvePathResultPrefixInfo::Enum,
+            };
             result.segment_index = match (result.segment_index, new.segment_index) {
                 (Some(idx), None) => Some(idx),
                 (Some(old), Some(new)) => Some(old.max(new)),
@@ -403,14 +426,14 @@ impl DefMap {
 
     fn resolve_remaining_segments<'a>(
         &self,
-        segments: impl Iterator<Item = (usize, &'a Name)>,
+        mut segments: impl Iterator<Item = (usize, &'a Name)>,
         mut curr_per_ns: PerNs,
         path: &ModPath,
         db: &dyn DefDatabase,
         shadow: BuiltinShadowMode,
         original_module: LocalModuleId,
     ) -> ResolvePathResult {
-        for (i, segment) in segments {
+        while let Some((i, segment)) = segments.next() {
             let curr = match curr_per_ns.take_types_full() {
                 Some(r) => r,
                 None => {
@@ -443,7 +466,7 @@ impl DefMap {
                             def,
                             ReachedFixedPoint::Yes,
                             s.map(|s| s + i),
-                            true,
+                            ResolvePathResultPrefixInfo::DifferingCrate,
                         );
                     }
 
@@ -488,17 +511,28 @@ impl DefMap {
                             ),
                         })
                     });
-                    match res {
-                        Some(res) => res,
-                        None => {
-                            return ResolvePathResult::new(
-                                PerNs::types(e.into(), curr.vis, curr.import),
-                                ReachedFixedPoint::Yes,
-                                Some(i),
-                                false,
-                            )
+                    // FIXME: Need to filter visibility here and below? Not sure.
+                    return match res {
+                        Some(res) => {
+                            if segments.next().is_some() {
+                                // Enum variants are in value namespace, segments left => no resolution.
+                                ResolvePathResult::empty(ReachedFixedPoint::No)
+                            } else {
+                                ResolvePathResult::new(
+                                    res,
+                                    ReachedFixedPoint::Yes,
+                                    None,
+                                    ResolvePathResultPrefixInfo::Enum,
+                                )
+                            }
                         }
-                    }
+                        None => ResolvePathResult::new(
+                            PerNs::types(e.into(), curr.vis, curr.import),
+                            ReachedFixedPoint::Yes,
+                            Some(i),
+                            ResolvePathResultPrefixInfo::None,
+                        ),
+                    };
                 }
                 s => {
                     // could be an inherent method call in UFCS form
@@ -513,7 +547,7 @@ impl DefMap {
                         PerNs::types(s, curr.vis, curr.import),
                         ReachedFixedPoint::Yes,
                         Some(i),
-                        false,
+                        ResolvePathResultPrefixInfo::None,
                     );
                 }
             };
@@ -522,7 +556,12 @@ impl DefMap {
                 .filter_visibility(|vis| vis.is_visible_from_def_map(db, self, original_module));
         }
 
-        ResolvePathResult::new(curr_per_ns, ReachedFixedPoint::Yes, None, false)
+        ResolvePathResult::new(
+            curr_per_ns,
+            ReachedFixedPoint::Yes,
+            None,
+            ResolvePathResultPrefixInfo::None,
+        )
     }
 
     fn resolve_name_in_module(

src/tools/rust-analyzer/crates/hir-def/src/path.rs

@@ -240,6 +240,7 @@ pub struct PathSegment<'a> {
     pub args_and_bindings: Option<&'a GenericArgs>,
 }
 
+#[derive(Debug, Clone, Copy)]
 pub struct PathSegments<'a> {
     segments: &'a [Name],
     generic_args: Option<&'a [Option<GenericArgs>]>,
@@ -259,31 +260,45 @@ impl<'a> PathSegments<'a> {
     pub fn last(&self) -> Option<PathSegment<'a>> {
         self.get(self.len().checked_sub(1)?)
     }
+
     pub fn get(&self, idx: usize) -> Option<PathSegment<'a>> {
         let res = PathSegment {
             name: self.segments.get(idx)?,
             args_and_bindings: self.generic_args.and_then(|it| it.get(idx)?.as_ref()),
         };
         Some(res)
     }
+
     pub fn skip(&self, len: usize) -> PathSegments<'a> {
         PathSegments {
             segments: self.segments.get(len..).unwrap_or(&[]),
             generic_args: self.generic_args.and_then(|it| it.get(len..)),
         }
     }
+
     pub fn take(&self, len: usize) -> PathSegments<'a> {
         PathSegments {
             segments: self.segments.get(..len).unwrap_or(self.segments),
             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 strip_last_two(&self) -> PathSegments<'a> {
+        PathSegments {
+            segments: self.segments.get(..self.segments.len().saturating_sub(2)).unwrap_or(&[]),
+            generic_args: self
+                .generic_args
+                .map(|it| it.get(..it.len().saturating_sub(2)).unwrap_or(&[])),
+        }
+    }
+
     pub fn iter(&self) -> impl Iterator<Item = PathSegment<'a>> {
         self.segments
             .iter()