Skip to content

Commit 1f0fc02

Browse files
borsbors
committed
Auto merge of #80524 - jyn514:unknown-tool-lints, r=flip1995,matthewjasper
Don't make tools responsible for checking unknown and renamed lints Previously, clippy (and any other tool emitting lints) had to have their own separate UNKNOWN_LINTS pass, because the compiler assumed any tool lint could be valid. Now, as long as any lint starting with the tool prefix exists, the compiler will warn when an unknown lint is present. This may interact with the unstable `tool_lint` feature, which I don't entirely understand, but it will take the burden off those external tools to add their own lint pass, which seems like a step in the right direction to me. - Don't mark `ineffective_unstable_trait_impl` as an internal lint - Use clippy's more advanced lint suggestions - Deprecate the `UNKNOWN_CLIPPY_LINTS` pass (and make it a no-op) - Say 'unknown lint `clippy::x`' instead of 'unknown lint x' This is tested by existing clippy tests. When #80527 merges, it will also be tested in rustdoc tests. AFAIK there is no way to test this with rustc directly.
2 parents edeb631 + 13728b8 commit 1f0fc02

File tree

12 files changed

+116
-112
lines changed

12 files changed

+116
-112
lines changed

Diff for: ‎compiler/rustc_lint/src/context.rs

+31-14
Original file line numberDiff line numberDiff line change
@@ -354,10 +354,23 @@ impl LintStore {
354354
lint_name.to_string()
355355
};
356356
// If the lint was scoped with `tool::` check if the tool lint exists
357-
if tool_name.is_some() {
357+
if let Some(tool_name) = tool_name {
358358
match self.by_name.get(&complete_name) {
359359
None => match self.lint_groups.get(&*complete_name) {
360-
None => return CheckLintNameResult::Tool(Err((None, String::new()))),
360+
// If the lint isn't registered, there are two possibilities:
361+
None => {
362+
// 1. The tool is currently running, so this lint really doesn't exist.
363+
// FIXME: should this handle tools that never register a lint, like rustfmt?
364+
tracing::debug!("lints={:?}", self.by_name.keys().collect::<Vec<_>>());
365+
let tool_prefix = format!("{}::", tool_name);
366+
return if self.by_name.keys().any(|lint| lint.starts_with(&tool_prefix)) {
367+
self.no_lint_suggestion(&complete_name)
368+
} else {
369+
// 2. The tool isn't currently running, so no lints will be registered.
370+
// To avoid giving a false positive, ignore all unknown lints.
371+
CheckLintNameResult::Tool(Err((None, String::new())))
372+
};
373+
}
361374
Some(LintGroup { lint_ids, .. }) => {
362375
return CheckLintNameResult::Tool(Ok(&lint_ids));
363376
}
@@ -398,6 +411,21 @@ impl LintStore {
398411
}
399412
}
400413

414+
fn no_lint_suggestion(&self, lint_name: &str) -> CheckLintNameResult<'_> {
415+
let name_lower = lint_name.to_lowercase();
416+
let symbols =
417+
self.get_lints().iter().map(|l| Symbol::intern(&l.name_lower())).collect::<Vec<_>>();
418+
419+
if lint_name.chars().any(char::is_uppercase) && self.find_lints(&name_lower).is_ok() {
420+
// First check if the lint name is (partly) in upper case instead of lower case...
421+
CheckLintNameResult::NoLint(Some(Symbol::intern(&name_lower)))
422+
} else {
423+
// ...if not, search for lints with a similar name
424+
let suggestion = find_best_match_for_name(&symbols, Symbol::intern(&name_lower), None);
425+
CheckLintNameResult::NoLint(suggestion)
426+
}
427+
}
428+
401429
fn check_tool_name_for_backwards_compat(
402430
&self,
403431
lint_name: &str,
@@ -407,18 +435,7 @@ impl LintStore {
407435
match self.by_name.get(&complete_name) {
408436
None => match self.lint_groups.get(&*complete_name) {
409437
// Now we are sure, that this lint exists nowhere
410-
None => {
411-
let symbols =
412-
self.by_name.keys().map(|name| Symbol::intern(&name)).collect::<Vec<_>>();
413-
414-
let suggestion = find_best_match_for_name(
415-
&symbols,
416-
Symbol::intern(&lint_name.to_lowercase()),
417-
None,
418-
);
419-
420-
CheckLintNameResult::NoLint(suggestion)
421-
}
438+
None => self.no_lint_suggestion(lint_name),
422439
Some(LintGroup { lint_ids, depr, .. }) => {
423440
// Reaching this would be weird, but let's cover this case anyway
424441
if let Some(LintAlias { name, silent }) = depr {

Diff for: ‎compiler/rustc_lint/src/levels.rs

+5
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,11 @@ impl<'s> LintLevelsBuilder<'s> {
381381
src,
382382
Some(li.span().into()),
383383
|lint| {
384+
let name = if let Some(tool_name) = tool_name {
385+
format!("{}::{}", tool_name, name)
386+
} else {
387+
name.to_string()
388+
};
384389
let mut db = lint.build(&format!("unknown lint: `{}`", name));
385390
if let Some(suggestion) = suggestion {
386391
db.span_suggestion(

Diff for: ‎compiler/rustc_lint_defs/src/builtin.rs

+24-3
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
//! compiler code, rather than using their own custom pass. Those
55
//! lints are all available in `rustc_lint::builtin`.
66
7-
use crate::{declare_lint, declare_lint_pass, declare_tool_lint};
7+
use crate::{declare_lint, declare_lint_pass};
88
use rustc_span::edition::Edition;
99
use rustc_span::symbol::sym;
1010

@@ -2825,8 +2825,29 @@ declare_lint! {
28252825
};
28262826
}
28272827

2828-
declare_tool_lint! {
2829-
pub rustc::INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
2828+
declare_lint! {
2829+
/// The `ineffective_unstable_trait_impl` lint detects `#[unstable]` attributes which are not used.
2830+
///
2831+
/// ### Example
2832+
///
2833+
/// ```compile_fail
2834+
/// #![feature(staged_api)]
2835+
///
2836+
/// #[derive(Clone)]
2837+
/// #[stable(feature = "x", since = "1")]
2838+
/// struct S {}
2839+
///
2840+
/// #[unstable(feature = "y", issue = "none")]
2841+
/// impl Copy for S {}
2842+
/// ```
2843+
///
2844+
/// {{produces}}
2845+
///
2846+
/// ### Explanation
2847+
///
2848+
/// `staged_api` does not currently support using a stability attribute on `impl` blocks.
2849+
/// `impl`s are always stable if both the type and trait are stable, and always unstable otherwise.
2850+
pub INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
28302851
Deny,
28312852
"detects `#[unstable]` on stable trait implementations for stable types"
28322853
}

Diff for: ‎library/alloc/src/task.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ pub trait Wake {
3333
}
3434
}
3535

36-
#[allow(rustc::ineffective_unstable_trait_impl)]
36+
#[cfg_attr(bootstrap, allow(rustc::ineffective_unstable_trait_impl))]
37+
#[cfg_attr(not(bootstrap), allow(ineffective_unstable_trait_impl))]
3738
#[unstable(feature = "wake_trait", issue = "69912")]
3839
impl<W: Wake + Send + Sync + 'static> From<Arc<W>> for Waker {
3940
fn from(waker: Arc<W>) -> Waker {
@@ -43,7 +44,8 @@ impl<W: Wake + Send + Sync + 'static> From<Arc<W>> for Waker {
4344
}
4445
}
4546

46-
#[allow(rustc::ineffective_unstable_trait_impl)]
47+
#[cfg_attr(bootstrap, allow(rustc::ineffective_unstable_trait_impl))]
48+
#[cfg_attr(not(bootstrap), allow(ineffective_unstable_trait_impl))]
4749
#[unstable(feature = "wake_trait", issue = "69912")]
4850
impl<W: Wake + Send + Sync + 'static> From<Arc<W>> for RawWaker {
4951
fn from(waker: Arc<W>) -> RawWaker {

Diff for: ‎src/test/ui/stability-attribute/stability-attribute-trait-impl.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ impl StableTrait for UnstableType {}
2222
impl UnstableTrait for StableType {}
2323

2424
#[unstable(feature = "x", issue = "none")]
25-
//~^ ERROR an `#[unstable]` annotation here has no effect [rustc::ineffective_unstable_trait_impl]
25+
//~^ ERROR an `#[unstable]` annotation here has no effect [ineffective_unstable_trait_impl]
2626
impl StableTrait for StableType {}
2727

2828
fn main() {}

Diff for: ‎src/test/ui/stability-attribute/stability-attribute-trait-impl.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ error: an `#[unstable]` annotation here has no effect
44
LL | #[unstable(feature = "x", issue = "none")]
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
66
|
7-
= note: `#[deny(rustc::ineffective_unstable_trait_impl)]` on by default
7+
= note: `#[deny(ineffective_unstable_trait_impl)]` on by default
88
= note: see issue #55436 <https://github.com/rust-lang/rust/issues/55436> for more information
99

1010
error: aborting due to previous error

Diff for: ‎src/tools/clippy/clippy_lints/src/attrs.rs

+2-70
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,10 @@ use rustc_errors::Applicability;
1010
use rustc_hir::{
1111
Block, Expr, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, StmtKind, TraitFn, TraitItem, TraitItemKind,
1212
};
13-
use rustc_lint::{CheckLintNameResult, EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
13+
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
1414
use rustc_middle::lint::in_external_macro;
1515
use rustc_middle::ty;
1616
use rustc_session::{declare_lint_pass, declare_tool_lint};
17-
use rustc_span::lev_distance::find_best_match_for_name;
1817
use rustc_span::source_map::Span;
1918
use rustc_span::sym;
2019
use rustc_span::symbol::{Symbol, SymbolStr};
@@ -156,33 +155,6 @@ declare_clippy_lint! {
156155
"empty line after outer attribute"
157156
}
158157

159-
declare_clippy_lint! {
160-
/// **What it does:** Checks for `allow`/`warn`/`deny`/`forbid` attributes with scoped clippy
161-
/// lints and if those lints exist in clippy. If there is an uppercase letter in the lint name
162-
/// (not the tool name) and a lowercase version of this lint exists, it will suggest to lowercase
163-
/// the lint name.
164-
///
165-
/// **Why is this bad?** A lint attribute with a mistyped lint name won't have an effect.
166-
///
167-
/// **Known problems:** None.
168-
///
169-
/// **Example:**
170-
/// Bad:
171-
/// ```rust
172-
/// #![warn(if_not_els)]
173-
/// #![deny(clippy::All)]
174-
/// ```
175-
///
176-
/// Good:
177-
/// ```rust
178-
/// #![warn(if_not_else)]
179-
/// #![deny(clippy::all)]
180-
/// ```
181-
pub UNKNOWN_CLIPPY_LINTS,
182-
style,
183-
"unknown_lints for scoped Clippy lints"
184-
}
185-
186158
declare_clippy_lint! {
187159
/// **What it does:** Checks for `warn`/`deny`/`forbid` attributes targeting the whole clippy::restriction category.
188160
///
@@ -272,7 +244,6 @@ declare_lint_pass!(Attributes => [
272244
INLINE_ALWAYS,
273245
DEPRECATED_SEMVER,
274246
USELESS_ATTRIBUTE,
275-
UNKNOWN_CLIPPY_LINTS,
276247
BLANKET_CLIPPY_RESTRICTION_LINTS,
277248
]);
278249

@@ -409,48 +380,9 @@ fn extract_clippy_lint(lint: &NestedMetaItem) -> Option<SymbolStr> {
409380
}
410381

411382
fn check_clippy_lint_names(cx: &LateContext<'_>, ident: &str, items: &[NestedMetaItem]) {
412-
let lint_store = cx.lints();
413383
for lint in items {
414384
if let Some(lint_name) = extract_clippy_lint(lint) {
415-
if let CheckLintNameResult::Tool(Err((None, _))) = lint_store.check_lint_name(&lint_name, Some(sym::clippy))
416-
{
417-
span_lint_and_then(
418-
cx,
419-
UNKNOWN_CLIPPY_LINTS,
420-
lint.span(),
421-
&format!("unknown clippy lint: clippy::{}", lint_name),
422-
|diag| {
423-
let name_lower = lint_name.to_lowercase();
424-
let symbols = lint_store
425-
.get_lints()
426-
.iter()
427-
.map(|l| Symbol::intern(&l.name_lower()))
428-
.collect::<Vec<_>>();
429-
let sugg = find_best_match_for_name(
430-
&symbols,
431-
Symbol::intern(&format!("clippy::{}", name_lower)),
432-
None,
433-
);
434-
if lint_name.chars().any(char::is_uppercase)
435-
&& lint_store.find_lints(&format!("clippy::{}", name_lower)).is_ok()
436-
{
437-
diag.span_suggestion(
438-
lint.span(),
439-
"lowercase the lint name",
440-
format!("clippy::{}", name_lower),
441-
Applicability::MachineApplicable,
442-
);
443-
} else if let Some(sugg) = sugg {
444-
diag.span_suggestion(
445-
lint.span(),
446-
"did you mean",
447-
sugg.to_string(),
448-
Applicability::MachineApplicable,
449-
);
450-
}
451-
},
452-
);
453-
} else if lint_name == "restriction" && ident != "allow" {
385+
if lint_name == "restriction" && ident != "allow" {
454386
span_lint_and_help(
455387
cx,
456388
BLANKET_CLIPPY_RESTRICTION_LINTS,

Diff for: ‎src/tools/clippy/clippy_lints/src/deprecated_lints.rs

+13
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,19 @@ declare_deprecated_lint! {
163163
}
164164

165165
declare_deprecated_lint! {
166+
/// **What it does:** Nothing. This lint has been deprecated.
167+
///
168+
/// **Deprecation reason:** This lint has been uplifted to rustc and is now called
169+
/// `panic_fmt`.
166170
pub PANIC_PARAMS,
167171
"this lint has been uplifted to rustc and is now called `panic_fmt`"
168172
}
173+
174+
declare_deprecated_lint! {
175+
/// **What it does:** Nothing. This lint has been deprecated.
176+
///
177+
/// **Deprecation reason:** This lint has been integrated into the `unknown_lints`
178+
/// rustc lint.
179+
pub UNKNOWN_CLIPPY_LINTS,
180+
"this lint has been integrated into the `unknown_lints` rustc lint"
181+
}

Diff for: ‎src/tools/clippy/clippy_lints/src/lib.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
500500
"clippy::panic_params",
501501
"this lint has been uplifted to rustc and is now called `panic_fmt`",
502502
);
503+
store.register_removed(
504+
"clippy::unknown_clippy_lints",
505+
"this lint has been integrated into the `unknown_lints` rustc lint",
506+
);
503507
// end deprecated lints, do not remove this comment, it’s used in `update_lints`
504508

505509
// begin register lints, do not remove this comment, it’s used in `update_lints`
@@ -541,7 +545,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
541545
&attrs::EMPTY_LINE_AFTER_OUTER_ATTR,
542546
&attrs::INLINE_ALWAYS,
543547
&attrs::MISMATCHED_TARGET_OS,
544-
&attrs::UNKNOWN_CLIPPY_LINTS,
545548
&attrs::USELESS_ATTRIBUTE,
546549
&await_holding_invalid::AWAIT_HOLDING_LOCK,
547550
&await_holding_invalid::AWAIT_HOLDING_REFCELL_REF,
@@ -1375,7 +1378,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
13751378
LintId::of(&attrs::DEPRECATED_CFG_ATTR),
13761379
LintId::of(&attrs::DEPRECATED_SEMVER),
13771380
LintId::of(&attrs::MISMATCHED_TARGET_OS),
1378-
LintId::of(&attrs::UNKNOWN_CLIPPY_LINTS),
13791381
LintId::of(&attrs::USELESS_ATTRIBUTE),
13801382
LintId::of(&bit_mask::BAD_BIT_MASK),
13811383
LintId::of(&bit_mask::INEFFECTIVE_BIT_MASK),
@@ -1650,7 +1652,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16501652
LintId::of(&assertions_on_constants::ASSERTIONS_ON_CONSTANTS),
16511653
LintId::of(&assign_ops::ASSIGN_OP_PATTERN),
16521654
LintId::of(&attrs::BLANKET_CLIPPY_RESTRICTION_LINTS),
1653-
LintId::of(&attrs::UNKNOWN_CLIPPY_LINTS),
16541655
LintId::of(&blacklisted_name::BLACKLISTED_NAME),
16551656
LintId::of(&blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS),
16561657
LintId::of(&collapsible_if::COLLAPSIBLE_IF),

Diff for: ‎src/tools/clippy/tests/ui/deprecated.rs

+1
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,6 @@
99
#[warn(clippy::drop_bounds)]
1010
#[warn(clippy::temporary_cstring_as_ptr)]
1111
#[warn(clippy::panic_params)]
12+
#[warn(clippy::unknown_clippy_lints)]
1213

1314
fn main() {}

Diff for: ‎src/tools/clippy/tests/ui/deprecated.stderr

+7-1
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,17 @@ error: lint `clippy::panic_params` has been removed: `this lint has been uplifte
6666
LL | #[warn(clippy::panic_params)]
6767
| ^^^^^^^^^^^^^^^^^^^^
6868

69+
error: lint `clippy::unknown_clippy_lints` has been removed: `this lint has been integrated into the `unknown_lints` rustc lint`
70+
--> $DIR/deprecated.rs:12:8
71+
|
72+
LL | #[warn(clippy::unknown_clippy_lints)]
73+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
74+
6975
error: lint `clippy::unstable_as_slice` has been removed: ``Vec::as_slice` has been stabilized in 1.7`
7076
--> $DIR/deprecated.rs:1:8
7177
|
7278
LL | #[warn(clippy::unstable_as_slice)]
7379
| ^^^^^^^^^^^^^^^^^^^^^^^^^
7480

75-
error: aborting due to 12 previous errors
81+
error: aborting due to 13 previous errors
7682

0 commit comments

Comments
 (0)