You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Detect cases where a package's Cargo.toml implicitly or explicitly defines multiple features whose names may be confused for each other. For example, feature names differing only in a single - vs _ character, like mock-instant vs mock_instant. This is likely unintentional, and bad practice even if intentional, as the confusion will likely lead to bugs.
Advantage
This is motivated by a real-world example accidentally discovered by a cargo-semver-checks user, who:
committed code that removes one of the features in the confusable pair
had cargo-semver-checks discover and report the feature removal as a breaking change
didn't notice the - vs _ difference in feature names (it's not at all obvious! it took me a while too)
was shocked to find out that their crate had been shipping two features when they had only intended to have one of them
While cargo-semver-checks prevented the accidental breaking change, the user is now stuck with both features in their crate until the next major version bump. Meanwhile, downstream users might be enabling one feature or the other, so some portion of them will need to update their Cargo.toml feature lists for the next major release when one of the two features gets removed. In addition, I believe the crate has a bug where a #[cfg] condition checks for only one of the two confusable features instead of both, causing some functionality to diverge from users' expectations when enabling one of the two confusable features.
This lint would have caught the confusable features being added on day 1 and prevented the whole situation.
Drawbacks
I understand that cargo manifest linting may have some performance concerns in clippy, due to the cost of reading cargo metadata output.
On the other hand, this would be a great candidate for a cargo lint, if cargo were to gain its own linting system.
Here we should recommend that the user consider renaming one of the features to avoid confusion, or merge them if they are intended to refer to the same thing.
The text was updated successfully, but these errors were encountered:
What it does
Detect cases where a package's
Cargo.toml
implicitly or explicitly defines multiple features whose names may be confused for each other. For example, feature names differing only in a single-
vs_
character, likemock-instant
vsmock_instant
. This is likely unintentional, and bad practice even if intentional, as the confusion will likely lead to bugs.Advantage
This is motivated by a real-world example accidentally discovered by a
cargo-semver-checks
user, who:cargo-semver-checks
discover and report the feature removal as a breaking change-
vs_
difference in feature names (it's not at all obvious! it took me a while too)cargo-semver-checks
reporting the "false positive": False-positive for removed feature fromCargo.toml
obi1kenobi/cargo-semver-checks#1070While
cargo-semver-checks
prevented the accidental breaking change, the user is now stuck with both features in their crate until the next major version bump. Meanwhile, downstream users might be enabling one feature or the other, so some portion of them will need to update theirCargo.toml
feature lists for the next major release when one of the two features gets removed. In addition, I believe the crate has a bug where a#[cfg]
condition checks for only one of the two confusable features instead of both, causing some functionality to diverge from users' expectations when enabling one of the two confusable features.This lint would have caught the confusable features being added on day 1 and prevented the whole situation.
Drawbacks
I understand that cargo manifest linting may have some performance concerns in clippy, due to the cost of reading
cargo metadata
output.On the other hand, this would be a great candidate for a cargo lint, if cargo were to gain its own linting system.
Example
Real-world example: firezone/boringtun@9ff747e
Could be written as:
Note that the
dep:
syntax disables the implicitmock_instant
feature.In the more general case, there might not be a machine-applicable fix but the lint should still fire. For example:
Here we should recommend that the user consider renaming one of the features to avoid confusion, or merge them if they are intended to refer to the same thing.
The text was updated successfully, but these errors were encountered: