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

Don't make tools responsible for checking unknown and renamed lints #80524

Merged
merged 4 commits into from
Jan 17, 2021
Merged
Show file tree
Hide file tree
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
45 changes: 31 additions & 14 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,10 +354,23 @@ impl LintStore {
lint_name.to_string()
};
// If the lint was scoped with `tool::` check if the tool lint exists
if tool_name.is_some() {
if let Some(tool_name) = tool_name {
match self.by_name.get(&complete_name) {
None => match self.lint_groups.get(&*complete_name) {
None => return CheckLintNameResult::Tool(Err((None, String::new()))),
// If the lint isn't registered, there are two possibilities:
None => {
// 1. The tool is currently running, so this lint really doesn't exist.
// FIXME: should this handle tools that never register a lint, like rustfmt?
tracing::debug!("lints={:?}", self.by_name.keys().collect::<Vec<_>>());
let tool_prefix = format!("{}::", tool_name);
return if self.by_name.keys().any(|lint| lint.starts_with(&tool_prefix)) {
self.no_lint_suggestion(&complete_name)
} else {
// 2. The tool isn't currently running, so no lints will be registered.
// To avoid giving a false positive, ignore all unknown lints.
CheckLintNameResult::Tool(Err((None, String::new())))
};
}
Some(LintGroup { lint_ids, .. }) => {
return CheckLintNameResult::Tool(Ok(&lint_ids));
}
Expand Down Expand Up @@ -398,6 +411,21 @@ impl LintStore {
}
}

fn no_lint_suggestion(&self, lint_name: &str) -> CheckLintNameResult<'_> {
let name_lower = lint_name.to_lowercase();
let symbols =
self.get_lints().iter().map(|l| Symbol::intern(&l.name_lower())).collect::<Vec<_>>();

if lint_name.chars().any(char::is_uppercase) && self.find_lints(&name_lower).is_ok() {
// First check if the lint name is (partly) in upper case instead of lower case...
CheckLintNameResult::NoLint(Some(Symbol::intern(&name_lower)))
} else {
// ...if not, search for lints with a similar name
let suggestion = find_best_match_for_name(&symbols, Symbol::intern(&name_lower), None);
CheckLintNameResult::NoLint(suggestion)
}
}

fn check_tool_name_for_backwards_compat(
&self,
lint_name: &str,
Expand All @@ -407,18 +435,7 @@ impl LintStore {
match self.by_name.get(&complete_name) {
None => match self.lint_groups.get(&*complete_name) {
// Now we are sure, that this lint exists nowhere
None => {
let symbols =
self.by_name.keys().map(|name| Symbol::intern(&name)).collect::<Vec<_>>();

let suggestion = find_best_match_for_name(
&symbols,
Symbol::intern(&lint_name.to_lowercase()),
None,
);

CheckLintNameResult::NoLint(suggestion)
}
None => self.no_lint_suggestion(lint_name),
Some(LintGroup { lint_ids, depr, .. }) => {
// Reaching this would be weird, but let's cover this case anyway
if let Some(LintAlias { name, silent }) = depr {
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,11 @@ impl<'s> LintLevelsBuilder<'s> {
src,
Some(li.span().into()),
|lint| {
let name = if let Some(tool_name) = tool_name {
format!("{}::{}", tool_name, name)
} else {
name.to_string()
};
let mut db = lint.build(&format!("unknown lint: `{}`", name));
if let Some(suggestion) = suggestion {
db.span_suggestion(
Expand Down
27 changes: 24 additions & 3 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
//! compiler code, rather than using their own custom pass. Those
//! lints are all available in `rustc_lint::builtin`.

use crate::{declare_lint, declare_lint_pass, declare_tool_lint};
use crate::{declare_lint, declare_lint_pass};
use rustc_span::edition::Edition;
use rustc_span::symbol::sym;

Expand Down Expand Up @@ -2825,8 +2825,29 @@ declare_lint! {
};
}

declare_tool_lint! {
pub rustc::INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
declare_lint! {
/// The `ineffective_unstable_trait_impl` lint detects `#[unstable]` attributes which are not used.
///
/// ### Example
///
/// ```compile_fail
/// #![feature(staged_api)]
///
/// #[derive(Clone)]
/// #[stable(feature = "x", since = "1")]
/// struct S {}
///
/// #[unstable(feature = "y", issue = "none")]
/// impl Copy for S {}
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// `staged_api` does not currently support using a stability attribute on `impl` blocks.
/// `impl`s are always stable if both the type and trait are stable, and always unstable otherwise.
pub INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
Deny,
"detects `#[unstable]` on stable trait implementations for stable types"
}
Expand Down
6 changes: 4 additions & 2 deletions library/alloc/src/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ pub trait Wake {
}
}

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

#[allow(rustc::ineffective_unstable_trait_impl)]
#[cfg_attr(bootstrap, allow(rustc::ineffective_unstable_trait_impl))]
#[cfg_attr(not(bootstrap), allow(ineffective_unstable_trait_impl))]
#[unstable(feature = "wake_trait", issue = "69912")]
impl<W: Wake + Send + Sync + 'static> From<Arc<W>> for RawWaker {
fn from(waker: Arc<W>) -> RawWaker {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl StableTrait for UnstableType {}
impl UnstableTrait for StableType {}

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

fn main() {}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error: an `#[unstable]` annotation here has no effect
LL | #[unstable(feature = "x", issue = "none")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[deny(rustc::ineffective_unstable_trait_impl)]` on by default
= note: `#[deny(ineffective_unstable_trait_impl)]` on by default
= note: see issue #55436 <https://github.com/rust-lang/rust/issues/55436> for more information

error: aborting due to previous error
Expand Down
72 changes: 2 additions & 70 deletions src/tools/clippy/clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@ use rustc_errors::Applicability;
use rustc_hir::{
Block, Expr, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, StmtKind, TraitFn, TraitItem, TraitItemKind,
};
use rustc_lint::{CheckLintNameResult, EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::lev_distance::find_best_match_for_name;
use rustc_span::source_map::Span;
use rustc_span::sym;
use rustc_span::symbol::{Symbol, SymbolStr};
Expand Down Expand Up @@ -156,33 +155,6 @@ declare_clippy_lint! {
"empty line after outer attribute"
}

declare_clippy_lint! {
/// **What it does:** Checks for `allow`/`warn`/`deny`/`forbid` attributes with scoped clippy
/// lints and if those lints exist in clippy. If there is an uppercase letter in the lint name
/// (not the tool name) and a lowercase version of this lint exists, it will suggest to lowercase
/// the lint name.
///
/// **Why is this bad?** A lint attribute with a mistyped lint name won't have an effect.
///
/// **Known problems:** None.
///
/// **Example:**
/// Bad:
/// ```rust
/// #![warn(if_not_els)]
/// #![deny(clippy::All)]
/// ```
///
/// Good:
/// ```rust
/// #![warn(if_not_else)]
/// #![deny(clippy::all)]
/// ```
pub UNKNOWN_CLIPPY_LINTS,
style,
"unknown_lints for scoped Clippy lints"
}

declare_clippy_lint! {
/// **What it does:** Checks for `warn`/`deny`/`forbid` attributes targeting the whole clippy::restriction category.
///
Expand Down Expand Up @@ -272,7 +244,6 @@ declare_lint_pass!(Attributes => [
INLINE_ALWAYS,
DEPRECATED_SEMVER,
USELESS_ATTRIBUTE,
UNKNOWN_CLIPPY_LINTS,
BLANKET_CLIPPY_RESTRICTION_LINTS,
]);

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

fn check_clippy_lint_names(cx: &LateContext<'_>, ident: &str, items: &[NestedMetaItem]) {
let lint_store = cx.lints();
for lint in items {
if let Some(lint_name) = extract_clippy_lint(lint) {
if let CheckLintNameResult::Tool(Err((None, _))) = lint_store.check_lint_name(&lint_name, Some(sym::clippy))
{
span_lint_and_then(
cx,
UNKNOWN_CLIPPY_LINTS,
lint.span(),
&format!("unknown clippy lint: clippy::{}", lint_name),
|diag| {
let name_lower = lint_name.to_lowercase();
let symbols = lint_store
.get_lints()
.iter()
.map(|l| Symbol::intern(&l.name_lower()))
.collect::<Vec<_>>();
let sugg = find_best_match_for_name(
&symbols,
Symbol::intern(&format!("clippy::{}", name_lower)),
None,
);
if lint_name.chars().any(char::is_uppercase)
&& lint_store.find_lints(&format!("clippy::{}", name_lower)).is_ok()
{
diag.span_suggestion(
lint.span(),
"lowercase the lint name",
format!("clippy::{}", name_lower),
Applicability::MachineApplicable,
);
} else if let Some(sugg) = sugg {
diag.span_suggestion(
lint.span(),
"did you mean",
sugg.to_string(),
Applicability::MachineApplicable,
);
}
},
);
} else if lint_name == "restriction" && ident != "allow" {
if lint_name == "restriction" && ident != "allow" {
span_lint_and_help(
cx,
BLANKET_CLIPPY_RESTRICTION_LINTS,
Expand Down
13 changes: 13 additions & 0 deletions src/tools/clippy/clippy_lints/src/deprecated_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,19 @@ declare_deprecated_lint! {
}

declare_deprecated_lint! {
/// **What it does:** Nothing. This lint has been deprecated.
///
/// **Deprecation reason:** This lint has been uplifted to rustc and is now called
/// `panic_fmt`.
pub PANIC_PARAMS,
"this lint has been uplifted to rustc and is now called `panic_fmt`"
}

declare_deprecated_lint! {
/// **What it does:** Nothing. This lint has been deprecated.
///
/// **Deprecation reason:** This lint has been integrated into the `unknown_lints`
/// rustc lint.
pub UNKNOWN_CLIPPY_LINTS,
"this lint has been integrated into the `unknown_lints` rustc lint"
}
7 changes: 4 additions & 3 deletions src/tools/clippy/clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
"clippy::panic_params",
"this lint has been uplifted to rustc and is now called `panic_fmt`",
);
store.register_removed(
"clippy::unknown_clippy_lints",
"this lint has been integrated into the `unknown_lints` rustc lint",
);
// end deprecated lints, do not remove this comment, it’s used in `update_lints`

// begin register lints, do not remove this comment, it’s used in `update_lints`
Expand Down Expand Up @@ -541,7 +545,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&attrs::EMPTY_LINE_AFTER_OUTER_ATTR,
&attrs::INLINE_ALWAYS,
&attrs::MISMATCHED_TARGET_OS,
&attrs::UNKNOWN_CLIPPY_LINTS,
&attrs::USELESS_ATTRIBUTE,
&await_holding_invalid::AWAIT_HOLDING_LOCK,
&await_holding_invalid::AWAIT_HOLDING_REFCELL_REF,
Expand Down Expand Up @@ -1375,7 +1378,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&attrs::DEPRECATED_CFG_ATTR),
LintId::of(&attrs::DEPRECATED_SEMVER),
LintId::of(&attrs::MISMATCHED_TARGET_OS),
LintId::of(&attrs::UNKNOWN_CLIPPY_LINTS),
LintId::of(&attrs::USELESS_ATTRIBUTE),
LintId::of(&bit_mask::BAD_BIT_MASK),
LintId::of(&bit_mask::INEFFECTIVE_BIT_MASK),
Expand Down Expand Up @@ -1650,7 +1652,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&assertions_on_constants::ASSERTIONS_ON_CONSTANTS),
LintId::of(&assign_ops::ASSIGN_OP_PATTERN),
LintId::of(&attrs::BLANKET_CLIPPY_RESTRICTION_LINTS),
LintId::of(&attrs::UNKNOWN_CLIPPY_LINTS),
LintId::of(&blacklisted_name::BLACKLISTED_NAME),
LintId::of(&blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS),
LintId::of(&collapsible_if::COLLAPSIBLE_IF),
Expand Down
1 change: 1 addition & 0 deletions src/tools/clippy/tests/ui/deprecated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@
#[warn(clippy::drop_bounds)]
#[warn(clippy::temporary_cstring_as_ptr)]
#[warn(clippy::panic_params)]
#[warn(clippy::unknown_clippy_lints)]

fn main() {}
8 changes: 7 additions & 1 deletion src/tools/clippy/tests/ui/deprecated.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,17 @@ error: lint `clippy::panic_params` has been removed: `this lint has been uplifte
LL | #[warn(clippy::panic_params)]
| ^^^^^^^^^^^^^^^^^^^^

error: lint `clippy::unknown_clippy_lints` has been removed: `this lint has been integrated into the `unknown_lints` rustc lint`
--> $DIR/deprecated.rs:12:8
|
LL | #[warn(clippy::unknown_clippy_lints)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: lint `clippy::unstable_as_slice` has been removed: ``Vec::as_slice` has been stabilized in 1.7`
--> $DIR/deprecated.rs:1:8
|
LL | #[warn(clippy::unstable_as_slice)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 12 previous errors
error: aborting due to 13 previous errors

Loading