- Bump `package.version` in `./Cargo.toml` (no need to manually bump `dependencies.clippy_lints.version`).
- Write a changelog entry.
-- If a nightly update is needed, update `min_version.txt` using `rustc -vV > min_version.txt`
- Run `./pre_publish.sh`
- Review and commit all changed files
- `git push`
-//! This build script ensures that Clippy is not compiled with an
-//! incompatible version of rust. It will panic with a descriptive
-//! error message instead.
-//!
-//! We specifially want to ensure that Clippy is only built with a
-//! rustc version that is newer or equal to the one specified in the
-//! `min_version.txt` file.
-//!
-//! `min_version.txt` is in the repo but also in the `.gitignore` to
-//! make sure that it is not updated manually by accident. Only CI
-//! should update that file.
-//!
-//! This build script was originally taken from the Rocket web framework:
-//! https://github.com/SergioBenitez/Rocket
-
use std::env;
fn main() {
function check() {
# run clippy on a project, try to be verbose and trigger as many warnings as possible for greater coverage
- RUST_BACKTRACE=full cargo clippy --all-targets --all-features -- --cap-lints warn -W clippy_pedantic -W clippy_nursery &> clippy_output
+ RUST_BACKTRACE=full cargo clippy --all-targets --all-features -- --cap-lints warn -W clippy::pedantic -W clippy::nursery &> clippy_output
cat clippy_output
! cat clippy_output | grep -q "internal compiler error\|query stack during panic"
if [[ $? != 0 ]]; then
then {
match qpath {
QPath::Resolved(..) => {
+ if_chain! {
+ // Detect and ignore <Foo as Default>::default() because these calls do
+ // explicitly name the type.
+ if let ExprKind::Call(ref method, ref _args) = expr.node;
+ if let ExprKind::Path(ref p) = method.node;
+ if let QPath::Resolved(Some(_ty), _path) = p;
+ then {
+ return;
+ }
+ }
+
// TODO: Work out a way to put "whatever the imported way of referencing
// this type in this file" rather than a fully-qualified type.
let expr_ty = cx.tables.expr_ty(expr);
if_not_else::IF_NOT_ELSE,
infinite_iter::MAYBE_INFINITE_ITER,
items_after_statements::ITEMS_AFTER_STATEMENTS,
+ loops::EXPLICIT_INTO_ITER_LOOP,
+ loops::EXPLICIT_ITER_LOOP,
matches::SINGLE_MATCH_ELSE,
methods::FILTER_MAP,
methods::OPTION_MAP_UNWRAP_OR,
literal_representation::UNREADABLE_LITERAL,
loops::EMPTY_LOOP,
loops::EXPLICIT_COUNTER_LOOP,
- loops::EXPLICIT_INTO_ITER_LOOP,
- loops::EXPLICIT_ITER_LOOP,
loops::FOR_KV_MAP,
loops::FOR_LOOP_OVER_OPTION,
loops::FOR_LOOP_OVER_RESULT,
loops::ITER_NEXT_LOOP,
loops::MANUAL_MEMCPY,
loops::MUT_RANGE_BOUND,
+ loops::NEEDLESS_COLLECT,
loops::NEEDLESS_RANGE_LOOP,
loops::NEVER_LOOP,
loops::REVERSE_RANGE_LOOP,
literal_representation::LARGE_DIGIT_GROUPS,
literal_representation::UNREADABLE_LITERAL,
loops::EMPTY_LOOP,
- loops::EXPLICIT_INTO_ITER_LOOP,
- loops::EXPLICIT_ITER_LOOP,
loops::FOR_KV_MAP,
loops::NEEDLESS_RANGE_LOOP,
loops::WHILE_LET_ON_ITERATOR,
escape::BOXED_LOCAL,
large_enum_variant::LARGE_ENUM_VARIANT,
loops::MANUAL_MEMCPY,
+ loops::NEEDLESS_COLLECT,
loops::UNUSED_COLLECT,
methods::EXPECT_FUN_CALL,
methods::ITER_NTH,
use rustc::middle::mem_categorization::cmt_;
use rustc::ty::{self, Ty};
use rustc::ty::subst::Subst;
+use rustc_errors::Applicability;
use std::collections::{HashMap, HashSet};
use std::iter::{once, Iterator};
use syntax::ast;
use syntax::source_map::Span;
+use syntax_pos::BytePos;
use crate::utils::{sugg, sext};
use crate::utils::usage::mutated_variables;
use crate::consts::{constant, Constant};
/// ```
declare_clippy_lint! {
pub EXPLICIT_ITER_LOOP,
- style,
+ pedantic,
"for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do"
}
/// ```
declare_clippy_lint! {
pub EXPLICIT_INTO_ITER_LOOP,
- style,
+ pedantic,
"for-looping over `_.into_iter()` when `_` would do"
}
written as a for loop"
}
+/// **What it does:** Checks for functions collecting an iterator when collect
+/// is not needed.
+///
+/// **Why is this bad?** `collect` causes the allocation of a new data structure,
+/// when this allocation may not be needed.
+///
+/// **Known problems:**
+/// None
+///
+/// **Example:**
+/// ```rust
+/// let len = iterator.collect::<Vec<_>>().len();
+/// // should be
+/// let len = iterator.count();
+/// ```
+declare_clippy_lint! {
+ pub NEEDLESS_COLLECT,
+ perf,
+ "collecting an iterator when collect is not needed"
+}
+
/// **What it does:** 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(_)`.
FOR_LOOP_OVER_OPTION,
WHILE_LET_LOOP,
UNUSED_COLLECT,
+ NEEDLESS_COLLECT,
REVERSE_RANGE_LOOP,
EXPLICIT_COUNTER_LOOP,
EMPTY_LOOP,
if let ExprKind::While(ref cond, _, _) = expr.node {
check_infinite_loop(cx, cond, expr);
}
+
+ check_needless_collect(expr, cx);
}
fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt) {
cx,
EXPLICIT_ITER_LOOP,
arg.span,
- "it is more idiomatic to loop over references to containers instead of using explicit \
+ "it is more concise to loop over references to containers instead of using explicit \
iteration methods",
"to write this more concisely, try",
format!("&{}{}", muta, object),
cx,
EXPLICIT_INTO_ITER_LOOP,
arg.span,
- "it is more idiomatic to loop over containers instead of using explicit \
+ "it is more concise to loop over containers instead of using explicit \
iteration methods`",
"to write this more concisely, try",
object.to_string(),
NestedVisitorMap::None
}
}
+
+const NEEDLESS_COLLECT_MSG: &str = "avoid using `collect()` when not needed";
+
+fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr, cx: &LateContext<'a, 'tcx>) {
+ if_chain! {
+ if let ExprKind::MethodCall(ref method, _, ref args) = expr.node;
+ if let ExprKind::MethodCall(ref chain_method, _, _) = args[0].node;
+ if chain_method.ident.name == "collect" && match_trait_method(cx, &args[0], &paths::ITERATOR);
+ if let Some(ref generic_args) = chain_method.args;
+ if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0);
+ then {
+ let ty = cx.tables.node_id_to_type(ty.hir_id);
+ if match_type(cx, ty, &paths::VEC) ||
+ match_type(cx, ty, &paths::VEC_DEQUE) ||
+ match_type(cx, ty, &paths::BTREEMAP) ||
+ match_type(cx, ty, &paths::HASHMAP) {
+ if method.ident.name == "len" {
+ let span = shorten_needless_collect_span(expr);
+ span_lint_and_then(cx, NEEDLESS_COLLECT, span, NEEDLESS_COLLECT_MSG, |db| {
+ db.span_suggestion_with_applicability(
+ span,
+ "replace with",
+ ".count()".to_string(),
+ Applicability::MachineApplicable,
+ );
+ });
+ }
+ if method.ident.name == "is_empty" {
+ let span = shorten_needless_collect_span(expr);
+ span_lint_and_then(cx, NEEDLESS_COLLECT, span, NEEDLESS_COLLECT_MSG, |db| {
+ db.span_suggestion_with_applicability(
+ span,
+ "replace with",
+ ".next().is_none()".to_string(),
+ Applicability::MachineApplicable,
+ );
+ });
+ }
+ if method.ident.name == "contains" {
+ let contains_arg = snippet(cx, args[1].span, "??");
+ let span = shorten_needless_collect_span(expr);
+ span_lint_and_then(cx, NEEDLESS_COLLECT, span, NEEDLESS_COLLECT_MSG, |db| {
+ db.span_suggestion_with_applicability(
+ span,
+ "replace with",
+ format!(
+ ".any(|&x| x == {})",
+ if contains_arg.starts_with('&') { &contains_arg[1..] } else { &contains_arg }
+ ),
+ Applicability::MachineApplicable,
+ );
+ });
+ }
+ }
+ }
+ }
+}
+
+fn shorten_needless_collect_span(expr: &Expr) -> Span {
+ if_chain! {
+ if let ExprKind::MethodCall(_, _, ref args) = expr.node;
+ if let ExprKind::MethodCall(_, ref span, _) = args[0].node;
+ then {
+ return expr.span.with_lo(span.lo() - BytePos(1));
+ }
+ }
+ unreachable!()
+}
use rustc::hir::*;
use syntax::ast::RangeLimits;
use syntax::source_map::Spanned;
-use crate::utils::{is_integer_literal, paths, snippet, span_lint, span_lint_and_then};
+use crate::utils::{is_integer_literal, paths, snippet, span_lint, span_lint_and_then, snippet_opt};
use crate::utils::{get_trait_def_id, higher, implements_trait, SpanlessEq};
use crate::utils::sugg::Sugg;
/// **Why is this bad?** The code is more readable with an inclusive range
/// like `x..=y`.
///
-/// **Known problems:** None.
+/// **Known problems:** Will add unnecessary pair of parentheses when the
+/// expression is not wrapped in a pair but starts with a opening parenthesis
+/// and ends with a closing one.
+/// I.e: `let _ = (f()+1)..(f()+1)` results in `let _ = ((f()+1)..=f())`.
///
/// **Example:**
/// ```rust
|db| {
let start = start.map_or("".to_owned(), |x| Sugg::hir(cx, x, "x").to_string());
let end = Sugg::hir(cx, y, "y");
- db.span_suggestion(expr.span,
+ if let Some(is_wrapped) = &snippet_opt(cx, expr.span) {
+ if is_wrapped.starts_with('(') && is_wrapped.ends_with(')') {
+ db.span_suggestion(expr.span,
+ "use",
+ format!("({}..={})", start, end));
+ } else {
+ db.span_suggestion(expr.span,
"use",
format!("{}..={}", start, end));
+ }
+ }
},
);
}
#![deny(clippy::missing_docs_in_private_items)]
use lazy_static::lazy_static;
+use std::default::Default;
use std::{env, fmt, fs, io, path};
use std::io::Read;
use syntax::{ast, source_map};
mod helpers {
use serde_derive::Deserialize;
/// Type used to store lint configuration.
- #[derive(Default, Deserialize)]
+ #[derive(Deserialize)]
#[serde(rename_all="kebab-case", deny_unknown_fields)]
pub struct Conf {
$(#[$doc] #[serde(default=$rust_name_str)] #[serde(with=$rust_name_str)]
(trivial_copy_size_limit, "trivial_copy_size_limit", None => Option<u64>),
}
+impl Default for Conf {
+ fn default() -> Conf {
+ toml::from_str("").expect("we never error on empty config files")
+ }
+}
+
/// Search for the configuration file.
pub fn lookup_conf_file() -> io::Result<Option<path::PathBuf>> {
/// Possible filename to search for.
///
/// Used internally for convenience
fn default(errors: Vec<Error>) -> (Conf, Vec<Error>) {
- (toml::from_str("").expect("we never error on empty config files"), errors)
+ (Conf::default(), errors)
}
/// Read the `toml` configuration file.
self.0.help(&format!(
"for further information visit https://rust-lang-nursery.github.io/rust-clippy/v{}/index.html#{}",
env!("CARGO_PKG_VERSION"),
- lint.name_lower()
+ lint.name_lower().replacen("clippy::", "", 1)
));
}
}
let result = without_block_comments(vec!["foo", "bar", "baz"]);
assert_eq!(result, vec!["foo", "bar", "baz"]);
}
-}
\ No newline at end of file
+}
let s18 = TupleStructDerivedDefault::default();
+ let s19 = <DerivedDefault as Default>::default();
+
println!(
- "[{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}]",
+ "[{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}], [{:?}]",
s1,
s2,
s3,
s16,
s17,
s18,
+ s19,
);
}
193 | for i in (5 + 2)..(8 - 1) {
| ^^^^^^^^^^^^^^^^
-error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
+error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:215:15
|
215 | for _v in vec.iter() {}
|
= note: `-D clippy::explicit-iter-loop` implied by `-D warnings`
-error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
+error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:217:15
|
217 | for _v in vec.iter_mut() {}
| ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&mut vec`
-error: it is more idiomatic to loop over containers instead of using explicit iteration methods`
+error: it is more concise to loop over containers instead of using explicit iteration methods`
--> $DIR/for_loop.rs:220:15
|
220 | for _v in out_vec.into_iter() {}
|
= note: `-D clippy::explicit-into-iter-loop` implied by `-D warnings`
-error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
+error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:223:15
|
223 | for _v in array.into_iter() {}
| ^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&array`
-error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
+error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:228:15
|
228 | for _v in [1, 2, 3].iter() {}
| ^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[1, 2, 3]`
-error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
+error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:232:15
|
232 | for _v in [0; 32].iter() {}
| ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[0; 32]`
-error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
+error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:237:15
|
237 | for _v in ll.iter() {}
| ^^^^^^^^^ help: to write this more concisely, try: `&ll`
-error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
+error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:240:15
|
240 | for _v in vd.iter() {}
| ^^^^^^^^^ help: to write this more concisely, try: `&vd`
-error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
+error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:243:15
|
243 | for _v in bh.iter() {}
| ^^^^^^^^^ help: to write this more concisely, try: `&bh`
-error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
+error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:246:15
|
246 | for _v in hm.iter() {}
| ^^^^^^^^^ help: to write this more concisely, try: `&hm`
-error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
+error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:249:15
|
249 | for _v in bt.iter() {}
| ^^^^^^^^^ help: to write this more concisely, try: `&bt`
-error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
+error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:252:15
|
252 | for _v in hs.iter() {}
| ^^^^^^^^^ help: to write this more concisely, try: `&hs`
-error: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
+error: it is more concise to loop over references to containers instead of using explicit iteration methods
--> $DIR/for_loop.rs:255:15
|
255 | for _v in bs.iter() {}
--- /dev/null
+#![feature(tool_lints)]
+
+use std::collections::{HashMap, HashSet, BTreeSet};
+
+#[warn(clippy::needless_collect)]
+#[allow(unused_variables, clippy::iter_cloned_collect)]
+fn main() {
+ let sample = [1; 5];
+ let len = sample.iter().collect::<Vec<_>>().len();
+ if sample.iter().collect::<Vec<_>>().is_empty() {
+ // Empty
+ }
+ sample.iter().cloned().collect::<Vec<_>>().contains(&1);
+ sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().len();
+ // Notice the `HashSet`--this should not be linted
+ sample.iter().collect::<HashSet<_>>().len();
+ // Neither should this
+ sample.iter().collect::<BTreeSet<_>>().len();
+}
--- /dev/null
+error: avoid using `collect()` when not needed
+ --> $DIR/needless_collect.rs:9:28
+ |
+9 | let len = sample.iter().collect::<Vec<_>>().len();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.count()`
+ |
+ = note: `-D clippy::needless-collect` implied by `-D warnings`
+
+error: avoid using `collect()` when not needed
+ --> $DIR/needless_collect.rs:10:21
+ |
+10 | if sample.iter().collect::<Vec<_>>().is_empty() {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.next().is_none()`
+
+error: avoid using `collect()` when not needed
+ --> $DIR/needless_collect.rs:13:27
+ |
+13 | sample.iter().cloned().collect::<Vec<_>>().contains(&1);
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.any(|&x| x == 1)`
+
+error: avoid using `collect()` when not needed
+ --> $DIR/needless_collect.rs:14:34
+ |
+14 | sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().len();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.count()`
+
+error: aborting due to 4 previous errors
+
fn test_cow(c: Cow<[i32]>) {
let _c = c;
}
+
+trait Foo2 {
+ fn do_string(&self);
+}
+
+// no error for &self references where self is of type String (#2293)
+impl Foo2 for String { fn do_string(&self) {} }
let _ = ..11-1;
let _ = ..=11-1;
let _ = ..=(11-1);
+ let _ = (1..11+1);
let _ = (f()+1)..(f()+1);
let mut vec: Vec<()> = std::vec::Vec::new();
error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:30:13
|
-30 | let _ = (f()+1)..(f()+1);
- | ^^^^^^^^^^^^^^^^ help: use: `(f()+1)..=f()`
+30 | let _ = (1..11+1);
+ | ^^^^^^^^^ help: use: `(1..=11)`
-error: aborting due to 7 previous errors
+error: an inclusive range would be more readable
+ --> $DIR/range_plus_minus_one.rs:31:13
+ |
+31 | let _ = (f()+1)..(f()+1);
+ | ^^^^^^^^^^^^^^^^ help: use: `((f()+1)..=f())`
+
+error: aborting due to 8 previous errors
projects / rust.git / commitdiff