Lint againt non semver-open dependencies

[matklad]

EDIT: the issue now has mentoring instructions. Note that to implement this, we'll need to add additional API to semver crate as well!

Cargo more-or-less enforces contract on crate authors that version 1.x+n.u can always be used instead of 1.x.v. That means that any semver requirement, except for ^x.y.z, does not make sense. Moreover, exotic version requirements may version resolution significantly more complicated.

So we should lint against any sevmer requirement other than x, x.y or x.y.z.

Specifically,

• ^x.y.z is not necessary, because ^ is default
• ~ violates versioning contract. As a consequence Cargo will generally refuse to compile a project with two tilde dependencies with same major version.
• >, >= do not make sense, because major version upgrade, by contract, can break anything.
• = is in the wrong place, lockfiles should be used to pin dependencies.
• <, <= again, version pining belongs to the lockfile.
• * we already forbid. It actually makes sense for suerp-quick local development though!

The interesting not clear-cut use-case is version striding (compatibility with several major versions). For example, serde = ">= 2, < 4" means that you are source-compatible with both serde 2 and serde 3. However, looks like the proper solution here is for the upstream crate to do dtolnay's trick? That is, serde 2 should depend on serde 3, and reexport it's APIs.

There are several design decisions on how to actually implement it:

• We can warn on cargo publish enforcing this for crate.io crates, or we can warn on parsing Cargo.toml, enforcing this for anyone.

• We can add an opt-out mechanism. For example, we might make you to write a reason if you use non-standard requirement: foo = { version = "~9.2", reason = "this crate has weird rustc compatibility policy" }.

• Finally, the most radical option, what if we flatly deprecate all forms of version specification, except for x, x.y, x.y.z? That way, we won't need to document and explain various forms of version requirements at all.

I personally am leaning towards the last option, because it makes the overall system radically simpler for the user, but I am curious if there are some real use-cases for exotic version requirements?

[matklad]

@rust-lang/cargo: this is the issue for the idea we've discussed at all hands and on the last meeting.

[alexcrichton]

Thanks for opening an issue for this @matklad! I think the general idea here is a good one to basically raise awareness about the dangers of overly restrictive version requirements, but I do think we need to tread carefully as well. These sorts of version requirements are often useful in a pinch in some situations and can act as a form of relief rather than constantly feeling like Cargo is slapping you on the wrist.

I think this came up as well when publishing to crates.io awhile back as well. We considered rejecting any publishes with a version requirement that has an overly restrictive upper bound (aka in the middle of a semver range) and ended up only disallowing * dependencies.

I'd also want to make sure that the warning story here is something more sophisticated than "warn every time we see these version requirements", although I'm not sure how we'd best do that...

[withoutboats]

I think the most important time to enforce this is on publish, right? You don't want your libraries with overly restrictive requirements causing you problems, but you might want to use = in your end project for several good reasons, as Alex notes.

We could, as sort of the most moderate thing, not upload something with non-open requirements unless you pass an additional flag (similar to --allow-dirty). That way we are "linting" you when you upload, but not totally stopping you the way we do with *.

[matklad]

Agree with @withoutboats that that would a nice conservative first step, which actually will get us 90% of the most aggressive solution anyway.

We've also talked several times before about various publish-time checks (like filling fields in Cargo.toml, having basic docs, etc), so perhaps it's time to introduce some general CLI for such checks? Like

$ cargo publish
   ...
   ...
warning: license, repository and documentation fields of Cargo.toml are not filled
warning: `clap = "~2.10"` is an overly restrictive dependency, consider `clap = "2.10"` instead

return with `--suppress-preflight-checks` to proceed with publication

If that sounds great, I'll be happy to write some mentoring instructions!

[alexcrichton]

At least starting out with warnings on publish sounds great to me!

[dwijnand]

it would probably be useful to be able to run these checks independently of publishing, such as in CI. cargo preflight-checks (or doctor/audit etc).

[withoutboats]

@dwijnand If I'm not mistaken this exists as cargo publish --dry-run

[dwijnand]

oh yeah.. easy :) 👍

[matklad]

Mentoring instructions:

We already implement similar functionality for warning about missing Cargo.toml fields, this happens here:
https://github.com/rust-lang/cargo/blob/master/src/cargo/ops/cargo_package.rs#L39-L41

To add checks for versions, the steps are:

1. Add --non-standard-requirements flag here, to be able to suppress the warning (name is subject to bike-shedding).
2. Actually check for versions here. To get the list of declared dependencies, use pkg.manifest().dependencies(). The version_req field of a dependency is a semver::VersionReq which needs to be checked. Looks like the semver crate does not expose API to inspect the precise form of version requirement, so first such an API needs to be implemented. Perhaps just a VersionReq::is_semver_open method should be added? Semver crate source is here
3. Add a test. A similar one can be found here

[csmoe]

I will take this.