Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-10 15:14:29 +00:00
Code Issues Projects Releases Packages Wiki Activity

Merge branch 'master' of https://github.com/Manishearth/rust-clippy into #471

Browse source
This commit is contained in:
mcarton 2016-01-02 19:36:45 +01:00
commit 542685dad1
32 changed files with 609 additions and 227 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
Cargo.toml
View file

@ -1,6 +1,6 @@
[package]
name = "clippy"
version = "0.0.32"
version = "0.0.35"
authors = [
"Manish Goregaokar <manishsmail@gmail.com>",
"Andre Bogus <bogusandre@gmail.com>",
@ -17,7 +17,7 @@ name = "clippy"
plugin = true
[dependencies]
unicode-normalization = "*"
unicode-normalization = "0.1"
[dev-dependencies]
compiletest_rs = "0.0.11"

11
README.md
View file

@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code.
[Jump to usage instructions](#usage)
##Lints
There are 86 lints included in this crate:
There are 89 lints included in this crate:
name | default | meaning
---------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@ -28,6 +28,7 @@ name
[eq_op](https://github.com/Manishearth/rust-clippy/wiki#eq_op) | warn | equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`)
[explicit_counter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_counter_loop) | warn | for-looping with an explicit counter when `_.enumerate()` would do
[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
[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_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)
[identity_op](https://github.com/Manishearth/rust-clippy/wiki#identity_op) | warn | using identity operations, e.g. `x + 0` or `y / 1`
[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`
@ -41,7 +42,7 @@ name
[map_clone](https://github.com/Manishearth/rust-clippy/wiki#map_clone) | warn | using `.map(|x| x.clone())` to clone an iterator or option's contents (recommends `.cloned()` instead)
[match_bool](https://github.com/Manishearth/rust-clippy/wiki#match_bool) | warn | a match on boolean expression; recommends `if..else` block instead
[match_overlapping_arm](https://github.com/Manishearth/rust-clippy/wiki#match_overlapping_arm) | warn | a match has overlapping arms
[match_ref_pats](https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats) | warn | a match has all arms prefixed with `&`; the match expression can be dereferenced instead
[match_ref_pats](https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats) | warn | a match or `if let` has all arms prefixed with `&`; the match expression can be dereferenced instead
[min_max](https://github.com/Manishearth/rust-clippy/wiki#min_max) | warn | `min(_, max(_, _))` (or vice versa) with bounds clamping the result to a constant
[modulo_one](https://github.com/Manishearth/rust-clippy/wiki#modulo_one) | warn | taking a number modulo 1, which always returns 0
[mut_mut](https://github.com/Manishearth/rust-clippy/wiki#mut_mut) | allow | usage of double-mut refs, e.g. `&mut &mut ...` (either copy'n'paste error, or shows a fundamental misunderstanding of references)
@ -56,10 +57,11 @@ name
[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
[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)`)
[option_map_unwrap_or_else](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or_else) | warn | using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `map_or_else(g, f)`)
[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)`
[option_map_unwrap_or_else](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or_else) | warn | using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `map_or_else(g, f)`
[option_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#option_unwrap_used) | allow | using `Option.unwrap()`, which should at least get a better message using `expect()`
[out_of_bounds_indexing](https://github.com/Manishearth/rust-clippy/wiki#out_of_bounds_indexing) | deny | out of bound constant indexing
[panic_params](https://github.com/Manishearth/rust-clippy/wiki#panic_params) | warn | missing parameters in `panic!`
[precedence](https://github.com/Manishearth/rust-clippy/wiki#precedence) | warn | catches operations where precedence may be unclear. See the wiki for a list of cases caught
[ptr_arg](https://github.com/Manishearth/rust-clippy/wiki#ptr_arg) | warn | fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively
[range_step_by_zero](https://github.com/Manishearth/rust-clippy/wiki#range_step_by_zero) | warn | using Range::step_by(0), which produces an infinite iterator
@ -68,6 +70,7 @@ name
[redundant_pattern](https://github.com/Manishearth/rust-clippy/wiki#redundant_pattern) | warn | using `name @ _` in a pattern
[result_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#result_unwrap_used) | allow | using `Result.unwrap()`, which might be better handled
[reverse_range_loop](https://github.com/Manishearth/rust-clippy/wiki#reverse_range_loop) | warn | Iterating over an empty range, such as `10..0` or `5..5`
[search_is_some](https://github.com/Manishearth/rust-clippy/wiki#search_is_some) | warn | using an iterator search followed by `is_some()`, which is more succinctly expressed as a call to `any()`
[shadow_reuse](https://github.com/Manishearth/rust-clippy/wiki#shadow_reuse) | allow | rebinding a name to an expression that re-uses the original value, e.g. `let x = x + 1`
[shadow_same](https://github.com/Manishearth/rust-clippy/wiki#shadow_same) | allow | rebinding a name to itself, e.g. `let mut x = &mut x`
[shadow_unrelated](https://github.com/Manishearth/rust-clippy/wiki#shadow_unrelated) | allow | The name is re-bound without even using the original value

6
src/attrs.rs
View file

@ -6,9 +6,9 @@ use reexport::*;
use syntax::codemap::Span;
use syntax::attr::*;
use syntax::ast::{Attribute, MetaList, MetaWord};
use utils::{in_macro, match_path, span_lint};
use utils::{in_macro, match_path, span_lint, BEGIN_UNWIND};
/// **What it does:** This lint warns on items annotated with `#[inline(always)]`, unless the annotated function is empty or simply panics.
/// **What it does:** This lint `Warn`s on items annotated with `#[inline(always)]`, unless the annotated function is empty or simply panics.
///
/// **Why is this bad?** While there are valid uses of this annotation (and once you know when to use it, by all means `allow` this lint), it's a common newbie-mistake to pepper one's code with it.
///
@ -94,7 +94,7 @@ fn is_relevant_expr(expr: &Expr) -> bool {
ExprRet(None) | ExprBreak(_) => false,
ExprCall(ref path_expr, _) => {
if let ExprPath(_, ref path) = path_expr.node {
!match_path(path, &["std", "rt", "begin_unwind"])
!match_path(path, &BEGIN_UNWIND)
} else { true }
}
_ => true

41
src/block_in_if_condition.rs
View file

@ -3,7 +3,7 @@ use rustc::lint::{LateLintPass, LateContext, LintArray, LintPass};
use rustc_front::intravisit::{Visitor, walk_expr};
use utils::*;
/// **What it does:** This lint checks for `if` conditions that use blocks to contain an expression.
/// **What it does:** This lint checks for `if` conditions that use blocks to contain an expression. It is `Warn` by default.
///
/// **Why is this bad?** It isn't really rust style, same as using parentheses to contain expressions.
///
@ -15,7 +15,7 @@ declare_lint! {
"braces can be eliminated in conditions that are expressions, e.g `if { true } ...`"
}
/// **What it does:** This lint checks for `if` conditions that use blocks containing statements, or conditions that use closures with blocks.
/// **What it does:** This lint checks for `if` conditions that use blocks containing statements, or conditions that use closures with blocks. It is `Warn` by default.
///
/// **Why is this bad?** Using blocks in the condition makes it hard to read.
///
@ -74,23 +74,30 @@ impl LateLintPass for BlockInIfCondition {
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
if let ExprIf(ref check, ref then, _) = expr.node {
if let ExprBlock(ref block) = check.node {
if block.stmts.is_empty() {
if let Some(ref ex) = block.expr {
// don't dig into the expression here, just suggest that they remove
// the block
span_help_and_lint(cx, BLOCK_IN_IF_CONDITION_EXPR, check.span,
BRACED_EXPR_MESSAGE,
&format!("try\nif {} {} ... ", snippet_block(cx, ex.span, ".."),
if block.rules == DefaultBlock {
if block.stmts.is_empty() {
if let Some(ref ex) = block.expr {
// don't dig into the expression here, just suggest that they remove
// the block
if differing_macro_contexts(expr.span, ex.span) {
return;
}
span_help_and_lint(cx, BLOCK_IN_IF_CONDITION_EXPR, check.span,
BRACED_EXPR_MESSAGE,
&format!("try\nif {} {} ... ", snippet_block(cx, ex.span, ".."),
snippet_block(cx, then.span, "..")));
}
} else {
if differing_macro_contexts(expr.span, block.stmts[0].span) {
return;
}
// move block higher
span_help_and_lint(cx, BLOCK_IN_IF_CONDITION_STMT, check.span,
COMPLEX_BLOCK_MESSAGE,
&format!("try\nlet res = {};\nif res {} ... ",
snippet_block(cx, block.span, ".."),
snippet_block(cx, then.span, "..")));
}
} else {
// move block higher
span_help_and_lint(cx, BLOCK_IN_IF_CONDITION_STMT, check.span,
COMPLEX_BLOCK_MESSAGE,
&format!("try\nlet res = {};\nif res {} ... ",
snippet_block(cx, block.span, ".."),
snippet_block(cx, then.span, "..")));
}
} else {
let mut visitor = ExVisitor { found_block: None };

2
src/consts.rs
View file

@ -75,7 +75,7 @@ impl Constant {
if let ConstantInt(val, _) = *self {
val // TODO we may want to check the sign if any
} else {
panic!("Could not convert a {:?} to u64");
panic!("Could not convert a {:?} to u64", self);
}
}

13
src/cyclomatic_complexity.rs
View file

@ -9,7 +9,7 @@ use syntax::attr::*;
use syntax::ast::Attribute;
use rustc_front::intravisit::{Visitor, walk_expr};
use utils::{in_macro, LimitStack};
use utils::{in_macro, LimitStack, span_help_and_lint};
/// **What it does:** It `Warn`s on methods with high cyclomatic complexity
///
@ -59,8 +59,8 @@ impl CyclomaticComplexity {
} else {
let rust_cc = cc + divergence - narms;
if rust_cc > self.limit.limit() {
cx.span_lint_help(CYCLOMATIC_COMPLEXITY, span,
&format!("The function has a cyclomatic complexity of {}.", rust_cc),
span_help_and_lint(cx, CYCLOMATIC_COMPLEXITY, span,
&format!("The function has a cyclomatic complexity of {}", rust_cc),
"You could split it up into multiple smaller functions");
}
}
@ -140,8 +140,9 @@ fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, div: u64, span: Span) {
#[cfg(not(feature="debugging"))]
fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, div: u64, span: Span) {
if cx.current_level(CYCLOMATIC_COMPLEXITY) != Level::Allow {
cx.sess().span_note(span, &format!("Clippy encountered a bug calculating cyclomatic complexity \
(hide this message with `#[allow(cyclomatic_complexity)]`): \
cc = {}, arms = {}, div = {}. Please file a bug report.", cc, narms, div));
cx.sess().span_note_without_error(span,
&format!("Clippy encountered a bug calculating cyclomatic complexity \
(hide this message with `#[allow(cyclomatic_complexity)]`): \
cc = {}, arms = {}, div = {}. Please file a bug report.", cc, narms, div));
}
}

6
src/escape.rs
View file

@ -14,11 +14,11 @@ use utils::span_lint;
pub struct EscapePass;
/// **What it does:** This lint checks for usage of `Box<T>` where an unboxed `T` would work fine
/// **What it does:** This lint checks for usage of `Box<T>` where an unboxed `T` would work fine. It is `Warn` by default.
///
/// **Why is this bad?** This is an unnecessary allocation, and bad for performance
/// **Why is this bad?** This is an unnecessary allocation, and bad for performance. It is only necessary to allocate if you wish to move the box into something.
///
/// It is only necessary to allocate if you wish to move the box into something.
/// **Known problems:** None
///
/// **Example:**
///

4
src/eta_reduction.rs
View file

@ -84,9 +84,9 @@ fn check_closure(cx: &LateContext, expr: &Expr) {
}
span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span,
"redundant closure found",
|| {
|db| {
if let Some(snippet) = snippet_opt(cx, caller.span) {
cx.sess().span_suggestion(expr.span,
db.span_suggestion(expr.span,
"remove closure as shown:",
snippet);
}

2
src/len_zero.rs
View file

@ -135,7 +135,7 @@ fn check_len_zero(cx: &LateContext, span: Span, name: &Name,
has_is_empty(cx, &args[0]) {
span_lint(cx, LEN_ZERO, span, &format!(
"consider replacing the len comparison with `{}{}.is_empty()`",
op, snippet(cx, args[0].span, "_")))
op, snippet(cx, args[0].span, "_")));
}
}
}

5
src/lib.rs
View file

@ -67,6 +67,7 @@ pub mod cyclomatic_complexity;
pub mod escape;
pub mod misc_early;
pub mod array_indexing;
pub mod panic;
mod reexport {
pub use syntax::ast::{Name, NodeId};
@ -123,6 +124,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
reg.register_early_lint_pass(box misc_early::MiscEarly);
reg.register_late_lint_pass(box misc::UsedUnderscoreBinding);
reg.register_late_lint_pass(box array_indexing::ArrayIndexing);
reg.register_late_lint_pass(box panic::PanicPass);
reg.register_lint_group("clippy_pedantic", vec![
methods::OPTION_UNWRAP_USED,
@ -175,9 +177,11 @@ pub fn plugin_registrar(reg: &mut Registry) {
matches::MATCH_OVERLAPPING_ARM,
matches::MATCH_REF_PATS,
matches::SINGLE_MATCH,
methods::FILTER_NEXT,
methods::OK_EXPECT,
methods::OPTION_MAP_UNWRAP_OR,
methods::OPTION_MAP_UNWRAP_OR_ELSE,
methods::SEARCH_IS_SOME,
methods::SHOULD_IMPLEMENT_TRAIT,
methods::STR_TO_STRING,
methods::STRING_TO_STRING,
@ -199,6 +203,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
needless_update::NEEDLESS_UPDATE,
no_effect::NO_EFFECT,
open_options::NONSENSICAL_OPEN_OPTIONS,
panic::PANIC_PARAMS,
precedence::PRECEDENCE,
ptr_arg::PTR_ARG,
ranges::RANGE_STEP_BY_ZERO,

4
src/loops.rs
View file

@ -50,7 +50,7 @@ declare_lint!{ pub EXPLICIT_ITER_LOOP, Warn,
declare_lint!{ pub ITER_NEXT_LOOP, Warn,
"for-looping over `_.next()` which is probably not intended" }
/// **What it does:** This lint detects `loop + match` combinations that are easier written as a `while let` loop.
/// **What it does:** This lint detects `loop + match` combinations that are easier written as a `while let` loop. It is `Warn` by default.
///
/// **Why is this bad?** The `while let` loop is usually shorter and more readable
///
@ -85,7 +85,7 @@ declare_lint!{ pub UNUSED_COLLECT, Warn,
"`collect()`ing an iterator without using the result; this is usually better \
written as a for loop" }
/// **What it does:** This lint checks for loops over ranges `x..y` where both `x` and `y` are constant and `x` is greater or equal to `y`, unless the range is reversed or has a negative `.step_by(_)`.
/// **What it does:** This lint checks for loops over ranges `x..y` where both `x` and `y` are constant and `x` is greater or equal to `y`, unless the range is reversed or has a negative `.step_by(_)`. It is `Warn` by default.
///
/// **Why is it bad?** Such loops will either be skipped or loop until wrap-around (in debug code, this may `panic!()`). Both options are probably not intended.
///

2
src/map_clone.rs
View file

@ -8,7 +8,7 @@ use utils::{walk_ptrs_ty, walk_ptrs_ty_depth};
///
/// **Why is this bad?** It makes the code less readable.
///
/// **Known problems:** False negative: The lint currently misses mapping `Clone::clone` directly. Issue #436 is tracking this.
/// **Known problems:** None
///
/// **Example:** `x.map(|e| e.clone());`
declare_lint!(pub MAP_CLONE, Warn,

4
src/matches.rs
View file

@ -27,7 +27,7 @@ declare_lint!(pub SINGLE_MATCH, Warn,
"a match statement with a single nontrivial arm (i.e, where the other arm \
is `_ => {}`) is used; recommends `if let` instead");
/// **What it does:** This lint checks for matches where all arms match a reference, suggesting to remove the reference and deref the matched expression instead. It is `Warn` by default.
/// **What it does:** This lint checks for matches where all arms match a reference, suggesting to remove the reference and deref the matched expression instead. It also checks for `if let &foo = bar` blocks. It is `Warn` by default.
///
/// **Why is this bad?** It just makes the code less readable. That reference destructuring adds nothing to the code.
///
@ -43,7 +43,7 @@ declare_lint!(pub SINGLE_MATCH, Warn,
/// }
/// ```
declare_lint!(pub MATCH_REF_PATS, Warn,
"a match has all arms prefixed with `&`; the match expression can be \
"a match or `if let` has all arms prefixed with `&`; the match expression can be \
dereferenced instead");
/// **What it does:** This lint checks for matches where match expression is a `bool`. It suggests to replace the expression with an `if...else` block. It is `Warn` by default.

311
src/methods.rs
View file

@ -5,9 +5,10 @@ use rustc::middle::subst::{Subst, TypeSpace};
use std::iter;
use std::borrow::Cow;
use utils::{snippet, span_lint, span_note_and_lint, match_path, match_type, walk_ptrs_ty_depth,
walk_ptrs_ty};
use utils::{snippet, span_lint, span_note_and_lint, match_path, match_type, method_chain_args,
match_trait_method, walk_ptrs_ty_depth, walk_ptrs_ty};
use utils::{OPTION_PATH, RESULT_PATH, STRING_PATH};
use utils::MethodArgs;
use self::SelfKind::*;
use self::OutType::*;
@ -134,7 +135,7 @@ declare_lint!(pub OK_EXPECT, Warn,
/// **Example:** `x.map(|a| a + 1).unwrap_or(0)`
declare_lint!(pub OPTION_MAP_UNWRAP_OR, Warn,
"using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as \
`map_or(a, f)`)");
`map_or(a, f)`");
/// **What it does:** This lint `Warn`s on `_.map(_).unwrap_or_else(_)`.
///
@ -145,7 +146,29 @@ declare_lint!(pub OPTION_MAP_UNWRAP_OR, Warn,
/// **Example:** `x.map(|a| a + 1).unwrap_or_else(some_function)`
declare_lint!(pub OPTION_MAP_UNWRAP_OR_ELSE, Warn,
"using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as \
`map_or_else(g, f)`)");
`map_or_else(g, f)`");
/// **What it does:** This lint `Warn`s on `_.filter(_).next()`.
///
/// **Why is this bad?** Readability, this can be written more concisely as `_.find(_)`.
///
/// **Known problems:** None.
///
/// **Example:** `iter.filter(|x| x == 0).next()`
declare_lint!(pub FILTER_NEXT, Warn,
"using `filter(p).next()`, which is more succinctly expressed as `.find(p)`");
/// **What it does:** This lint `Warn`s on an iterator search (such as `find()`, `position()`, or
/// `rposition()`) followed by a call to `is_some()`.
///
/// **Why is this bad?** Readability, this can be written more concisely as `_.any(_)`.
///
/// **Known problems:** None.
///
/// **Example:** `iter.find(|x| x == 0).is_some()`
declare_lint!(pub SEARCH_IS_SOME, Warn,
"using an iterator search followed by `is_some()`, which is more succinctly \
expressed as a call to `any()`");
impl LintPass for MethodsPass {
fn get_lints(&self) -> LintArray {
@ -157,107 +180,33 @@ impl LintPass for MethodsPass {
impl LateLintPass for MethodsPass {
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
if let ExprMethodCall(ref name, _, ref args) = expr.node {
let (obj_ty, ptr_depth) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&args[0]));
match &*name.node.as_str() {
"unwrap" if match_type(cx, obj_ty, &OPTION_PATH) => {
span_lint(cx, OPTION_UNWRAP_USED, expr.span,
"used unwrap() on an Option value. If you don't want \
to handle the None case gracefully, consider using \
expect() to provide a better panic message");
},
"unwrap" if match_type(cx, obj_ty, &RESULT_PATH) => {
span_lint(cx, RESULT_UNWRAP_USED, expr.span,
"used unwrap() on a Result value. Graceful handling \
of Err values is preferred");
},
"to_string" if obj_ty.sty == ty::TyStr => {
let mut arg_str = snippet(cx, args[0].span, "_");
if ptr_depth > 1 {
arg_str = Cow::Owned(format!(
"({}{})",
iter::repeat('*').take(ptr_depth - 1).collect::<String>(),
arg_str));
}
span_lint(cx, STR_TO_STRING, expr.span, &format!(
"`{}.to_owned()` is faster", arg_str));
},
"to_string" if match_type(cx, obj_ty, &STRING_PATH) => {
span_lint(cx, STRING_TO_STRING, expr.span, "`String.to_string()` is a no-op; use \
`clone()` to make a copy");
},
"expect" => if let ExprMethodCall(ref inner_name, _, ref inner_args) = args[0].node {
if inner_name.node.as_str() == "ok"
&& match_type(cx, cx.tcx.expr_ty(&inner_args[0]), &RESULT_PATH) {
let result_type = cx.tcx.expr_ty(&inner_args[0]);
if let Some(error_type) = get_error_type(cx, result_type) {
if has_debug_impl(error_type, cx) {
span_lint(cx, OK_EXPECT, expr.span,
"called `ok().expect()` on a Result \
value. You can call `expect` directly \
on the `Result`");
}
}
}
},
// check Option.map(_).unwrap_or(_)
"unwrap_or" => if let ExprMethodCall(ref inner_name, _, ref inner_args) = args[0].node {
if inner_name.node.as_str() == "map"
&& match_type(cx, cx.tcx.expr_ty(&inner_args[0]), &OPTION_PATH) {
// lint message
let msg =
"called `map(f).unwrap_or(a)` on an Option value. This can be done \
more directly by calling `map_or(a, f)` instead";
// get args to map() and unwrap_or()
let map_arg = snippet(cx, inner_args[1].span, "..");
let unwrap_arg = snippet(cx, args[1].span, "..");
// lint, with note if neither arg is > 1 line and both map() and
// unwrap_or() have the same span
let multiline = map_arg.lines().count() > 1
|| unwrap_arg.lines().count() > 1;
let same_span = inner_args[1].span.expn_id == args[1].span.expn_id;
if same_span && !multiline {
span_note_and_lint(
cx, OPTION_MAP_UNWRAP_OR, expr.span, msg, expr.span,
&format!("replace this with map_or({1}, {0})",
map_arg, unwrap_arg)
);
}
else if same_span && multiline {
span_lint(cx, OPTION_MAP_UNWRAP_OR, expr.span, msg);
};
}
},
// check Option.map(_).unwrap_or_else(_)
"unwrap_or_else" => if let ExprMethodCall(ref inner_name, _, ref inner_args) = args[0].node {
if inner_name.node.as_str() == "map"
&& match_type(cx, cx.tcx.expr_ty(&inner_args[0]), &OPTION_PATH) {
// lint message
let msg =
"called `map(f).unwrap_or_else(g)` on an Option value. This can be \
done more directly by calling `map_or_else(g, f)` instead";
// get args to map() and unwrap_or_else()
let map_arg = snippet(cx, inner_args[1].span, "..");
let unwrap_arg = snippet(cx, args[1].span, "..");
// lint, with note if neither arg is > 1 line and both map() and
// unwrap_or_else() have the same span
let multiline = map_arg.lines().count() > 1
|| unwrap_arg.lines().count() > 1;
let same_span = inner_args[1].span.expn_id == args[1].span.expn_id;
if same_span && !multiline {
span_note_and_lint(
cx, OPTION_MAP_UNWRAP_OR_ELSE, expr.span, msg, expr.span,
&format!("replace this with map_or_else({1}, {0})",
map_arg, unwrap_arg)
);
}
else if same_span && multiline {
span_lint(cx, OPTION_MAP_UNWRAP_OR_ELSE, expr.span, msg);
};
}
},
_ => {},
if let ExprMethodCall(_, _, _) = expr.node {
if let Some(arglists) = method_chain_args(expr, &["unwrap"]) {
lint_unwrap(cx, expr, arglists[0]);
}
else if let Some(arglists) = method_chain_args(expr, &["to_string"]) {
lint_to_string(cx, expr, arglists[0]);
}
else if let Some(arglists) = method_chain_args(expr, &["ok", "expect"]) {
lint_ok_expect(cx, expr, arglists[0]);
}
else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or"]) {
lint_map_unwrap_or(cx, expr, arglists[0], arglists[1]);
}
else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or_else"]) {
lint_map_unwrap_or_else(cx, expr, arglists[0], arglists[1]);
}
else if let Some(arglists) = method_chain_args(expr, &["filter", "next"]) {
lint_filter_next(cx, expr, arglists[0]);
}
else if let Some(arglists) = method_chain_args(expr, &["find", "is_some"]) {
lint_search_is_some(cx, expr, "find", arglists[0], arglists[1]);
}
else if let Some(arglists) = method_chain_args(expr, &["position", "is_some"]) {
lint_search_is_some(cx, expr, "position", arglists[0], arglists[1]);
}
else if let Some(arglists) = method_chain_args(expr, &["rposition", "is_some"]) {
lint_search_is_some(cx, expr, "rposition", arglists[0], arglists[1]);
}
}
}
@ -304,6 +253,158 @@ impl LateLintPass for MethodsPass {
}
}
#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
/// lint use of `unwrap()` for `Option`s and `Result`s
fn lint_unwrap(cx: &LateContext, expr: &Expr, unwrap_args: &MethodArgs) {
let (obj_ty, _) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&unwrap_args[0]));
if match_type(cx, obj_ty, &OPTION_PATH) {
span_lint(cx, OPTION_UNWRAP_USED, expr.span,
"used unwrap() on an Option value. If you don't want to handle the None case \
gracefully, consider using expect() to provide a better panic message");
}
else if match_type(cx, obj_ty, &RESULT_PATH) {
span_lint(cx, RESULT_UNWRAP_USED, expr.span,
"used unwrap() on a Result value. Graceful handling of Err values is preferred");
}
}
#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
/// lint use of `to_string()` for `&str`s and `String`s
fn lint_to_string(cx: &LateContext, expr: &Expr, to_string_args: &MethodArgs) {
let (obj_ty, ptr_depth) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&to_string_args[0]));
if obj_ty.sty == ty::TyStr {
let mut arg_str = snippet(cx, to_string_args[0].span, "_");
if ptr_depth > 1 {
arg_str = Cow::Owned(format!(
"({}{})",
iter::repeat('*').take(ptr_depth - 1).collect::<String>(),
arg_str));
}
span_lint(cx, STR_TO_STRING, expr.span,
&format!("`{}.to_owned()` is faster", arg_str));
}
else if match_type(cx, obj_ty, &STRING_PATH) {
span_lint(cx, STRING_TO_STRING, expr.span,
"`String.to_string()` is a no-op; use `clone()` to make a copy");
}
}
#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
/// lint use of `ok().expect()` for `Result`s
fn lint_ok_expect(cx: &LateContext, expr: &Expr, ok_args: &MethodArgs) {
// lint if the caller of `ok()` is a `Result`
if match_type(cx, cx.tcx.expr_ty(&ok_args[0]), &RESULT_PATH) {
let result_type = cx.tcx.expr_ty(&ok_args[0]);
if let Some(error_type) = get_error_type(cx, result_type) {
if has_debug_impl(error_type, cx) {
span_lint(cx, OK_EXPECT, expr.span,
"called `ok().expect()` on a Result value. You can call `expect` \
directly on the `Result`");
}
}
}
}
#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
/// lint use of `map().unwrap_or()` for `Option`s
fn lint_map_unwrap_or(cx: &LateContext, expr: &Expr, map_args: &MethodArgs,
unwrap_args: &MethodArgs) {
// lint if the caller of `map()` is an `Option`
if match_type(cx, cx.tcx.expr_ty(&map_args[0]), &OPTION_PATH) {
// lint message
let msg = "called `map(f).unwrap_or(a)` on an Option value. This can be done more \
directly by calling `map_or(a, f)` instead";
// get snippets for args to map() and unwrap_or()
let map_snippet = snippet(cx, map_args[1].span, "..");
let unwrap_snippet = snippet(cx, unwrap_args[1].span, "..");
// lint, with note if neither arg is > 1 line and both map() and
// unwrap_or() have the same span
let multiline = map_snippet.lines().count() > 1
|| unwrap_snippet.lines().count() > 1;
let same_span = map_args[1].span.expn_id == unwrap_args[1].span.expn_id;
if same_span && !multiline {
span_note_and_lint(
cx, OPTION_MAP_UNWRAP_OR, expr.span, msg, expr.span,
&format!("replace `map({0}).unwrap_or({1})` with `map_or({1}, {0})`", map_snippet,
unwrap_snippet)
);
}
else if same_span && multiline {
span_lint(cx, OPTION_MAP_UNWRAP_OR, expr.span, msg);
};
}
}
#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
/// lint use of `map().unwrap_or_else()` for `Option`s
fn lint_map_unwrap_or_else(cx: &LateContext, expr: &Expr, map_args: &MethodArgs,
unwrap_args: &MethodArgs) {
// lint if the caller of `map()` is an `Option`
if match_type(cx, cx.tcx.expr_ty(&map_args[0]), &OPTION_PATH) {
// lint message
let msg = "called `map(f).unwrap_or_else(g)` on an Option value. This can be done more \
directly by calling `map_or_else(g, f)` instead";
// get snippets for args to map() and unwrap_or_else()
let map_snippet = snippet(cx, map_args[1].span, "..");
let unwrap_snippet = snippet(cx, unwrap_args[1].span, "..");
// lint, with note if neither arg is > 1 line and both map() and
// unwrap_or_else() have the same span
let multiline = map_snippet.lines().count() > 1
|| unwrap_snippet.lines().count() > 1;
let same_span = map_args[1].span.expn_id == unwrap_args[1].span.expn_id;
if same_span && !multiline {
span_note_and_lint(
cx, OPTION_MAP_UNWRAP_OR_ELSE, expr.span, msg, expr.span,
&format!("replace `map({0}).unwrap_or_else({1})` with `with map_or_else({1}, {0})`",
map_snippet, unwrap_snippet)
);
}
else if same_span && multiline {
span_lint(cx, OPTION_MAP_UNWRAP_OR_ELSE, expr.span, msg);
};
}
}
#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
/// lint use of `filter().next() for Iterators`
fn lint_filter_next(cx: &LateContext, expr: &Expr, filter_args: &MethodArgs) {
// lint if caller of `.filter().next()` is an Iterator
if match_trait_method(cx, expr, &["core", "iter", "Iterator"]) {
let msg = "called `filter(p).next()` on an Iterator. This is more succinctly expressed by \
calling `.find(p)` instead.";
let filter_snippet = snippet(cx, filter_args[1].span, "..");
if filter_snippet.lines().count() <= 1 { // add note if not multi-line
span_note_and_lint(cx, FILTER_NEXT, expr.span, msg, expr.span,
&format!("replace `filter({0}).next()` with `find({0})`", filter_snippet));
}
else {
span_lint(cx, FILTER_NEXT, expr.span, msg);
}
}
}
#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
/// lint searching an Iterator followed by `is_some()`
fn lint_search_is_some(cx: &LateContext, expr: &Expr, search_method: &str, search_args: &MethodArgs,
is_some_args: &MethodArgs) {
// lint if caller of search is an Iterator
if match_trait_method(cx, &*is_some_args[0], &["core", "iter", "Iterator"]) {
let msg = format!("called `is_some()` after searching an iterator with {}. This is more \
succinctly expressed by calling `any()`.", search_method);
let search_snippet = snippet(cx, search_args[1].span, "..");
if search_snippet.lines().count() <= 1 { // add note if not multi-line
span_note_and_lint(cx, SEARCH_IS_SOME, expr.span, &msg, expr.span,
&format!("replace `{0}({1}).is_some()` with `any({1})`", search_method,
search_snippet));
}
else {
span_lint(cx, SEARCH_IS_SOME, expr.span, &msg);
}
}
}
// Given a `Result<T, E>` type, return its error type (`E`)
fn get_error_type<'a>(cx: &LateContext, ty: ty::Ty<'a>) -> Option<ty::Ty<'a>> {
if !match_type(cx, ty, &RESULT_PATH) {

4
src/minmax.rs
View file

@ -8,7 +8,7 @@ use consts::{Constant, constant_simple};
use utils::{match_def_path, span_lint};
use self::MinMax::{Min, Max};
/// **What it does:** This lint checks for expressions where `std::cmp::min` and `max` are used to clamp values, but switched so that the result is constant.
/// **What it does:** This lint checks for expressions where `std::cmp::min` and `max` are used to clamp values, but switched so that the result is constant. It is `Warn` by default.
///
/// **Why is this bad?** This is in all probability not the intended outcome. At the least it hurts readability of the code.
///
@ -37,7 +37,7 @@ impl LateLintPass for MinMaxPass {
(_, None) | (Max, Some(Less)) | (Min, Some(Greater)) => (),
_ => {
span_lint(cx, MIN_MAX, expr.span,
"this min/max combination leads to constant result")
"this min/max combination leads to constant result");
}
}
}

2
src/misc.rs
View file

@ -281,7 +281,7 @@ impl LateLintPass for ModuloOne {
}
}
/// **What it does:** This lint checks for patterns in the form `name @ _`.
/// **What it does:** This lint checks for patterns in the form `name @ _`. It is `Warn` by default.
///
/// **Why is this bad?** It's almost always more readable to just use direct bindings.
///

8
src/mut_mut.rs
View file

@ -30,8 +30,8 @@ impl LateLintPass for MutMut {
}
fn check_ty(&mut self, cx: &LateContext, ty: &Ty) {
unwrap_mut(ty).and_then(unwrap_mut).map_or((), |_| span_lint(cx, MUT_MUT,
ty.span, "generally you want to avoid `&mut &mut _` if possible"))
unwrap_mut(ty).and_then(unwrap_mut).map_or((), |_| { span_lint(cx, MUT_MUT,
ty.span, "generally you want to avoid `&mut &mut _` if possible"); });
}
}
@ -52,12 +52,12 @@ fn check_expr_mut(cx: &LateContext, expr: &Expr) {
cx.tcx.expr_ty(e).sty {
span_lint(cx, MUT_MUT, expr.span,
"this expression mutably borrows a mutable reference. \
Consider reborrowing")
Consider reborrowing");
}
},
|_| {
span_lint(cx, MUT_MUT, expr.span,
"generally you want to avoid `&mut &mut _` if possible")
"generally you want to avoid `&mut &mut _` if possible");
}
)
})

10
src/mut_reference.rs
View file

@ -4,7 +4,7 @@ use utils::span_lint;
use rustc::middle::ty::{TypeAndMut, TypeVariants, MethodCall, TyS};
use syntax::ptr::P;
/// **What it does:** This lint detects giving a mutable reference to a function that only requires an immutable reference.
/// **What it does:** This lint detects giving a mutable reference to a function that only requires an immutable reference. It is `Warn` by default.
///
/// **Why is this bad?** The immutable reference rules out all other references to the value. Also the code misleads about the intent of the call site.
///
@ -36,7 +36,7 @@ impl LateLintPass for UnnecessaryMutPassed {
match borrowed_table.node_types.get(&fn_expr.id) {
Some(function_type) => {
if let ExprPath(_, ref path) = fn_expr.node {
check_arguments(cx, &arguments, function_type,
check_arguments(cx, &arguments, function_type,
&format!("{}", path));
}
}
@ -50,7 +50,7 @@ impl LateLintPass for UnnecessaryMutPassed {
ExprMethodCall(ref name, _, ref arguments) => {
let method_call = MethodCall::expr(e.id);
match borrowed_table.method_map.get(&method_call) {
Some(method_type) => check_arguments(cx, &arguments, method_type.ty,
Some(method_type) => check_arguments(cx, &arguments, method_type.ty,
&format!("{}", name.node.as_str())),
None => unreachable!(), // Just like above, this should never happen.
};
@ -68,9 +68,9 @@ fn check_arguments(cx: &LateContext, arguments: &[P<Expr>], type_definition: &Ty
TypeVariants::TyRef(_, TypeAndMut {mutbl: MutImmutable, ..}) |
TypeVariants::TyRawPtr(TypeAndMut {mutbl: MutImmutable, ..}) => {
if let ExprAddrOf(MutMutable, _) = argument.node {
span_lint(cx, UNNECESSARY_MUT_PASSED,
span_lint(cx, UNNECESSARY_MUT_PASSED,
argument.span, &format!("The function/method \"{}\" \
doesn't need a mutable reference",
doesn't need a mutable reference",
name));
}
}

2
src/mutex_atomic.rs
View file

@ -63,7 +63,7 @@ impl LateLintPass for MutexAtomic {
ty::TyInt(t) if t != ast::TyIs =>
span_lint(cx, MUTEX_INTEGER, expr.span, &msg),
_ => span_lint(cx, MUTEX_ATOMIC, expr.span, &msg)
}
};
}
}
}

49
src/panic.rs Normal file
View file

@ -0,0 +1,49 @@
use rustc::lint::*;
use rustc_front::hir::*;
use syntax::ast::Lit_::LitStr;
use utils::{span_lint, in_external_macro, match_path, BEGIN_UNWIND};
/// **What it does:** Warn about missing parameters in `panic!`.
///
/// **Known problems:** Should you want to use curly brackets in `panic!` without any parameter,
/// this lint will warn.
///
/// **Example:**
/// ```
/// panic!("This panic! is probably missing a parameter there: {}");
/// ```
declare_lint!(pub PANIC_PARAMS, Warn, "missing parameters in `panic!`");
#[allow(missing_copy_implementations)]
pub struct PanicPass;
impl LintPass for PanicPass {
fn get_lints(&self) -> LintArray {
lint_array!(PANIC_PARAMS)
}
}
impl LateLintPass for PanicPass {
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
if_let_chain! {[
in_external_macro(cx, expr.span),
let ExprBlock(ref block) = expr.node,
let Some(ref ex) = block.expr,
let ExprCall(ref fun, ref params) = ex.node,
params.len() == 2,
let ExprPath(None, ref path) = fun.node,
match_path(path, &BEGIN_UNWIND),
let ExprLit(ref lit) = params[0].node,
let LitStr(ref string, _) = lit.node,
string.contains('{'),
let Some(sp) = cx.sess().codemap()
.with_expn_info(expr.span.expn_id,
|info| info.map(|i| i.call_site))
], {
span_lint(cx, PANIC_PARAMS, sp,
"You probably are missing some parameter in your `panic!` call");
}}
}
}

25
src/precedence.rs
View file

@ -4,7 +4,7 @@ use syntax::ast::*;
use utils::{span_lint, snippet};
/// **What it does:** This lint checks for operations where precedence may be unclear and `Warn`'s about them by default, suggesting to add parentheses. Currently it catches the following:
/// **What it does:** This lint checks for operations where precedence may be unclear and `Warn`s about them by default, suggesting to add parentheses. Currently it catches the following:
/// * mixed usage of arithmetic and bit shifting/combining operators without parentheses
/// * a "negative" numeric literal (which is really a unary `-` followed by a numeric literal) followed by a method call
///
@ -33,21 +33,27 @@ impl EarlyLintPass for Precedence {
if let ExprBinary(Spanned { node: op, ..}, ref left, ref right) = expr.node {
if !is_bit_op(op) { return; }
match (is_arith_expr(left), is_arith_expr(right)) {
(true, true) => span_lint(cx, PRECEDENCE, expr.span,
(true, true) => {
span_lint(cx, PRECEDENCE, expr.span,
&format!("operator precedence can trip the unwary. \
Consider parenthesizing your expression:\
`({}) {} ({})`", snippet(cx, left.span, ".."),
op.to_string(), snippet(cx, right.span, ".."))),
(true, false) => span_lint(cx, PRECEDENCE, expr.span,
op.to_string(), snippet(cx, right.span, "..")));
},
(true, false) => {
span_lint(cx, PRECEDENCE, expr.span,
&format!("operator precedence can trip the unwary. \
Consider parenthesizing your expression:\
`({}) {} {}`", snippet(cx, left.span, ".."),
op.to_string(), snippet(cx, right.span, ".."))),
(false, true) => span_lint(cx, PRECEDENCE, expr.span,
op.to_string(), snippet(cx, right.span, "..")));
},
(false, true) => {
span_lint(cx, PRECEDENCE, expr.span,
&format!("operator precedence can trip the unwary. \
Consider parenthesizing your expression:\
`{} {} ({})`", snippet(cx, left.span, ".."),
op.to_string(), snippet(cx, right.span, ".."))),
op.to_string(), snippet(cx, right.span, "..")));
},
_ => (),
}
}
@ -57,12 +63,13 @@ impl EarlyLintPass for Precedence {
if let Some(slf) = args.first() {
if let ExprLit(ref lit) = slf.node {
match lit.node {
LitInt(..) | LitFloat(..) | LitFloatUnsuffixed(..) =>
LitInt(..) | LitFloat(..) | LitFloatUnsuffixed(..) => {
span_lint(cx, PRECEDENCE, expr.span, &format!(
"unary minus has lower precedence than \
method call. Consider adding parentheses \
to clarify your intent: -({})",
snippet(cx, rhs.span, ".."))),
snippet(cx, rhs.span, "..")));
}
_ => ()
}
}

8
src/returns.rs
View file

@ -75,9 +75,9 @@ impl ReturnPass {
if in_external_macro(cx, spans.1) {return;}
span_lint_and_then(cx, NEEDLESS_RETURN, spans.0,
"unneeded return statement",
|| {
|db| {
if let Some(snippet) = snippet_opt(cx, spans.1) {
cx.sess().span_suggestion(spans.0,
db.span_suggestion(spans.0,
"remove `return` as shown:",
snippet);
}
@ -105,11 +105,11 @@ impl ReturnPass {
fn emit_let_lint(&mut self, cx: &EarlyContext, lint_span: Span, note_span: Span) {
if in_external_macro(cx, note_span) {return;}
span_lint(cx, LET_AND_RETURN, lint_span,
let mut db = span_lint(cx, LET_AND_RETURN, lint_span,
"returning the result of a let binding from a block. \
Consider returning the expression directly.");
if cx.current_level(LET_AND_RETURN) != Level::Allow {
cx.sess().span_note(note_span,
db.span_note(note_span,
"this expression can be directly returned");
}
}

22
src/shadow.rs
View file

@ -7,7 +7,7 @@ use rustc_front::intravisit::{Visitor, FnKind};
use rustc::lint::*;
use rustc::middle::def::Def::{DefVariant, DefStruct};
use utils::{is_from_for_desugar, in_external_macro, snippet, span_lint, span_note_and_lint};
use utils::{is_from_for_desugar, in_external_macro, snippet, span_lint, span_note_and_lint, DiagnosticWrapper};
/// **What it does:** This lint checks for bindings that shadow other bindings already in scope, while just changing reference level or mutability. It is `Allow` by default.
///
@ -180,39 +180,39 @@ fn check_pat(cx: &LateContext, pat: &Pat, init: &Option<&Expr>, span: Span,
fn lint_shadow<T>(cx: &LateContext, name: Name, span: Span, lspan: Span, init:
&Option<T>, prev_span: Span) where T: Deref<Target=Expr> {
fn note_orig(cx: &LateContext, lint: &'static Lint, span: Span) {
fn note_orig(cx: &LateContext, mut db: DiagnosticWrapper, lint: &'static Lint, span: Span) {
if cx.current_level(lint) != Level::Allow {
cx.sess().span_note(span, "previous binding is here");
db.span_note(span, "previous binding is here");
}
}
if let Some(ref expr) = *init {
if is_self_shadow(name, expr) {
span_lint(cx, SHADOW_SAME, span, &format!(
let db = span_lint(cx, SHADOW_SAME, span, &format!(
"{} is shadowed by itself in {}",
snippet(cx, lspan, "_"),
snippet(cx, expr.span, "..")));
note_orig(cx, SHADOW_SAME, prev_span);
note_orig(cx, db, SHADOW_SAME, prev_span);
} else {
if contains_self(name, expr) {
span_note_and_lint(cx, SHADOW_REUSE, lspan, &format!(
let db = span_note_and_lint(cx, SHADOW_REUSE, lspan, &format!(
"{} is shadowed by {} which reuses the original value",
snippet(cx, lspan, "_"),
snippet(cx, expr.span, "..")),
expr.span, "initialization happens here");
note_orig(cx, SHADOW_REUSE, prev_span);
note_orig(cx, db, SHADOW_REUSE, prev_span);
} else {
span_note_and_lint(cx, SHADOW_UNRELATED, lspan, &format!(
let db = span_note_and_lint(cx, SHADOW_UNRELATED, lspan, &format!(
"{} is shadowed by {}",
snippet(cx, lspan, "_"),
snippet(cx, expr.span, "..")),
expr.span, "initialization happens here");
note_orig(cx, SHADOW_UNRELATED, prev_span);
note_orig(cx, db, SHADOW_UNRELATED, prev_span);
}
}
} else {
span_lint(cx, SHADOW_UNRELATED, span, &format!(
let db = span_lint(cx, SHADOW_UNRELATED, span, &format!(
"{} shadows a previous declaration", snippet(cx, lspan, "_")));
note_orig(cx, SHADOW_UNRELATED, prev_span);
note_orig(cx, db, SHADOW_UNRELATED, prev_span);
}
}

6
src/strings.rs
View file

@ -11,7 +11,7 @@ use eq_op::is_exp_equal;
use utils::{match_type, span_lint, walk_ptrs_ty, get_parent_expr};
use utils::STRING_PATH;
/// **What it does:** This lint matches code of the form `x = x + y` (without `let`!)
/// **What it does:** This lint matches code of the form `x = x + y` (without `let`!). It is `Allow` by default.
///
/// **Why is this bad?** Because this expression needs another copy as opposed to `x.push_str(y)` (in practice LLVM will usually elide it, though). Despite [llogiq](https://github.com/llogiq)'s reservations, this lint also is `allow` by default, as some people opine that it's more readable.
///
@ -75,13 +75,13 @@ impl LateLintPass for StringAdd {
}
span_lint(cx, STRING_ADD, e.span,
"you added something to a string. \
Consider using `String::push_str()` instead")
Consider using `String::push_str()` instead");
}
} else if let ExprAssign(ref target, ref src) = e.node {
if is_string(cx, target) && is_add(cx, src, target) {
span_lint(cx, STRING_ADD_ASSIGN, e.span,
"you assigned the result of adding something to this string. \
Consider using `String::push_str()` instead")
Consider using `String::push_str()` instead");
}
}
}

12
src/types.rs
View file

@ -9,15 +9,13 @@ use syntax::ast::IntTy::*;
use syntax::ast::UintTy::*;
use syntax::ast::FloatTy::*;
use utils::{match_type, snippet, span_lint, span_help_and_lint};
use utils::{is_from_for_desugar, in_macro, in_external_macro};
use utils::{LL_PATH, VEC_PATH};
use utils::*;
/// Handles all the linting of funky types
#[allow(missing_copy_implementations)]
pub struct TypePass;
/// **What it does:** This lint checks for use of `Box<Vec<_>>` anywhere in the code.
/// **What it does:** This lint checks for use of `Box<Vec<_>>` anywhere in the code. It is `Warn` by default.
///
/// **Why is this bad?** `Vec` already keeps its contents in a separate area on the heap. So if you `Box` it, you just add another level of indirection without any benefit whatsoever.
///
@ -26,7 +24,8 @@ pub struct TypePass;
/// **Example:** `struct X { values: Box<Vec<Foo>> }`
declare_lint!(pub BOX_VEC, Warn,
"usage of `Box<Vec<T>>`, vector elements are already on the heap");
/// **What it does:** This lint checks for usage of any `LinkedList`, suggesting to use a `Vec` or a `VecDeque` (formerly called `RingBuf`).
/// **What it does:** This lint checks for usage of any `LinkedList`, suggesting to use a `Vec` or a `VecDeque` (formerly called `RingBuf`). It is `Warn` by default.
///
/// **Why is this bad?** Gankro says:
///
@ -49,6 +48,9 @@ impl LintPass for TypePass {
impl LateLintPass for TypePass {
fn check_ty(&mut self, cx: &LateContext, ast_ty: &Ty) {
if in_macro(cx, ast_ty.span) {
return
}
if let Some(ty) = cx.tcx.ast_ty_to_ty_cache.borrow().get(&ast_ty.id) {
if let ty::TyBox(ref inner) = ty.sty {
if match_type(cx, inner, &VEC_PATH) {

122
src/utils.rs
View file

@ -8,9 +8,14 @@ use rustc::middle::ty;
use std::borrow::Cow;
use syntax::ast::Lit_::*;
use syntax::ast;
use syntax::errors::DiagnosticBuilder;
use syntax::ptr::P;
use rustc::session::Session;
use std::str::FromStr;
use std::ops::{Deref, DerefMut};
pub type MethodArgs = HirVec<P<Expr>>;
// module DefPaths for certain structs/enums we check for
pub const OPTION_PATH: [&'static str; 3] = ["core", "option", "Option"];
@ -21,6 +26,7 @@ pub const LL_PATH: [&'static str; 3] = ["collections", "linked_list", "Linke
pub const OPEN_OPTIONS_PATH: [&'static str; 3] = ["std", "fs", "OpenOptions"];
pub const MUTEX_PATH: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"];
pub const CLONE_PATH: [&'static str; 2] = ["Clone", "clone"];
pub const BEGIN_UNWIND:[&'static str; 3] = ["std", "rt", "begin_unwind"];
/// Produce a nested chain of if-lets and ifs from the patterns:
///
@ -68,6 +74,10 @@ macro_rules! if_let_chain {
};
}
/// Returns true if the two spans come from differing expansions (i.e. one is from a macro and one isn't)
pub fn differing_macro_contexts(sp1: Span, sp2: Span) -> bool {
sp1.expn_id != sp2.expn_id
}
/// returns true if this expn_info was expanded by any macro
pub fn in_macro<T: LintContext>(cx: &T, span: Span) -> bool {
cx.sess().codemap().with_expn_info(span.expn_id,
@ -135,6 +145,7 @@ pub fn match_impl_method(cx: &LateContext, expr: &Expr, path: &[&str]) -> bool {
false
}
}
/// check if method call given in "expr" belongs to given trait
pub fn match_trait_method(cx: &LateContext, expr: &Expr, path: &[&str]) -> bool {
let method_call = ty::MethodCall::expr(expr.id);
@ -162,6 +173,31 @@ pub fn match_path_ast(path: &ast::Path, segments: &[&str]) -> bool {
|(a, b)| a.identifier.name.as_str() == *b)
}
/// match an Expr against a chain of methods, and return the matched Exprs. For example, if `expr`
/// represents the `.baz()` in `foo.bar().baz()`, `matched_method_chain(expr, &["bar", "baz"])`
/// will return a Vec containing the Exprs for `.bar()` and `.baz()`
pub fn method_chain_args<'a>(expr: &'a Expr, methods: &[&str]) -> Option<Vec<&'a MethodArgs>> {
let mut current = expr;
let mut matched = Vec::with_capacity(methods.len());
for method_name in methods.iter().rev() { // method chains are stored last -> first
if let ExprMethodCall(ref name, _, ref args) = current.node {
if name.node.as_str() == *method_name {
matched.push(args); // build up `matched` backwards
current = &args[0] // go to parent expression
}
else {
return None;
}
}
else {
return None;
}
}
matched.reverse(); // reverse `matched`, so that it is in the same order as `methods`
Some(matched)
}
/// get the name of the item the expression is in, if available
pub fn get_item_name(cx: &LateContext, expr: &Expr) -> Option<Name> {
let parent_id = cx.tcx.map.get_parent(expr.id);
@ -277,63 +313,91 @@ pub fn get_enclosing_block<'c>(cx: &'c LateContext, node: NodeId) -> Option<&'c
} else { None }
}
#[cfg(not(feature="structured_logging"))]
pub fn span_lint<T: LintContext>(cx: &T, lint: &'static Lint, sp: Span, msg: &str) {
cx.span_lint(lint, sp, msg);
if cx.current_level(lint) != Level::Allow {
cx.sess().fileline_help(sp, &format!("for further information visit \
https://github.com/Manishearth/rust-clippy/wiki#{}",
lint.name_lower()))
pub struct DiagnosticWrapper<'a>(pub DiagnosticBuilder<'a>);
impl<'a> Drop for DiagnosticWrapper<'a> {
fn drop(&mut self) {
self.0.emit();
}
}
impl<'a> DerefMut for DiagnosticWrapper<'a> {
fn deref_mut(&mut self) -> &mut DiagnosticBuilder<'a> {
&mut self.0
}
}
impl<'a> Deref for DiagnosticWrapper<'a> {
type Target = DiagnosticBuilder<'a>;
fn deref(&self) -> &DiagnosticBuilder<'a> {
&self.0
}
}
#[cfg(not(feature="structured_logging"))]
pub fn span_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint,
sp: Span, msg: &str) -> DiagnosticWrapper<'a> {
let mut db = cx.struct_span_lint(lint, sp, msg);
if cx.current_level(lint) != Level::Allow {
db.fileline_help(sp, &format!("for further information visit \
https://github.com/Manishearth/rust-clippy/wiki#{}",
lint.name_lower()));
}
DiagnosticWrapper(db)
}
#[cfg(feature="structured_logging")]
pub fn span_lint<T: LintContext>(cx: &T, lint: &'static Lint, sp: Span, msg: &str) {
pub fn span_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint,
sp: Span, msg: &str) -> DiagnosticWrapper<'a> {
// lint.name / lint.desc is can give details of the lint
// cx.sess().codemap() has all these nice functions for line/column/snippet details
// http://doc.rust-lang.org/syntax/codemap/struct.CodeMap.html#method.span_to_string
cx.span_lint(lint, sp, msg);
let mut db = cx.struct_span_lint(lint, sp, msg);
if cx.current_level(lint) != Level::Allow {
cx.sess().fileline_help(sp, &format!("for further information visit \
db.fileline_help(sp, &format!("for further information visit \
https://github.com/Manishearth/rust-clippy/wiki#{}",
lint.name_lower()))
lint.name_lower()));
}
DiagnosticWrapper(db)
}
pub fn span_help_and_lint<T: LintContext>(cx: &T, lint: &'static Lint, span: Span,
msg: &str, help: &str) {
cx.span_lint(lint, span, msg);
pub fn span_help_and_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint, span: Span,
msg: &str, help: &str) -> DiagnosticWrapper<'a> {
let mut db = cx.struct_span_lint(lint, span, msg);
if cx.current_level(lint) != Level::Allow {
cx.sess().fileline_help(span, &format!("{}\nfor further information \
db.fileline_help(span, &format!("{}\nfor further information \
visit https://github.com/Manishearth/rust-clippy/wiki#{}",
help, lint.name_lower()))
help, lint.name_lower()));
}
DiagnosticWrapper(db)
}
pub fn span_note_and_lint<T: LintContext>(cx: &T, lint: &'static Lint, span: Span,
msg: &str, note_span: Span, note: &str) {
cx.span_lint(lint, span, msg);
pub fn span_note_and_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint, span: Span,
msg: &str, note_span: Span, note: &str) -> DiagnosticWrapper<'a> {
let mut db = cx.struct_span_lint(lint, span, msg);
if cx.current_level(lint) != Level::Allow {
if note_span == span {
cx.sess().fileline_note(note_span, note)
db.fileline_note(note_span, note);
} else {
cx.sess().span_note(note_span, note)
db.span_note(note_span, note);
}
cx.sess().fileline_help(span, &format!("for further information visit \
db.fileline_help(span, &format!("for further information visit \
https://github.com/Manishearth/rust-clippy/wiki#{}",
lint.name_lower()))
lint.name_lower()));
}
DiagnosticWrapper(db)
}
pub fn span_lint_and_then<T: LintContext, F>(cx: &T, lint: &'static Lint, sp: Span,
msg: &str, f: F) where F: Fn() {
cx.span_lint(lint, sp, msg);
pub fn span_lint_and_then<'a, T: LintContext, F>(cx: &'a T, lint: &'static Lint, sp: Span,
msg: &str, f: F) -> DiagnosticWrapper<'a> where F: Fn(&mut DiagnosticWrapper) {
let mut db = DiagnosticWrapper(cx.struct_span_lint(lint, sp, msg));
if cx.current_level(lint) != Level::Allow {
f();
cx.sess().fileline_help(sp, &format!("for further information visit \
f(&mut db);
db.fileline_help(sp, &format!("for further information visit \
https://github.com/Manishearth/rust-clippy/wiki#{}",
lint.name_lower()))
lint.name_lower()));
}
db
}
/// return the base type for references and raw pointers

2
src/zero_div_zero.rs
View file

@ -9,7 +9,7 @@ use consts::{Constant, constant_simple, FloatWidth};
/// 0.0/0.0 with std::f32::NaN or std::f64::NaN, depending on the precision.
pub struct ZeroDivZeroPass;
/// **What it does:** This lint checks for `0.0 / 0.0`
/// **What it does:** This lint checks for `0.0 / 0.0`. It is `Warn` by default.
///
/// **Why is this bad?** It's less readable than `std::f32::NAN` or `std::f64::NAN`
///

18
tests/compile-fail/block_in_if_condition.rs
View file

@ -5,6 +5,15 @@
#![deny(block_in_if_condition_stmt)]
#![allow(unused)]
macro_rules! blocky {
() => {{true}}
}
fn macro_if() {
if blocky!() {
}
}
fn condition_has_block() -> i32 {
if { //~ERROR in an 'if' condition, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a 'let'
@ -60,5 +69,14 @@ fn closure_without_block() {
}
}
fn condition_is_unsafe_block() {
let a: i32 = 1;
// this should not warn because the condition is an unsafe block
if unsafe { 1u32 == std::mem::transmute(a) } {
println!("1u32 == a");
}
}
fn main() {
}

10
tests/compile-fail/box_vec.rs
View file

@ -3,6 +3,15 @@
#![plugin(clippy)]
#![deny(clippy)]
macro_rules! boxit {
($init:expr, $x:ty) => {
let _: Box<$x> = Box::new($init);
}
}
fn test_macro() {
boxit!(Vec::new(), Vec<u8>);
}
pub fn test(foo: Box<Vec<bool>>) { //~ ERROR you seem to be trying to use `Box<Vec<T>>`
println!("{:?}", foo.get(0))
}
@ -14,4 +23,5 @@ pub fn test2(foo: Box<Fn(Vec<u32>)>) { // pass if #31 is fixed
fn main(){
test(Box::new(Vec::new()));
test2(Box::new(|v| println!("{:?}", v)));
test_macro();
}

3
tests/compile-fail/cyclomatic_complexity.rs
View file

@ -4,7 +4,8 @@
#![deny(cyclomatic_complexity)]
#![allow(unused)]
fn main() { //~ ERROR: The function has a cyclomatic complexity of 28.
fn main() { //~ERROR The function has a cyclomatic complexity of 28
if true {
println!("a");
}

96
tests/compile-fail/methods.rs
View file

@ -50,7 +50,7 @@ fn option_methods() {
// Check OPTION_MAP_UNWRAP_OR
// single line case
let _ = opt.map(|x| x + 1) //~ ERROR called `map(f).unwrap_or(a)`
//~| NOTE replace this
//~| NOTE replace `map(|x| x + 1).unwrap_or(0)`
.unwrap_or(0); // should lint even though this call is on a separate line
// multi line cases
let _ = opt.map(|x| { //~ ERROR called `map(f).unwrap_or(a)`
@ -67,7 +67,7 @@ fn option_methods() {
// Check OPTION_MAP_UNWRAP_OR_ELSE
// single line case
let _ = opt.map(|x| x + 1) //~ ERROR called `map(f).unwrap_or_else(g)`
//~| NOTE replace this
//~| NOTE replace `map(|x| x + 1).unwrap_or_else(|| 0)`
.unwrap_or_else(|| 0); // should lint even though this call is on a separate line
// multi line cases
let _ = opt.map(|x| { //~ ERROR called `map(f).unwrap_or_else(g)`
@ -83,6 +83,98 @@ fn option_methods() {
}
/// Struct to generate false positive for Iterator-based lints
#[derive(Copy, Clone)]
struct IteratorFalsePositives {
foo: u32,
}
impl IteratorFalsePositives {
fn filter(self) -> IteratorFalsePositives {
self
}
fn next(self) -> IteratorFalsePositives {
self
}
fn find(self) -> Option<u32> {
Some(self.foo)
}
fn position(self) -> Option<u32> {
Some(self.foo)
}
fn rposition(self) -> Option<u32> {
Some(self.foo)
}
}
/// Checks implementation of FILTER_NEXT lint
fn filter_next() {
let v = vec![3, 2, 1, 0, -1, -2, -3];
// check single-line case
let _ = v.iter().filter(|&x| *x < 0).next();
//~^ ERROR called `filter(p).next()` on an Iterator.
//~| NOTE replace `filter(|&x| *x < 0).next()`
// check multi-line case
let _ = v.iter().filter(|&x| { //~ERROR called `filter(p).next()` on an Iterator.
*x < 0
}
).next();
// check that we don't lint if the caller is not an Iterator
let foo = IteratorFalsePositives { foo: 0 };
let _ = foo.filter().next();
}
/// Checks implementation of SEARCH_IS_SOME lint
fn search_is_some() {
let v = vec![3, 2, 1, 0, -1, -2, -3];
// check `find().is_some()`, single-line
let _ = v.iter().find(|&x| *x < 0).is_some();
//~^ ERROR called `is_some()` after searching
//~| NOTE replace `find(|&x| *x < 0).is_some()`
// check `find().is_some()`, multi-line
let _ = v.iter().find(|&x| { //~ERROR called `is_some()` after searching
*x < 0
}
).is_some();
// check `position().is_some()`, single-line
let _ = v.iter().position(|&x| x < 0).is_some();
//~^ ERROR called `is_some()` after searching
//~| NOTE replace `position(|&x| x < 0).is_some()`
// check `position().is_some()`, multi-line
let _ = v.iter().position(|&x| { //~ERROR called `is_some()` after searching
x < 0
}
).is_some();
// check `rposition().is_some()`, single-line
let _ = v.iter().rposition(|&x| x < 0).is_some();
//~^ ERROR called `is_some()` after searching
//~| NOTE replace `rposition(|&x| x < 0).is_some()`
// check `rposition().is_some()`, multi-line
let _ = v.iter().rposition(|&x| { //~ERROR called `is_some()` after searching
x < 0
}
).is_some();
// check that we don't lint if the caller is not an Iterator
let foo = IteratorFalsePositives { foo: 0 };
let _ = foo.find().is_some();
let _ = foo.position().is_some();
let _ = foo.rposition().is_some();
}
fn main() {
use std::io;

22
tests/compile-fail/panic.rs Normal file
View file

@ -0,0 +1,22 @@
#![feature(plugin)]
#![plugin(clippy)]
#[deny(panic_params)]
fn missing() {
panic!("{}"); //~ERROR: You probably are missing some parameter
}
fn ok_sigle() {
panic!("foo bar");
}
fn ok_multiple() {
panic!("{}", "This is {ok}");
}
fn main() {
missing();
ok_sigle();
ok_multiple();
}