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

Clarify how to choose a FutureIncompatibilityReason variant. #117927

Merged
merged 1 commit into from
Dec 12, 2023
Merged
Changes from all 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
67 changes: 67 additions & 0 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,30 +345,97 @@ 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 <https://rustc-dev-guide.rust-lang.org/bug-fix-procedure.html> and
/// <https://rustc-dev-guide.rust-lang.org/diagnostics.html#future-incompatible-lints>
/// 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
///
/// 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggests a crater run with the lint made a hard error before bumping the future compat level, if I understand correctly. Sadly this is not easy -- making a lint into a hard error for a crater run is non-trivial. One has to call completely different functions to emit the lint, and now with translatable diagnostics these functions even take things of completely different arguments.

///
/// 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),
}

Expand Down
Loading