Fix rustdoc .json files overwriting each other

[staniewzki]

Fixes #269 and #309.

When target-dir is specified in .cargo/config.toml, the path for generated rustdoc .json file is the same for the baseline and current crate version. When generating the .json files both in advance, and them processing them later, this is obviously a problem, as one of them gets overwritten.

I don't know if the solution I have came up with is the best one, as we might load too much data into memory when processing multiple crates at once. On the other hand, this way we are sure, overwritting is not an issue.

[obi1kenobi]

Nice catch!

Would it be possible to reorganize the code to do this instead:

• for each crate that needs to be checked:
  • generate the current rustdoc, then load it
  • generate the baseline, then load it
  • check that crate
  • move on to the next one

This way, we both don't have to worry about the files colliding or disappearing, and also we give the user feedback sooner on whether their crate passed or failed. I've recently been running cargo-semver-checks on some pretty large workspaces (dozens of crates), and I didn't love that I had to wait for all the crates' rustdocs to be generated (half an hour or more!) before any crate got checked.

[tonowak]

for each crate that needs to be checked: generate the current rustdoc, then load it, generate the baseline, then load it, check that crate, move on to the next one

Done.

[obi1kenobi]

Thanks to both of you for putting this together! Just some stylistic comments to help readability.

[obi1kenobi]

The fix looks good overall. Just a small refactoring idea to look into to make it even better.

Is there a test case we could reasonably add for this? Perhaps an integration-style test on the clap release with the UnwindSafe semver issue, with a custom target dir, requiring that the semver issue be found and reported? (I.e. not just "exit 1" but also a grep in the output for the failing lint and the name of the affected type.)

[tonowak]

The fix looks good overall. Just a small refactoring idea to look into to make it even better.

Is there a test case we could reasonably add for this? Perhaps an integration-style test on the clap release with the UnwindSafe semver issue, with a custom target dir, requiring that the semver issue be found and reported? (I.e. not just "exit 1" but also a grep in the output for the failing lint and the name of the affected type.)

Sounds good. From what I remember, the semver break is exactly between clap's v3.1.18 and v3.2.0.

[tonowak]

Nice, I like those changes -- now it would be hard to make a similar bug.

[obi1kenobi]

This looks great, well done @staniewzki and @tonowak!