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

Add semver rule for lints #11596

Merged
merged 3 commits into from
Jan 19, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/doc/semver-check/src/main.rs
Original file line number Diff line number Diff line change
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
59 changes: 59 additions & 0 deletions src/doc/src/reference/semver.md
Original file line number Diff line number Diff line change
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,64 @@ 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:
* Options for dealing with denying warnings:
* Understand you may need to deal with resolving new warnings whenever you update your dependencies.
* Place `deny(warnings)` behind a [feature][Cargo features], for example `#![cfg_attr(feature = "deny-warnings", deny(warnings))]`.
epage marked this conversation as resolved.
Show resolved Hide resolved
Set up your automated CI to check your crate with the feature enabled, possibly as an allowed failure with a notification.
Beware that features are exposed to users, so you may want to clearly document it as something that is not to be used.
epage marked this conversation as resolved.
Show resolved Hide resolved
* 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.
epage marked this conversation as resolved.
Show resolved Hide resolved
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