Merge pull request #889 from Manishearth/fix-887
fix #887: New lints for integer/floating-point arithmetic
This commit is contained in:
Manish Goregaokar
commit
469b3d51b4
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
102
src/arithmetic.rs
Normal file
102
src/arithmetic.rs
Normal file
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
14
src/lib.rs
14
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,
|
||||
|
||||
30
tests/compile-fail/arithmetic.rs
Normal file
30
tests/compile-fail/arithmetic.rs
Normal file
@ -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
|
||||
}
|
||||
@ -21,12 +21,18 @@ declare_deprecated_lint_re = re.compile(r'''
|
||||
" (?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
|
||||
|
||||
Loading…
Reference in New Issue
Block a user