lib usage: manifest errors with manifest path & source position

[alexheretic]

I've been looking at generating manifest diagnostics from cargo errors in rls (rust-lang/rls#1079). It's not easy to deduce which workspace manifests are affected by the error, or where exactly in the manifest relates.

Would it be possible to include somewhere in the causal chain:

• a manifest path
• a source position/range

The latter actually already exists for toml deserialise errors with toml::de::Error::line_col, though not the former.

For example at the moment in rls the best I can do, without more fragile inference, is assume the root manifest is good enough and highlight all of it.

Here I'd prefer to be highlighting the version-check = "0.1" line.

[ehuss]

It sounds like it would be quite difficult since the TOML information is discarded immediately after parsing. What kind of API are you interested in? Do you want Error objects with extra fields returned from compile_with_exec?

[alexheretic]

I can see source info is lost in deserialization, though the manifest path would still be very useful.

Maybe a method in CargoError

pub fn manifest_path(&self) -> Option<Path>

This means we can highlight a specific workspace manifest rather than the root, or guessing from the message. For deserialise errors this actually gives me everything I need, as I have line_col.

[alexheretic]

Ok I see now CargoError is just a failure::Error. But some way of getting the manifest path from compile_with_exec errors is what I'm after.

[alexcrichton]

The toml crate awhile back got support for Spanned<T> which means we now have the capability to preserve span information beyond deserialization (yay!)

I think the fix here would probably be to annotate some TOML fields that we deserialize with Spanned<T> and then if an error happens when processing that field we've got the span information to resolve to a file/line number

[alexheretic]

We now have a decent way to pick the workspace member manifest with ManifestErrors & ResolveErrors, this covers a decent bunch of cases.

Though outside of toml::de::Errors, I'm still highlighting the whole manifest.

In general, I think it would be better for cargo to have errors with info via methods and have this info built into an error string as late as possible. This would give lib users much more to work with. But as cargo is mostly a cli project I can see why we are where we are.

Enriching the new errors with info and adding new ones if necessary is probably the way forward.

[alexcrichton]

That sounds like a good strategy to me! I'm all for improving Cargo's library experience as well :)

[epage]

With RLS dead, is anyone still requesting this or should we close this?