From 7e6c96a31977a85c0c03f7e52afdf3409d1a0e8f Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 18 Jan 2023 14:45:12 -0800 Subject: [PATCH 1/3] Add semver rule for lints --- src/doc/semver-check/src/main.rs | 13 ++++++-- src/doc/src/reference/semver.md | 56 ++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/src/doc/semver-check/src/main.rs b/src/doc/semver-check/src/main.rs index 9b7e5d13c08..51aacbe1105 100644 --- a/src/doc/semver-check/src/main.rs +++ b/src/doc/semver-check/src/main.rs @@ -31,11 +31,15 @@ fn doit() -> Result<(), Box> { 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(()), @@ -73,7 +77,10 @@ fn doit() -> Result<(), Box> { } 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'); diff --git a/src/doc/src/reference/semver.md b/src/doc/src/reference/semver.md index 37dccf68aba..c3fdec61cac 100644 --- a/src/doc/src/reference/semver.md +++ b/src/doc/src/reference/semver.md @@ -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) @@ -1164,6 +1165,61 @@ Mitigation strategies: * Document the platforms and environments you specifically support. * Test your code on a wide range of environments in CI. + +### 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))]`. + Set up your automated CI to check your crate with the feature enabled, possibly as an allowed failure with a notification. +* 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. + + +[`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 From 21d913401a34ead6cb6983b1fa5926a1c698ae40 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 18 Jan 2023 19:59:20 -0800 Subject: [PATCH 2/3] Add more to lint semver mitigations. --- src/doc/src/reference/semver.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/doc/src/reference/semver.md b/src/doc/src/reference/semver.md index c3fdec61cac..72be6557daa 100644 --- a/src/doc/src/reference/semver.md +++ b/src/doc/src/reference/semver.md @@ -1210,10 +1210,13 @@ Mitigating strategies: * 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))]`. 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. + * 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 From 408b8d5f5101f35d92aa332dfae92e0a44ce583d Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Thu, 19 Jan 2023 08:35:04 -0800 Subject: [PATCH 3/3] Remove lint mitigation suggesting to use a cargo feature. Since it is not possible to have "private" features, it can be an issue if users enable it without understanding what it is for. --- src/doc/src/reference/semver.md | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/doc/src/reference/semver.md b/src/doc/src/reference/semver.md index 72be6557daa..36482ff6e65 100644 --- a/src/doc/src/reference/semver.md +++ b/src/doc/src/reference/semver.md @@ -1206,12 +1206,8 @@ 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))]`. - 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. - * If using RUSTFLAGS to pass `-Dwarnings`, also add the `-A` flag to allow lints that are likely to cause issues, such as `-Adeprecated`. +* 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.