E0433 needs to be updated to new format

[sophiajt]

From: src/test/compile-fail/derived-errors/issue-31997-1.rs

Error E0433 needs a span_label, updating it from:

error[E0433]: failed to resolve. Use of undeclared type or module `HashMap`
  --> src/test/compile-fail/derived-errors/issue-31997-1.rs:30:19
   |
30 |     let mut map = HashMap::new();
   |                   ^^^^^^^^^^^^

To:

error[E0433]: failed to resolve. Use of undeclared type or module `HashMap`
  --> src/test/compile-fail/derived-errors/issue-31997-1.rs:30:19
   |
30 |     let mut map = HashMap::new();
   |                   ^^^^^^^^^^^^ undeclared type or module `HashMap`

Bonus, focus the span on just the part that is causing the issue:

error[E0433]: failed to resolve. Use of undeclared type or module `HashMap`
  --> src/test/compile-fail/derived-errors/issue-31997-1.rs:30:19
   |
30 |     let mut map = HashMap::new();
   |                   ^^^^^^^ undeclared type or module

[hank-der-hafenarbeiter]

Not so sure about how the bonus part can be implemented.

Here:

         ResolutionError::FailedToResolve(msg) => {
            struct_span_err!(resolver.session, span, E0433, "failed to resolve. {}", msg)
            let mut err = struct_span_err!(resolver.session, span, E0433, "failed to resolve. {}", msg);
            err.span_label(span, &msg);
            err
         }

The type/module name (if you don't want to parse it from msg) is not available and the span is the wrong size.

A solution would be to make fn resolve_module_path_from_root(...) and fn resolve_module_path(...) hand you a message and the name of the module seperately. So:

pub type ErrorMessage = Option<(Span, String)>;

would become

pub type ErrorMessage = Option<(Span, String, String)>;

Problem now is this part of fn resolve_module_path_from_root(...) :

let msg = if "???" == &module_name {
    match search_parent_externals(name, &self.current_module) {
        Some(module) => {
            let path_str = names_to_string(module_path);
            let target_mod_str = module_to_string(&module);
            let current_mod_str = module_to_string(&self.current_module);
            let prefix = if target_mod_str == current_mod_str {
                "self::".to_string()
            } else {
                format!("{}::", target_mod_str)
            };
            format!("Did you mean `{}{}`?", prefix, path_str)
        }
        None => format!("Maybe a missing `extern crate {}`?", segment_name),
    }
} else {
    format!("Could not find `{}` in `{}`", segment_name, module_name)
};

here the finished message is not a simple concatenation of "unresolved [...]" + <name of the module/type>

Any tips?

[sophiajt]

Yeah, this one looks like it might take some significant changes. When I wrote the bonuses, I wasn't sure which ones would be doable with the information local to the error.

Pinging @arielb1 and @nikomatsakis, who know this area better than I do and who might be able say for sure if this is doable.

[nikomatsakis]

Actually @jseyfried knows this area of the code best -- it does seem like you'd have to tweak things a bit to get the tighter span.

[hank-der-hafenarbeiter]

I've been looking around src/librustc_resolve/lib.rs some more:

The span is generated in fn resolve_path(..., path:&Path, ...):

let span = path.span; 
let segments = &path.segments[..path.segments.len() - path_depth]; 
[...]
let qualified_binding = self.resolve_module_relative_path(span, segments, namespace); 

Since we want to point to the segment of the path that couldn't be resolved, we could go into src/libsyntax/ast.rs and give struct PathSegment a Span-member:

pub struct PathSegment {
    /// The identifier portion of this path segment.
    pub identifier: Ident,
    pub span:Span,

    /// Type/lifetime parameters attached to this path. They come in
    /// two flavors: `Path<A,B,C>` and `Path(A,B) -> C`. Note that
    /// this is more than just simple syntactic sugar; the use of
    /// parens affects the region binding rules, so we preserve the
    /// distinction.
    pub parameters: PathParameters,
}

So when a segment of the path generates an error you can get the span of that segment and hand it back up the stack in the error type (which carries the span of the whole path at the moment) where resolve_error(...) is called.

So basically the span in

pub type ErrorMessage = Option<(Span, String)>;

Would be the span belonging to the PathSegment which threw the resolver off not the span of the whole path.

Now we only need two different strings in there (one for the error message and one for the label). and we're done.

Is this something that could work?

[jseyfried]

@hank-der-hafenarbeiter Yeah, I believe that would work.

The only downside I can think of is the increase in memory usage from the extra Spans -- we might want to measure to see if it's worth optimizing for the common case of single segment paths.

[hank-der-hafenarbeiter]

@jseyfried the span of a path is the combined path of it's path segments, aren't they? So if the span for the whole path is needed we can get that from combining the segment's spans. I don't know enough about how spans are generated and how much memory this would cost but considering that most of the time a span to the segment will be more helpful anyway this might very well be worth it.

If I can assist in any way I'd love to. Just let me know!

[sophiajt]

Closing this issue, but feel free to open a new issue for the bonus