Merge #3161 #3387 #3388

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



3388: RIIR update lints: Generate deprecated lints r=phansch a=phansch

The update script now also generates the 'register_removed' section in
`clippy_lints/src/lib.rs`.

Also, instead of using `let mut store ...`, I added a new identifier
line so that the replacement will continue to work in case `let mut
store ...` ever changes.

cc #2882

Co-authored-by: flip1995 <[email protected]>
Co-authored-by: flip1995 <[email protected]>
Co-authored-by: Michael Wright <[email protected]>
Co-authored-by: Philipp Hansch <[email protected]>

Author: 4 people

CHANGELOG.md

@@ -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

README.md

@@ -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:
 

clippy_dev/src/lib.rs

@@ -72,14 +72,34 @@ impl Lint {
     }
 }
 
+/// Generates the list of lint links at the bottom of the README
 pub fn gen_changelog_lint_list(lints: Vec<Lint>) -> Vec<String> {
     let mut lint_list_sorted: Vec<Lint> = lints;
     lint_list_sorted.sort_by_key(|l| l.name.clone());
     lint_list_sorted
         .iter()
-        .filter(|l| !l.is_internal())
-        .map(|l| {
-            format!("[`{}`]: {}#{}", l.name, DOCS_LINK.clone(), l.name)
+        .filter_map(|l| {
+            if l.is_internal() {
+                None
+            } else {
+                Some(format!("[`{}`]: {}#{}", l.name, DOCS_LINK.clone(), l.name))
+            }
+        }).collect()
+}
+
+/// Generates the `register_removed` code in `./clippy_lints/src/lib.rs`.
+pub fn gen_deprecated(lints: &[Lint]) -> Vec<String> {
+    lints.iter()
+        .filter_map(|l| {
+            l.clone().deprecation.and_then(|depr_text| {
+                Some(
+                    format!(
+                        "    store.register_removed(\n        \"{}\",\n        \"{}\",\n    );",
+                        l.name,
+                        depr_text
+                    )
+                )
+            })
         })
         .collect()
 }
@@ -321,3 +341,18 @@ fn test_gen_changelog_lint_list() {
     ];
     assert_eq!(expected, gen_changelog_lint_list(lints));
 }
+
+#[test]
+fn test_gen_deprecated() {
+    let lints = vec![
+        Lint::new("should_assert_eq", "group1", "abc", Some("has been superseeded by should_assert_eq2"), "module_name"),
+        Lint::new("should_assert_eq2", "group2", "abc", None, "module_name")
+    ];
+    let expected: Vec<String> = vec![
+        r#"    store.register_removed(
+        "should_assert_eq",
+        "has been superseeded by should_assert_eq2",
+    );"#.to_string()
+    ];
+    assert_eq!(expected, gen_deprecated(&lints));
+}

clippy_dev/src/main.rs

@@ -82,4 +82,12 @@ fn update_lints() {
         false,
         || { gen_changelog_lint_list(lint_list.clone()) }
     );
+
+    replace_region_in_file(
+        "../clippy_lints/src/lib.rs",
+        "begin deprecated lints",
+        "end deprecated lints",
+        false,
+        || { gen_deprecated(&lint_list) }
+    );
 }

clippy_lints/src/attrs.rs

@@ -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;
 
@@ -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;
 
@@ -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;
             }
@@ -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)

clippy_lints/src/lib.rs

@@ -271,6 +271,7 @@ pub fn read_conf(reg: &rustc_plugin::Registry<'_>) -> Conf {
 #[rustfmt::skip]
 pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     let mut store = reg.sess.lint_store.borrow_mut();
+    // begin deprecated lints, do not remove this comment, it’s used in `update_lints`
     store.register_removed(
         "should_assert_eq",
         "`assert!()` will be more flexible with RFC 2011",
@@ -532,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,
@@ -748,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,

clippy_lints/src/methods/mod.rs

@@ -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;
@@ -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);

clippy_lints/src/utils/mod.rs

@@ -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;
 
@@ -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()`,