Merge commit '2b030eb03d9e5837440b1ee0b98c50b97c0c5889' into clippyup

CHANGELOG.md

@@ -5463,6 +5463,7 @@ Released 2018-09-13
 [`string_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_to_string
 [`strlen_on_c_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#strlen_on_c_strings
 [`struct_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#struct_excessive_bools
+[`struct_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#struct_field_names
 [`stutter`]: https://rust-lang.github.io/rust-clippy/master/index.html#stutter
 [`suboptimal_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#suboptimal_flops
 [`suspicious_arithmetic_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_arithmetic_impl
@@ -5625,6 +5626,7 @@ Released 2018-09-13
 [`single-char-binding-names-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#single-char-binding-names-threshold
 [`too-large-for-stack`]: https://doc.rust-lang.org/clippy/lint_configuration.html#too-large-for-stack
 [`enum-variant-name-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enum-variant-name-threshold
+[`struct-field-name-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#struct-field-name-threshold
 [`enum-variant-size-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enum-variant-size-threshold
 [`verbose-bit-mask-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#verbose-bit-mask-threshold
 [`literal-representation-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#literal-representation-threshold

Cargo.toml

@@ -29,7 +29,7 @@ color-print = "0.3.4" # Sync version with Cargo
 anstream = "0.5.0"
 
 [dev-dependencies]
-ui_test = "0.20"
+ui_test = "0.21.2"
 tester = "0.9"
 regex = "1.5"
 toml = "0.7.3"

book/src/development/adding_lints.md

@@ -30,6 +30,7 @@ because that's clearly a non-descriptive name.
   - [Documentation](#documentation)
   - [Running rustfmt](#running-rustfmt)
   - [Debugging](#debugging)
+  - [Conflicting lints](#conflicting-lints)
   - [PR Checklist](#pr-checklist)
   - [Adding configuration to a lint](#adding-configuration-to-a-lint)
   - [Cheat Sheet](#cheat-sheet)
@@ -612,6 +613,24 @@ output in the `stdout` part.
 
 [`dbg!`]: https://doc.rust-lang.org/std/macro.dbg.html
 
+## Conflicting lints
+
+There are several lints that deal with the same pattern but suggest different approaches. In other words, some lints
+may suggest modifications that go in the opposite direction to what some other lints already propose for the same
+code, creating conflicting diagnostics.
+
+When you are creating a lint that ends up in this scenario, the following tips should be encouraged to guide
+classification:
+
+* The only case where they should be in the same category is if that category is `restriction`. For example,
+`semicolon_inside_block` and `semicolon_outside_block`.
+* For all the other cases, they should be in different categories with different levels of allowance. For example,
+`implicit_return` (restriction, allow) and `needless_return` (style, warn).
+
+For lints that are in different categories, it is also recommended that at least one of them should be in the
+`restriction` category. The reason for this is that the `restriction` group is the only group where we don't
+recommend to enable the entire set, but cherry pick lints out of.
+
 ## PR Checklist
 
 Before submitting your PR make sure you followed all the basic requirements:

book/src/lint_configuration.md

@@ -100,7 +100,7 @@ Suppress lints whenever the suggested change would cause breakage for other crat
 ## `msrv`
 The minimum rust version that the project supports
 
-**Default Value:** `None` (`Option<String>`)
+**Default Value:** `Msrv { stack: [] }` (`crate::Msrv`)
 
 ---
 **Affected lints:**
@@ -273,6 +273,16 @@ The minimum number of enum variants for the lints about variant names to trigger
 * [`enum_variant_names`](https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names)
 
 
+## `struct-field-name-threshold`
+The minimum number of struct fields for the lints about field names to trigger
+
+**Default Value:** `3` (`u64`)
+
+---
+**Affected lints:**
+* [`struct_variant_names`](https://rust-lang.github.io/rust-clippy/master/index.html#struct_variant_names)
+
+
 ## `enum-variant-size-threshold`
 The maximum size of an enum's variant to avoid box suggestion
 

clippy_lints/src/async_yields_async.rs

@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_hir_and_then;
 use clippy_utils::source::snippet;
 use clippy_utils::ty::implements_trait;
 use rustc_errors::Applicability;
-use rustc_hir::{AsyncCoroutineKind, Body, BodyId, ExprKind, CoroutineKind, QPath};
+use rustc_hir::{AsyncCoroutineKind, Body, BodyId, CoroutineKind, ExprKind, QPath};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 

clippy_lints/src/await_holding_invalid.rs

@@ -287,5 +287,8 @@ fn is_mutex_guard(cx: &LateContext<'_>, def_id: DefId) -> bool {
 }
 
 fn is_refcell_ref(cx: &LateContext<'_>, def_id: DefId) -> bool {
-    matches!(cx.tcx.get_diagnostic_name(def_id), Some(sym::RefCellRef | sym::RefCellRefMut))
+    matches!(
+        cx.tcx.get_diagnostic_name(def_id),
+        Some(sym::RefCellRef | sym::RefCellRefMut)
+    )
 }

clippy_lints/src/box_default.rs

@@ -3,8 +3,9 @@ use clippy_utils::macros::macro_backtrace;
 use clippy_utils::ty::expr_sig;
 use clippy_utils::{get_parent_node, is_default_equivalent, path_def_id};
 use rustc_errors::Applicability;
+use rustc_hir::def::Res;
 use rustc_hir::intravisit::{walk_ty, Visitor};
-use rustc_hir::{def::Res, Block, Expr, ExprKind, Local, Node, QPath, TyKind};
+use rustc_hir::{Block, Expr, ExprKind, Local, Node, QPath, TyKind};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::print::with_forced_trimmed_paths;

clippy_lints/src/declared_lints.rs

@@ -154,9 +154,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::endian_bytes::LITTLE_ENDIAN_BYTES_INFO,
     crate::entry::MAP_ENTRY_INFO,
     crate::enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT_INFO,
-    crate::enum_variants::ENUM_VARIANT_NAMES_INFO,
-    crate::enum_variants::MODULE_INCEPTION_INFO,
-    crate::enum_variants::MODULE_NAME_REPETITIONS_INFO,
     crate::equatable_if_let::EQUATABLE_IF_LET_INFO,
     crate::error_impl_error::ERROR_IMPL_ERROR_INFO,
     crate::escape::BOXED_LOCAL_INFO,
@@ -226,6 +223,10 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::instant_subtraction::UNCHECKED_DURATION_SUBTRACTION_INFO,
     crate::int_plus_one::INT_PLUS_ONE_INFO,
     crate::invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS_INFO,
+    crate::item_name_repetitions::ENUM_VARIANT_NAMES_INFO,
+    crate::item_name_repetitions::MODULE_INCEPTION_INFO,
+    crate::item_name_repetitions::MODULE_NAME_REPETITIONS_INFO,
+    crate::item_name_repetitions::STRUCT_FIELD_NAMES_INFO,
     crate::items_after_statements::ITEMS_AFTER_STATEMENTS_INFO,
     crate::items_after_test_module::ITEMS_AFTER_TEST_MODULE_INFO,
     crate::iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR_INFO,

clippy_lints/src/default_instead_of_iter_empty.rs

@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::source::snippet_with_context;
 use clippy_utils::last_path_segment;
+use clippy_utils::source::snippet_with_context;
 use rustc_errors::Applicability;
 use rustc_hir::{def, Expr, ExprKind, GenericArg, QPath, TyKind};
 use rustc_lint::{LateContext, LateLintPass};

clippy_lints/src/exit.rs

@@ -1,5 +1,5 @@
 use clippy_utils::diagnostics::span_lint;
-use clippy_utils::{is_entrypoint_fn};
+use clippy_utils::is_entrypoint_fn;
 use if_chain::if_chain;
 use rustc_hir::{Expr, ExprKind, Item, ItemKind, Node};
 use rustc_lint::{LateContext, LateLintPass};

clippy_lints/src/from_raw_with_void_ptr.rs

@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_help;
-use clippy_utils::ty::is_c_void;
 use clippy_utils::path_def_id;
+use clippy_utils::ty::is_c_void;
 use rustc_hir::def_id::DefId;
 use rustc_hir::{Expr, ExprKind, QPath};
 use rustc_lint::{LateContext, LateLintPass};

clippy_lints/src/functions/impl_trait_in_params.rs

@@ -1,50 +1,104 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::is_in_test_function;
 
+use rustc_hir as hir;
 use rustc_hir::intravisit::FnKind;
-use rustc_hir::{Body, HirId};
+use rustc_hir::{Body, GenericParam, Generics, HirId, ImplItem, ImplItemKind, TraitItem, TraitItemKind};
 use rustc_lint::LateContext;
-use rustc_span::Span;
+use rustc_span::symbol::Ident;
+use rustc_span::{BytePos, Span};
 
 use super::IMPL_TRAIT_IN_PARAMS;
 
+fn report(
+    cx: &LateContext<'_>,
+    param: &GenericParam<'_>,
+    ident: &Ident,
+    generics: &Generics<'_>,
+    first_param_span: Span,
+) {
+    // No generics with nested generics, and no generics like FnMut(x)
+    span_lint_and_then(
+        cx,
+        IMPL_TRAIT_IN_PARAMS,
+        param.span,
+        "`impl Trait` used as a function parameter",
+        |diag| {
+            if let Some(gen_span) = generics.span_for_param_suggestion() {
+                // If there's already a generic param with the same bound, do not lint **this** suggestion.
+                diag.span_suggestion_with_style(
+                    gen_span,
+                    "add a type parameter",
+                    format!(", {{ /* Generic name */ }}: {}", &param.name.ident().as_str()[5..]),
+                    rustc_errors::Applicability::HasPlaceholders,
+                    rustc_errors::SuggestionStyle::ShowAlways,
+                );
+            } else {
+                diag.span_suggestion_with_style(
+                    Span::new(
+                        first_param_span.lo() - rustc_span::BytePos(1),
+                        ident.span.hi(),
+                        ident.span.ctxt(),
+                        ident.span.parent(),
+                    ),
+                    "add a type parameter",
+                    format!("<{{ /* Generic name */ }}: {}>", &param.name.ident().as_str()[5..]),
+                    rustc_errors::Applicability::HasPlaceholders,
+                    rustc_errors::SuggestionStyle::ShowAlways,
+                );
+            }
+        },
+    );
+}
+
 pub(super) fn check_fn<'tcx>(cx: &LateContext<'_>, kind: &'tcx FnKind<'_>, body: &'tcx Body<'_>, hir_id: HirId) {
-    if cx.tcx.visibility(cx.tcx.hir().body_owner_def_id(body.id())).is_public() && !is_in_test_function(cx.tcx, hir_id)
-    {
-        if let FnKind::ItemFn(ident, generics, _) = kind {
+    if_chain! {
+        if let FnKind::ItemFn(ident, generics, _) = kind;
+        if cx.tcx.visibility(cx.tcx.hir().body_owner_def_id(body.id())).is_public();
+        if !is_in_test_function(cx.tcx, hir_id);
+        then {
             for param in generics.params {
                 if param.is_impl_trait() {
-                    // No generics with nested generics, and no generics like FnMut(x)
-                    span_lint_and_then(
-                        cx,
-                        IMPL_TRAIT_IN_PARAMS,
-                        param.span,
-                        "'`impl Trait` used as a function parameter'",
-                        |diag| {
-                            if let Some(gen_span) = generics.span_for_param_suggestion() {
-                                diag.span_suggestion_with_style(
-                                    gen_span,
-                                    "add a type parameter",
-                                    format!(", {{ /* Generic name */ }}: {}", &param.name.ident().as_str()[5..]),
-                                    rustc_errors::Applicability::HasPlaceholders,
-                                    rustc_errors::SuggestionStyle::ShowAlways,
-                                );
-                            } else {
-                                diag.span_suggestion_with_style(
-                                    Span::new(
-                                        body.params[0].span.lo() - rustc_span::BytePos(1),
-                                        ident.span.hi(),
-                                        ident.span.ctxt(),
-                                        ident.span.parent(),
-                                    ),
-                                    "add a type parameter",
-                                    format!("<{{ /* Generic name */ }}: {}>", &param.name.ident().as_str()[5..]),
-                                    rustc_errors::Applicability::HasPlaceholders,
-                                    rustc_errors::SuggestionStyle::ShowAlways,
-                                );
-                            }
-                        },
-                    );
+                    report(cx, param, ident, generics, body.params[0].span);
+                };
+            }
+        }
+    }
+}
+
+pub(super) fn check_impl_item(cx: &LateContext<'_>, impl_item: &ImplItem<'_>) {
+    if_chain! {
+        if let ImplItemKind::Fn(_, body_id) = impl_item.kind;
+        if let hir::Node::Item(item) = cx.tcx.hir().get_parent(impl_item.hir_id());
+        if let hir::ItemKind::Impl(impl_) = item.kind;
+        if let hir::Impl { of_trait, .. } = *impl_;
+        if of_trait.is_none();
+        let body = cx.tcx.hir().body(body_id);
+        if cx.tcx.visibility(cx.tcx.hir().body_owner_def_id(body.id())).is_public();
+        if !is_in_test_function(cx.tcx, impl_item.hir_id());
+        then {
+            for param in impl_item.generics.params {
+                if param.is_impl_trait() {
+                    report(cx, param, &impl_item.ident, impl_item.generics, body.params[0].span);
+                }
+            }
+        }
+    }
+}
+
+pub(super) fn check_trait_item(cx: &LateContext<'_>, trait_item: &TraitItem<'_>, avoid_breaking_exported_api: bool) {
+    if_chain! {
+        if !avoid_breaking_exported_api;
+        if let TraitItemKind::Fn(_, _) = trait_item.kind;
+        if let hir::Node::Item(item) = cx.tcx.hir().get_parent(trait_item.hir_id());
+        // ^^ (Will always be a trait)
+        if !item.vis_span.is_empty(); // Is public
+        if !is_in_test_function(cx.tcx, trait_item.hir_id());
+        then {
+            for param in trait_item.generics.params {
+                if param.is_impl_trait() {
+                    let sp = trait_item.ident.span.with_hi(trait_item.ident.span.hi() + BytePos(1));
+                    report(cx, param, &trait_item.ident, trait_item.generics, sp.shrink_to_hi());
                 }
             }
         }

clippy_lints/src/functions/mod.rs

@@ -360,18 +360,26 @@ declare_clippy_lint! {
 }
 
 #[derive(Copy, Clone)]
+#[allow(clippy::struct_field_names)]
 pub struct Functions {
     too_many_arguments_threshold: u64,
     too_many_lines_threshold: u64,
     large_error_threshold: u64,
+    avoid_breaking_exported_api: bool,
 }
 
 impl Functions {
-    pub fn new(too_many_arguments_threshold: u64, too_many_lines_threshold: u64, large_error_threshold: u64) -> Self {
+    pub fn new(
+        too_many_arguments_threshold: u64,
+        too_many_lines_threshold: u64,
+        large_error_threshold: u64,
+        avoid_breaking_exported_api: bool,
+    ) -> Self {
         Self {
             too_many_arguments_threshold,
             too_many_lines_threshold,
             large_error_threshold,
+            avoid_breaking_exported_api,
         }
     }
 }
@@ -415,12 +423,14 @@ impl<'tcx> LateLintPass<'tcx> for Functions {
     fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'_>) {
         must_use::check_impl_item(cx, item);
         result::check_impl_item(cx, item, self.large_error_threshold);
+        impl_trait_in_params::check_impl_item(cx, item);
     }
 
     fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) {
         too_many_arguments::check_trait_item(cx, item, self.too_many_arguments_threshold);
         not_unsafe_ptr_arg_deref::check_trait_item(cx, item);
         must_use::check_trait_item(cx, item);
         result::check_trait_item(cx, item, self.large_error_threshold);
+        impl_trait_in_params::check_trait_item(cx, item, self.avoid_breaking_exported_api);
     }
 }