mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #1411 from theemathas/forget_ref

Browse Source 
Add forget_ref lint.
This commit is contained in:
Oliver Schneider 2017-01-06 13:43:29 +01:00 committed by GitHub
commit 5e7727119e
7 changed files with 157 additions and 112 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -298,6 +298,7 @@ All notable changes to this project will be documented in this file.
[`for_kv_map`]: https://github.com/Manishearth/rust-clippy/wiki#for_kv_map
[`for_loop_over_option`]: https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_option
[`for_loop_over_result`]: https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_result
[`forget_ref`]: https://github.com/Manishearth/rust-clippy/wiki#forget_ref
[`get_unwrap`]: https://github.com/Manishearth/rust-clippy/wiki#get_unwrap
[`identity_op`]: https://github.com/Manishearth/rust-clippy/wiki#identity_op
[`if_let_redundant_pattern_matching`]: https://github.com/Manishearth/rust-clippy/wiki#if_let_redundant_pattern_matching

3
README.md
View File

@ -179,7 +179,7 @@ transparently:
## Lints
There are 182 lints included in this crate:
There are 183 lints included in this crate:
name | default | triggers on
-----------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
@ -237,6 +237,7 @@ name
[for_kv_map](https://github.com/Manishearth/rust-clippy/wiki#for_kv_map) | warn | looping on a map using `iter` when `keys` or `values` would do
[for_loop_over_option](https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_option) | warn | for-looping over an `Option`, which is more clearly expressed as an `if let`
[for_loop_over_result](https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_result) | warn | for-looping over a `Result`, which is more clearly expressed as an `if let`
[forget_ref](https://github.com/Manishearth/rust-clippy/wiki#forget_ref) | warn | calls to `std::mem::forget` with a reference instead of an owned value
[get_unwrap](https://github.com/Manishearth/rust-clippy/wiki#get_unwrap) | warn | using `.get().unwrap()` or `.get_mut().unwrap()` when using `[]` would work instead
[identity_op](https://github.com/Manishearth/rust-clippy/wiki#identity_op) | warn | using identity operations, e.g. `x + 0` or `y / 1`
[if_let_redundant_pattern_matching](https://github.com/Manishearth/rust-clippy/wiki#if_let_redundant_pattern_matching) | warn | use the proper utility function avoiding an `if let`

90
clippy_lints/src/drop_forget_ref.rs Normal file
View File

@ -0,0 +1,90 @@
use rustc::lint::*;
use rustc::ty;
use rustc::hir::*;
use utils::{match_def_path, paths, span_note_and_lint};
/// **What it does:** Checks for calls to `std::mem::drop` with a reference
/// instead of an owned value.
///
/// **Why is this bad?** Calling `drop` on a reference will only drop the
/// reference itself, which is a no-op. It will not call the `drop` method (from
/// the `Drop` trait implementation) on the underlying referenced value, which
/// is likely what was intended.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// let mut lock_guard = mutex.lock();
/// std::mem::drop(&lock_guard) // Should have been drop(lock_guard), mutex still locked
/// operation_that_requires_mutex_to_be_unlocked();
/// ```
declare_lint! {
pub DROP_REF,
Warn,
"calls to `std::mem::drop` with a reference instead of an owned value"
}
/// **What it does:** Checks for calls to `std::mem::forget` with a reference
/// instead of an owned value.
///
/// **Why is this bad?** Calling `forget` on a reference will only forget the
/// reference itself, which is a no-op. It will not forget the underlying referenced
/// value, which is likely what was intended.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// let x = Box::new(1);
/// std::mem::forget(&x) // Should have been forget(x), x will still be dropped
/// ```
declare_lint! {
pub FORGET_REF,
Warn,
"calls to `std::mem::forget` with a reference instead of an owned value"
}
#[allow(missing_copy_implementations)]
pub struct Pass;
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(DROP_REF, FORGET_REF)
}
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if_let_chain!{[
let ExprCall(ref path, ref args) = expr.node,
let ExprPath(ref qpath) = path.node,
args.len() == 1,
], {
let def_id = cx.tcx.tables().qpath_def(qpath, path.id).def_id();
let lint;
let msg;
if match_def_path(cx, def_id, &paths::DROP) {
lint = DROP_REF;
msg = "call to `std::mem::drop` with a reference argument. \
Dropping a reference does nothing";
} else if match_def_path(cx, def_id, &paths::MEM_FORGET) {
lint = FORGET_REF;
msg = "call to `std::mem::forget` with a reference argument. \
Forgetting a reference does nothing";
} else {
return;
}
let arg = &args[0];
let arg_ty = cx.tcx.tables().expr_ty(arg);
if let ty::TyRef(..) = arg_ty.sty {
span_note_and_lint(cx,
lint,
expr.span,
msg,
arg.span,
&format!("argument has type {}", arg_ty.sty));
}
}}
}
}

65
clippy_lints/src/drop_ref.rs
View File

@ -1,65 +0,0 @@
use rustc::lint::*;
use rustc::ty;
use rustc::hir::*;
use syntax::codemap::Span;
use utils::{match_def_path, paths, span_note_and_lint};
/// **What it does:** Checks for calls to `std::mem::drop` with a reference
/// instead of an owned value.
///
/// **Why is this bad?** Calling `drop` on a reference will only drop the
/// reference itself, which is a no-op. It will not call the `drop` method (from
/// the `Drop` trait implementation) on the underlying referenced value, which
/// is likely what was intended.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// let mut lock_guard = mutex.lock();
/// std::mem::drop(&lock_guard) // Should have been drop(lock_guard), mutex still locked
/// operation_that_requires_mutex_to_be_unlocked();
/// ```
declare_lint! {
pub DROP_REF,
Warn,
"calls to `std::mem::drop` with a reference instead of an owned value"
}
#[allow(missing_copy_implementations)]
pub struct Pass;
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(DROP_REF)
}
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if let ExprCall(ref path, ref args) = expr.node {
if let ExprPath(ref qpath) = path.node {
let def_id = cx.tcx.tables().qpath_def(qpath, path.id).def_id();
if match_def_path(cx, def_id, &paths::DROP) {
if args.len() != 1 {
return;
}
check_drop_arg(cx, expr.span, &args[0]);
}
}
}
}
}
fn check_drop_arg(cx: &LateContext, call_span: Span, arg: &Expr) {
let arg_ty = cx.tcx.tables().expr_ty(arg);
if let ty::TyRef(..) = arg_ty.sty {
span_note_and_lint(cx,
DROP_REF,
call_span,
"call to `std::mem::drop` with a reference argument. \
Dropping a reference does nothing",
arg.span,
&format!("argument has type {}", arg_ty.sty));
}
}

7
clippy_lints/src/lib.rs
View File

@ -72,7 +72,7 @@ pub mod cyclomatic_complexity;
pub mod derive;
pub mod doc;
pub mod double_parens;
pub mod drop_ref;
pub mod drop_forget_ref;
pub mod entry;
pub mod enum_clike;
pub mod enum_glob_use;
@ -259,7 +259,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
reg.register_early_lint_pass(box non_expressive_names::NonExpressiveNames {
max_single_char_names: conf.max_single_char_names,
});
reg.register_late_lint_pass(box drop_ref::Pass);
reg.register_late_lint_pass(box drop_forget_ref::Pass);
reg.register_late_lint_pass(box types::AbsurdExtremeComparisons);
reg.register_late_lint_pass(box types::InvalidUpcastComparisons);
reg.register_late_lint_pass(box regex::Pass::default());
@ -360,7 +360,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
derive::EXPL_IMPL_CLONE_ON_COPY,
doc::DOC_MARKDOWN,
double_parens::DOUBLE_PARENS,
drop_ref::DROP_REF,
drop_forget_ref::DROP_REF,
drop_forget_ref::FORGET_REF,
entry::MAP_ENTRY,
enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT,
enum_variants::ENUM_VARIANT_NAMES,

60
tests/compile-fail/drop_forget_ref.rs Normal file
View File

@ -0,0 +1,60 @@
#![feature(plugin)]
#![plugin(clippy)]
#![deny(drop_ref, forget_ref)]
#![allow(toplevel_ref_arg, similar_names)]
use std::mem::{drop, forget};
struct SomeStruct;
fn main() {
drop(&SomeStruct); //~ERROR call to `std::mem::drop` with a reference argument
forget(&SomeStruct); //~ERROR call to `std::mem::forget` with a reference argument
let mut owned1 = SomeStruct;
drop(&owned1); //~ERROR call to `std::mem::drop` with a reference argument
drop(&&owned1); //~ERROR call to `std::mem::drop` with a reference argument
drop(&mut owned1); //~ERROR call to `std::mem::drop` with a reference argument
drop(owned1); //OK
let mut owned2 = SomeStruct;
forget(&owned2); //~ERROR call to `std::mem::forget` with a reference argument
forget(&&owned2); //~ERROR call to `std::mem::forget` with a reference argument
forget(&mut owned2); //~ERROR call to `std::mem::forget` with a reference argument
forget(owned2); //OK
let reference1 = &SomeStruct;
drop(reference1); //~ERROR call to `std::mem::drop` with a reference argument
forget(&*reference1); //~ERROR call to `std::mem::forget` with a reference argument
let reference2 = &mut SomeStruct;
drop(reference2); //~ERROR call to `std::mem::drop` with a reference argument
let reference3 = &mut SomeStruct;
forget(reference3); //~ERROR call to `std::mem::forget` with a reference argument
let ref reference4 = SomeStruct;
drop(reference4); //~ERROR call to `std::mem::drop` with a reference argument
forget(reference4); //~ERROR call to `std::mem::forget` with a reference argument
}
#[allow(dead_code)]
fn test_generic_fn_drop<T>(val: T) {
drop(&val); //~ERROR call to `std::mem::drop` with a reference argument
drop(val); //OK
}
#[allow(dead_code)]
fn test_generic_fn_forget<T>(val: T) {
forget(&val); //~ERROR call to `std::mem::forget` with a reference argument
forget(val); //OK
}
#[allow(dead_code)]
fn test_similarly_named_function() {
fn drop<T>(_val: T) {}
drop(&SomeStruct); //OK; call to unrelated function which happens to have the same name
std::mem::drop(&SomeStruct); //~ERROR call to `std::mem::drop` with a reference argument
fn forget<T>(_val: T) {}
forget(&SomeStruct); //OK; call to unrelated function which happens to have the same name
std::mem::forget(&SomeStruct); //~ERROR call to `std::mem::forget` with a reference argument
}

43
tests/compile-fail/drop_ref.rs
View File

@ -1,43 +0,0 @@
#![feature(plugin)]
#![plugin(clippy)]
#![deny(drop_ref)]
#![allow(toplevel_ref_arg, similar_names)]
use std::mem::drop;
struct DroppableStruct;
impl Drop for DroppableStruct { fn drop(&mut self) {} }
fn main() {
drop(&DroppableStruct); //~ERROR call to `std::mem::drop` with a reference argument
let mut owned = DroppableStruct;
drop(&owned); //~ERROR call to `std::mem::drop` with a reference argument
drop(&&owned); //~ERROR call to `std::mem::drop` with a reference argument
drop(&mut owned); //~ERROR call to `std::mem::drop` with a reference argument
drop(owned); //OK
let reference1 = &DroppableStruct;
drop(reference1); //~ERROR call to `std::mem::drop` with a reference argument
drop(&*reference1); //~ERROR call to `std::mem::drop` with a reference argument
let reference2 = &mut DroppableStruct;
drop(reference2); //~ERROR call to `std::mem::drop` with a reference argument
let ref reference3 = DroppableStruct;
drop(reference3); //~ERROR call to `std::mem::drop` with a reference argument
}
#[allow(dead_code)]
fn test_generic_fn<T>(val: T) {
drop(&val); //~ERROR call to `std::mem::drop` with a reference argument
drop(val); //OK
}
#[allow(dead_code)]
fn test_similarly_named_function() {
fn drop<T>(_val: T) {}
drop(&DroppableStruct); //OK; call to unrelated function which happens to have the same name
std::mem::drop(&DroppableStruct); //~ERROR call to `std::mem::drop` with a reference argument
}