Test function imports replaced by module imports.

[rwakulszowa]

Test for #18347.
The issue affects all code using import merging. When a function and a module share the same identifier, e.g.

pub fn foo() {}
pub mod foo { pub fn bar() {} }

rust analyzer often confuses the function with the module, generating invalid code.

[rwakulszowa]

I haven't been able to actually find a proper fix yet. I can't find a reliable way to tell the merging function whether an import is a module (and can be used for merging) or a function.

I'm not sure if I manage to find a solution, so I'm sending a small PR with what I have.

[Veykril]

The IDE layer currently lacks being able to resolve something to multiple namespaces (Semantics::resolve_path only yields one namespace at most), so that needs tackling first and is unfortunately a bigger change.

[Veykril]

I'll close this for now due to inactivity, feel free to re-open if you try to tackle it again

[rwakulszowa]

I'll give it another try.

---

The IDE layer currently lacks being able to resolve something to multiple namespaces (Semantics::resolve_path only yields one namespace at most), so that needs tackling first and is unfortunately a bigger change.

I'm not sure I get this part. Why would we need resolve_path to resolve to multiple namespaces?

Let's look at the auto_import case.

rust-analyzer/crates/ide-assists/src/handlers/auto_import.rs

Lines 131 to 144 in 62dea27

	acc.add_group(
	&group_label,
	assist_id,
	format!("Import `{import_name}`"),
	range,
	|builder| {
	let scope = match scope.clone() {
	ImportScope::File(it) => ImportScope::File(builder.make_mut(it)),
	ImportScope::Module(it) => ImportScope::Module(builder.make_mut(it)),
	ImportScope::Block(it) => ImportScope::Block(builder.make_mut(it)),
	};
	insert_use(&scope, mod_path_to_ast(&import_path, edition), &ctx.config.insert_use);
	},
	);

My (limited) understanding is as follows. When the user selects the suggestion, we generate a new use ... tree

rust-analyzer/crates/ide-db/src/imports/insert_use.rs

Line 213 in 62dea27

let mut use_tree = make::use_tree(path, None, alias, false);

and then try to merge it into existing imports

rust-analyzer/crates/ide-db/src/imports/insert_use.rs

Lines 220 to 231 in 62dea27

	// merge into existing imports if possible
	if let Some(mb) = mb {
	let filter = |it: &_| !(cfg.skip_glob_imports && ast::Use::is_simple_glob(it));
	for existing_use in
	scope.as_syntax_node().children().filter_map(ast::Use::cast).filter(filter)
	{
	if let Some(merged) = try_merge_imports(&existing_use, &use_item, mb) {
	ted::replace(existing_use.syntax(), merged.syntax());
	return;
	}
	}
	}

ultimately ending at

rust-analyzer/crates/ide-db/src/imports/merge_imports.rs

Lines 87 to 118 in 62dea27

	fn try_merge_trees_mut(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehavior) -> Option<()> {
	if merge == MergeBehavior::One {
	lhs.wrap_in_tree_list();
	rhs.wrap_in_tree_list();
	} else {
	let lhs_path = lhs.path()?;
	let rhs_path = rhs.path()?;
	let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?;
	if lhs.is_simple_path()
	&& rhs.is_simple_path()
	&& lhs_path == lhs_prefix
	&& rhs_path == rhs_prefix
	{
	// we can't merge if the renames are different (`A as a` and `A as b`),
	// and we can safely return here
	let lhs_name = lhs.rename().and_then(|lhs_name| lhs_name.name());
	let rhs_name = rhs.rename().and_then(|rhs_name| rhs_name.name());
	if lhs_name != rhs_name {
	return None;
	}
	ted::replace(lhs.syntax(), rhs.syntax());
	// we can safely return here, in this case `recursive_merge` doesn't do anything
	return Some(());
	} else {
	lhs.split_prefix(&lhs_prefix);
	rhs.split_prefix(&rhs_prefix);
	}
	}
	recursive_merge(lhs, rhs, merge)
	}

IIUC, the root cause is how the common prefix is calculated - the current logic is not Sema aware, it finds the common prefix (== merge point) doing simple string comparison - if the same string is a module and a function, we end up with the buggy suggestion. If we add Sema to that function, we will be able to make sure that common prefix ends with a module

// pseudocode
let mut prefix = /*current logic*/
if Sema::resolve_path(prefix) is module {
  prefix = prefix.parent();
}

Would that do the trick?

[Veykril]

My point is that resolve_path might tell us that the path resolves to a module, but it also reoslves to a macro (because of namespaces). So now we think it only resolves to a module, while it actually resolves to other things as well and thus the check is incorrect. We need to discern between the case of the path only resolving to a module or to resolving at least one non-module thing.

[rwakulszowa]

Mkay.
Nvm then :/