Auto merge of #54683 - zackmdavis:critique_of_pure_lints, r=petrochenkov
lint reasons (RFC 2883, part 1) This implements the `reason =` functionality described in [the RFC](https://github.com/rust-lang/rfcs/blob/master/text/2383-lint-reasons.md) under a `lint_reasons` feature gate. 
This commit is contained in:
commit
18311a6c47
12 changed files with 304 additions and 20 deletions
|
|
@ -21,6 +21,7 @@ use rustc_data_structures::stable_hasher::{HashStable, ToStableHashKey,
|
|||
use session::Session;
|
||||
use syntax::ast;
|
||||
use syntax::attr;
|
||||
use syntax::feature_gate;
|
||||
use syntax::source_map::MultiSpan;
|
||||
use syntax::symbol::Symbol;
|
||||
use util::nodemap::FxHashMap;
|
||||
|
|
@ -199,8 +200,7 @@ impl<'a> LintLevelsBuilder<'a> {
|
|||
let store = self.sess.lint_store.borrow();
|
||||
let sess = self.sess;
|
||||
let bad_attr = |span| {
|
||||
span_err!(sess, span, E0452,
|
||||
"malformed lint attribute");
|
||||
struct_span_err!(sess, span, E0452, "malformed lint attribute")
|
||||
};
|
||||
for attr in attrs {
|
||||
let level = match Level::from_str(&attr.name().as_str()) {
|
||||
|
|
@ -211,19 +211,76 @@ impl<'a> LintLevelsBuilder<'a> {
|
|||
let meta = unwrap_or!(attr.meta(), continue);
|
||||
attr::mark_used(attr);
|
||||
|
||||
let metas = if let Some(metas) = meta.meta_item_list() {
|
||||
let mut metas = if let Some(metas) = meta.meta_item_list() {
|
||||
metas
|
||||
} else {
|
||||
bad_attr(meta.span);
|
||||
continue
|
||||
let mut err = bad_attr(meta.span);
|
||||
err.emit();
|
||||
continue;
|
||||
};
|
||||
|
||||
if metas.is_empty() {
|
||||
// FIXME (#55112): issue unused-attributes lint for `#[level()]`
|
||||
continue;
|
||||
}
|
||||
|
||||
// Before processing the lint names, look for a reason (RFC 2383)
|
||||
// at the end.
|
||||
let mut reason = None;
|
||||
let tail_li = &metas[metas.len()-1];
|
||||
if let Some(item) = tail_li.meta_item() {
|
||||
match item.node {
|
||||
ast::MetaItemKind::Word => {} // actual lint names handled later
|
||||
ast::MetaItemKind::NameValue(ref name_value) => {
|
||||
let gate_reasons = !self.sess.features_untracked().lint_reasons;
|
||||
if item.ident == "reason" {
|
||||
// found reason, reslice meta list to exclude it
|
||||
metas = &metas[0..metas.len()-1];
|
||||
// FIXME (#55112): issue unused-attributes lint if we thereby
|
||||
// don't have any lint names (`#[level(reason = "foo")]`)
|
||||
if let ast::LitKind::Str(rationale, _) = name_value.node {
|
||||
if gate_reasons {
|
||||
feature_gate::emit_feature_err(
|
||||
&self.sess.parse_sess,
|
||||
"lint_reasons",
|
||||
item.span,
|
||||
feature_gate::GateIssue::Language,
|
||||
"lint reasons are experimental"
|
||||
);
|
||||
} else {
|
||||
reason = Some(rationale);
|
||||
}
|
||||
} else {
|
||||
let mut err = bad_attr(name_value.span);
|
||||
err.help("reason must be a string literal");
|
||||
err.emit();
|
||||
}
|
||||
} else {
|
||||
let mut err = bad_attr(item.span);
|
||||
err.emit();
|
||||
}
|
||||
},
|
||||
ast::MetaItemKind::List(_) => {
|
||||
let mut err = bad_attr(item.span);
|
||||
err.emit();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for li in metas {
|
||||
let word = match li.word() {
|
||||
Some(word) => word,
|
||||
None => {
|
||||
bad_attr(li.span);
|
||||
continue
|
||||
let mut err = bad_attr(li.span);
|
||||
if let Some(item) = li.meta_item() {
|
||||
if let ast::MetaItemKind::NameValue(_) = item.node {
|
||||
if item.ident == "reason" {
|
||||
err.help("reason in lint attribute must come last");
|
||||
}
|
||||
}
|
||||
}
|
||||
err.emit();
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let tool_name = if let Some(lint_tool) = word.is_scoped() {
|
||||
|
|
@ -245,7 +302,7 @@ impl<'a> LintLevelsBuilder<'a> {
|
|||
let name = word.name();
|
||||
match store.check_lint_name(&name.as_str(), tool_name) {
|
||||
CheckLintNameResult::Ok(ids) => {
|
||||
let src = LintSource::Node(name, li.span);
|
||||
let src = LintSource::Node(name, li.span, reason);
|
||||
for id in ids {
|
||||
specs.insert(*id, (level, src));
|
||||
}
|
||||
|
|
@ -255,7 +312,9 @@ impl<'a> LintLevelsBuilder<'a> {
|
|||
match result {
|
||||
Ok(ids) => {
|
||||
let complete_name = &format!("{}::{}", tool_name.unwrap(), name);
|
||||
let src = LintSource::Node(Symbol::intern(complete_name), li.span);
|
||||
let src = LintSource::Node(
|
||||
Symbol::intern(complete_name), li.span, reason
|
||||
);
|
||||
for id in ids {
|
||||
specs.insert(*id, (level, src));
|
||||
}
|
||||
|
|
@ -286,7 +345,9 @@ impl<'a> LintLevelsBuilder<'a> {
|
|||
Applicability::MachineApplicable,
|
||||
).emit();
|
||||
|
||||
let src = LintSource::Node(Symbol::intern(&new_lint_name), li.span);
|
||||
let src = LintSource::Node(
|
||||
Symbol::intern(&new_lint_name), li.span, reason
|
||||
);
|
||||
for id in ids {
|
||||
specs.insert(*id, (level, src));
|
||||
}
|
||||
|
|
@ -368,11 +429,11 @@ impl<'a> LintLevelsBuilder<'a> {
|
|||
};
|
||||
let forbidden_lint_name = match forbid_src {
|
||||
LintSource::Default => id.to_string(),
|
||||
LintSource::Node(name, _) => name.to_string(),
|
||||
LintSource::Node(name, _, _) => name.to_string(),
|
||||
LintSource::CommandLine(name) => name.to_string(),
|
||||
};
|
||||
let (lint_attr_name, lint_attr_span) = match *src {
|
||||
LintSource::Node(name, span) => (name, span),
|
||||
LintSource::Node(name, span, _) => (name, span),
|
||||
_ => continue,
|
||||
};
|
||||
let mut diag_builder = struct_span_err!(self.sess,
|
||||
|
|
@ -384,15 +445,19 @@ impl<'a> LintLevelsBuilder<'a> {
|
|||
forbidden_lint_name);
|
||||
diag_builder.span_label(lint_attr_span, "overruled by previous forbid");
|
||||
match forbid_src {
|
||||
LintSource::Default => &mut diag_builder,
|
||||
LintSource::Node(_, forbid_source_span) => {
|
||||
LintSource::Default => {},
|
||||
LintSource::Node(_, forbid_source_span, reason) => {
|
||||
diag_builder.span_label(forbid_source_span,
|
||||
"`forbid` level set here")
|
||||
"`forbid` level set here");
|
||||
if let Some(rationale) = reason {
|
||||
diag_builder.note(&rationale.as_str());
|
||||
}
|
||||
},
|
||||
LintSource::CommandLine(_) => {
|
||||
diag_builder.note("`forbid` lint level was set on command line")
|
||||
diag_builder.note("`forbid` lint level was set on command line");
|
||||
}
|
||||
}.emit();
|
||||
}
|
||||
diag_builder.emit();
|
||||
// don't set a separate error for every lint in the group
|
||||
break
|
||||
}
|
||||
|
|
|
|||
|
|
@ -470,7 +470,7 @@ pub enum LintSource {
|
|||
Default,
|
||||
|
||||
/// Lint level was set by an attribute.
|
||||
Node(ast::Name, Span),
|
||||
Node(ast::Name, Span, Option<Symbol> /* RFC 2383 reason */),
|
||||
|
||||
/// Lint level was set by a command-line flag.
|
||||
CommandLine(Symbol),
|
||||
|
|
@ -478,7 +478,7 @@ pub enum LintSource {
|
|||
|
||||
impl_stable_hash_for!(enum self::LintSource {
|
||||
Default,
|
||||
Node(name, span),
|
||||
Node(name, span, reason),
|
||||
CommandLine(text)
|
||||
});
|
||||
|
||||
|
|
@ -578,7 +578,10 @@ pub fn struct_lint_level<'a>(sess: &'a Session,
|
|||
hyphen_case_flag_val));
|
||||
}
|
||||
}
|
||||
LintSource::Node(lint_attr_name, src) => {
|
||||
LintSource::Node(lint_attr_name, src, reason) => {
|
||||
if let Some(rationale) = reason {
|
||||
err.note(&rationale.as_str());
|
||||
}
|
||||
sess.diag_span_note_once(&mut err, DiagnosticMessageId::from(lint),
|
||||
src, "lint level defined here");
|
||||
if lint_attr_name.as_str() != name {
|
||||
|
|
|
|||
|
|
@ -504,6 +504,9 @@ declare_features! (
|
|||
|
||||
// `extern crate foo as bar;` puts `bar` into extern prelude.
|
||||
(active, extern_crate_item_prelude, "1.31.0", Some(54658), None),
|
||||
|
||||
// `reason = ` in lint attributes and `expect` lint attribute
|
||||
(active, lint_reasons, "1.31.0", Some(54503), None),
|
||||
);
|
||||
|
||||
declare_features! (
|
||||
|
|
|
|||
4
src/test/ui/feature-gates/feature-gate-lint-reasons.rs
Normal file
4
src/test/ui/feature-gates/feature-gate-lint-reasons.rs
Normal file
|
|
@ -0,0 +1,4 @@
|
|||
#![warn(nonstandard_style, reason = "the standard should be respected")]
|
||||
//~^ ERROR lint reasons are experimental
|
||||
|
||||
fn main() {}
|
||||
11
src/test/ui/feature-gates/feature-gate-lint-reasons.stderr
Normal file
11
src/test/ui/feature-gates/feature-gate-lint-reasons.stderr
Normal file
|
|
@ -0,0 +1,11 @@
|
|||
error[E0658]: lint reasons are experimental (see issue #54503)
|
||||
--> $DIR/feature-gate-lint-reasons.rs:1:28
|
||||
|
|
||||
LL | #![warn(nonstandard_style, reason = "the standard should be respected")]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= help: add #![feature(lint_reasons)] to the crate attributes to enable
|
||||
|
||||
error: aborting due to previous error
|
||||
|
||||
For more information about this error, try `rustc --explain E0658`.
|
||||
17
src/test/ui/lint/empty-lint-attributes.rs
Normal file
17
src/test/ui/lint/empty-lint-attributes.rs
Normal file
|
|
@ -0,0 +1,17 @@
|
|||
#![feature(lint_reasons)]
|
||||
|
||||
// run-pass
|
||||
|
||||
// Empty (and reason-only) lint attributes are legal—although we may want to
|
||||
// lint them in the future (Issue #55112).
|
||||
|
||||
#![allow()]
|
||||
#![warn(reason = "observationalism")]
|
||||
|
||||
#[forbid()]
|
||||
fn devoir() {}
|
||||
|
||||
#[deny(reason = "ultion")]
|
||||
fn waldgrave() {}
|
||||
|
||||
fn main() {}
|
||||
24
src/test/ui/lint/reasons-erroneous.rs
Normal file
24
src/test/ui/lint/reasons-erroneous.rs
Normal file
|
|
@ -0,0 +1,24 @@
|
|||
#![feature(lint_reasons)]
|
||||
|
||||
#![warn(absolute_paths_not_starting_with_crate, reason = 0)]
|
||||
//~^ ERROR malformed lint attribute
|
||||
//~| HELP reason must be a string literal
|
||||
#![warn(anonymous_parameters, reason = b"consider these, for we have condemned them")]
|
||||
//~^ ERROR malformed lint attribute
|
||||
//~| HELP reason must be a string literal
|
||||
#![warn(bare_trait_objects, reasons = "leaders to no sure land, guides their bearings lost")]
|
||||
//~^ ERROR malformed lint attribute
|
||||
#![warn(box_pointers, blerp = "or in league with robbers have reversed the signposts")]
|
||||
//~^ ERROR malformed lint attribute
|
||||
#![warn(elided_lifetimes_in_paths, reason("disrespectful to ancestors", "irresponsible to heirs"))]
|
||||
//~^ ERROR malformed lint attribute
|
||||
#![warn(ellipsis_inclusive_range_patterns, reason = "born barren", reason = "a freak growth")]
|
||||
//~^ ERROR malformed lint attribute
|
||||
//~| HELP reason in lint attribute must come last
|
||||
#![warn(keyword_idents, reason = "root in rubble", macro_use_extern_crate)]
|
||||
//~^ ERROR malformed lint attribute
|
||||
//~| HELP reason in lint attribute must come last
|
||||
#![warn(missing_copy_implementations, reason)]
|
||||
//~^ WARN unknown lint
|
||||
|
||||
fn main() {}
|
||||
61
src/test/ui/lint/reasons-erroneous.stderr
Normal file
61
src/test/ui/lint/reasons-erroneous.stderr
Normal file
|
|
@ -0,0 +1,61 @@
|
|||
error[E0452]: malformed lint attribute
|
||||
--> $DIR/reasons-erroneous.rs:3:58
|
||||
|
|
||||
LL | #![warn(absolute_paths_not_starting_with_crate, reason = 0)]
|
||||
| ^
|
||||
|
|
||||
= help: reason must be a string literal
|
||||
|
||||
error[E0452]: malformed lint attribute
|
||||
--> $DIR/reasons-erroneous.rs:6:40
|
||||
|
|
||||
LL | #![warn(anonymous_parameters, reason = b"consider these, for we have condemned them")]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= help: reason must be a string literal
|
||||
|
||||
error[E0452]: malformed lint attribute
|
||||
--> $DIR/reasons-erroneous.rs:9:29
|
||||
|
|
||||
LL | #![warn(bare_trait_objects, reasons = "leaders to no sure land, guides their bearings lost")]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error[E0452]: malformed lint attribute
|
||||
--> $DIR/reasons-erroneous.rs:11:23
|
||||
|
|
||||
LL | #![warn(box_pointers, blerp = "or in league with robbers have reversed the signposts")]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error[E0452]: malformed lint attribute
|
||||
--> $DIR/reasons-erroneous.rs:13:36
|
||||
|
|
||||
LL | #![warn(elided_lifetimes_in_paths, reason("disrespectful to ancestors", "irresponsible to heirs"))]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error[E0452]: malformed lint attribute
|
||||
--> $DIR/reasons-erroneous.rs:15:44
|
||||
|
|
||||
LL | #![warn(ellipsis_inclusive_range_patterns, reason = "born barren", reason = "a freak growth")]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= help: reason in lint attribute must come last
|
||||
|
||||
error[E0452]: malformed lint attribute
|
||||
--> $DIR/reasons-erroneous.rs:18:25
|
||||
|
|
||||
LL | #![warn(keyword_idents, reason = "root in rubble", macro_use_extern_crate)]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= help: reason in lint attribute must come last
|
||||
|
||||
warning: unknown lint: `reason`
|
||||
--> $DIR/reasons-erroneous.rs:21:39
|
||||
|
|
||||
LL | #![warn(missing_copy_implementations, reason)]
|
||||
| ^^^^^^
|
||||
|
|
||||
= note: #[warn(unknown_lints)] on by default
|
||||
|
||||
error: aborting due to 7 previous errors
|
||||
|
||||
For more information about this error, try `rustc --explain E0452`.
|
||||
21
src/test/ui/lint/reasons-forbidden.rs
Normal file
21
src/test/ui/lint/reasons-forbidden.rs
Normal file
|
|
@ -0,0 +1,21 @@
|
|||
#![feature(lint_reasons)]
|
||||
|
||||
#![forbid(
|
||||
unsafe_code,
|
||||
//~^ NOTE `forbid` level set here
|
||||
reason = "our errors & omissions insurance policy doesn't cover unsafe Rust"
|
||||
)]
|
||||
|
||||
use std::ptr;
|
||||
|
||||
fn main() {
|
||||
let a_billion_dollar_mistake = ptr::null();
|
||||
|
||||
#[allow(unsafe_code)]
|
||||
//~^ ERROR allow(unsafe_code) overruled by outer forbid(unsafe_code)
|
||||
//~| NOTE overruled by previous forbid
|
||||
//~| NOTE our errors & omissions insurance policy doesn't cover unsafe Rust
|
||||
unsafe {
|
||||
*a_billion_dollar_mistake
|
||||
}
|
||||
}
|
||||
14
src/test/ui/lint/reasons-forbidden.stderr
Normal file
14
src/test/ui/lint/reasons-forbidden.stderr
Normal file
|
|
@ -0,0 +1,14 @@
|
|||
error[E0453]: allow(unsafe_code) overruled by outer forbid(unsafe_code)
|
||||
--> $DIR/reasons-forbidden.rs:14:13
|
||||
|
|
||||
LL | unsafe_code,
|
||||
| ----------- `forbid` level set here
|
||||
...
|
||||
LL | #[allow(unsafe_code)]
|
||||
| ^^^^^^^^^^^ overruled by previous forbid
|
||||
|
|
||||
= note: our errors & omissions insurance policy doesn't cover unsafe Rust
|
||||
|
||||
error: aborting due to previous error
|
||||
|
||||
For more information about this error, try `rustc --explain E0453`.
|
||||
33
src/test/ui/lint/reasons.rs
Normal file
33
src/test/ui/lint/reasons.rs
Normal file
|
|
@ -0,0 +1,33 @@
|
|||
// compile-pass
|
||||
|
||||
#![feature(lint_reasons)]
|
||||
|
||||
#![warn(elided_lifetimes_in_paths,
|
||||
//~^ NOTE lint level defined here
|
||||
reason = "explicit anonymous lifetimes aid reasoning about ownership")]
|
||||
#![warn(
|
||||
nonstandard_style,
|
||||
//~^ NOTE lint level defined here
|
||||
reason = r#"people shouldn't have to change their usual style habits
|
||||
to contribute to our project"#
|
||||
)]
|
||||
#![allow(unused, reason = "unused code has never killed anypony")]
|
||||
|
||||
use std::fmt;
|
||||
|
||||
pub struct CheaterDetectionMechanism {}
|
||||
|
||||
impl fmt::Debug for CheaterDetectionMechanism {
|
||||
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
|
||||
//~^ WARN hidden lifetime parameters in types are deprecated
|
||||
//~| NOTE explicit anonymous lifetimes aid
|
||||
//~| HELP indicate the anonymous lifetime
|
||||
fmt.debug_struct("CheaterDetectionMechanism").finish()
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {
|
||||
let Social_exchange_psychology = CheaterDetectionMechanism {};
|
||||
//~^ WARN should have a snake case name such as
|
||||
//~| NOTE people shouldn't have to change their usual style habits
|
||||
}
|
||||
28
src/test/ui/lint/reasons.stderr
Normal file
28
src/test/ui/lint/reasons.stderr
Normal file
|
|
@ -0,0 +1,28 @@
|
|||
warning: hidden lifetime parameters in types are deprecated
|
||||
--> $DIR/reasons.rs:21:29
|
||||
|
|
||||
LL | fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
|
||||
| ^^^^^^^^^^^^^^- help: indicate the anonymous lifetime: `<'_>`
|
||||
|
|
||||
= note: explicit anonymous lifetimes aid reasoning about ownership
|
||||
note: lint level defined here
|
||||
--> $DIR/reasons.rs:5:9
|
||||
|
|
||||
LL | #![warn(elided_lifetimes_in_paths,
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
warning: variable `Social_exchange_psychology` should have a snake case name such as `social_exchange_psychology`
|
||||
--> $DIR/reasons.rs:30:9
|
||||
|
|
||||
LL | let Social_exchange_psychology = CheaterDetectionMechanism {};
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: people shouldn't have to change their usual style habits
|
||||
to contribute to our project
|
||||
note: lint level defined here
|
||||
--> $DIR/reasons.rs:9:5
|
||||
|
|
||||
LL | nonstandard_style,
|
||||
| ^^^^^^^^^^^^^^^^^
|
||||
= note: #[warn(non_snake_case)] implied by #[warn(nonstandard_style)]
|
||||
|
||||
Loading…
Add table
Add a link
Reference in a new issue