about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-06-06 21:10:29 +0000
committerbors <bors@rust-lang.org>2023-06-06 21:10:29 +0000
commitcc8ead2cce5a99ac8d4e963570c7eb6ed18f66b7 (patch)
tree45c4b4aa53efad1198c343e82b42d41e3e31dff9
parent7ee3dcdb3270d7f27cf8a2aa1d801e7d006539db (diff)
parenta9b468f2b51a251d0d718059299d0ccd0e561389 (diff)
downloadrust-cc8ead2cce5a99ac8d4e963570c7eb6ed18f66b7.tar.gz
rust-cc8ead2cce5a99ac8d4e963570c7eb6ed18f66b7.zip
Auto merge of #10570 - AlessioC31:redundant_type_annotations, r=xFrednet
Add redundant type annotations lint

Hello, I'm trying to add the `redundat_type_annotations` lint.

It's still WIP but I'd like to start gathering some feedbacks to be sure that I'm not doing things 100% wrong :)

Right now it still misses lints like:

- [x] `let foo: u32 = 5_u32`,
- [x] `let foo: String = STest2::func()`
- [x] `let foo: String = self.func()` (`MethodCall`)
- [x] refs
- [ ] Generics

I've some problems regarding the second example above, in the `init` part of the `Local` I have:

```rust
init: Some(
                Expr {
                    hir_id: HirId(DefId(0:24 ~ playground[e1bd]::main).58),
                    kind: Call(
                        Expr {
                            hir_id: HirId(DefId(0:24 ~ playground[e1bd]::main).59),
                            kind: Path(
                                TypeRelative(
                                    Ty {
                                        hir_id: HirId(DefId(0:24 ~ playground[e1bd]::main).61),
                                        kind: Path(
                                            Resolved(
                                                None,
                                                Path {
                                                    span: src/main.rs:77:21: 77:27 (#0),
                                                    res: Def(
                                                        Struct,
                                                        DefId(0:17 ~ playground[e1bd]::STest2),
                                                    ),
                                                    segments: [
                                                        PathSegment {
                                                            ident: STest2#0,
                                                            hir_id: HirId(DefId(0:24 ~ playground[e1bd]::main).60),
                                                            res: Def(
                                                                Struct,
                                                                DefId(0:17 ~ playground[e1bd]::STest2),
                                                            ),
                                                            args: None,
                                                            infer_args: true,
                                                        },
                                                    ],
                                                },
                                            ),
                                        ),
                                        span: src/main.rs:77:21: 77:27 (#0),
                                    },
                                    PathSegment {
                                        ident: get_numb#0,
                                        hir_id: HirId(DefId(0:24 ~ playground[e1bd]::main).62),
                                        res: Err,
                                        args: None,
                                        infer_args: true,
                                    },
                                ),
                            ),
                            span: src/main.rs:77:21: 77:37 (#0),
                        },
                        [],
                    ),
                    span: src/main.rs:77:21: 77:39 (#0),
                },
            ),
```

And I'm not sure how to get the return type of the function `STest2::func()` since the resolved path `DefId` points to the struct itself and not the function. Do you have any idea on how I could get this information in this case?

Thanks!

changelog: changelog: [`redundant_type_annotations`]: New lint to warn on redundant type annotations

fixes #9155
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/declared_lints.rs1
-rw-r--r--clippy_lints/src/lib.rs2
-rw-r--r--clippy_lints/src/redundant_type_annotations.rs210
-rw-r--r--tests/ui/redundant_type_annotations.rs176
-rw-r--r--tests/ui/redundant_type_annotations.stderr106
6 files changed, 496 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4648af231a4..a65ac4d46de 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5110,6 +5110,7 @@ Released 2018-09-13
 [`redundant_pub_crate`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pub_crate
 [`redundant_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing
 [`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
+[`redundant_type_annotations`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_type_annotations
 [`ref_binding_to_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_binding_to_reference
 [`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
 [`ref_option_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_option_ref
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index 68272e1a77a..d014534cee9 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -543,6 +543,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::redundant_slicing::DEREF_BY_SLICING_INFO,
     crate::redundant_slicing::REDUNDANT_SLICING_INFO,
     crate::redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES_INFO,
+    crate::redundant_type_annotations::REDUNDANT_TYPE_ANNOTATIONS_INFO,
     crate::ref_option_ref::REF_OPTION_REF_INFO,
     crate::ref_patterns::REF_PATTERNS_INFO,
     crate::reference::DEREF_ADDROF_INFO,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 222b0cc0921..d53c6de5451 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -267,6 +267,7 @@ mod redundant_field_names;
 mod redundant_pub_crate;
 mod redundant_slicing;
 mod redundant_static_lifetimes;
+mod redundant_type_annotations;
 mod ref_option_ref;
 mod ref_patterns;
 mod reference;
@@ -1012,6 +1013,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_early_pass(|| Box::new(needless_else::NeedlessElse));
     store.register_late_pass(|_| Box::new(missing_fields_in_debug::MissingFieldsInDebug));
     store.register_late_pass(|_| Box::new(endian_bytes::EndianBytes));
+    store.register_late_pass(|_| Box::new(redundant_type_annotations::RedundantTypeAnnotations));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 
diff --git a/clippy_lints/src/redundant_type_annotations.rs b/clippy_lints/src/redundant_type_annotations.rs
new file mode 100644
index 00000000000..8e9234bba3c
--- /dev/null
+++ b/clippy_lints/src/redundant_type_annotations.rs
@@ -0,0 +1,210 @@
+use clippy_utils::diagnostics::span_lint;
+use rustc_ast::LitKind;
+use rustc_hir as hir;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::Ty;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Warns about needless / redundant type annotations.
+    ///
+    /// ### Why is this bad?
+    /// Code without type annotations is shorter and in most cases
+    /// more idiomatic and easier to modify.
+    ///
+    /// ### Limitations
+    /// This lint doesn't support:
+    ///
+    /// - Generics
+    /// - Refs returned from anything else than a `MethodCall`
+    /// - Complex types (tuples, arrays, etc...)
+    /// - `Path` to anything else than a primitive type.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let foo: String = String::new();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let foo = String::new();
+    /// ```
+    #[clippy::version = "1.70.0"]
+    pub REDUNDANT_TYPE_ANNOTATIONS,
+    restriction,
+    "warns about needless / redundant type annotations."
+}
+declare_lint_pass!(RedundantTypeAnnotations => [REDUNDANT_TYPE_ANNOTATIONS]);
+
+fn is_same_type<'tcx>(cx: &LateContext<'tcx>, ty_resolved_path: hir::def::Res, func_return_type: Ty<'tcx>) -> bool {
+    // type annotation is primitive
+    if let hir::def::Res::PrimTy(primty) = ty_resolved_path
+        && func_return_type.is_primitive()
+        && let Some(func_return_type_sym) = func_return_type.primitive_symbol()
+    {
+        return primty.name() == func_return_type_sym;
+    }
+
+    // type annotation is any other non generic type
+    if let hir::def::Res::Def(_, defid) = ty_resolved_path
+        && let Some(annotation_ty) = cx.tcx.type_of(defid).no_bound_vars()
+    {
+        return annotation_ty == func_return_type;
+    }
+
+    false
+}
+
+fn func_hir_id_to_func_ty<'tcx>(cx: &LateContext<'tcx>, hir_id: hir::hir_id::HirId) -> Option<Ty<'tcx>> {
+    if let Some((defkind, func_defid)) = cx.typeck_results().type_dependent_def(hir_id)
+        && defkind == hir::def::DefKind::AssocFn
+        && let Some(init_ty) = cx.tcx.type_of(func_defid).no_bound_vars()
+    {
+        Some(init_ty)
+    } else {
+        None
+    }
+}
+
+fn func_ty_to_return_type<'tcx>(cx: &LateContext<'tcx>, func_ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
+    if func_ty.is_fn() {
+        Some(func_ty.fn_sig(cx.tcx).output().skip_binder())
+    } else {
+        None
+    }
+}
+
+/// Extracts the fn Ty, e.g. `fn() -> std::string::String {f}`
+fn extract_fn_ty<'tcx>(
+    cx: &LateContext<'tcx>,
+    call: &hir::Expr<'tcx>,
+    func_return_path: &hir::QPath<'tcx>,
+) -> Option<Ty<'tcx>> {
+    match func_return_path {
+        // let a: String = f(); where f: fn f() -> String
+        hir::QPath::Resolved(_, resolved_path) => {
+            if let hir::def::Res::Def(_, defid) = resolved_path.res
+                && let Some(middle_ty_init) = cx.tcx.type_of(defid).no_bound_vars()
+            {
+                Some(middle_ty_init)
+            } else {
+                None
+            }
+        },
+        // Associated functions like
+        // let a: String = String::new();
+        // let a: String = String::get_string();
+        hir::QPath::TypeRelative(..) => func_hir_id_to_func_ty(cx, call.hir_id),
+        hir::QPath::LangItem(..) => None,
+    }
+}
+
+fn is_redundant_in_func_call<'tcx>(
+    cx: &LateContext<'tcx>,
+    ty_resolved_path: hir::def::Res,
+    call: &hir::Expr<'tcx>,
+) -> bool {
+    if let hir::ExprKind::Path(init_path) = &call.kind {
+        let func_type = extract_fn_ty(cx, call, init_path);
+
+        if let Some(func_type) = func_type
+            && let Some(init_return_type) = func_ty_to_return_type(cx, func_type)
+        {
+            return is_same_type(cx, ty_resolved_path, init_return_type);
+        }
+    }
+
+    false
+}
+
+fn extract_primty(ty_kind: &hir::TyKind<'_>) -> Option<hir::PrimTy> {
+    if let hir::TyKind::Path(ty_path) = ty_kind
+        && let hir::QPath::Resolved(_, resolved_path_ty) = ty_path
+        && let hir::def::Res::PrimTy(primty) = resolved_path_ty.res
+    {
+        Some(primty)
+    } else {
+        None
+    }
+}
+
+impl LateLintPass<'_> for RedundantTypeAnnotations {
+    fn check_local<'tcx>(&mut self, cx: &LateContext<'tcx>, local: &'tcx rustc_hir::Local<'tcx>) {
+        // type annotation part
+        if !local.span.from_expansion()
+            && let Some(ty) = &local.ty
+
+            // initialization part
+            && let Some(init) = local.init
+        {
+            match &init.kind {
+                // When the initialization is a call to a function
+                hir::ExprKind::Call(init_call, _) => {
+                    if let hir::TyKind::Path(ty_path) = &ty.kind
+                        && let hir::QPath::Resolved(_, resolved_path_ty) = ty_path
+
+                        && is_redundant_in_func_call(cx, resolved_path_ty.res, init_call) {
+                        span_lint(cx, REDUNDANT_TYPE_ANNOTATIONS, local.span, "redundant type annotation");
+                    }
+                },
+                hir::ExprKind::MethodCall(_, _, _, _) => {
+                    let mut is_ref = false;
+                    let mut ty_kind = &ty.kind;
+
+                    // If the annotation is a ref we "peel" it
+                    if let hir::TyKind::Ref(_, mut_ty) = &ty.kind {
+                        is_ref = true;
+                        ty_kind = &mut_ty.ty.kind;
+                    }
+
+                    if let hir::TyKind::Path(ty_path) = ty_kind
+                        && let hir::QPath::Resolved(_, resolved_path_ty) = ty_path
+                        && let Some(func_ty) = func_hir_id_to_func_ty(cx, init.hir_id)
+                        && let Some(return_type) = func_ty_to_return_type(cx, func_ty)
+                        && is_same_type(cx, resolved_path_ty.res, if is_ref {
+                            return_type.peel_refs()
+                        } else {
+                            return_type
+                        })
+                    {
+                        span_lint(cx, REDUNDANT_TYPE_ANNOTATIONS, local.span, "redundant type annotation");
+                    }
+                },
+                // When the initialization is a path for example u32::MAX
+                hir::ExprKind::Path(init_path) => {
+                    // TODO: check for non primty
+                    if let Some(primty) = extract_primty(&ty.kind)
+
+                        && let hir::QPath::TypeRelative(init_ty, _) = init_path
+                        && let Some(primty_init) = extract_primty(&init_ty.kind)
+
+                        && primty == primty_init
+                    {
+                        span_lint(cx, REDUNDANT_TYPE_ANNOTATIONS, local.span, "redundant type annotation");
+                    }
+                },
+                hir::ExprKind::Lit(init_lit) => {
+                    match init_lit.node {
+                        // In these cases the annotation is redundant
+                        LitKind::Str(..)
+                        | LitKind::ByteStr(..)
+                        | LitKind::Byte(..)
+                        | LitKind::Char(..)
+                        | LitKind::Bool(..)
+                        | LitKind::CStr(..) => {
+                            span_lint(cx, REDUNDANT_TYPE_ANNOTATIONS, local.span, "redundant type annotation");
+                        },
+                        LitKind::Int(..) | LitKind::Float(..) => {
+                            // If the initialization value is a suffixed literal we lint
+                            if init_lit.node.is_suffixed() {
+                                span_lint(cx, REDUNDANT_TYPE_ANNOTATIONS, local.span, "redundant type annotation");
+                            }
+                        },
+                        LitKind::Err => (),
+                    }
+                }
+                _ => ()
+            }
+        };
+    }
+}
diff --git a/tests/ui/redundant_type_annotations.rs b/tests/ui/redundant_type_annotations.rs
new file mode 100644
index 00000000000..cc507b8d658
--- /dev/null
+++ b/tests/ui/redundant_type_annotations.rs
@@ -0,0 +1,176 @@
+#![allow(unused)]
+#![warn(clippy::redundant_type_annotations)]
+
+#[derive(Debug, Default)]
+struct Cake<T> {
+    _data: T,
+}
+
+fn make_something<T: Default>() -> T {
+    T::default()
+}
+
+fn make_cake<T: Default>() -> Cake<T> {
+    Cake::<T>::default()
+}
+
+fn plus_one<T: std::ops::Add<u8, Output = T>>(val: T) -> T {
+    val + 1
+}
+
+#[derive(Default)]
+struct Slice {
+    inner: u32,
+}
+
+#[derive(Default)]
+struct Pie {
+    inner: u32,
+    inner_struct: Slice,
+}
+
+enum Pizza {
+    One,
+    Two,
+}
+
+fn return_a_string() -> String {
+    String::new()
+}
+
+fn return_a_struct() -> Pie {
+    Pie::default()
+}
+
+fn return_an_enum() -> Pizza {
+    Pizza::One
+}
+
+fn return_an_int() -> u32 {
+    5
+}
+
+impl Pie {
+    fn return_an_int(&self) -> u32 {
+        self.inner
+    }
+
+    fn return_a_ref(&self) -> &u32 {
+        &self.inner
+    }
+
+    fn return_a_ref_to_struct(&self) -> &Slice {
+        &self.inner_struct
+    }
+
+    fn associated_return_an_int() -> u32 {
+        5
+    }
+
+    fn new() -> Self {
+        Self::default()
+    }
+
+    fn associated_return_a_string() -> String {
+        String::from("")
+    }
+
+    fn test_method_call(&self) {
+        // Everything here should be lint
+
+        let v: u32 = self.return_an_int();
+        let v: &u32 = self.return_a_ref();
+        let v: &Slice = self.return_a_ref_to_struct();
+    }
+}
+
+fn test_generics() {
+    // The type annotation is needed to determine T
+    let _c: Cake<i32> = make_something();
+
+    // The type annotation is needed to determine the topic
+    let _c: Cake<u8> = make_cake();
+
+    // This could be lint, but currently doesn't
+    let _c: Cake<u8> = make_cake::<u8>();
+
+    // This could be lint, but currently doesn't
+    let _c: u8 = make_something::<u8>();
+
+    // This could be lint, but currently doesn't
+    let _c: u8 = plus_one(5_u8);
+
+    // Annotation needed otherwise T is i32
+    let _c: u8 = plus_one(5);
+
+    // This could be lint, but currently doesn't
+    let _return: String = String::from("test");
+}
+
+fn test_non_locals() {
+    // This shouldn't be lint
+    fn _arg(x: u32) -> u32 {
+        x
+    }
+
+    // This could lint, but probably shouldn't
+    let _closure_arg = |x: u32| x;
+}
+
+fn test_complex_types() {
+    // Shouldn't be lint, since the literal will be i32 otherwise
+    let _u8: u8 = 128;
+
+    // This could be lint, but currently doesn't
+    let _tuple_i32: (i32, i32) = (12, 13);
+
+    // Shouldn't be lint, since the tuple will be i32 otherwise
+    let _tuple_u32: (u32, u32) = (1, 2);
+
+    // Should be lint, since the type is determined by the init value, but currently doesn't
+    let _tuple_u32: (u32, u32) = (3_u32, 4_u32);
+
+    // This could be lint, but currently doesn't
+    let _array: [i32; 3] = [5, 6, 7];
+
+    // Shouldn't be lint
+    let _array: [u32; 2] = [8, 9];
+}
+
+fn test_functions() {
+    // Everything here should be lint
+
+    let _return: String = return_a_string();
+
+    let _return: Pie = return_a_struct();
+
+    let _return: Pizza = return_an_enum();
+
+    let _return: u32 = return_an_int();
+
+    let _return: String = String::new();
+
+    let new_pie: Pie = Pie::new();
+
+    let _return: u32 = new_pie.return_an_int();
+
+    let _return: u32 = Pie::associated_return_an_int();
+
+    let _return: String = Pie::associated_return_a_string();
+}
+
+fn test_simple_types() {
+    // Everything here should be lint
+
+    let _var: u32 = u32::MAX;
+
+    let _var: u32 = 5_u32;
+
+    let _var: &str = "test";
+
+    let _var: &[u8] = b"test";
+
+    let _var: bool = false;
+}
+
+fn main() {}
diff --git a/tests/ui/redundant_type_annotations.stderr b/tests/ui/redundant_type_annotations.stderr
new file mode 100644
index 00000000000..e8b2fe5c384
--- /dev/null
+++ b/tests/ui/redundant_type_annotations.stderr
@@ -0,0 +1,106 @@
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:81:9
+   |
+LL |         let v: u32 = self.return_an_int();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::redundant-type-annotations` implied by `-D warnings`
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:82:9
+   |
+LL |         let v: &u32 = self.return_a_ref();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:83:9
+   |
+LL |         let v: &Slice = self.return_a_ref_to_struct();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:143:5
+   |
+LL |     let _return: String = return_a_string();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:145:5
+   |
+LL |     let _return: Pie = return_a_struct();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:147:5
+   |
+LL |     let _return: Pizza = return_an_enum();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:149:5
+   |
+LL |     let _return: u32 = return_an_int();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:151:5
+   |
+LL |     let _return: String = String::new();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:153:5
+   |
+LL |     let new_pie: Pie = Pie::new();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:155:5
+   |
+LL |     let _return: u32 = new_pie.return_an_int();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:157:5
+   |
+LL |     let _return: u32 = Pie::associated_return_an_int();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:159:5
+   |
+LL |     let _return: String = Pie::associated_return_a_string();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:165:5
+   |
+LL |     let _var: u32 = u32::MAX;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:167:5
+   |
+LL |     let _var: u32 = 5_u32;
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:169:5
+   |
+LL |     let _var: &str = "test";
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:171:5
+   |
+LL |     let _var: &[u8] = b"test";
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: redundant type annotation
+  --> $DIR/redundant_type_annotations.rs:173:5
+   |
+LL |     let _var: bool = false;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 17 previous errors
+