Expand unnecessary_literal_bound to literal arrays and slices

Author: GnomedDev

clippy_lints/src/unnecessary_literal_bound.rs

@@ -1,10 +1,12 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::{ReturnType, ReturnVisitor, path_res, visit_returns};
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::{ReturnType, ReturnVisitor, visit_returns};
+use rustc_ast::BorrowKind;
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
 use rustc_hir::intravisit::FnKind;
-use rustc_hir::{Body, ExprKind, FnDecl, FnRetTy, Lit, MutTy, Mutability, PrimTy, Ty, TyKind};
+use rustc_hir::{Body, Expr, ExprKind, FnDecl, FnRetTy, Lit, MutTy, Mutability, PrimTy, Ty, TyKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
 use rustc_span::Span;
@@ -13,11 +15,11 @@ use rustc_span::def_id::LocalDefId;
 declare_clippy_lint! {
     /// ### What it does
     ///
-    /// Detects functions that are written to return `&str` that could return `&'static str` but instead return a `&'a str`.
+    /// Detects functions that are written to return a reference to a literal that could return a static reference but instead return a lifetime-bounded reference.
     ///
     /// ### Why is this bad?
     ///
-    /// This leaves the caller unable to use the `&str` as `&'static str`, causing unneccessary allocations or confusion.
+    /// This leaves the caller unable to use the reference as `'static`, causing unneccessary allocations or confusion.
     /// This is also most likely what you meant to write.
     ///
     /// ### Example
@@ -67,29 +69,63 @@ fn extract_anonymous_ref<'tcx>(hir_ty: &Ty<'tcx>) -> Option<&'tcx Ty<'tcx>> {
     Some(ty)
 }
 
-struct LiteralReturnVisitor;
+enum ReturnTy {
+    Str,
+    Slice,
+    Array,
+}
+
+fn fetch_return_mode(cx: &LateContext<'_>, hir_ty: &Ty<'_>) -> Option<ReturnTy> {
+    match &hir_ty.kind {
+        TyKind::Array(_, _) => Some(ReturnTy::Array),
+        TyKind::Slice(_) => Some(ReturnTy::Slice),
+        TyKind::Path(other) => {
+            if let Res::PrimTy(PrimTy::Str) = cx.qpath_res(other, hir_ty.hir_id) {
+                Some(ReturnTy::Str)
+            } else {
+                None
+            }
+        },
+        _ => None,
+    }
+}
+
+struct LiteralReturnVisitor {
+    return_mode: ReturnTy,
+}
 
 impl ReturnVisitor for LiteralReturnVisitor {
     type Result = std::ops::ControlFlow<()>;
     fn visit_return(&mut self, kind: ReturnType<'_>) -> Self::Result {
         let expr = match kind {
             ReturnType::Implicit(e) | ReturnType::Explicit(e) => e,
             ReturnType::UnitReturnExplicit(_) | ReturnType::MissingElseImplicit(_) => {
-                panic!("Function which returns `&str` has a unit return!")
+                panic!("Function which returns a type has a unit return!")
             },
             ReturnType::DivergingImplicit(_) => {
-                // If this block is implicitly returning `!`, it can return `&'static str`.
+                // If this block is implicitly returning `!`, it can return `&'static T`.
                 return Self::Result::Continue(());
             },
         };
 
-        if matches!(
-            expr.kind,
-            ExprKind::Lit(Lit {
-                node: LitKind::Str(..),
-                ..
-            })
-        ) {
+        let returns_literal = match self.return_mode {
+            ReturnTy::Str => matches!(
+                expr.kind,
+                ExprKind::Lit(Lit {
+                    node: LitKind::Str(..),
+                    ..
+                })
+            ),
+            ReturnTy::Slice | ReturnTy::Array => matches!(
+                expr.kind,
+                ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, Expr {
+                    kind: ExprKind::Array(_),
+                    ..
+                })
+            ),
+        };
+
+        if returns_literal {
             Self::Result::Continue(())
         } else {
             Self::Result::Break(())
@@ -116,7 +152,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryLiteralBound {
             return;
         }
 
-        // Check for `-> &str`
+        // Check for `-> &str/&[T]/&[T; N]`
         let FnRetTy::Return(ret_hir_ty) = decl.output else {
             return;
         };
@@ -125,20 +161,22 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryLiteralBound {
             return;
         };
 
-        if path_res(cx, inner_hir_ty) != Res::PrimTy(PrimTy::Str) {
+        let Some(return_mode) = fetch_return_mode(cx, inner_hir_ty) else {
             return;
-        }
+        };
 
         // Check for all return statements returning literals
-        if visit_returns(LiteralReturnVisitor, body.value).is_continue() {
+        if visit_returns(LiteralReturnVisitor { return_mode }, body.value).is_continue() {
+            let mut applicability = Applicability::MachineApplicable;
+            let snippet = snippet_with_applicability(cx, inner_hir_ty.span, "..", &mut applicability);
             span_lint_and_sugg(
                 cx,
                 UNNECESSARY_LITERAL_BOUND,
                 ret_hir_ty.span,
-                "returning a `str` unnecessarily tied to the lifetime of arguments",
+                "returning a literal unnecessarily tied to the lifetime of arguments",
                 "try",
-                "&'static str".into(), // how ironic, a lint about `&'static str` requiring a `String` alloc...
-                Applicability::MachineApplicable,
+                format!("&'static {snippet}"),
+                applicability,
             );
         }
     }

tests/ui/unnecessary_literal_bound.fixed

@@ -2,20 +2,35 @@
 
 struct Struct<'a> {
     not_literal: &'a str,
+    non_literal_b: &'a [u8],
 }
 
 impl Struct<'_> {
     fn returns_lit(&self) -> &'static str {
-        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
+        //~^ error: returning a literal unnecessarily tied to the lifetime of arguments
         "Hello"
     }
 
+    fn returns_lit_b(&self) -> &'static [u8] {
+        //~^ error: returning a literal unnecessarily tied to the lifetime of arguments
+        &[0, 1, 2]
+    }
+
+    fn returns_lit_b_fixed(&self) -> &'static [u8; 3] {
+        //~^ error: returning a literal unnecessarily tied to the lifetime of arguments
+        &[0, 1, 2]
+    }
+
     fn returns_non_lit(&self) -> &str {
         self.not_literal
     }
 
+    fn returns_non_lit_b(&self) -> &[u8] {
+        self.non_literal_b
+    }
+
     fn conditionally_returns_lit(&self, cond: bool) -> &'static str {
-        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
+        //~^ error: returning a literal unnecessarily tied to the lifetime of arguments
         if cond { "Literal" } else { "also a literal" }
     }
 
@@ -24,7 +39,7 @@ impl Struct<'_> {
     }
 
     fn contionally_returns_literals_explicit(&self, cond: bool) -> &'static str {
-        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
+        //~^ error: returning a literal unnecessarily tied to the lifetime of arguments
         if cond {
             return "Literal";
         }
@@ -47,7 +62,7 @@ trait ReturnsStr {
 
 impl ReturnsStr for u8 {
     fn trait_method(&self) -> &'static str {
-        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
+        //~^ error: returning a literal unnecessarily tied to the lifetime of arguments
         "Literal"
     }
 }

tests/ui/unnecessary_literal_bound.rs

@@ -2,20 +2,35 @@
 
 struct Struct<'a> {
     not_literal: &'a str,
+    non_literal_b: &'a [u8],
 }
 
 impl Struct<'_> {
     fn returns_lit(&self) -> &str {
-        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
+        //~^ error: returning a literal unnecessarily tied to the lifetime of arguments
         "Hello"
     }
 
+    fn returns_lit_b(&self) -> &[u8] {
+        //~^ error: returning a literal unnecessarily tied to the lifetime of arguments
+        &[0, 1, 2]
+    }
+
+    fn returns_lit_b_fixed(&self) -> &[u8; 3] {
+        //~^ error: returning a literal unnecessarily tied to the lifetime of arguments
+        &[0, 1, 2]
+    }
+
     fn returns_non_lit(&self) -> &str {
         self.not_literal
     }
 
+    fn returns_non_lit_b(&self) -> &[u8] {
+        self.non_literal_b
+    }
+
     fn conditionally_returns_lit(&self, cond: bool) -> &str {
-        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
+        //~^ error: returning a literal unnecessarily tied to the lifetime of arguments
         if cond { "Literal" } else { "also a literal" }
     }
 
@@ -24,7 +39,7 @@ impl Struct<'_> {
     }
 
     fn contionally_returns_literals_explicit(&self, cond: bool) -> &str {
-        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
+        //~^ error: returning a literal unnecessarily tied to the lifetime of arguments
         if cond {
             return "Literal";
         }
@@ -47,7 +62,7 @@ trait ReturnsStr {
 
 impl ReturnsStr for u8 {
     fn trait_method(&self) -> &str {
-        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
+        //~^ error: returning a literal unnecessarily tied to the lifetime of arguments
         "Literal"
     }
 }

tests/ui/unnecessary_literal_bound.stderr

@@ -1,29 +1,41 @@
-error: returning a `str` unnecessarily tied to the lifetime of arguments
-  --> tests/ui/unnecessary_literal_bound.rs:8:30
+error: returning a literal unnecessarily tied to the lifetime of arguments
+  --> tests/ui/unnecessary_literal_bound.rs:9:30
    |
 LL |     fn returns_lit(&self) -> &str {
    |                              ^^^^ help: try: `&'static str`
    |
    = note: `-D clippy::unnecessary-literal-bound` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::unnecessary_literal_bound)]`
 
-error: returning a `str` unnecessarily tied to the lifetime of arguments
-  --> tests/ui/unnecessary_literal_bound.rs:17:56
+error: returning a literal unnecessarily tied to the lifetime of arguments
+  --> tests/ui/unnecessary_literal_bound.rs:14:32
+   |
+LL |     fn returns_lit_b(&self) -> &[u8] {
+   |                                ^^^^^ help: try: `&'static [u8]`
+
+error: returning a literal unnecessarily tied to the lifetime of arguments
+  --> tests/ui/unnecessary_literal_bound.rs:19:38
+   |
+LL |     fn returns_lit_b_fixed(&self) -> &[u8; 3] {
+   |                                      ^^^^^^^^ help: try: `&'static [u8; 3]`
+
+error: returning a literal unnecessarily tied to the lifetime of arguments
+  --> tests/ui/unnecessary_literal_bound.rs:32:56
    |
 LL |     fn conditionally_returns_lit(&self, cond: bool) -> &str {
    |                                                        ^^^^ help: try: `&'static str`
 
-error: returning a `str` unnecessarily tied to the lifetime of arguments
-  --> tests/ui/unnecessary_literal_bound.rs:26:68
+error: returning a literal unnecessarily tied to the lifetime of arguments
+  --> tests/ui/unnecessary_literal_bound.rs:41:68
    |
 LL |     fn contionally_returns_literals_explicit(&self, cond: bool) -> &str {
    |                                                                    ^^^^ help: try: `&'static str`
 
-error: returning a `str` unnecessarily tied to the lifetime of arguments
-  --> tests/ui/unnecessary_literal_bound.rs:49:31
+error: returning a literal unnecessarily tied to the lifetime of arguments
+  --> tests/ui/unnecessary_literal_bound.rs:64:31
    |
 LL |     fn trait_method(&self) -> &str {
    |                               ^^^^ help: try: `&'static str`
 
-error: aborting due to 4 previous errors
+error: aborting due to 6 previous errors