Skip to content

Commit

Permalink
let_and_return: do not lint if last statement borrows
Browse files Browse the repository at this point in the history
  • Loading branch information
ebroto committed Jun 7, 2020
1 parent 9c205d7 commit dac8a3c
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 2 deletions.
61 changes: 60 additions & 1 deletion clippy_lints/src/let_and_return.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Block, ExprKind, PatKind, StmtKind};
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{Block, Expr, ExprKind, PatKind, StmtKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::hir::map::Map;
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_session::{declare_lint_pass, declare_tool_lint};

use crate::utils::{in_macro, match_qpath, snippet_opt, span_lint_and_then};
Expand Down Expand Up @@ -49,6 +53,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetReturn {
if let PatKind::Binding(.., ident, _) = local.pat.kind;
if let ExprKind::Path(qpath) = &retexpr.kind;
if match_qpath(qpath, &[&*ident.name.as_str()]);
if !last_statement_borrows(cx, initexpr);
if !in_external_macro(cx.sess(), initexpr.span);
if !in_external_macro(cx.sess(), retexpr.span);
if !in_external_macro(cx.sess(), local.span);
Expand Down Expand Up @@ -80,3 +85,57 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetReturn {
}
}
}

fn last_statement_borrows<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
let mut visitor = BorrowVisitor { cx, borrows: false };
walk_expr(&mut visitor, expr);
visitor.borrows
}

struct BorrowVisitor<'a, 'tcx> {
cx: &'a LateContext<'a, 'tcx>,
borrows: bool,
}

impl BorrowVisitor<'_, '_> {
fn fn_def_id(&self, expr: &Expr<'_>) -> Option<DefId> {
match &expr.kind {
ExprKind::MethodCall(..) => self.cx.tables.type_dependent_def_id(expr.hir_id),
ExprKind::Call(
Expr {
kind: ExprKind::Path(qpath),
..
},
..,
) => self.cx.tables.qpath_res(qpath, expr.hir_id).opt_def_id(),
_ => None,
}
}
}

impl<'tcx> Visitor<'tcx> for BorrowVisitor<'_, 'tcx> {
type Map = Map<'tcx>;

fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if self.borrows {
return;
}

if let Some(def_id) = self.fn_def_id(expr) {
self.borrows = self
.cx
.tcx
.fn_sig(def_id)
.output()
.skip_binder()
.walk()
.any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_)));
}

walk_expr(self, expr);
}

fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
}
2 changes: 1 addition & 1 deletion clippy_lints/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ pub fn method_calls<'tcx>(
/// Matches an `Expr` against a chain of methods, and return the matched `Expr`s.
///
/// For example, if `expr` represents the `.baz()` in `foo.bar().baz()`,
/// `matched_method_chain(expr, &["bar", "baz"])` will return a `Vec`
/// `method_chain_args(expr, &["bar", "baz"])` will return a `Vec`
/// containing the `Expr`s for
/// `.bar()` and `.baz()`
pub fn method_chain_args<'a>(expr: &'a Expr<'_>, methods: &[&str]) -> Option<Vec<&'a [Expr<'a>]>> {
Expand Down
68 changes: 68 additions & 0 deletions tests/ui/let_and_return.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,72 @@ macro_rules! tuple_encode {

tuple_encode!(T0, T1, T2, T3, T4, T5, T6, T7);

mod no_lint_if_stmt_borrows {
mod issue_3792 {
use std::io::{self, BufRead, Stdin};

fn read_line() -> String {
let stdin = io::stdin();
let line = stdin.lock().lines().next().unwrap().unwrap();
line
}
}

mod issue_3324 {
use std::cell::RefCell;
use std::rc::{Rc, Weak};

fn test(value: Weak<RefCell<Bar>>) -> u32 {
let value = value.upgrade().unwrap();
let ret = value.borrow().baz();
ret
}

struct Bar {}

impl Bar {
fn new() -> Self {
Bar {}
}
fn baz(&self) -> u32 {
0
}
}

fn main() {
let a = Rc::new(RefCell::new(Bar::new()));
let b = Rc::downgrade(&a);
test(b);
}
}

mod free_function {
struct Inner;

struct Foo<'a> {
inner: &'a Inner,
}

impl Drop for Foo<'_> {
fn drop(&mut self) {}
}

impl Foo<'_> {
fn value(&self) -> i32 {
42
}
}

fn some_foo(inner: &Inner) -> Foo<'_> {
Foo { inner }
}

fn test() -> i32 {
let x = Inner {};
let value = some_foo(&x).value();
value
}
}
}

fn main() {}

0 comments on commit dac8a3c

Please sign in to comment.