Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False-positives due to renaming re-exports + name collisions in the same crate #202

Closed
obi1kenobi opened this issue Dec 2, 2022 · 2 comments · Fixed by #330
Closed

False-positives due to renaming re-exports + name collisions in the same crate #202

obi1kenobi opened this issue Dec 2, 2022 · 2 comments · Fixed by #330
Labels
C-bug Category: doesn't meet expectations C-enhancement Category: raise the bar on expectations

Comments

@obi1kenobi
Copy link
Owner

obi1kenobi commented Dec 2, 2022

Our cargo semver-checks CI steps currently fail for libp2p-dcutr and libp2p-relay, even though neither of them merged any changes since the last libp2p v0.50.0 release.

Originally posted by @mxinden in libp2p/rust-libp2p#2647 (comment)

I was able to replicate the issue locally. The generated rustdoc for libp2p-dcutr at first glance doesn't seem to have anything out of place. I spot-checked one of the enums and variants that reported an error, and they were present in both the baseline and current rustdoc with identical IDs and reasonable-looking data.

This issue needs further triage.

@obi1kenobi
Copy link
Owner Author

Unfortunately, I found that this is because of a combination of renaming re-exports causing confusion between multiple enums with the same name. For example, a pub enum UpgradeError exists in all of:

protocols/dcutr/src/protocol/outbound.rs
protocols/dcutr/src/protocol/inbound.rs
protocols/dcutr/src/behavior.rs

The false-positives happen because the lint tries to match up all the variants across different enums, which obviously isn't going to do the right thing.

cargo-semver-checks doesn't currently handle re-export renames (and re-exports are already tricky because of the possibility of having infinitely many re-exported names for each item: #152 ), so this isn't a trivial fix.

That said, it's still important to fix and this should be sufficient motivation to come up with a permanent solution to the re-exports problem :)

@obi1kenobi obi1kenobi changed the title False-positives for libp2p-dcutr and libp2p-relay when the crates haven't changed False-positives due to renaming re-exports + name collisions in the same crate Dec 7, 2022
@obi1kenobi obi1kenobi added C-bug Category: doesn't meet expectations C-enhancement Category: raise the bar on expectations labels Dec 7, 2022
@obi1kenobi
Copy link
Owner Author

It's tempting to use the canonical_path edge instead of importable_path edge in the affected lints. Right now, this wouldn't work, since re-exported items do not currently have a canonical_path available, since that path uses the rustdoc index information which currently does not include re-exported items.

Relevant rustdoc issues:
rust-lang/rust#93522
rust-lang/rust#99513

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: doesn't meet expectations C-enhancement Category: raise the bar on expectations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant