diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index c5d37cafd9..3a91050d15 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -97,7 +97,8 @@ pub use crate::{ diagnostics::*, has_source::HasSource, semantics::{ - PathResolution, Semantics, SemanticsImpl, SemanticsScope, TypeInfo, VisibleTraits, + PathResolution, PathResolutionPerNs, Semantics, SemanticsImpl, SemanticsScope, TypeInfo, + VisibleTraits, }, }; diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index 327fe8a758..7ee72614e2 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -103,6 +103,26 @@ impl PathResolution { } } +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub struct PathResolutionPerNs { + pub type_ns: Option, + pub value_ns: Option, + pub macro_ns: Option, +} + +impl PathResolutionPerNs { + pub fn new( + type_ns: Option, + value_ns: Option, + macro_ns: Option, + ) -> Self { + PathResolutionPerNs { type_ns, value_ns, macro_ns } + } + pub fn take_path(&self) -> Option { + self.type_ns.or(self.value_ns).or(self.macro_ns) + } +} + #[derive(Debug)] pub struct TypeInfo { /// The original type of the expression or pattern. @@ -1606,6 +1626,10 @@ impl<'db> SemanticsImpl<'db> { self.resolve_path_with_subst(path).map(|(it, _)| it) } + pub fn resolve_path_per_ns(&self, path: &ast::Path) -> Option { + self.analyze(path.syntax())?.resolve_hir_path_per_ns(self.db, path) + } + pub fn resolve_path_with_subst( &self, path: &ast::Path, diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs index 42da7b942c..610770d0f3 100644 --- a/crates/hir/src/source_analyzer.rs +++ b/crates/hir/src/source_analyzer.rs @@ -10,7 +10,9 @@ use std::iter::{self, once}; use crate::{ Adt, AssocItem, BindingMode, BuiltinAttr, BuiltinType, Callable, Const, DeriveHelper, Field, Function, GenericSubstitution, Local, Macro, ModuleDef, Static, Struct, ToolModule, Trait, - TraitAlias, TupleField, Type, TypeAlias, Variant, db::HirDatabase, semantics::PathResolution, + TraitAlias, TupleField, Type, TypeAlias, Variant, + db::HirDatabase, + semantics::{PathResolution, PathResolutionPerNs}, }; use either::Either; use hir_def::{ @@ -1159,7 +1161,9 @@ impl<'db> SourceAnalyzer<'db> { prefer_value_ns, name_hygiene(db, InFile::new(self.file_id, path.syntax())), Some(&store), - )?; + false, + ) + .take_path()?; let subst = (|| { let parent = parent()?; let ty = if let Some(expr) = ast::Expr::cast(parent.clone()) { @@ -1209,6 +1213,26 @@ impl<'db> SourceAnalyzer<'db> { } } + pub(crate) fn resolve_hir_path_per_ns( + &self, + db: &dyn HirDatabase, + path: &ast::Path, + ) -> Option { + let mut collector = ExprCollector::new(db, self.resolver.module(), self.file_id); + let hir_path = + collector.lower_path(path.clone(), &mut ExprCollector::impl_trait_error_allocator)?; + let store = collector.store.finish(); + Some(resolve_hir_path_( + db, + &self.resolver, + &hir_path, + false, + name_hygiene(db, InFile::new(self.file_id, path.syntax())), + Some(&store), + true, + )) + } + pub(crate) fn record_literal_missing_fields( &self, db: &'db dyn HirDatabase, @@ -1532,7 +1556,7 @@ pub(crate) fn resolve_hir_path( hygiene: HygieneId, store: Option<&ExpressionStore>, ) -> Option { - resolve_hir_path_(db, resolver, path, false, hygiene, store) + resolve_hir_path_(db, resolver, path, false, hygiene, store, false).take_path() } #[inline] @@ -1554,7 +1578,8 @@ fn resolve_hir_path_( prefer_value_ns: bool, hygiene: HygieneId, store: Option<&ExpressionStore>, -) -> Option { + resolve_per_ns: bool, +) -> PathResolutionPerNs { let types = || { let (ty, unresolved) = match path.type_anchor() { Some(type_ref) => resolver.generic_def().and_then(|def| { @@ -1635,9 +1660,31 @@ fn resolve_hir_path_( .map(|(def, _)| PathResolution::Def(ModuleDef::Macro(def.into()))) }; - if prefer_value_ns { values().or_else(types) } else { types().or_else(values) } - .or_else(items) - .or_else(macros) + if resolve_per_ns { + PathResolutionPerNs { + type_ns: types().or_else(items), + value_ns: values(), + macro_ns: macros(), + } + } else { + let res = if prefer_value_ns { + values() + .map(|value_ns| PathResolutionPerNs::new(None, Some(value_ns), None)) + .unwrap_or_else(|| PathResolutionPerNs::new(types(), None, None)) + } else { + types() + .map(|type_ns| PathResolutionPerNs::new(Some(type_ns), None, None)) + .unwrap_or_else(|| PathResolutionPerNs::new(None, values(), None)) + }; + + if res.take_path().is_some() { + res + } else if let Some(type_ns) = items() { + PathResolutionPerNs::new(Some(type_ns), None, None) + } else { + PathResolutionPerNs::new(None, None, macros()) + } + } } fn resolve_hir_value_path( diff --git a/crates/ide-assists/src/handlers/remove_unused_imports.rs b/crates/ide-assists/src/handlers/remove_unused_imports.rs index 1baf814ca6..fb96882ed4 100644 --- a/crates/ide-assists/src/handlers/remove_unused_imports.rs +++ b/crates/ide-assists/src/handlers/remove_unused_imports.rs @@ -1,6 +1,9 @@ use std::collections::hash_map::Entry; -use hir::{FileRange, InFile, InRealFile, Module, ModuleSource}; +use hir::{ + FileRange, InFile, InRealFile, Module, ModuleDef, ModuleSource, PathResolution, + PathResolutionPerNs, +}; use ide_db::text_edit::TextRange; use ide_db::{ FxHashMap, RootDatabase, @@ -77,49 +80,52 @@ pub(crate) fn remove_unused_imports(acc: &mut Assists, ctx: &AssistContext<'_>) }; // Get the actual definition associated with this use item. - let res = match ctx.sema.resolve_path(&path) { - Some(x) => x, - None => { + let res = match ctx.sema.resolve_path_per_ns(&path) { + Some(x) if x.take_path().is_some() => x, + Some(_) | None => { return None; } }; + match res { + PathResolutionPerNs { type_ns: Some(type_ns), .. } if u.star_token().is_some() => { + // Check if any of the children of this module are used + let def_mod = match type_ns { + PathResolution::Def(ModuleDef::Module(module)) => module, + _ => return None, + }; - let def = match res { - hir::PathResolution::Def(d) => Definition::from(d), - _ => return None, - }; - - if u.star_token().is_some() { - // Check if any of the children of this module are used - let def_mod = match def { - Definition::Module(module) => module, - _ => return None, - }; - - if !def_mod - .scope(ctx.db(), Some(use_module)) - .iter() - .filter_map(|(_, x)| match x { - hir::ScopeDef::ModuleDef(d) => Some(Definition::from(*d)), - _ => None, - }) - .any(|d| used_once_in_scope(ctx, d, u.rename(), scope)) - { - return Some(u); + if !def_mod + .scope(ctx.db(), Some(use_module)) + .iter() + .filter_map(|(_, x)| match x { + hir::ScopeDef::ModuleDef(d) => Some(Definition::from(*d)), + _ => None, + }) + .any(|d| used_once_in_scope(ctx, d, u.rename(), scope)) + { + Some(u) + } else { + None + } } - } else if let Definition::Trait(ref t) = def { - // If the trait or any item is used. - if !std::iter::once((def, u.rename())) - .chain(t.items(ctx.db()).into_iter().map(|item| (item.into(), None))) - .any(|(d, rename)| used_once_in_scope(ctx, d, rename, scope)) - { - return Some(u); + PathResolutionPerNs { + type_ns: Some(PathResolution::Def(ModuleDef::Trait(ref t))), + value_ns, + macro_ns, + } => { + // If the trait or any item is used. + if is_trait_unused_in_scope(ctx, &u, scope, t) { + let path = [value_ns, macro_ns]; + is_path_unused_in_scope(ctx, &u, scope, &path).then_some(u) + } else { + None + } + } + PathResolutionPerNs { type_ns, value_ns, macro_ns } => { + let path = [type_ns, value_ns, macro_ns]; + is_path_unused_in_scope(ctx, &u, scope, &path).then_some(u) } - } else if !used_once_in_scope(ctx, def, u.rename(), scope) { - return Some(u); } - - None }) .peekable(); @@ -141,6 +147,33 @@ pub(crate) fn remove_unused_imports(acc: &mut Assists, ctx: &AssistContext<'_>) } } +fn is_path_unused_in_scope( + ctx: &AssistContext<'_>, + u: &ast::UseTree, + scope: &mut Vec, + path: &[Option], +) -> bool { + !path + .iter() + .filter_map(|path| *path) + .filter_map(|res| match res { + PathResolution::Def(d) => Some(Definition::from(d)), + _ => None, + }) + .any(|def| used_once_in_scope(ctx, def, u.rename(), scope)) +} + +fn is_trait_unused_in_scope( + ctx: &AssistContext<'_>, + u: &ast::UseTree, + scope: &mut Vec, + t: &hir::Trait, +) -> bool { + !std::iter::once((Definition::Trait(*t), u.rename())) + .chain(t.items(ctx.db()).into_iter().map(|item| (item.into(), None))) + .any(|(d, rename)| used_once_in_scope(ctx, d, rename, scope)) +} + fn used_once_in_scope( ctx: &AssistContext<'_>, def: Definition, @@ -1009,6 +1042,112 @@ fn test(_: Bar) { let a = (); a.quxx(); } +"#, + ); + } + + #[test] + fn test_unused_macro() { + check_assist( + remove_unused_imports, + r#" +//- /foo.rs crate:foo +#[macro_export] +macro_rules! m { () => {} } + +//- /main.rs crate:main deps:foo +use foo::m;$0 +fn main() {} +"#, + r#" +fn main() {} +"#, + ); + + check_assist_not_applicable( + remove_unused_imports, + r#" +//- /foo.rs crate:foo +#[macro_export] +macro_rules! m { () => {} } + +//- /main.rs crate:main deps:foo +use foo::m;$0 +fn main() { + m!(); +} +"#, + ); + + check_assist_not_applicable( + remove_unused_imports, + r#" +//- /foo.rs crate:foo +#[macro_export] +macro_rules! m { () => {} } + +//- /bar.rs crate:bar deps:foo +pub use foo::m; +fn m() {} + + +//- /main.rs crate:main deps:bar +use bar::m;$0 +fn main() { + m!(); +} +"#, + ); + } + + #[test] + fn test_conflict_derive_macro() { + check_assist_not_applicable( + remove_unused_imports, + r#" +//- proc_macros: derive_identity +//- minicore: derive +//- /bar.rs crate:bar +pub use proc_macros::DeriveIdentity; +pub trait DeriveIdentity {} + +//- /main.rs crate:main deps:bar +$0use bar::DeriveIdentity;$0 +#[derive(DeriveIdentity)] +struct S; +"#, + ); + + check_assist_not_applicable( + remove_unused_imports, + r#" +//- proc_macros: derive_identity +//- minicore: derive +//- /bar.rs crate:bar +pub use proc_macros::DeriveIdentity; +pub fn DeriveIdentity() {} + +//- /main.rs crate:main deps:bar +$0use bar::DeriveIdentity;$0 +#[derive(DeriveIdentity)] +struct S; +"#, + ); + + check_assist_not_applicable( + remove_unused_imports, + r#" +//- proc_macros: derive_identity +//- minicore: derive +//- /bar.rs crate:bar +pub use proc_macros::DeriveIdentity; +pub fn DeriveIdentity() {} + +//- /main.rs crate:main deps:bar +$0use bar::DeriveIdentity;$0 +fn main() { + DeriveIdentity(); +} "#, ); }