fix: Removing all unused imports removes used imports for imports used for Derive macros #19793
Conversation
…d for Derive macros Signed-off-by: Hayashi Mikihiro <[email protected]>
rustbot
added
the
S-waiting-on-review
| 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 { |
There was a problem hiding this comment.
I resolve type per namespace and make Struct for keep with split.
| 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, | ||
| )) | ||
| } |
There was a problem hiding this comment.
When we check unused import, we check only tail of path.
use foo::bar;
^^^So I check only by resolve_hir_path_ instead of resolve_path
| ) -> Self { | ||
| PathResolutionPerNs { type_ns, value_ns, macro_ns } | ||
| } | ||
| pub fn take_path(&self) -> Option<PathResolution> { |
There was a problem hiding this comment.
| pub fn take_path(&self) -> Option<PathResolution> { | |
| pub fn any(&self) -> Option<PathResolution> { |
take methods usually modify the object (e.g. Option::take()), I propose to call this any().
| { | ||
| return Some(u); | ||
| match res { | ||
| PathResolutionPerNs { type_ns: Some(type_ns), .. } if u.star_token().is_some() => { |
There was a problem hiding this comment.
I think the condition needs to be put differently: if u.star_token().is_some(), then extract the module from it or return None if it isn't a module. This is because if we don't have a type ns resolution but it's a glob import, it makes little sense to handle it like value/macro ns.
| { | ||
| return Some(u); | ||
| PathResolutionPerNs { | ||
| type_ns: Some(PathResolution::Def(ModuleDef::Trait(ref t))), |
There was a problem hiding this comment.
I think it's better to handle traits in is_path_unused_in_scope() specifically and not duplicate the arm here.
fix: #19119
2025-05-15.1.05.49.mov