Skip to content

Commit 4cb53cd

Browse files
GnomedDevGnomedDev
committed
Use ReturnVisitor in unnecessary_literal_bound
1 parent f6449d6 commit 4cb53cd

File tree

5 files changed

+46
-61
lines changed

5 files changed

+46
-61
lines changed

‎clippy_lints/src/unnecessary_literal_bound.rs

Lines changed: 26 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::path_res;
2+
use clippy_utils::{ReturnType, ReturnVisitor, path_res, visit_returns};
33
use rustc_ast::ast::LitKind;
44
use rustc_errors::Applicability;
55
use rustc_hir::def::Res;
6-
use rustc_hir::intravisit::{FnKind, Visitor};
7-
use rustc_hir::{Body, Expr, ExprKind, FnDecl, FnRetTy, Lit, MutTy, Mutability, PrimTy, Ty, TyKind, intravisit};
6+
use rustc_hir::intravisit::FnKind;
7+
use rustc_hir::{Body, ExprKind, FnDecl, FnRetTy, Lit, MutTy, Mutability, PrimTy, Ty, TyKind};
88
use rustc_lint::{LateContext, LateLintPass};
99
use rustc_session::declare_lint_pass;
1010
use rustc_span::Span;
@@ -67,49 +67,36 @@ fn extract_anonymous_ref<'tcx>(hir_ty: &Ty<'tcx>) -> Option<&'tcx Ty<'tcx>> {
6767
Some(ty)
6868
}
6969

70-
fn is_str_literal(expr: &Expr<'_>) -> bool {
71-
matches!(
72-
expr.kind,
73-
ExprKind::Lit(Lit {
74-
node: LitKind::Str(..),
75-
..
76-
}),
77-
)
78-
}
79-
80-
struct FindNonLiteralReturn;
70+
struct LiteralReturnVisitor;
8171

82-
impl<'hir> Visitor<'hir> for FindNonLiteralReturn {
72+
impl ReturnVisitor for LiteralReturnVisitor {
8373
type Result = std::ops::ControlFlow<()>;
84-
type NestedFilter = intravisit::nested_filter::None;
74+
fn visit_return(&mut self, kind: ReturnType<'_>) -> Self::Result {
75+
let expr = match kind {
76+
ReturnType::Implicit(e) | ReturnType::Explicit(e) => e,
77+
ReturnType::UnitReturnExplicit(_) | ReturnType::MissingElseImplicit(_) => {
78+
panic!("Function which returns `&str` has a unit return!")
79+
},
80+
ReturnType::DivergingImplicit(_) => {
81+
// If this block is implicitly returning `!`, it can return `&'static str`.
82+
return Self::Result::Continue(());
83+
},
84+
};
8585

86-
fn visit_expr(&mut self, expr: &'hir Expr<'hir>) -> Self::Result {
87-
if let ExprKind::Ret(Some(ret_val_expr)) = expr.kind
88-
&& !is_str_literal(ret_val_expr)
89-
{
90-
Self::Result::Break(())
86+
if matches!(
87+
expr.kind,
88+
ExprKind::Lit(Lit {
89+
node: LitKind::Str(..),
90+
..
91+
})
92+
) {
93+
Self::Result::Continue(())
9194
} else {
92-
intravisit::walk_expr(self, expr)
95+
Self::Result::Break(())
9396
}
9497
}
9598
}
9699

97-
fn check_implicit_returns_static_str(body: &Body<'_>) -> bool {
98-
// TODO: Improve this to the same complexity as the Visitor to catch more implicit return cases.
99-
if let ExprKind::Block(block, _) = body.value.kind
100-
&& let Some(implicit_ret) = block.expr
101-
{
102-
return is_str_literal(implicit_ret);
103-
}
104-
105-
false
106-
}
107-
108-
fn check_explicit_returns_static_str(expr: &Expr<'_>) -> bool {
109-
let mut visitor = FindNonLiteralReturn;
110-
visitor.visit_expr(expr).is_continue()
111-
}
112-
113100
impl<'tcx> LateLintPass<'tcx> for UnnecessaryLiteralBound {
114101
fn check_fn(
115102
&mut self,
@@ -143,7 +130,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryLiteralBound {
143130
}
144131

145132
// Check for all return statements returning literals
146-
if check_explicit_returns_static_str(body.value) && check_implicit_returns_static_str(body) {
133+
if visit_returns(LiteralReturnVisitor, body.value).is_continue() {
147134
span_lint_and_sugg(
148135
cx,
149136
UNNECESSARY_LITERAL_BOUND,

‎clippy_utils/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ pub use self::check_proc_macro::{is_from_proc_macro, is_span_if, is_span_match};
8484
pub use self::hir_utils::{
8585
HirEqInterExpr, SpanlessEq, SpanlessHash, both, count_eq, eq_expr_value, hash_expr, hash_stmt, is_bool, over,
8686
};
87-
pub use returns::{ReturnVisitor, visit_returns};
87+
pub use returns::{ReturnType, ReturnVisitor, visit_returns};
8888

8989
use core::mem;
9090
use core::ops::ControlFlow;

‎tests/ui/unnecessary_literal_bound.fixed

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,36 +5,33 @@ struct Struct<'a> {
55
}
66

77
impl Struct<'_> {
8-
// Should warn
98
fn returns_lit(&self) -> &'static str {
9+
//~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
1010
"Hello"
1111
}
1212

13-
// Should NOT warn
1413
fn returns_non_lit(&self) -> &str {
1514
self.not_literal
1615
}
1716

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
2019
if cond { "Literal" } else { "also a literal" }
2120
}
2221

23-
// Should NOT warn
2422
fn conditionally_returns_non_lit(&self, cond: bool) -> &str {
2523
if cond { "Literal" } else { self.not_literal }
2624
}
2725

28-
// Should warn
2926
fn contionally_returns_literals_explicit(&self, cond: bool) -> &'static str {
27+
//~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
3028
if cond {
3129
return "Literal";
3230
}
3331

3432
"also a literal"
3533
}
3634

37-
// Should NOT warn
3835
fn conditionally_returns_non_lit_explicit(&self, cond: bool) -> &str {
3936
if cond {
4037
return self.not_literal;
@@ -49,14 +46,13 @@ trait ReturnsStr {
4946
}
5047

5148
impl ReturnsStr for u8 {
52-
// Should warn, even though not useful without trait refinement
5349
fn trait_method(&self) -> &'static str {
50+
//~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
5451
"Literal"
5552
}
5653
}
5754

5855
impl ReturnsStr for Struct<'_> {
59-
// Should NOT warn
6056
fn trait_method(&self) -> &str {
6157
self.not_literal
6258
}

‎tests/ui/unnecessary_literal_bound.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,36 +5,33 @@ struct Struct<'a> {
55
}
66

77
impl Struct<'_> {
8-
// Should warn
98
fn returns_lit(&self) -> &str {
9+
//~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
1010
"Hello"
1111
}
1212

13-
// Should NOT warn
1413
fn returns_non_lit(&self) -> &str {
1514
self.not_literal
1615
}
1716

18-
// Should warn, does not currently
1917
fn conditionally_returns_lit(&self, cond: bool) -> &str {
18+
//~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
2019
if cond { "Literal" } else { "also a literal" }
2120
}
2221

23-
// Should NOT warn
2422
fn conditionally_returns_non_lit(&self, cond: bool) -> &str {
2523
if cond { "Literal" } else { self.not_literal }
2624
}
2725

28-
// Should warn
2926
fn contionally_returns_literals_explicit(&self, cond: bool) -> &str {
27+
//~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
3028
if cond {
3129
return "Literal";
3230
}
3331

3432
"also a literal"
3533
}
3634

37-
// Should NOT warn
3835
fn conditionally_returns_non_lit_explicit(&self, cond: bool) -> &str {
3936
if cond {
4037
return self.not_literal;
@@ -49,14 +46,13 @@ trait ReturnsStr {
4946
}
5047

5148
impl ReturnsStr for u8 {
52-
// Should warn, even though not useful without trait refinement
5349
fn trait_method(&self) -> &str {
50+
//~^ error: returning a `str` unnecessarily tied to the lifetime of arguments
5451
"Literal"
5552
}
5653
}
5754

5855
impl ReturnsStr for Struct<'_> {
59-
// Should NOT warn
6056
fn trait_method(&self) -> &str {
6157
self.not_literal
6258
}

‎tests/ui/unnecessary_literal_bound.stderr

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: returning a `str` unnecessarily tied to the lifetime of arguments
2-
--> tests/ui/unnecessary_literal_bound.rs:9:30
2+
--> tests/ui/unnecessary_literal_bound.rs:8:30
33
|
44
LL | fn returns_lit(&self) -> &str {
55
| ^^^^ help: try: `&'static str`
@@ -8,16 +8,22 @@ LL | fn returns_lit(&self) -> &str {
88
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_literal_bound)]`
99

1010
error: returning a `str` unnecessarily tied to the lifetime of arguments
11-
--> tests/ui/unnecessary_literal_bound.rs:29:68
11+
--> tests/ui/unnecessary_literal_bound.rs:17:56
12+
|
13+
LL | fn conditionally_returns_lit(&self, cond: bool) -> &str {
14+
| ^^^^ help: try: `&'static str`
15+
16+
error: returning a `str` unnecessarily tied to the lifetime of arguments
17+
--> tests/ui/unnecessary_literal_bound.rs:26:68
1218
|
1319
LL | fn contionally_returns_literals_explicit(&self, cond: bool) -> &str {
1420
| ^^^^ help: try: `&'static str`
1521

1622
error: returning a `str` unnecessarily tied to the lifetime of arguments
17-
--> tests/ui/unnecessary_literal_bound.rs:53:31
23+
--> tests/ui/unnecessary_literal_bound.rs:49:31
1824
|
1925
LL | fn trait_method(&self) -> &str {
2026
| ^^^^ help: try: `&'static str`
2127

22-
error: aborting due to 3 previous errors
28+
error: aborting due to 4 previous errors
2329

0 commit comments

Comments
 (0)