From 93d097eb12fc0b7e34187d2cac19b0059fa18ed4 Mon Sep 17 00:00:00 2001
From: Oliver Schneider <git-spam-no-reply9815368754983@oli-obk.de>
Date: Wed, 23 Mar 2016 12:19:13 +0100
Subject: [PATCH] better simplification
---
Cargo.toml | 1 +
README.md | 3 +-
src/booleans.rs | 155 ++++++++++++++++++++
src/lib.rs | 8 +
tests/compile-fail/block_in_if_condition.rs | 2 +-
tests/compile-fail/eq_op.rs | 2 +
6 files changed, 169 insertions(+), 2 deletions(-)
create mode 100644 src/booleans.rs
diff --git a/Cargo.toml b/Cargo.toml
index 3bb7a82f6e2..93886e214e0 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -23,6 +23,7 @@ regex_macros = { version = "0.1.33", optional = true }
semver = "0.2.1"
toml = "0.1"
unicode-normalization = "0.1"
+quine-mc_cluskey = "0.2"
[dev-dependencies]
compiletest_rs = "0.1.0"
diff --git a/README.md b/README.md
index 43caa80543a..cd8b0257c6e 100644
--- a/README.md
+++ b/README.md
@@ -14,7 +14,7 @@ Table of contents:
* [License](#license)
##Lints
-There are 137 lints included in this crate:
+There are 138 lints included in this crate:
name | default | meaning
---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -97,6 +97,7 @@ name
[new_without_default](https://github.com/Manishearth/rust-clippy/wiki#new_without_default) | warn | `fn new() -> Self` method without `Default` implementation
[no_effect](https://github.com/Manishearth/rust-clippy/wiki#no_effect) | warn | statements with no effect
[non_ascii_literal](https://github.com/Manishearth/rust-clippy/wiki#non_ascii_literal) | allow | using any literal non-ASCII chars in a string literal; suggests using the \\u escape instead
+[nonminimal_bool](https://github.com/Manishearth/rust-clippy/wiki#nonminimal_bool) | warn | checks for boolean expressions that can be written more concisely
[nonsensical_open_options](https://github.com/Manishearth/rust-clippy/wiki#nonsensical_open_options) | warn | nonsensical combination of options for opening a file
[ok_expect](https://github.com/Manishearth/rust-clippy/wiki#ok_expect) | warn | using `ok().expect()`, which gives worse error messages than calling `expect` directly on the Result
[option_map_unwrap_or](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or) | warn | using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as `map_or(a, f)`
diff --git a/src/booleans.rs b/src/booleans.rs
new file mode 100644
index 00000000000..90e4dee9769
--- /dev/null
+++ b/src/booleans.rs
@@ -0,0 +1,155 @@
+use rustc::lint::*;
+use rustc_front::hir::*;
+use rustc_front::intravisit::*;
+use syntax::ast::LitKind;
+use utils::{span_lint_and_then, in_macro, snippet_opt};
+
+/// **What it does:** This lint checks for boolean expressions that can be written more concisely
+///
+/// **Why is this bad?** Readability of boolean expressions suffers from unnecesessary duplication
+///
+/// **Known problems:** None
+///
+/// **Example:** `if a && b || a` should be `if a`
+declare_lint! {
+ pub NONMINIMAL_BOOL, Warn,
+ "checks for boolean expressions that can be written more concisely"
+}
+
+#[derive(Copy,Clone)]
+pub struct NonminimalBool;
+
+impl LintPass for NonminimalBool {
+ fn get_lints(&self) -> LintArray {
+ lint_array!(NONMINIMAL_BOOL)
+ }
+}
+
+impl LateLintPass for NonminimalBool {
+ fn check_crate(&mut self, cx: &LateContext, krate: &Crate) {
+ krate.visit_all_items(&mut NonminimalBoolVisitor(cx))
+ }
+}
+
+struct NonminimalBoolVisitor<'a, 'tcx: 'a>(&'a LateContext<'a, 'tcx>);
+
+use quine_mc_cluskey::Bool;
+struct Hir2Qmm<'tcx>(Vec<&'tcx Expr>);
+
+impl<'tcx> Hir2Qmm<'tcx> {
+ fn extract(&mut self, op: BinOp_, a: &[&'tcx Expr], mut v: Vec<Bool>) -> Result<Vec<Bool>, String> {
+ for a in a {
+ if let ExprBinary(binop, ref lhs, ref rhs) = a.node {
+ if binop.node == op {
+ v = self.extract(op, &[lhs, rhs], v)?;
+ continue;
+ }
+ }
+ v.push(self.run(a)?);
+ }
+ Ok(v)
+ }
+
+ fn run(&mut self, e: &'tcx Expr) -> Result<Bool, String> {
+ match e.node {
+ ExprUnary(UnNot, ref inner) => return Ok(Bool::Not(box self.run(inner)?)),
+ ExprBinary(binop, ref lhs, ref rhs) => {
+ match binop.node {
+ BiOr => return Ok(Bool::Or(self.extract(BiOr, &[lhs, rhs], Vec::new())?)),
+ BiAnd => return Ok(Bool::And(self.extract(BiAnd, &[lhs, rhs], Vec::new())?)),
+ _ => {},
+ }
+ },
+ ExprLit(ref lit) => {
+ match lit.node {
+ LitKind::Bool(true) => return Ok(Bool::True),
+ LitKind::Bool(false) => return Ok(Bool::False),
+ _ => {},
+ }
+ },
+ _ => {},
+ }
+ let n = self.0.len();
+ self.0.push(e);
+ if n < 32 {
+ #[allow(cast_possible_truncation)]
+ Ok(Bool::Term(n as u8))
+ } else {
+ Err("too many literals".to_owned())
+ }
+ }
+}
+
+fn suggest(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr]) -> String {
+ fn recurse(cx: &LateContext, suggestion: &Bool, terminals: &[&Expr], mut s: String) -> String {
+ use quine_mc_cluskey::Bool::*;
+ match *suggestion {
+ True => {
+ s.extend("true".chars());
+ s
+ },
+ False => {
+ s.extend("false".chars());
+ s
+ },
+ Not(ref inner) => {
+ s.push('!');
+ recurse(cx, inner, terminals, s)
+ },
+ And(ref v) => {
+ s = recurse(cx, &v[0], terminals, s);
+ for inner in &v[1..] {
+ s.extend(" && ".chars());
+ s = recurse(cx, inner, terminals, s);
+ }
+ s
+ },
+ Or(ref v) => {
+ s = recurse(cx, &v[0], terminals, s);
+ for inner in &v[1..] {
+ s.extend(" || ".chars());
+ s = recurse(cx, inner, terminals, s);
+ }
+ s
+ },
+ Term(n) => {
+ s.extend(snippet_opt(cx, terminals[n as usize].span).expect("don't try to improve booleans created by macros").chars());
+ s
+ }
+ }
+ }
+ recurse(cx, suggestion, terminals, String::new())
+}
+
+impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
+ fn bool_expr(&self, e: &Expr) {
+ let mut h2q = Hir2Qmm(Vec::new());
+ if let Ok(expr) = h2q.run(e) {
+ let simplified = expr.simplify();
+ if !simplified.iter().any(|s| *s == expr) {
+ span_lint_and_then(self.0, NONMINIMAL_BOOL, e.span, "this boolean expression can be simplified", |db| {
+ for suggestion in &simplified {
+ db.span_suggestion(e.span, "try", suggest(self.0, suggestion, &h2q.0));
+ }
+ });
+ }
+ }
+ }
+}
+
+impl<'a, 'v, 'tcx> Visitor<'v> for NonminimalBoolVisitor<'a, 'tcx> {
+ fn visit_expr(&mut self, e: &'v Expr) {
+ if in_macro(self.0, e.span) { return }
+ match e.node {
+ ExprBinary(binop, _, _) if binop.node == BiOr || binop.node == BiAnd => self.bool_expr(e),
+ ExprUnary(UnNot, ref inner) => {
+ if self.0.tcx.node_types()[&inner.id].is_bool() {
+ self.bool_expr(e);
+ } else {
+ walk_expr(self, e);
+ }
+ },
+ _ => walk_expr(self, e),
+ }
+ }
+}
diff --git a/src/lib.rs b/src/lib.rs
index 0a51385f6f2..575e318e7ad 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -4,6 +4,8 @@
#![feature(iter_arith)]
#![feature(custom_attribute)]
#![feature(slice_patterns)]
+#![feature(question_mark)]
+#![feature(stmt_expr_attributes)]
#![allow(indexing_slicing, shadow_reuse, unknown_lints)]
// this only exists to allow the "dogfood" integration test to work
@@ -35,6 +37,9 @@ extern crate semver;
// for regex checking
extern crate regex_syntax;
+// for finding minimal boolean expressions
+extern crate quine_mc_cluskey;
+
extern crate rustc_plugin;
extern crate rustc_const_eval;
use rustc_plugin::Registry;
@@ -50,6 +55,7 @@ pub mod attrs;
pub mod bit_mask;
pub mod blacklisted_name;
pub mod block_in_if_condition;
+pub mod booleans;
pub mod collapsible_if;
pub mod copies;
pub mod cyclomatic_complexity;
@@ -149,6 +155,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
// end deprecated lints, do not remove this comment, it’s used in `update_lints`
reg.register_late_lint_pass(box types::TypePass);
+ reg.register_late_lint_pass(box booleans::NonminimalBool);
reg.register_late_lint_pass(box misc::TopLevelRefPass);
reg.register_late_lint_pass(box misc::CmpNan);
reg.register_late_lint_pass(box eq_op::EqOp);
@@ -260,6 +267,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
blacklisted_name::BLACKLISTED_NAME,
block_in_if_condition::BLOCK_IN_IF_CONDITION_EXPR,
block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT,
+ booleans::NONMINIMAL_BOOL,
collapsible_if::COLLAPSIBLE_IF,
copies::IF_SAME_THEN_ELSE,
copies::IFS_SAME_COND,
diff --git a/tests/compile-fail/block_in_if_condition.rs b/tests/compile-fail/block_in_if_condition.rs
index 5158e74c1e0..19fa949fb84 100644
--- a/tests/compile-fail/block_in_if_condition.rs
+++ b/tests/compile-fail/block_in_if_condition.rs
@@ -67,7 +67,7 @@ fn pred_test() {
fn condition_is_normal() -> i32 {
let x = 3;
- if true && x == 3 {
+ if true && x == 3 { //~ WARN this boolean expression can be simplified
6
} else {
10
diff --git a/tests/compile-fail/eq_op.rs b/tests/compile-fail/eq_op.rs
index 487185516bf..bd76695e6ad 100644
--- a/tests/compile-fail/eq_op.rs
+++ b/tests/compile-fail/eq_op.rs
@@ -38,7 +38,9 @@ fn main() {
1 - 1; //~ERROR equal expressions
1 / 1; //~ERROR equal expressions
true && true; //~ERROR equal expressions
+ //~|WARN this boolean expression can be simplified
true || true; //~ERROR equal expressions
+ //~|WARN this boolean expression can be simplified
let mut a = vec![1];
a == a; //~ERROR equal expressions