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

New lint: Unknown clippy lints #3161

Merged
merged 9 commits into from
Nov 2, 2018
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -866,6 +866,7 @@ All notable changes to this project will be documented in this file.
[`unimplemented`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unimplemented
[`unit_arg`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unit_arg
[`unit_cmp`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unit_cmp
[`unknown_clippy_lints`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unknown_clippy_lints
[`unnecessary_cast`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unnecessary_cast
[`unnecessary_filter_map`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unnecessary_filter_map
[`unnecessary_fold`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unnecessary_fold
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ We are currently in the process of discussing Clippy 1.0 via the RFC process in

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 283 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
[There are 284 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
93 changes: 86 additions & 7 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,20 @@

use crate::reexport::*;
use crate::utils::{
in_macro, last_line_of_span, match_def_path, opt_def_id, paths, snippet_opt, span_lint, span_lint_and_then,
without_block_comments,
in_macro, last_line_of_span, match_def_path, opt_def_id, paths, snippet_opt, span_lint,
span_lint_and_then, without_block_comments,
};
use crate::rustc::hir::*;
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use crate::rustc::{declare_tool_lint, lint_array};
use if_chain::if_chain;
use crate::rustc::hir::*;
use crate::rustc::lint::{
CheckLintNameResult, LateContext, LateLintPass, LintArray, LintContext, LintPass,
};
use crate::rustc::ty::{self, TyCtxt};
use crate::rustc::{declare_tool_lint, lint_array};
use semver::Version;
use crate::syntax::ast::{AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem, NestedMetaItemKind};
use crate::syntax::ast::{
AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem, NestedMetaItemKind,
};
use crate::syntax::source_map::Span;
use crate::rustc_errors::Applicability;

Expand Down Expand Up @@ -138,6 +142,33 @@ declare_clippy_lint! {
"empty line after outer attribute"
}

/// **What it does:** Checks for `allow`/`warn`/`deny`/`forbid` attributes with scoped clippy
/// lints and if those lints exist in clippy. If there is a 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)]
/// ```
declare_clippy_lint! {
pub UNKNOWN_CLIPPY_LINTS,
style,
"unknown_lints for scoped Clippy lints"
}

#[derive(Copy, Clone)]
pub struct AttrPass;

Expand All @@ -147,14 +178,21 @@ impl LintPass for AttrPass {
INLINE_ALWAYS,
DEPRECATED_SEMVER,
USELESS_ATTRIBUTE,
EMPTY_LINE_AFTER_OUTER_ATTR
EMPTY_LINE_AFTER_OUTER_ATTR,
UNKNOWN_CLIPPY_LINTS,
)
}
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AttrPass {
fn check_attribute(&mut self, cx: &LateContext<'a, 'tcx>, attr: &'tcx Attribute) {
if let Some(ref items) = attr.meta_item_list() {
match &*attr.name().as_str() {
"allow" | "warn" | "deny" | "forbid" => {
check_clippy_lint_names(cx, items);
}
_ => {}
}
if items.is_empty() || attr.name() != "deprecated" {
return;
}
Expand Down Expand Up @@ -247,6 +285,47 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AttrPass {
}
}

#[allow(clippy::single_match_else)]
fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
let lint_store = cx.lints();
for lint in items {
if_chain! {
if let Some(word) = lint.word();
if let Some(tool_name) = word.is_scoped();
if tool_name.as_str() == "clippy";
let name = word.name();
if let CheckLintNameResult::Tool(Err((None, _))) = lint_store.check_lint_name(
&name.as_str(),
Some(tool_name.as_str()),
);
then {
span_lint_and_then(
cx,
UNKNOWN_CLIPPY_LINTS,
lint.span,
&format!("unknown clippy lint: clippy::{}", name),
|db| {
if name.as_str().chars().any(|c| c.is_uppercase()) {
let name_lower = name.as_str().to_lowercase().to_string();
match lint_store.check_lint_name(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing there's currently no way to manually iterate the list and do a levensthein search for close matches?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so. This is the behavior of the unknown_lints lint of the compiler. This could be an enhancement for this lint AND the compiler lint. (I'm not sure if the performance of this would be bad though)

Copy link
Contributor

Choose a reason for hiding this comment

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

We do have some tricks like checking whether a lint is allowed before doing the heavy work. But yea, we should fix this in the compiler first

Copy link
Member Author

Choose a reason for hiding this comment

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

There is lev_distance in the compiler.

Copy link
Member

Choose a reason for hiding this comment

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

I opened rust-lang/rust#54737 for improving the rustc unknown_lints suggestion.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure when I get around to work on Rust's unknown_lints, I'm fine with merging this and creating an issue to improve the suggestions later on.

&name_lower,
Some(tool_name.as_str())
) {
CheckLintNameResult::NoLint => (),
_ => {
db.span_suggestion(lint.span,
"lowercase the lint name",
name_lower);
}
}
}
}
);
}
};
}
}

fn is_relevant_item(tcx: TyCtxt<'_, '_, '_>, item: &Item) -> bool {
if let ItemKind::Fn(_, _, _, eid) = item.node {
is_relevant_expr(tcx, tcx.body_tables(eid), &tcx.hir.body(eid).value)
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
assign_ops::ASSIGN_OP_PATTERN,
assign_ops::MISREFACTORED_ASSIGN_OP,
attrs::DEPRECATED_SEMVER,
attrs::UNKNOWN_CLIPPY_LINTS,
attrs::USELESS_ATTRIBUTE,
bit_mask::BAD_BIT_MASK,
bit_mask::INEFFECTIVE_BIT_MASK,
Expand Down Expand Up @@ -749,6 +750,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {

reg.register_lint_group("clippy::style", Some("clippy_style"), vec![
assign_ops::ASSIGN_OP_PATTERN,
attrs::UNKNOWN_CLIPPY_LINTS,
bit_mask::VERBOSE_BIT_MASK,
blacklisted_name::BLACKLISTED_NAME,
block_in_if_condition::BLOCK_IN_IF_CONDITION_EXPR,
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/unknown_clippy_lints.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

Copy link
Member

Choose a reason for hiding this comment

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

Should this test also the old cfg_attr attributes?

Copy link
Member Author

@flip1995 flip1995 Nov 1, 2018

Choose a reason for hiding this comment

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

I would recommend to explicitly write a lint for cfg_attr. I actually already did: flip1995@1f9c0a9

The problem is, that it's impossible to lint crate-level cfg_attrs, which makes this lint only semi valuable.

#![allow(clippy::All)]
#![warn(clippy::pedantic)]

#[warn(clippy::if_not_els)]
fn main() {

}
16 changes: 16 additions & 0 deletions tests/ui/unknown_clippy_lints.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: unknown clippy lint: clippy::if_not_els
--> $DIR/unknown_clippy_lints.rs:13:8
|
13 | #[warn(clippy::if_not_els)]
| ^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::unknown-clippy-lints` implied by `-D warnings`

error: unknown clippy lint: clippy::All
--> $DIR/unknown_clippy_lints.rs:10:10
|
10 | #![allow(clippy::All)]
| ^^^^^^^^^^^ help: lowercase the lint name: `all`

error: aborting due to 2 previous errors