mirror of
				https://github.com/rust-lang/rust-analyzer.git
				synced 2025-10-31 12:04:43 +00:00 
			
		
		
		
	Fix incorrect handling of unresolved non-module imports in name resolution
This commit is contained in:
		
							parent
							
								
									7d51ec376a
								
							
						
					
					
						commit
						9a62507f2e
					
				
					 6 changed files with 303 additions and 22 deletions
				
			
		|  | @ -701,6 +701,16 @@ pub enum AssocItemId { | |||
| // casting them, and somehow making the constructors private, which would be annoying.
 | ||||
| impl_from!(FunctionId, ConstId, TypeAliasId for AssocItemId); | ||||
| 
 | ||||
| impl From<AssocItemId> for ModuleDefId { | ||||
|     fn from(item: AssocItemId) -> Self { | ||||
|         match item { | ||||
|             AssocItemId::FunctionId(f) => f.into(), | ||||
|             AssocItemId::ConstId(c) => c.into(), | ||||
|             AssocItemId::TypeAliasId(t) => t.into(), | ||||
|         } | ||||
|     } | ||||
| } | ||||
| 
 | ||||
| #[derive(Debug, PartialOrd, Ord, Clone, Copy, PartialEq, Eq, Hash, salsa_macros::Supertype)] | ||||
| pub enum GenericDefId { | ||||
|     AdtId(AdtId), | ||||
|  |  | |||
|  | @ -66,6 +66,15 @@ impl TraitItems { | |||
|         }) | ||||
|     } | ||||
| 
 | ||||
|     pub fn assoc_item_by_name(&self, name: &Name) -> Option<AssocItemId> { | ||||
|         self.items.iter().find_map(|&(ref item_name, item)| match item { | ||||
|             AssocItemId::FunctionId(_) if item_name == name => Some(item), | ||||
|             AssocItemId::TypeAliasId(_) if item_name == name => Some(item), | ||||
|             AssocItemId::ConstId(_) if item_name == name => Some(item), | ||||
|             _ => None, | ||||
|         }) | ||||
|     } | ||||
| 
 | ||||
|     pub fn attribute_calls(&self) -> impl Iterator<Item = (AstId<ast::Item>, MacroCallId)> + '_ { | ||||
|         self.macro_calls.iter().flat_map(|it| it.iter()).copied() | ||||
|     } | ||||
|  |  | |||
|  | @ -26,7 +26,7 @@ use syntax::ast; | |||
| use triomphe::Arc; | ||||
| 
 | ||||
| use crate::{ | ||||
|     AdtId, AstId, AstIdWithPath, ConstLoc, CrateRootModuleId, EnumLoc, ExternBlockLoc, | ||||
|     AdtId, AssocItemId, AstId, AstIdWithPath, ConstLoc, CrateRootModuleId, EnumLoc, ExternBlockLoc, | ||||
|     ExternCrateId, ExternCrateLoc, FunctionId, FunctionLoc, ImplLoc, Intern, ItemContainerId, | ||||
|     LocalModuleId, Lookup, Macro2Id, Macro2Loc, MacroExpander, MacroId, MacroRulesId, | ||||
|     MacroRulesLoc, MacroRulesLocFlags, ModuleDefId, ModuleId, ProcMacroId, ProcMacroLoc, StaticLoc, | ||||
|  | @ -45,7 +45,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, ResolvePathResult}, | ||||
|         proc_macro::{ProcMacroDef, ProcMacroKind, parse_macro_name_and_helper_attrs}, | ||||
|         sub_namespace_match, | ||||
|     }, | ||||
|  | @ -811,32 +811,35 @@ impl DefCollector<'_> { | |||
|         let _p = tracing::info_span!("resolve_import", import_path = %import.path.display(self.db, Edition::LATEST)) | ||||
|             .entered(); | ||||
|         tracing::debug!("resolving import: {:?} ({:?})", import, self.def_map.data.edition); | ||||
|         let res = self.def_map.resolve_path_fp_with_macro( | ||||
|             self.crate_local_def_map.as_deref().unwrap_or(&self.local_def_map), | ||||
|             self.db, | ||||
|             ResolveMode::Import, | ||||
|             module_id, | ||||
|             &import.path, | ||||
|             BuiltinShadowMode::Module, | ||||
|             None, // An import may resolve to any kind of macro.
 | ||||
|         ); | ||||
|         let ResolvePathResult { resolved_def, segment_index, reached_fixedpoint, prefix_info } = | ||||
|             self.def_map.resolve_path_fp_with_macro( | ||||
|                 self.crate_local_def_map.as_deref().unwrap_or(&self.local_def_map), | ||||
|                 self.db, | ||||
|                 ResolveMode::Import, | ||||
|                 module_id, | ||||
|                 &import.path, | ||||
|                 BuiltinShadowMode::Module, | ||||
|                 None, // An import may resolve to any kind of macro.
 | ||||
|             ); | ||||
| 
 | ||||
|         let def = res.resolved_def; | ||||
|         if res.reached_fixedpoint == ReachedFixedPoint::No || def.is_none() { | ||||
|         if reached_fixedpoint == ReachedFixedPoint::No | ||||
|             || resolved_def.is_none() | ||||
|             || segment_index.is_some() | ||||
|         { | ||||
|             return PartialResolvedImport::Unresolved; | ||||
|         } | ||||
| 
 | ||||
|         if res.prefix_info.differing_crate { | ||||
|         if prefix_info.differing_crate { | ||||
|             return PartialResolvedImport::Resolved( | ||||
|                 def.filter_visibility(|v| matches!(v, Visibility::Public)), | ||||
|                 resolved_def.filter_visibility(|v| matches!(v, Visibility::Public)), | ||||
|             ); | ||||
|         } | ||||
| 
 | ||||
|         // Check whether all namespaces are resolved.
 | ||||
|         if def.is_full() { | ||||
|             PartialResolvedImport::Resolved(def) | ||||
|         if resolved_def.is_full() { | ||||
|             PartialResolvedImport::Resolved(resolved_def) | ||||
|         } else { | ||||
|             PartialResolvedImport::Indeterminate(def) | ||||
|             PartialResolvedImport::Indeterminate(resolved_def) | ||||
|         } | ||||
|     } | ||||
| 
 | ||||
|  | @ -986,6 +989,43 @@ impl DefCollector<'_> { | |||
|                             Some(ImportOrExternCrate::Glob(glob)), | ||||
|                         ); | ||||
|                     } | ||||
|                     Some(ModuleDefId::TraitId(it)) => { | ||||
|                         // FIXME: Implement this correctly
 | ||||
|                         // We can't actually call `trait_items`, the reason being that if macro calls
 | ||||
|                         // occur, they will call back into the def map which we might be computing right
 | ||||
|                         // now resulting in a cycle.
 | ||||
|                         // To properly implement this, trait item collection needs to be done in def map
 | ||||
|                         // collection...
 | ||||
|                         let resolutions = if true { | ||||
|                             vec![] | ||||
|                         } else { | ||||
|                             self.db | ||||
|                                 .trait_items(it) | ||||
|                                 .items | ||||
|                                 .iter() | ||||
|                                 .map(|&(ref name, variant)| { | ||||
|                                     let res = match variant { | ||||
|                                         AssocItemId::FunctionId(it) => { | ||||
|                                             PerNs::values(it.into(), vis, None) | ||||
|                                         } | ||||
|                                         AssocItemId::ConstId(it) => { | ||||
|                                             PerNs::values(it.into(), vis, None) | ||||
|                                         } | ||||
|                                         AssocItemId::TypeAliasId(it) => { | ||||
|                                             PerNs::types(it.into(), vis, None) | ||||
|                                         } | ||||
|                                     }; | ||||
|                                     (Some(name.clone()), res) | ||||
|                                 }) | ||||
|                                 .collect::<Vec<_>>() | ||||
|                         }; | ||||
|                         self.update( | ||||
|                             module_id, | ||||
|                             &resolutions, | ||||
|                             vis, | ||||
|                             Some(ImportOrExternCrate::Glob(glob)), | ||||
|                         ); | ||||
|                     } | ||||
|                     Some(d) => { | ||||
|                         tracing::debug!("glob import {:?} from non-module/enum {:?}", import, d); | ||||
|                     } | ||||
|  |  | |||
|  | @ -17,6 +17,7 @@ use hir_expand::{ | |||
|     name::Name, | ||||
| }; | ||||
| use span::Edition; | ||||
| use stdx::TupleExt; | ||||
| use triomphe::Arc; | ||||
| 
 | ||||
| use crate::{ | ||||
|  | @ -44,6 +45,7 @@ pub(super) enum ReachedFixedPoint { | |||
| #[derive(Debug, Clone)] | ||||
| pub(super) struct ResolvePathResult { | ||||
|     pub(super) resolved_def: PerNs, | ||||
|     /// The index of the last resolved segment, or `None` if the full path has been resolved.
 | ||||
|     pub(super) segment_index: Option<usize>, | ||||
|     pub(super) reached_fixedpoint: ReachedFixedPoint, | ||||
|     pub(super) prefix_info: ResolvePathResultPrefixInfo, | ||||
|  | @ -364,7 +366,15 @@ impl DefMap { | |||
|             }, | ||||
|         }; | ||||
| 
 | ||||
|         self.resolve_remaining_segments(segments, curr_per_ns, path, db, shadow, original_module) | ||||
|         self.resolve_remaining_segments( | ||||
|             db, | ||||
|             mode, | ||||
|             segments, | ||||
|             curr_per_ns, | ||||
|             path, | ||||
|             shadow, | ||||
|             original_module, | ||||
|         ) | ||||
|     } | ||||
| 
 | ||||
|     /// Resolves a path only in the preludes, without accounting for item scopes.
 | ||||
|  | @ -413,7 +423,15 @@ impl DefMap { | |||
|             } | ||||
|         }; | ||||
| 
 | ||||
|         self.resolve_remaining_segments(segments, curr_per_ns, path, db, shadow, original_module) | ||||
|         self.resolve_remaining_segments( | ||||
|             db, | ||||
|             mode, | ||||
|             segments, | ||||
|             curr_per_ns, | ||||
|             path, | ||||
|             shadow, | ||||
|             original_module, | ||||
|         ) | ||||
|     } | ||||
| 
 | ||||
|     /// 2018-style absolute path -- only extern prelude
 | ||||
|  | @ -441,10 +459,11 @@ impl DefMap { | |||
| 
 | ||||
|     fn resolve_remaining_segments<'a>( | ||||
|         &self, | ||||
|         db: &dyn DefDatabase, | ||||
|         mode: ResolveMode, | ||||
|         mut segments: impl Iterator<Item = (usize, &'a Name)>, | ||||
|         mut curr_per_ns: PerNs, | ||||
|         path: &ModPath, | ||||
|         db: &dyn DefDatabase, | ||||
|         shadow: BuiltinShadowMode, | ||||
|         original_module: LocalModuleId, | ||||
|     ) -> ResolvePathResult { | ||||
|  | @ -478,7 +497,7 @@ impl DefMap { | |||
|                         let resolution = defp_map.resolve_path_fp_with_macro( | ||||
|                             LocalDefMap::EMPTY, | ||||
|                             db, | ||||
|                             ResolveMode::Other, | ||||
|                             mode, | ||||
|                             module.local_id, | ||||
|                             &path, | ||||
|                             shadow, | ||||
|  | @ -553,6 +572,44 @@ impl DefMap { | |||
|                         ), | ||||
|                     }; | ||||
|                 } | ||||
|                 def @ ModuleDefId::TraitId(t) if mode == ResolveMode::Import => { | ||||
|                     // FIXME: Implement this correctly
 | ||||
|                     // We can't actually call `trait_items`, the reason being that if macro calls
 | ||||
|                     // occur, they will call back into the def map which we might be computing right
 | ||||
|                     // now resulting in a cycle.
 | ||||
|                     // To properly implement this, trait item collection needs to be done in def map
 | ||||
|                     // collection...
 | ||||
|                     let item = | ||||
|                         if true { None } else { db.trait_items(t).assoc_item_by_name(segment) }; | ||||
|                     return match item { | ||||
|                         Some(item) => ResolvePathResult::new( | ||||
|                             match item { | ||||
|                                 crate::AssocItemId::FunctionId(function_id) => PerNs::values( | ||||
|                                     function_id.into(), | ||||
|                                     curr.vis, | ||||
|                                     curr.import.and_then(|it| it.import_or_glob()), | ||||
|                                 ), | ||||
|                                 crate::AssocItemId::ConstId(const_id) => PerNs::values( | ||||
|                                     const_id.into(), | ||||
|                                     curr.vis, | ||||
|                                     curr.import.and_then(|it| it.import_or_glob()), | ||||
|                                 ), | ||||
|                                 crate::AssocItemId::TypeAliasId(type_alias_id) => { | ||||
|                                     PerNs::types(type_alias_id.into(), curr.vis, curr.import) | ||||
|                                 } | ||||
|                             }, | ||||
|                             ReachedFixedPoint::Yes, | ||||
|                             segments.next().map(TupleExt::head), | ||||
|                             ResolvePathResultPrefixInfo::default(), | ||||
|                         ), | ||||
|                         None => ResolvePathResult::new( | ||||
|                             PerNs::types(def, curr.vis, curr.import), | ||||
|                             ReachedFixedPoint::Yes, | ||||
|                             Some(i), | ||||
|                             ResolvePathResultPrefixInfo::default(), | ||||
|                         ), | ||||
|                     }; | ||||
|                 } | ||||
|                 s => { | ||||
|                     // could be an inherent method call in UFCS form
 | ||||
|                     // (`Struct::method`), or some other kind of associated item
 | ||||
|  |  | |||
|  | @ -894,3 +894,149 @@ struct AlsoShouldNotAppear; | |||
|         "#]],
 | ||||
|     ) | ||||
| } | ||||
| 
 | ||||
| #[test] | ||||
| fn invalid_imports() { | ||||
|     check( | ||||
|         r#" | ||||
| //- /main.rs
 | ||||
| mod module; | ||||
| 
 | ||||
| use self::module::S::new; | ||||
| use self::module::unresolved; | ||||
| use self::module::C::const_based; | ||||
| use self::module::Enum::Variant::NoAssoc; | ||||
| 
 | ||||
| //- /module.rs
 | ||||
| pub struct S; | ||||
| impl S { | ||||
|     pub fn new() {} | ||||
| } | ||||
| pub const C: () = (); | ||||
| pub enum Enum { | ||||
|     Variant, | ||||
| } | ||||
|         "#,
 | ||||
|         expect![[r#" | ||||
|             crate | ||||
|             NoAssoc: _ | ||||
|             const_based: _ | ||||
|             module: t | ||||
|             new: _ | ||||
|             unresolved: _ | ||||
| 
 | ||||
|             crate::module | ||||
|             C: v | ||||
|             Enum: t | ||||
|             S: t v | ||||
|         "#]],
 | ||||
|     ); | ||||
| } | ||||
| 
 | ||||
| #[test] | ||||
| fn trait_item_imports_same_crate() { | ||||
|     check( | ||||
|         r#" | ||||
| //- /main.rs
 | ||||
| mod module; | ||||
| 
 | ||||
| use self::module::Trait::{AssocType, ASSOC_CONST, MACRO_CONST, method}; | ||||
| 
 | ||||
| //- /module.rs
 | ||||
| macro_rules! m { | ||||
|     ($name:ident) => { const $name: () = (); }; | ||||
| } | ||||
| pub trait Trait { | ||||
|     type AssocType; | ||||
|     const ASSOC_CONST: (); | ||||
|     fn method(&self); | ||||
|     m!(MACRO_CONST); | ||||
| } | ||||
|         "#,
 | ||||
|         expect![[r#" | ||||
|             crate | ||||
|             ASSOC_CONST: _ | ||||
|             AssocType: _ | ||||
|             MACRO_CONST: _ | ||||
|             method: _ | ||||
|             module: t | ||||
| 
 | ||||
|             crate::module | ||||
|             Trait: t | ||||
|         "#]],
 | ||||
|     ); | ||||
|     check( | ||||
|         r#" | ||||
| //- /main.rs
 | ||||
| mod module; | ||||
| 
 | ||||
| use self::module::Trait::*; | ||||
| 
 | ||||
| //- /module.rs
 | ||||
| macro_rules! m { | ||||
|     ($name:ident) => { const $name: () = (); }; | ||||
| } | ||||
| pub trait Trait { | ||||
|     type AssocType; | ||||
|     const ASSOC_CONST: (); | ||||
|     fn method(&self); | ||||
|     m!(MACRO_CONST); | ||||
| } | ||||
|         "#,
 | ||||
|         expect![[r#" | ||||
|             crate | ||||
|             module: t | ||||
| 
 | ||||
|             crate::module | ||||
|             Trait: t | ||||
|         "#]],
 | ||||
|     ); | ||||
| } | ||||
| 
 | ||||
| #[test] | ||||
| fn trait_item_imports_differing_crate() { | ||||
|     check( | ||||
|         r#" | ||||
| //- /main.rs deps:lib crate:main
 | ||||
| use lib::Trait::{AssocType, ASSOC_CONST, MACRO_CONST, method}; | ||||
| 
 | ||||
| //- /lib.rs crate:lib
 | ||||
| macro_rules! m { | ||||
|     ($name:ident) => { const $name: () = (); }; | ||||
| } | ||||
| pub trait Trait { | ||||
|     type AssocType; | ||||
|     const ASSOC_CONST: (); | ||||
|     fn method(&self); | ||||
|     m!(MACRO_CONST); | ||||
| } | ||||
|         "#,
 | ||||
|         expect![[r#" | ||||
|             crate | ||||
|             ASSOC_CONST: _ | ||||
|             AssocType: _ | ||||
|             MACRO_CONST: _ | ||||
|             method: _ | ||||
|         "#]],
 | ||||
|     ); | ||||
|     check( | ||||
|         r#" | ||||
| //- /main.rs deps:lib crate:main
 | ||||
| use lib::Trait::*; | ||||
| 
 | ||||
| //- /lib.rs crate:lib
 | ||||
| macro_rules! m { | ||||
|     ($name:ident) => { const $name: () = (); }; | ||||
| } | ||||
| pub trait Trait { | ||||
|     type AssocType; | ||||
|     const ASSOC_CONST: (); | ||||
|     fn method(&self); | ||||
|     m!(MACRO_CONST); | ||||
| } | ||||
|         "#,
 | ||||
|         expect![[r#" | ||||
|             crate | ||||
|         "#]],
 | ||||
|     ); | ||||
| } | ||||
|  |  | |||
|  | @ -4884,3 +4884,22 @@ async fn baz<T: AsyncFnOnce(u32) -> i32>(c: T) { | |||
|         "#]],
 | ||||
|     ); | ||||
| } | ||||
| 
 | ||||
| #[test] | ||||
| fn import_trait_items() { | ||||
|     check_infer( | ||||
|         r#" | ||||
| //- minicore: default
 | ||||
| use core::default::Default::default; | ||||
| fn main() { | ||||
|     let a: i32 = default(); | ||||
| } | ||||
|     "#,
 | ||||
|         expect![[r#" | ||||
|             47..78 '{     ...t(); }': () | ||||
|             57..58 'a': i32 | ||||
|             66..73 'default': {unknown} | ||||
|             66..75 'default()': i32 | ||||
|         "#]],
 | ||||
|     ); | ||||
| } | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Lukas Wirth
						Lukas Wirth