Skip to content

Commit

Permalink
Merge #3161 #3387
Browse files Browse the repository at this point in the history 
3161: New lint: Unknown clippy lints r=phansch a=flip1995

This is the Clippy version of the `rustc` lint `unknown_lints`. The behavior of this lint is pretty much the same.

Before this is merged a small change in the compiler needs to be done: `CheckLintNameResult` needs to be public. See rust-lang/rust#54106

3387: Replace big if/else expression with match r=flip1995 a=mikerite



Co-authored-by: flip1995 <9744647+flip1995@users.noreply.github.com>
Co-authored-by: flip1995 <hello@philkrones.com>
Co-authored-by: Michael Wright <mikerite@lavabit.com>
  • Loading branch information
@bors @flip1995
4 people committed Nov 2, 2018
3 parents 0ad5b9b + 11fea50 + 0a41dfd commit c6302de
Show file tree
Hide file tree
Showing 8 changed files with 181 additions and 65 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
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
View file
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
92 changes: 85 additions & 7 deletions clippy_lints/src/attrs.rs
View file
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,46 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AttrPass {
}
}

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(
&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
View file
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
92 changes: 36 additions & 56 deletions clippy_lints/src/methods/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ use crate::rustc::{declare_tool_lint, lint_array};
use crate::rustc_errors::Applicability;
use crate::syntax::ast;
use crate::syntax::source_map::{BytePos, Span};
use crate::syntax::symbol::LocalInternedString;
use crate::utils::paths;
use crate::utils::sugg;
use crate::utils::{
get_arg_name, get_trait_def_id, implements_trait, in_macro, is_copy, is_expn_of, is_self, is_self_ty,
iter_input_pats, last_path_segment, match_def_path, match_path, match_qpath, match_trait_method, match_type,
match_var, method_chain_args, remove_blocks, return_ty, same_tys, single_segment_path, snippet, snippet_with_macro_callsite, span_lint,
match_var, method_calls, method_chain_args, remove_blocks, return_ty, same_tys, single_segment_path, snippet, snippet_with_macro_callsite, span_lint,
span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth, SpanlessEq,
};
use if_chain::if_chain;
Expand Down Expand Up @@ -790,63 +791,42 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
return;
}

let (method_names, arg_lists) = method_calls(expr, 2);
let method_names: Vec<LocalInternedString> = method_names.iter().map(|s| s.as_str()).collect();
let method_names: Vec<&str> = method_names.iter().map(|s| s.as_ref()).collect();

match method_names.as_slice() {
["unwrap", "get"] => lint_get_unwrap(cx, expr, arg_lists[1], false),
["unwrap", "get_mut"] => lint_get_unwrap(cx, expr, arg_lists[1], true),
["unwrap", ..] => lint_unwrap(cx, expr, arg_lists[0]),
["expect", "ok"] => lint_ok_expect(cx, expr, arg_lists[1]),
["unwrap_or", "map"] => lint_map_unwrap_or(cx, expr, arg_lists[1], arg_lists[0]),
["unwrap_or_else", "map"] => lint_map_unwrap_or_else(cx, expr, arg_lists[1], arg_lists[0]),
["map_or", ..] => lint_map_or_none(cx, expr, arg_lists[0]),
["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]),
["map", "filter"] => lint_filter_map(cx, expr, arg_lists[1], arg_lists[0]),
["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]),
["flat_map", "filter"] => lint_filter_flat_map(cx, expr, arg_lists[1], arg_lists[0]),
["flat_map", "filter_map"] => lint_filter_map_flat_map(cx, expr, arg_lists[1], arg_lists[0]),
["flatten", "map"] => lint_map_flatten(cx, expr, arg_lists[1]),
["is_some", "find"] => lint_search_is_some(cx, expr, "find", arg_lists[1], arg_lists[0]),
["is_some", "position"] => lint_search_is_some(cx, expr, "position", arg_lists[1], arg_lists[0]),
["is_some", "rposition"] => lint_search_is_some(cx, expr, "rposition", arg_lists[1], arg_lists[0]),
["extend", ..] => lint_extend(cx, expr, arg_lists[0]),
["as_ptr", "unwrap"] => lint_cstring_as_ptr(cx, expr, &arg_lists[1][0], &arg_lists[0][0]),
["nth", "iter"] => lint_iter_nth(cx, expr, arg_lists[1], false),
["nth", "iter_mut"] => lint_iter_nth(cx, expr, arg_lists[1], true),
["next", "skip"] => lint_iter_skip_next(cx, expr),
["collect", "cloned"] => lint_iter_cloned_collect(cx, expr, arg_lists[1]),
["as_ref", ..] => lint_asref(cx, expr, "as_ref", arg_lists[0]),
["as_mut", ..] => lint_asref(cx, expr, "as_mut", arg_lists[0]),
["fold", ..] => lint_unnecessary_fold(cx, expr, arg_lists[0]),
["filter_map", ..] => unnecessary_filter_map::lint(cx, expr, arg_lists[0]),
_ => {}
}

match expr.node {
hir::ExprKind::MethodCall(ref method_call, ref method_span, ref args) => {
// Chain calls
// GET_UNWRAP needs to be checked before general `UNWRAP` lints
if let Some(arglists) = method_chain_args(expr, &["get", "unwrap"]) {
lint_get_unwrap(cx, expr, arglists[0], false);
} else if let Some(arglists) = method_chain_args(expr, &["get_mut", "unwrap"]) {
lint_get_unwrap(cx, expr, arglists[0], true);
} else if let Some(arglists) = method_chain_args(expr, &["unwrap"]) {
lint_unwrap(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["ok", "expect"]) {
lint_ok_expect(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or"]) {
lint_map_unwrap_or(cx, expr, arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or_else"]) {
lint_map_unwrap_or_else(cx, expr, arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["map_or"]) {
lint_map_or_none(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["filter", "next"]) {
lint_filter_next(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["filter", "map"]) {
lint_filter_map(cx, expr, arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["filter_map", "map"]) {
lint_filter_map_map(cx, expr, arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["filter", "flat_map"]) {
lint_filter_flat_map(cx, expr, arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["filter_map", "flat_map"]) {
lint_filter_map_flat_map(cx, expr, arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["map", "flatten"]) {
lint_map_flatten(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["find", "is_some"]) {
lint_search_is_some(cx, expr, "find", arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["position", "is_some"]) {
lint_search_is_some(cx, expr, "position", arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["rposition", "is_some"]) {
lint_search_is_some(cx, expr, "rposition", arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["extend"]) {
lint_extend(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["unwrap", "as_ptr"]) {
lint_cstring_as_ptr(cx, expr, &arglists[0][0], &arglists[1][0]);
} else if let Some(arglists) = method_chain_args(expr, &["iter", "nth"]) {
lint_iter_nth(cx, expr, arglists[0], false);
} else if let Some(arglists) = method_chain_args(expr, &["iter_mut", "nth"]) {
lint_iter_nth(cx, expr, arglists[0], true);
} else if method_chain_args(expr, &["skip", "next"]).is_some() {
lint_iter_skip_next(cx, expr);
} else if let Some(arglists) = method_chain_args(expr, &["cloned", "collect"]) {
lint_iter_cloned_collect(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["as_ref"]) {
lint_asref(cx, expr, "as_ref", arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["as_mut"]) {
lint_asref(cx, expr, "as_mut", arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["fold"]) {
lint_unnecessary_fold(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["filter_map"]) {
unnecessary_filter_map::lint(cx, expr, arglists[0]);
}

lint_or_fun_call(cx, expr, *method_span, &method_call.ident.as_str(), args);
lint_expect_fun_call(cx, expr, *method_span, &method_call.ident.as_str(), args);
Expand Down
25 changes: 24 additions & 1 deletion clippy_lints/src/utils/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use crate::syntax::ast::{self, LitKind};
use crate::syntax::attr;
use crate::syntax::source_map::{Span, DUMMY_SP};
use crate::syntax::errors::DiagnosticBuilder;
use crate::syntax::symbol::keywords;
use crate::syntax::symbol::{keywords, Symbol};

pub mod camel_case;

Expand Down Expand Up @@ -274,6 +274,29 @@ pub fn resolve_node(cx: &LateContext<'_, '_>, qpath: &QPath, id: HirId) -> def::
cx.tables.qpath_def(qpath, id)
}

/// Return the method names and argument list of nested method call expressions that make up
/// `expr`.
pub fn method_calls<'a>(expr: &'a Expr, max_depth: usize) -> (Vec<Symbol>, Vec<&'a [Expr]>) {
let mut method_names = Vec::with_capacity(max_depth);
let mut arg_lists = Vec::with_capacity(max_depth);

let mut current = expr;
for _ in 0..max_depth {
if let ExprKind::MethodCall(path, _, args) = &current.node {
if args.iter().any(|e| in_macro(e.span)) {
break;
}
method_names.push(path.ident.name);
arg_lists.push(&**args);
current = &args[0];
} else {
break;
}
}

(method_names, arg_lists)
}

/// Match an `Expr` against a chain of methods, and return the matched `Expr`s.
///
/// For example, if `expr` represents the `.baz()` in `foo.bar().baz()`,
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/unknown_clippy_lints.rs
View file
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.

#![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
View file
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:5:8
|
5 | #[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:2:10
|
2 | #![allow(clippy::All)]
| ^^^^^^^^^^^ help: lowercase the lint name: `all`

error: aborting due to 2 previous errors

0 comments on commit c6302de

Please sign in to comment.