From 8ec9543f13ba0fe6af76816441b7c7d54653b854 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Tue, 10 Jan 2023 00:18:24 +0100 Subject: [PATCH 1/9] Add `impl_trait_param` lint As this is a lint about "style", and a purely cosmetical choice (using `` over `impl Trait`), a lot of other files needed to be allowed this lint. --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + .../src/functions/impl_trait_param.rs | 88 +++++++++++++++++++ clippy_lints/src/functions/mod.rs | 29 ++++++ clippy_utils/src/ast_utils.rs | 7 +- clippy_utils/src/check_proc_macro.rs | 1 + clippy_utils/src/hir_utils.rs | 1 + clippy_utils/src/macros.rs | 2 +- clippy_utils/src/source.rs | 2 +- tests/ui/borrow_box.rs | 2 +- tests/ui/eta.fixed | 3 +- tests/ui/eta.rs | 3 +- tests/ui/eta.stderr | 52 +++++------ tests/ui/impl_trait_param.rs | 15 ++++ tests/ui/impl_trait_param.stderr | 25 ++++++ tests/ui/trait_duplication_in_bounds.fixed | 1 + tests/ui/trait_duplication_in_bounds.rs | 1 + tests/ui/trait_duplication_in_bounds.stderr | 16 ++-- .../trait_duplication_in_bounds_unfixable.rs | 1 + ...ait_duplication_in_bounds_unfixable.stderr | 16 ++-- 20 files changed, 219 insertions(+), 48 deletions(-) create mode 100644 clippy_lints/src/functions/impl_trait_param.rs create mode 100644 tests/ui/impl_trait_param.rs create mode 100644 tests/ui/impl_trait_param.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 30727beb8e7f..e2d971b38bcb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4430,6 +4430,7 @@ Released 2018-09-13 [`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else [`if_then_some_else_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none [`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond +[`impl_trait_param`]: https://rust-lang.github.io/rust-clippy/master/index.html#impl_trait_param [`implicit_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_clone [`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher [`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index c1feabf05aaf..41b04f13721b 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -179,6 +179,7 @@ crate::from_raw_with_void_ptr::FROM_RAW_WITH_VOID_PTR_INFO, crate::from_str_radix_10::FROM_STR_RADIX_10_INFO, crate::functions::DOUBLE_MUST_USE_INFO, + crate::functions::IMPL_TRAIT_PARAM_INFO, crate::functions::MISNAMED_GETTERS_INFO, crate::functions::MUST_USE_CANDIDATE_INFO, crate::functions::MUST_USE_UNIT_INFO, diff --git a/clippy_lints/src/functions/impl_trait_param.rs b/clippy_lints/src/functions/impl_trait_param.rs new file mode 100644 index 000000000000..1e819dc6c82a --- /dev/null +++ b/clippy_lints/src/functions/impl_trait_param.rs @@ -0,0 +1,88 @@ +use clippy_utils::{diagnostics::span_lint_and_then, is_in_test_function}; + +use rustc_hir::{intravisit::FnKind, Body, Generics, HirId}; +use rustc_lint::LateContext; +use rustc_span::Span; + +use super::IMPL_TRAIT_PARAM; + +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 { + for param in generics.params { + if param.is_impl_trait() + && !param.name.ident().as_str().contains('<') + && !param.name.ident().as_str().contains('(') + { + // No generics with nested generics, and no generics like FnMut(x) + span_lint_and_then( + cx, + IMPL_TRAIT_PARAM, + param.span, + &format!("'{}' in the function's parameters", param.name.ident().as_str()), + |diag| { + let next_letter = next_valid_letter(generics); + if let Some(gen_span) = generics.span_for_param_suggestion() { + diag.span_suggestion_with_style( + gen_span, + format!( + "create a generic type here and replace that `{}` with `{}`", + param.name.ident().as_str(), + next_letter + ), + ", T: Trait", + rustc_errors::Applicability::MaybeIncorrect, + rustc_errors::SuggestionStyle::ShowAlways, + ); + } else { + // multispan.push_span_label(param.span, format!("Replace this with `{}`", + // next_letter)); + + 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(), + ), + format!( + "create a generic type here and replace that '{}' with `{}`", + param.name.ident().as_str(), + next_letter + ), + "", + rustc_errors::Applicability::MaybeIncorrect, + rustc_errors::SuggestionStyle::ShowAlways, + ); + } + }, + ); + } + } + } + } +} + +fn next_valid_letter(generics: &Generics<'_>) -> char { + let mut generics_names = Vec::new(); + + generics.params.iter().for_each(|param| { + generics_names.push(param.name.ident().as_str().to_owned()); + }); + + // If T exists, try with U, then with V, and so on... + let mut current_letter = 84u32; // ASCII code for "T" + while generics_names.contains(&String::from(char::from_u32(current_letter).unwrap())) { + current_letter += 1; + if current_letter == 91 { + // ASCII code for "Z" + current_letter = 65; + } else if current_letter == 83 { + // ASCII "S" + current_letter = 97; // "a" + }; + } + + char::from_u32(current_letter).unwrap() +} diff --git a/clippy_lints/src/functions/mod.rs b/clippy_lints/src/functions/mod.rs index 4399c68e130f..cd0df1438df4 100644 --- a/clippy_lints/src/functions/mod.rs +++ b/clippy_lints/src/functions/mod.rs @@ -1,3 +1,4 @@ +mod impl_trait_param; mod misnamed_getters; mod must_use; mod not_unsafe_ptr_arg_deref; @@ -327,6 +328,32 @@ "getter method returning the wrong field" } +declare_clippy_lint! { + /// ### What it does + /// Lints when `impl Trait` is being used in a function's paremeters. + /// ### Why is this bad? + /// Turbofish syntax (`::<>`) cannot be used when `impl Trait` is being used, making `impl Trait` less powerful. Readability may also be a factor. + /// + /// ### Example + /// ```rust + /// trait MyTrait {} + /// fn foo(a: impl MyTrait) { + /// // [...] + /// } + /// ``` + /// Use instead: + /// ```rust + /// trait MyTrait {} + /// fn foo(a: A) { + /// // [...] + /// } + /// ``` + #[clippy::version = "1.68.0"] + pub IMPL_TRAIT_PARAM, + style, + "`impl Trait` is used in the function's parameters" +} + #[derive(Copy, Clone)] pub struct Functions { too_many_arguments_threshold: u64, @@ -354,6 +381,7 @@ pub fn new(too_many_arguments_threshold: u64, too_many_lines_threshold: u64, lar RESULT_UNIT_ERR, RESULT_LARGE_ERR, MISNAMED_GETTERS, + IMPL_TRAIT_PARAM, ]); impl<'tcx> LateLintPass<'tcx> for Functions { @@ -371,6 +399,7 @@ fn check_fn( too_many_lines::check_fn(cx, kind, span, body, self.too_many_lines_threshold); not_unsafe_ptr_arg_deref::check_fn(cx, kind, decl, body, def_id); misnamed_getters::check_fn(cx, kind, decl, body, span); + impl_trait_param::check_fn(cx, &kind, body, hir_id); } fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) { diff --git a/clippy_utils/src/ast_utils.rs b/clippy_utils/src/ast_utils.rs index 9d0263e93be7..62089320ce35 100644 --- a/clippy_utils/src/ast_utils.rs +++ b/clippy_utils/src/ast_utils.rs @@ -2,7 +2,12 @@ //! //! - The `eq_foobar` functions test for semantic equality but ignores `NodeId`s and `Span`s. -#![allow(clippy::similar_names, clippy::wildcard_imports, clippy::enum_glob_use)] +#![allow( + clippy::similar_names, + clippy::wildcard_imports, + clippy::enum_glob_use, + clippy::impl_trait_param +)] use crate::{both, over}; use rustc_ast::ptr::P; diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index 43f0df145f0e..8650f3c68289 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -1,3 +1,4 @@ +#![allow(clippy::impl_trait_param)] //! This module handles checking if the span given is from a proc-macro or not. //! //! Proc-macros are capable of setting the span of every token they output to a few possible spans. diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 2bbe1a19b625..2cd213ed705a 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -1,3 +1,4 @@ +#![allow(clippy::impl_trait_param)] use crate::consts::constant_simple; use crate::macros::macro_backtrace; use crate::source::snippet_opt; diff --git a/clippy_utils/src/macros.rs b/clippy_utils/src/macros.rs index 63dccbf697c2..a85d11f6234c 100644 --- a/clippy_utils/src/macros.rs +++ b/clippy_utils/src/macros.rs @@ -1,4 +1,4 @@ -#![allow(clippy::similar_names)] // `expr` and `expn` +#![allow(clippy::similar_names, clippy::impl_trait_param)] // `expr` and `expn` use crate::source::snippet_opt; use crate::visitors::{for_each_expr, Descend}; diff --git a/clippy_utils/src/source.rs b/clippy_utils/src/source.rs index cd5dcfdaca34..39d00e89551a 100644 --- a/clippy_utils/src/source.rs +++ b/clippy_utils/src/source.rs @@ -1,6 +1,6 @@ //! Utils for extracting, inspecting or transforming source code -#![allow(clippy::module_name_repetitions)] +#![allow(clippy::module_name_repetitions, clippy::impl_trait_param)] use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; diff --git a/tests/ui/borrow_box.rs b/tests/ui/borrow_box.rs index 3b5b6bf4c950..e3e5b8464b77 100644 --- a/tests/ui/borrow_box.rs +++ b/tests/ui/borrow_box.rs @@ -1,6 +1,6 @@ #![deny(clippy::borrowed_box)] #![allow(dead_code, unused_variables)] -#![allow(clippy::uninlined_format_args, clippy::disallowed_names)] +#![allow(clippy::uninlined_format_args, clippy::disallowed_names, clippy::impl_trait_param)] use std::fmt::Display; diff --git a/tests/ui/eta.fixed b/tests/ui/eta.fixed index dc129591eac4..8c19143cb2e0 100644 --- a/tests/ui/eta.fixed +++ b/tests/ui/eta.fixed @@ -7,7 +7,8 @@ clippy::no_effect, clippy::option_map_unit_fn, clippy::redundant_closure_call, - clippy::uninlined_format_args + clippy::uninlined_format_args, + clippy::impl_trait_param )] use std::path::{Path, PathBuf}; diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs index 025fd6a0b7af..10d4924c5992 100644 --- a/tests/ui/eta.rs +++ b/tests/ui/eta.rs @@ -7,7 +7,8 @@ clippy::no_effect, clippy::option_map_unit_fn, clippy::redundant_closure_call, - clippy::uninlined_format_args + clippy::uninlined_format_args, + clippy::impl_trait_param )] use std::path::{Path, PathBuf}; diff --git a/tests/ui/eta.stderr b/tests/ui/eta.stderr index a521fb868607..6ddecc58eaf1 100644 --- a/tests/ui/eta.stderr +++ b/tests/ui/eta.stderr @@ -1,5 +1,5 @@ error: redundant closure - --> $DIR/eta.rs:28:27 + --> $DIR/eta.rs:29:27 | LL | let a = Some(1u8).map(|a| foo(a)); | ^^^^^^^^^^ help: replace the closure with the function itself: `foo` @@ -7,31 +7,31 @@ LL | let a = Some(1u8).map(|a| foo(a)); = note: `-D clippy::redundant-closure` implied by `-D warnings` error: redundant closure - --> $DIR/eta.rs:32:40 + --> $DIR/eta.rs:33:40 | LL | let _: Option> = true.then(|| vec![]); // special case vec! | ^^^^^^^^^ help: replace the closure with `Vec::new`: `std::vec::Vec::new` error: redundant closure - --> $DIR/eta.rs:33:35 + --> $DIR/eta.rs:34:35 | LL | let d = Some(1u8).map(|a| foo((|b| foo2(b))(a))); //is adjusted? | ^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo2` error: redundant closure - --> $DIR/eta.rs:34:26 + --> $DIR/eta.rs:35:26 | LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted | ^^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `below` error: redundant closure - --> $DIR/eta.rs:41:27 + --> $DIR/eta.rs:42:27 | LL | let e = Some(1u8).map(|a| generic(a)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `generic` error: redundant closure - --> $DIR/eta.rs:87:51 + --> $DIR/eta.rs:88:51 | LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo()); | ^^^^^^^^^^^ help: replace the closure with the method itself: `TestStruct::foo` @@ -39,121 +39,121 @@ LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo()); = note: `-D clippy::redundant-closure-for-method-calls` implied by `-D warnings` error: redundant closure - --> $DIR/eta.rs:88:51 + --> $DIR/eta.rs:89:51 | LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.trait_foo()); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `TestTrait::trait_foo` error: redundant closure - --> $DIR/eta.rs:90:42 + --> $DIR/eta.rs:91:42 | LL | let e = Some(&mut vec![1, 2, 3]).map(|v| v.clear()); | ^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::vec::Vec::clear` error: redundant closure - --> $DIR/eta.rs:94:29 + --> $DIR/eta.rs:95:29 | LL | let e = Some("str").map(|s| s.to_string()); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::string::ToString::to_string` error: redundant closure - --> $DIR/eta.rs:95:27 + --> $DIR/eta.rs:96:27 | LL | let e = Some('a').map(|s| s.to_uppercase()); | ^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_uppercase` error: redundant closure - --> $DIR/eta.rs:97:65 + --> $DIR/eta.rs:98:65 | LL | let e: std::vec::Vec = vec!['a', 'b', 'c'].iter().map(|c| c.to_ascii_uppercase()).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_ascii_uppercase` error: redundant closure - --> $DIR/eta.rs:160:22 + --> $DIR/eta.rs:161:22 | LL | requires_fn_once(|| x()); | ^^^^^^ help: replace the closure with the function itself: `x` error: redundant closure - --> $DIR/eta.rs:167:27 + --> $DIR/eta.rs:168:27 | LL | let a = Some(1u8).map(|a| foo_ptr(a)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo_ptr` error: redundant closure - --> $DIR/eta.rs:172:27 + --> $DIR/eta.rs:173:27 | LL | let a = Some(1u8).map(|a| closure(a)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `closure` error: redundant closure - --> $DIR/eta.rs:204:28 + --> $DIR/eta.rs:205:28 | LL | x.into_iter().for_each(|x| add_to_res(x)); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res` error: redundant closure - --> $DIR/eta.rs:205:28 + --> $DIR/eta.rs:206:28 | LL | y.into_iter().for_each(|x| add_to_res(x)); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res` error: redundant closure - --> $DIR/eta.rs:206:28 + --> $DIR/eta.rs:207:28 | LL | z.into_iter().for_each(|x| add_to_res(x)); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `add_to_res` error: redundant closure - --> $DIR/eta.rs:213:21 + --> $DIR/eta.rs:214:21 | LL | Some(1).map(|n| closure(n)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut closure` error: redundant closure - --> $DIR/eta.rs:217:21 + --> $DIR/eta.rs:218:21 | LL | Some(1).map(|n| in_loop(n)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `in_loop` error: redundant closure - --> $DIR/eta.rs:310:18 + --> $DIR/eta.rs:311:18 | LL | takes_fn_mut(|| f()); | ^^^^^^ help: replace the closure with the function itself: `&mut f` error: redundant closure - --> $DIR/eta.rs:313:19 + --> $DIR/eta.rs:314:19 | LL | takes_fn_once(|| f()); | ^^^^^^ help: replace the closure with the function itself: `&mut f` error: redundant closure - --> $DIR/eta.rs:317:26 + --> $DIR/eta.rs:318:26 | LL | move || takes_fn_mut(|| f_used_once()) | ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut f_used_once` error: redundant closure - --> $DIR/eta.rs:329:19 + --> $DIR/eta.rs:330:19 | LL | array_opt.map(|a| a.as_slice()); | ^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8; 3]>::as_slice` error: redundant closure - --> $DIR/eta.rs:332:19 + --> $DIR/eta.rs:333:19 | LL | slice_opt.map(|s| s.len()); | ^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8]>::len` error: redundant closure - --> $DIR/eta.rs:335:17 + --> $DIR/eta.rs:336:17 | LL | ptr_opt.map(|p| p.is_null()); | ^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<*const usize>::is_null` error: redundant closure - --> $DIR/eta.rs:339:17 + --> $DIR/eta.rs:340:17 | LL | dyn_opt.map(|d| d.method_on_dyn()); | ^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `::method_on_dyn` diff --git a/tests/ui/impl_trait_param.rs b/tests/ui/impl_trait_param.rs new file mode 100644 index 000000000000..3bb9cdbaa094 --- /dev/null +++ b/tests/ui/impl_trait_param.rs @@ -0,0 +1,15 @@ +#![allow(unused)] +#![warn(clippy::impl_trait_param)] + +pub trait Trait {} + +// Should warn +pub fn a(_: impl Trait) {} +pub fn c(_: C, _: impl Trait) {} + +// Shouldn't warn + +pub fn b(_: B) {} +fn d(_: D, _: impl Trait) {} + +fn main() {} diff --git a/tests/ui/impl_trait_param.stderr b/tests/ui/impl_trait_param.stderr new file mode 100644 index 000000000000..55dec5d0d99f --- /dev/null +++ b/tests/ui/impl_trait_param.stderr @@ -0,0 +1,25 @@ +error: 'impl Trait' in the function's parameters + --> $DIR/impl_trait_param.rs:7:13 + | +LL | pub fn a(_: impl Trait) {} + | ^^^^^^^^^^ + | + = note: `-D clippy::impl-trait-param` implied by `-D warnings` +help: create a generic type here and replace that 'impl Trait' with `T` + | +LL | pub fn a(_: impl Trait) {} + | ++++++++++ + +error: 'impl Trait' in the function's parameters + --> $DIR/impl_trait_param.rs:8:29 + | +LL | pub fn c(_: C, _: impl Trait) {} + | ^^^^^^^^^^ + | +help: create a generic type here and replace that `impl Trait` with `T` + | +LL | pub fn c(_: C, _: impl Trait) {} + | ++++++++++ + +error: aborting due to 2 previous errors + diff --git a/tests/ui/trait_duplication_in_bounds.fixed b/tests/ui/trait_duplication_in_bounds.fixed index 4ce5d4217822..a587cc8b456f 100644 --- a/tests/ui/trait_duplication_in_bounds.fixed +++ b/tests/ui/trait_duplication_in_bounds.fixed @@ -1,5 +1,6 @@ // run-rustfix #![deny(clippy::trait_duplication_in_bounds)] +#![allow(clippy::impl_trait_param)] #![allow(unused)] fn bad_foo(arg0: T, argo1: U) { diff --git a/tests/ui/trait_duplication_in_bounds.rs b/tests/ui/trait_duplication_in_bounds.rs index 7f2e96a22e66..beb41ea26d96 100644 --- a/tests/ui/trait_duplication_in_bounds.rs +++ b/tests/ui/trait_duplication_in_bounds.rs @@ -1,5 +1,6 @@ // run-rustfix #![deny(clippy::trait_duplication_in_bounds)] +#![allow(clippy::impl_trait_param)] #![allow(unused)] fn bad_foo(arg0: T, argo1: U) { diff --git a/tests/ui/trait_duplication_in_bounds.stderr b/tests/ui/trait_duplication_in_bounds.stderr index af800ba78880..5216b2250dbd 100644 --- a/tests/ui/trait_duplication_in_bounds.stderr +++ b/tests/ui/trait_duplication_in_bounds.stderr @@ -1,5 +1,5 @@ error: these bounds contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:5:15 + --> $DIR/trait_duplication_in_bounds.rs:6:15 | LL | fn bad_foo(arg0: T, argo1: U) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy` @@ -11,43 +11,43 @@ LL | #![deny(clippy::trait_duplication_in_bounds)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: these where clauses contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:11:8 + --> $DIR/trait_duplication_in_bounds.rs:12:8 | LL | T: Clone + Clone + Clone + Copy, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy` error: these bounds contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:39:26 + --> $DIR/trait_duplication_in_bounds.rs:40:26 | LL | trait BadSelfTraitBound: Clone + Clone + Clone { | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone` error: these where clauses contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:46:15 + --> $DIR/trait_duplication_in_bounds.rs:47:15 | LL | Self: Clone + Clone + Clone; | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone` error: these bounds contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:60:24 + --> $DIR/trait_duplication_in_bounds.rs:61:24 | LL | trait BadTraitBound { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy` error: these where clauses contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:67:12 + --> $DIR/trait_duplication_in_bounds.rs:68:12 | LL | T: Clone + Clone + Clone + Copy, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy` error: these bounds contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:100:19 + --> $DIR/trait_duplication_in_bounds.rs:101:19 | LL | fn bad_generic + GenericTrait + GenericTrait>(arg0: T) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `GenericTrait + GenericTrait` error: these bounds contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:108:22 + --> $DIR/trait_duplication_in_bounds.rs:109:22 | LL | fn qualified_path(arg0: T) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::clone::Clone + foo::Clone` diff --git a/tests/ui/trait_duplication_in_bounds_unfixable.rs b/tests/ui/trait_duplication_in_bounds_unfixable.rs index 5630a0345adb..63ae4493143e 100644 --- a/tests/ui/trait_duplication_in_bounds_unfixable.rs +++ b/tests/ui/trait_duplication_in_bounds_unfixable.rs @@ -1,4 +1,5 @@ #![deny(clippy::trait_duplication_in_bounds)] +#![allow(clippy::impl_trait_param)] use std::collections::BTreeMap; use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign}; diff --git a/tests/ui/trait_duplication_in_bounds_unfixable.stderr b/tests/ui/trait_duplication_in_bounds_unfixable.stderr index 4d56a94646cb..48091b38fb41 100644 --- a/tests/ui/trait_duplication_in_bounds_unfixable.stderr +++ b/tests/ui/trait_duplication_in_bounds_unfixable.stderr @@ -1,5 +1,5 @@ error: this trait bound is already specified in the where clause - --> $DIR/trait_duplication_in_bounds_unfixable.rs:6:15 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:7:15 | LL | fn bad_foo(arg0: T, arg1: Z) | ^^^^^ @@ -12,7 +12,7 @@ LL | #![deny(clippy::trait_duplication_in_bounds)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this trait bound is already specified in the where clause - --> $DIR/trait_duplication_in_bounds_unfixable.rs:6:23 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:7:23 | LL | fn bad_foo(arg0: T, arg1: Z) | ^^^^^^^ @@ -20,7 +20,7 @@ LL | fn bad_foo(arg0: T, arg1: Z) = help: consider removing this trait bound error: this trait bound is already specified in trait declaration - --> $DIR/trait_duplication_in_bounds_unfixable.rs:35:15 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:36:15 | LL | Self: Default; | ^^^^^^^ @@ -28,7 +28,7 @@ LL | Self: Default; = help: consider removing this trait bound error: this trait bound is already specified in trait declaration - --> $DIR/trait_duplication_in_bounds_unfixable.rs:49:15 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:50:15 | LL | Self: Default + Clone; | ^^^^^^^ @@ -36,7 +36,7 @@ LL | Self: Default + Clone; = help: consider removing this trait bound error: this trait bound is already specified in trait declaration - --> $DIR/trait_duplication_in_bounds_unfixable.rs:55:15 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:56:15 | LL | Self: Default + Clone; | ^^^^^^^ @@ -44,7 +44,7 @@ LL | Self: Default + Clone; = help: consider removing this trait bound error: this trait bound is already specified in trait declaration - --> $DIR/trait_duplication_in_bounds_unfixable.rs:55:25 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:56:25 | LL | Self: Default + Clone; | ^^^^^ @@ -52,7 +52,7 @@ LL | Self: Default + Clone; = help: consider removing this trait bound error: this trait bound is already specified in trait declaration - --> $DIR/trait_duplication_in_bounds_unfixable.rs:58:15 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:59:15 | LL | Self: Default; | ^^^^^^^ @@ -60,7 +60,7 @@ LL | Self: Default; = help: consider removing this trait bound error: this trait bound is already specified in trait declaration - --> $DIR/trait_duplication_in_bounds_unfixable.rs:93:15 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:94:15 | LL | Self: Iterator, | ^^^^^^^^^^^^^^^^^^^^ From ade4c9b2b6e99e0663408318e41139d65a9d0630 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Fri, 13 Jan 2023 12:17:30 +0100 Subject: [PATCH 2/9] Rename lint to better fit lint naming conventions --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 2 +- .../{impl_trait_param.rs => impl_trait_in_params.rs} | 4 ++-- clippy_lints/src/functions/mod.rs | 8 ++++---- clippy_utils/src/ast_utils.rs | 2 +- clippy_utils/src/check_proc_macro.rs | 2 +- clippy_utils/src/hir_utils.rs | 2 +- clippy_utils/src/macros.rs | 2 +- clippy_utils/src/source.rs | 2 +- tests/ui/borrow_box.rs | 2 +- tests/ui/eta.fixed | 2 +- tests/ui/eta.rs | 2 +- tests/ui/{impl_trait_param.rs => impl_trait_in_params.rs} | 2 +- ...mpl_trait_param.stderr => impl_trait_in_params.stderr} | 0 tests/ui/trait_duplication_in_bounds.fixed | 2 +- tests/ui/trait_duplication_in_bounds.rs | 2 +- tests/ui/trait_duplication_in_bounds_unfixable.rs | 2 +- 17 files changed, 20 insertions(+), 19 deletions(-) rename clippy_lints/src/functions/{impl_trait_param.rs => impl_trait_in_params.rs} (98%) rename tests/ui/{impl_trait_param.rs => impl_trait_in_params.rs} (85%) rename tests/ui/{impl_trait_param.stderr => impl_trait_in_params.stderr} (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2d971b38bcb..db2602285c93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4430,6 +4430,7 @@ Released 2018-09-13 [`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else [`if_then_some_else_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none [`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond +[`impl_trait_in_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#impl_trait_in_params [`impl_trait_param`]: https://rust-lang.github.io/rust-clippy/master/index.html#impl_trait_param [`implicit_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_clone [`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 41b04f13721b..9be4f5bf7fc9 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -179,7 +179,7 @@ crate::from_raw_with_void_ptr::FROM_RAW_WITH_VOID_PTR_INFO, crate::from_str_radix_10::FROM_STR_RADIX_10_INFO, crate::functions::DOUBLE_MUST_USE_INFO, - crate::functions::IMPL_TRAIT_PARAM_INFO, + crate::functions::IMPL_TRAIT_IN_PARAMS_INFO, crate::functions::MISNAMED_GETTERS_INFO, crate::functions::MUST_USE_CANDIDATE_INFO, crate::functions::MUST_USE_UNIT_INFO, diff --git a/clippy_lints/src/functions/impl_trait_param.rs b/clippy_lints/src/functions/impl_trait_in_params.rs similarity index 98% rename from clippy_lints/src/functions/impl_trait_param.rs rename to clippy_lints/src/functions/impl_trait_in_params.rs index 1e819dc6c82a..f0bfbbf1ea3f 100644 --- a/clippy_lints/src/functions/impl_trait_param.rs +++ b/clippy_lints/src/functions/impl_trait_in_params.rs @@ -4,7 +4,7 @@ use rustc_lint::LateContext; use rustc_span::Span; -use super::IMPL_TRAIT_PARAM; +use super::IMPL_TRAIT_IN_PARAMS; 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) @@ -18,7 +18,7 @@ pub(super) fn check_fn<'tcx>(cx: &LateContext<'_>, kind: &'tcx FnKind<'_>, body: // No generics with nested generics, and no generics like FnMut(x) span_lint_and_then( cx, - IMPL_TRAIT_PARAM, + IMPL_TRAIT_IN_PARAMS, param.span, &format!("'{}' in the function's parameters", param.name.ident().as_str()), |diag| { diff --git a/clippy_lints/src/functions/mod.rs b/clippy_lints/src/functions/mod.rs index cd0df1438df4..81b7bfe7181b 100644 --- a/clippy_lints/src/functions/mod.rs +++ b/clippy_lints/src/functions/mod.rs @@ -1,4 +1,4 @@ -mod impl_trait_param; +mod impl_trait_in_params; mod misnamed_getters; mod must_use; mod not_unsafe_ptr_arg_deref; @@ -349,7 +349,7 @@ /// } /// ``` #[clippy::version = "1.68.0"] - pub IMPL_TRAIT_PARAM, + pub IMPL_TRAIT_IN_PARAMS, style, "`impl Trait` is used in the function's parameters" } @@ -381,7 +381,7 @@ pub fn new(too_many_arguments_threshold: u64, too_many_lines_threshold: u64, lar RESULT_UNIT_ERR, RESULT_LARGE_ERR, MISNAMED_GETTERS, - IMPL_TRAIT_PARAM, + IMPL_TRAIT_IN_PARAMS, ]); impl<'tcx> LateLintPass<'tcx> for Functions { @@ -399,7 +399,7 @@ fn check_fn( too_many_lines::check_fn(cx, kind, span, body, self.too_many_lines_threshold); not_unsafe_ptr_arg_deref::check_fn(cx, kind, decl, body, def_id); misnamed_getters::check_fn(cx, kind, decl, body, span); - impl_trait_param::check_fn(cx, &kind, body, hir_id); + impl_trait_in_params::check_fn(cx, &kind, body, hir_id); } fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) { diff --git a/clippy_utils/src/ast_utils.rs b/clippy_utils/src/ast_utils.rs index 62089320ce35..65d287d493b4 100644 --- a/clippy_utils/src/ast_utils.rs +++ b/clippy_utils/src/ast_utils.rs @@ -6,7 +6,7 @@ clippy::similar_names, clippy::wildcard_imports, clippy::enum_glob_use, - clippy::impl_trait_param + clippy::impl_trait_in_params )] use crate::{both, over}; diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index 8650f3c68289..353926074b5d 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -1,4 +1,4 @@ -#![allow(clippy::impl_trait_param)] +#![allow(clippy::impl_trait_in_params)] //! This module handles checking if the span given is from a proc-macro or not. //! //! Proc-macros are capable of setting the span of every token they output to a few possible spans. diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 2cd213ed705a..115d8aafaf77 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -1,4 +1,4 @@ -#![allow(clippy::impl_trait_param)] +#![allow(clippy::impl_trait_in_params)] use crate::consts::constant_simple; use crate::macros::macro_backtrace; use crate::source::snippet_opt; diff --git a/clippy_utils/src/macros.rs b/clippy_utils/src/macros.rs index a85d11f6234c..cac29ac9c015 100644 --- a/clippy_utils/src/macros.rs +++ b/clippy_utils/src/macros.rs @@ -1,4 +1,4 @@ -#![allow(clippy::similar_names, clippy::impl_trait_param)] // `expr` and `expn` +#![allow(clippy::similar_names, clippy::impl_trait_in_params)] // `expr` and `expn` use crate::source::snippet_opt; use crate::visitors::{for_each_expr, Descend}; diff --git a/clippy_utils/src/source.rs b/clippy_utils/src/source.rs index 39d00e89551a..e802941b5d8a 100644 --- a/clippy_utils/src/source.rs +++ b/clippy_utils/src/source.rs @@ -1,6 +1,6 @@ //! Utils for extracting, inspecting or transforming source code -#![allow(clippy::module_name_repetitions, clippy::impl_trait_param)] +#![allow(clippy::module_name_repetitions, clippy::impl_trait_in_params)] use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; diff --git a/tests/ui/borrow_box.rs b/tests/ui/borrow_box.rs index e3e5b8464b77..3dfd8191adff 100644 --- a/tests/ui/borrow_box.rs +++ b/tests/ui/borrow_box.rs @@ -1,6 +1,6 @@ #![deny(clippy::borrowed_box)] #![allow(dead_code, unused_variables)] -#![allow(clippy::uninlined_format_args, clippy::disallowed_names, clippy::impl_trait_param)] +#![allow(clippy::uninlined_format_args, clippy::disallowed_names, clippy::impl_trait_in_params)] use std::fmt::Display; diff --git a/tests/ui/eta.fixed b/tests/ui/eta.fixed index 8c19143cb2e0..c1ad0b66e01b 100644 --- a/tests/ui/eta.fixed +++ b/tests/ui/eta.fixed @@ -8,7 +8,7 @@ clippy::option_map_unit_fn, clippy::redundant_closure_call, clippy::uninlined_format_args, - clippy::impl_trait_param + clippy::impl_trait_in_params )] use std::path::{Path, PathBuf}; diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs index 10d4924c5992..000af653c152 100644 --- a/tests/ui/eta.rs +++ b/tests/ui/eta.rs @@ -8,7 +8,7 @@ clippy::option_map_unit_fn, clippy::redundant_closure_call, clippy::uninlined_format_args, - clippy::impl_trait_param + clippy::impl_trait_in_params )] use std::path::{Path, PathBuf}; diff --git a/tests/ui/impl_trait_param.rs b/tests/ui/impl_trait_in_params.rs similarity index 85% rename from tests/ui/impl_trait_param.rs rename to tests/ui/impl_trait_in_params.rs index 3bb9cdbaa094..8402730d3105 100644 --- a/tests/ui/impl_trait_param.rs +++ b/tests/ui/impl_trait_in_params.rs @@ -1,5 +1,5 @@ #![allow(unused)] -#![warn(clippy::impl_trait_param)] +#![warn(clippy::impl_trait_in_params)] pub trait Trait {} diff --git a/tests/ui/impl_trait_param.stderr b/tests/ui/impl_trait_in_params.stderr similarity index 100% rename from tests/ui/impl_trait_param.stderr rename to tests/ui/impl_trait_in_params.stderr diff --git a/tests/ui/trait_duplication_in_bounds.fixed b/tests/ui/trait_duplication_in_bounds.fixed index a587cc8b456f..373859a25648 100644 --- a/tests/ui/trait_duplication_in_bounds.fixed +++ b/tests/ui/trait_duplication_in_bounds.fixed @@ -1,6 +1,6 @@ // run-rustfix #![deny(clippy::trait_duplication_in_bounds)] -#![allow(clippy::impl_trait_param)] +#![allow(clippy::impl_trait_in_params)] #![allow(unused)] fn bad_foo(arg0: T, argo1: U) { diff --git a/tests/ui/trait_duplication_in_bounds.rs b/tests/ui/trait_duplication_in_bounds.rs index beb41ea26d96..3afd74aa9b2c 100644 --- a/tests/ui/trait_duplication_in_bounds.rs +++ b/tests/ui/trait_duplication_in_bounds.rs @@ -1,6 +1,6 @@ // run-rustfix #![deny(clippy::trait_duplication_in_bounds)] -#![allow(clippy::impl_trait_param)] +#![allow(clippy::impl_trait_in_params)] #![allow(unused)] fn bad_foo(arg0: T, argo1: U) { diff --git a/tests/ui/trait_duplication_in_bounds_unfixable.rs b/tests/ui/trait_duplication_in_bounds_unfixable.rs index 63ae4493143e..7435a4ba609c 100644 --- a/tests/ui/trait_duplication_in_bounds_unfixable.rs +++ b/tests/ui/trait_duplication_in_bounds_unfixable.rs @@ -1,5 +1,5 @@ #![deny(clippy::trait_duplication_in_bounds)] -#![allow(clippy::impl_trait_param)] +#![allow(clippy::impl_trait_in_params)] use std::collections::BTreeMap; use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign}; From 6c9983b936645629d8bf66d2aac9ba5e588f5fb7 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Fri, 13 Jan 2023 12:21:17 +0100 Subject: [PATCH 3/9] Exec `cargo dev update_lints` --- CHANGELOG.md | 1 - tests/ui/borrow_box.rs | 6 +++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db2602285c93..486ffd725c0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4431,7 +4431,6 @@ Released 2018-09-13 [`if_then_some_else_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none [`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond [`impl_trait_in_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#impl_trait_in_params -[`impl_trait_param`]: https://rust-lang.github.io/rust-clippy/master/index.html#impl_trait_param [`implicit_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_clone [`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher [`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return diff --git a/tests/ui/borrow_box.rs b/tests/ui/borrow_box.rs index 3dfd8191adff..8fe160e8e0f5 100644 --- a/tests/ui/borrow_box.rs +++ b/tests/ui/borrow_box.rs @@ -1,6 +1,10 @@ #![deny(clippy::borrowed_box)] #![allow(dead_code, unused_variables)] -#![allow(clippy::uninlined_format_args, clippy::disallowed_names, clippy::impl_trait_in_params)] +#![allow( + clippy::uninlined_format_args, + clippy::disallowed_names, + clippy::impl_trait_in_params +)] use std::fmt::Display; From bdf4fd3e82542cea1f9104cef7bd82839ed84213 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Fri, 13 Jan 2023 12:54:51 +0100 Subject: [PATCH 4/9] Tests pass --- clippy_lints/src/functions/mod.rs | 2 +- tests/ui/borrow_box.stderr | 20 ++++++++++---------- tests/ui/impl_trait_in_params.rs | 4 +++- tests/ui/impl_trait_in_params.stderr | 6 +++--- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/functions/mod.rs b/clippy_lints/src/functions/mod.rs index 81b7bfe7181b..e7f22f8d778c 100644 --- a/clippy_lints/src/functions/mod.rs +++ b/clippy_lints/src/functions/mod.rs @@ -344,7 +344,7 @@ /// Use instead: /// ```rust /// trait MyTrait {} - /// fn foo(a: A) { + /// fn foo(a: T) { /// // [...] /// } /// ``` diff --git a/tests/ui/borrow_box.stderr b/tests/ui/borrow_box.stderr index 99cb60a1ead9..90e752211ff0 100644 --- a/tests/ui/borrow_box.stderr +++ b/tests/ui/borrow_box.stderr @@ -1,5 +1,5 @@ error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:20:14 + --> $DIR/borrow_box.rs:24:14 | LL | let foo: &Box; | ^^^^^^^^^^ help: try: `&bool` @@ -11,55 +11,55 @@ LL | #![deny(clippy::borrowed_box)] | ^^^^^^^^^^^^^^^^^^^^ error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:24:10 + --> $DIR/borrow_box.rs:28:10 | LL | foo: &'a Box, | ^^^^^^^^^^^^^ help: try: `&'a bool` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:28:17 + --> $DIR/borrow_box.rs:32:17 | LL | fn test4(a: &Box); | ^^^^^^^^^^ help: try: `&bool` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:94:25 + --> $DIR/borrow_box.rs:98:25 | LL | pub fn test14(_display: &Box) {} | ^^^^^^^^^^^^^^^^^ help: try: `&dyn Display` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:95:25 + --> $DIR/borrow_box.rs:99:25 | LL | pub fn test15(_display: &Box) {} | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&(dyn Display + Send)` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:96:29 + --> $DIR/borrow_box.rs:100:29 | LL | pub fn test16<'a>(_display: &'a Box) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&'a (dyn Display + 'a)` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:98:25 + --> $DIR/borrow_box.rs:102:25 | LL | pub fn test17(_display: &Box) {} | ^^^^^^^^^^^^^^^^^^ help: try: `&impl Display` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:99:25 + --> $DIR/borrow_box.rs:103:25 | LL | pub fn test18(_display: &Box) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&(impl Display + Send)` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:100:29 + --> $DIR/borrow_box.rs:104:29 | LL | pub fn test19<'a>(_display: &'a Box) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&'a (impl Display + 'a)` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:105:25 + --> $DIR/borrow_box.rs:109:25 | LL | pub fn test20(_display: &Box<(dyn Display + Send)>) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&(dyn Display + Send)` diff --git a/tests/ui/impl_trait_in_params.rs b/tests/ui/impl_trait_in_params.rs index 8402730d3105..07560101a416 100644 --- a/tests/ui/impl_trait_in_params.rs +++ b/tests/ui/impl_trait_in_params.rs @@ -2,14 +2,16 @@ #![warn(clippy::impl_trait_in_params)] pub trait Trait {} +pub trait AnotherTrait {} // Should warn pub fn a(_: impl Trait) {} pub fn c(_: C, _: impl Trait) {} +fn d(_: impl AnotherTrait) {} // Shouldn't warn pub fn b(_: B) {} -fn d(_: D, _: impl Trait) {} +fn e>(_: T) {} fn main() {} diff --git a/tests/ui/impl_trait_in_params.stderr b/tests/ui/impl_trait_in_params.stderr index 55dec5d0d99f..a43986017feb 100644 --- a/tests/ui/impl_trait_in_params.stderr +++ b/tests/ui/impl_trait_in_params.stderr @@ -1,17 +1,17 @@ error: 'impl Trait' in the function's parameters - --> $DIR/impl_trait_param.rs:7:13 + --> $DIR/impl_trait_in_params.rs:8:13 | LL | pub fn a(_: impl Trait) {} | ^^^^^^^^^^ | - = note: `-D clippy::impl-trait-param` implied by `-D warnings` + = note: `-D clippy::impl-trait-in-params` implied by `-D warnings` help: create a generic type here and replace that 'impl Trait' with `T` | LL | pub fn a(_: impl Trait) {} | ++++++++++ error: 'impl Trait' in the function's parameters - --> $DIR/impl_trait_param.rs:8:29 + --> $DIR/impl_trait_in_params.rs:9:29 | LL | pub fn c(_: C, _: impl Trait) {} | ^^^^^^^^^^ From 8a2245dcb6ccf8d9491d6fdc7681a4f34739093c Mon Sep 17 00:00:00 2001 From: blyxyas Date: Fri, 13 Jan 2023 19:41:06 +0100 Subject: [PATCH 5/9] Change lint's from `style` to `restriction` --- clippy_lints/src/functions/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/functions/mod.rs b/clippy_lints/src/functions/mod.rs index e7f22f8d778c..d2852b4acad1 100644 --- a/clippy_lints/src/functions/mod.rs +++ b/clippy_lints/src/functions/mod.rs @@ -350,7 +350,7 @@ /// ``` #[clippy::version = "1.68.0"] pub IMPL_TRAIT_IN_PARAMS, - style, + restriction, "`impl Trait` is used in the function's parameters" } From 6aa06b757dde06b43c198d352c51e1eef82bbb1c Mon Sep 17 00:00:00 2001 From: blyxyas Date: Thu, 19 Jan 2023 16:24:47 +0100 Subject: [PATCH 6/9] Remove `#[allow]`s. Apply conversations from @Jarcho --- .../src/functions/impl_trait_in_params.rs | 23 +++----- clippy_utils/src/ast_utils.rs | 7 +-- clippy_utils/src/check_proc_macro.rs | 1 - clippy_utils/src/hir_utils.rs | 1 - clippy_utils/src/macros.rs | 2 +- clippy_utils/src/source.rs | 2 +- tests/ui/borrow_box.rs | 6 +-- tests/ui/borrow_box.stderr | 20 +++---- tests/ui/eta.fixed | 3 +- tests/ui/eta.rs | 3 +- tests/ui/eta.stderr | 52 +++++++++---------- tests/ui/impl_trait_in_params.stderr | 8 +-- tests/ui/trait_duplication_in_bounds.fixed | 1 - tests/ui/trait_duplication_in_bounds.rs | 1 - tests/ui/trait_duplication_in_bounds.stderr | 16 +++--- .../trait_duplication_in_bounds_unfixable.rs | 1 - ...ait_duplication_in_bounds_unfixable.stderr | 16 +++--- 17 files changed, 68 insertions(+), 95 deletions(-) diff --git a/clippy_lints/src/functions/impl_trait_in_params.rs b/clippy_lints/src/functions/impl_trait_in_params.rs index f0bfbbf1ea3f..ace5932f4e4a 100644 --- a/clippy_lints/src/functions/impl_trait_in_params.rs +++ b/clippy_lints/src/functions/impl_trait_in_params.rs @@ -11,27 +11,20 @@ pub(super) fn check_fn<'tcx>(cx: &LateContext<'_>, kind: &'tcx FnKind<'_>, body: { if let FnKind::ItemFn(ident, generics, _) = kind { for param in generics.params { - if param.is_impl_trait() - && !param.name.ident().as_str().contains('<') - && !param.name.ident().as_str().contains('(') - { + 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, - &format!("'{}' in the function's parameters", param.name.ident().as_str()), + "'`impl Trait` used as a function parameter'", |diag| { let next_letter = next_valid_letter(generics); if let Some(gen_span) = generics.span_for_param_suggestion() { diag.span_suggestion_with_style( gen_span, - format!( - "create a generic type here and replace that `{}` with `{}`", - param.name.ident().as_str(), - next_letter - ), - ", T: Trait", + "add a type paremeter, `{}`: `{}`", + format!(", {next_letter}: {}", ¶m.name.ident().as_str()[5..]), rustc_errors::Applicability::MaybeIncorrect, rustc_errors::SuggestionStyle::ShowAlways, ); @@ -46,12 +39,8 @@ pub(super) fn check_fn<'tcx>(cx: &LateContext<'_>, kind: &'tcx FnKind<'_>, body: ident.span.ctxt(), ident.span.parent(), ), - format!( - "create a generic type here and replace that '{}' with `{}`", - param.name.ident().as_str(), - next_letter - ), - "", + "add a type paremeter", + format!("<{next_letter}: {}>", ¶m.name.ident().as_str()[5..]), rustc_errors::Applicability::MaybeIncorrect, rustc_errors::SuggestionStyle::ShowAlways, ); diff --git a/clippy_utils/src/ast_utils.rs b/clippy_utils/src/ast_utils.rs index 65d287d493b4..9d0263e93be7 100644 --- a/clippy_utils/src/ast_utils.rs +++ b/clippy_utils/src/ast_utils.rs @@ -2,12 +2,7 @@ //! //! - The `eq_foobar` functions test for semantic equality but ignores `NodeId`s and `Span`s. -#![allow( - clippy::similar_names, - clippy::wildcard_imports, - clippy::enum_glob_use, - clippy::impl_trait_in_params -)] +#![allow(clippy::similar_names, clippy::wildcard_imports, clippy::enum_glob_use)] use crate::{both, over}; use rustc_ast::ptr::P; diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index 353926074b5d..43f0df145f0e 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -1,4 +1,3 @@ -#![allow(clippy::impl_trait_in_params)] //! This module handles checking if the span given is from a proc-macro or not. //! //! Proc-macros are capable of setting the span of every token they output to a few possible spans. diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 115d8aafaf77..2bbe1a19b625 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -1,4 +1,3 @@ -#![allow(clippy::impl_trait_in_params)] use crate::consts::constant_simple; use crate::macros::macro_backtrace; use crate::source::snippet_opt; diff --git a/clippy_utils/src/macros.rs b/clippy_utils/src/macros.rs index cac29ac9c015..63dccbf697c2 100644 --- a/clippy_utils/src/macros.rs +++ b/clippy_utils/src/macros.rs @@ -1,4 +1,4 @@ -#![allow(clippy::similar_names, clippy::impl_trait_in_params)] // `expr` and `expn` +#![allow(clippy::similar_names)] // `expr` and `expn` use crate::source::snippet_opt; use crate::visitors::{for_each_expr, Descend}; diff --git a/clippy_utils/src/source.rs b/clippy_utils/src/source.rs index e802941b5d8a..cd5dcfdaca34 100644 --- a/clippy_utils/src/source.rs +++ b/clippy_utils/src/source.rs @@ -1,6 +1,6 @@ //! Utils for extracting, inspecting or transforming source code -#![allow(clippy::module_name_repetitions, clippy::impl_trait_in_params)] +#![allow(clippy::module_name_repetitions)] use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; diff --git a/tests/ui/borrow_box.rs b/tests/ui/borrow_box.rs index 8fe160e8e0f5..3b5b6bf4c950 100644 --- a/tests/ui/borrow_box.rs +++ b/tests/ui/borrow_box.rs @@ -1,10 +1,6 @@ #![deny(clippy::borrowed_box)] #![allow(dead_code, unused_variables)] -#![allow( - clippy::uninlined_format_args, - clippy::disallowed_names, - clippy::impl_trait_in_params -)] +#![allow(clippy::uninlined_format_args, clippy::disallowed_names)] use std::fmt::Display; diff --git a/tests/ui/borrow_box.stderr b/tests/ui/borrow_box.stderr index 90e752211ff0..99cb60a1ead9 100644 --- a/tests/ui/borrow_box.stderr +++ b/tests/ui/borrow_box.stderr @@ -1,5 +1,5 @@ error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:24:14 + --> $DIR/borrow_box.rs:20:14 | LL | let foo: &Box; | ^^^^^^^^^^ help: try: `&bool` @@ -11,55 +11,55 @@ LL | #![deny(clippy::borrowed_box)] | ^^^^^^^^^^^^^^^^^^^^ error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:28:10 + --> $DIR/borrow_box.rs:24:10 | LL | foo: &'a Box, | ^^^^^^^^^^^^^ help: try: `&'a bool` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:32:17 + --> $DIR/borrow_box.rs:28:17 | LL | fn test4(a: &Box); | ^^^^^^^^^^ help: try: `&bool` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:98:25 + --> $DIR/borrow_box.rs:94:25 | LL | pub fn test14(_display: &Box) {} | ^^^^^^^^^^^^^^^^^ help: try: `&dyn Display` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:99:25 + --> $DIR/borrow_box.rs:95:25 | LL | pub fn test15(_display: &Box) {} | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&(dyn Display + Send)` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:100:29 + --> $DIR/borrow_box.rs:96:29 | LL | pub fn test16<'a>(_display: &'a Box) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&'a (dyn Display + 'a)` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:102:25 + --> $DIR/borrow_box.rs:98:25 | LL | pub fn test17(_display: &Box) {} | ^^^^^^^^^^^^^^^^^^ help: try: `&impl Display` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:103:25 + --> $DIR/borrow_box.rs:99:25 | LL | pub fn test18(_display: &Box) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&(impl Display + Send)` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:104:29 + --> $DIR/borrow_box.rs:100:29 | LL | pub fn test19<'a>(_display: &'a Box) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&'a (impl Display + 'a)` error: you seem to be trying to use `&Box`. Consider using just `&T` - --> $DIR/borrow_box.rs:109:25 + --> $DIR/borrow_box.rs:105:25 | LL | pub fn test20(_display: &Box<(dyn Display + Send)>) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&(dyn Display + Send)` diff --git a/tests/ui/eta.fixed b/tests/ui/eta.fixed index c1ad0b66e01b..dc129591eac4 100644 --- a/tests/ui/eta.fixed +++ b/tests/ui/eta.fixed @@ -7,8 +7,7 @@ clippy::no_effect, clippy::option_map_unit_fn, clippy::redundant_closure_call, - clippy::uninlined_format_args, - clippy::impl_trait_in_params + clippy::uninlined_format_args )] use std::path::{Path, PathBuf}; diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs index 000af653c152..025fd6a0b7af 100644 --- a/tests/ui/eta.rs +++ b/tests/ui/eta.rs @@ -7,8 +7,7 @@ clippy::no_effect, clippy::option_map_unit_fn, clippy::redundant_closure_call, - clippy::uninlined_format_args, - clippy::impl_trait_in_params + clippy::uninlined_format_args )] use std::path::{Path, PathBuf}; diff --git a/tests/ui/eta.stderr b/tests/ui/eta.stderr index 6ddecc58eaf1..a521fb868607 100644 --- a/tests/ui/eta.stderr +++ b/tests/ui/eta.stderr @@ -1,5 +1,5 @@ error: redundant closure - --> $DIR/eta.rs:29:27 + --> $DIR/eta.rs:28:27 | LL | let a = Some(1u8).map(|a| foo(a)); | ^^^^^^^^^^ help: replace the closure with the function itself: `foo` @@ -7,31 +7,31 @@ LL | let a = Some(1u8).map(|a| foo(a)); = note: `-D clippy::redundant-closure` implied by `-D warnings` error: redundant closure - --> $DIR/eta.rs:33:40 + --> $DIR/eta.rs:32:40 | LL | let _: Option> = true.then(|| vec![]); // special case vec! | ^^^^^^^^^ help: replace the closure with `Vec::new`: `std::vec::Vec::new` error: redundant closure - --> $DIR/eta.rs:34:35 + --> $DIR/eta.rs:33:35 | LL | let d = Some(1u8).map(|a| foo((|b| foo2(b))(a))); //is adjusted? | ^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo2` error: redundant closure - --> $DIR/eta.rs:35:26 + --> $DIR/eta.rs:34:26 | LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted | ^^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `below` error: redundant closure - --> $DIR/eta.rs:42:27 + --> $DIR/eta.rs:41:27 | LL | let e = Some(1u8).map(|a| generic(a)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `generic` error: redundant closure - --> $DIR/eta.rs:88:51 + --> $DIR/eta.rs:87:51 | LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo()); | ^^^^^^^^^^^ help: replace the closure with the method itself: `TestStruct::foo` @@ -39,121 +39,121 @@ LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo()); = note: `-D clippy::redundant-closure-for-method-calls` implied by `-D warnings` error: redundant closure - --> $DIR/eta.rs:89:51 + --> $DIR/eta.rs:88:51 | LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.trait_foo()); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `TestTrait::trait_foo` error: redundant closure - --> $DIR/eta.rs:91:42 + --> $DIR/eta.rs:90:42 | LL | let e = Some(&mut vec![1, 2, 3]).map(|v| v.clear()); | ^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::vec::Vec::clear` error: redundant closure - --> $DIR/eta.rs:95:29 + --> $DIR/eta.rs:94:29 | LL | let e = Some("str").map(|s| s.to_string()); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::string::ToString::to_string` error: redundant closure - --> $DIR/eta.rs:96:27 + --> $DIR/eta.rs:95:27 | LL | let e = Some('a').map(|s| s.to_uppercase()); | ^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_uppercase` error: redundant closure - --> $DIR/eta.rs:98:65 + --> $DIR/eta.rs:97:65 | LL | let e: std::vec::Vec = vec!['a', 'b', 'c'].iter().map(|c| c.to_ascii_uppercase()).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_ascii_uppercase` error: redundant closure - --> $DIR/eta.rs:161:22 + --> $DIR/eta.rs:160:22 | LL | requires_fn_once(|| x()); | ^^^^^^ help: replace the closure with the function itself: `x` error: redundant closure - --> $DIR/eta.rs:168:27 + --> $DIR/eta.rs:167:27 | LL | let a = Some(1u8).map(|a| foo_ptr(a)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo_ptr` error: redundant closure - --> $DIR/eta.rs:173:27 + --> $DIR/eta.rs:172:27 | LL | let a = Some(1u8).map(|a| closure(a)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `closure` error: redundant closure - --> $DIR/eta.rs:205:28 + --> $DIR/eta.rs:204:28 | LL | x.into_iter().for_each(|x| add_to_res(x)); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res` error: redundant closure - --> $DIR/eta.rs:206:28 + --> $DIR/eta.rs:205:28 | LL | y.into_iter().for_each(|x| add_to_res(x)); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res` error: redundant closure - --> $DIR/eta.rs:207:28 + --> $DIR/eta.rs:206:28 | LL | z.into_iter().for_each(|x| add_to_res(x)); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `add_to_res` error: redundant closure - --> $DIR/eta.rs:214:21 + --> $DIR/eta.rs:213:21 | LL | Some(1).map(|n| closure(n)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut closure` error: redundant closure - --> $DIR/eta.rs:218:21 + --> $DIR/eta.rs:217:21 | LL | Some(1).map(|n| in_loop(n)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `in_loop` error: redundant closure - --> $DIR/eta.rs:311:18 + --> $DIR/eta.rs:310:18 | LL | takes_fn_mut(|| f()); | ^^^^^^ help: replace the closure with the function itself: `&mut f` error: redundant closure - --> $DIR/eta.rs:314:19 + --> $DIR/eta.rs:313:19 | LL | takes_fn_once(|| f()); | ^^^^^^ help: replace the closure with the function itself: `&mut f` error: redundant closure - --> $DIR/eta.rs:318:26 + --> $DIR/eta.rs:317:26 | LL | move || takes_fn_mut(|| f_used_once()) | ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut f_used_once` error: redundant closure - --> $DIR/eta.rs:330:19 + --> $DIR/eta.rs:329:19 | LL | array_opt.map(|a| a.as_slice()); | ^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8; 3]>::as_slice` error: redundant closure - --> $DIR/eta.rs:333:19 + --> $DIR/eta.rs:332:19 | LL | slice_opt.map(|s| s.len()); | ^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8]>::len` error: redundant closure - --> $DIR/eta.rs:336:17 + --> $DIR/eta.rs:335:17 | LL | ptr_opt.map(|p| p.is_null()); | ^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<*const usize>::is_null` error: redundant closure - --> $DIR/eta.rs:340:17 + --> $DIR/eta.rs:339:17 | LL | dyn_opt.map(|d| d.method_on_dyn()); | ^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `::method_on_dyn` diff --git a/tests/ui/impl_trait_in_params.stderr b/tests/ui/impl_trait_in_params.stderr index a43986017feb..62b264f0051c 100644 --- a/tests/ui/impl_trait_in_params.stderr +++ b/tests/ui/impl_trait_in_params.stderr @@ -1,22 +1,22 @@ -error: 'impl Trait' in the function's parameters +error: '`impl Trait` used as a function parameter' --> $DIR/impl_trait_in_params.rs:8:13 | LL | pub fn a(_: impl Trait) {} | ^^^^^^^^^^ | = note: `-D clippy::impl-trait-in-params` implied by `-D warnings` -help: create a generic type here and replace that 'impl Trait' with `T` +help: add a type paremeter | LL | pub fn a(_: impl Trait) {} | ++++++++++ -error: 'impl Trait' in the function's parameters +error: '`impl Trait` used as a function parameter' --> $DIR/impl_trait_in_params.rs:9:29 | LL | pub fn c(_: C, _: impl Trait) {} | ^^^^^^^^^^ | -help: create a generic type here and replace that `impl Trait` with `T` +help: add a type paremeter, `{}`: `{}` | LL | pub fn c(_: C, _: impl Trait) {} | ++++++++++ diff --git a/tests/ui/trait_duplication_in_bounds.fixed b/tests/ui/trait_duplication_in_bounds.fixed index 373859a25648..4ce5d4217822 100644 --- a/tests/ui/trait_duplication_in_bounds.fixed +++ b/tests/ui/trait_duplication_in_bounds.fixed @@ -1,6 +1,5 @@ // run-rustfix #![deny(clippy::trait_duplication_in_bounds)] -#![allow(clippy::impl_trait_in_params)] #![allow(unused)] fn bad_foo(arg0: T, argo1: U) { diff --git a/tests/ui/trait_duplication_in_bounds.rs b/tests/ui/trait_duplication_in_bounds.rs index 3afd74aa9b2c..7f2e96a22e66 100644 --- a/tests/ui/trait_duplication_in_bounds.rs +++ b/tests/ui/trait_duplication_in_bounds.rs @@ -1,6 +1,5 @@ // run-rustfix #![deny(clippy::trait_duplication_in_bounds)] -#![allow(clippy::impl_trait_in_params)] #![allow(unused)] fn bad_foo(arg0: T, argo1: U) { diff --git a/tests/ui/trait_duplication_in_bounds.stderr b/tests/ui/trait_duplication_in_bounds.stderr index 5216b2250dbd..af800ba78880 100644 --- a/tests/ui/trait_duplication_in_bounds.stderr +++ b/tests/ui/trait_duplication_in_bounds.stderr @@ -1,5 +1,5 @@ error: these bounds contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:6:15 + --> $DIR/trait_duplication_in_bounds.rs:5:15 | LL | fn bad_foo(arg0: T, argo1: U) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy` @@ -11,43 +11,43 @@ LL | #![deny(clippy::trait_duplication_in_bounds)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: these where clauses contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:12:8 + --> $DIR/trait_duplication_in_bounds.rs:11:8 | LL | T: Clone + Clone + Clone + Copy, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy` error: these bounds contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:40:26 + --> $DIR/trait_duplication_in_bounds.rs:39:26 | LL | trait BadSelfTraitBound: Clone + Clone + Clone { | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone` error: these where clauses contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:47:15 + --> $DIR/trait_duplication_in_bounds.rs:46:15 | LL | Self: Clone + Clone + Clone; | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone` error: these bounds contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:61:24 + --> $DIR/trait_duplication_in_bounds.rs:60:24 | LL | trait BadTraitBound { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy` error: these where clauses contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:68:12 + --> $DIR/trait_duplication_in_bounds.rs:67:12 | LL | T: Clone + Clone + Clone + Copy, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy` error: these bounds contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:101:19 + --> $DIR/trait_duplication_in_bounds.rs:100:19 | LL | fn bad_generic + GenericTrait + GenericTrait>(arg0: T) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `GenericTrait + GenericTrait` error: these bounds contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:109:22 + --> $DIR/trait_duplication_in_bounds.rs:108:22 | LL | fn qualified_path(arg0: T) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::clone::Clone + foo::Clone` diff --git a/tests/ui/trait_duplication_in_bounds_unfixable.rs b/tests/ui/trait_duplication_in_bounds_unfixable.rs index 7435a4ba609c..5630a0345adb 100644 --- a/tests/ui/trait_duplication_in_bounds_unfixable.rs +++ b/tests/ui/trait_duplication_in_bounds_unfixable.rs @@ -1,5 +1,4 @@ #![deny(clippy::trait_duplication_in_bounds)] -#![allow(clippy::impl_trait_in_params)] use std::collections::BTreeMap; use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign}; diff --git a/tests/ui/trait_duplication_in_bounds_unfixable.stderr b/tests/ui/trait_duplication_in_bounds_unfixable.stderr index 48091b38fb41..4d56a94646cb 100644 --- a/tests/ui/trait_duplication_in_bounds_unfixable.stderr +++ b/tests/ui/trait_duplication_in_bounds_unfixable.stderr @@ -1,5 +1,5 @@ error: this trait bound is already specified in the where clause - --> $DIR/trait_duplication_in_bounds_unfixable.rs:7:15 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:6:15 | LL | fn bad_foo(arg0: T, arg1: Z) | ^^^^^ @@ -12,7 +12,7 @@ LL | #![deny(clippy::trait_duplication_in_bounds)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this trait bound is already specified in the where clause - --> $DIR/trait_duplication_in_bounds_unfixable.rs:7:23 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:6:23 | LL | fn bad_foo(arg0: T, arg1: Z) | ^^^^^^^ @@ -20,7 +20,7 @@ LL | fn bad_foo(arg0: T, arg1: Z) = help: consider removing this trait bound error: this trait bound is already specified in trait declaration - --> $DIR/trait_duplication_in_bounds_unfixable.rs:36:15 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:35:15 | LL | Self: Default; | ^^^^^^^ @@ -28,7 +28,7 @@ LL | Self: Default; = help: consider removing this trait bound error: this trait bound is already specified in trait declaration - --> $DIR/trait_duplication_in_bounds_unfixable.rs:50:15 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:49:15 | LL | Self: Default + Clone; | ^^^^^^^ @@ -36,7 +36,7 @@ LL | Self: Default + Clone; = help: consider removing this trait bound error: this trait bound is already specified in trait declaration - --> $DIR/trait_duplication_in_bounds_unfixable.rs:56:15 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:55:15 | LL | Self: Default + Clone; | ^^^^^^^ @@ -44,7 +44,7 @@ LL | Self: Default + Clone; = help: consider removing this trait bound error: this trait bound is already specified in trait declaration - --> $DIR/trait_duplication_in_bounds_unfixable.rs:56:25 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:55:25 | LL | Self: Default + Clone; | ^^^^^ @@ -52,7 +52,7 @@ LL | Self: Default + Clone; = help: consider removing this trait bound error: this trait bound is already specified in trait declaration - --> $DIR/trait_duplication_in_bounds_unfixable.rs:59:15 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:58:15 | LL | Self: Default; | ^^^^^^^ @@ -60,7 +60,7 @@ LL | Self: Default; = help: consider removing this trait bound error: this trait bound is already specified in trait declaration - --> $DIR/trait_duplication_in_bounds_unfixable.rs:94:15 + --> $DIR/trait_duplication_in_bounds_unfixable.rs:93:15 | LL | Self: Iterator, | ^^^^^^^^^^^^^^^^^^^^ From 4166b7dcfe0384c57dbf24d219b05ccdba30e203 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Wed, 25 Jan 2023 19:19:16 +0100 Subject: [PATCH 7/9] Fix lint message --- clippy_lints/src/functions/impl_trait_in_params.rs | 2 +- tests/ui/impl_trait_in_params.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/functions/impl_trait_in_params.rs b/clippy_lints/src/functions/impl_trait_in_params.rs index ace5932f4e4a..e4b270db672c 100644 --- a/clippy_lints/src/functions/impl_trait_in_params.rs +++ b/clippy_lints/src/functions/impl_trait_in_params.rs @@ -23,7 +23,7 @@ pub(super) fn check_fn<'tcx>(cx: &LateContext<'_>, kind: &'tcx FnKind<'_>, body: if let Some(gen_span) = generics.span_for_param_suggestion() { diag.span_suggestion_with_style( gen_span, - "add a type paremeter, `{}`: `{}`", + "add a type paremeter", format!(", {next_letter}: {}", ¶m.name.ident().as_str()[5..]), rustc_errors::Applicability::MaybeIncorrect, rustc_errors::SuggestionStyle::ShowAlways, diff --git a/tests/ui/impl_trait_in_params.stderr b/tests/ui/impl_trait_in_params.stderr index 62b264f0051c..c0bcdfd6d425 100644 --- a/tests/ui/impl_trait_in_params.stderr +++ b/tests/ui/impl_trait_in_params.stderr @@ -16,7 +16,7 @@ error: '`impl Trait` used as a function parameter' LL | pub fn c(_: C, _: impl Trait) {} | ^^^^^^^^^^ | -help: add a type paremeter, `{}`: `{}` +help: add a type paremeter | LL | pub fn c(_: C, _: impl Trait) {} | ++++++++++ From 6ef34bf009343a398b3d04850c5f0784329ebe99 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Fri, 3 Feb 2023 16:46:55 +0100 Subject: [PATCH 8/9] Remove commented code --- clippy_lints/src/functions/impl_trait_in_params.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/clippy_lints/src/functions/impl_trait_in_params.rs b/clippy_lints/src/functions/impl_trait_in_params.rs index e4b270db672c..1f00e357bfa4 100644 --- a/clippy_lints/src/functions/impl_trait_in_params.rs +++ b/clippy_lints/src/functions/impl_trait_in_params.rs @@ -29,9 +29,6 @@ pub(super) fn check_fn<'tcx>(cx: &LateContext<'_>, kind: &'tcx FnKind<'_>, body: rustc_errors::SuggestionStyle::ShowAlways, ); } else { - // multispan.push_span_label(param.span, format!("Replace this with `{}`", - // next_letter)); - diag.span_suggestion_with_style( Span::new( body.params[0].span.lo() - rustc_span::BytePos(1), From 89fde4abf2c689c0ad5a09cc423a0a7be475d6ee Mon Sep 17 00:00:00 2001 From: blyxyas Date: Sat, 18 Feb 2023 20:05:30 +0100 Subject: [PATCH 9/9] Add placeholders, remove name suggesting --- .../src/functions/impl_trait_in_params.rs | 34 +++---------------- tests/ui/impl_trait_in_params.stderr | 8 ++--- 2 files changed, 9 insertions(+), 33 deletions(-) diff --git a/clippy_lints/src/functions/impl_trait_in_params.rs b/clippy_lints/src/functions/impl_trait_in_params.rs index 1f00e357bfa4..2811a73f6c18 100644 --- a/clippy_lints/src/functions/impl_trait_in_params.rs +++ b/clippy_lints/src/functions/impl_trait_in_params.rs @@ -1,6 +1,6 @@ use clippy_utils::{diagnostics::span_lint_and_then, is_in_test_function}; -use rustc_hir::{intravisit::FnKind, Body, Generics, HirId}; +use rustc_hir::{intravisit::FnKind, Body, HirId}; use rustc_lint::LateContext; use rustc_span::Span; @@ -19,13 +19,12 @@ pub(super) fn check_fn<'tcx>(cx: &LateContext<'_>, kind: &'tcx FnKind<'_>, body: param.span, "'`impl Trait` used as a function parameter'", |diag| { - let next_letter = next_valid_letter(generics); if let Some(gen_span) = generics.span_for_param_suggestion() { diag.span_suggestion_with_style( gen_span, "add a type paremeter", - format!(", {next_letter}: {}", ¶m.name.ident().as_str()[5..]), - rustc_errors::Applicability::MaybeIncorrect, + format!(", {{ /* Generic name */ }}: {}", ¶m.name.ident().as_str()[5..]), + rustc_errors::Applicability::HasPlaceholders, rustc_errors::SuggestionStyle::ShowAlways, ); } else { @@ -37,8 +36,8 @@ pub(super) fn check_fn<'tcx>(cx: &LateContext<'_>, kind: &'tcx FnKind<'_>, body: ident.span.parent(), ), "add a type paremeter", - format!("<{next_letter}: {}>", ¶m.name.ident().as_str()[5..]), - rustc_errors::Applicability::MaybeIncorrect, + format!("<{{ /* Generic name */ }}: {}>", ¶m.name.ident().as_str()[5..]), + rustc_errors::Applicability::HasPlaceholders, rustc_errors::SuggestionStyle::ShowAlways, ); } @@ -49,26 +48,3 @@ pub(super) fn check_fn<'tcx>(cx: &LateContext<'_>, kind: &'tcx FnKind<'_>, body: } } } - -fn next_valid_letter(generics: &Generics<'_>) -> char { - let mut generics_names = Vec::new(); - - generics.params.iter().for_each(|param| { - generics_names.push(param.name.ident().as_str().to_owned()); - }); - - // If T exists, try with U, then with V, and so on... - let mut current_letter = 84u32; // ASCII code for "T" - while generics_names.contains(&String::from(char::from_u32(current_letter).unwrap())) { - current_letter += 1; - if current_letter == 91 { - // ASCII code for "Z" - current_letter = 65; - } else if current_letter == 83 { - // ASCII "S" - current_letter = 97; // "a" - }; - } - - char::from_u32(current_letter).unwrap() -} diff --git a/tests/ui/impl_trait_in_params.stderr b/tests/ui/impl_trait_in_params.stderr index c0bcdfd6d425..acfcc21445eb 100644 --- a/tests/ui/impl_trait_in_params.stderr +++ b/tests/ui/impl_trait_in_params.stderr @@ -7,8 +7,8 @@ LL | pub fn a(_: impl Trait) {} = note: `-D clippy::impl-trait-in-params` implied by `-D warnings` help: add a type paremeter | -LL | pub fn a(_: impl Trait) {} - | ++++++++++ +LL | pub fn a<{ /* Generic name */ }: Trait>(_: impl Trait) {} + | +++++++++++++++++++++++++++++++ error: '`impl Trait` used as a function parameter' --> $DIR/impl_trait_in_params.rs:9:29 @@ -18,8 +18,8 @@ LL | pub fn c(_: C, _: impl Trait) {} | help: add a type paremeter | -LL | pub fn c(_: C, _: impl Trait) {} - | ++++++++++ +LL | pub fn c(_: C, _: impl Trait) {} + | +++++++++++++++++++++++++++++++ error: aborting due to 2 previous errors