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

Merge pull request #1550 from sinkuu/should_assert_eq

Browse Source 
Lint `assert!(x == y)`
This commit is contained in:
Oliver Schneider 2017-02-18 08:23:11 +01:00 committed by GitHub
commit 8a227c7577
12 changed files with 130 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -423,6 +423,7 @@ All notable changes to this project will be documented in this file.
[`shadow_same`]: https://github.com/Manishearth/rust-clippy/wiki#shadow_same
[`shadow_unrelated`]: https://github.com/Manishearth/rust-clippy/wiki#shadow_unrelated
[`short_circuit_statement`]: https://github.com/Manishearth/rust-clippy/wiki#short_circuit_statement
[`should_assert_eq`]: https://github.com/Manishearth/rust-clippy/wiki#should_assert_eq
[`should_implement_trait`]: https://github.com/Manishearth/rust-clippy/wiki#should_implement_trait
[`similar_names`]: https://github.com/Manishearth/rust-clippy/wiki#similar_names
[`single_char_pattern`]: https://github.com/Manishearth/rust-clippy/wiki#single_char_pattern

3
README.md
View File

@ -180,7 +180,7 @@ transparently:
## Lints
There are 190 lints included in this crate:
There are 191 lints included in this crate:
name | default | triggers on
-----------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
@ -329,6 +329,7 @@ name
[shadow_same](https://github.com/Manishearth/rust-clippy/wiki#shadow_same) | allow | rebinding a name to itself, e.g. `let mut x = &mut x`
[shadow_unrelated](https://github.com/Manishearth/rust-clippy/wiki#shadow_unrelated) | allow | rebinding a name without even using the original value
[short_circuit_statement](https://github.com/Manishearth/rust-clippy/wiki#short_circuit_statement) | warn | using a short circuit boolean condition as a statement
[should_assert_eq](https://github.com/Manishearth/rust-clippy/wiki#should_assert_eq) | warn | using `assert` macro for asserting equality
[should_implement_trait](https://github.com/Manishearth/rust-clippy/wiki#should_implement_trait) | warn | defining a method that should be implementing a std trait
[similar_names](https://github.com/Manishearth/rust-clippy/wiki#similar_names) | allow | similarly named items and bindings
[single_char_pattern](https://github.com/Manishearth/rust-clippy/wiki#single_char_pattern) | warn | using a single-character str where a char could be used, e.g. `_.split("x")`

2
clippy_lints/src/assign_ops.rs
View File

@ -152,7 +152,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps {
let hir::Item_::ItemImpl(_, _, _, Some(ref trait_ref), _, _) = item.node,
trait_ref.path.def.def_id() == trait_id
], { return; }}
implements_trait($cx, $ty, trait_id, vec![$rty])
implements_trait($cx, $ty, trait_id, &[$rty], None)
},)*
_ => false,
}

3
clippy_lints/src/lib.rs
View File

@ -126,6 +126,7 @@ pub mod regex;
pub mod returns;
pub mod serde;
pub mod shadow;
pub mod should_assert_eq;
pub mod strings;
pub mod swap;
pub mod temporary_assignment;
@ -297,6 +298,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
reg.register_early_lint_pass(box double_parens::DoubleParens);
reg.register_late_lint_pass(box unused_io_amount::UnusedIoAmount);
reg.register_late_lint_pass(box large_enum_variant::LargeEnumVariant::new(conf.enum_variant_size_threshold));
reg.register_late_lint_pass(box should_assert_eq::ShouldAssertEq);
reg.register_lint_group("clippy_restrictions", vec![
arithmetic::FLOAT_ARITHMETIC,
@ -479,6 +481,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
returns::LET_AND_RETURN,
returns::NEEDLESS_RETURN,
serde::SERDE_API_MISUSE,
should_assert_eq::SHOULD_ASSERT_EQ,
strings::STRING_LIT_AS_BYTES,
swap::ALMOST_SWAPPED,
swap::MANUAL_SWAP,

4
clippy_lints/src/methods.rs
View File

@ -724,7 +724,7 @@ fn lint_or_fun_call(cx: &LateContext, expr: &hir::Expr, name: &str, args: &[hir:
return false;
};
if implements_trait(cx, arg_ty, default_trait_id, Vec::new()) {
if implements_trait(cx, arg_ty, default_trait_id, &[], None) {
span_lint_and_then(cx,
OR_FUN_CALL,
span,
@ -1268,7 +1268,7 @@ fn get_error_type<'a>(cx: &LateContext, ty: ty::Ty<'a>) -> Option<ty::Ty<'a>> {
/// This checks whether a given type is known to implement Debug.
fn has_debug_impl<'a, 'b>(ty: ty::Ty<'a>, cx: &LateContext<'b, 'a>) -> bool {
match cx.tcx.lang_items.debug_trait() {
Some(debug) => implements_trait(cx, ty, debug, Vec::new()),
Some(debug) => implements_trait(cx, ty, debug, &[], None),
None => false,
}
}

2
clippy_lints/src/misc.rs
View File

@ -420,7 +420,7 @@ fn check_to_owned(cx: &LateContext, expr: &Expr, other: &Expr, left: bool, op: S
None => return,
};
if !implements_trait(cx, arg_ty, partial_eq_trait_id, vec![other_ty]) {
if !implements_trait(cx, arg_ty, partial_eq_trait_id, &[other_ty], None) {
return;
}

4
clippy_lints/src/new_without_default.rs
View File

@ -115,7 +115,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault {
self_ty.walk_shallow().next().is_none(), // implements_trait does not work with generics
same_tys(cx, self_ty, return_ty(cx, id), id),
let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT),
!implements_trait(cx, self_ty, default_trait_id, Vec::new())
!implements_trait(cx, self_ty, default_trait_id, &[], None)
], {
if let Some(sp) = can_derive_default(self_ty, cx, default_trait_id) {
span_lint_and_then(cx,
@ -156,7 +156,7 @@ fn can_derive_default<'t, 'c>(ty: ty::Ty<'t>, cx: &LateContext<'c, 't>, default_
ty::TyAdt(adt_def, substs) if adt_def.is_struct() => {
for field in adt_def.all_fields() {
let f_ty = field.ty(cx.tcx, substs);
if !implements_trait(cx, f_ty, default_trait_id, Vec::new()) {
if !implements_trait(cx, f_ty, default_trait_id, &[], None) {
return None;
}
}

54
clippy_lints/src/should_assert_eq.rs Normal file
View File

@ -0,0 +1,54 @@
use rustc::lint::*;
use rustc::hir::*;
use utils::{is_direct_expn_of, implements_trait, span_lint};
/// **What it does:** Checks for `assert!(x == y)` which can be better written
/// as `assert_eq!(x, y)` if `x` and `y` implement `Debug` trait.
///
/// **Why is this bad?** `assert_eq` provides better assertion failure reporting.
///
/// **Known problems:** Hopefully none.
///
/// **Example:**
/// ```rust
/// let (x, y) = (1, 2);
///
/// assert!(x == y); // assertion failed: x == y
/// assert_eq!(x, y); // assertion failed: `(left == right)` (left: `1`, right: `2`)
/// ```
declare_lint! {
pub SHOULD_ASSERT_EQ,
Warn,
"using `assert` macro for asserting equality"
}
pub struct ShouldAssertEq;
impl LintPass for ShouldAssertEq {
fn get_lints(&self) -> LintArray {
lint_array![SHOULD_ASSERT_EQ]
}
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ShouldAssertEq {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
if_let_chain! {[
let ExprIf(ref cond, ..) = e.node,
let ExprUnary(UnOp::UnNot, ref cond) = cond.node,
let ExprBinary(ref binop, ref expr1, ref expr2) = cond.node,
binop.node == BinOp_::BiEq,
is_direct_expn_of(cx, e.span, "assert").is_some(),
let Some(debug_trait) = cx.tcx.lang_items.debug_trait(),
], {
let ty1 = cx.tables.expr_ty(expr1);
let ty2 = cx.tables.expr_ty(expr2);
let parent = cx.tcx.hir.get_parent(e.id);
if implements_trait(cx, ty1, debug_trait, &[], Some(parent)) &&
implements_trait(cx, ty2, debug_trait, &[], Some(parent)) {
span_lint(cx, SHOULD_ASSERT_EQ, e.span, "use `assert_eq` for better reporting");
}
}}
}
}

12
clippy_lints/src/utils/mod.rs
View File

@ -317,13 +317,19 @@ pub fn implements_trait<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
ty: ty::Ty<'tcx>,
trait_id: DefId,
ty_params: Vec<ty::Ty<'tcx>>
ty_params: &[ty::Ty<'tcx>],
parent_node_id: Option<NodeId>
) -> bool {
cx.tcx.populate_implementations_for_trait_if_necessary(trait_id);
let ty = cx.tcx.erase_regions(&ty);
cx.tcx.infer_ctxt((), Reveal::All).enter(|infcx| {
let obligation = cx.tcx.predicate_for_trait_def(traits::ObligationCause::dummy(), trait_id, 0, ty, &ty_params);
let mut b = if let Some(id) = parent_node_id {
cx.tcx.infer_ctxt(BodyId { node_id: id }, Reveal::All)
} else {
cx.tcx.infer_ctxt((), Reveal::All)
};
b.enter(|infcx| {
let obligation = cx.tcx.predicate_for_trait_def(traits::ObligationCause::dummy(), trait_id, 0, ty, ty_params);
traits::SelectionContext::new(&infcx).evaluate_obligation_conservatively(&obligation)
})

2
tests/ui/float_cmp.rs
View File

@ -92,5 +92,5 @@ fn main() {
let a: *const f32 = xs.as_ptr();
let b: *const f32 = xs.as_ptr();
assert!(a == b); // no errors
assert_eq!(a, b); // no errors
}

23
tests/ui/should_assert_eq.rs Normal file
View File

@ -0,0 +1,23 @@
#![feature(plugin)]
#![plugin(clippy)]
#![deny(should_assert_eq)]
#[derive(PartialEq, Eq)]
struct NonDebug(i32);
#[derive(Debug, PartialEq, Eq)]
struct Debug(i32);
fn main() {
assert!(1 == 2);
assert!(Debug(1) == Debug(2));
assert!(NonDebug(1) == NonDebug(1)); // ok
test_generic(1, 2, 3, 4);
}
fn test_generic<T: std::fmt::Debug + Eq, U: Eq>(x: T, y: T, z: U, w: U) {
assert!(x == y);
assert!(z == w); // ok
}

31
tests/ui/should_assert_eq.stderr Normal file
View File

@ -0,0 +1,31 @@
error: use `assert_eq` for better reporting
--> $DIR/should_assert_eq.rs:13:5
|
13 | assert!(1 == 2);
| ^^^^^^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/should_assert_eq.rs:4:9
|
4 | #![deny(should_assert_eq)]
| ^^^^^^^^^^^^^^^^
= note: this error originates in a macro outside of the current crate
error: use `assert_eq` for better reporting
--> $DIR/should_assert_eq.rs:14:5
|
14 | assert!(Debug(1) == Debug(2));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this error originates in a macro outside of the current crate
error: use `assert_eq` for better reporting
--> $DIR/should_assert_eq.rs:21:5
|
21 | assert!(x == y);
| ^^^^^^^^^^^^^^^^
|
= note: this error originates in a macro outside of the current crate
error: aborting due to 3 previous errors