Skip to content

Commit

Permalink
factor ifs into function, add differing mutex test
Browse files Browse the repository at this point in the history
  • Loading branch information
DevinR528 committed Apr 20, 2020
1 parent d1b1a4c commit 489dd2e
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 12 deletions.
28 changes: 18 additions & 10 deletions clippy_lints/src/if_let_mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,9 @@ impl<'tcx, 'l> Visitor<'tcx> for OppVisitor<'tcx, 'l> {

fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if_chain! {
if let ExprKind::MethodCall(path, _span, args) = &expr.kind;
if path.ident.to_string() == "lock";
let ty = self.cx.tables.expr_ty(&args[0]);
if match_type(self.cx, ty, &paths::MUTEX);
if let Some(mutex) = is_mutex_lock_call(self.cx, expr);
then {
self.found_mutex = Some(&args[0]);
self.found_mutex = Some(mutex);
self.mutex_lock_called = true;
return;
}
Expand All @@ -123,12 +120,9 @@ impl<'tcx, 'l> Visitor<'tcx> for ArmVisitor<'tcx, 'l> {

fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
if_chain! {
if let ExprKind::MethodCall(path, _span, args) = &expr.kind;
if path.ident.to_string() == "lock";
let ty = self.cx.tables.expr_ty(&args[0]);
if match_type(self.cx, ty, &paths::MUTEX);
if let Some(mutex) = is_mutex_lock_call(self.cx, expr);
then {
self.found_mutex = Some(&args[0]);
self.found_mutex = Some(mutex);
self.mutex_lock_called = true;
return;
}
Expand All @@ -150,3 +144,17 @@ impl<'tcx, 'l> ArmVisitor<'tcx, 'l> {
}
}
}

fn is_mutex_lock_call<'a>(cx: &LateContext<'a, '_>, expr: &'a Expr<'_>) -> Option<&'a Expr<'a>> {
if_chain! {
if let ExprKind::MethodCall(path, _span, args) = &expr.kind;
if path.ident.to_string() == "lock";
let ty = cx.tables.expr_ty(&args[0]);
if match_type(cx, ty, &paths::MUTEX);
then {
Some(&args[0])
} else {
None
}
}
}
4 changes: 2 additions & 2 deletions doc/adding_lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ Once we are satisfied with the output, we need to run
Please note that, we should run `TESTNAME=foo_functions cargo uitest`
every time before running `tests/ui/update-all-references.sh`.
Running `TESTNAME=foo_functions cargo uitest` should pass then. When we commit
our lint, we need to commit the generated `.stderr` files, too. In general you
should only commit changed files by `tests/ui/update-all-references.sh` for the
our lint, we need to commit the generated `.stderr` files, too. In general, you
should only commit files changed by `tests/ui/update-all-references.sh` for the
specific lint you are creating/editing.

## Rustfix tests
Expand Down
14 changes: 14 additions & 0 deletions tests/ui/if_let_mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ fn if_let() {
};
}

// This is the most common case as the above case is pretty
// contrived.
fn if_let_option() {
let m = Mutex::new(Some(0_u8));
if let Some(locked) = m.lock().unwrap().deref() {
Expand All @@ -25,4 +27,16 @@ fn if_let_option() {
};
}

// When mutexs are different don't warn
fn if_let_different_mutex() {
let m = Mutex::new(Some(0_u8));
let other = Mutex::new(None::<u8>);
if let Some(locked) = m.lock().unwrap().deref() {
do_stuff(locked);
} else {
let lock = other.lock().unwrap();
do_stuff(lock);
};
}

fn main() {}

0 comments on commit 489dd2e

Please sign in to comment.