From 7a812c131149e2e730c3605c30d6fb1ae376d4bd Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 14 Nov 2023 19:36:24 -0800 Subject: [PATCH] Clarify how to choose a FutureIncompatibilityReason variant. There has been some confusion about how to choose these variants, or what the procedure is for handling future-incompatible errors. Hopefully this helps provide some more information on how these work. --- compiler/rustc_lint_defs/src/lib.rs | 67 +++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index 878c1a65dbf6e..185634848b49c 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -345,12 +345,34 @@ pub struct FutureIncompatibleInfo { } /// The reason for future incompatibility +/// +/// Future-incompatible lints come in roughly two categories: +/// +/// 1. There was a mistake in the compiler (such as a soundness issue), and +/// we're trying to fix it, but it may be a breaking change. +/// 2. A change across an Edition boundary, typically used for the +/// introduction of new language features that can't otherwise be +/// introduced in a backwards-compatible way. +/// +/// See and +/// +/// for more information. #[derive(Copy, Clone, Debug)] pub enum FutureIncompatibilityReason { /// This will be an error in a future release for all editions /// /// This will *not* show up in cargo's future breakage report. /// The warning will hence only be seen in local crates, not in dependencies. + /// + /// Choose this variant when you are first introducing a "future + /// incompatible" warning that is intended to eventually be fixed in the + /// future. This allows crate developers an opportunity to fix the warning + /// before blasting all dependents with a warning they can't fix + /// (dependents have to wait for a new release of the affected crate to be + /// published). + /// + /// After a lint has been in this state for a while, consider graduating + /// it to [`FutureIncompatibilityReason::FutureReleaseErrorReportInDeps`]. FutureReleaseErrorDontReportInDeps, /// This will be an error in a future release, and /// Cargo should create a report even for dependencies @@ -358,17 +380,62 @@ pub enum FutureIncompatibilityReason { /// This is the *only* reason that will make future incompatibility warnings show up in cargo's /// reports. All other future incompatibility warnings are not visible when they occur in a /// dependency. + /// + /// Choose this variant after the lint has been sitting in the + /// [`FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps`] + /// state for a while, and you feel like it is ready to graduate to + /// warning everyone. It is a good signal that it is ready if you can + /// determine that all or most affected crates on crates.io have been + /// updated. + /// + /// After some period of time, lints with this variant can be turned into + /// hard errors (and the lint removed). Preferably when there is some + /// confidence that the number of impacted projects is very small (few + /// should have a broken dependency in their dependency tree). FutureReleaseErrorReportInDeps, /// Code that changes meaning in some way in a /// future release. + /// + /// Choose this variant when the semantics of existing code is changing, + /// (as opposed to + /// [`FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps`], + /// which is for when code is going to be rejected in the future). FutureReleaseSemanticsChange, /// Previously accepted code that will become an /// error in the provided edition + /// + /// Choose this variant for code that you want to start rejecting across + /// an edition boundary. This will automatically include the lint in the + /// `rust-20xx-compatibility` lint group, which is used by `cargo fix + /// --edition` to do migrations. The lint *should* be auto-fixable with + /// [`Applicability::MachineApplicable`]. + /// + /// The lint can either be `Allow` or `Warn` by default. If it is `Allow`, + /// users usually won't see this warning unless they are doing an edition + /// migration manually or there is a problem during the migration (cargo's + /// automatic migrations will force the level to `Warn`). If it is `Warn` + /// by default, users on all editions will see this warning (only do this + /// if you think it is important for everyone to be aware of the change, + /// and to encourage people to update their code on all editions). + /// + /// See also [`FutureIncompatibilityReason::EditionSemanticsChange`] if + /// you have code that is changing semantics across the edition (as + /// opposed to being rejected). EditionError(Edition), /// Code that changes meaning in some way in /// the provided edition + /// + /// This is the same as [`FutureIncompatibilityReason::EditionError`], + /// except for situations where the semantics change across an edition. It + /// slightly changes the text of the diagnostic, but is otherwise the + /// same. EditionSemanticsChange(Edition), /// A custom reason. + /// + /// Choose this variant if the built-in text of the diagnostic of the + /// other variants doesn't match your situation. This is behaviorally + /// equivalent to + /// [`FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps`]. Custom(&'static str), }