Use ReturnVisitor in unnecessary_literal_bound

GnomedDev · GnomedDev · commit 6c5a5ce2dfdd · 2024-10-16T21:34:37.000+01:00
diff --git a/clippy_lints/src/unnecessary_literal_bound.rs b/clippy_lints/src/unnecessary_literal_bound.rs
@@ -1,10 +1,10 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::path_res;
+use clippy_utils::{ReturnVisitor, path_res, visit_returns};
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
-use rustc_hir::intravisit::{FnKind, Visitor};
-use rustc_hir::{Body, Expr, ExprKind, FnDecl, FnRetTy, Lit, MutTy, Mutability, PrimTy, Ty, TyKind, intravisit};
+use rustc_hir::intravisit::FnKind;
+use rustc_hir::{Block, 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;
@@ -67,47 +67,28 @@ fn extract_anonymous_ref<'tcx>(hir_ty: &Ty<'tcx>) -> Option<&'tcx Ty<'tcx>> {
     Some(ty)
 }
 
-fn is_str_literal(expr: &Expr<'_>) -> bool {
-    matches!(
-        expr.kind,
-        ExprKind::Lit(Lit {
-            node: LitKind::Str(..),
-            ..
-        }),
-    )
-}
-
-struct FindNonLiteralReturn;
+struct LiteralReturnVisitor;
 
-impl<'hir> Visitor<'hir> for FindNonLiteralReturn {
+impl ReturnVisitor for LiteralReturnVisitor {
     type Result = std::ops::ControlFlow<()>;
-    type NestedFilter = intravisit::nested_filter::None;
-
-    fn visit_expr(&mut self, expr: &'hir Expr<'hir>) -> Self::Result {
-        if let ExprKind::Ret(Some(ret_val_expr)) = expr.kind
-            && !is_str_literal(ret_val_expr)
-        {
-            Self::Result::Break(())
+    fn visit_return(&mut self, expr: &Expr<'_>) -> Self::Result {
+        if matches!(
+            expr.kind,
+            ExprKind::Lit(Lit {
+                node: LitKind::Str(..),
+                ..
+            })
+        ) {
+            Self::Result::Continue(())
         } else {
-            intravisit::walk_expr(self, expr)
+            Self::Result::Break(())
         }
     }
-}
 
-fn check_implicit_returns_static_str(body: &Body<'_>) -> bool {
-    // TODO: Improve this to the same complexity as the Visitor to catch more implicit return cases.
-    if let ExprKind::Block(block, _) = body.value.kind
-        && let Some(implicit_ret) = block.expr
-    {
-        return is_str_literal(implicit_ret);
+    fn visit_diverging_implicit_return(&mut self, _: &Block<'_>) -> Self::Result {
+        // If this block is implicitly returning `!`, it can return `&'static str`.
+        Self::Result::Continue(())
     }
-
-    false
-}
-
-fn check_explicit_returns_static_str(expr: &Expr<'_>) -> bool {
-    let mut visitor = FindNonLiteralReturn;
-    visitor.visit_expr(expr).is_continue()
 }
 
 impl<'tcx> LateLintPass<'tcx> for UnnecessaryLiteralBound {
@@ -143,7 +124,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryLiteralBound {
         }
 
         // Check for all return statements returning literals
-        if check_explicit_returns_static_str(body.value) && check_implicit_returns_static_str(body) {
+        if visit_returns(LiteralReturnVisitor, body.value).is_continue() {
             span_lint_and_sugg(
                 cx,
                 UNNECESSARY_LITERAL_BOUND,
diff --git a/tests/ui/unnecessary_literal_bound.fixed b/tests/ui/unnecessary_literal_bound.fixed
@@ -5,36 +5,33 @@ struct Struct<'a> {
 }
 
 impl Struct<'_> {
-    // Should warn
     fn returns_lit(&self) -> &'static str {
+        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
         "Hello"
     }
 
-    // Should NOT warn
     fn returns_non_lit(&self) -> &str {
         self.not_literal
     }
 
-    // Should warn, does not currently
-    fn conditionally_returns_lit(&self, cond: bool) -> &str {
+    fn conditionally_returns_lit(&self, cond: bool) -> &'static str {
+        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
         if cond { "Literal" } else { "also a literal" }
     }
 
-    // Should NOT warn
     fn conditionally_returns_non_lit(&self, cond: bool) -> &str {
         if cond { "Literal" } else { self.not_literal }
     }
 
-    // Should warn
     fn contionally_returns_literals_explicit(&self, cond: bool) -> &'static str {
+        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
         if cond {
             return "Literal";
         }
 
         "also a literal"
     }
 
-    // Should NOT warn
     fn conditionally_returns_non_lit_explicit(&self, cond: bool) -> &str {
         if cond {
             return self.not_literal;
@@ -49,14 +46,13 @@ trait ReturnsStr {
 }
 
 impl ReturnsStr for u8 {
-    // Should warn, even though not useful without trait refinement
     fn trait_method(&self) -> &'static str {
+        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
         "Literal"
     }
 }
 
 impl ReturnsStr for Struct<'_> {
-    // Should NOT warn
     fn trait_method(&self) -> &str {
         self.not_literal
     }
diff --git a/tests/ui/unnecessary_literal_bound.rs b/tests/ui/unnecessary_literal_bound.rs
@@ -5,36 +5,33 @@ struct Struct<'a> {
 }
 
 impl Struct<'_> {
-    // Should warn
     fn returns_lit(&self) -> &str {
+        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
         "Hello"
     }
 
-    // Should NOT warn
     fn returns_non_lit(&self) -> &str {
         self.not_literal
     }
 
-    // Should warn, does not currently
     fn conditionally_returns_lit(&self, cond: bool) -> &str {
+        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
         if cond { "Literal" } else { "also a literal" }
     }
 
-    // Should NOT warn
     fn conditionally_returns_non_lit(&self, cond: bool) -> &str {
         if cond { "Literal" } else { self.not_literal }
     }
 
-    // Should warn
     fn contionally_returns_literals_explicit(&self, cond: bool) -> &str {
+        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
         if cond {
             return "Literal";
         }
 
         "also a literal"
     }
 
-    // Should NOT warn
     fn conditionally_returns_non_lit_explicit(&self, cond: bool) -> &str {
         if cond {
             return self.not_literal;
@@ -49,14 +46,13 @@ trait ReturnsStr {
 }
 
 impl ReturnsStr for u8 {
-    // Should warn, even though not useful without trait refinement
     fn trait_method(&self) -> &str {
+        //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
         "Literal"
     }
 }
 
 impl ReturnsStr for Struct<'_> {
-    // Should NOT warn
     fn trait_method(&self) -> &str {
         self.not_literal
     }
diff --git a/tests/ui/unnecessary_literal_bound.stderr b/tests/ui/unnecessary_literal_bound.stderr
@@ -1,5 +1,5 @@
 error: returning a `str` unnecessarily tied to the lifetime of arguments
-  --> tests/ui/unnecessary_literal_bound.rs:9:30
+  --> tests/ui/unnecessary_literal_bound.rs:8:30
    |
 LL |     fn returns_lit(&self) -> &str {
    |                              ^^^^ help: try: `&'static str`
@@ -8,16 +8,22 @@ LL |     fn returns_lit(&self) -> &str {
    = 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:29:68
+  --> tests/ui/unnecessary_literal_bound.rs:17: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
    |
 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:53:31
+  --> tests/ui/unnecessary_literal_bound.rs:49:31
    |
 LL |     fn trait_method(&self) -> &str {
    |                               ^^^^ help: try: `&'static str`
 
-error: aborting due to 3 previous errors
+error: aborting due to 4 previous errors
 

Original file line number	Diff line number	Diff line change
`@@ -5,36 +5,33 @@ struct Struct<'a> {`
`5`	`5`	`}`
`6`	`6`
`7`	`7`	`impl Struct<'_> {`
`8`		`- // Should warn`
`9`	`8`	`fn returns_lit(&self) -> &'static str {`
	`9`	+ //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
`10`	`10`	`"Hello"`
`11`	`11`	`}`
`12`	`12`
`13`		`- // Should NOT warn`
`14`	`13`	`fn returns_non_lit(&self) -> &str {`
`15`	`14`	`self.not_literal`
`16`	`15`	`}`
`17`	`16`
`18`		`- // Should warn, does not currently`
`19`		`- fn conditionally_returns_lit(&self, cond: bool) -> &str {`
	`17`	`+ fn conditionally_returns_lit(&self, cond: bool) -> &'static str {`
	`18`	+ //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
`20`	`19`	`if cond { "Literal" } else { "also a literal" }`
`21`	`20`	`}`
`22`	`21`
`23`		`- // Should NOT warn`
`24`	`22`	`fn conditionally_returns_non_lit(&self, cond: bool) -> &str {`
`25`	`23`	`if cond { "Literal" } else { self.not_literal }`
`26`	`24`	`}`
`27`	`25`
`28`		`- // Should warn`
`29`	`26`	`fn contionally_returns_literals_explicit(&self, cond: bool) -> &'static str {`
	`27`	+ //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
`30`	`28`	`if cond {`
`31`	`29`	`return "Literal";`
`32`	`30`	`}`
`33`	`31`
`34`	`32`	`"also a literal"`
`35`	`33`	`}`
`36`	`34`
`37`		`- // Should NOT warn`
`38`	`35`	`fn conditionally_returns_non_lit_explicit(&self, cond: bool) -> &str {`
`39`	`36`	`if cond {`
`40`	`37`	`return self.not_literal;`
`@@ -49,14 +46,13 @@ trait ReturnsStr {`
`49`	`46`	`}`
`50`	`47`
`51`	`48`	`impl ReturnsStr for u8 {`
`52`		`- // Should warn, even though not useful without trait refinement`
`53`	`49`	`fn trait_method(&self) -> &'static str {`
	`50`	+ //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
`54`	`51`	`"Literal"`
`55`	`52`	`}`
`56`	`53`	`}`
`57`	`54`
`58`	`55`	`impl ReturnsStr for Struct<'_> {`
`59`		`- // Should NOT warn`
`60`	`56`	`fn trait_method(&self) -> &str {`
`61`	`57`	`self.not_literal`
`62`	`58`	`}`