False negative: Removing an empty module should be a breaking change

[nmathewson]

Steps to reproduce the bug with the above code

Check out the demo repository at https://github.com/nmathewson/cargo-semver-checks-demo-1 .

Note that in main, lib.rs is empty.

Note that in v0.1.0, lib.rs contains an empty public module.

Run cargo semver-checks --baseline-rev v0.1.0.

Actual Behaviour

The output is:

[1115]$ cargo semver-checks --baseline-rev v0.1.0
     Cloning v0.1.0
     Parsing csc-demo-1 v0.1.1 (current)
     Parsing csc-demo-1 v0.1.0 (baseline)
    Checking csc-demo-1 v0.1.0 -> v0.1.1 (minor change)
   Completed [   0.004s] 39 checks; 39 passed, 6 unnecessary

Expected Behaviour

There should be an error, since any code that tried to import the empty module from an external crate will be broken by a removal of the empty module.

I think this is plausible in realistic scenarios. For example, you could make a crate with an (empty) prelude, and tell people that they are expected to say use mycrate::prelude::*, even if you don't yet have anything in the prelude.

Generated System Information

cargo-semver-checks 0.22.1

#### Operating system

Linux 6.3.8-200.fc38.x86_64

#### Command-line

```bash
/home/nickm/.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-unknown-linux-gnu
• Family: unix
• OS: linux
• Architecture: x86_64
• Pointer width: 64
• Endian: little
• CPU features: fxsr,sse,sse2
• Host: x86_64-unknown-linux-gnu

### Build Configuration

_No response_

### Additional Context

I'm happy to work on this if you can give me some pointers of what to look at and where to start.

[obi1kenobi]

Thanks for offering to help! Adding a lint for this is not difficult but it does require a bit of context on a few different moving parts, since it requires changes in both schema and lints.

To be honest, the schema change here isn't the easiest to pull off — it isn't difficult but it does require a bunch of related changes in different places, and I don't trust our onboarding docs enough to feel like I'd be setting you up for success rather than frustration. If you'd rather just add the lint, I'm happy to take care of the schema editing (though I'm also happy to guide you through it, if you feel up for a challenge!) I'm also happy to find you an easier case where you can make a schema extension and write some new lints with it where there wouldn't be as many moving pieces to learn about on day 1.

Make sure to read the CONTRIBUTING guide, especially the section on adding new lints: https://github.com/obi1kenobi/cargo-semver-checks/blob/main/CONTRIBUTING.md

It would also be helpful to read through this prior issue, where another new contributor exposed traits' supertraits to the linter by similarly adding new schema and then using it in a new lint: #232

Here's what needs to be done:

• add type Module to the trustfall-rustdoc-adapter crate's schema — it would be somewhat analogous to type Struct here (implements Item & Importable etc.)
• implement the new type in the adapter — might be good to read a bit about Trustfall and see its example adapters; analogous prior PR for a different lint: Add supertrait to schema trustfall-rustdoc-adapter#188
• Use the make_new_lint.sh script to make stubs for a new lint and new test crate
• Make sure the test crate has both a true-positive semver issue as well as some cases that would catch false-positives (e.g. if the removed module was never pub)
• Write the new lint — here's an analogous lint for "pub struct was removed": https://github.com/obi1kenobi/cargo-semver-checks/blob/main/src/lints/struct_missing.ron

Again, I'm happy to do the first two points here so you can focus on the last three, and once you've gotten a chance to "warm up" here I can suggest another issue where you can try out making edits to the schema and writing more lints.

We also have a playground where you can write queries against rustdoc. It uses the same underlying code as cargo-semver-checks and you can use it to get a sense of the query syntax and what everything does: https://play.predr.ag/rustdoc