Skip to content

Commit 8ec9543

Browse files
committed
Add impl_trait_param lint
As this is a lint about "style", and a purely cosmetical choice (using `<A: Trait>` over `impl Trait`), a lot of other files needed to be allowed this lint.
1 parent 5b6795f commit 8ec9543

20 files changed

+219
-48
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4430,6 +4430,7 @@ Released 2018-09-13
44304430
[`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else
44314431
[`if_then_some_else_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none
44324432
[`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond
4433+
[`impl_trait_param`]: https://rust-lang.github.io/rust-clippy/master/index.html#impl_trait_param
44334434
[`implicit_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_clone
44344435
[`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher
44354436
[`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
179179
crate::from_raw_with_void_ptr::FROM_RAW_WITH_VOID_PTR_INFO,
180180
crate::from_str_radix_10::FROM_STR_RADIX_10_INFO,
181181
crate::functions::DOUBLE_MUST_USE_INFO,
182+
crate::functions::IMPL_TRAIT_PARAM_INFO,
182183
crate::functions::MISNAMED_GETTERS_INFO,
183184
crate::functions::MUST_USE_CANDIDATE_INFO,
184185
crate::functions::MUST_USE_UNIT_INFO,
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
use clippy_utils::{diagnostics::span_lint_and_then, is_in_test_function};
2+
3+
use rustc_hir::{intravisit::FnKind, Body, Generics, HirId};
4+
use rustc_lint::LateContext;
5+
use rustc_span::Span;
6+
7+
use super::IMPL_TRAIT_PARAM;
8+
9+
pub(super) fn check_fn<'tcx>(cx: &LateContext<'_>, kind: &'tcx FnKind<'_>, body: &'tcx Body<'_>, hir_id: HirId) {
10+
if cx.tcx.visibility(cx.tcx.hir().body_owner_def_id(body.id())).is_public() && !is_in_test_function(cx.tcx, hir_id)
11+
{
12+
if let FnKind::ItemFn(ident, generics, _) = kind {
13+
for param in generics.params {
14+
if param.is_impl_trait()
15+
&& !param.name.ident().as_str().contains('<')
16+
&& !param.name.ident().as_str().contains('(')
17+
{
18+
// No generics with nested generics, and no generics like FnMut(x)
19+
span_lint_and_then(
20+
cx,
21+
IMPL_TRAIT_PARAM,
22+
param.span,
23+
&format!("'{}' in the function's parameters", param.name.ident().as_str()),
24+
|diag| {
25+
let next_letter = next_valid_letter(generics);
26+
if let Some(gen_span) = generics.span_for_param_suggestion() {
27+
diag.span_suggestion_with_style(
28+
gen_span,
29+
format!(
30+
"create a generic type here and replace that `{}` with `{}`",
31+
param.name.ident().as_str(),
32+
next_letter
33+
),
34+
", T: Trait",
35+
rustc_errors::Applicability::MaybeIncorrect,
36+
rustc_errors::SuggestionStyle::ShowAlways,
37+
);
38+
} else {
39+
// multispan.push_span_label(param.span, format!("Replace this with `{}`",
40+
// next_letter));
41+
42+
diag.span_suggestion_with_style(
43+
Span::new(
44+
body.params[0].span.lo() - rustc_span::BytePos(1),
45+
ident.span.hi(),
46+
ident.span.ctxt(),
47+
ident.span.parent(),
48+
),
49+
format!(
50+
"create a generic type here and replace that '{}' with `{}`",
51+
param.name.ident().as_str(),
52+
next_letter
53+
),
54+
"<T: Trait>",
55+
rustc_errors::Applicability::MaybeIncorrect,
56+
rustc_errors::SuggestionStyle::ShowAlways,
57+
);
58+
}
59+
},
60+
);
61+
}
62+
}
63+
}
64+
}
65+
}
66+
67+
fn next_valid_letter(generics: &Generics<'_>) -> char {
68+
let mut generics_names = Vec::new();
69+
70+
generics.params.iter().for_each(|param| {
71+
generics_names.push(param.name.ident().as_str().to_owned());
72+
});
73+
74+
// If T exists, try with U, then with V, and so on...
75+
let mut current_letter = 84u32; // ASCII code for "T"
76+
while generics_names.contains(&String::from(char::from_u32(current_letter).unwrap())) {
77+
current_letter += 1;
78+
if current_letter == 91 {
79+
// ASCII code for "Z"
80+
current_letter = 65;
81+
} else if current_letter == 83 {
82+
// ASCII "S"
83+
current_letter = 97; // "a"
84+
};
85+
}
86+
87+
char::from_u32(current_letter).unwrap()
88+
}

clippy_lints/src/functions/mod.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
mod impl_trait_param;
12
mod misnamed_getters;
23
mod must_use;
34
mod not_unsafe_ptr_arg_deref;
@@ -327,6 +328,32 @@ declare_clippy_lint! {
327328
"getter method returning the wrong field"
328329
}
329330

331+
declare_clippy_lint! {
332+
/// ### What it does
333+
/// Lints when `impl Trait` is being used in a function's paremeters.
334+
/// ### Why is this bad?
335+
/// Turbofish syntax (`::<>`) cannot be used when `impl Trait` is being used, making `impl Trait` less powerful. Readability may also be a factor.
336+
///
337+
/// ### Example
338+
/// ```rust
339+
/// trait MyTrait {}
340+
/// fn foo(a: impl MyTrait) {
341+
/// // [...]
342+
/// }
343+
/// ```
344+
/// Use instead:
345+
/// ```rust
346+
/// trait MyTrait {}
347+
/// fn foo<T: A>(a: A) {
348+
/// // [...]
349+
/// }
350+
/// ```
351+
#[clippy::version = "1.68.0"]
352+
pub IMPL_TRAIT_PARAM,
353+
style,
354+
"`impl Trait` is used in the function's parameters"
355+
}
356+
330357
#[derive(Copy, Clone)]
331358
pub struct Functions {
332359
too_many_arguments_threshold: u64,
@@ -354,6 +381,7 @@ impl_lint_pass!(Functions => [
354381
RESULT_UNIT_ERR,
355382
RESULT_LARGE_ERR,
356383
MISNAMED_GETTERS,
384+
IMPL_TRAIT_PARAM,
357385
]);
358386

359387
impl<'tcx> LateLintPass<'tcx> for Functions {
@@ -371,6 +399,7 @@ impl<'tcx> LateLintPass<'tcx> for Functions {
371399
too_many_lines::check_fn(cx, kind, span, body, self.too_many_lines_threshold);
372400
not_unsafe_ptr_arg_deref::check_fn(cx, kind, decl, body, def_id);
373401
misnamed_getters::check_fn(cx, kind, decl, body, span);
402+
impl_trait_param::check_fn(cx, &kind, body, hir_id);
374403
}
375404

376405
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {

clippy_utils/src/ast_utils.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@
22
//!
33
//! - The `eq_foobar` functions test for semantic equality but ignores `NodeId`s and `Span`s.
44
5-
#![allow(clippy::similar_names, clippy::wildcard_imports, clippy::enum_glob_use)]
5+
#![allow(
6+
clippy::similar_names,
7+
clippy::wildcard_imports,
8+
clippy::enum_glob_use,
9+
clippy::impl_trait_param
10+
)]
611

712
use crate::{both, over};
813
use rustc_ast::ptr::P;

clippy_utils/src/check_proc_macro.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![allow(clippy::impl_trait_param)]
12
//! This module handles checking if the span given is from a proc-macro or not.
23
//!
34
//! Proc-macros are capable of setting the span of every token they output to a few possible spans.

clippy_utils/src/hir_utils.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![allow(clippy::impl_trait_param)]
12
use crate::consts::constant_simple;
23
use crate::macros::macro_backtrace;
34
use crate::source::snippet_opt;

clippy_utils/src/macros.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#![allow(clippy::similar_names)] // `expr` and `expn`
1+
#![allow(clippy::similar_names, clippy::impl_trait_param)] // `expr` and `expn`
22

33
use crate::source::snippet_opt;
44
use crate::visitors::{for_each_expr, Descend};

clippy_utils/src/source.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Utils for extracting, inspecting or transforming source code
22
3-
#![allow(clippy::module_name_repetitions)]
3+
#![allow(clippy::module_name_repetitions, clippy::impl_trait_param)]
44

55
use rustc_errors::Applicability;
66
use rustc_hir::{Expr, ExprKind};

tests/ui/borrow_box.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#![deny(clippy::borrowed_box)]
22
#![allow(dead_code, unused_variables)]
3-
#![allow(clippy::uninlined_format_args, clippy::disallowed_names)]
3+
#![allow(clippy::uninlined_format_args, clippy::disallowed_names, clippy::impl_trait_param)]
44

55
use std::fmt::Display;
66

0 commit comments

Comments
 (0)