suggest crate::... for "local" paths in 2018

[davidtwco]

Fixes #54230.

This commit adds suggestions for unresolved imports in the cases where
there could be a missing crate::, super::, self:: or a missing
external crate name before an import.

r? @nikomatsakis

[nikomatsakis]

Hmm, nice!

[nikomatsakis]

The code looks pretty good to me. I think there are some policy questions, e.g. the way that it prefers self or super to crate, that are not entirely clear to me. But probably not a big deal in practice.

[nikomatsakis]

Or, I guess that the current heuristic is:

• self::foo
• crate::foo
• super::bar
• extern_crate::bar

It feels like it might be reasonable to show more than one, too, though it'd be silly to do so if they are the same thing underneath.

[davidtwco]

The code looks pretty good to me. I think there are some policy questions, e.g. the way that it prefers self or super to crate, that are not entirely clear to me. But probably not a big deal in practice.

Yeah, I've ordered these entirely arbitrarily - we can adjust that. I also experimented with having it attempt to correct misspellings but I couldn't quite tweak it so it wasn't over- or under-correcting. I would get it so if I had tsd::path it would suggest std::path but it would also suggest that for things that were completely wrong; and on the other end of the scale, if I had a long path that had one or two characters swapped, it wouldn't suggest anything.

[nikomatsakis]

I'm feeling a bit confused now because I know there is also this lookup_import_candidates_from_module function, and I'm trying to figure out how/if these ought to relate to one another. =)

[nikomatsakis]

Maybe that code is only invoked if the bar part of foo::bar fails, not the foo part?

[eddyb]

r? @eddyb

[davidtwco]

@nikomatsakis If it is at all helpful, these are the rough notes I threw together when initially working out what was happening and where these errors were coming from:

Searching for the error message, we find it in resolve_path_with_scope:

rust/src/librustc_resolve/lib.rs

Lines 3673 to 3689 in 186fe09

	let msg = if module_def == self.graph_root.def() {
	let is_mod = |def| match def { Def::Mod(..) => true, _ => false };
	let mut candidates =
	self.lookup_import_candidates(name, TypeNS, is_mod);
	candidates.sort_by_cached_key(|c| {
	(c.path.segments.len(), c.path.to_string())
	});
	if let Some(candidate) = candidates.get(0) {
	format!("Did you mean `{}`?", candidate.path)
	} else {
	format!("Maybe a missing `extern crate {};`?", ident)
	}
	} else if i == 0 {
	format!("Use of undeclared type or module `{}`", ident)
	} else {
	format!("Could not find `{}` in `{}`", ident, path[i - 1])
	};

This returns a PathResult::Failed(..) with a span, message and boolean indicating if this is the last error. resolve_path_with_parent_scope is called by resolve_path.

resolved_path is called by resolve_import where the PathResult is matched, returning true:

rust/src/librustc_resolve/resolve_imports.rs

Lines 876 to 880 in 186fe09

	match result {
	PathResult::Module(module) => module,
	PathResult::Indeterminate => return false,
	_ => return true,
	}

resolve_import is called by resolved_imports where the true is matched against to add the import that failed to the determined_imports list:

rust/src/librustc_resolve/resolve_imports.rs

Lines 628 to 631 in 186fe09

	match self.resolve_import(&import) {
	true => self.determined_imports.push(import),
	false => self.indeterminate_imports.push(import),
	}

Later, finalize_imports is called, this iterates over the determined_imports which calls finalize_import:

rust/src/librustc_resolve/resolve_imports.rs

Lines 657 to 659 in 186fe09

	for i in 0 .. self.determined_imports.len() {
	let import = self.determined_imports[i];
	let error = self.finalize_import(import);

finalize_import eventually returns a span and a message, this is where we can see the original self:: suggestion being made:

rust/src/librustc_resolve/resolve_imports.rs

Lines 960 to 973 in 186fe09

	let (mut self_path, mut self_result) = (module_path.clone(), None);
	let is_special = |ident: Ident| ident.is_path_segment_keyword() &&
	ident.name != keywords::CrateRoot.name();
	if !self_path.is_empty() && !is_special(self_path[0]) &&
	!(self_path.len() > 1 && is_special(self_path[1])) {
	self_path[0].name = keywords::SelfValue.name();
	self_result = Some(self.resolve_path(None, &self_path, None, false,
	span, CrateLint::No));
	}
	return if let Some(PathResult::Module(..)) = self_result {
	Some((span, format!("Did you mean `{}`?", names_to_string(&self_path[..]))))
	} else {
	Some((span, msg))
	};

This then goes back to finalize_imports which calls adds the span and message to a vector:

rust/src/librustc_resolve/resolve_imports.rs

Line 728 in 186fe09

error_vec.push((span, path, err));

That vector then is passed to throw_unresolved_import_error:

rust/src/librustc_resolve/resolve_imports.rs

Line 805 in 186fe09

self.throw_unresolved_import_error(error_vec.clone(), None);

throw_unresolved_import_error throws the actual error that we see:

rust/src/librustc_resolve/resolve_imports.rs

Line 821 in 186fe09

fn throw_unresolved_import_error(&self, error_vec: Vec<(Span, String, String)>,

[eddyb]

Hardcoding this is wrong - you want to use something like this instead:

rust/src/librustc_resolve/lib.rs

Lines 4403 to 4407 in c222479

	if self.session.rust_2018() {
	let extern_prelude_names = self.extern_prelude.clone();
	for &name in extern_prelude_names.iter() {
	let ident = Ident::with_empty_ctxt(name);
	match self.crate_loader.maybe_process_path_extern(name, ident.span) {

[davidtwco]

I've updated this.

[eddyb]

I think this PR might need to check the edition? Or somehow involve the compat lints.
That said,
r? @petrochenkov

[davidtwco]

@eddyb I've added a test demonstrating the 2015 behaviour for the same test case.

[petrochenkov]

@bors r+

[bors]

📌 Commit 1525e97 has been approved by petrochenkov

[davidtwco]

@bors r=petrochenkov

[bors]

📌 Commit 5872d3e has been approved by petrochenkov

[bors]

⌛ Testing commit 5872d3e with merge 4bf883b...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: petrochenkov
Pushing 4bf883b to master...