Merge pull request #889 from Manishearth/fix-887

Manishearth · Manishearth · commit 469b3d51b48c · 2016-05-01T03:41:34.000+05:30
fix #887: New lints for integer/floating-point arithmetic
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -107,6 +107,7 @@ All notable changes to this project will be documented in this file.
 [`explicit_iter_loop`]: https://github.com/Manishearth/rust-clippy/wiki#explicit_iter_loop
 [`extend_from_slice`]: https://github.com/Manishearth/rust-clippy/wiki#extend_from_slice
 [`filter_next`]: https://github.com/Manishearth/rust-clippy/wiki#filter_next
+[`float_arithmetic`]: https://github.com/Manishearth/rust-clippy/wiki#float_arithmetic
 [`float_cmp`]: https://github.com/Manishearth/rust-clippy/wiki#float_cmp
 [`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
@@ -118,6 +119,7 @@ All notable changes to this project will be documented in this file.
 [`indexing_slicing`]: https://github.com/Manishearth/rust-clippy/wiki#indexing_slicing
 [`ineffective_bit_mask`]: https://github.com/Manishearth/rust-clippy/wiki#ineffective_bit_mask
 [`inline_always`]: https://github.com/Manishearth/rust-clippy/wiki#inline_always
+[`integer_arithmetic`]: https://github.com/Manishearth/rust-clippy/wiki#integer_arithmetic
 [`invalid_regex`]: https://github.com/Manishearth/rust-clippy/wiki#invalid_regex
 [`invalid_upcast_comparisons`]: https://github.com/Manishearth/rust-clippy/wiki#invalid_upcast_comparisons
 [`items_after_statements`]: https://github.com/Manishearth/rust-clippy/wiki#items_after_statements
diff --git a/README.md b/README.md
@@ -14,7 +14,7 @@ Table of contents:
 * [License](#license)
 
 ##Lints
-There are 144 lints included in this crate:
+There are 146 lints included in this crate:
 
 name                                                                                                                 | default | meaning
 ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -56,6 +56,7 @@ name
 [explicit_iter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_iter_loop)                             | warn    | for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do
 [extend_from_slice](https://github.com/Manishearth/rust-clippy/wiki#extend_from_slice)                               | warn    | `.extend_from_slice(_)` is a faster way to extend a Vec by a slice
 [filter_next](https://github.com/Manishearth/rust-clippy/wiki#filter_next)                                           | warn    | using `filter(p).next()`, which is more succinctly expressed as `.find(p)`
+[float_arithmetic](https://github.com/Manishearth/rust-clippy/wiki#float_arithmetic)                                 | allow   | Any floating-point arithmetic statement
 [float_cmp](https://github.com/Manishearth/rust-clippy/wiki#float_cmp)                                               | warn    | using `==` or `!=` on float values (as floating-point operations usually involve rounding errors, it is always better to check for approximate equality within small bounds)
 [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`
@@ -67,6 +68,7 @@ name
 [indexing_slicing](https://github.com/Manishearth/rust-clippy/wiki#indexing_slicing)                                 | allow   | indexing/slicing usage
 [ineffective_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#ineffective_bit_mask)                         | warn    | expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2`
 [inline_always](https://github.com/Manishearth/rust-clippy/wiki#inline_always)                                       | warn    | `#[inline(always)]` is a bad idea in most cases
+[integer_arithmetic](https://github.com/Manishearth/rust-clippy/wiki#integer_arithmetic)                             | allow   | Any integer arithmetic statement
 [invalid_regex](https://github.com/Manishearth/rust-clippy/wiki#invalid_regex)                                       | deny    | finds invalid regular expressions in `Regex::new(_)` invocations
 [invalid_upcast_comparisons](https://github.com/Manishearth/rust-clippy/wiki#invalid_upcast_comparisons)             | warn    | a comparison involving an upcast which is always true or false
 [items_after_statements](https://github.com/Manishearth/rust-clippy/wiki#items_after_statements)                     | allow   | finds blocks where an item comes after a statement
diff --git a/src/arithmetic.rs b/src/arithmetic.rs
@@ -0,0 +1,102 @@
+use rustc::hir;
+use rustc::lint::*;
+use syntax::codemap::Span;
+use utils::span_lint;
+
+/// **What it does:** This lint checks for plain integer arithmetic
+///
+/// **Why is this bad?** This is only checked against overflow in debug builds.
+/// In some applications one wants explicitly checked, wrapping or saturating
+/// arithmetic.
+///
+/// **Known problems:** None
+///
+/// **Example:**
+/// ```
+/// a + 1
+/// ```
+declare_restriction_lint! {
+    pub INTEGER_ARITHMETIC,
+    "Any integer arithmetic statement"
+}
+
+/// **What it does:** This lint checks for float arithmetic
+///
+/// **Why is this bad?** For some embedded systems or kernel development, it
+/// can be useful to rule out floating-point numbers
+///
+/// **Known problems:** None
+///
+/// **Example:**
+/// ```
+/// a + 1.0
+/// ```
+declare_restriction_lint! {
+    pub FLOAT_ARITHMETIC,
+    "Any floating-point arithmetic statement"
+}
+
+#[derive(Copy, Clone, Default)]
+pub struct Arithmetic {
+    span: Option<Span>
+}
+
+impl LintPass for Arithmetic {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(INTEGER_ARITHMETIC, FLOAT_ARITHMETIC)
+    }
+}
+
+impl LateLintPass for Arithmetic {
+    fn check_expr(&mut self, cx: &LateContext, expr: &hir::Expr) {
+        if let Some(_) = self.span { return; }
+        match expr.node {
+            hir::ExprBinary(ref op, ref l, ref r) => {
+                match op.node {
+                    hir::BiAnd | hir::BiOr | hir::BiBitAnd |
+                    hir::BiBitOr | hir::BiBitXor | hir::BiShl | hir::BiShr |
+                    hir::BiEq | hir::BiLt | hir::BiLe | hir::BiNe | hir::BiGe |
+                    hir::BiGt => return,
+                    _ => ()
+                }
+                let (l_ty, r_ty) = (cx.tcx.expr_ty(l), cx.tcx.expr_ty(r));
+                if l_ty.is_integral() && r_ty.is_integral() {
+                    span_lint(cx,
+                              INTEGER_ARITHMETIC,
+                              expr.span,
+                              "integer arithmetic detected");
+                    self.span = Some(expr.span);
+                } else if l_ty.is_floating_point() && r_ty.is_floating_point() {
+                    span_lint(cx,
+                              FLOAT_ARITHMETIC,
+                              expr.span,
+                              "floating-point arithmetic detected");
+                    self.span = Some(expr.span);
+                }
+            },
+            hir::ExprUnary(hir::UnOp::UnNeg, ref arg) => {
+                let ty = cx.tcx.expr_ty(arg);
+                if ty.is_integral() {
+                    span_lint(cx,
+                              INTEGER_ARITHMETIC,
+                              expr.span,
+                              "integer arithmetic detected");
+                    self.span = Some(expr.span);
+                } else if ty.is_floating_point() {
+                    span_lint(cx,
+                              FLOAT_ARITHMETIC,
+                              expr.span,
+                              "floating-point arithmetic detected");
+                    self.span = Some(expr.span);
+                }
+            },
+            _ => ()
+        }
+    }
+
+    fn check_expr_post(&mut self, _: &LateContext, expr: &hir::Expr) {
+        if Some(expr.span) == self.span {
+            self.span = None;
+        }
+    }
+}
diff --git a/src/lib.rs b/src/lib.rs
@@ -7,6 +7,7 @@
 #![feature(question_mark)]
 #![feature(stmt_expr_attributes)]
 #![allow(indexing_slicing, shadow_reuse, unknown_lints)]
+#![allow(float_arithmetic, integer_arithmetic)]
 
 // this only exists to allow the "dogfood" integration test to work
 #[allow(dead_code)]
@@ -43,12 +44,19 @@ extern crate rustc_const_eval;
 extern crate rustc_const_math;
 use rustc_plugin::Registry;
 
+macro_rules! declare_restriction_lint {
+    { pub $name:tt, $description:tt } => {
+        declare_lint! { pub $name, Allow, $description }
+    };
+}
+
 pub mod consts;
 #[macro_use]
 pub mod utils;
 
 // begin lints modules, do not remove this comment, it’s used in `update_lints`
 pub mod approx_const;
+pub mod arithmetic;
 pub mod array_indexing;
 pub mod attrs;
 pub mod bit_mask;
@@ -238,6 +246,12 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_late_lint_pass(box neg_multiply::NegMultiply);
     reg.register_late_lint_pass(box unsafe_removed_from_name::UnsafeNameRemoval);
     reg.register_late_lint_pass(box mem_forget::MemForget);
+    reg.register_late_lint_pass(box arithmetic::Arithmetic::default());
+
+    reg.register_lint_group("clippy_restrictions", vec![
+        arithmetic::FLOAT_ARITHMETIC,
+        arithmetic::INTEGER_ARITHMETIC,
+    ]);
 
     reg.register_lint_group("clippy_pedantic", vec![
         array_indexing::INDEXING_SLICING,
diff --git a/tests/compile-fail/arithmetic.rs b/tests/compile-fail/arithmetic.rs
@@ -0,0 +1,30 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![deny(integer_arithmetic, float_arithmetic)]
+#![allow(unused, shadow_reuse, shadow_unrelated, no_effect)]
+fn main() {
+    let i = 1i32;
+    1 + i; //~ERROR integer arithmetic detected
+    i * 2; //~ERROR integer arithmetic detected
+    1 % //~ERROR integer arithmetic detected
+    i / 2; // no error, this is part of the expression in the preceding line
+    i - 2 + 2 - i; //~ERROR integer arithmetic detected
+    -i; //~ERROR integer arithmetic detected
+    
+    i & 1; // no wrapping
+    i | 1; 
+    i ^ 1;
+    i >> 1;
+    i << 1;
+    
+    let f = 1.0f32;
+    
+    f * 2.0; //~ERROR floating-point arithmetic detected
+    
+    1.0 + f; //~ERROR floating-point arithmetic detected
+    f * 2.0; //~ERROR floating-point arithmetic detected
+    f / 2.0; //~ERROR floating-point arithmetic detected
+    f - 2.0 * 4.2; //~ERROR floating-point arithmetic detected
+    -f; //~ERROR floating-point arithmetic detected
+}
diff --git a/util/update_lints.py b/util/update_lints.py
@@ -21,12 +21,18 @@
     " (?P<desc>(?:[^"\\]+|\\.)*) " \s* [})]
 ''', re.VERBOSE | re.DOTALL)
 
+declare_restriction_lint_re = re.compile(r'''
+    declare_restriction_lint! \s* [{(] \s*
+    pub \s+ (?P<name>[A-Z_][A-Z_0-9]*) \s*,\s*
+    " (?P<desc>(?:[^"\\]+|\\.)*) " \s* [})]
+''', re.VERBOSE | re.DOTALL)
+
 nl_escape_re = re.compile(r'\\\n\s*')
 
 wiki_link = 'https://github.com/Manishearth/rust-clippy/wiki'
 
 
-def collect(lints, deprecated_lints, fn):
+def collect(lints, deprecated_lints, restriction_lints, fn):
     """Collect all lints from a file.
 
     Adds entries to the lints list as `(module, name, level, desc)`.
@@ -48,6 +54,14 @@ def collect(lints, deprecated_lints, fn):
                                 match.group('name').lower(),
                                 desc.replace('\\"', '"')))
 
+    for match in declare_restriction_lint_re.finditer(code):
+        # remove \-newline escapes from description string
+        desc = nl_escape_re.sub('', match.group('desc'))
+        restriction_lints.append((os.path.splitext(os.path.basename(fn))[0],
+                                match.group('name').lower(),
+                                "allow",
+                                desc.replace('\\"', '"')))
+        
 
 def gen_table(lints, link=None):
     """Write lint table in Markdown format."""
@@ -86,7 +100,6 @@ def gen_deprecated(lints):
     for lint in lints:
         yield '    store.register_removed("%s", "%s");\n' % (lint[1], lint[2])
 
-
 def replace_region(fn, region_start, region_end, callback,
                    replace_start=True, write_back=True):
     """Replace a region in a file delimited by two lines matching regexes.
@@ -128,6 +141,7 @@ def replace_region(fn, region_start, region_end, callback,
 def main(print_only=False, check=False):
     lints = []
     deprecated_lints = []
+    restriction_lints = []
 
     # check directory
     if not os.path.isfile('src/lib.rs'):
@@ -138,22 +152,24 @@ def main(print_only=False, check=False):
     for root, _, files in os.walk('src'):
         for fn in files:
             if fn.endswith('.rs'):
-                collect(lints, deprecated_lints, os.path.join(root, fn))
+                collect(lints, deprecated_lints, restriction_lints, 
+                        os.path.join(root, fn))
 
     if print_only:
-        sys.stdout.writelines(gen_table(lints))
+        sys.stdout.writelines(gen_table(lints + restriction_lints))
         return
 
     # replace table in README.md
     changed = replace_region(
         'README.md', r'^name +\|', '^$',
-        lambda: gen_table(lints, link=wiki_link),
+        lambda: gen_table(lints + restriction_lints, link=wiki_link),
         write_back=not check)
 
     changed |= replace_region(
         'README.md',
         r'^There are \d+ lints included in this crate:', "",
-        lambda: ['There are %d lints included in this crate:\n' % len(lints)],
+        lambda: ['There are %d lints included in this crate:\n' % (len(lints)
+            + len(restriction_lints))],
         write_back=not check)
 
     # update the links in the CHANGELOG
@@ -162,13 +178,14 @@ def main(print_only=False, check=False):
         "<!-- begin autogenerated links to wiki -->",
         "<!-- end autogenerated links to wiki -->",
         lambda: ["[`{0}`]: {1}#{0}\n".format(l[1], wiki_link) for l in
-                 sorted(lints + deprecated_lints, key=lambda l: l[1])],
+                 sorted(lints + restriction_lints + deprecated_lints, 
+                        key=lambda l: l[1])],
         replace_start=False, write_back=not check)
 
     # update the `pub mod` list
     changed |= replace_region(
         'src/lib.rs', r'begin lints modules', r'end lints modules',
-        lambda: gen_mods(lints),
+        lambda: gen_mods(lints + restriction_lints),
         replace_start=False, write_back=not check)
 
     # same for "clippy" lint collection
@@ -190,6 +207,12 @@ def main(print_only=False, check=False):
         lambda: gen_group(lints, levels=('allow',)),
         replace_start=False, write_back=not check)
 
+    # same for "clippy_restrictions" lint collection
+    changed |= replace_region(
+        'src/lib.rs', r'reg.register_lint_group\("clippy_restrictions"', 
+        r'\]\);', lambda: gen_group(restriction_lints),
+        replace_start=False, write_back=not check)
+
     if check and changed:
         print('Please run util/update_lints.py to regenerate lints lists.')
         return 1
