cargo-semver-checks
cargo-semver-checks copied to clipboard
Check if trait items with defaults may need special handling to avoid false-positives
Oooh interesting, this is a false-positive — we wanted this to not get flagged.
Perhaps because the
N
constant isn't defined in theimpl
block and is instead a default item on the trait? 🤔
- Add a new test case, similar to the trait+struct one we have already, but where the
const
doesn't have a default value and is instead given a value in theimpl
block for the trait.- Add comments on both the old and new test cases, to explain what they test, how they are different, and why it's important to have both of them and not just one or the other.
- Depending on how this new test case interacts with the query (whether it gets flagged or not), we'd continue tweaking the query -- for example, by checking two separate cases explicitly: "inherent impl must not have the constant" and "impl of a trait, where that trait must not have a const by that name either."
Originally posted by @obi1kenobi in https://github.com/obi1kenobi/cargo-semver-checks/pull/714#discussion_r1545470986
It's plausible that other lints that look for inherent items, such as inherent_method_missing
, may also have the same subtle logic error that fails to prevent a false positive in a "move the inherent item to a defaulted item on an impl'd trait" scenario.
More test cases are needed, and I expect multiple lints will need updates.