Merge remote-tracking branch 'upstream/master'

This commit is contained in:
Maxwell Anderson 2018-10-18 15:45:05 -06:00
commit 9f637288cf
63 changed files with 1441 additions and 311 deletions

View File

@ -5,6 +5,7 @@ rust: nightly
os:
- linux
- osx
- windows
sudo: false
@ -24,10 +25,15 @@ before_install:
install:
- |
if [ -z ${INTEGRATION} ]; then
. $HOME/.nvm/nvm.sh
nvm install stable
nvm use stable
npm install remark-cli remark-lint
if [ "$TRAVIS_OS_NAME" == "linux" ]; then
. $HOME/.nvm/nvm.sh
nvm install stable
nvm use stable
npm install remark-cli remark-lint
fi
if [ "$TRAVIS_OS_NAME" == "windows" ]; then
choco install windows-sdk-10.0
fi
fi
matrix:
@ -36,6 +42,8 @@ matrix:
env: BASE_TESTS=true
- os: linux
env: BASE_TESTS=true
- os: windows
env: BASE_TEST=true
- env: INTEGRATION=rust-lang/cargo
- env: INTEGRATION=rust-lang-nursery/rand
- env: INTEGRATION=rust-lang-nursery/stdsimd
@ -49,10 +57,14 @@ matrix:
- env: INTEGRATION=serde-rs/serde
- env: INTEGRATION=Geal/nom
- env: INTEGRATION=hyperium/hyper
allow_failures:
- os: windows
env: BASE_TEST=true
# prevent these jobs with default env vars
exclude:
- os: linux
- os: osx
- os: windows
script:
- |

View File

@ -816,6 +816,7 @@ All notable changes to this project will be documented in this file.
[`redundant_closure_call`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#redundant_closure_call
[`redundant_field_names`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#redundant_field_names
[`redundant_pattern`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#redundant_pattern
[`redundant_pattern_matching`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#redundant_pattern_matching
[`ref_in_deref`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#ref_in_deref
[`regex_macro`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#regex_macro
[`replace_consts`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#replace_consts
@ -878,6 +879,7 @@ All notable changes to this project will be documented in this file.
[`unused_collect`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unused_collect
[`unused_io_amount`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unused_io_amount
[`unused_label`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unused_label
[`unused_unit`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unused_unit
[`use_debug`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#use_debug
[`use_self`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#use_self
[`used_underscore_binding`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#used_underscore_binding

View File

@ -9,7 +9,7 @@ We are currently in the process of discussing Clippy 1.0 via the RFC process in
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
[There are 279 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
[There are 280 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
@ -24,6 +24,15 @@ We have a bunch of lint categories to allow you to choose how much Clippy is sup
More to come, please [file an issue](https://github.com/rust-lang-nursery/rust-clippy/issues) if you have ideas!
Only the following of those categories are enabled by default:
* `clippy::style`
* `clippy::correctness`
* `clippy::complexity`
* `clippy::perf`
Other categories need to be enabled in order for their lints to be executed.
Table of contents:
* [Usage instructions](#usage)

View File

@ -14,7 +14,9 @@ set -ex
echo "Running clippy base tests"
PATH=$PATH:./node_modules/.bin
remark -f *.md > /dev/null
if [ "$TRAVIS_OS_NAME" == "linux" ]; then
remark -f *.md > /dev/null
fi
# build clippy in debug mode and run tests
cargo build --features debugging
cargo test --features debugging

View File

@ -9,3 +9,4 @@ clap = "~2.32"
itertools = "0.7"
regex = "1"
lazy_static = "1.0"
walkdir = "2"

View File

@ -14,6 +14,7 @@
use itertools::Itertools;
use lazy_static::lazy_static;
use regex::Regex;
use walkdir::WalkDir;
use std::collections::HashMap;
use std::ffi::OsStr;
use std::fs;
@ -35,6 +36,7 @@ lazy_static! {
pub static ref DOCS_LINK: String = "https://rust-lang-nursery.github.io/rust-clippy/master/index.html".to_string();
}
/// Lint data parsed from the Clippy source code.
#[derive(Clone, PartialEq, Debug)]
pub struct Lint {
pub name: String,
@ -55,22 +57,39 @@ impl Lint {
}
}
/// Returns all non-deprecated lints
pub fn active_lints(lints: impl Iterator<Item=Self>) -> impl Iterator<Item=Self> {
lints.filter(|l| l.deprecation.is_none())
/// Returns all non-deprecated lints and non-internal lints
pub fn usable_lints(lints: impl Iterator<Item=Self>) -> impl Iterator<Item=Self> {
lints.filter(|l| l.deprecation.is_none() && !l.is_internal())
}
/// Returns the lints in a HashMap, grouped by the different lint groups
pub fn by_lint_group(lints: &[Self]) -> HashMap<String, Vec<Self>> {
lints.iter().map(|lint| (lint.group.to_string(), lint.clone())).into_group_map()
}
pub fn is_internal(&self) -> bool {
self.group.starts_with("internal")
}
}
pub fn gen_changelog_lint_list(lints: Vec<Lint>) -> Vec<String> {
let mut lint_list_sorted: Vec<Lint> = lints;
lint_list_sorted.sort_by_key(|l| l.name.clone());
lint_list_sorted
.iter()
.filter(|l| !l.is_internal())
.map(|l| {
format!("[`{}`]: {}#{}", l.name, DOCS_LINK.clone(), l.name)
})
.collect()
}
/// Gathers all files in `src/clippy_lints` and gathers all lints inside
pub fn gather_all() -> impl Iterator<Item=Lint> {
lint_files().flat_map(|f| gather_from_file(&f))
}
fn gather_from_file(dir_entry: &fs::DirEntry) -> impl Iterator<Item=Lint> {
fn gather_from_file(dir_entry: &walkdir::DirEntry) -> impl Iterator<Item=Lint> {
let mut file = fs::File::open(dir_entry.path()).unwrap();
let mut content = String::new();
file.read_to_string(&mut content).unwrap();
@ -89,13 +108,98 @@ fn parse_contents(content: &str, filename: &str) -> impl Iterator<Item=Lint> {
}
/// Collects all .rs files in the `clippy_lints/src` directory
fn lint_files() -> impl Iterator<Item=fs::DirEntry> {
fs::read_dir("../clippy_lints/src")
.unwrap()
fn lint_files() -> impl Iterator<Item=walkdir::DirEntry> {
// We use `WalkDir` instead of `fs::read_dir` here in order to recurse into subdirectories.
// Otherwise we would not collect all the lints, for example in `clippy_lints/src/methods/`.
WalkDir::new("../clippy_lints/src")
.into_iter()
.filter_map(|f| f.ok())
.filter(|f| f.path().extension() == Some(OsStr::new("rs")))
}
/// Replace a region in a file delimited by two lines matching regexes.
///
/// `path` is the relative path to the file on which you want to perform the replacement.
///
/// See `replace_region_in_text` for documentation of the other options.
#[allow(clippy::expect_fun_call)]
pub fn replace_region_in_file<F>(path: &str, start: &str, end: &str, replace_start: bool, replacements: F) where F: Fn() -> Vec<String> {
let mut f = fs::File::open(path).expect(&format!("File not found: {}", path));
let mut contents = String::new();
f.read_to_string(&mut contents).expect("Something went wrong reading the file");
let replaced = replace_region_in_text(&contents, start, end, replace_start, replacements);
let mut f = fs::File::create(path).expect(&format!("File not found: {}", path));
f.write_all(replaced.as_bytes()).expect("Unable to write file");
// Ensure we write the changes with a trailing newline so that
// the file has the proper line endings.
f.write_all(b"\n").expect("Unable to write file");
}
/// Replace a region in a text delimited by two lines matching regexes.
///
/// * `text` is the input text on which you want to perform the replacement
/// * `start` is a `&str` that describes the delimiter line before the region you want to replace.
/// As the `&str` will be converted to a `Regex`, this can contain regex syntax, too.
/// * `end` is a `&str` that describes the delimiter line until where the replacement should
/// happen. As the `&str` will be converted to a `Regex`, this can contain regex syntax, too.
/// * If `replace_start` is true, the `start` delimiter line is replaced as well.
/// The `end` delimiter line is never replaced.
/// * `replacements` is a closure that has to return a `Vec<String>` which contains the new text.
///
/// If you want to perform the replacement on files instead of already parsed text,
/// use `replace_region_in_file`.
///
/// # Example
///
/// ```
/// let the_text = "replace_start\nsome text\nthat will be replaced\nreplace_end";
/// let result = clippy_dev::replace_region_in_text(
/// the_text,
/// r#"replace_start"#,
/// r#"replace_end"#,
/// false,
/// || {
/// vec!["a different".to_string(), "text".to_string()]
/// }
/// );
/// assert_eq!("replace_start\na different\ntext\nreplace_end", result);
/// ```
pub fn replace_region_in_text<F>(text: &str, start: &str, end: &str, replace_start: bool, replacements: F) -> String where F: Fn() -> Vec<String> {
let lines = text.lines();
let mut in_old_region = false;
let mut found = false;
let mut new_lines = vec![];
let start = Regex::new(start).unwrap();
let end = Regex::new(end).unwrap();
for line in lines {
if in_old_region {
if end.is_match(&line) {
in_old_region = false;
new_lines.extend(replacements());
new_lines.push(line.to_string());
}
} else if start.is_match(&line) {
if !replace_start {
new_lines.push(line.to_string());
}
in_old_region = true;
found = true;
} else {
new_lines.push(line.to_string());
}
}
if !found {
// This happens if the provided regex in `clippy_dev/src/main.rs` is not found in the
// given text or file. Most likely this is an error on the programmer's side and the Regex
// is incorrect.
println!("regex {:?} not found. You may have to update it.", start);
}
new_lines.join("\n")
}
#[test]
fn test_parse_contents() {
let result: Vec<Lint> = parse_contents(
@ -136,15 +240,54 @@ declare_deprecated_lint! {
}
#[test]
fn test_active_lints() {
fn test_replace_region() {
let text = r#"
abc
123
789
def
ghi"#;
let expected = r#"
abc
hello world
def
ghi"#;
let result = replace_region_in_text(text, r#"^\s*abc$"#, r#"^\s*def"#, false, || {
vec!["hello world".to_string()]
});
assert_eq!(expected, result);
}
#[test]
fn test_replace_region_with_start() {
let text = r#"
abc
123
789
def
ghi"#;
let expected = r#"
hello world
def
ghi"#;
let result = replace_region_in_text(text, r#"^\s*abc$"#, r#"^\s*def"#, true, || {
vec!["hello world".to_string()]
});
assert_eq!(expected, result);
}
#[test]
fn test_usable_lints() {
let lints = vec![
Lint::new("should_assert_eq", "Deprecated", "abc", Some("Reason"), "module_name"),
Lint::new("should_assert_eq2", "Not Deprecated", "abc", None, "module_name")
Lint::new("should_assert_eq2", "Not Deprecated", "abc", None, "module_name"),
Lint::new("should_assert_eq2", "internal", "abc", None, "module_name"),
Lint::new("should_assert_eq2", "internal_style", "abc", None, "module_name")
];
let expected = vec![
Lint::new("should_assert_eq2", "Not Deprecated", "abc", None, "module_name")
];
assert_eq!(expected, Lint::active_lints(lints.into_iter()).collect::<Vec<Lint>>());
assert_eq!(expected, Lint::usable_lints(lints.into_iter()).collect::<Vec<Lint>>());
}
#[test]
@ -164,3 +307,17 @@ fn test_by_lint_group() {
]);
assert_eq!(expected, Lint::by_lint_group(&lints));
}
#[test]
fn test_gen_changelog_lint_list() {
let lints = vec![
Lint::new("should_assert_eq", "group1", "abc", None, "module_name"),
Lint::new("should_assert_eq2", "group2", "abc", None, "module_name"),
Lint::new("incorrect_internal", "internal_style", "abc", None, "module_name"),
];
let expected = vec![
format!("[`should_assert_eq`]: {}#should_assert_eq", DOCS_LINK.to_string()),
format!("[`should_assert_eq2`]: {}#should_assert_eq2", DOCS_LINK.to_string())
];
assert_eq!(expected, gen_changelog_lint_list(lints));
}

View File

@ -32,24 +32,54 @@ fn main() {
if let Some(matches) = matches.subcommand_matches("update_lints") {
if matches.is_present("print-only") {
print_lints();
} else {
update_lints();
}
}
}
fn print_lints() {
let lint_list = gather_all().collect::<Vec<Lint>>();
let grouped_by_lint_group = Lint::by_lint_group(&lint_list);
let lint_list = gather_all();
let usable_lints: Vec<Lint> = Lint::usable_lints(lint_list).collect();
let lint_count = usable_lints.len();
let grouped_by_lint_group = Lint::by_lint_group(&usable_lints);
for (lint_group, mut lints) in grouped_by_lint_group {
if lint_group == "Deprecated" { continue; }
println!("\n## {}", lint_group);
lints.sort_by(|a, b| a.name.cmp(&b.name));
lints.sort_by_key(|l| l.name.clone());
for lint in lints {
println!("* [{}]({}#{}) ({})", lint.name, clippy_dev::DOCS_LINK.clone(), lint.name, lint.desc);
}
}
println!("there are {} lints", Lint::active_lints(lint_list.into_iter()).count());
println!("there are {} lints", lint_count);
}
fn update_lints() {
let lint_list: Vec<Lint> = gather_all().collect();
let usable_lints: Vec<Lint> = Lint::usable_lints(lint_list.clone().into_iter()).collect();
let lint_count = usable_lints.len();
replace_region_in_file(
"../README.md",
r#"\[There are \d+ lints included in this crate!\]\(https://rust-lang-nursery.github.io/rust-clippy/master/index.html\)"#,
"",
true,
|| {
vec![
format!("[There are {} lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)", lint_count)
]
}
);
replace_region_in_file(
"../CHANGELOG.md",
"<!-- begin autogenerated links to lint list -->",
"<!-- end autogenerated links to lint list -->",
false,
|| { gen_changelog_lint_list(lint_list.clone()) }
);
}

View File

@ -1,5 +1,5 @@
[package]
name = "clippy" # rename to clippy before publishing
name = "clippy_dummy" # rename to clippy before publishing
version = "0.0.302"
authors = ["Manish Goregaokar <manishsmail@gmail.com>"]
edition = "2018"

View File

@ -112,9 +112,17 @@ fn check_if(cx: &EarlyContext<'_>, expr: &ast::Expr) {
}
}
fn block_starts_with_comment(cx: &EarlyContext<'_>, expr: &ast::Block) -> bool {
// We trim all opening braces and whitespaces and then check if the next string is a comment.
let trimmed_block_text =
snippet_block(cx, expr.span, "..").trim_left_matches(|c: char| c.is_whitespace() || c == '{').to_owned();
trimmed_block_text.starts_with("//") || trimmed_block_text.starts_with("/*")
}
fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {
if_chain! {
if let ast::ExprKind::Block(ref block, _) = else_.node;
if !block_starts_with_comment(cx, block);
if let Some(else_) = expr_block(block);
if !in_macro(else_.span);
then {
@ -135,6 +143,7 @@ fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {
fn check_collapsible_no_if_let(cx: &EarlyContext<'_>, expr: &ast::Expr, check: &ast::Expr, then: &ast::Block) {
if_chain! {
if !block_starts_with_comment(cx, then);
if let Some(inner) = expr_block(then);
if let ast::ExprKind::If(ref check_inner, ref content, None) = inner.node;
then {

View File

@ -16,7 +16,7 @@ macro_rules! declare_deprecated_lint {
/// **What it does:** Nothing. This lint has been deprecated.
///
/// **Deprecation reason:** This used to check for `assert!(a == b)` and recommend
/// **Deprecation reason:** This used to check for `assert!(a == b)` and recommend
/// replacement with `assert_eq!(a, b)`, but this is no longer needed after RFC 2011.
declare_deprecated_lint! {
pub SHOULD_ASSERT_EQ,
@ -102,3 +102,13 @@ declare_deprecated_lint! {
pub ASSIGN_OPS,
"using compound assignment operators (e.g. `+=`) is harmless"
}
/// **What it does:** Nothing. This lint has been deprecated.
///
/// **Deprecation reason:** The original rule will only lint for `if let`. After
/// making it support to lint `match`, naming as `if let` is not suitable for it.
/// So, this lint is deprecated.
declare_deprecated_lint! {
pub IF_LET_REDUNDANT_PATTERN_MATCHING,
"this lint has been changed to redundant_pattern_matching"
}

View File

@ -40,15 +40,16 @@ declare_clippy_lint! {
"unnecessary double comparisons that can be simplified"
}
pub struct DoubleComparisonPass;
pub struct Pass;
impl LintPass for DoubleComparisonPass {
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(DOUBLE_COMPARISONS)
}
}
impl<'a, 'tcx> DoubleComparisonPass {
impl<'a, 'tcx> Pass {
#[allow(clippy::similar_names)]
fn check_binop(
&self,
cx: &LateContext<'a, 'tcx>,
@ -87,7 +88,7 @@ impl<'a, 'tcx> DoubleComparisonPass {
}
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DoubleComparisonPass {
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if let ExprKind::Binary(ref kind, ref lhs, ref rhs) = expr.node {
self.check_binop(cx, kind.node, lhs, rhs, expr.span);

View File

@ -63,16 +63,16 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnportableVariant {
let variant = &var.node;
if let Some(ref anon_const) = variant.disr_expr {
let param_env = ty::ParamEnv::empty();
let did = cx.tcx.hir.body_owner_def_id(anon_const.body);
let substs = Substs::identity_for_item(cx.tcx.global_tcx(), did);
let instance = ty::Instance::new(did, substs);
let cid = GlobalId {
let def_id = cx.tcx.hir.body_owner_def_id(anon_const.body);
let substs = Substs::identity_for_item(cx.tcx.global_tcx(), def_id);
let instance = ty::Instance::new(def_id, substs);
let c_id = GlobalId {
instance,
promoted: None
};
let constant = cx.tcx.const_eval(param_env.and(cid)).ok();
let constant = cx.tcx.const_eval(param_env.and(c_id)).ok();
if let Some(Constant::Int(val)) = constant.and_then(|c| miri_to_const(cx.tcx, c)) {
let mut ty = cx.tcx.type_of(did);
let mut ty = cx.tcx.type_of(def_id);
if let ty::Adt(adt, _) = ty.sty {
if adt.is_enum() {
ty = adt.repr.discr_type().to_ty(cx.tcx);

View File

@ -16,7 +16,7 @@ use crate::syntax::ast::*;
use crate::syntax::source_map::Span;
use crate::syntax::symbol::LocalInternedString;
use crate::utils::{span_help_and_lint, span_lint};
use crate::utils::{camel_case_from, camel_case_until, in_macro};
use crate::utils::{camel_case, in_macro};
/// **What it does:** Detects enumeration variants that are prefixed or suffixed
/// by the same characters.
@ -184,19 +184,19 @@ fn check_variant(
}
}
let first = var2str(&def.variants[0]);
let mut pre = &first[..camel_case_until(&*first)];
let mut post = &first[camel_case_from(&*first)..];
let mut pre = &first[..camel_case::until(&*first)];
let mut post = &first[camel_case::from(&*first)..];
for var in &def.variants {
let name = var2str(var);
let pre_match = partial_match(pre, &name);
pre = &pre[..pre_match];
let pre_camel = camel_case_until(pre);
let pre_camel = camel_case::until(pre);
pre = &pre[..pre_camel];
while let Some((next, last)) = name[pre.len()..].chars().zip(pre.chars().rev()).next() {
if next.is_lowercase() {
let last = pre.len() - last.len_utf8();
let last_camel = camel_case_until(&pre[..last]);
let last_camel = camel_case::until(&pre[..last]);
pre = &pre[..last_camel];
} else {
break;
@ -206,7 +206,7 @@ fn check_variant(
let post_match = partial_rmatch(post, &name);
let post_end = post.len() - post_match;
post = &post[post_end..];
let post_camel = camel_case_from(post);
let post_camel = camel_case::from(post);
post = &post[post_camel..];
}
let (what, value) = match (pre.is_empty(), post.is_empty()) {
@ -255,6 +255,7 @@ impl EarlyLintPass for EnumVariantNames {
assert!(last.is_some());
}
#[allow(clippy::similar_names)]
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
let item_name = item.ident.as_str();
let item_name_chars = item_name.chars().count();

View File

@ -63,6 +63,7 @@ impl LintPass for EqOp {
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EqOp {
#[allow(clippy::similar_names)]
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
if let ExprKind::Binary(op, ref left, ref right) = e.node {
if in_macro(e.span) {

View File

@ -65,6 +65,7 @@ impl LintPass for Pass {
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
fn check_fn(
&mut self,
cx: &LateContext<'a, 'tcx>,
@ -74,13 +75,23 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
_: Span,
node_id: NodeId,
) {
let fn_def_id = cx.tcx.hir.local_def_id(node_id);
// If the method is an impl for a trait, don't warn
let parent_id = cx.tcx.hir.get_parent(node_id);
let parent_node = cx.tcx.hir.find(parent_id);
if let Some(Node::Item(item)) = parent_node {
if let ItemKind::Impl(_, _, _, _, Some(..), _, _) = item.node {
return;
}
}
let mut v = EscapeDelegate {
cx,
set: NodeSet(),
too_large_for_stack: self.too_large_for_stack,
};
let fn_def_id = cx.tcx.hir.local_def_id(node_id);
let region_scope_tree = &cx.tcx.region_scope_tree(fn_def_id);
ExprUseVisitor::new(&mut v, cx.tcx, cx.param_env, region_scope_tree, cx.tables, None).consume_body(body);

View File

@ -108,6 +108,7 @@ impl ExcessivePrecision {
}
}
#[allow(clippy::doc_markdown)]
/// Should we exclude the float because it has a `.0` or `.` suffix
/// Ex 1_000_000_000.0
/// Ex 1_000_000_000.

View File

@ -1,102 +0,0 @@
// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use crate::rustc::{declare_tool_lint, lint_array};
use crate::rustc::hir::*;
use crate::utils::{match_qpath, paths, snippet, span_lint_and_then};
use crate::rustc_errors::Applicability;
/// **What it does:** Lint for redundant pattern matching over `Result` or
/// `Option`
///
/// **Why is this bad?** It's more concise and clear to just use the proper
/// utility function
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// if let Ok(_) = Ok::<i32, i32>(42) {}
/// if let Err(_) = Err::<i32, i32>(42) {}
/// if let None = None::<()> {}
/// if let Some(_) = Some(42) {}
/// ```
///
/// The more idiomatic use would be:
///
/// ```rust
/// if Ok::<i32, i32>(42).is_ok() {}
/// if Err::<i32, i32>(42).is_err() {}
/// if None::<()>.is_none() {}
/// if Some(42).is_some() {}
/// ```
///
declare_clippy_lint! {
pub IF_LET_REDUNDANT_PATTERN_MATCHING,
style,
"use the proper utility function avoiding an `if let`"
}
#[derive(Copy, Clone)]
pub struct Pass;
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(IF_LET_REDUNDANT_PATTERN_MATCHING)
}
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if let ExprKind::Match(ref op, ref arms, MatchSource::IfLetDesugar { .. }) = expr.node {
if arms[0].pats.len() == 1 {
let good_method = match arms[0].pats[0].node {
PatKind::TupleStruct(ref path, ref pats, _) if pats.len() == 1 => {
if let PatKind::Wild = pats[0].node {
if match_qpath(path, &paths::RESULT_OK) {
"is_ok()"
} else if match_qpath(path, &paths::RESULT_ERR) {
"is_err()"
} else if match_qpath(path, &paths::OPTION_SOME) {
"is_some()"
} else {
return;
}
} else {
return;
}
},
PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()",
_ => return,
};
span_lint_and_then(
cx,
IF_LET_REDUNDANT_PATTERN_MATCHING,
arms[0].pats[0].span,
&format!("redundant pattern matching, consider using `{}`", good_method),
|db| {
let span = expr.span.to(op.span);
db.span_suggestion_with_applicability(
span,
"try this",
format!("if {}.{}", snippet(cx, op.span, "_"), good_method),
Applicability::MachineApplicable, // snippet
);
},
);
}
}
}
}

View File

@ -240,7 +240,7 @@ fn check_len(cx: &LateContext<'_, '_>, span: Span, method_name: Name, args: &[Ex
LEN_ZERO,
span,
&format!("length comparison to {}", if compare_to == 0 { "zero" } else { "one" }),
"using `is_empty` is more concise",
"using `is_empty` is clearer and more explicit",
format!("{}{}.is_empty()", op, snippet(cx, args[0].span, "_")),
);
}
@ -276,7 +276,7 @@ fn has_is_empty(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
let ty = &walk_ptrs_ty(cx.tables.expr_ty(expr));
match ty.sty {
ty::Dynamic(ref tt, ..) => cx.tcx
.associated_items(tt.principal().expect("trait impl not found").def_id())
.associated_items(tt.principal().def_id())
.any(|item| is_is_empty(cx, &item)),
ty::Projection(ref proj) => has_is_empty_impl(cx, proj.item_def_id),
ty::Adt(id, _) => has_is_empty_impl(cx, id.did),

View File

@ -46,6 +46,8 @@ extern crate syntax_pos;
use toml;
// Currently, categories "style", "correctness", "complexity" and "perf" are enabled by default,
// as said in the README.md of this repository. If this changes, please update README.md.
macro_rules! declare_clippy_lint {
{ pub $name:tt, style, $description:tt } => {
declare_tool_lint! { pub clippy::$name, Warn, $description, report_in_external_macro: true }
@ -124,7 +126,6 @@ pub mod formatting;
pub mod functions;
pub mod identity_conversion;
pub mod identity_op;
pub mod if_let_redundant_pattern_matching;
pub mod if_not_else;
pub mod indexing_slicing;
pub mod infallible_destructuring_match;
@ -178,6 +179,7 @@ pub mod ptr_offset_with_cast;
pub mod question_mark;
pub mod ranges;
pub mod redundant_field_names;
pub mod redundant_pattern_matching;
pub mod reference;
pub mod regex;
pub mod replace_consts;
@ -301,6 +303,10 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
"assign_ops",
"using compound assignment operators (e.g. `+=`) is harmless",
);
store.register_removed(
"if_let_redundant_pattern_matching",
"this lint has been changed to redundant_pattern_matching",
);
// end deprecated lints, do not remove this comment, its used in `update_lints`
reg.register_late_lint_pass(box serde_api::Serde);
@ -400,7 +406,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
reg.register_late_lint_pass(box missing_doc::MissingDoc::new());
reg.register_late_lint_pass(box missing_inline::MissingInline);
reg.register_late_lint_pass(box ok_if_let::Pass);
reg.register_late_lint_pass(box if_let_redundant_pattern_matching::Pass);
reg.register_late_lint_pass(box redundant_pattern_matching::Pass);
reg.register_late_lint_pass(box partialeq_ne_impl::Pass);
reg.register_early_lint_pass(box reference::Pass);
reg.register_early_lint_pass(box reference::DerefPass);
@ -428,8 +434,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
reg.register_late_lint_pass(box fallible_impl_from::FallibleImplFrom);
reg.register_late_lint_pass(box replace_consts::ReplaceConsts);
reg.register_late_lint_pass(box types::UnitArg);
reg.register_late_lint_pass(box double_comparison::DoubleComparisonPass);
reg.register_late_lint_pass(box question_mark::QuestionMarkPass);
reg.register_late_lint_pass(box double_comparison::Pass);
reg.register_late_lint_pass(box question_mark::Pass);
reg.register_late_lint_pass(box suspicious_trait_impl::SuspiciousImpl);
reg.register_early_lint_pass(box multiple_crate_versions::Pass);
reg.register_late_lint_pass(box map_unit_fn::Pass);
@ -563,7 +569,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
functions::TOO_MANY_ARGUMENTS,
identity_conversion::IDENTITY_CONVERSION,
identity_op::IDENTITY_OP,
if_let_redundant_pattern_matching::IF_LET_REDUNDANT_PATTERN_MATCHING,
indexing_slicing::OUT_OF_BOUNDS_INDEXING,
infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH,
infinite_iter::INFINITE_ITER,
@ -678,6 +683,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
ranges::RANGE_PLUS_ONE,
ranges::RANGE_ZIP_WITH_LEN,
redundant_field_names::REDUNDANT_FIELD_NAMES,
redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING,
reference::DEREF_ADDROF,
reference::REF_IN_DEREF,
regex::INVALID_REGEX,
@ -685,6 +691,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
regex::TRIVIAL_REGEX,
returns::LET_AND_RETURN,
returns::NEEDLESS_RETURN,
returns::UNUSED_UNIT,
serde_api::SERDE_API_MISUSE,
strings::STRING_LIT_AS_BYTES,
suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL,
@ -746,7 +753,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
excessive_precision::EXCESSIVE_PRECISION,
formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
formatting::SUSPICIOUS_ELSE_FORMATTING,
if_let_redundant_pattern_matching::IF_LET_REDUNDANT_PATTERN_MATCHING,
infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH,
len_zero::LEN_WITHOUT_IS_EMPTY,
len_zero::LEN_ZERO,
@ -797,10 +803,12 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
ptr::PTR_ARG,
question_mark::QUESTION_MARK,
redundant_field_names::REDUNDANT_FIELD_NAMES,
redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING,
regex::REGEX_MACRO,
regex::TRIVIAL_REGEX,
returns::LET_AND_RETURN,
returns::NEEDLESS_RETURN,
returns::UNUSED_UNIT,
strings::STRING_LIT_AS_BYTES,
types::FN_TO_NUMERIC_CAST,
types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION,

View File

@ -27,10 +27,11 @@ use crate::rustc::ty::subst::Subst;
use crate::rustc_errors::Applicability;
use crate::rustc_data_structures::fx::{FxHashMap, FxHashSet};
use std::iter::{once, Iterator};
use std::mem;
use crate::syntax::ast;
use crate::syntax::source_map::Span;
use crate::syntax_pos::BytePos;
use crate::utils::{sugg, sext};
use crate::utils::{in_macro, sugg, sext};
use crate::utils::usage::mutated_variables;
use crate::consts::{constant, Constant};
@ -949,8 +950,20 @@ fn detect_manual_memcpy<'a, 'tcx>(
("0", _, x, false) | (x, false, "0", false) => x.into(),
("0", _, x, true) | (x, false, "0", true) => format!("-{}", x),
(x, false, y, false) => format!("({} + {})", x, y),
(x, false, y, true) => format!("({} - {})", x, y),
(x, true, y, false) => format!("({} - {})", y, x),
(x, false, y, true) => {
if x == y {
"0".into()
} else {
format!("({} - {})", x, y)
}
},
(x, true, y, false) => {
if x == y {
"0".into()
} else {
format!("({} - {})", y, x)
}
},
(x, true, y, true) => format!("-({} + {})", x, y),
}
};
@ -1029,6 +1042,10 @@ fn check_for_loop_range<'a, 'tcx>(
body: &'tcx Expr,
expr: &'tcx Expr,
) {
if in_macro(expr.span) {
return;
}
if let Some(higher::Range {
start: Some(start),
ref end,
@ -1082,16 +1099,35 @@ fn check_for_loop_range<'a, 'tcx>(
format!(".skip({})", snippet(cx, start.span, ".."))
};
let mut end_is_start_plus_val = false;
let take = if let Some(end) = *end {
let mut take_expr = end;
if let ExprKind::Binary(ref op, ref left, ref right) = end.node {
if let BinOpKind::Add = op.node {
let start_equal_left = SpanlessEq::new(cx).eq_expr(start, left);
let start_equal_right = SpanlessEq::new(cx).eq_expr(start, right);
if start_equal_left {
take_expr = right;
} else if start_equal_right {
take_expr = left;
}
end_is_start_plus_val = start_equal_left | start_equal_right;
}
}
if is_len_call(end, indexed) {
String::new()
} else {
match limits {
ast::RangeLimits::Closed => {
let end = sugg::Sugg::hir(cx, end, "<count>");
format!(".take({})", end + sugg::ONE)
let take_expr = sugg::Sugg::hir(cx, take_expr, "<count>");
format!(".take({})", take_expr + sugg::ONE)
},
ast::RangeLimits::HalfOpen => format!(".take({})", snippet(cx, end.span, "..")),
ast::RangeLimits::HalfOpen => format!(".take({})", snippet(cx, take_expr.span, "..")),
}
}
} else {
@ -1104,6 +1140,14 @@ fn check_for_loop_range<'a, 'tcx>(
("", "iter")
};
let take_is_empty = take.is_empty();
let mut method_1 = take;
let mut method_2 = skip;
if end_is_start_plus_val {
mem::swap(&mut method_1, &mut method_2);
}
if visitor.nonindex {
span_lint_and_then(
cx,
@ -1116,16 +1160,16 @@ fn check_for_loop_range<'a, 'tcx>(
"consider using an iterator".to_string(),
vec![
(pat.span, format!("({}, <item>)", ident.name)),
(arg.span, format!("{}.{}().enumerate(){}{}", indexed, method, take, skip)),
(arg.span, format!("{}.{}().enumerate(){}{}", indexed, method, method_1, method_2)),
],
);
},
);
} else {
let repl = if starts_at_zero && take.is_empty() {
let repl = if starts_at_zero && take_is_empty {
format!("&{}{}", ref_mut, indexed)
} else {
format!("{}.{}(){}{}", indexed, method, take, skip)
format!("{}.{}(){}{}", indexed, method, method_1, method_2)
};
span_lint_and_then(
@ -1952,10 +1996,7 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
_ => (),
}
}
} else if is_loop(expr) {
walk_expr(self, expr);
return;
} else if is_conditional(expr) {
} else if is_loop(expr) || is_conditional(expr) {
self.depth += 1;
walk_expr(self, expr);
self.depth -= 1;

View File

@ -179,7 +179,7 @@ fn unit_closure<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'a hir::Expr) -> Op
None
}
/// Builds a name for the let binding variable (var_arg)
/// Builds a name for the let binding variable (`var_arg`)
///
/// `x.field` => `x_field`
/// `y` => `_y`

View File

@ -23,7 +23,7 @@ use crate::utils::{match_def_path, opt_def_id, paths, span_lint};
///
/// **Example:**
/// ```rust
/// mem::forget(Rc::new(55)))
/// mem::forget(Rc::new(55))
/// ```
declare_clippy_lint! {
pub MEM_FORGET,

View File

@ -11,7 +11,7 @@
use crate::rustc::hir;
use crate::rustc::hir::def::Def;
use crate::rustc::lint::{in_external_macro, LateContext, LateLintPass, Lint, LintArray, LintContext, LintPass};
use crate::rustc::ty::{self, Ty};
use crate::rustc::ty::{self, Ty, TyKind, Predicate};
use crate::rustc::{declare_tool_lint, lint_array};
use crate::rustc_errors::Applicability;
use crate::syntax::ast;
@ -688,7 +688,8 @@ declare_clippy_lint! {
///
/// **Why is this bad?** Readability.
///
/// **Known problems:** None.
/// **Known problems:** False positive in pattern guards. Will be resolved once
/// non-lexical lifetimes are stable.
///
/// **Example:**
/// ```rust
@ -878,6 +879,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
let name = implitem.ident.name;
let parent = cx.tcx.hir.get_parent(implitem.id);
let item = cx.tcx.hir.expect_item(parent);
let def_id = cx.tcx.hir.local_def_id(item.id);
let ty = cx.tcx.type_of(def_id);
if_chain! {
if let hir::ImplItemKind::Method(ref sig, id) = implitem.node;
if let Some(first_arg_ty) = sig.decl.inputs.get(0);
@ -899,8 +902,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
}
// check conventions w.r.t. conversion method names and predicates
let def_id = cx.tcx.hir.local_def_id(item.id);
let ty = cx.tcx.type_of(def_id);
let is_copy = is_copy(cx, ty);
for &(ref conv, self_kinds) in &CONVENTIONS {
if conv.check(&name.as_str()) {
@ -928,16 +929,37 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
break;
}
}
}
}
let ret_ty = return_ty(cx, implitem.id);
if name == "new" &&
!ret_ty.walk().any(|t| same_tys(cx, t, ty)) {
span_lint(cx,
NEW_RET_NO_SELF,
implitem.span,
"methods called `new` usually return `Self`");
if let hir::ImplItemKind::Method(_, _) = implitem.node {
let ret_ty = return_ty(cx, implitem.id);
// if return type is impl trait
if let TyKind::Opaque(def_id, _) = ret_ty.sty {
// then one of the associated types must be Self
for predicate in cx.tcx.predicates_of(def_id).predicates.iter() {
match predicate {
(Predicate::Projection(poly_projection_predicate), _) => {
let binder = poly_projection_predicate.ty();
let associated_type = binder.skip_binder();
let associated_type_is_self_type = same_tys(cx, ty, associated_type);
// if the associated type is self, early return and do not trigger lint
if associated_type_is_self_type { return; }
},
(_, _) => {},
}
}
}
if name == "new" && !same_tys(cx, ret_ty, ty) {
span_lint(cx,
NEW_RET_NO_SELF,
implitem.span,
"methods called `new` usually return `Self`");
}
}
}
}

View File

@ -21,7 +21,7 @@ use crate::utils::{get_item_name, get_parent_expr, implements_trait, in_constant
iter_input_pats, last_path_segment, match_qpath, match_trait_method, paths, snippet, span_lint,
span_lint_and_then, walk_ptrs_ty, SpanlessEq};
use crate::utils::sugg::Sugg;
use crate::syntax::ast::{LitKind, CRATE_NODE_ID};
use crate::syntax::ast::LitKind;
use crate::consts::{constant, Constant};
use crate::rustc_errors::Applicability;
@ -535,34 +535,39 @@ fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr, other: &Expr) {
return;
}
let other_gets_derefed = match other.node {
ExprKind::Unary(UnDeref, _) => true,
_ => false,
};
let lint_span = if other_gets_derefed {
expr.span.to(other.span)
} else {
expr.span
};
span_lint_and_then(
cx,
CMP_OWNED,
expr.span,
lint_span,
"this creates an owned instance just for comparison",
|db| {
// this is as good as our recursion check can get, we can't prove that the
// current function is
// called by
// PartialEq::eq, but we can at least ensure that this code is not part of it
let parent_fn = cx.tcx.hir.get_parent(expr.id);
let parent_impl = cx.tcx.hir.get_parent(parent_fn);
if parent_impl != CRATE_NODE_ID {
if let Node::Item(item) = cx.tcx.hir.get(parent_impl) {
if let ItemKind::Impl(.., Some(ref trait_ref), _, _) = item.node {
if trait_ref.path.def.def_id() == partial_eq_trait_id {
// we are implementing PartialEq, don't suggest not doing `to_owned`, otherwise
// we go into
// recursion
db.span_label(expr.span, "try calling implementing the comparison without allocating");
return;
}
}
}
// this also catches PartialEq implementations that call to_owned
if other_gets_derefed {
db.span_label(lint_span, "try implementing the comparison without allocating");
return;
}
let try_hint = if deref_arg_impl_partial_eq_other { format!("*{}", snip) } else { snip.to_string() };
let try_hint = if deref_arg_impl_partial_eq_other {
// suggest deref on the left
format!("*{}", snip)
} else {
// suggest dropping the to_owned on the left
snip.to_string()
};
db.span_suggestion_with_applicability(
expr.span,
lint_span,
"try",
try_hint,
Applicability::MachineApplicable, // snippet

View File

@ -45,15 +45,15 @@ declare_clippy_lint!{
}
#[derive(Copy, Clone)]
pub struct QuestionMarkPass;
pub struct Pass;
impl LintPass for QuestionMarkPass {
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(QUESTION_MARK)
}
}
impl QuestionMarkPass {
impl Pass {
/// Check if the given expression on the given context matches the following structure:
///
/// ```ignore
@ -145,7 +145,7 @@ impl QuestionMarkPass {
}
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for QuestionMarkPass {
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
Self::check_is_none_and_early_return_none(cx, expr);
}

View File

@ -0,0 +1,228 @@
// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use crate::rustc::{declare_tool_lint, lint_array};
use crate::rustc::hir::*;
use crate::syntax::ptr::P;
use crate::syntax::ast::LitKind;
use crate::utils::{match_qpath, paths, snippet, span_lint_and_then};
use crate::rustc_errors::Applicability;
/// **What it does:** Lint for redundant pattern matching over `Result` or
/// `Option`
///
/// **Why is this bad?** It's more concise and clear to just use the proper
/// utility function
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// if let Ok(_) = Ok::<i32, i32>(42) {}
/// if let Err(_) = Err::<i32, i32>(42) {}
/// if let None = None::<()> {}
/// if let Some(_) = Some(42) {}
/// match Ok::<i32, i32>(42) {
/// Ok(_) => true,
/// Err(_) => false,
/// };
/// ```
///
/// The more idiomatic use would be:
///
/// ```rust
/// if Ok::<i32, i32>(42).is_ok() {}
/// if Err::<i32, i32>(42).is_err() {}
/// if None::<()>.is_none() {}
/// if Some(42).is_some() {}
/// Ok::<i32, i32>(42).is_ok();
/// ```
///
declare_clippy_lint! {
pub REDUNDANT_PATTERN_MATCHING,
style,
"use the proper utility function avoiding an `if let`"
}
#[derive(Copy, Clone)]
pub struct Pass;
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(REDUNDANT_PATTERN_MATCHING)
}
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
#[allow(clippy::similar_names)]
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if let ExprKind::Match(ref op, ref arms, ref match_source) = expr.node {
match match_source {
MatchSource::Normal => find_sugg_for_match(cx, expr, op, arms),
MatchSource::IfLetDesugar { .. } => find_sugg_for_if_let(cx, expr, op, arms),
_ => return,
}
}
}
}
fn find_sugg_for_if_let<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
expr: &'tcx Expr,
op: &P<Expr>,
arms: &HirVec<Arm>
) {
if arms[0].pats.len() == 1 {
let good_method = match arms[0].pats[0].node {
PatKind::TupleStruct(ref path, ref pats, _) if pats.len() == 1 => {
if let PatKind::Wild = pats[0].node {
if match_qpath(path, &paths::RESULT_OK) {
"is_ok()"
} else if match_qpath(path, &paths::RESULT_ERR) {
"is_err()"
} else if match_qpath(path, &paths::OPTION_SOME) {
"is_some()"
} else {
return;
}
} else {
return;
}
},
PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()",
_ => return,
};
span_lint_and_then(
cx,
REDUNDANT_PATTERN_MATCHING,
arms[0].pats[0].span,
&format!("redundant pattern matching, consider using `{}`", good_method),
|db| {
let span = expr.span.to(op.span);
db.span_suggestion_with_applicability(
span,
"try this",
format!("if {}.{}", snippet(cx, op.span, "_"), good_method),
Applicability::MachineApplicable, // snippet
);
},
);
} else {
return;
}
}
fn find_sugg_for_match<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
expr: &'tcx Expr,
op: &P<Expr>,
arms: &HirVec<Arm>
) {
if arms.len() == 2 {
let node_pair = (&arms[0].pats[0].node, &arms[1].pats[0].node);
let found_good_method = match node_pair {
(
PatKind::TupleStruct(ref path_left, ref pats_left, _),
PatKind::TupleStruct(ref path_right, ref pats_right, _)
) if pats_left.len() == 1 && pats_right.len() == 1 => {
if let (PatKind::Wild, PatKind::Wild) = (&pats_left[0].node, &pats_right[0].node) {
find_good_method_for_match(
arms,
path_left,
path_right,
&paths::RESULT_OK,
&paths::RESULT_ERR,
"is_ok()",
"is_err()"
)
} else {
None
}
},
(
PatKind::TupleStruct(ref path_left, ref pats, _),
PatKind::Path(ref path_right)
) | (
PatKind::Path(ref path_left),
PatKind::TupleStruct(ref path_right, ref pats, _)
) if pats.len() == 1 => {
if let PatKind::Wild = pats[0].node {
find_good_method_for_match(
arms,
path_left,
path_right,
&paths::OPTION_SOME,
&paths::OPTION_NONE,
"is_some()",
"is_none()"
)
} else {
None
}
},
_ => None,
};
if let Some(good_method) = found_good_method {
span_lint_and_then(
cx,
REDUNDANT_PATTERN_MATCHING,
expr.span,
&format!("redundant pattern matching, consider using `{}`", good_method),
|db| {
let span = expr.span.to(op.span);
db.span_suggestion_with_applicability(
span,
"try this",
format!("{}.{}", snippet(cx, op.span, "_"), good_method),
Applicability::MachineApplicable, // snippet
);
},
);
}
} else {
return;
}
}
fn find_good_method_for_match<'a>(
arms: &HirVec<Arm>,
path_left: &QPath,
path_right: &QPath,
expected_left: &[&str],
expected_right: &[&str],
should_be_left: &'a str,
should_be_right: &'a str
) -> Option<&'a str> {
let body_node_pair = if match_qpath(path_left, expected_left) && match_qpath(path_right, expected_right) {
(&(*arms[0].body).node, &(*arms[1].body).node)
} else if match_qpath(path_right, expected_left) && match_qpath(path_left, expected_right) {
(&(*arms[1].body).node, &(*arms[0].body).node)
} else {
return None;
};
match body_node_pair {
(ExprKind::Lit(ref lit_left), ExprKind::Lit(ref lit_right)) => {
match (&lit_left.node, &lit_right.node) {
(LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
(LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
_ => None,
}
},
_ => None,
}
}

View File

@ -14,8 +14,8 @@ use if_chain::if_chain;
use crate::syntax::ast;
use crate::syntax::source_map::Span;
use crate::syntax::visit::FnKind;
use crate::syntax_pos::BytePos;
use crate::rustc_errors::Applicability;
use crate::utils::{in_macro, match_path_ast, snippet_opt, span_lint_and_then, span_note_and_lint};
/// **What it does:** Checks for return statements at the end of a block.
@ -68,6 +68,25 @@ declare_clippy_lint! {
the end of a block"
}
/// **What it does:** Checks for unit (`()`) expressions that can be removed.
///
/// **Why is this bad?** Such expressions add no value, but can make the code
/// less readable. Depending on formatting they can make a `break` or `return`
/// statement look like a function call.
///
/// **Known problems:** The lint currently misses unit return types in types,
/// e.g. the `F` in `fn generic_unit<F: Fn() -> ()>(f: F) { .. }`.
///
/// **Example:**
/// ```rust
/// fn return_unit() -> () { () }
/// ```
declare_clippy_lint! {
pub UNUSED_UNIT,
style,
"needless unit expression"
}
#[derive(Copy, Clone)]
pub struct ReturnPass;
@ -162,23 +181,98 @@ impl ReturnPass {
impl LintPass for ReturnPass {
fn get_lints(&self) -> LintArray {
lint_array!(NEEDLESS_RETURN, LET_AND_RETURN)
lint_array!(NEEDLESS_RETURN, LET_AND_RETURN, UNUSED_UNIT)
}
}
impl EarlyLintPass for ReturnPass {
fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, _: &ast::FnDecl, _: Span, _: ast::NodeId) {
fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, decl: &ast::FnDecl, span: Span, _: ast::NodeId) {
match kind {
FnKind::ItemFn(.., block) | FnKind::Method(.., block) => self.check_block_return(cx, block),
FnKind::Closure(body) => self.check_final_expr(cx, body, Some(body.span)),
}
if_chain! {
if let ast::FunctionRetTy::Ty(ref ty) = decl.output;
if let ast::TyKind::Tup(ref vals) = ty.node;
if vals.is_empty() && !in_macro(ty.span) && get_def(span) == get_def(ty.span);
then {
let (rspan, appl) = if let Ok(fn_source) =
cx.sess().source_map()
.span_to_snippet(span.with_hi(ty.span.hi())) {
if let Some(rpos) = fn_source.rfind("->") {
(ty.span.with_lo(BytePos(span.lo().0 + rpos as u32)),
Applicability::MachineApplicable)
} else {
(ty.span, Applicability::MaybeIncorrect)
}
} else {
(ty.span, Applicability::MaybeIncorrect)
};
span_lint_and_then(cx, UNUSED_UNIT, rspan, "unneeded unit return type", |db| {
db.span_suggestion_with_applicability(
rspan,
"remove the `-> ()`",
String::new(),
appl,
);
});
}
}
}
fn check_block(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) {
self.check_let_return(cx, block);
if_chain! {
if let Some(ref stmt) = block.stmts.last();
if let ast::StmtKind::Expr(ref expr) = stmt.node;
if is_unit_expr(expr) && !in_macro(expr.span);
then {
let sp = expr.span;
span_lint_and_then(cx, UNUSED_UNIT, sp, "unneeded unit expression", |db| {
db.span_suggestion_with_applicability(
sp,
"remove the final `()`",
String::new(),
Applicability::MachineApplicable,
);
});
}
}
}
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
match e.node {
ast::ExprKind::Ret(Some(ref expr)) | ast::ExprKind::Break(_, Some(ref expr)) => {
if is_unit_expr(expr) && !in_macro(expr.span) {
span_lint_and_then(cx, UNUSED_UNIT, expr.span, "unneeded `()`", |db| {
db.span_suggestion_with_applicability(
expr.span,
"remove the `()`",
String::new(),
Applicability::MachineApplicable,
);
});
}
}
_ => ()
}
}
}
fn attr_is_cfg(attr: &ast::Attribute) -> bool {
attr.meta_item_list().is_some() && attr.name() == "cfg"
}
// get the def site
fn get_def(span: Span) -> Option<Span> {
span.ctxt().outer().expn_info().and_then(|info| info.def_site)
}
// is this expr a `()` unit?
fn is_unit_expr(expr: &ast::Expr) -> bool {
if let ast::ExprKind::Tup(ref vals) = expr.node {
vals.is_empty()
} else {
false
}
}

View File

@ -227,6 +227,7 @@ impl LintPass for Transmute {
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Transmute {
#[allow(clippy::similar_names)]
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
if let ExprKind::Call(ref path_expr, ref args) = e.node {
if let ExprKind::Path(ref qpath) = path_expr.node {

View File

@ -1920,6 +1920,7 @@ enum ImplicitHasherType<'tcx> {
impl<'tcx> ImplicitHasherType<'tcx> {
/// Checks that `ty` is a target type without a BuildHasher.
#[allow(clippy::new_ret_no_self)]
fn new<'a>(cx: &LateContext<'a, 'tcx>, hir_ty: &hir::Ty) -> Option<Self> {
if let TyKind::Path(QPath::Resolved(None, ref path)) = hir_ty.node {
let params: Vec<_> = path.segments.last().as_ref()?.args.as_ref()?

View File

@ -10,7 +10,7 @@
/// Return the index of the character after the first camel-case component of
/// `s`.
pub fn camel_case_until(s: &str) -> usize {
pub fn until(s: &str) -> usize {
let mut iter = s.char_indices();
if let Some((_, first)) = iter.next() {
if !first.is_uppercase() {
@ -43,7 +43,7 @@ pub fn camel_case_until(s: &str) -> usize {
}
/// Return index of the last camel-case component of `s`.
pub fn camel_case_from(s: &str) -> usize {
pub fn from(s: &str) -> usize {
let mut iter = s.char_indices().rev();
if let Some((_, first)) = iter.next() {
if !first.is_lowercase() {
@ -73,52 +73,52 @@ pub fn camel_case_from(s: &str) -> usize {
#[cfg(test)]
mod test {
use super::{camel_case_from, camel_case_until};
use super::{from, until};
#[test]
fn from_full() {
assert_eq!(camel_case_from("AbcDef"), 0);
assert_eq!(camel_case_from("Abc"), 0);
assert_eq!(from("AbcDef"), 0);
assert_eq!(from("Abc"), 0);
}
#[test]
fn from_partial() {
assert_eq!(camel_case_from("abcDef"), 3);
assert_eq!(camel_case_from("aDbc"), 1);
assert_eq!(from("abcDef"), 3);
assert_eq!(from("aDbc"), 1);
}
#[test]
fn from_not() {
assert_eq!(camel_case_from("AbcDef_"), 7);
assert_eq!(camel_case_from("AbcDD"), 5);
assert_eq!(from("AbcDef_"), 7);
assert_eq!(from("AbcDD"), 5);
}
#[test]
fn from_caps() {
assert_eq!(camel_case_from("ABCD"), 4);
assert_eq!(from("ABCD"), 4);
}
#[test]
fn until_full() {
assert_eq!(camel_case_until("AbcDef"), 6);
assert_eq!(camel_case_until("Abc"), 3);
assert_eq!(until("AbcDef"), 6);
assert_eq!(until("Abc"), 3);
}
#[test]
fn until_not() {
assert_eq!(camel_case_until("abcDef"), 0);
assert_eq!(camel_case_until("aDbc"), 0);
assert_eq!(until("abcDef"), 0);
assert_eq!(until("aDbc"), 0);
}
#[test]
fn until_partial() {
assert_eq!(camel_case_until("AbcDef_"), 6);
assert_eq!(camel_case_until("CallTypeC"), 8);
assert_eq!(camel_case_until("AbcDD"), 3);
assert_eq!(until("AbcDef_"), 6);
assert_eq!(until("CallTypeC"), 8);
assert_eq!(until("AbcDD"), 3);
}
#[test]
fn until_caps() {
assert_eq!(camel_case_until("ABCD"), 0);
assert_eq!(until("ABCD"), 0);
}
}
}

View File

@ -73,6 +73,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
&& both(&left.expr, &right.expr, |l, r| self.eq_expr(l, r))
}
#[allow(clippy::similar_names)]
pub fn eq_expr(&mut self, left: &Expr, right: &Expr) -> bool {
if self.ignore_fn && differing_macro_contexts(left.span, right.span) {
return false;
@ -208,6 +209,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
}
}
#[allow(clippy::similar_names)]
fn eq_qpath(&mut self, left: &QPath, right: &QPath) -> bool {
match (left, right) {
(&QPath::Resolved(ref lty, ref lpath), &QPath::Resolved(ref rty, ref rpath)) => {
@ -262,6 +264,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
self.eq_ty_kind(&left.node, &right.node)
}
#[allow(clippy::similar_names)]
pub fn eq_ty_kind(&mut self, left: &TyKind, right: &TyKind) -> bool {
match (left, right) {
(&TyKind::Slice(ref l_vec), &TyKind::Slice(ref r_vec)) => self.eq_ty(l_vec, r_vec),

View File

@ -166,6 +166,7 @@ fn print_decl(cx: &LateContext<'_, '_>, decl: &hir::Decl) {
}
}
#[allow(clippy::similar_names)]
fn print_expr(cx: &LateContext<'_, '_>, expr: &hir::Expr, indent: usize) {
let ind = " ".repeat(indent);
println!("{}+", ind);
@ -424,6 +425,7 @@ fn print_item(cx: &LateContext<'_, '_>, item: &hir::Item) {
}
}
#[allow(clippy::similar_names)]
fn print_pat(cx: &LateContext<'_, '_>, pat: &hir::Pat, indent: usize) {
let ind = " ".repeat(indent);
println!("{}+", ind);

View File

@ -33,8 +33,7 @@ use crate::syntax::source_map::{Span, DUMMY_SP};
use crate::syntax::errors::DiagnosticBuilder;
use crate::syntax::symbol::keywords;
mod camel_case;
pub use self::camel_case::{camel_case_from, camel_case_until};
pub mod camel_case;
pub mod comparisons;
pub mod conf;

View File

@ -54,6 +54,7 @@ struct MutVarsDelegate {
}
impl<'tcx> MutVarsDelegate {
#[allow(clippy::similar_names)]
fn update(&mut self, cat: &'tcx Categorization<'_>) {
match *cat {
Categorization::Local(id) => {

View File

@ -17,5 +17,10 @@ use proc_macro::{TokenStream, quote};
pub fn mini_macro(_: TokenStream) -> TokenStream {
quote!(
#[allow(unused)] fn needless_take_by_value(s: String) { println!("{}", s.len()); }
#[allow(unused)] fn needless_loop(items: &[u8]) {
for i in 0..items.len() {
println!("{}", items[i]);
}
}
)
}

View File

View File

@ -35,6 +35,16 @@ fn main() {
"abc".chars().filter(|c| c.to_owned() != 'X');
"abc".chars().filter(|c| *c != 'X');
let x = &Baz;
let y = &Baz;
y.to_owned() == *x;
let x = &&Baz;
let y = &Baz;
y.to_owned() == **x;
}
struct Foo;
@ -67,3 +77,13 @@ impl std::borrow::Borrow<Foo> for Bar {
&FOO
}
}
#[derive(PartialEq)]
struct Baz;
impl ToOwned for Baz {
type Owned = Baz;
fn to_owned(&self) -> Baz {
Baz
}
}

View File

@ -37,10 +37,22 @@ error: this creates an owned instance just for comparison
| ^^^^^^^^^^^^ help: try: `*c`
error: this creates an owned instance just for comparison
--> $DIR/cmp_owned.rs:44:9
--> $DIR/cmp_owned.rs:42:5
|
44 | self.to_owned() == *other
| ^^^^^^^^^^^^^^^ try calling implementing the comparison without allocating
42 | y.to_owned() == *x;
| ^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating
error: aborting due to 7 previous errors
error: this creates an owned instance just for comparison
--> $DIR/cmp_owned.rs:47:5
|
47 | y.to_owned() == **x;
| ^^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating
error: this creates an owned instance just for comparison
--> $DIR/cmp_owned.rs:54:9
|
54 | self.to_owned() == *other
| ^^^^^^^^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating
error: aborting due to 9 previous errors

View File

@ -151,4 +151,62 @@ fn main() {
} else {
assert!(true); // assert! is just an `if`
}
// The following tests check for the fix of https://github.com/rust-lang-nursery/rust-clippy/issues/798
if x == "hello" {// Not collapsible
if y == "world" {
println!("Hello world!");
}
}
if x == "hello" { // Not collapsible
if y == "world" {
println!("Hello world!");
}
}
if x == "hello" {
// Not collapsible
if y == "world" {
println!("Hello world!");
}
}
if x == "hello" {
if y == "world" { // Collapsible
println!("Hello world!");
}
}
if x == "hello" {
print!("Hello ");
} else {
// Not collapsible
if y == "world" {
println!("world!")
}
}
if x == "hello" {
print!("Hello ");
} else {
// Not collapsible
if let Some(42) = Some(42) {
println!("world!")
}
}
if x == "hello" {
/* Not collapsible */
if y == "world" {
println!("Hello world!");
}
}
if x == "hello" { /* Not collapsible */
if y == "world" {
println!("Hello world!");
}
}
}

View File

@ -240,5 +240,21 @@ help: try
122 | }
|
error: aborting due to 13 previous errors
error: this if statement can be collapsed
--> $DIR/collapsible_if.rs:176:5
|
176 | / if x == "hello" {
177 | | if y == "world" { // Collapsible
178 | | println!("Hello world!");
179 | | }
180 | | }
| |_____^
help: try
|
176 | if x == "hello" && y == "world" { // Collapsible
177 | println!("Hello world!");
178 | }
|
error: aborting due to 14 previous errors

View File

@ -7,11 +7,10 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![allow(clippy::blacklisted_name, clippy::collapsible_if, clippy::cyclomatic_complexity, clippy::eq_op, clippy::needless_continue,
clippy::needless_return, clippy::never_loop, clippy::no_effect, clippy::zero_divided_by_zero)]
clippy::needless_return, clippy::never_loop, clippy::no_effect, clippy::zero_divided_by_zero, clippy::unused_unit)]
fn bar<T>(_: T) {}
fn foo() -> bool { unimplemented!() }
@ -28,6 +27,7 @@ pub enum Abc {
#[warn(clippy::if_same_then_else)]
#[warn(clippy::match_same_arms)]
#[allow(clippy::unused_unit)]
fn if_same_then_else() -> Result<&'static str, ()> {
if true {
Foo { bar: 42 };

View File

@ -7,12 +7,10 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![feature(box_syntax)]
#![allow(warnings, clippy)]
#![warn(boxed_local)]
#![allow(clippy::borrowed_box, clippy::needless_pass_by_value, clippy::unused_unit)]
#![warn(clippy::boxed_local)]
#[derive(Clone)]
struct A;
@ -70,8 +68,7 @@ fn warn_pass() {
}
fn nowarn_return() -> Box<A> {
let fx = box A;
fx // moved out, "escapes"
box A // moved out, "escapes"
}
fn nowarn_move() {
@ -139,3 +136,28 @@ pub struct PeekableSeekable<I: Foo> {
pub fn new(_needs_name: Box<PeekableSeekable<&()>>) -> () {
}
/// Regression for #916, #1123
///
/// This shouldn't warn for `boxed_local`as the implementation of a trait
/// can't change much about the trait definition.
trait BoxedAction {
fn do_sth(self: Box<Self>);
}
impl BoxedAction for u64 {
fn do_sth(self: Box<Self>) {
println!("{}", *self)
}
}
/// Regression for #1478
///
/// This shouldn't warn for `boxed_local`as self itself is a box type.
trait MyTrait {
fn do_sth(self);
}
impl<T> MyTrait for Box<T> {
fn do_sth(self) {}
}

View File

@ -0,0 +1,16 @@
error: local variable doesn't need to be boxed here
--> $DIR/escape_analysis.rs:45:13
|
45 | fn warn_arg(x: Box<A>) {
| ^
|
= note: `-D clippy::boxed-local` implied by `-D warnings`
error: local variable doesn't need to be boxed here
--> $DIR/escape_analysis.rs:137:12
|
137 | pub fn new(_needs_name: Box<PeekableSeekable<&()>>) -> () {
| ^^^^^^^^^^^
error: aborting due to 2 previous errors

View File

@ -550,6 +550,19 @@ pub fn manual_copy(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
for i in 0..10 {
dst_vec[i] = src[i];
}
// Simplify suggestion (issue #3004)
let src = [0, 1, 2, 3, 4];
let mut dst = [0, 0, 0, 0, 0, 0];
let from = 1;
for i in from..from + src.len() {
dst[i] = src[i - from];
}
for i in from..from + 3 {
dst[i] = src[i - from];
}
}
#[warn(clippy::needless_range_loop)]
@ -646,3 +659,38 @@ mod issue_1219 {
}
}
}
mod issue_3308 {
#[warn(clippy::explicit_counter_loop)]
pub fn test() {
// should not trigger the lint because the count is incremented multiple times
let mut skips = 0;
let erasures = vec![];
for i in 0..10 {
while erasures.contains(&(i + skips)) {
skips += 1;
}
println!("{}", skips);
}
// should not trigger the lint because the count is incremented multiple times
let mut skips = 0;
for i in 0..10 {
let mut j = 0;
while j < 5 {
skips += 1;
j += 1;
}
println!("{}", skips);
}
// should not trigger the lint because the count is incremented multiple times
let mut skips = 0;
for i in 0..10 {
for j in 0..5 {
skips += 1;
}
println!("{}", skips);
}
}
}

View File

@ -482,22 +482,34 @@ error: it looks like you're manually copying between slices
| ^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst_vec[..src_vec.len()].clone_from_slice(&src_vec[..])`
error: it looks like you're manually copying between slices
--> $DIR/for_loop.rs:557:14
--> $DIR/for_loop.rs:559:14
|
557 | for i in 0..src.len() {
559 | for i in from..from + src.len() {
| ^^^^^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..from + src.len()].clone_from_slice(&src[0..(from + src.len() - from)])`
error: it looks like you're manually copying between slices
--> $DIR/for_loop.rs:563:14
|
563 | for i in from..from + 3 {
| ^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..from + 3].clone_from_slice(&src[0..(from + 3 - from)])`
error: it looks like you're manually copying between slices
--> $DIR/for_loop.rs:570:14
|
570 | for i in 0..src.len() {
| ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])`
error: the variable `count` is used as a loop counter. Consider using `for (count, item) in text.chars().enumerate()` or similar iterators
--> $DIR/for_loop.rs:618:19
--> $DIR/for_loop.rs:631:19
|
618 | for ch in text.chars() {
631 | for ch in text.chars() {
| ^^^^^^^^^^^^
error: the variable `count` is used as a loop counter. Consider using `for (count, item) in text.chars().enumerate()` or similar iterators
--> $DIR/for_loop.rs:629:19
--> $DIR/for_loop.rs:642:19
|
629 | for ch in text.chars() {
642 | for ch in text.chars() {
| ^^^^^^^^^^^^
error: aborting due to 61 previous errors
error: aborting due to 63 previous errors

0
tests/ui/for_loop.stdout Normal file
View File

View File

@ -1,34 +0,0 @@
error: redundant pattern matching, consider using `is_ok()`
--> $DIR/if_let_redundant_pattern_matching.rs:19:12
|
19 | if let Ok(_) = Ok::<i32, i32>(42) {}
| -------^^^^^------------------------ help: try this: `if Ok::<i32, i32>(42).is_ok()`
|
= note: `-D clippy::if-let-redundant-pattern-matching` implied by `-D warnings`
error: redundant pattern matching, consider using `is_err()`
--> $DIR/if_let_redundant_pattern_matching.rs:21:12
|
21 | if let Err(_) = Err::<i32, i32>(42) {
| _____- ^^^^^^
22 | | }
| |_____- help: try this: `if Err::<i32, i32>(42).is_err()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/if_let_redundant_pattern_matching.rs:24:12
|
24 | if let None = None::<()> {
| _____- ^^^^
25 | | }
| |_____- help: try this: `if None::<()>.is_none()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/if_let_redundant_pattern_matching.rs:27:12
|
27 | if let Some(_) = Some(42) {
| _____- ^^^^^^^
28 | | }
| |_____- help: try this: `if Some(42).is_some()`
error: aborting due to 4 previous errors

View File

@ -46,7 +46,7 @@ error: length comparison to zero
--> $DIR/len_zero.rs:151:8
|
151 | if x.len() == 0 {
| ^^^^^^^^^^^^ help: using `is_empty` is more concise: `x.is_empty()`
| ^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `x.is_empty()`
|
= note: `-D clippy::len-zero` implied by `-D warnings`
@ -54,79 +54,79 @@ error: length comparison to zero
--> $DIR/len_zero.rs:155:8
|
155 | if "".len() == 0 {}
| ^^^^^^^^^^^^^ help: using `is_empty` is more concise: `"".is_empty()`
| ^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `"".is_empty()`
error: length comparison to zero
--> $DIR/len_zero.rs:170:8
|
170 | if has_is_empty.len() == 0 {
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `has_is_empty.is_empty()`
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
error: length comparison to zero
--> $DIR/len_zero.rs:173:8
|
173 | if has_is_empty.len() != 0 {
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `!has_is_empty.is_empty()`
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
error: length comparison to zero
--> $DIR/len_zero.rs:176:8
|
176 | if has_is_empty.len() > 0 {
| ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `!has_is_empty.is_empty()`
| ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
error: length comparison to one
--> $DIR/len_zero.rs:179:8
|
179 | if has_is_empty.len() < 1 {
| ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `has_is_empty.is_empty()`
| ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
error: length comparison to one
--> $DIR/len_zero.rs:182:8
|
182 | if has_is_empty.len() >= 1 {
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `!has_is_empty.is_empty()`
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
error: length comparison to zero
--> $DIR/len_zero.rs:193:8
|
193 | if 0 == has_is_empty.len() {
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `has_is_empty.is_empty()`
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
error: length comparison to zero
--> $DIR/len_zero.rs:196:8
|
196 | if 0 != has_is_empty.len() {
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `!has_is_empty.is_empty()`
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
error: length comparison to zero
--> $DIR/len_zero.rs:199:8
|
199 | if 0 < has_is_empty.len() {
| ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `!has_is_empty.is_empty()`
| ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
error: length comparison to one
--> $DIR/len_zero.rs:202:8
|
202 | if 1 <= has_is_empty.len() {
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `!has_is_empty.is_empty()`
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
error: length comparison to one
--> $DIR/len_zero.rs:205:8
|
205 | if 1 > has_is_empty.len() {
| ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `has_is_empty.is_empty()`
| ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
error: length comparison to zero
--> $DIR/len_zero.rs:219:8
|
219 | if with_is_empty.len() == 0 {
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `with_is_empty.is_empty()`
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `with_is_empty.is_empty()`
error: length comparison to zero
--> $DIR/len_zero.rs:232:8
|
232 | if b.len() != 0 {}
| ^^^^^^^^^^^^ help: using `is_empty` is more concise: `!b.is_empty()`
| ^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `!b.is_empty()`
error: trait `DependsOnFoo` has a `len` method but no (possibly inherited) `is_empty` method
--> $DIR/len_zero.rs:238:1

View File

@ -13,7 +13,7 @@
#![warn(clippy::all)]
#![allow(unused, clippy::if_let_redundant_pattern_matching)]
#![allow(unused, clippy::redundant_pattern_matching)]
#![warn(clippy::single_match_else, clippy::match_same_arms)]
enum ExprNode {

View File

@ -14,7 +14,7 @@
#![warn(clippy::all, clippy::pedantic, clippy::option_unwrap_used)]
#![allow(clippy::blacklisted_name, unused, clippy::print_stdout, clippy::non_ascii_literal, clippy::new_without_default,
clippy::new_without_default_derive, clippy::missing_docs_in_private_items, clippy::needless_pass_by_value,
clippy::default_trait_access, clippy::use_self, clippy::useless_format)]
clippy::default_trait_access, clippy::use_self, clippy::new_ret_no_self, clippy::useless_format)]
use std::collections::BTreeMap;
use std::collections::HashMap;
@ -43,7 +43,7 @@ impl T {
fn to_something(self) -> u32 { 0 }
fn new(self) {}
fn new(self) -> Self { unimplemented!(); }
}
struct Lt<'a> {

View File

@ -23,17 +23,9 @@ error: methods called `to_*` usually take self by reference; consider choosing a
error: methods called `new` usually take no self; consider choosing a less ambiguous name
--> $DIR/methods.rs:46:12
|
46 | fn new(self) {}
46 | fn new(self) -> Self { unimplemented!(); }
| ^^^^
error: methods called `new` usually return `Self`
--> $DIR/methods.rs:46:5
|
46 | fn new(self) {}
| ^^^^^^^^^^^^^^^
|
= note: `-D clippy::new-ret-no-self` implied by `-D warnings`
error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead
--> $DIR/methods.rs:114:13
|
@ -465,5 +457,5 @@ error: used unwrap() on an Option value. If you don't want to handle the None ca
|
= note: `-D clippy::option-unwrap-used` implied by `-D warnings`
error: aborting due to 58 previous errors
error: aborting due to 57 previous errors

7
tests/ui/my_lint.rs Normal file
View File

@ -0,0 +1,7 @@
#[clippy::author]
#[cfg(any(target_arch = "x86"))]
pub struct Foo {
x: u32,
}
fn main() {}

View File

@ -11,7 +11,7 @@
#![warn(clippy::needless_pass_by_value)]
#![allow(dead_code, clippy::single_match, clippy::if_let_redundant_pattern_matching, clippy::many_single_char_names, clippy::option_option)]
#![allow(dead_code, clippy::single_match, clippy::redundant_pattern_matching, clippy::many_single_char_names, clippy::option_option)]
use std::borrow::Borrow;
use std::convert::AsRef;

View File

@ -62,4 +62,18 @@ fn main() {
g[i] = g[i+1..].iter().sum();
}
assert_eq!(g, vec![20, 18, 15, 11, 6, 0]);
let x = 5;
let mut vec = vec![0; 9];
for i in x..x + 4 {
vec[i] += 1;
}
let x = 5;
let mut vec = vec![0; 10];
for i in x..=x + 4 {
vec[i] += 1;
}
}

View File

@ -30,5 +30,25 @@ help: consider using an iterator
45 | for <item> in &mut ms {
| ^^^^^^ ^^^^^^^
error: aborting due to 3 previous errors
error: the loop variable `i` is only used to index `vec`.
--> $DIR/needless_range_loop.rs:69:14
|
69 | for i in x..x + 4 {
| ^^^^^^^^
help: consider using an iterator
|
69 | for <item> in vec.iter_mut().skip(x).take(4) {
| ^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: the loop variable `i` is only used to index `vec`.
--> $DIR/needless_range_loop.rs:76:14
|
76 | for i in x..=x + 4 {
| ^^^^^^^^^
help: consider using an iterator
|
76 | for <item> in vec.iter_mut().skip(x).take(4 + 1) {
| ^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 5 previous errors

View File

@ -0,0 +1,93 @@
#![warn(clippy::new_ret_no_self)]
#![allow(dead_code, clippy::trivially_copy_pass_by_ref)]
fn main(){}
trait R {
type Item;
}
trait Q {
type Item;
type Item2;
}
struct S;
impl R for S {
type Item = Self;
}
impl S {
// should not trigger the lint
pub fn new() -> impl R<Item = Self> {
S
}
}
struct S2;
impl R for S2 {
type Item = Self;
}
impl S2 {
// should not trigger the lint
pub fn new(_: String) -> impl R<Item = Self> {
S2
}
}
struct S3;
impl R for S3 {
type Item = u32;
}
impl S3 {
// should trigger the lint
pub fn new(_: String) -> impl R<Item = u32> {
S3
}
}
struct S4;
impl Q for S4 {
type Item = u32;
type Item2 = Self;
}
impl S4 {
// should not trigger the lint
pub fn new(_: String) -> impl Q<Item = u32, Item2 = Self> {
S4
}
}
struct T;
impl T {
// should not trigger lint
pub fn new() -> Self {
unimplemented!();
}
}
struct U;
impl U {
// should trigger lint
pub fn new() -> u32 {
unimplemented!();
}
}
struct V;
impl V {
// should trigger lint
pub fn new(_: String) -> u32 {
unimplemented!();
}
}

View File

@ -0,0 +1,28 @@
error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:49:5
|
49 | / pub fn new(_: String) -> impl R<Item = u32> {
50 | | S3
51 | | }
| |_____^
|
= note: `-D clippy::new-ret-no-self` implied by `-D warnings`
error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:81:5
|
81 | / pub fn new() -> u32 {
82 | | unimplemented!();
83 | | }
| |_____^
error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:90:5
|
90 | / pub fn new(_: String) -> u32 {
91 | | unimplemented!();
92 | | }
| |_____^
error: aborting due to 3 previous errors

View File

@ -12,7 +12,7 @@
#![warn(clippy::all)]
#![warn(clippy::if_let_redundant_pattern_matching)]
#![warn(clippy::redundant_pattern_matching)]
fn main() {
@ -42,4 +42,34 @@ fn main() {
if let Ok(x) = Ok::<i32,i32>(42) {
println!("{}", x);
}
match Ok::<i32, i32>(42) {
Ok(_) => true,
Err(_) => false,
};
match Ok::<i32, i32>(42) {
Ok(_) => false,
Err(_) => true,
};
match Err::<i32, i32>(42) {
Ok(_) => false,
Err(_) => true,
};
match Err::<i32, i32>(42) {
Ok(_) => true,
Err(_) => false,
};
match Some(42) {
Some(_) => true,
None => false,
};
match None::<()> {
Some(_) => false,
None => true,
};
}

View File

@ -0,0 +1,88 @@
error: redundant pattern matching, consider using `is_ok()`
--> $DIR/redundant_pattern_matching.rs:19:12
|
19 | if let Ok(_) = Ok::<i32, i32>(42) {}
| -------^^^^^------------------------ help: try this: `if Ok::<i32, i32>(42).is_ok()`
|
= note: `-D clippy::redundant-pattern-matching` implied by `-D warnings`
error: redundant pattern matching, consider using `is_err()`
--> $DIR/redundant_pattern_matching.rs:21:12
|
21 | if let Err(_) = Err::<i32, i32>(42) {
| _____- ^^^^^^
22 | | }
| |_____- help: try this: `if Err::<i32, i32>(42).is_err()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching.rs:24:12
|
24 | if let None = None::<()> {
| _____- ^^^^
25 | | }
| |_____- help: try this: `if None::<()>.is_none()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching.rs:27:12
|
27 | if let Some(_) = Some(42) {
| _____- ^^^^^^^
28 | | }
| |_____- help: try this: `if Some(42).is_some()`
error: redundant pattern matching, consider using `is_ok()`
--> $DIR/redundant_pattern_matching.rs:46:5
|
46 | / match Ok::<i32, i32>(42) {
47 | | Ok(_) => true,
48 | | Err(_) => false,
49 | | };
| |_____^ help: try this: `Ok::<i32, i32>(42).is_ok()`
error: redundant pattern matching, consider using `is_err()`
--> $DIR/redundant_pattern_matching.rs:51:5
|
51 | / match Ok::<i32, i32>(42) {
52 | | Ok(_) => false,
53 | | Err(_) => true,
54 | | };
| |_____^ help: try this: `Ok::<i32, i32>(42).is_err()`
error: redundant pattern matching, consider using `is_err()`
--> $DIR/redundant_pattern_matching.rs:56:5
|
56 | / match Err::<i32, i32>(42) {
57 | | Ok(_) => false,
58 | | Err(_) => true,
59 | | };
| |_____^ help: try this: `Err::<i32, i32>(42).is_err()`
error: redundant pattern matching, consider using `is_ok()`
--> $DIR/redundant_pattern_matching.rs:61:5
|
61 | / match Err::<i32, i32>(42) {
62 | | Ok(_) => true,
63 | | Err(_) => false,
64 | | };
| |_____^ help: try this: `Err::<i32, i32>(42).is_ok()`
error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching.rs:66:5
|
66 | / match Some(42) {
67 | | Some(_) => true,
68 | | None => false,
69 | | };
| |_____^ help: try this: `Some(42).is_some()`
error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching.rs:71:5
|
71 | / match None::<()> {
72 | | Some(_) => false,
73 | | None => true,
74 | | };
| |_____^ help: try this: `None::<()>.is_none()`
error: aborting due to 10 previous errors

View File

52
tests/ui/unused_unit.rs Normal file
View File

@ -0,0 +1,52 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// run-rustfix
// compile-pass
// The output for humans should just highlight the whole span without showing
// the suggested replacement, but we also want to test that suggested
// replacement only removes one set of parentheses, rather than naïvely
// stripping away any starting or ending parenthesis characters—hence this
// test of the JSON error format.
#![deny(clippy::unused_unit)]
#![allow(clippy::needless_return)]
struct Unitter;
impl Unitter {
// try to disorient the lint with multiple unit returns and newlines
pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) ->
()
where G: Fn() -> () {
let _y: &Fn() -> () = &f;
(); // this should not lint, as it's not in return type position
}
}
impl Into<()> for Unitter {
fn into(self) -> () {
()
}
}
fn return_unit() -> () { () }
fn main() {
let u = Unitter;
assert_eq!(u.get_unit(|| {}, return_unit), u.into());
return_unit();
loop {
break();
}
return();
}

View File

@ -0,0 +1,52 @@
error: unneeded unit return type
--> $DIR/unused_unit.rs:28:59
|
28 | pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) ->
| ___________________________________________________________^
29 | | ()
| |__________^ help: remove the `-> ()`
|
note: lint level defined here
--> $DIR/unused_unit.rs:21:9
|
21 | #![deny(clippy::unused_unit)]
| ^^^^^^^^^^^^^^^^^^^
error: unneeded unit return type
--> $DIR/unused_unit.rs:37:19
|
37 | fn into(self) -> () {
| ^^^^^ help: remove the `-> ()`
error: unneeded unit expression
--> $DIR/unused_unit.rs:38:9
|
38 | ()
| ^^ help: remove the final `()`
error: unneeded unit return type
--> $DIR/unused_unit.rs:42:18
|
42 | fn return_unit() -> () { () }
| ^^^^^ help: remove the `-> ()`
error: unneeded unit expression
--> $DIR/unused_unit.rs:42:26
|
42 | fn return_unit() -> () { () }
| ^^ help: remove the final `()`
error: unneeded `()`
--> $DIR/unused_unit.rs:49:14
|
49 | break();
| ^^ help: remove the `()`
error: unneeded `()`
--> $DIR/unused_unit.rs:51:11
|
51 | return();
| ^^ help: remove the `()`
error: aborting due to 7 previous errors

View File

@ -167,6 +167,19 @@
});
}
function selectGroup($scope, selectedGroup) {
var groups = $scope.groups;
for (var group in groups) {
if (groups.hasOwnProperty(group)) {
if (group === selectedGroup) {
groups[group] = true;
} else {
groups[group] = false;
}
}
}
}
angular.module("clippy", [])
.filter('markdown', function ($sce) {
return function (text) {
@ -223,6 +236,11 @@
return result;
}, {});
var selectedGroup = getQueryVariable("sel");
if (selectedGroup) {
selectGroup($scope, selectedGroup.toLowerCase());
}
scrollToLintByURL($scope);
})
.error(function (data) {
@ -243,6 +261,17 @@
}, false);
});
})();
function getQueryVariable(variable) {
var query = window.location.search.substring(1);
var vars = query.split('&');
for (var i = 0; i < vars.length; i++) {
var pair = vars[i].split('=');
if (decodeURIComponent(pair[0]) == variable) {
return decodeURIComponent(pair[1]);
}
}
}
</script>
</body>
</html>