Skip to content

Commit 174b63c

Browse files
committed
fix incorrect suggestions related to parentheses in needless_return
1 parent e02c885 commit 174b63c

File tree

4 files changed

+99
-71
lines changed

4 files changed

+99
-71
lines changed

Diff for: clippy_lints/src/returns.rs

+10-12
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
2-
use clippy_utils::source::{SpanRangeExt, snippet_with_context};
3-
use clippy_utils::sugg::has_enclosing_paren;
2+
use clippy_utils::source::SpanRangeExt;
3+
use clippy_utils::sugg::{Sugg, has_enclosing_paren};
44
use clippy_utils::visitors::{Descend, for_each_expr};
55
use clippy_utils::{
66
binary_expr_needs_parentheses, fn_def_id, is_from_proc_macro, is_inside_let_else, is_res_lang_ctor,
@@ -137,7 +137,6 @@ enum RetReplacement<'tcx> {
137137
Empty,
138138
Block,
139139
Unit,
140-
NeedsPar(Cow<'tcx, str>, Applicability),
141140
Expr(Cow<'tcx, str>, Applicability),
142141
}
143142

@@ -147,13 +146,12 @@ impl RetReplacement<'_> {
147146
Self::Empty | Self::Expr(..) => "remove `return`",
148147
Self::Block => "replace `return` with an empty block",
149148
Self::Unit => "replace `return` with a unit value",
150-
Self::NeedsPar(..) => "remove `return` and wrap the sequence with parentheses",
151149
}
152150
}
153151

154152
fn applicability(&self) -> Applicability {
155153
match self {
156-
Self::Expr(_, ap) | Self::NeedsPar(_, ap) => *ap,
154+
Self::Expr(_, ap) => *ap,
157155
_ => Applicability::MachineApplicable,
158156
}
159157
}
@@ -165,7 +163,6 @@ impl Display for RetReplacement<'_> {
165163
Self::Empty => write!(f, ""),
166164
Self::Block => write!(f, "{{}}"),
167165
Self::Unit => write!(f, "()"),
168-
Self::NeedsPar(inner, _) => write!(f, "({inner})"),
169166
Self::Expr(inner, _) => write!(f, "{inner}"),
170167
}
171168
}
@@ -365,12 +362,13 @@ fn check_final_expr<'tcx>(
365362
}
366363

367364
let mut applicability = Applicability::MachineApplicable;
368-
let (snippet, _) = snippet_with_context(cx, inner_expr.span, ret_span.ctxt(), "..", &mut applicability);
369-
if binary_expr_needs_parentheses(inner_expr) {
370-
RetReplacement::NeedsPar(snippet, applicability)
371-
} else {
372-
RetReplacement::Expr(snippet, applicability)
373-
}
365+
let sugg = Sugg::hir_with_context(cx, inner_expr, ret_span.ctxt(), "..", &mut applicability);
366+
let snippet = match sugg {
367+
Sugg::BinOp(..) => sugg.maybe_par().to_string(),
368+
_ => sugg.to_string(),
369+
};
370+
371+
RetReplacement::Expr(snippet.into(), applicability)
374372
} else {
375373
match match_ty_opt {
376374
Some(match_ty) => {

Diff for: tests/ui/needless_return.fixed

+10-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
clippy::single_match,
77
clippy::needless_bool,
88
clippy::equatable_if_let,
9-
clippy::needless_else
9+
clippy::needless_else,
10+
clippy::missing_safety_doc
1011
)]
1112
#![warn(clippy::needless_return)]
1213

@@ -388,3 +389,11 @@ fn b(x: Option<u8>) -> Option<u8> {
388389
},
389390
}
390391
}
392+
393+
unsafe fn todo() -> *const u8 {
394+
todo!()
395+
}
396+
397+
pub unsafe fn issue_12157() -> *const i32 {
398+
(unsafe { todo() } as *const i32)
399+
}

Diff for: tests/ui/needless_return.rs

+10-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
clippy::single_match,
77
clippy::needless_bool,
88
clippy::equatable_if_let,
9-
clippy::needless_else
9+
clippy::needless_else,
10+
clippy::missing_safety_doc
1011
)]
1112
#![warn(clippy::needless_return)]
1213

@@ -398,3 +399,11 @@ fn b(x: Option<u8>) -> Option<u8> {
398399
},
399400
}
400401
}
402+
403+
unsafe fn todo() -> *const u8 {
404+
todo!()
405+
}
406+
407+
pub unsafe fn issue_12157() -> *const i32 {
408+
return unsafe { todo() } as *const i32;
409+
}

0 commit comments

Comments
 (0)