squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Merge branch 'pr-613'

Browse source 
Conflicts:
	src/lib.rs
	src/types.rs
This commit is contained in:
Manish Goregaokar 2016-02-10 01:03:26 +05:30
commit 93461afffc
5 changed files with 183 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
README.md
View file

@ -10,7 +10,7 @@ There are 117 lints included in this crate:
name | default | meaning
---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
[absurd_unsigned_comparisons](https://github.com/Manishearth/rust-clippy/wiki#absurd_unsigned_comparisons) | warn | testing whether an unsigned integer is non-positive
[absurd_extreme_comparisons](https://github.com/Manishearth/rust-clippy/wiki#absurd_extreme_comparisons) | warn | a comparison involving a maximum or minimum value involves a case that is always true or always false
[approx_constant](https://github.com/Manishearth/rust-clippy/wiki#approx_constant) | warn | the approximate of a known float constant (in `std::f64::consts` or `std::f32::consts`) is found; suggests to use the constant
[bad_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#bad_bit_mask) | warn | expressions of the form `_ & mask == select` that will only ever return `true` or `false` (because in the example `select` containing bits that `mask` doesn't have)
[block_in_if_condition_expr](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_expr) | warn | braces can be eliminated in conditions that are expressions, e.g `if { true } ...`

4
src/lib.rs
View file

@ -156,7 +156,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
reg.register_late_lint_pass(box print::PrintLint);
reg.register_late_lint_pass(box vec::UselessVec);
reg.register_late_lint_pass(box drop_ref::DropRefPass);
reg.register_late_lint_pass(box types::AbsurdUnsignedComparisons);
reg.register_late_lint_pass(box types::AbsurdExtremeComparisons);
reg.register_late_lint_pass(box regex::RegexPass);
reg.register_late_lint_pass(box copies::CopyAndPaste);
@ -271,7 +271,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
strings::STRING_LIT_AS_BYTES,
temporary_assignment::TEMPORARY_ASSIGNMENT,
transmute::USELESS_TRANSMUTE,
types::ABSURD_UNSIGNED_COMPARISONS,
types::ABSURD_EXTREME_COMPARISONS,
types::BOX_VEC,
types::CHAR_LIT_AS_U8,
types::LET_UNIT_VALUE,

167
src/types.rs
View file

@ -5,6 +5,7 @@ use rustc_front::util::{is_comparison_binop, binop_to_string};
use syntax::codemap::Span;
use rustc_front::intravisit::{FnKind, Visitor, walk_ty};
use rustc::middle::ty;
use rustc::middle::const_eval;
use syntax::ast::{IntTy, UintTy, FloatTy};
use utils::*;
@ -579,54 +580,160 @@ impl LateLintPass for CharLitAsU8 {
}
}
/// **What it does:** This lint checks for expressions where an unsigned integer is tested to be non-positive and suggests testing for equality with zero instead.
/// **What it does:** This lint checks for comparisons where one side of the relation is either the minimum or maximum value for its type and warns if it involves a case that is always true or always false. Only integer and boolean types are checked.
///
/// **Why is this bad?** `x <= 0` may mislead the reader into thinking `x` can be negative. `x == 0` makes explicit that zero is the only possibility.
/// **Why is this bad?** An expression like `min <= x` may misleadingly imply that is is possible for `x` to be less than the minimum. Expressions like `max < x` are probably mistakes.
///
/// **Known problems:** None
///
/// **Example:** `vec.len() <= 0`
/// **Example:** `vec.len() <= 0`, `100 > std::i32::MAX`
declare_lint! {
pub ABSURD_UNSIGNED_COMPARISONS, Warn,
"testing whether an unsigned integer is non-positive"
pub ABSURD_EXTREME_COMPARISONS, Warn,
"a comparison involving a maximum or minimum value involves a case that is always \
true or always false"
}
pub struct AbsurdUnsignedComparisons;
pub struct AbsurdExtremeComparisons;
impl LintPass for AbsurdUnsignedComparisons {
impl LintPass for AbsurdExtremeComparisons {
fn get_lints(&self) -> LintArray {
lint_array!(ABSURD_UNSIGNED_COMPARISONS)
lint_array!(ABSURD_EXTREME_COMPARISONS)
}
}
fn is_zero_lit(expr: &Expr) -> bool {
use syntax::ast::Lit_;
if let ExprLit(ref l) = expr.node {
if let Lit_::LitInt(val, _) = l.node {
return val == 0;
}
}
false
enum ExtremeType {
Minimum,
Maximum,
}
impl LateLintPass for AbsurdUnsignedComparisons {
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
if let ExprBinary(ref cmp, ref lhs, ref rhs) = expr.node {
let op = cmp.node;
struct ExtremeExpr<'a> {
which: ExtremeType,
expr: &'a Expr,
}
let comparee = match op {
BiLe if is_zero_lit(rhs) => lhs, // x <= 0
BiGe if is_zero_lit(lhs) => rhs, // 0 >= x
_ => return,
enum AbsurdComparisonResult {
AlwaysFalse,
AlwaysTrue,
InequalityImpossible,
}
fn detect_absurd_comparison<'a>(cx: &LateContext, op: BinOp_, lhs: &'a Expr, rhs: &'a Expr)
-> Option<(ExtremeExpr<'a>, AbsurdComparisonResult)> {
use types::ExtremeType::*;
use types::AbsurdComparisonResult::*;
type Extr<'a> = ExtremeExpr<'a>;
// Put the expression in the form lhs < rhs or lhs <= rhs.
enum Rel { Lt, Le };
let (rel, lhs2, rhs2) = match op {
BiLt => (Rel::Lt, lhs, rhs),
BiLe => (Rel::Le, lhs, rhs),
BiGt => (Rel::Lt, rhs, lhs),
BiGe => (Rel::Le, rhs, lhs),
_ => return None,
};
if let ty::TyUint(_) = cx.tcx.expr_ty(comparee).sty {
let lx = detect_extreme_expr(cx, lhs2);
let rx = detect_extreme_expr(cx, rhs2);
Some(match rel {
Rel::Lt => {
match (lx, rx) {
(Some(l @ Extr { which: Maximum, ..}), _) => (l, AlwaysFalse), // max < x
(_, Some(r @ Extr { which: Minimum, ..})) => (r, AlwaysFalse), // x < min
_ => return None,
}
}
Rel::Le => {
match (lx, rx) {
(Some(l @ Extr { which: Minimum, ..}), _) => (l, AlwaysTrue), // min <= x
(Some(l @ Extr { which: Maximum, ..}), _) => (l, InequalityImpossible), //max <= x
(_, Some(r @ Extr { which: Minimum, ..})) => (r, InequalityImpossible), // x <= min
(_, Some(r @ Extr { which: Maximum, ..})) => (r, AlwaysTrue), // x <= max
_ => return None,
}
}
})
}
fn detect_extreme_expr<'a>(cx: &LateContext, expr: &'a Expr) -> Option<ExtremeExpr<'a>> {
use rustc::middle::const_eval::EvalHint::ExprTypeChecked;
use types::ExtremeType::*;
use rustc::middle::const_eval::ConstVal::*;
let ty = &cx.tcx.expr_ty(expr).sty;
match *ty {
ty::TyBool | ty::TyInt(_) | ty::TyUint(_) => (),
_ => return None,
};
let cv = match const_eval::eval_const_expr_partial(cx.tcx, expr, ExprTypeChecked, None) {
Ok(val) => val,
Err(_) => return None,
};
let which = match (ty, cv) {
(&ty::TyBool, Bool(false)) => Minimum,
(&ty::TyInt(IntTy::TyIs), Int(x)) if x == ::std::isize::MIN as i64 => Minimum,
(&ty::TyInt(IntTy::TyI8), Int(x)) if x == ::std::i8::MIN as i64 => Minimum,
(&ty::TyInt(IntTy::TyI16), Int(x)) if x == ::std::i16::MIN as i64 => Minimum,
(&ty::TyInt(IntTy::TyI32), Int(x)) if x == ::std::i32::MIN as i64 => Minimum,
(&ty::TyInt(IntTy::TyI64), Int(x)) if x == ::std::i64::MIN as i64 => Minimum,
(&ty::TyUint(UintTy::TyUs), Uint(x)) if x == ::std::usize::MIN as u64 => Minimum,
(&ty::TyUint(UintTy::TyU8), Uint(x)) if x == ::std::u8::MIN as u64 => Minimum,
(&ty::TyUint(UintTy::TyU16), Uint(x)) if x == ::std::u16::MIN as u64 => Minimum,
(&ty::TyUint(UintTy::TyU32), Uint(x)) if x == ::std::u32::MIN as u64 => Minimum,
(&ty::TyUint(UintTy::TyU64), Uint(x)) if x == ::std::u64::MIN as u64 => Minimum,
(&ty::TyBool, Bool(true)) => Maximum,
(&ty::TyInt(IntTy::TyIs), Int(x)) if x == ::std::isize::MAX as i64 => Maximum,
(&ty::TyInt(IntTy::TyI8), Int(x)) if x == ::std::i8::MAX as i64 => Maximum,
(&ty::TyInt(IntTy::TyI16), Int(x)) if x == ::std::i16::MAX as i64 => Maximum,
(&ty::TyInt(IntTy::TyI32), Int(x)) if x == ::std::i32::MAX as i64 => Maximum,
(&ty::TyInt(IntTy::TyI64), Int(x)) if x == ::std::i64::MAX as i64 => Maximum,
(&ty::TyUint(UintTy::TyUs), Uint(x)) if x == ::std::usize::MAX as u64 => Maximum,
(&ty::TyUint(UintTy::TyU8), Uint(x)) if x == ::std::u8::MAX as u64 => Maximum,
(&ty::TyUint(UintTy::TyU16), Uint(x)) if x == ::std::u16::MAX as u64 => Maximum,
(&ty::TyUint(UintTy::TyU32), Uint(x)) if x == ::std::u32::MAX as u64 => Maximum,
(&ty::TyUint(UintTy::TyU64), Uint(x)) if x == ::std::u64::MAX as u64 => Maximum,
_ => return None,
};
Some(ExtremeExpr { which: which, expr: expr })
}
impl LateLintPass for AbsurdExtremeComparisons {
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
use types::ExtremeType::*;
use types::AbsurdComparisonResult::*;
if let ExprBinary(ref cmp, ref lhs, ref rhs) = expr.node {
if let Some((culprit, result)) = detect_absurd_comparison(cx, cmp.node, lhs, rhs) {
if !in_macro(cx, expr.span) {
let msg = "testing whether an unsigned integer is non-positive";
let help = format!("consider using {} == 0 instead",
snippet(cx, comparee.span, "x"));
span_help_and_lint(cx, ABSURD_UNSIGNED_COMPARISONS, expr.span, msg, &help);
let msg = "this comparison involving the minimum or maximum element for this \
type contains a case that is always true or always false";
let conclusion = match result {
AlwaysFalse => "this comparison is always false".to_owned(),
AlwaysTrue => "this comparison is always true".to_owned(),
InequalityImpossible =>
format!("the case where the two sides are not equal never occurs, \
consider using {} == {} instead",
snippet(cx, lhs.span, "lhs"),
snippet(cx, rhs.span, "rhs")),
};
let help = format!("because {} is the {} value for this type, {}",
snippet(cx, culprit.expr.span, "x"),
match culprit.which { Minimum => "minimum", Maximum => "maximum" },
conclusion);
span_help_and_lint(cx, ABSURD_EXTREME_COMPARISONS, expr.span, msg, &help);
}
}
}

44
tests/compile-fail/absurd-extreme-comparisons.rs Normal file
View file

@ -0,0 +1,44 @@
#![feature(plugin)]
#![plugin(clippy)]
#![deny(absurd_extreme_comparisons)]
#![allow(unused, eq_op)]
fn main() {
const Z: u32 = 0;
let u: u32 = 42;
u <= 0; //~ERROR this comparison involving the minimum or maximum element for this type contains a case that is always true or always false
u <= Z; //~ERROR this comparison involving
u < Z; //~ERROR this comparison involving
Z >= u; //~ERROR this comparison involving
Z > u; //~ERROR this comparison involving
u > std::u32::MAX; //~ERROR this comparison involving
u >= std::u32::MAX; //~ERROR this comparison involving
std::u32::MAX < u; //~ERROR this comparison involving
std::u32::MAX <= u; //~ERROR this comparison involving
1-1 > u;
//~^ ERROR this comparison involving
//~| HELP because 1-1 is the minimum value for this type, this comparison is always false
u >= !0;
//~^ ERROR this comparison involving
//~| HELP because !0 is the maximum value for this type, the case where the two sides are not equal never occurs, consider using u == !0 instead
u <= 12 - 2*6;
//~^ ERROR this comparison involving
//~| HELP because 12 - 2*6 is the minimum value for this type, the case where the two sides are not equal never occurs, consider using u == 12 - 2*6 instead
let i: i8 = 0;
i < -127 - 1; //~ERROR this comparison involving
std::i8::MAX >= i; //~ERROR this comparison involving
3-7 < std::i32::MIN; //~ERROR this comparison involving
let b = false;
b >= true; //~ERROR this comparison involving
false > b; //~ERROR this comparison involving
u > 0; // ok
// this is handled by unit_cmp
() < {}; //~WARNING <-comparison of unit values detected.
}

14
tests/compile-fail/absurd_unsigned_comparisons.rs
View file

@ -1,14 +0,0 @@
#![feature(plugin)]
#![plugin(clippy)]
#![allow(unused)]
#[deny(absurd_unsigned_comparisons)]
fn main() {
1u32 <= 0; //~ERROR testing whether an unsigned integer is non-positive
1u8 <= 0; //~ERROR testing whether an unsigned integer is non-positive
1i32 <= 0;
0 >= 1u32; //~ERROR testing whether an unsigned integer is non-positive
0 >= 1;
1u32 > 0;
}