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

Begin resolving cargo-semver-checks blockers for merging into cargo #104

Open
2 of 4 tasks
nikomatsakis opened this issue Jul 22, 2024 · 6 comments
Open
2 of 4 tasks
Assignees
Milestone

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jul 22, 2024

Metadata
Owner(s) @obi1kenobi
Team(s) cargo
Goal document 2024h2/cargo-semver-checks

Summary

Design and implement cargo-semver-checks functionality that lies on the critical path for merging the tool into cargo itself.

Tasks and status

  • Implementation of cargo manifest linting + CLI (@obi1kenobi)
  • Initial design for cross-crate checking (@obi1kenobi)
  • Initial design for type-checking lints (@obi1kenobi)
  • Discussion and moral support (cargo Team)
@nikomatsakis nikomatsakis added this to the 2024h2 milestone Jul 22, 2024
@rust-lang rust-lang locked and limited conversation to collaborators Jul 25, 2024
@nikomatsakis
Copy link
Contributor Author

This issue is intended for status updates only.

For general questions or comments, please contact the owner(s) directly.

@obi1kenobi
Copy link
Member

Status update as of September 1.

Key developments: We've laid a lot of the groundwork to make manifest linting possible.

  • In preparation for the significant expansion of our CLI, we now have the ability to test how CLI invocations get interpreted internally. We can also now snapshot the output of any CLI invocation over a given workspace, allowing us to proceed with refactoring the implementation without breaking existing use cases.
  • We have a concrete design for the expansion of the CLI, and for the Trustfall schema changes that will be necessary to enable manifest linting.

Not blocked on anything. No opportunities to contribute yet, but stay tuned — there will be some in future updates!

@obi1kenobi
Copy link
Member

Status update as of September 17.

Key developments: Working prototype of manifest querying.

WIP branch here: https://github.com/obi1kenobi/trustfall-rustdoc-adapter/compare/manifest_querying

An example SemVer lint this enables would be: "a non-nightly, non-underscore-prefixed feature has been removed from the package between releases."

{
    CrateDiff {
        baseline {
            feature {
                feature: name
                    @filter(op: "!=", value: ["$nightly"])
                    @tag
                    @output
            }
        }

        current {
            feature @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
                name @filter(op: "=", value: ["%feature"])
            }
        }
    }
}

# args:
{
    "nightly": "nightly",
    "zero": 0
}

This is SemVer-major because it'll break uses like: my_dep = { version = "1.0", features = ["removed_feature"] }. We're ignoring any features named nightly because community consensus is that such features are unstable — I'll do some more research on other expected-unstable feature patterns in the community before we ship any lints around this.

Not blocked on anything. No opportunities to contribute yet, but stay tuned — there will be some in future updates!

@obi1kenobi
Copy link
Member

No major updates to report. Preparing my talk for next week's EuroRust has taken away most of my free time.

@obi1kenobi
Copy link
Member

Key developments:

  • Partial schema design and implementation of type information in lints
  • Resolved multi-team questions that were blocking cross-crate checking
  • Refactor of cargo-semver-checks data handling to enable supporting manifest information

Partial schema design and implementation of type information in lints

The schema we use for writing lints now contains information on:

This newly allows the implementation of breaking-change lints such as "generic const parameter no longer has default value" or "trait bound removed from generic parameter in trait method."

Additionally, @suaviloquence has implemented the ability to print rustdoc JSON type information into human-readable source code form, which will allow us to produce higher-quality diagnostics for type-related breaking changes. This required a lot of care, perseverance, and testing — thank you 🙏

Resolved multi-team questions that were blocking cross-crate checking

For a long time, cross-crate checking was blocked on a multi-team coordination problem: cargo-semver-checks needs more data from rustdoc, which in turn needs more data from rustc. This was a classic "everyone's third-most important thing" problem: important, but not critical and therefore never top of the agenda for resolving the open questions there.

In-person meetings with Rust Project members during my trips to RustConf and EuroRust helped nudge this forward:

  • The compiler team MCP intended to provide additional data to rustdoc was merged and can now be implemented.
  • I synced up with T-rustdoc members on how rustdoc would then present that data to cargo-semver-checks in rustdoc JSON. Along the way, we were able to discuss and agree on several additional improvements to rustdoc JSON that improved performance in cargo-semver-checks.

Refactor of cargo-semver-checks data handling

Plumbing in manifest information was proving challenging due to a significant amount of tech debt accrued in the cargo-semver-checks code that loads the data of scanned crates. This PR is a significant clean-up and refactor of that portion of the tool, allowing us to move forward with retrieving manifest information while also supporting other quality-of-life improvements.

With the mountain of tech debt largely resolved, I expect that adding manifest information should be relatively straightforward from here.

This portion of the goal also greatly benefited from the Google Summer of Code work that @suaviloquence did — it wouldn't have been possible without the significant effort he put into improving our test suites and harnesses.

@obi1kenobi
Copy link
Member

Key developments:

Cargo manifest linting

As a recap, the idea here is catching breakage caused by manifest (Cargo.toml) changes, not just source code changes.

An example of such breakage is the removal of a package feature: any crates that enabled the removed feature will no longer build.

The PR above builds upon the data-handling refactor from the previous update post, and makes it possible to lint manifests for breaking changes. The PR includes the feature_missing lint that checks for the exact case mentioned above. That is but one of many new lints that are now possible with this feature. Hooray! 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants