fix: Removing all unused imports removes used imports for imports used for Derive macros
Signed-off-by: Hayashi Mikihiro <34ttrweoewiwe28@gmail.com>
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index c5d37ca..3a91050 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -97,7 +97,8 @@
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 327fe8a..7ee7261 100644
--- a/crates/hir/src/semantics.rs
+++ b/crates/hir/src/semantics.rs
@@ -103,6 +103,26 @@
}
}
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+pub struct PathResolutionPerNs {
+ pub type_ns: Option<PathResolution>,
+ pub value_ns: Option<PathResolution>,
+ pub macro_ns: Option<PathResolution>,
+}
+
+impl PathResolutionPerNs {
+ pub fn new(
+ type_ns: Option<PathResolution>,
+ value_ns: Option<PathResolution>,
+ macro_ns: Option<PathResolution>,
+ ) -> Self {
+ PathResolutionPerNs { type_ns, value_ns, macro_ns }
+ }
+ pub fn take_path(&self) -> Option<PathResolution> {
+ 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 @@
self.resolve_path_with_subst(path).map(|(it, _)| it)
}
+ pub fn resolve_path_per_ns(&self, path: &ast::Path) -> Option<PathResolutionPerNs> {
+ 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 42da7b9..610770d 100644
--- a/crates/hir/src/source_analyzer.rs
+++ b/crates/hir/src/source_analyzer.rs
@@ -10,7 +10,9 @@
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 @@
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 @@
}
}
+ pub(crate) fn resolve_hir_path_per_ns(
+ &self,
+ db: &dyn HirDatabase,
+ path: &ast::Path,
+ ) -> Option<PathResolutionPerNs> {
+ 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 @@
hygiene: HygieneId,
store: Option<&ExpressionStore>,
) -> Option<PathResolution> {
- 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 @@
prefer_value_ns: bool,
hygiene: HygieneId,
store: Option<&ExpressionStore>,
-) -> Option<PathResolution> {
+ 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 @@
.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 1baf814..fb96882 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 @@
};
// 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
+ }
}
- } else if !used_once_in_scope(ctx, def, u.rename(), scope) {
- return Some(u);
+ 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)
+ }
}
-
- None
})
.peekable();
@@ -141,6 +147,33 @@
}
}
+fn is_path_unused_in_scope(
+ ctx: &AssistContext<'_>,
+ u: &ast::UseTree,
+ scope: &mut Vec<SearchScope>,
+ path: &[Option<PathResolution>],
+) -> 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<SearchScope>,
+ 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,
@@ -1012,4 +1045,110 @@
"#,
);
}
+
+ #[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();
+}
+"#,
+ );
+ }
}
