Skip to content

Commit

Permalink
Auto merge of #11596 - ehuss:semver-lints, r=epage
Browse files Browse the repository at this point in the history
Add semver rule for lints

This adds a new rule to the semver compatibility list explaining the expectations around diagnostic lints.

cc #8736
  • Loading branch information
bors committed Jan 19, 2023
2 parents 50eb688 + 408b8d5 commit d73b935
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 3 deletions.
13 changes: 10 additions & 3 deletions src/doc/semver-check/src/main.rs
Expand Up @@ -31,11 +31,15 @@ fn doit() -> Result<(), Box<dyn Error>> {

loop {
// Find a rust block.
let (block_start, run_program) = loop {
let (block_start, run_program, deny_warnings) = loop {
match lines.next() {
Some((lineno, line)) => {
if line.trim().starts_with("```rust") && !line.contains("skip") {
break (lineno + 1, line.contains("run-fail"));
break (
lineno + 1,
line.contains("run-fail"),
!line.contains("dont-deny"),
);
}
}
None => return Ok(()),
Expand Down Expand Up @@ -73,7 +77,10 @@ fn doit() -> Result<(), Box<dyn Error>> {
}
let join = |part: &[&str]| {
let mut result = String::new();
result.push_str("#![allow(unused)]\n#![deny(warnings)]\n");
result.push_str("#![allow(unused)]\n");
if deny_warnings {
result.push_str("#![deny(warnings)]\n");
}
result.push_str(&part.join("\n"));
if !result.ends_with('\n') {
result.push('\n');
Expand Down
55 changes: 55 additions & 0 deletions src/doc/src/reference/semver.md
Expand Up @@ -93,6 +93,7 @@ considered incompatible.
* Tooling and environment compatibility
* [Possibly-breaking: changing the minimum version of Rust required](#env-new-rust)
* [Possibly-breaking: changing the platform and environment requirements](#env-change-requirements)
* [Minor: introducing new lints](#new-lints)
* Cargo
* [Minor: adding a new Cargo feature](#cargo-feature-add)
* [Major: removing a Cargo feature](#cargo-feature-remove)
Expand Down Expand Up @@ -1164,6 +1165,60 @@ Mitigation strategies:
* Document the platforms and environments you specifically support.
* Test your code on a wide range of environments in CI.

<a id="new-lints"></a>
### Minor: introducing new lints

Some changes to a library may cause new lints to be triggered in users of that library.
This should generally be considered a compatible change.

```rust,ignore,dont-deny
// MINOR CHANGE
///////////////////////////////////////////////////////////
// Before
pub fn foo() {}
///////////////////////////////////////////////////////////
// After
#[deprecated]
pub fn foo() {}
///////////////////////////////////////////////////////////
// Example use of the library that will safely work.
fn main() {
updated_crate::foo(); // Warning: use of deprecated function
}
```

Beware that it may be possible for this to technically cause a project to fail if they have explicitly denied the warning, and the updated crate is a direct dependency.
Denying warnings should be done with care and the understanding that new lints may be introduced over time.
However, library authors should be cautious about introducing new warnings and may want to consider the potential impact on their users.

The following lints are examples of those that may be introduced when updating a dependency:

* [`deprecated`][deprecated-lint] — Introduced when a dependency adds the [`#[deprecated]` attribute][deprecated] to an item you are using.
* [`unused_must_use`] — Introduced when a dependency adds the [`#[must_use]` attribute][must-use-attr] to an item where you are not consuming the result.
* [`unused_unsafe`] — Introduced when a dependency *removes* the `unsafe` qualifier from a function, and that is the only unsafe function called in an unsafe block.

Additionally, updating `rustc` to a new version may introduce new lints.

Transitive dependencies which introduce new lints should not usually cause a failure because Cargo uses [`--cap-lints`](../../rustc/lints/levels.html#capping-lints) to suppress all lints in dependencies.

Mitigating strategies:
* If you build with warnings denied, understand you may need to deal with resolving new warnings whenever you update your dependencies.
If using RUSTFLAGS to pass `-Dwarnings`, also add the `-A` flag to allow lints that are likely to cause issues, such as `-Adeprecated`.
* Introduce deprecations behind a [feature][Cargo features].
For example `#[cfg_attr(feature = "deprecated", deprecated="use bar instead")]`.
Then, when you plan to remove an item in a future SemVer breaking change, you can communicate with your users that they should enable the `deprecated` feature *before* updating to remove the use of the deprecated items.
This allows users to choose when to respond to deprecations without needing to immediately respond to them.
A downside is that it can be difficult to communicate to users that they need to take these manual steps to prepare for a major update.

[`unused_must_use`]: ../../rustc/lints/listing/warn-by-default.html#unused-must-use
[deprecated-lint]: ../../rustc/lints/listing/warn-by-default.html#deprecated
[must-use-attr]: ../../reference/attributes/diagnostics.html#the-must_use-attribute
[`unused_unsafe`]: ../../rustc/lints/listing/warn-by-default.html#unused-unsafe

### Cargo

<a id="cargo-feature-add"></a>
Expand Down

0 comments on commit d73b935

Please sign in to comment.