OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

New is_integer_const to check more const ints

Browse Source 
This mostly affects loop checks and the modulo_one lint. Tests
were also updated where applicable.
This commit is contained in:
Andre Bogus 2019-09-09 17:01:01 +02:00
parent 144d940c2f
commit 5823e9468d
11 changed files with 98 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
clippy_lints/src/loops.rs
View File

@ -27,7 +27,7 @@ use syntax_pos::{BytePos, Symbol};
use crate::utils::paths;
use crate::utils::{
get_enclosing_block, get_parent_expr, has_iter_method, higher, is_integer_literal, is_refutable, last_path_segment,
get_enclosing_block, get_parent_expr, has_iter_method, higher, is_integer_const, is_refutable, last_path_segment,
match_trait_method, match_type, match_var, multispan_sugg, snippet, snippet_opt, snippet_with_applicability,
span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, SpanlessEq,
};
@ -1096,7 +1096,7 @@ fn check_for_loop_range<'a, 'tcx>(
return;
}
let starts_at_zero = is_integer_literal(start, 0);
let starts_at_zero = is_integer_const(cx, start, 0);
let skip = if starts_at_zero {
String::new()
@ -2042,7 +2042,7 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
match parent.node {
ExprKind::AssignOp(op, ref lhs, ref rhs) => {
if lhs.hir_id == expr.hir_id {
if op.node == BinOpKind::Add && is_integer_literal(rhs, 1) {
if op.node == BinOpKind::Add && is_integer_const(self.cx, rhs, 1) {
*state = match *state {
VarState::Initial if self.depth == 0 => VarState::IncrOnce,
_ => VarState::DontWarn,
@ -2094,7 +2094,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
self.name = Some(ident.name);
self.state = if let Some(ref init) = local.init {
if is_integer_literal(init, 0) {
if is_integer_const(&self.cx, init, 0) {
VarState::Warn
} else {
VarState::Declared
@ -2130,7 +2130,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
self.state = VarState::DontWarn;
},
ExprKind::Assign(ref lhs, ref rhs) if lhs.hir_id == expr.hir_id => {
self.state = if is_integer_literal(rhs, 0) && self.depth == 0 {
self.state = if is_integer_const(&self.cx, rhs, 0) && self.depth == 0 {
VarState::Warn
} else {
VarState::DontWarn

4
clippy_lints/src/misc.rs
View File

@ -12,7 +12,7 @@ use syntax::source_map::{ExpnKind, Span};
use crate::consts::{constant, Constant};
use crate::utils::sugg::Sugg;
use crate::utils::{
get_item_name, get_parent_expr, implements_trait, in_constant, is_integer_literal, iter_input_pats,
get_item_name, get_parent_expr, implements_trait, in_constant, is_integer_const, iter_input_pats,
last_path_segment, match_qpath, match_trait_method, paths, snippet, span_lint, span_lint_and_then,
span_lint_hir_and_then, walk_ptrs_ty, SpanlessEq,
};
@ -388,7 +388,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints {
);
db.span_note(expr.span, "std::f32::EPSILON and std::f64::EPSILON are available.");
});
} else if op == BinOpKind::Rem && is_integer_literal(right, 1) {
} else if op == BinOpKind::Rem && is_integer_const(cx, right, 1) {
span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0");
}
},

18
clippy_lints/src/ranges.rs
View File

@ -8,7 +8,7 @@ use syntax::source_map::Spanned;
use crate::utils::sugg::Sugg;
use crate::utils::{get_trait_def_id, higher, implements_trait, SpanlessEq};
use crate::utils::{is_integer_literal, paths, snippet, snippet_opt, span_lint, span_lint_and_then};
use crate::utils::{is_integer_const, paths, snippet, snippet_opt, span_lint, span_lint_and_then};
declare_clippy_lint! {
/// **What it does:** Checks for calling `.step_by(0)` on iterators,
@ -132,7 +132,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Ranges {
if iter_path.ident.name == sym!(iter);
// range expression in `.zip()` call: `0..x.len()`
if let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::range(cx, zip_arg);
if is_integer_literal(start, 0);
if is_integer_const(cx, start, 0);
// `.len()` call
if let ExprKind::MethodCall(ref len_path, _, ref len_args) = end.node;
if len_path.ident.name == sym!(len) && len_args.len() == 1;
@ -164,7 +164,7 @@ fn check_exclusive_range_plus_one(cx: &LateContext<'_, '_>, expr: &Expr) {
end: Some(end),
limits: RangeLimits::HalfOpen
}) = higher::range(cx, expr);
if let Some(y) = y_plus_one(end);
if let Some(y) = y_plus_one(cx, end);
then {
let span = if expr.span.from_expansion() {
expr.span
@ -209,7 +209,7 @@ fn check_exclusive_range_plus_one(cx: &LateContext<'_, '_>, expr: &Expr) {
fn check_inclusive_range_minus_one(cx: &LateContext<'_, '_>, expr: &Expr) {
if_chain! {
if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::range(cx, expr);
if let Some(y) = y_minus_one(end);
if let Some(y) = y_minus_one(cx, end);
then {
span_lint_and_then(
cx,
@ -239,7 +239,7 @@ fn has_step_by(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
get_trait_def_id(cx, &paths::ITERATOR).map_or(false, |iterator_trait| implements_trait(cx, ty, iterator_trait, &[]))
}
fn y_plus_one(expr: &Expr) -> Option<&Expr> {
fn y_plus_one<'t>(cx: &LateContext<'_, '_>, expr: &'t Expr) -> Option<&'t Expr> {
match expr.node {
ExprKind::Binary(
Spanned {
@ -248,9 +248,9 @@ fn y_plus_one(expr: &Expr) -> Option<&Expr> {
ref lhs,
ref rhs,
) => {
if is_integer_literal(lhs, 1) {
if is_integer_const(cx, lhs, 1) {
Some(rhs)
} else if is_integer_literal(rhs, 1) {
} else if is_integer_const(cx, rhs, 1) {
Some(lhs)
} else {
None
@ -260,7 +260,7 @@ fn y_plus_one(expr: &Expr) -> Option<&Expr> {
}
}
fn y_minus_one(expr: &Expr) -> Option<&Expr> {
fn y_minus_one<'t>(cx: &LateContext<'_, '_>, expr: &'t Expr) -> Option<&'t Expr> {
match expr.node {
ExprKind::Binary(
Spanned {
@ -268,7 +268,7 @@ fn y_minus_one(expr: &Expr) -> Option<&Expr> {
},
ref lhs,
ref rhs,
) if is_integer_literal(rhs, 1) => Some(lhs),
) if is_integer_const(cx, rhs, 1) => Some(lhs),
_ => None,
}
}

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

@ -48,6 +48,7 @@ use syntax::source_map::{Span, DUMMY_SP};
use syntax::symbol::{kw, Symbol};
use crate::reexport::*;
use crate::consts::{constant, Constant};
/// Returns `true` if the two spans come from differing expansions (i.e., one is
/// from a macro and one isn't).
@ -669,6 +670,22 @@ pub fn walk_ptrs_ty_depth(ty: Ty<'_>) -> (Ty<'_>, usize) {
inner(ty, 0)
}
/// Checks whether the given expression is a constant integer of the given value.
/// unlike `is_integer_literal`, this version does const folding
pub fn is_integer_const(cx: &LateContext<'_, '_>, e: &Expr, value: u128) -> bool {
if is_integer_literal(e, value) {
return true;
}
let map = cx.tcx.hir();
let parent_item = map.get_parent_item(e.hir_id);
if let Some((Constant::Int(v), _)) = map.maybe_body_owned_by(parent_item)
.and_then(|body_id| constant(cx, cx.tcx.body_tables(body_id), e)) {
value == v
} else {
false
}
}
/// Checks whether the given expression is a constant literal of the given value.
pub fn is_integer_literal(expr: &Expr, value: u128) -> bool {
// FIXME: use constant folding

8
tests/ui/for_loop.rs
View File

@ -275,6 +275,14 @@ fn main() {
for mid in 1..vec.len() {
let (_, _) = vec.split_at(mid);
}
const ZERO: usize = 0;
for i in ZERO..vec.len() {
if f(&vec[i], &vec[i]) {
panic!("at the disco");
}
}
}
#[allow(dead_code)]

14
tests/ui/for_loop.stderr
View File

@ -152,5 +152,17 @@ LL | for _v in vec.iter().next() {}
|
= note: `-D clippy::iter-next-loop` implied by `-D warnings`
error: aborting due to 21 previous errors
error: the loop variable `i` is only used to index `vec`.
--> $DIR/for_loop.rs:281:14
|
LL | for i in ZERO..vec.len() {
| ^^^^^^^^^^^^^^^
|
= note: `-D clippy::needless-range-loop` implied by `-D warnings`
help: consider using an iterator
|
LL | for <item> in &vec {
| ^^^^^^ ^^^^
error: aborting due to 22 previous errors

7
tests/ui/modulo_one.rs
View File

@ -1,7 +1,14 @@
#![warn(clippy::modulo_one)]
#![allow(clippy::no_effect, clippy::unnecessary_operation)]
static STATIC_ONE: usize = 2 - 1;
fn main() {
10 % 1;
10 % 2;
const ONE: u32 = 1 * 1;
2 % ONE;
5 % STATIC_ONE;
}

24
tests/ui/modulo_one.stderr
View File

@ -1,10 +1,30 @@
error: any number modulo 1 will be 0
--> $DIR/modulo_one.rs:5:5
--> $DIR/modulo_one.rs:7:5
|
LL | 10 % 1;
| ^^^^^^
|
= note: `-D clippy::modulo-one` implied by `-D warnings`
error: aborting due to previous error
error: the operation is ineffective. Consider reducing it to `1`
--> $DIR/modulo_one.rs:10:22
|
LL | const ONE: u32 = 1 * 1;
| ^^^^^
|
= note: `-D clippy::identity-op` implied by `-D warnings`
error: the operation is ineffective. Consider reducing it to `1`
--> $DIR/modulo_one.rs:10:22
|
LL | const ONE: u32 = 1 * 1;
| ^^^^^
error: any number modulo 1 will be 0
--> $DIR/modulo_one.rs:12:5
|
LL | 2 % ONE;
| ^^^^^^^
error: aborting due to 4 previous errors

4
tests/ui/range_plus_minus_one.fixed
View File

@ -32,6 +32,10 @@ fn main() {
let _ = (1..=11);
let _ = ((f() + 1)..=f());
const ONE: usize = 1;
// integer consts are linted, too
for _ in 1..=ONE {}
let mut vec: Vec<()> = std::vec::Vec::new();
vec.drain(..);
}

4
tests/ui/range_plus_minus_one.rs
View File

@ -32,6 +32,10 @@ fn main() {
let _ = (1..11 + 1);
let _ = (f() + 1)..(f() + 1);
const ONE: usize = 1;
// integer consts are linted, too
for _ in 1..ONE + ONE {}
let mut vec: Vec<()> = std::vec::Vec::new();
vec.drain(..);
}

8
tests/ui/range_plus_minus_one.stderr
View File

@ -50,5 +50,11 @@ error: an inclusive range would be more readable
LL | let _ = (f() + 1)..(f() + 1);
| ^^^^^^^^^^^^^^^^^^^^ help: use: `((f() + 1)..=f())`
error: aborting due to 8 previous errors
error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:37:14
|
LL | for _ in 1..ONE + ONE {}
| ^^^^^^^^^^^^ help: use: `1..=ONE`
error: aborting due to 9 previous errors