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

Type aliases of types defined outside the current crate are not checked #473

Open
jrmuizel opened this issue Jun 19, 2023 · 6 comments
Open
Labels
A-lint Area: new or existing lint C-enhancement Category: raise the bar on expectations

Comments

@jrmuizel
Copy link
Sponsor

Steps to reproduce the bug with the above code

Run cargo semver-checks check-release in the core-foundation-sys directory of https://github.com/servo/core-foundation-rs @ eb00adae0c6d3694dccc54b195132c2929ac8c96

Actual Behaviour

No changes detected

Expected Behaviour

semver-checks fails to detect that UniChar has moved from string:: to base::

Generated System Information

Software version

cargo-semver-checks 0.21.0

Operating system

Mac OS X 10.15.6 (Darwin 19.6.0)

Command-line

/Users/jrmuizel/.cargo/bin/cargo-semver-checks semver-checks --bugreport

cargo version

> cargo -V
cargo 1.70.0 (ec8a8a0ca 2023-04-25)

Compile time information

  • Profile: release
  • Target triple: x86_64-apple-darwin
  • Family: unix
  • OS: macos
  • Architecture: x86_64
  • Pointer width: 64
  • Endian: little
  • CPU features: fxsr,sse,sse2,sse3,ssse3
  • Host: x86_64-apple-darwin

Build Configuration

No response

Additional Context

No response

@jrmuizel jrmuizel added the C-bug Category: doesn't meet expectations label Jun 19, 2023
@jrmuizel
Copy link
Sponsor Author

Actually, semver-checks will detect

  --- failure function_missing: pub fn removed or renamed ---

Description:
A publicly-visible function cannot be imported by its prior path. A `pub use` may have been removed, or the function itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-check/tree/v0.21.0/src/lints/function_missing.ron

Failed in:
  function core_foundation_sys::url::CFURLStopAccessingSecurityScopedResource, previously in file /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/core-foundation-sys-0.8.4/src/url.rs:162
  function core_foundation_sys::url::CFURLStartAccessingSecurityScopedResource, previously in file /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/core-foundation-sys-0.8.4/src/url.rs:161

but not the moved type mentioned above.

@jrmuizel
Copy link
Sponsor Author

If you use cf25b7d943624947ff027b38f75ce0a451b01eb0 instead it now shows no changes detected.

@obi1kenobi obi1kenobi changed the title semver-checks fails to detect semver change in core-foundation-sys Type aliases of types defined outside the current crate are not checked Jun 19, 2023
@obi1kenobi
Copy link
Owner

Thanks for the report.

This is a known issue, caused in part by the same limitation that also causes #355: cargo-semver-checks today cannot see or analyze types defined outside the crate being scanned. UniChar is defined as:

use std::os::raw::{c_uint, c_void, c_int, c_short, c_uchar, c_ushort};
// ...
pub type UniChar = c_ushort;

and c_ushort is an external type.

The limitation is due to the fact that external types' definitions are not part of the rustdoc JSON file of the scanned crate. There is currently no good way to connect data in multiple rustdoc JSON files together.

The rustdoc team is aware of this limitation and is interested in resolving it; I've stayed in touch with them on this.

Sorry I don't have a better answer!

@jrmuizel
Copy link
Sponsor Author

Is there an issue tracking the rustdoc problem?

@obi1kenobi
Copy link
Owner

This is the closest thing to a single tracking issue I could find: rust-lang/compiler-team#635

That's a prerequisite rustc change after which rustdoc JSON will probably tweak the format in some way that resolves the problems in this link, which is also mentioned in the issue.

@obi1kenobi obi1kenobi added A-lint Area: new or existing lint C-enhancement Category: raise the bar on expectations and removed C-bug Category: doesn't meet expectations labels Jun 29, 2023
@jrmuizel
Copy link
Sponsor Author

We ended up getting burnt with this with the release of core-foundation-sys 0.8.5. servo/core-foundation-rs#619

a pub use CFIndex was remove in servo/core-foundation-rs@6d00383 which broke semver and was not detected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: new or existing lint C-enhancement Category: raise the bar on expectations
Projects
None yet
Development

No branches or pull requests

2 participants