suggestions from other crates only work when an explicit `extern crate` is present

[nikomatsakis]

UPDATE: There has been some progress on this issue. crate:: prefixes now work, but you still don't get suggestions from other crates except when you write an explicit extern crate.

When you get name resolution suggestions from resolve after an error, they should use crate:: prefixes where appropriate.

Example (play):

#![feature(crate_visibility_modifier)]
#![feature(crate_in_paths)]

mod bar {
    crate struct Foo;
}

fn main() {
    Foo;
}

gives

error[E0425]: cannot find value `Foo` in this scope
 --> src/main.rs:9:5
  |
9 |     Foo;
  |     ^^^ not found in this scope
help: possible candidate is found in another module, you can import it into scope
  |
4 | use bar::Foo;
  |

but I want to see use crate::bar::Foo suggested.

[nikomatsakis]

It looks like the suggestions are issued here:

rust/src/librustc_resolve/lib.rs

Lines 4436 to 4463 in c131bdc

	let better = if better { "better " } else { "" };
	let msg_diff = match path_strings.len() {
	1 => " is found in another module, you can import it",
	_ => "s are found in other modules, you can import them",
	};
	let msg = format!("possible {}candidate{} into scope", better, msg_diff);
	if let Some(span) = span {
	for candidate in &mut path_strings {
	// produce an additional newline to separate the new use statement
	// from the directly following item.
	let additional_newline = if found_use {
	""
	} else {
	"\n"
	};
	*candidate = format!("use {};\n{}", candidate, additional_newline);
	}
	err.span_suggestions(span, &msg, path_strings);
	} else {
	let mut msg = msg;
	msg.push(':');
	for candidate in path_strings {
	msg.push('\n');
	msg.push_str(&candidate);
	}
	}

The behavior I think we want is this:

If the crate_in_paths feature is enabled, then we should suggest use crate::bar::Foo when bar is from a module and not an external crate.

The function that prepares those suggestions appears to be this one:

rust/src/librustc_resolve/lib.rs

Lines 4398 to 4402 in c131bdc

	fn path_names_to_string(path: &Path) -> String {
	names_to_string(&path.segments.iter()
	.map(|seg| seg.ident)
	.collect::<Vec<_>>())
	}

Here is an example of checking for whether the feature is enabled:

rust/src/librustc_resolve/lib.rs

Line 3493 in c131bdc

if !self.session.features_untracked().crate_in_paths {

[nikomatsakis]

@qmx expressed some interest in tackling this.

[nikomatsakis]

Hmm, now i'm worried that this will apply even when it shouldn't. Consider this testcase:

fn main() {
    let x: &Debug = &22;
}

This currently prints:

error[E0412]: cannot find type `Debug` in this scope
 --> src/main.rs:2:13
  |
2 |     let x: &Debug = &22;
  |             ^^^^^ not found in this scope
help: possible candidate is found in another module, you can import it into scope
  |
1 | use std::fmt::Debug;
  |

and I think we do not want it to print use crate::std::fmt::Debug.

I think this suggests an alternative fix to what I recommended before (and what @qmx did in #51456) -- but I'd like to get @petrochenkov's opinion. Before I suggested changing how the ImportSuggestion values are printed out, but now I think we should change them at the source:

rust/src/librustc_resolve/lib.rs

Lines 4003 to 4015 in a32e979

	/// When name resolution fails, this method can be used to look up candidate
	/// entities with the expected name. It allows filtering them using the
	/// supplied predicate (which should be used to only accept the types of
	/// definitions expected e.g. traits). The lookup spans across all crates.
	///
	/// NOTE: The method does not look into imports, but this is not a problem,
	/// since we report the definitions (thus, the de-aliased imports).
	fn lookup_import_candidates<FilterFn>(&mut self,
	lookup_name: Name,
	namespace: Namespace,
	filter_fn: FilterFn)
	-> Vec<ImportSuggestion>
	where FilterFn: Fn(Def) -> bool

Right now, this works by walking through the crate and finding candidates. I suspect (but we should test) that this means it will not work when the extern_prelude option is enabled and no explicit extern crate exists, because there is nothing to walk over. Therefore, the change I propose is as follows:

1. Modify the function to (if crate_in_paths is enabled) to push a crate at the front of the import suggestions it is currently creating. This is because all of them are found by trawling the crate source.
2. Then, if extern_prelude is enabled, add a loop that executes first. This would go over all of the crates that are available from the command line, load them up, and search for the item within (basically the same as the existing search, but beginning from the crate directly). In this case, we would not prepend crate:: to the resulting path, but instead the name of the crate we are searching.

• One flaw I can see here is that it may be that the crate is locally shadowed. I suspect we can overlook that possibility for now.

[nikomatsakis]

@petrochenkov do the above notes seem ok?

[petrochenkov]

Looks reasonable.

[nikomatsakis]

To get a bit more detailed in how to make this modification, ultimately I envision we'll have a helper like this:

fn lookup_import_candidates_from_module(
    &mut self, 
    lookup_name: Name, 
    namespace: Namespace, 
    start_module: Module<'_>,
    filter_fn: impl Fn(Def) -> bool,
) -> Vec<ImportSuggestion> {
    ...
}

This will be roughly the same as the existing lookup_import_candidates, except that instead of starting from self.graph_root, it will seed the worklist with start_module:

rust/src/librustc_resolve/lib.rs

Line 4020 in a32e979

worklist.push((self.graph_root, Vec::new(), false));

Then, later, when it finds a suggestion:

rust/src/librustc_resolve/lib.rs

Line 4053 in a32e979

candidates.push(ImportSuggestion { path: path });

It will check if start_module == self.graph_root and the crate_in_paths feature is enabled. In that case, it adds crate:: to the start of the suggestion.

This is step 1. Step 2 in a second. =)