about summary refs log tree commit diff
diff options
context:
space:
mode:
authorjrqc <jrqc01@hotmail.com>2020-08-13 15:14:08 +0300
committerjrqc <jrqc01@hotmail.com>2020-08-16 00:24:27 +0300
commit65d10c7abf4752923f040264c79433da8fc234ea (patch)
tree7d4952d15cebf038989e944e5468fe9e73abe943
parent85f4ef0fbd92453cf480af4e3f9eed877071ea2e (diff)
downloadrust-65d10c7abf4752923f040264c79433da8fc234ea.tar.gz
rust-65d10c7abf4752923f040264c79433da8fc234ea.zip
Borrow checker added
-rw-r--r--clippy_lints/src/needless_return.rs99
-rw-r--r--clippy_lints/src/returns.rs5
-rw-r--r--tests/ui/needless_return.fixed10
-rw-r--r--tests/ui/needless_return.rs10
4 files changed, 79 insertions, 45 deletions
diff --git a/clippy_lints/src/needless_return.rs b/clippy_lints/src/needless_return.rs
index 861a7ec558c..ba280cd5a61 100644
--- a/clippy_lints/src/needless_return.rs
+++ b/clippy_lints/src/needless_return.rs
@@ -1,13 +1,13 @@
-use rustc_lint::{LateLintPass, LateContext};
 use rustc_ast::ast::Attribute;
-use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_errors::Applicability;
-use rustc_hir::intravisit::{FnKind, walk_expr, NestedVisitorMap, Visitor};
-use rustc_span::source_map::Span;
-use rustc_middle::lint::in_external_macro;
+use rustc_hir::intravisit::{walk_expr, FnKind, NestedVisitorMap, Visitor};
+use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, HirId, MatchSource, StmtKind};
+use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::hir::map::Map;
+use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::subst::GenericArgKind;
-use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, HirId, MatchSource, StmtKind};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::source_map::Span;
 
 use crate::utils::{fn_def_id, snippet_opt, span_lint_and_sugg, span_lint_and_then};
 
@@ -46,16 +46,39 @@ enum RetReplacement {
 declare_lint_pass!(NeedlessReturn => [NEEDLESS_RETURN]);
 
 impl<'tcx> LateLintPass<'tcx> for NeedlessReturn {
-    fn check_fn(&mut self, cx: &LateContext<'tcx>, kind: FnKind<'tcx>, _: &'tcx FnDecl<'tcx>, body: &'tcx Body<'tcx>, _: Span, _: HirId) {
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'tcx>,
+        kind: FnKind<'tcx>,
+        _: &'tcx FnDecl<'tcx>,
+        body: &'tcx Body<'tcx>,
+        _: Span,
+        _: HirId,
+    ) {
         match kind {
             FnKind::Closure(_) => {
-                check_final_expr(cx, &body.value, Some(body.value.span), RetReplacement::Empty)
-            }
+                if !last_statement_borrows(cx, &body.value) {
+                    check_final_expr(cx, &body.value, Some(body.value.span), RetReplacement::Empty)
+                }
+            },
             FnKind::ItemFn(..) | FnKind::Method(..) => {
                 if let ExprKind::Block(ref block, _) = body.value.kind {
-                    check_block_return(cx, block)
+                    if let Some(expr) = block.expr {
+                        if !last_statement_borrows(cx, expr) {
+                            check_final_expr(cx, expr, Some(expr.span), RetReplacement::Empty);
+                        }
+                    } else if let Some(stmt) = block.stmts.iter().last() {
+                        match stmt.kind {
+                            StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => {
+                                if !last_statement_borrows(cx, expr) {
+                                    check_final_expr(cx, expr, Some(stmt.span), RetReplacement::Empty);
+                                }
+                            },
+                            _ => (),
+                        }
+                    }
                 }
-            }
+            },
         }
     }
 }
@@ -71,23 +94,16 @@ fn check_block_return(cx: &LateContext<'_>, block: &Block<'_>) {
         match stmt.kind {
             StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => {
                 check_final_expr(cx, expr, Some(stmt.span), RetReplacement::Empty);
-            }
+            },
             _ => (),
         }
     }
 }
 
-
-fn check_final_expr(
-    cx: &LateContext<'_>,
-    expr: &Expr<'_>,
-    span: Option<Span>,
-    replacement: RetReplacement,
-) {
+fn check_final_expr(cx: &LateContext<'_>, expr: &Expr<'_>, span: Option<Span>, replacement: RetReplacement) {
     match expr.kind {
         // simple return is always "bad"
         ExprKind::Ret(ref inner) => {
-
             // allow `#[cfg(a)] return a; #[cfg(b)] return b;`
             if !expr.attrs.iter().any(attr_is_cfg) {
                 emit_return_lint(
@@ -97,32 +113,34 @@ fn check_final_expr(
                     replacement,
                 );
             }
-        }
+        },
         // a whole block? check it!
         ExprKind::Block(ref block, _) => {
             check_block_return(cx, block);
-        }
+        },
         // a match expr, check all arms
         // an if/if let expr, check both exprs
         // note, if without else is going to be a type checking error anyways
         // (except for unit type functions) so we don't match it
-
-        ExprKind::Match(_, ref arms, source) => {
-            match source {
-                MatchSource::Normal => {
-                    for arm in arms.iter() {
-                        check_final_expr(cx, &arm.body, Some(arm.body.span), RetReplacement::Block);
-                    }
-                }
-                MatchSource::IfDesugar { contains_else_clause: true } | MatchSource::IfLetDesugar { contains_else_clause: true } => {
-                    if let ExprKind::Block(ref ifblock, _) = arms[0].body.kind {
-                        check_block_return(cx, ifblock);
-                    }
-                    check_final_expr(cx, arms[1].body, None, RetReplacement::Empty);
+        ExprKind::Match(_, ref arms, source) => match source {
+            MatchSource::Normal => {
+                for arm in arms.iter() {
+                    check_final_expr(cx, &arm.body, Some(arm.body.span), RetReplacement::Block);
                 }
-                _ => ()
+            },
+            MatchSource::IfDesugar {
+                contains_else_clause: true,
             }
-        }
+            | MatchSource::IfLetDesugar {
+                contains_else_clause: true,
+            } => {
+                if let ExprKind::Block(ref ifblock, _) = arms[0].body.kind {
+                    check_block_return(cx, ifblock);
+                }
+                check_final_expr(cx, arms[1].body, None, RetReplacement::Empty);
+            },
+            _ => (),
+        },
         _ => (),
     }
 }
@@ -139,7 +157,7 @@ fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, inner_span: Option<Spa
                     diag.span_suggestion(ret_span, "remove `return`", snippet, Applicability::MachineApplicable);
                 }
             })
-        }
+        },
         None => match replacement {
             RetReplacement::Empty => {
                 span_lint_and_sugg(
@@ -151,7 +169,7 @@ fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, inner_span: Option<Spa
                     String::new(),
                     Applicability::MachineApplicable,
                 );
-            }
+            },
             RetReplacement::Block => {
                 span_lint_and_sugg(
                     cx,
@@ -162,7 +180,7 @@ fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, inner_span: Option<Spa
                     "{}".to_string(),
                     Applicability::MachineApplicable,
                 );
-            }
+            },
         },
     }
 }
@@ -204,4 +222,3 @@ impl<'tcx> Visitor<'tcx> for BorrowVisitor<'_, 'tcx> {
         NestedVisitorMap::None
     }
 }
-
diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
index 2bd0cccd39d..593e2f6c74b 100644
--- a/clippy_lints/src/returns.rs
+++ b/clippy_lints/src/returns.rs
@@ -7,7 +7,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::Span;
 use rustc_span::BytePos;
 
-use crate::utils::{span_lint_and_sugg};
+use crate::utils::span_lint_and_sugg;
 
 declare_clippy_lint! {
     /// **What it does:** Checks for unit (`()`) expressions that can be removed.
@@ -30,12 +30,10 @@ declare_clippy_lint! {
     "needless unit expression"
 }
 
-
 declare_lint_pass!(Return => [UNUSED_UNIT]);
 
 impl EarlyLintPass for Return {
     fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, span: Span, _: ast::NodeId) {
-
         if_chain! {
             if let ast::FnRetTy::Ty(ref ty) = kind.decl().output;
             if let ast::TyKind::Tup(ref vals) = ty.kind;
@@ -102,7 +100,6 @@ impl EarlyLintPass for Return {
     }
 }
 
-
 // get the def site
 #[must_use]
 fn get_def(span: Span) -> Option<Span> {
diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed
index ad20e238107..6b5cf2626df 100644
--- a/tests/ui/needless_return.fixed
+++ b/tests/ui/needless_return.fixed
@@ -69,6 +69,16 @@ fn test_void_match(x: u32) {
     }
 }
 
+mod no_lint_if_stmt_borrows {
+    mod issue_5858 {
+        fn read_line() -> String {
+            use std::io::BufRead;
+            let stdin = ::std::io::stdin();
+            return stdin.lock().lines().next().unwrap().unwrap();
+        }
+    }
+}
+
 fn main() {
     let _ = test_end_of_fn();
     let _ = test_no_semicolon();
diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs
index af0cdfb207f..1a693c9aa53 100644
--- a/tests/ui/needless_return.rs
+++ b/tests/ui/needless_return.rs
@@ -69,6 +69,16 @@ fn test_void_match(x: u32) {
     }
 }
 
+mod no_lint_if_stmt_borrows {
+    mod issue_5858 {
+        fn read_line() -> String {
+            use std::io::BufRead;
+            let stdin = ::std::io::stdin();
+            return stdin.lock().lines().next().unwrap().unwrap();
+        }
+    }
+}
+
 fn main() {
     let _ = test_end_of_fn();
     let _ = test_no_semicolon();