diff --git a/clippy_lints/src/double_comparison.rs b/clippy_lints/src/double_comparison.rs new file mode 100644 index 000000000000..a4124883fcb6 --- /dev/null +++ b/clippy_lints/src/double_comparison.rs @@ -0,0 +1,85 @@ +//! Lint on unnecessary double comparisons. Some examples: + +use rustc::hir::*; +use rustc::lint::*; +use syntax::codemap::Span; + +use utils::{snippet, span_lint_and_sugg, SpanlessEq}; + +/// **What it does:** Checks for double comparions that could be simpified to a single expression. +/// +/// +/// **Why is this bad?** Readability. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// x == y || x < y +/// ``` +/// +/// Could be written as: +/// +/// ```rust +/// x <= y +/// ``` +declare_lint! { + pub DOUBLE_COMPARISONS, + Deny, + "unnecessary double comparisons that can be simplified" +} + +pub struct DoubleComparisonPass; + +impl LintPass for DoubleComparisonPass { + fn get_lints(&self) -> LintArray { + lint_array!(DOUBLE_COMPARISONS) + } +} + +impl<'a, 'tcx> DoubleComparisonPass { + fn check_binop( + &self, + cx: &LateContext<'a, 'tcx>, + op: BinOp_, + lhs: &'tcx Expr, + rhs: &'tcx Expr, + span: Span, + ) { + let (lkind, llhs, lrhs, rkind, rlhs, rrhs) = match (lhs.node.clone(), rhs.node.clone()) { + (ExprBinary(lb, llhs, lrhs), ExprBinary(rb, rlhs, rrhs)) => { + (lb.node, llhs, lrhs, rb.node, rlhs, rrhs) + } + _ => return, + }; + let spanless_eq = SpanlessEq::new(cx).ignore_fn(); + if !(spanless_eq.eq_expr(&llhs, &rlhs) && spanless_eq.eq_expr(&lrhs, &rrhs)) { + return; + } + macro_rules! lint_double_comparison { + ($op:tt) => {{ + let lhs_str = snippet(cx, llhs.span, ""); + let rhs_str = snippet(cx, lrhs.span, ""); + let sugg = format!("{} {} {}", lhs_str, stringify!($op), rhs_str); + span_lint_and_sugg(cx, DOUBLE_COMPARISONS, span, + "This binary expression can be simplified", + "try", sugg); + }} + } + match (op, lkind, rkind) { + (BiOr, BiEq, BiLt) | (BiOr, BiLt, BiEq) => lint_double_comparison!(<=), + (BiOr, BiEq, BiGt) | (BiOr, BiGt, BiEq) => lint_double_comparison!(>=), + (BiOr, BiLt, BiGt) | (BiOr, BiGt, BiLt) => lint_double_comparison!(!=), + (BiAnd, BiLe, BiGe) | (BiAnd, BiGe, BiLe) => lint_double_comparison!(==), + _ => (), + }; + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DoubleComparisonPass { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { + if let ExprBinary(ref kind, ref lhs, ref rhs) = expr.node { + self.check_binop(cx, kind.node, lhs, rhs, expr.span); + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 62df7fd10587..5f2d34d9eaad 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -86,6 +86,7 @@ pub mod copies; pub mod cyclomatic_complexity; pub mod derive; pub mod doc; +pub mod double_comparison; pub mod double_parens; pub mod drop_forget_ref; pub mod else_if_without_else; @@ -369,6 +370,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box fallible_impl_from::FallibleImplFrom); reg.register_late_lint_pass(box replace_consts::ReplaceConsts); reg.register_late_lint_pass(box types::UnitArg); + reg.register_late_lint_pass(box double_comparison::DoubleComparisonPass); reg.register_lint_group("clippy_restrictions", vec![ arithmetic::FLOAT_ARITHMETIC, diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 50ae695f4c51..04cc488d562b 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -545,11 +545,11 @@ fn check_to_owned(cx: &LateContext, expr: &Expr, other: &Expr) { // *arg impls PartialEq if !arg_ty - .builtin_deref(true, ty::LvaluePreference::NoPreference) + .builtin_deref(true) .map_or(false, |tam| implements_trait(cx, tam.ty, partial_eq_trait_id, &[other_ty])) // arg impls PartialEq<*other> && !other_ty - .builtin_deref(true, ty::LvaluePreference::NoPreference) + .builtin_deref(true) .map_or(false, |tam| implements_trait(cx, arg_ty, partial_eq_trait_id, &[tam.ty])) // arg impls PartialEq && !implements_trait(cx, arg_ty, partial_eq_trait_id, &[other_ty]) diff --git a/tests/ui/double_comparison.rs b/tests/ui/double_comparison.rs new file mode 100644 index 000000000000..2c8f116281bd --- /dev/null +++ b/tests/ui/double_comparison.rs @@ -0,0 +1,28 @@ +fn main() { + let x = 1; + let y = 2; + if x == y || x < y { + // do something + } + if x < y || x == y { + // do something + } + if x == y || x > y { + // do something + } + if x > y || x == y { + // do something + } + if x < y || x > y { + // do something + } + if x > y || x < y { + // do something + } + if x <= y && x >= y { + // do something + } + if x >= y && x <= y { + // do something + } +} diff --git a/tests/ui/double_comparison.stderr b/tests/ui/double_comparison.stderr new file mode 100644 index 000000000000..a97b0a246af3 --- /dev/null +++ b/tests/ui/double_comparison.stderr @@ -0,0 +1,52 @@ +error: This binary expression can be simplified + --> $DIR/double_comparison.rs:4:8 + | +4 | if x == y || x < y { + | ^^^^^^^^^^^^^^^ help: try: `x <= y` + | + = note: #[deny(double_comparisons)] on by default + +error: This binary expression can be simplified + --> $DIR/double_comparison.rs:7:8 + | +7 | if x < y || x == y { + | ^^^^^^^^^^^^^^^ help: try: `x <= y` + +error: This binary expression can be simplified + --> $DIR/double_comparison.rs:10:8 + | +10 | if x == y || x > y { + | ^^^^^^^^^^^^^^^ help: try: `x >= y` + +error: This binary expression can be simplified + --> $DIR/double_comparison.rs:13:8 + | +13 | if x > y || x == y { + | ^^^^^^^^^^^^^^^ help: try: `x >= y` + +error: This binary expression can be simplified + --> $DIR/double_comparison.rs:16:8 + | +16 | if x < y || x > y { + | ^^^^^^^^^^^^^^ help: try: `x != y` + +error: This binary expression can be simplified + --> $DIR/double_comparison.rs:19:8 + | +19 | if x > y || x < y { + | ^^^^^^^^^^^^^^ help: try: `x != y` + +error: This binary expression can be simplified + --> $DIR/double_comparison.rs:22:8 + | +22 | if x <= y && x >= y { + | ^^^^^^^^^^^^^^^^ help: try: `x == y` + +error: This binary expression can be simplified + --> $DIR/double_comparison.rs:25:8 + | +25 | if x >= y && x <= y { + | ^^^^^^^^^^^^^^^^ help: try: `x == y` + +error: aborting due to 8 previous errors +