Skip to content

Commit 65d10c7

Browse files
jrqcjrqc
committed
Borrow checker added
1 parent 85f4ef0 commit 65d10c7

File tree

4 files changed

+79
-45
lines changed

4 files changed

+79
-45
lines changed

‎clippy_lints/src/needless_return.rs

Lines changed: 58 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
1-
use rustc_lint::{LateLintPass, LateContext};
21
use rustc_ast::ast::Attribute;
3-
use rustc_session::{declare_lint_pass, declare_tool_lint};
42
use rustc_errors::Applicability;
5-
use rustc_hir::intravisit::{FnKind, walk_expr, NestedVisitorMap, Visitor};
6-
use rustc_span::source_map::Span;
7-
use rustc_middle::lint::in_external_macro;
3+
use rustc_hir::intravisit::{walk_expr, FnKind, NestedVisitorMap, Visitor};
4+
use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, HirId, MatchSource, StmtKind};
5+
use rustc_lint::{LateContext, LateLintPass};
86
use rustc_middle::hir::map::Map;
7+
use rustc_middle::lint::in_external_macro;
98
use rustc_middle::ty::subst::GenericArgKind;
10-
use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, HirId, MatchSource, StmtKind};
9+
use rustc_session::{declare_lint_pass, declare_tool_lint};
10+
use rustc_span::source_map::Span;
1111

1212
use crate::utils::{fn_def_id, snippet_opt, span_lint_and_sugg, span_lint_and_then};
1313

@@ -46,16 +46,39 @@ enum RetReplacement {
4646
declare_lint_pass!(NeedlessReturn => [NEEDLESS_RETURN]);
4747

4848
impl<'tcx> LateLintPass<'tcx> for NeedlessReturn {
49-
fn check_fn(&mut self, cx: &LateContext<'tcx>, kind: FnKind<'tcx>, _: &'tcx FnDecl<'tcx>, body: &'tcx Body<'tcx>, _: Span, _: HirId) {
49+
fn check_fn(
50+
&mut self,
51+
cx: &LateContext<'tcx>,
52+
kind: FnKind<'tcx>,
53+
_: &'tcx FnDecl<'tcx>,
54+
body: &'tcx Body<'tcx>,
55+
_: Span,
56+
_: HirId,
57+
) {
5058
match kind {
5159
FnKind::Closure(_) => {
52-
check_final_expr(cx, &body.value, Some(body.value.span), RetReplacement::Empty)
53-
}
60+
if !last_statement_borrows(cx, &body.value) {
61+
check_final_expr(cx, &body.value, Some(body.value.span), RetReplacement::Empty)
62+
}
63+
},
5464
FnKind::ItemFn(..) | FnKind::Method(..) => {
5565
if let ExprKind::Block(ref block, _) = body.value.kind {
56-
check_block_return(cx, block)
66+
if let Some(expr) = block.expr {
67+
if !last_statement_borrows(cx, expr) {
68+
check_final_expr(cx, expr, Some(expr.span), RetReplacement::Empty);
69+
}
70+
} else if let Some(stmt) = block.stmts.iter().last() {
71+
match stmt.kind {
72+
StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => {
73+
if !last_statement_borrows(cx, expr) {
74+
check_final_expr(cx, expr, Some(stmt.span), RetReplacement::Empty);
75+
}
76+
},
77+
_ => (),
78+
}
79+
}
5780
}
58-
}
81+
},
5982
}
6083
}
6184
}
@@ -71,23 +94,16 @@ fn check_block_return(cx: &LateContext<'_>, block: &Block<'_>) {
7194
match stmt.kind {
7295
StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => {
7396
check_final_expr(cx, expr, Some(stmt.span), RetReplacement::Empty);
74-
}
97+
},
7598
_ => (),
7699
}
77100
}
78101
}
79102

80-
81-
fn check_final_expr(
82-
cx: &LateContext<'_>,
83-
expr: &Expr<'_>,
84-
span: Option<Span>,
85-
replacement: RetReplacement,
86-
) {
103+
fn check_final_expr(cx: &LateContext<'_>, expr: &Expr<'_>, span: Option<Span>, replacement: RetReplacement) {
87104
match expr.kind {
88105
// simple return is always "bad"
89106
ExprKind::Ret(ref inner) => {
90-
91107
// allow `#[cfg(a)] return a; #[cfg(b)] return b;`
92108
if !expr.attrs.iter().any(attr_is_cfg) {
93109
emit_return_lint(
@@ -97,32 +113,34 @@ fn check_final_expr(
97113
replacement,
98114
);
99115
}
100-
}
116+
},
101117
// a whole block? check it!
102118
ExprKind::Block(ref block, _) => {
103119
check_block_return(cx, block);
104-
}
120+
},
105121
// a match expr, check all arms
106122
// an if/if let expr, check both exprs
107123
// note, if without else is going to be a type checking error anyways
108124
// (except for unit type functions) so we don't match it
109-
110-
ExprKind::Match(_, ref arms, source) => {
111-
match source {
112-
MatchSource::Normal => {
113-
for arm in arms.iter() {
114-
check_final_expr(cx, &arm.body, Some(arm.body.span), RetReplacement::Block);
115-
}
116-
}
117-
MatchSource::IfDesugar { contains_else_clause: true } | MatchSource::IfLetDesugar { contains_else_clause: true } => {
118-
if let ExprKind::Block(ref ifblock, _) = arms[0].body.kind {
119-
check_block_return(cx, ifblock);
120-
}
121-
check_final_expr(cx, arms[1].body, None, RetReplacement::Empty);
125+
ExprKind::Match(_, ref arms, source) => match source {
126+
MatchSource::Normal => {
127+
for arm in arms.iter() {
128+
check_final_expr(cx, &arm.body, Some(arm.body.span), RetReplacement::Block);
122129
}
123-
_ => ()
130+
},
131+
MatchSource::IfDesugar {
132+
contains_else_clause: true,
124133
}
125-
}
134+
| MatchSource::IfLetDesugar {
135+
contains_else_clause: true,
136+
} => {
137+
if let ExprKind::Block(ref ifblock, _) = arms[0].body.kind {
138+
check_block_return(cx, ifblock);
139+
}
140+
check_final_expr(cx, arms[1].body, None, RetReplacement::Empty);
141+
},
142+
_ => (),
143+
},
126144
_ => (),
127145
}
128146
}
@@ -139,7 +157,7 @@ fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, inner_span: Option<Spa
139157
diag.span_suggestion(ret_span, "remove `return`", snippet, Applicability::MachineApplicable);
140158
}
141159
})
142-
}
160+
},
143161
None => match replacement {
144162
RetReplacement::Empty => {
145163
span_lint_and_sugg(
@@ -151,7 +169,7 @@ fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, inner_span: Option<Spa
151169
String::new(),
152170
Applicability::MachineApplicable,
153171
);
154-
}
172+
},
155173
RetReplacement::Block => {
156174
span_lint_and_sugg(
157175
cx,
@@ -162,7 +180,7 @@ fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, inner_span: Option<Spa
162180
"{}".to_string(),
163181
Applicability::MachineApplicable,
164182
);
165-
}
183+
},
166184
},
167185
}
168186
}
@@ -204,4 +222,3 @@ impl<'tcx> Visitor<'tcx> for BorrowVisitor<'_, 'tcx> {
204222
NestedVisitorMap::None
205223
}
206224
}
207-

‎clippy_lints/src/returns.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};
77
use rustc_span::source_map::Span;
88
use rustc_span::BytePos;
99

10-
use crate::utils::{span_lint_and_sugg};
10+
use crate::utils::span_lint_and_sugg;
1111

1212
declare_clippy_lint! {
1313
/// **What it does:** Checks for unit (`()`) expressions that can be removed.
@@ -30,12 +30,10 @@ declare_clippy_lint! {
3030
"needless unit expression"
3131
}
3232

33-
3433
declare_lint_pass!(Return => [UNUSED_UNIT]);
3534

3635
impl EarlyLintPass for Return {
3736
fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, span: Span, _: ast::NodeId) {
38-
3937
if_chain! {
4038
if let ast::FnRetTy::Ty(ref ty) = kind.decl().output;
4139
if let ast::TyKind::Tup(ref vals) = ty.kind;
@@ -102,7 +100,6 @@ impl EarlyLintPass for Return {
102100
}
103101
}
104102

105-
106103
// get the def site
107104
#[must_use]
108105
fn get_def(span: Span) -> Option<Span> {

‎tests/ui/needless_return.fixed

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,16 @@ fn test_void_match(x: u32) {
6969
}
7070
}
7171

72+
mod no_lint_if_stmt_borrows {
73+
mod issue_5858 {
74+
fn read_line() -> String {
75+
use std::io::BufRead;
76+
let stdin = ::std::io::stdin();
77+
return stdin.lock().lines().next().unwrap().unwrap();
78+
}
79+
}
80+
}
81+
7282
fn main() {
7383
let _ = test_end_of_fn();
7484
let _ = test_no_semicolon();

‎tests/ui/needless_return.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,16 @@ fn test_void_match(x: u32) {
6969
}
7070
}
7171

72+
mod no_lint_if_stmt_borrows {
73+
mod issue_5858 {
74+
fn read_line() -> String {
75+
use std::io::BufRead;
76+
let stdin = ::std::io::stdin();
77+
return stdin.lock().lines().next().unwrap().unwrap();
78+
}
79+
}
80+
}
81+
7282
fn main() {
7383
let _ = test_end_of_fn();
7484
let _ = test_no_semicolon();

0 commit comments

Comments
 (0)