Improve `cargo add -F crate/invalid_feat` error discovery

[Emilgardis]

Problem

When using cargo add -F crate/feat, if the feat feature does not exist, the error is not presented until the end.

This can be easy to miss, as most cargo warnings are presented with more context/text around them. cargo add however spits out a features table as if everything is ok and then presents an error.

Proposed Solution

Attach more context in the output to clearly signal that "hey, this is not correct, check the error message"

Notes

this has been implemented in #11098, creating this per request.

[epage]

This can be easy to miss, as most cargo warnings are presented with more context/text around them.

I feel like this is the key to the problem that we should focus on.

cargo add however spits out a features table as if everything is ok and then presents an error.

Good call, we should probably not show the table like this. We should probably split up print_msg to separate out the "Adding" from the table and put the table in after everything has been processed for that dependency.

The error should probably follow our usual pattern of providing a suggestion if one exists and showing everything if one doesn't exist.

[Emilgardis]

One nice thing with providing the table, even on error, is that you can quickly see if you accidentally typoed or misremembered the feature flags name.

The error should probably follow our usual pattern of providing a suggestion if one exists

Perhaps for the attached example:

add tokio --features sync,net,rt,rr     

the error could be

error: unrecognized feature `rr`
help: a feature with a similar name exists: `rt`

and showing everything if one doesn't exist.

what does this mean?

[epage]

One nice thing with providing the table, even on error, is that you can quickly see if you accidentally typoed or misremembered the feature flags name.

I think that is also what the alternative I brought up does.

the error could be
...
what does this mean?

This is an example of an existing "not found" error in cargo: https://github.com/rust-lang/cargo/pull/10999/files

[Emilgardis]

so something like this?

crate has one feature existing_feat

$ cargo add -F crate/invalid_feat, crate/invalid_feat2 crate/existing_feat1
Adding crate v0.0.1 to dependencies
error: no feature `invalid_feat`, `invalid_feat2` or `existing_feat1` available for `crate`
Available features:
    existing_feat

I don't see how to do multiple errors from cargo add, is it possible?

[epage]

so something like this?

There are 3 scenarios

• A close match is found, suggest that
• No close matches found but features exist, so list them all
• No features exist

I don't see how to do multiple errors from cargo add, is it possible?

In what way are you wanting to do multiple errors?

[Emilgardis]

In what way are you wanting to do multiple errors?

in the cargo add -F crate/invalid_feat crate/invalid_feat2 crate/existing_feat1 example, we have two features with "no close match", and one with a close match. I think it would make sense to present the close match, but also list all.

$ cargo add -F crate/invalid_feat crate/invalid_feat2 crate/existing_feat1
Adding crate v0.0.1 to dependencies
error: no feature "existing_feat1" exists for "crate", did you mean "existing_feat"
error: features "invalid_feat" and "invalid_feat2" does not exist
    Available features: "existing_feat"

error: unrecognized features ["invalid_feat", "invalid_feat2", "existing_feat1"]

I was thinking of something like https://docs.rs/color-eyre/latest/color_eyre/section/trait.Section.html#tymethod.with_error but already built-in, i.e emitting errors, not just one error.

[epage]

Hmm, I'm thinking we should start with a simpler error message and move on from there. We should just do Available features for now instead of doing any typo suggestions.

[Emilgardis]

something like this? or maybe

[weihanglo]

I like the latter! Looks better and less noisy.

[Emilgardis]

How about this? Showing available (currently not enabled features) and enabled features

[epage]

While the long list is less cluttered, I feel like it undoes the goal of this Issue which is to draw more attention to the error itself. I would prefer the comma-separated list when things are long like this.

[Emilgardis]

maybe

?

max length 84

edit: an alternative

[weihanglo]

…when things are long like this.

You made me recall this auto-generated feature list. 1505 features in a Cargo.toml

https://github.com/rustwasm/wasm-bindgen/blob/564ce74168904e95a7905a828488ec3029bcaad4/crates/web-sys/Cargo.toml#L31-L1535

This may deserve a new issue if we want to address, along with other places with similar issues.

[epage]

@Emilgardis I think I prefer the first as the second is a bit more repetitive

"available features" isn't actually available because its missing enabled features (its legal to specify already enabled features)