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

Let's #[expect] some lints: Stabilize lint_reasons (RFC 2383) #120924

Merged
merged 8 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from 7 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
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
#![allow(internal_features)]
#![allow(rustc::diagnostic_outside_of_impl)]
#![allow(rustc::untranslatable_diagnostic)]
#![cfg_attr(bootstrap, feature(lint_reasons))]
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
#![doc(rust_logo)]
#![feature(assert_matches)]
#![feature(box_patterns)]
#![feature(decl_macro)]
#![feature(if_let_guard)]
#![feature(let_chains)]
#![feature(lint_reasons)]
#![feature(proc_macro_internals)]
#![feature(proc_macro_quote)]
#![feature(rustdoc_internals)]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_data_structures/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#![allow(internal_features)]
#![allow(rustc::default_hash_types)]
#![allow(rustc::potential_query_instability)]
#![cfg_attr(bootstrap, feature(lint_reasons))]
#![cfg_attr(not(parallel_compiler), feature(cell_leak))]
#![deny(unsafe_op_in_unsafe_fn)]
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
Expand All @@ -24,7 +25,6 @@
#![feature(extend_one)]
#![feature(hash_raw_entry)]
#![feature(hasher_prefixfree_extras)]
#![feature(lint_reasons)]
#![feature(macro_metavar_expr)]
#![feature(map_try_insert)]
#![feature(min_specialization)]
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_errors/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,12 @@ impl Emitter for JsonEmitter {
let data: Vec<FutureBreakageItem<'_>> = diags
.into_iter()
.map(|mut diag| {
if diag.level == crate::Level::Allow {
// The `FutureBreakageItem` is collected and serialized.
// However, the `allow` and `expect` lint levels can't usually
// be serialized. The lint level is overwritten to allow the
// serialization again and force a lint emission.
// (This is an educated guess. I didn't originally add this)
if matches!(diag.level, crate::Level::Allow | crate::Level::Expect(..)) {
Urgau marked this conversation as resolved.
Show resolved Hide resolved
diag.level = crate::Level::Warning;
}
FutureBreakageItem {
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/accepted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,8 @@ declare_features! (
(accepted, label_break_value, "1.65.0", Some(48594)),
/// Allows `let...else` statements.
(accepted, let_else, "1.65.0", Some(87335)),
/// Allows using `reason` in lint attributes and the `#[expect(lint)]` lint check.
(accepted, lint_reasons, "CURRENT_RUSTC_VERSION", Some(54503)),
/// Allows `break {expr}` with a value inside `loop`s.
(accepted, loop_break_value, "1.19.0", Some(37339)),
/// Allows use of `?` as the Kleene "at most one" operator in macros.
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,9 +369,9 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
allow, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#),
DuplicatesOk, EncodeCrossCrate::No,
),
gated!(
expect, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#), DuplicatesOk,
EncodeCrossCrate::No, lint_reasons, experimental!(expect)
ungated!(
expect, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#),
DuplicatesOk, EncodeCrossCrate::No,
),
ungated!(
forbid, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#),
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,8 +512,6 @@ declare_features! (
/// Allows using `#[link(kind = "link-arg", name = "...")]`
/// to pass custom arguments to the linker.
(unstable, link_arg_attribute, "1.76.0", Some(99427)),
/// Allows using `reason` in lint attributes and the `#[expect(lint)]` lint check.
(unstable, lint_reasons, "1.31.0", Some(54503)),
/// Give access to additional metadata about declarative macro meta-variables.
(unstable, macro_metavar_expr, "1.61.0", Some(83527)),
/// Provides a way to concatenate identifiers using metavariable expressions.
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_lint/src/expect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,13 @@ use rustc_middle::query::Providers;
use rustc_middle::ty::TyCtxt;
use rustc_session::lint::builtin::UNFULFILLED_LINT_EXPECTATIONS;
use rustc_session::lint::LintExpectationId;
use rustc_span::symbol::sym;
use rustc_span::Symbol;

pub(crate) fn provide(providers: &mut Providers) {
*providers = Providers { check_expectations, ..*providers };
}

fn check_expectations(tcx: TyCtxt<'_>, tool_filter: Option<Symbol>) {
if !tcx.features().active(sym::lint_reasons) {
return;
}

let lint_expectations = tcx.lint_expectations(());
let fulfilled_expectations = tcx.dcx().steal_fulfilled_expectation_ids();

Expand Down
10 changes: 0 additions & 10 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ use rustc_session::lint::{
},
Level, Lint, LintExpectationId, LintId,
};
use rustc_session::parse::feature_err;
use rustc_session::Session;
use rustc_span::symbol::{sym, Symbol};
use rustc_span::{Span, DUMMY_SP};
Expand Down Expand Up @@ -788,15 +787,6 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
ast::MetaItemKind::NameValue(ref name_value) => {
if item.path == sym::reason {
if let ast::LitKind::Str(rationale, _) = name_value.kind {
if !self.features.lint_reasons {
feature_err(
&self.sess,
sym::lint_reasons,
item.span,
"lint reasons are experimental",
)
.emit();
}
reason = Some(rationale);
} else {
sess.dcx().emit_err(MalformedAttribute {
Expand Down
26 changes: 8 additions & 18 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,13 +608,13 @@ declare_lint! {
}

declare_lint! {
/// The `unfulfilled_lint_expectations` lint detects lint trigger expectations
/// that have not been fulfilled.
/// The `unfulfilled_lint_expectations` lint detects when a lint expectation is
/// unfulfilled.
///
/// ### Example
///
/// ```rust
/// #![feature(lint_reasons)]
/// #![cfg_attr(bootstrap, feature(lint_reasons))]
///
/// #[expect(unused_variables)]
/// let x = 10;
Expand All @@ -625,24 +625,14 @@ declare_lint! {
///
/// ### Explanation
///
/// It was expected that the marked code would emit a lint. This expectation
/// has not been fulfilled.
/// The `#[expect]` attribute can be used to create a lint expectation. The
/// expectation is fulfilled, if a `#[warn]` attribute at the same location
/// would result in a lint emission. If the expectation is unfulfilled,
/// because no lint was emitted, this lint will be emitted on the attribute.
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
///
/// The `expect` attribute can be removed if this is intended behavior otherwise
/// it should be investigated why the expected lint is no longer issued.
///
/// In rare cases, the expectation might be emitted at a different location than
/// shown in the shown code snippet. In most cases, the `#[expect]` attribute
/// works when added to the outer scope. A few lints can only be expected
/// on a crate level.
///
/// Part of RFC 2383. The progress is being tracked in [#54503]
///
/// [#54503]: https://github.com/rust-lang/rust/issues/54503
pub UNFULFILLED_LINT_EXPECTATIONS,
Warn,
"unfulfilled lint expectation",
@feature_gate = rustc_span::sym::lint_reasons;
"unfulfilled lint expectation"
}

declare_lint! {
Expand Down
60 changes: 55 additions & 5 deletions src/doc/rustc/src/lints/levels.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
# Lint Levels

In `rustc`, lints are divided into five *levels*:
In `rustc`, lints are divided into six *levels*:

1. allow
2. warn
3. force-warn
4. deny
5. forbid
2. expect
3. warn
4. force-warn
5. deny
6. forbid

Each lint has a default level (explained in the lint listing later in this
chapter), and the compiler has a default warning level. First, let's explain
Expand All @@ -33,6 +34,40 @@ But this code violates the `missing_docs` lint.
These lints exist mostly to be manually turned on via configuration, as we'll
talk about later in this section.

## expect

Sometimes, it can be helpful to suppress lints, but at the same time ensure that
the code in question still emits them. The 'expect' level does exactly this. If
the lint in question is not emitted, the `unfulfilled_lint_expectation` lint
triggers on the `expect` attribute, notifying you that the expectation is no
longer fulfilled.

```rust
fn main() {
#[expect(unused_variables)]
let unused = "Everyone ignores me";

#[expect(unused_variables)] // `unused_variables` lint is not emitted
let used = "I'm useful"; // the expectation is therefore unfulfilled
println!("The `used` value is equal to: {:?}", used);
}
```

This will produce the following warning:

```txt
warning: this lint expectation is unfulfilled
--> src/main.rs:7:14
|
7 | #[expect(unused_variables)]
| ^^^^^^^^^^^^^^^^
|
= note: `#[warn(unfulfilled_lint_expectations)]` on by default
```

This level can only be defined via the `#[expect]` attribute, there is no equivalent
flag. Lints with the special 'force-warn' level will still be emitted as usual.

## warn

The 'warn' lint level will produce a warning if you violate the lint. For example,
Expand Down Expand Up @@ -240,6 +275,21 @@ And use multiple attributes together:
pub fn foo() {}
```

All lint attributes support an additional `reason` parameter, to give context why
a certain attribute was added. This reason will be displayed as part of the lint
message, if the lint is emitted at the defined level.

```rust
use std::path::PathBuf;
pub fn get_path() -> PathBuf {
#[allow(unused_mut, reason = "this is only modified on some platforms")]
let mut file_name = PathBuf::from("git");
#[cfg(target_os = "windows")]
file_name.set_extension("exe");
file_name
}
```

### Capping lints

`rustc` supports a flag, `--cap-lints LEVEL` that sets the "lint cap level."
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1943,7 +1943,7 @@ Released 2022-05-19
[#8218](https://github.com/rust-lang/rust-clippy/pull/8218)
* [`needless_match`]
[#8471](https://github.com/rust-lang/rust-clippy/pull/8471)
* [`allow_attributes_without_reason`] (Requires `#![feature(lint_reasons)]`)
* [`allow_attributes_without_reason`]
[#8504](https://github.com/rust-lang/rust-clippy/pull/8504)
* [`print_in_format_impl`]
[#8253](https://github.com/rust-lang/rust-clippy/pull/8253)
Expand Down
2 changes: 2 additions & 0 deletions src/tools/clippy/book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,8 @@ The minimum rust version that the project supports. Defaults to the `rust-versio

---
**Affected lints:**
* [`allow_attributes`](https://rust-lang.github.io/rust-clippy/master/index.html#allow_attributes)
* [`allow_attributes_without_reason`](https://rust-lang.github.io/rust-clippy/master/index.html#allow_attributes_without_reason)
* [`almost_complete_range`](https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_range)
* [`approx_constant`](https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant)
* [`assigning_clones`](https://rust-lang.github.io/rust-clippy/master/index.html#assigning_clones)
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ define_Conf! {
///
/// Suppress lints whenever the suggested change would cause breakage for other crates.
(avoid_breaking_exported_api: bool = true),
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE, ITER_KV_MAP, MANUAL_C_STR_LITERALS, ASSIGNING_CLONES, LEGACY_NUMERIC_CONSTANTS.
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE, ITER_KV_MAP, MANUAL_C_STR_LITERALS, ASSIGNING_CLONES, LEGACY_NUMERIC_CONSTANTS, ALLOW_ATTRIBUTES, ALLOW_ATTRIBUTES_WITHOUT_REASON.
///
/// The minimum rust version that the project supports. Defaults to the `rust-version` field in `Cargo.toml`
#[default_text = ""]
Expand Down
1 change: 1 addition & 0 deletions src/tools/clippy/clippy_config/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ macro_rules! msrv_aliases {

// names may refer to stabilized feature flags or library items
msrv_aliases! {
1,81,0 { LINT_REASONS_STABILIZATION }
Copy link

Choose a reason for hiding this comment

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

One space too much after 0

1,77,0 { C_STR_LITERALS }
1,76,0 { PTR_FROM_REF }
1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }
Expand Down
74 changes: 0 additions & 74 deletions src/tools/clippy/clippy_lints/src/allow_attributes.rs

This file was deleted.

26 changes: 26 additions & 0 deletions src/tools/clippy/clippy_lints/src/attrs/allow_attributes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_from_proc_macro;
use rustc_ast::{AttrStyle, Attribute};
use rustc_errors::Applicability;
use rustc_lint::{LateContext, LintContext};
use rustc_middle::lint::in_external_macro;
use super::ALLOW_ATTRIBUTES;

// Separate each crate's features.
pub fn check<'cx>(cx: &LateContext<'cx>, attr: &'cx Attribute) {
if !in_external_macro(cx.sess(), attr.span)
&& let AttrStyle::Outer = attr.style
&& let Some(ident) = attr.ident()
&& !is_from_proc_macro(cx, &attr)
{
span_lint_and_sugg(
cx,
ALLOW_ATTRIBUTES,
ident.span,
"#[allow] attribute found",
"replace it with",
"expect".into(),
Applicability::MachineApplicable,
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ use rustc_span::sym;
use rustc_span::symbol::Symbol;

pub(super) fn check<'cx>(cx: &LateContext<'cx>, name: Symbol, items: &[NestedMetaItem], attr: &'cx Attribute) {
// Check for the feature
if !cx.tcx.features().lint_reasons {
return;
}

// Check if the reason is present
if let Some(item) = items.last().and_then(NestedMetaItem::meta_item)
&& let MetaItemKind::NameValue(_) = &item.kind
Expand Down
Loading
Loading