diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index 22a2f2ca79c7..58b4f87e8727 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -4,6 +4,7 @@ use clap::{App, Arg, SubCommand}; use clippy_dev::*; mod fmt; +mod new_lint; mod stderr_length_check; #[derive(PartialEq)] @@ -51,6 +52,47 @@ fn main() { .help("Checks that util/dev update_lints has been run. Used on CI."), ), ) + .subcommand( + SubCommand::with_name("new_lint") + .about("Create new lint and run util/dev update_lints") + .arg( + Arg::with_name("pass") + .short("p") + .long("pass") + .help("Specify whether the lint runs during the early or late pass") + .takes_value(true) + .possible_values(&["early", "late"]) + .required(true), + ) + .arg( + Arg::with_name("name") + .short("n") + .long("name") + .help("Name of the new lint in snake case, ex: fn_too_long") + .takes_value(true) + .required(true), + ) + .arg( + Arg::with_name("category") + .short("c") + .long("category") + .help("What category the lint belongs to") + .default_value("nursery") + .possible_values(&[ + "style", + "correctness", + "complexity", + "perf", + "pedantic", + "restriction", + "cargo", + "nursery", + "internal", + "internal_warn", + ]) + .takes_value(true), + ), + ) .arg( Arg::with_name("limit-stderr-length") .long("limit-stderr-length") @@ -75,6 +117,16 @@ fn main() { update_lints(&UpdateMode::Change); } }, + ("new_lint", Some(matches)) => { + match new_lint::create( + matches.value_of("pass"), + matches.value_of("name"), + matches.value_of("category"), + ) { + Ok(_) => update_lints(&UpdateMode::Change), + Err(e) => eprintln!("Unable to create lint: {}", e), + } + }, _ => {}, } } diff --git a/clippy_dev/src/new_lint.rs b/clippy_dev/src/new_lint.rs new file mode 100644 index 000000000000..76b8dd60d432 --- /dev/null +++ b/clippy_dev/src/new_lint.rs @@ -0,0 +1,185 @@ +use std::fs::{File, OpenOptions}; +use std::io; +use std::io::prelude::*; +use std::io::ErrorKind; +use std::path::{Path, PathBuf}; + +pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str>) -> Result<(), io::Error> { + let pass = pass.expect("`pass` argument is validated by clap"); + let lint_name = lint_name.expect("`name` argument is validated by clap"); + let category = category.expect("`category` argument is validated by clap"); + + match open_files(lint_name) { + Ok((mut test_file, mut lint_file)) => { + let (pass_type, pass_lifetimes, pass_import, context_import) = match pass { + "early" => ("EarlyLintPass", "", "use syntax::ast::*;", "EarlyContext"), + "late" => ("LateLintPass", "<'_, '_>", "use rustc_hir::*;", "LateContext"), + _ => { + unreachable!("`pass_type` should only ever be `early` or `late`!"); + }, + }; + + let camel_case_name = to_camel_case(lint_name); + + if let Err(e) = test_file.write_all(get_test_file_contents(lint_name).as_bytes()) { + return Err(io::Error::new( + ErrorKind::Other, + format!("Could not write to test file: {}", e), + )); + }; + + if let Err(e) = lint_file.write_all( + get_lint_file_contents( + pass_type, + pass_lifetimes, + lint_name, + &camel_case_name, + category, + pass_import, + context_import, + ) + .as_bytes(), + ) { + return Err(io::Error::new( + ErrorKind::Other, + format!("Could not write to lint file: {}", e), + )); + } + Ok(()) + }, + Err(e) => Err(io::Error::new( + ErrorKind::Other, + format!("Unable to create lint: {}", e), + )), + } +} + +fn open_files(lint_name: &str) -> Result<(File, File), io::Error> { + let project_root = project_root()?; + + let test_file_path = project_root.join("tests").join("ui").join(format!("{}.rs", lint_name)); + let lint_file_path = project_root + .join("clippy_lints") + .join("src") + .join(format!("{}.rs", lint_name)); + + if Path::new(&test_file_path).exists() { + return Err(io::Error::new( + ErrorKind::AlreadyExists, + format!("test file {:?} already exists", test_file_path), + )); + } + if Path::new(&lint_file_path).exists() { + return Err(io::Error::new( + ErrorKind::AlreadyExists, + format!("lint file {:?} already exists", lint_file_path), + )); + } + + let test_file = OpenOptions::new().write(true).create_new(true).open(test_file_path)?; + let lint_file = OpenOptions::new().write(true).create_new(true).open(lint_file_path)?; + + Ok((test_file, lint_file)) +} + +fn project_root() -> Result { + let current_dir = std::env::current_dir()?; + for path in current_dir.ancestors() { + let result = std::fs::read_to_string(path.join("Cargo.toml")); + if let Err(err) = &result { + if err.kind() == io::ErrorKind::NotFound { + continue; + } + } + + let content = result?; + if content.contains("[package]\nname = \"clippy\"") { + return Ok(path.to_path_buf()); + } + } + Err(io::Error::new(ErrorKind::Other, "Unable to find project root")) +} + +fn to_camel_case(name: &str) -> String { + name.split('_') + .map(|s| { + if s.is_empty() { + String::from("") + } else { + [&s[0..1].to_uppercase(), &s[1..]].concat() + } + }) + .collect() +} + +fn get_test_file_contents(lint_name: &str) -> String { + format!( + "#![warn(clippy::{})] + +fn main() {{ + // test code goes here +}} +", + lint_name + ) +} + +fn get_lint_file_contents( + pass_type: &str, + pass_lifetimes: &str, + lint_name: &str, + camel_case_name: &str, + category: &str, + pass_import: &str, + context_import: &str, +) -> String { + format!( + "use rustc_lint::{{LintArray, LintPass, {type}, {context_import}}}; +use rustc_session::{{declare_lint_pass, declare_tool_lint}}; +{pass_import} + +declare_clippy_lint! {{ + /// **What it does:** + /// + /// **Why is this bad?** + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// // example code + /// ``` + pub {name_upper}, + {category}, + \"default lint description\" +}} + +declare_lint_pass!({name_camel} => [{name_upper}]); + +impl {type}{lifetimes} for {name_camel} {{}} +", + type=pass_type, + lifetimes=pass_lifetimes, + name_upper=lint_name.to_uppercase(), + name_camel=camel_case_name, + category=category, + pass_import=pass_import, + context_import=context_import + ) +} + +#[test] +fn test_camel_case() { + let s = "a_lint"; + let s2 = to_camel_case(s); + assert_eq!(s2, "ALint"); + + let name = "a_really_long_new_lint"; + let name2 = to_camel_case(name); + assert_eq!(name2, "AReallyLongNewLint"); + + let name3 = "lint__name"; + let name4 = to_camel_case(name3); + assert_eq!(name4, "LintName"); +} diff --git a/clippy_lints/src/booleans.rs b/clippy_lints/src/booleans.rs index 12f117927160..60e62542a725 100644 --- a/clippy_lints/src/booleans.rs +++ b/clippy_lints/src/booleans.rs @@ -207,7 +207,7 @@ impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> { } }, Or(v) => { - for (index, inner) in v.iter().enumerate() { + for (index, inner) in v.iter().rev().enumerate() { if index > 0 { self.output.push_str(" || "); } diff --git a/clippy_lints/src/copy_iterator.rs b/clippy_lints/src/copy_iterator.rs index c1010dd18682..720ff6bac9a2 100644 --- a/clippy_lints/src/copy_iterator.rs +++ b/clippy_lints/src/copy_iterator.rs @@ -33,7 +33,11 @@ declare_lint_pass!(CopyIterator => [COPY_ITERATOR]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyIterator { fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) { - if let ItemKind::Impl(_, _, _, _, Some(ref trait_ref), _, _) = item.kind { + if let ItemKind::Impl { + of_trait: Some(ref trait_ref), + .. + } = item.kind + { let ty = cx.tcx.type_of(cx.tcx.hir().local_def_id(item.hir_id)); if is_copy(cx, ty) && match_path(&trait_ref.path, &paths::ITERATOR) { diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index a49879e83109..efb44ab6dbb9 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -66,7 +66,11 @@ declare_lint_pass!(Derive => [EXPL_IMPL_CLONE_ON_COPY, DERIVE_HASH_XOR_EQ]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Derive { fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) { - if let ItemKind::Impl(_, _, _, _, Some(ref trait_ref), _, _) = item.kind { + if let ItemKind::Impl { + of_trait: Some(ref trait_ref), + .. + } = item.kind + { let ty = cx.tcx.type_of(cx.tcx.hir().local_def_id(item.hir_id)); let is_automatically_derived = is_automatically_derived(&*item.attrs); diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index dd09499c0ceb..5db1886bb52d 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -159,7 +159,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DocMarkdown { lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers); } }, - hir::ItemKind::Impl(_, _, _, _, ref trait_ref, ..) => { + hir::ItemKind::Impl { + of_trait: ref trait_ref, + .. + } => { self.in_trait_impl = trait_ref.is_some(); }, _ => {}, @@ -167,7 +170,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DocMarkdown { } fn check_item_post(&mut self, _cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item<'_>) { - if let hir::ItemKind::Impl(..) = item.kind { + if let hir::ItemKind::Impl { .. } = item.kind { self.in_trait_impl = false; } } diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs index 376ad3fe627a..6d0a1f673e77 100644 --- a/clippy_lints/src/escape.rs +++ b/clippy_lints/src/escape.rs @@ -64,7 +64,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoxedLocal { let parent_node = cx.tcx.hir().find(parent_id); if let Some(Node::Item(item)) = parent_node { - if let ItemKind::Impl(_, _, _, _, Some(..), _, _) = item.kind { + if let ItemKind::Impl { of_trait: Some(_), .. } = item.kind { return; } } diff --git a/clippy_lints/src/fallible_impl_from.rs b/clippy_lints/src/fallible_impl_from.rs index 40a68a89a380..87792b9fba47 100644 --- a/clippy_lints/src/fallible_impl_from.rs +++ b/clippy_lints/src/fallible_impl_from.rs @@ -36,7 +36,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FallibleImplFrom { // check for `impl From for ..` let impl_def_id = cx.tcx.hir().local_def_id(item.hir_id); if_chain! { - if let hir::ItemKind::Impl(.., impl_items) = item.kind; + if let hir::ItemKind::Impl{ items: impl_items, .. } = item.kind; if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(impl_def_id); if match_def_path(cx, impl_trait_ref.def_id, &FROM_TRAIT); then { diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index 2869bd9c4bda..5a5431f47a07 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -196,7 +196,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions { hir_id: hir::HirId, ) { let is_impl = if let Some(hir::Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { - matches!(item.kind, hir::ItemKind::Impl(_, _, _, _, Some(_), _, _)) + matches!(item.kind, hir::ItemKind::Impl{ of_trait: Some(_), .. }) } else { false }; diff --git a/clippy_lints/src/ok_if_let.rs b/clippy_lints/src/if_let_some_result.rs similarity index 54% rename from clippy_lints/src/ok_if_let.rs rename to clippy_lints/src/if_let_some_result.rs index 6145fae31562..a9826d586339 100644 --- a/clippy_lints/src/ok_if_let.rs +++ b/clippy_lints/src/if_let_some_result.rs @@ -1,5 +1,6 @@ -use crate::utils::{match_type, method_chain_args, paths, snippet, span_help_and_lint}; +use crate::utils::{match_type, method_chain_args, paths, snippet_with_applicability, span_lint_and_sugg}; use if_chain::if_chain; +use rustc_errors::Applicability; use rustc_hir::*; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -39,20 +40,32 @@ declare_lint_pass!(OkIfLet => [IF_LET_SOME_RESULT]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OkIfLet { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { if_chain! { //begin checking variables - if let ExprKind::Match(ref op, ref body, ref source) = expr.kind; //test if expr is a match - if let MatchSource::IfLetDesugar { .. } = *source; //test if it is an If Let - if let ExprKind::MethodCall(_, _, ref result_types) = op.kind; //check is expr.ok() has type Result.ok() + if let ExprKind::Match(ref op, ref body, source) = expr.kind; //test if expr is a match + if let MatchSource::IfLetDesugar { .. } = source; //test if it is an If Let + if let ExprKind::MethodCall(_, ok_span, ref result_types) = op.kind; //check is expr.ok() has type Result.ok() if let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = body[0].pat.kind; //get operation if method_chain_args(op, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized; + let is_result_type = match_type(cx, cx.tables.expr_ty(&result_types[0]), &paths::RESULT); + if print::to_string(print::NO_ANN, |s| s.print_path(x, false)) == "Some" && is_result_type; then { - let is_result_type = match_type(cx, cx.tables.expr_ty(&result_types[0]), &paths::RESULT); - let some_expr_string = snippet(cx, y[0].span, ""); - if print::to_string(print::NO_ANN, |s| s.print_path(x, false)) == "Some" && is_result_type { - span_help_and_lint(cx, IF_LET_SOME_RESULT, expr.span, + let mut applicability = Applicability::MachineApplicable; + let some_expr_string = snippet_with_applicability(cx, y[0].span, "", &mut applicability); + let trimmed_ok = snippet_with_applicability(cx, op.span.until(ok_span), "", &mut applicability); + let sugg = format!( + "if let Ok({}) = {}", + some_expr_string, + trimmed_ok.trim().trim_end_matches('.'), + ); + span_lint_and_sugg( + cx, + IF_LET_SOME_RESULT, + expr.span.with_hi(op.span.hi()), "Matching on `Some` with `ok()` is redundant", - &format!("Consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string)); - } + &format!("Consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string), + sugg, + applicability, + ); } } } diff --git a/clippy_lints/src/inherent_impl.rs b/clippy_lints/src/inherent_impl.rs index a3a16ced2e86..058ee7265fbc 100644 --- a/clippy_lints/src/inherent_impl.rs +++ b/clippy_lints/src/inherent_impl.rs @@ -49,7 +49,12 @@ impl_lint_pass!(MultipleInherentImpl => [MULTIPLE_INHERENT_IMPL]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MultipleInherentImpl { fn check_item(&mut self, _: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) { - if let ItemKind::Impl(_, _, _, ref generics, None, _, _) = item.kind { + if let ItemKind::Impl { + ref generics, + of_trait: None, + .. + } = item.kind + { // Remember for each inherent implementation encoutered its span and generics // but filter out implementations that have generic params (type or lifetime) // or are derived from a macro diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index 3a07ec183b44..b522a74fb0e8 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -78,7 +78,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LenZero { match item.kind { ItemKind::Trait(_, _, _, _, ref trait_items) => check_trait_items(cx, item, trait_items), - ItemKind::Impl(_, _, _, _, None, _, ref impl_items) => check_impl_items(cx, item, impl_items), + ItemKind::Impl { + of_trait: None, + items: ref impl_items, + .. + } => check_impl_items(cx, item, impl_items), _ => (), } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 576a4a8cd7c2..497146772a51 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -3,7 +3,6 @@ #![feature(box_syntax)] #![feature(box_patterns)] #![feature(rustc_private)] -#![feature(slice_patterns)] #![feature(stmt_expr_attributes)] #![allow(clippy::missing_docs_in_private_items, clippy::must_use_candidate)] #![recursion_limit = "512"] @@ -211,6 +210,7 @@ pub mod functions; pub mod get_last_with_len; pub mod identity_conversion; pub mod identity_op; +pub mod if_let_some_result; pub mod if_not_else; pub mod implicit_return; pub mod indexing_slicing; @@ -264,7 +264,6 @@ pub mod new_without_default; pub mod no_effect; pub mod non_copy_const; pub mod non_expressive_names; -pub mod ok_if_let; pub mod open_options; pub mod overflow_check_conditional; pub mod panic_unimplemented; @@ -546,6 +545,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &get_last_with_len::GET_LAST_WITH_LEN, &identity_conversion::IDENTITY_CONVERSION, &identity_op::IDENTITY_OP, + &if_let_some_result::IF_LET_SOME_RESULT, &if_not_else::IF_NOT_ELSE, &implicit_return::IMPLICIT_RETURN, &indexing_slicing::INDEXING_SLICING, @@ -704,7 +704,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &non_expressive_names::JUST_UNDERSCORES_AND_DIGITS, &non_expressive_names::MANY_SINGLE_CHAR_NAMES, &non_expressive_names::SIMILAR_NAMES, - &ok_if_let::IF_LET_SOME_RESULT, &open_options::NONSENSICAL_OPEN_OPTIONS, &overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL, &panic_unimplemented::PANIC, @@ -905,7 +904,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box eval_order_dependence::EvalOrderDependence); store.register_late_pass(|| box missing_doc::MissingDoc::new()); store.register_late_pass(|| box missing_inline::MissingInline); - store.register_late_pass(|| box ok_if_let::OkIfLet); + store.register_late_pass(|| box if_let_some_result::OkIfLet); store.register_late_pass(|| box redundant_pattern_matching::RedundantPatternMatching); store.register_late_pass(|| box partialeq_ne_impl::PartialEqNeImpl); store.register_late_pass(|| box unused_io_amount::UnusedIoAmount); @@ -1068,6 +1067,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&needless_continue::NEEDLESS_CONTINUE), LintId::of(&needless_pass_by_value::NEEDLESS_PASS_BY_VALUE), LintId::of(&non_expressive_names::SIMILAR_NAMES), + LintId::of(&ranges::RANGE_PLUS_ONE), LintId::of(&replace_consts::REPLACE_CONSTS), LintId::of(&shadow::SHADOW_UNRELATED), LintId::of(&strings::STRING_ADD_ASSIGN), @@ -1090,6 +1090,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS), LintId::of(&utils::internal_lints::OUTER_EXPN_EXPN_DATA), LintId::of(&utils::internal_lints::PRODUCE_ICE), + LintId::of(&utils::internal_lints::DEFAULT_LINT), ]); store.register_group(true, "clippy::all", Some("clippy"), vec![ @@ -1152,6 +1153,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&get_last_with_len::GET_LAST_WITH_LEN), LintId::of(&identity_conversion::IDENTITY_CONVERSION), LintId::of(&identity_op::IDENTITY_OP), + LintId::of(&if_let_some_result::IF_LET_SOME_RESULT), LintId::of(&indexing_slicing::OUT_OF_BOUNDS_INDEXING), LintId::of(&infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(&infinite_iter::INFINITE_ITER), @@ -1264,7 +1266,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST), LintId::of(&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), LintId::of(&non_expressive_names::MANY_SINGLE_CHAR_NAMES), - LintId::of(&ok_if_let::IF_LET_SOME_RESULT), LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS), LintId::of(&overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL), LintId::of(&panic_unimplemented::PANIC_PARAMS), @@ -1276,7 +1277,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&ptr_offset_with_cast::PTR_OFFSET_WITH_CAST), LintId::of(&question_mark::QUESTION_MARK), LintId::of(&ranges::RANGE_MINUS_ONE), - LintId::of(&ranges::RANGE_PLUS_ONE), LintId::of(&ranges::RANGE_ZIP_WITH_LEN), LintId::of(&redundant_clone::REDUNDANT_CLONE), LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES), @@ -1367,6 +1367,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&formatting::SUSPICIOUS_UNARY_OP_FORMATTING), LintId::of(&functions::DOUBLE_MUST_USE), LintId::of(&functions::MUST_USE_UNIT), + LintId::of(&if_let_some_result::IF_LET_SOME_RESULT), LintId::of(&infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(&inherent_to_string::INHERENT_TO_STRING), LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY), @@ -1413,7 +1414,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&new_without_default::NEW_WITHOUT_DEFAULT), LintId::of(&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), LintId::of(&non_expressive_names::MANY_SINGLE_CHAR_NAMES), - LintId::of(&ok_if_let::IF_LET_SOME_RESULT), LintId::of(&panic_unimplemented::PANIC_PARAMS), LintId::of(&ptr::CMP_NULL), LintId::of(&ptr::PTR_ARG), @@ -1494,7 +1494,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&precedence::PRECEDENCE), LintId::of(&ptr_offset_with_cast::PTR_OFFSET_WITH_CAST), LintId::of(&ranges::RANGE_MINUS_ONE), - LintId::of(&ranges::RANGE_PLUS_ONE), LintId::of(&ranges::RANGE_ZIP_WITH_LEN), LintId::of(&reference::DEREF_ADDROF), LintId::of(&reference::REF_IN_DEREF), diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 30322ec8ebb9..ed37d3411b5a 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1335,7 +1335,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { if_chain! { if let hir::ImplItemKind::Method(ref sig, id) = impl_item.kind; if let Some(first_arg) = iter_input_pats(&sig.decl, cx.tcx.hir().body(id)).next(); - if let hir::ItemKind::Impl(_, _, _, _, None, _, _) = item.kind; + if let hir::ItemKind::Impl{ of_trait: None, .. } = item.kind; let method_def_id = cx.tcx.hir().local_def_id(impl_item.hir_id); let method_sig = cx.tcx.fn_sig(method_def_id); diff --git a/clippy_lints/src/missing_doc.rs b/clippy_lints/src/missing_doc.rs index 4e6989f7076c..5ebc8a4d6479 100644 --- a/clippy_lints/src/missing_doc.rs +++ b/clippy_lints/src/missing_doc.rs @@ -154,7 +154,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDoc { hir::ItemKind::ExternCrate(..) | hir::ItemKind::ForeignMod(..) | hir::ItemKind::GlobalAsm(..) - | hir::ItemKind::Impl(..) + | hir::ItemKind::Impl { .. } | hir::ItemKind::Use(..) => return, }; diff --git a/clippy_lints/src/missing_inline.rs b/clippy_lints/src/missing_inline.rs index 562259ffc01a..407321effb80 100644 --- a/clippy_lints/src/missing_inline.rs +++ b/clippy_lints/src/missing_inline.rs @@ -124,7 +124,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingInline { | hir::ItemKind::OpaqueTy(..) | hir::ItemKind::ExternCrate(..) | hir::ItemKind::ForeignMod(..) - | hir::ItemKind::Impl(..) + | hir::ItemKind::Impl { .. } | hir::ItemKind::Use(..) => {}, }; } diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index ad4fec1272d0..8af5c3009e95 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/clippy_lints/src/needless_pass_by_value.rs @@ -91,7 +91,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue { // Exclude non-inherent impls if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { - if matches!(item.kind, ItemKind::Impl(_, _, _, _, Some(_), _, _) | + if matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), .. } | ItemKind::Trait(..)) { return; diff --git a/clippy_lints/src/new_without_default.rs b/clippy_lints/src/new_without_default.rs index 029a8c4d3152..303ba1e68087 100644 --- a/clippy_lints/src/new_without_default.rs +++ b/clippy_lints/src/new_without_default.rs @@ -93,8 +93,12 @@ pub struct NewWithoutDefault { impl_lint_pass!(NewWithoutDefault => [NEW_WITHOUT_DEFAULT]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault { + #[allow(clippy::too_many_lines)] fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item<'_>) { - if let hir::ItemKind::Impl(_, _, _, _, None, _, items) = item.kind { + if let hir::ItemKind::Impl { + of_trait: None, items, .. + } = item.kind + { for assoc_item in items { if let hir::AssocItemKind::Method { has_self: false } = assoc_item.kind { let impl_item = cx.tcx.hir().impl_item(assoc_item.id); diff --git a/clippy_lints/src/non_copy_const.rs b/clippy_lints/src/non_copy_const.rs index d977bb1529e2..48ffc113546f 100644 --- a/clippy_lints/src/non_copy_const.rs +++ b/clippy_lints/src/non_copy_const.rs @@ -168,7 +168,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonCopyConst { let item_hir_id = cx.tcx.hir().get_parent_node(impl_item.hir_id); let item = cx.tcx.hir().expect_item(item_hir_id); // Ensure the impl is an inherent impl. - if let ItemKind::Impl(_, _, _, _, None, _, _) = item.kind { + if let ItemKind::Impl { of_trait: None, .. } = item.kind { let ty = hir_ty_to_ty(cx.tcx, hir_ty); verify_ty_bound( cx, diff --git a/clippy_lints/src/partialeq_ne_impl.rs b/clippy_lints/src/partialeq_ne_impl.rs index b5ccfad8186d..dfd25f1c9db6 100644 --- a/clippy_lints/src/partialeq_ne_impl.rs +++ b/clippy_lints/src/partialeq_ne_impl.rs @@ -33,7 +33,7 @@ declare_lint_pass!(PartialEqNeImpl => [PARTIALEQ_NE_IMPL]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PartialEqNeImpl { fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) { if_chain! { - if let ItemKind::Impl(_, _, _, _, Some(ref trait_ref), _, impl_items) = item.kind; + if let ItemKind::Impl{ of_trait: Some(ref trait_ref), items: impl_items, .. } = item.kind; if !is_automatically_derived(&*item.attrs); if let Some(eq_trait) = cx.tcx.lang_items().eq_trait(); if trait_ref.path.res.def_id() == eq_trait; diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index 08a9824b8232..445f065aaced 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -110,7 +110,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Ptr { if let ImplItemKind::Method(ref sig, body_id) = item.kind { let parent_item = cx.tcx.hir().get_parent_item(item.hir_id); if let Some(Node::Item(it)) = cx.tcx.hir().find(parent_item) { - if let ItemKind::Impl(_, _, _, _, Some(_), _, _) = it.kind { + if let ItemKind::Impl { of_trait: Some(_), .. } = it.kind { return; // ignore trait impls } } diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index 19f88e770aed..1be597877213 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -45,6 +45,10 @@ declare_clippy_lint! { /// and ends with a closing one. /// I.e., `let _ = (f()+1)..(f()+1)` results in `let _ = ((f()+1)..=f())`. /// + /// Also in many cases, inclusive ranges are still slower to run than + /// exclusive ranges, because they essentially add an extra branch that + /// LLVM may fail to hoist out of the loop. + /// /// **Example:** /// ```rust,ignore /// for x..(y+1) { .. } @@ -54,7 +58,7 @@ declare_clippy_lint! { /// for x..=y { .. } /// ``` pub RANGE_PLUS_ONE, - complexity, + pedantic, "`x..(y+1)` reads better as `x..=y`" } diff --git a/clippy_lints/src/serde_api.rs b/clippy_lints/src/serde_api.rs index 3f6ff39613a7..63fdd599b51e 100644 --- a/clippy_lints/src/serde_api.rs +++ b/clippy_lints/src/serde_api.rs @@ -22,7 +22,12 @@ declare_lint_pass!(SerdeAPI => [SERDE_API_MISUSE]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for SerdeAPI { fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) { - if let ItemKind::Impl(_, _, _, _, Some(ref trait_ref), _, items) = item.kind { + if let ItemKind::Impl { + of_trait: Some(ref trait_ref), + items, + .. + } = item.kind + { let did = trait_ref.path.res.def_id(); if let Some(visit_did) = get_trait_def_id(cx, &paths::SERDE_DE_VISITOR) { if did == visit_did { diff --git a/clippy_lints/src/trivially_copy_pass_by_ref.rs b/clippy_lints/src/trivially_copy_pass_by_ref.rs index ac31757d17d5..1bacb4683c38 100644 --- a/clippy_lints/src/trivially_copy_pass_by_ref.rs +++ b/clippy_lints/src/trivially_copy_pass_by_ref.rs @@ -167,7 +167,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TriviallyCopyPassByRef { // Exclude non-inherent impls if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { - if matches!(item.kind, ItemKind::Impl(_, _, _, _, Some(_), _, _) | + if matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), .. } | ItemKind::Trait(..)) { return; diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 1a9c7502921d..52ced27c4775 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -181,7 +181,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Types { ) { // Skip trait implementations; see issue #605. if let Some(hir::Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_item(id)) { - if let ItemKind::Impl(_, _, _, _, Some(..), _, _) = item.kind { + if let ItemKind::Impl { of_trait: Some(_), .. } = item.kind { return; } } @@ -2106,7 +2106,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitHasher { } match item.kind { - ItemKind::Impl(_, _, _, ref generics, _, ref ty, ref items) => { + ItemKind::Impl { + ref generics, + self_ty: ref ty, + ref items, + .. + } => { let mut vis = ImplicitHasherTypeVisitor::new(cx); vis.visit_ty(ty); diff --git a/clippy_lints/src/unused_self.rs b/clippy_lints/src/unused_self.rs index caffb15cf810..247da580f451 100644 --- a/clippy_lints/src/unused_self.rs +++ b/clippy_lints/src/unused_self.rs @@ -2,7 +2,7 @@ use if_chain::if_chain; use rustc::hir::map::Map; use rustc_hir::def::Res; use rustc_hir::intravisit::{walk_path, NestedVisitorMap, Visitor}; -use rustc_hir::{AssocItemKind, HirId, ImplItemKind, ImplItemRef, Item, ItemKind, Path}; +use rustc_hir::{AssocItemKind, HirId, ImplItem, ImplItemKind, ImplItemRef, ItemKind, Path}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -40,21 +40,28 @@ declare_clippy_lint! { declare_lint_pass!(UnusedSelf => [UNUSED_SELF]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedSelf { - fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &Item<'_>) { - if item.span.from_expansion() { + fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, impl_item: &ImplItem<'_>) { + if impl_item.span.from_expansion() { return; } - if let ItemKind::Impl(_, _, _, _, None, _, impl_item_refs) = item.kind { + let parent = cx.tcx.hir().get_parent_item(impl_item.hir_id); + let item = cx.tcx.hir().expect_item(parent); + if let ItemKind::Impl { + of_trait: None, + items: impl_item_refs, + .. + } = item.kind + { for impl_item_ref in impl_item_refs { if_chain! { if let ImplItemRef { kind: AssocItemKind::Method { has_self: true }, .. } = impl_item_ref; - let impl_item = cx.tcx.hir().impl_item(impl_item_ref.id); if let ImplItemKind::Method(_, body_id) = &impl_item.kind; + let body = cx.tcx.hir().body(*body_id); + if !body.params.is_empty(); then { - let body = cx.tcx.hir().body(*body_id); let self_param = &body.params[0]; let self_hir_id = self_param.pat.hir_id; let mut visitor = UnusedSelfVisitor { @@ -70,7 +77,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedSelf { self_param.span, "unused `self` argument", "consider refactoring to a associated function", - ) + ); + return; } } } diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index 3762f0d0a4f6..4160370dd15a 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -173,7 +173,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseSelf { return; } if_chain! { - if let ItemKind::Impl(.., ref item_type, refs) = item.kind; + if let ItemKind::Impl{ self_ty: ref item_type, items: refs, .. } = item.kind; if let TyKind::Path(QPath::Resolved(_, ref item_path)) = item_type.kind; then { let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).args; @@ -269,7 +269,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> { | ItemKind::Enum(..) | ItemKind::Struct(..) | ItemKind::Union(..) - | ItemKind::Impl(..) + | ItemKind::Impl { .. } | ItemKind::Fn(..) => { // Don't check statements that shadow `Self` or where `Self` can't be used }, diff --git a/clippy_lints/src/utils/inspector.rs b/clippy_lints/src/utils/inspector.rs index 23bce8b0fd82..aa76ada7b484 100644 --- a/clippy_lints/src/utils/inspector.rs +++ b/clippy_lints/src/utils/inspector.rs @@ -388,10 +388,13 @@ fn print_item(cx: &LateContext<'_, '_>, item: &hir::Item<'_>) { hir::ItemKind::TraitAlias(..) => { println!("trait alias"); }, - hir::ItemKind::Impl(_, _, _, _, Some(ref _trait_ref), _, _) => { + hir::ItemKind::Impl { + of_trait: Some(ref _trait_ref), + .. + } => { println!("trait impl"); }, - hir::ItemKind::Impl(_, _, _, _, None, _, _) => { + hir::ItemKind::Impl { of_trait: None, .. } => { println!("impl"); }, } diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 825e796382b5..1723f7863b30 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -13,10 +13,10 @@ use rustc_hir::*; use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass}; use rustc_session::declare_tool_lint; use rustc_session::{declare_lint_pass, impl_lint_pass}; -use rustc_span::source_map::Span; +use rustc_span::source_map::{Span, Spanned}; use rustc_span::symbol::SymbolStr; use syntax::ast; -use syntax::ast::{Crate as AstCrate, ItemKind, Name}; +use syntax::ast::{Crate as AstCrate, ItemKind, LitKind, Name}; use syntax::visit::FnKind; declare_clippy_lint! { @@ -121,6 +121,29 @@ declare_clippy_lint! { "this message should not appear anywhere as we ICE before and don't emit the lint" } +declare_clippy_lint! { + /// **What it does:** Checks for cases of an auto-generated lint without an updated description, + /// i.e. `default lint description`. + /// + /// **Why is this bad?** Indicates that the lint is not finished. + /// + /// **Known problems:** None + /// + /// **Example:** + /// Bad: + /// ```rust,ignore + /// declare_lint! { pub COOL_LINT, nursery, "default lint description" } + /// ``` + /// + /// Good: + /// ```rust,ignore + /// declare_lint! { pub COOL_LINT, nursery, "a great new lint" } + /// ``` + pub DEFAULT_LINT, + internal, + "found 'default lint description' in a lint declaration" +} + declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]); impl EarlyLintPass for ClippyLintsInternal { @@ -163,18 +186,45 @@ pub struct LintWithoutLintPass { registered_lints: FxHashSet, } -impl_lint_pass!(LintWithoutLintPass => [LINT_WITHOUT_LINT_PASS]); +impl_lint_pass!(LintWithoutLintPass => [DEFAULT_LINT, LINT_WITHOUT_LINT_PASS]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LintWithoutLintPass { fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) { - if let hir::ItemKind::Static(ref ty, Mutability::Not, _) = item.kind { + if let hir::ItemKind::Static(ref ty, Mutability::Not, body_id) = item.kind { if is_lint_ref_type(cx, ty) { + let expr = &cx.tcx.hir().body(body_id).value; + if_chain! { + if let ExprKind::AddrOf(_, _, ref inner_exp) = expr.kind; + if let ExprKind::Struct(_, ref fields, _) = inner_exp.kind; + let field = fields.iter() + .find(|f| f.ident.as_str() == "desc") + .expect("lints must have a description field"); + if let ExprKind::Lit(Spanned { + node: LitKind::Str(ref sym, _), + .. + }) = field.expr.kind; + if sym.as_str() == "default lint description"; + + then { + span_lint( + cx, + DEFAULT_LINT, + item.span, + &format!("the lint `{}` has the default lint description", item.ident.name), + ); + } + } self.declared_lints.insert(item.ident.name, item.span); } } else if is_expn_of(item.span, "impl_lint_pass").is_some() || is_expn_of(item.span, "declare_lint_pass").is_some() { - if let hir::ItemKind::Impl(.., None, _, ref impl_item_refs) = item.kind { + if let hir::ItemKind::Impl { + of_trait: None, + items: ref impl_item_refs, + .. + } = item.kind + { let mut collector = LintCollector { output: &mut self.registered_lints, cx, diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index eca2baa8c809..9150d189f238 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -357,7 +357,7 @@ pub fn trait_ref_of_method<'tcx>(cx: &LateContext<'_, 'tcx>, hir_id: HirId) -> O if_chain! { if parent_impl != hir::CRATE_HIR_ID; if let hir::Node::Item(item) = cx.tcx.hir().get(parent_impl); - if let hir::ItemKind::Impl(_, _, _, _, trait_ref, _, _) = &item.kind; + if let hir::ItemKind::Impl{ of_trait: trait_ref, .. } = &item.kind; then { return trait_ref.as_ref(); } } None diff --git a/doc/adding_lints.md b/doc/adding_lints.md index 45b80a6b0c12..6fd052893bfc 100644 --- a/doc/adding_lints.md +++ b/doc/adding_lints.md @@ -9,6 +9,7 @@ because that's clearly a non-descriptive name. - [Adding a new lint](#adding-a-new-lint) - [Setup](#setup) + - [Getting Started](#getting-started) - [Testing](#testing) - [Rustfix tests](#rustfix-tests) - [Edition 2018 tests](#edition-2018-tests) @@ -31,6 +32,19 @@ which can change rapidly. Make sure you're working near rust-clippy's master, and use the `setup-toolchain.sh` script to configure the appropriate toolchain for the Clippy directory. +### Getting Started + +There is a bit of boilerplate code that needs to be set up when creating a new +lint. Fortunately, you can use the clippy dev tools to handle this for you. We +are naming our new lint `foo_functions` (lints are generally written in snake +case), and we don't need type information so it will have an early pass type +(more on this later on). To get started on this lint you can run +`./util/dev new_lint --name=foo_functions --pass=early --category=pedantic` +(category will default to nursery if not provided). This command will create +two files: `tests/ui/foo_functions.rs` and `clippy_lints/src/foo_functions.rs`, +as well as run `./util/dev update_lints` to register the new lint. Next, we'll +open up these files and add our lint! + ### Testing Let's write some tests first that we can execute while we iterate on our lint. @@ -41,11 +55,9 @@ we want to check. The output of Clippy is compared against a `.stderr` file. Note that you don't have to create this file yourself, we'll get to generating the `.stderr` files further down. -We start by creating the test file at `tests/ui/foo_functions.rs`. It doesn't -really matter what the file is called, but it's a good convention to name it -after the lint it is testing, so `foo_functions.rs` it is. +We start by opening the test file created at `tests/ui/foo_functions.rs`. -Inside the file we put some examples to get started: +Update the file with some examples to get started: ```rust #![warn(clippy::foo_functions)] @@ -90,8 +102,8 @@ Once we are satisfied with the output, we need to run `tests/ui/update-all-references.sh` to update the `.stderr` file for our lint. Please note that, we should run `TESTNAME=foo_functions cargo uitest` every time before running `tests/ui/update-all-references.sh`. -Running `TESTNAME=foo_functions cargo uitest` should pass then. When we -commit our lint, we need to commit the generated `.stderr` files, too. +Running `TESTNAME=foo_functions cargo uitest` should pass then. When we commit +our lint, we need to commit the generated `.stderr` files, too. ### Rustfix tests @@ -121,26 +133,42 @@ With tests in place, let's have a look at implementing our lint now. ### Lint declaration -We start by creating a new file in the `clippy_lints` crate. That's the crate -where all the lint code is. We are going to call the file -`clippy_lints/src/foo_functions.rs` and import some initial things we need: +Let's start by opening the new file created in the `clippy_lints` crate +at `clippy_lints/src/foo_functions.rs`. That's the crate where all the +lint code is. This file has already imported some initial things we will need: ```rust -use rustc::lint::{LintArray, LintPass, EarlyLintPass}; +use rustc::lint::{LintArray, LintPass, EarlyLintPass, EarlyContext}; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use syntax::ast::*; ``` -The next step is to provide a lint declaration. Lints are declared using the -[`declare_clippy_lint!`][declare_clippy_lint] macro: +The next step is to update the lint declaration. Lints are declared using the +[`declare_clippy_lint!`][declare_clippy_lint] macro, and we just need to update +the auto-generated lint declaration to have a real description, something like this: ```rust declare_clippy_lint! { + /// **What it does:** + /// + /// **Why is this bad?** + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// // example code + /// ``` pub FOO_FUNCTIONS, pedantic, "function named `foo`, which is not a descriptive name" } ``` +* The section of lines prefixed with `///` constitutes the lint documentation +section. This is the default documentation style and will be displayed at +https://rust-lang.github.io/rust-clippy/master/index.html. * `FOO_FUNCTIONS` is the name of our lint. Be sure to follow the [lint naming guidelines][lint_naming] here when naming your lint. In short, the name should state the thing that is being checked for and read well when used with @@ -150,8 +178,8 @@ state the thing that is being checked for and read well when used with * The last part should be a text that explains what exactly is wrong with the code -With our lint declaration done, we will now make sure that it is assigned to a -lint pass: +The rest of this file contains an empty implementation for our lint pass, +which in this case is `EarlyLintPass` and should look like this: ```rust // clippy_lints/src/foo_functions.rs @@ -166,12 +194,9 @@ impl EarlyLintPass for FooFunctions {} Don't worry about the `name` method here. As long as it includes the name of the lint pass it should be fine. -Next we need to run `util/dev update_lints` to register the lint in various -places, mainly in `clippy_lints/src/lib.rs`. - -While `update_lints` automates some things, it doesn't automate everything. We -will have to register our lint pass manually in the `register_plugins` function -in `clippy_lints/src/lib.rs`: +The new lint automation runs `update_lints`, which automates some things, but it +doesn't automate everything. We will have to register our lint pass manually in +the `register_plugins` function in `clippy_lints/src/lib.rs`: ```rust reg.register_early_lint_pass(box foo_functions::FooFunctions); @@ -195,14 +220,9 @@ In short, the `LateLintPass` has access to type information while the `EarlyLintPass`. The `EarlyLintPass` is also faster. However linting speed hasn't really been a concern with Clippy so far. -Since we don't need type information for checking the function name, we are -going to use the `EarlyLintPass`. It has to be imported as well, changing our -imports to: - -```rust -use rustc::lint::{LintArray, LintPass, EarlyLintPass, EarlyContext}; -use rustc::{declare_tool_lint, lint_array}; -``` +Since we don't need type information for checking the function name, we used +`--pass=early` when running the new lint automation and all the imports were +added accordingly. ### Emitting a lint diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 9cad1ac786af..c6339daf2ebe 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -705,7 +705,7 @@ pub const ALL_LINTS: [Lint; 347] = [ group: "style", desc: "usage of `ok()` in `if let Some(pat)` statements is unnecessary, match on `Ok(pat)` instead", deprecation: None, - module: "ok_if_let", + module: "if_let_some_result", }, Lint { name: "if_not_else", @@ -1654,7 +1654,7 @@ pub const ALL_LINTS: [Lint; 347] = [ }, Lint { name: "range_plus_one", - group: "complexity", + group: "pedantic", desc: "`x..(y+1)` reads better as `x..=y`", deprecation: None, module: "ranges", diff --git a/tests/ui/booleans.stderr b/tests/ui/booleans.stderr deleted file mode 100644 index b83df6f81385..000000000000 --- a/tests/ui/booleans.stderr +++ /dev/null @@ -1,206 +0,0 @@ -error: this boolean expression contains a logic bug - --> $DIR/booleans.rs:10:13 - | -LL | let _ = a && b || a; - | ^^^^^^^^^^^ help: it would look like the following: `a` - | - = note: `-D clippy::logic-bug` implied by `-D warnings` -help: this expression can be optimized out by applying boolean operations to the outer expression - --> $DIR/booleans.rs:10:18 - | -LL | let _ = a && b || a; - | ^ - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:12:13 - | -LL | let _ = !true; - | ^^^^^ help: try: `false` - | - = note: `-D clippy::nonminimal-bool` implied by `-D warnings` - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:13:13 - | -LL | let _ = !false; - | ^^^^^^ help: try: `true` - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:14:13 - | -LL | let _ = !!a; - | ^^^ help: try: `a` - -error: this boolean expression contains a logic bug - --> $DIR/booleans.rs:15:13 - | -LL | let _ = false && a; - | ^^^^^^^^^^ help: it would look like the following: `false` - | -help: this expression can be optimized out by applying boolean operations to the outer expression - --> $DIR/booleans.rs:15:22 - | -LL | let _ = false && a; - | ^ - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:16:13 - | -LL | let _ = false || a; - | ^^^^^^^^^^ help: try: `a` - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:21:13 - | -LL | let _ = !(!a && b); - | ^^^^^^^^^^ help: try: `!b || a` - -error: this boolean expression contains a logic bug - --> $DIR/booleans.rs:31:13 - | -LL | let _ = a == b && a != b; - | ^^^^^^^^^^^^^^^^ help: it would look like the following: `false` - | -help: this expression can be optimized out by applying boolean operations to the outer expression - --> $DIR/booleans.rs:31:13 - | -LL | let _ = a == b && a != b; - | ^^^^^^ - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:32:13 - | -LL | let _ = a == b && c == 5 && a == b; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try - | -LL | let _ = a == b && c == 5; - | ^^^^^^^^^^^^^^^^ -LL | let _ = !(c != 5 || a != b); - | ^^^^^^^^^^^^^^^^^^^ - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:33:13 - | -LL | let _ = a == b && c == 5 && b == a; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try - | -LL | let _ = a == b && c == 5; - | ^^^^^^^^^^^^^^^^ -LL | let _ = !(c != 5 || a != b); - | ^^^^^^^^^^^^^^^^^^^ - -error: this boolean expression contains a logic bug - --> $DIR/booleans.rs:34:13 - | -LL | let _ = a < b && a >= b; - | ^^^^^^^^^^^^^^^ help: it would look like the following: `false` - | -help: this expression can be optimized out by applying boolean operations to the outer expression - --> $DIR/booleans.rs:34:13 - | -LL | let _ = a < b && a >= b; - | ^^^^^ - -error: this boolean expression contains a logic bug - --> $DIR/booleans.rs:35:13 - | -LL | let _ = a > b && a <= b; - | ^^^^^^^^^^^^^^^ help: it would look like the following: `false` - | -help: this expression can be optimized out by applying boolean operations to the outer expression - --> $DIR/booleans.rs:35:13 - | -LL | let _ = a > b && a <= b; - | ^^^^^ - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:37:13 - | -LL | let _ = a != b || !(a != b || c == d); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try - | -LL | let _ = c != d || a != b; - | ^^^^^^^^^^^^^^^^ -LL | let _ = !(a == b && c == d); - | ^^^^^^^^^^^^^^^^^^^ - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:45:13 - | -LL | let _ = !a.is_some(); - | ^^^^^^^^^^^^ help: try: `a.is_none()` - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:47:13 - | -LL | let _ = !a.is_none(); - | ^^^^^^^^^^^^ help: try: `a.is_some()` - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:49:13 - | -LL | let _ = !b.is_err(); - | ^^^^^^^^^^^ help: try: `b.is_ok()` - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:51:13 - | -LL | let _ = !b.is_ok(); - | ^^^^^^^^^^ help: try: `b.is_err()` - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:53:13 - | -LL | let _ = !(a.is_some() && !c); - | ^^^^^^^^^^^^^^^^^^^^ help: try: `c || a.is_none()` - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:54:26 - | -LL | let _ = !(!c ^ c) || !a.is_some(); - | ^^^^^^^^^^^^ help: try: `a.is_none()` - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:55:25 - | -LL | let _ = (!c ^ c) || !a.is_some(); - | ^^^^^^^^^^^^ help: try: `a.is_none()` - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:56:23 - | -LL | let _ = !c ^ c || !a.is_some(); - | ^^^^^^^^^^^^ help: try: `a.is_none()` - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:128:8 - | -LL | if !res.is_ok() {} - | ^^^^^^^^^^^^ help: try: `res.is_err()` - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:129:8 - | -LL | if !res.is_err() {} - | ^^^^^^^^^^^^^ help: try: `res.is_ok()` - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:132:8 - | -LL | if !res.is_some() {} - | ^^^^^^^^^^^^^^ help: try: `res.is_none()` - -error: this boolean expression can be simplified - --> $DIR/booleans.rs:133:8 - | -LL | if !res.is_none() {} - | ^^^^^^^^^^^^^^ help: try: `res.is_some()` - -error: aborting due to 25 previous errors - diff --git a/tests/ui/default_lint.rs b/tests/ui/default_lint.rs new file mode 100644 index 000000000000..988a7b866dea --- /dev/null +++ b/tests/ui/default_lint.rs @@ -0,0 +1,28 @@ +#![deny(clippy::internal)] +#![feature(rustc_private)] + +#[macro_use] +extern crate rustc; +#[macro_use] +extern crate rustc_session; +extern crate rustc_lint; +use rustc_lint::{LintArray, LintPass}; + +declare_tool_lint! { + pub clippy::TEST_LINT, + Warn, + "", + report_in_external_macro: true +} + +declare_tool_lint! { + pub clippy::TEST_LINT_DEFAULT, + Warn, + "default lint description", + report_in_external_macro: true +} + +declare_lint_pass!(Pass => [TEST_LINT]); +declare_lint_pass!(Pass2 => [TEST_LINT_DEFAULT]); + +fn main() {} diff --git a/tests/ui/default_lint.stderr b/tests/ui/default_lint.stderr new file mode 100644 index 000000000000..21fd86ec6aad --- /dev/null +++ b/tests/ui/default_lint.stderr @@ -0,0 +1,21 @@ +error: the lint `TEST_LINT_DEFAULT` has the default lint description + --> $DIR/default_lint.rs:18:1 + | +LL | / declare_tool_lint! { +LL | | pub clippy::TEST_LINT_DEFAULT, +LL | | Warn, +LL | | "default lint description", +LL | | report_in_external_macro: true +LL | | } + | |_^ + | +note: lint level defined here + --> $DIR/default_lint.rs:1:9 + | +LL | #![deny(clippy::internal)] + | ^^^^^^^^^^^^^^^^ + = note: `#[deny(clippy::default_lint)]` implied by `#[deny(clippy::internal)]` + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +error: aborting due to previous error + diff --git a/tests/ui/if_let_some_result.fixed b/tests/ui/if_let_some_result.fixed new file mode 100644 index 000000000000..80505fd997f4 --- /dev/null +++ b/tests/ui/if_let_some_result.fixed @@ -0,0 +1,35 @@ +// run-rustfix + +#![warn(clippy::if_let_some_result)] + +fn str_to_int(x: &str) -> i32 { + if let Ok(y) = x.parse() { + y + } else { + 0 + } +} + +fn str_to_int_ok(x: &str) -> i32 { + if let Ok(y) = x.parse() { + y + } else { + 0 + } +} + +#[rustfmt::skip] +fn strange_some_no_else(x: &str) -> i32 { + { + if let Ok(y) = x . parse() { + return y; + }; + 0 + } +} + +fn main() { + let _ = str_to_int("1"); + let _ = str_to_int_ok("2"); + let _ = strange_some_no_else("3"); +} diff --git a/tests/ui/if_let_some_result.rs b/tests/ui/if_let_some_result.rs new file mode 100644 index 000000000000..ecac13574456 --- /dev/null +++ b/tests/ui/if_let_some_result.rs @@ -0,0 +1,35 @@ +// run-rustfix + +#![warn(clippy::if_let_some_result)] + +fn str_to_int(x: &str) -> i32 { + if let Some(y) = x.parse().ok() { + y + } else { + 0 + } +} + +fn str_to_int_ok(x: &str) -> i32 { + if let Ok(y) = x.parse() { + y + } else { + 0 + } +} + +#[rustfmt::skip] +fn strange_some_no_else(x: &str) -> i32 { + { + if let Some(y) = x . parse() . ok () { + return y; + }; + 0 + } +} + +fn main() { + let _ = str_to_int("1"); + let _ = str_to_int_ok("2"); + let _ = strange_some_no_else("3"); +} diff --git a/tests/ui/if_let_some_result.stderr b/tests/ui/if_let_some_result.stderr new file mode 100644 index 000000000000..334ccb010167 --- /dev/null +++ b/tests/ui/if_let_some_result.stderr @@ -0,0 +1,25 @@ +error: Matching on `Some` with `ok()` is redundant + --> $DIR/if_let_some_result.rs:6:5 + | +LL | if let Some(y) = x.parse().ok() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::if-let-some-result` implied by `-D warnings` +help: Consider matching on `Ok(y)` and removing the call to `ok` instead + | +LL | if let Ok(y) = x.parse() { + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: Matching on `Some` with `ok()` is redundant + --> $DIR/if_let_some_result.rs:24:9 + | +LL | if let Some(y) = x . parse() . ok () { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: Consider matching on `Ok(y)` and removing the call to `ok` instead + | +LL | if let Ok(y) = x . parse() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/tests/ui/if_same_then_else.rs b/tests/ui/if_same_then_else.rs index ecdc5623ca58..67b4c311085e 100644 --- a/tests/ui/if_same_then_else.rs +++ b/tests/ui/if_same_then_else.rs @@ -1,14 +1,11 @@ #![warn(clippy::if_same_then_else)] #![allow( clippy::blacklisted_name, - clippy::collapsible_if, - clippy::cognitive_complexity, clippy::eq_op, - clippy::needless_return, clippy::never_loop, clippy::no_effect, - clippy::zero_divided_by_zero, - clippy::unused_unit + clippy::unused_unit, + clippy::zero_divided_by_zero )] struct Foo { @@ -19,7 +16,7 @@ fn foo() -> bool { unimplemented!() } -fn if_same_then_else() -> Result<&'static str, ()> { +fn if_same_then_else() { if true { Foo { bar: 42 }; 0..10; @@ -94,27 +91,6 @@ fn if_same_then_else() -> Result<&'static str, ()> { 42 }; - if true { - for _ in &[42] { - let foo: &Option<_> = &Some::(42); - if true { - break; - } else { - continue; - } - } - } else { - //~ ERROR same body as `if` block - for _ in &[42] { - let foo: &Option<_> = &Some::(42); - if true { - break; - } else { - continue; - } - } - } - if true { let bar = if true { 42 } else { 43 }; @@ -149,113 +125,6 @@ fn if_same_then_else() -> Result<&'static str, ()> { _ => 4, }; } - - if true { - if let Some(a) = Some(42) {} - } else { - //~ ERROR same body as `if` block - if let Some(a) = Some(42) {} - } - - if true { - if let (1, .., 3) = (1, 2, 3) {} - } else { - //~ ERROR same body as `if` block - if let (1, .., 3) = (1, 2, 3) {} - } - - if true { - if let (1, .., 3) = (1, 2, 3) {} - } else { - if let (.., 3) = (1, 2, 3) {} - } - - if true { - if let (1, .., 3) = (1, 2, 3) {} - } else { - if let (.., 4) = (1, 2, 3) {} - } - - if true { - if let (1, .., 3) = (1, 2, 3) {} - } else { - if let (.., 1, 3) = (1, 2, 3) {} - } - - if true { - if let Some(42) = None {} - } else { - if let Option::Some(42) = None {} - } - - if true { - if let Some(42) = None:: {} - } else { - if let Some(42) = None {} - } - - if true { - if let Some(42) = None:: {} - } else { - if let Some(42) = None:: {} - } - - if true { - if let Some(a) = Some(42) {} - } else { - if let Some(a) = Some(43) {} - } - - // Same NaNs - let _ = if true { - std::f32::NAN - } else { - //~ ERROR same body as `if` block - std::f32::NAN - }; - - if true { - Ok("foo")?; - } else { - //~ ERROR same body as `if` block - Ok("foo")?; - } - - if true { - let foo = ""; - return Ok(&foo[0..]); - } else if false { - let foo = "bar"; - return Ok(&foo[0..]); - } else { - let foo = ""; - return Ok(&foo[0..]); - } - - if true { - let foo = ""; - return Ok(&foo[0..]); - } else if false { - let foo = "bar"; - return Ok(&foo[0..]); - } else if true { - let foo = ""; - return Ok(&foo[0..]); - } else { - let foo = ""; - return Ok(&foo[0..]); - } - - // False positive `if_same_then_else`: `let (x, y)` vs. `let (y, x)`; see issue #3559. - if true { - let foo = ""; - let (x, y) = (1, 2); - return Ok(&foo[x..y]); - } else { - let foo = ""; - let (y, x) = (1, 2); - return Ok(&foo[x..y]); - } } // Issue #2423. This was causing an ICE. diff --git a/tests/ui/if_same_then_else.stderr b/tests/ui/if_same_then_else.stderr index 0c2a1fdb78a4..d9fdf06fa8b7 100644 --- a/tests/ui/if_same_then_else.stderr +++ b/tests/ui/if_same_then_else.stderr @@ -1,5 +1,5 @@ error: this `if` has identical blocks - --> $DIR/if_same_then_else.rs:31:12 + --> $DIR/if_same_then_else.rs:28:12 | LL | } else { | ____________^ @@ -13,7 +13,7 @@ LL | | } | = note: `-D clippy::if-same-then-else` implied by `-D warnings` note: same as this - --> $DIR/if_same_then_else.rs:23:13 + --> $DIR/if_same_then_else.rs:20:13 | LL | if true { | _____________^ @@ -26,7 +26,7 @@ LL | | } else { | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else.rs:69:12 + --> $DIR/if_same_then_else.rs:66:12 | LL | } else { | ____________^ @@ -36,7 +36,7 @@ LL | | }; | |_____^ | note: same as this - --> $DIR/if_same_then_else.rs:67:21 + --> $DIR/if_same_then_else.rs:64:21 | LL | let _ = if true { | _____________________^ @@ -45,7 +45,7 @@ LL | | } else { | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else.rs:76:12 + --> $DIR/if_same_then_else.rs:73:12 | LL | } else { | ____________^ @@ -55,7 +55,7 @@ LL | | }; | |_____^ | note: same as this - --> $DIR/if_same_then_else.rs:74:21 + --> $DIR/if_same_then_else.rs:71:21 | LL | let _ = if true { | _____________________^ @@ -64,7 +64,7 @@ LL | | } else { | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else.rs:92:12 + --> $DIR/if_same_then_else.rs:89:12 | LL | } else { | ____________^ @@ -74,7 +74,7 @@ LL | | }; | |_____^ | note: same as this - --> $DIR/if_same_then_else.rs:90:21 + --> $DIR/if_same_then_else.rs:87:21 | LL | let _ = if true { | _____________________^ @@ -83,33 +83,7 @@ LL | | } else { | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else.rs:106:12 - | -LL | } else { - | ____________^ -LL | | //~ ERROR same body as `if` block -LL | | for _ in &[42] { -LL | | let foo: &Option<_> = &Some::(42); -... | -LL | | } -LL | | } - | |_____^ - | -note: same as this - --> $DIR/if_same_then_else.rs:97:13 - | -LL | if true { - | _____________^ -LL | | for _ in &[42] { -LL | | let foo: &Option<_> = &Some::(42); -LL | | if true { -... | -LL | | } -LL | | } else { - | |_____^ - -error: this `if` has identical blocks - --> $DIR/if_same_then_else.rs:125:12 + --> $DIR/if_same_then_else.rs:101:12 | LL | } else { | ____________^ @@ -122,7 +96,7 @@ LL | | } | |_____^ | note: same as this - --> $DIR/if_same_then_else.rs:118:13 + --> $DIR/if_same_then_else.rs:94:13 | LL | if true { | _____________^ @@ -134,114 +108,5 @@ LL | | bar + 1; LL | | } else { | |_____^ -error: this `if` has identical blocks - --> $DIR/if_same_then_else.rs:155:12 - | -LL | } else { - | ____________^ -LL | | //~ ERROR same body as `if` block -LL | | if let Some(a) = Some(42) {} -LL | | } - | |_____^ - | -note: same as this - --> $DIR/if_same_then_else.rs:153:13 - | -LL | if true { - | _____________^ -LL | | if let Some(a) = Some(42) {} -LL | | } else { - | |_____^ - -error: this `if` has identical blocks - --> $DIR/if_same_then_else.rs:162:12 - | -LL | } else { - | ____________^ -LL | | //~ ERROR same body as `if` block -LL | | if let (1, .., 3) = (1, 2, 3) {} -LL | | } - | |_____^ - | -note: same as this - --> $DIR/if_same_then_else.rs:160:13 - | -LL | if true { - | _____________^ -LL | | if let (1, .., 3) = (1, 2, 3) {} -LL | | } else { - | |_____^ - -error: this `if` has identical blocks - --> $DIR/if_same_then_else.rs:212:12 - | -LL | } else { - | ____________^ -LL | | //~ ERROR same body as `if` block -LL | | std::f32::NAN -LL | | }; - | |_____^ - | -note: same as this - --> $DIR/if_same_then_else.rs:210:21 - | -LL | let _ = if true { - | _____________________^ -LL | | std::f32::NAN -LL | | } else { - | |_____^ - -error: this `if` has identical blocks - --> $DIR/if_same_then_else.rs:219:12 - | -LL | } else { - | ____________^ -LL | | //~ ERROR same body as `if` block -LL | | Ok("foo")?; -LL | | } - | |_____^ - | -note: same as this - --> $DIR/if_same_then_else.rs:217:13 - | -LL | if true { - | _____________^ -LL | | Ok("foo")?; -LL | | } else { - | |_____^ - -error: this `if` has identical blocks - --> $DIR/if_same_then_else.rs:244:12 - | -LL | } else { - | ____________^ -LL | | let foo = ""; -LL | | return Ok(&foo[0..]); -LL | | } - | |_____^ - | -note: same as this - --> $DIR/if_same_then_else.rs:241:20 - | -LL | } else if true { - | ____________________^ -LL | | let foo = ""; -LL | | return Ok(&foo[0..]); -LL | | } else { - | |_____^ - -error: this `if` has the same condition as a previous `if` - --> $DIR/if_same_then_else.rs:241:15 - | -LL | } else if true { - | ^^^^ - | - = note: `#[deny(clippy::ifs_same_cond)]` on by default -note: same as this - --> $DIR/if_same_then_else.rs:235:8 - | -LL | if true { - | ^^^^ - -error: aborting due to 12 previous errors +error: aborting due to 5 previous errors diff --git a/tests/ui/if_same_then_else2.rs b/tests/ui/if_same_then_else2.rs new file mode 100644 index 000000000000..8e61bf3830be --- /dev/null +++ b/tests/ui/if_same_then_else2.rs @@ -0,0 +1,140 @@ +#![warn(clippy::if_same_then_else)] +#![allow( + clippy::blacklisted_name, + clippy::cognitive_complexity, + clippy::collapsible_if, + clippy::ifs_same_cond, + clippy::needless_return +)] + +fn if_same_then_else2() -> Result<&'static str, ()> { + if true { + for _ in &[42] { + let foo: &Option<_> = &Some::(42); + if true { + break; + } else { + continue; + } + } + } else { + //~ ERROR same body as `if` block + for _ in &[42] { + let foo: &Option<_> = &Some::(42); + if true { + break; + } else { + continue; + } + } + } + + if true { + if let Some(a) = Some(42) {} + } else { + //~ ERROR same body as `if` block + if let Some(a) = Some(42) {} + } + + if true { + if let (1, .., 3) = (1, 2, 3) {} + } else { + //~ ERROR same body as `if` block + if let (1, .., 3) = (1, 2, 3) {} + } + + if true { + if let (1, .., 3) = (1, 2, 3) {} + } else { + if let (.., 3) = (1, 2, 3) {} + } + + if true { + if let (1, .., 3) = (1, 2, 3) {} + } else { + if let (.., 4) = (1, 2, 3) {} + } + + if true { + if let (1, .., 3) = (1, 2, 3) {} + } else { + if let (.., 1, 3) = (1, 2, 3) {} + } + + if true { + if let Some(42) = None {} + } else { + if let Option::Some(42) = None {} + } + + if true { + if let Some(42) = None:: {} + } else { + if let Some(42) = None {} + } + + if true { + if let Some(42) = None:: {} + } else { + if let Some(42) = None:: {} + } + + if true { + if let Some(a) = Some(42) {} + } else { + if let Some(a) = Some(43) {} + } + + // Same NaNs + let _ = if true { + std::f32::NAN + } else { + //~ ERROR same body as `if` block + std::f32::NAN + }; + + if true { + Ok("foo")?; + } else { + //~ ERROR same body as `if` block + Ok("foo")?; + } + + if true { + let foo = ""; + return Ok(&foo[0..]); + } else if false { + let foo = "bar"; + return Ok(&foo[0..]); + } else { + let foo = ""; + return Ok(&foo[0..]); + } + + if true { + let foo = ""; + return Ok(&foo[0..]); + } else if false { + let foo = "bar"; + return Ok(&foo[0..]); + } else if true { + let foo = ""; + return Ok(&foo[0..]); + } else { + let foo = ""; + return Ok(&foo[0..]); + } + + // False positive `if_same_then_else`: `let (x, y)` vs. `let (y, x)`; see issue #3559. + if true { + let foo = ""; + let (x, y) = (1, 2); + return Ok(&foo[x..y]); + } else { + let foo = ""; + let (y, x) = (1, 2); + return Ok(&foo[x..y]); + } +} + +fn main() {} diff --git a/tests/ui/if_same_then_else2.stderr b/tests/ui/if_same_then_else2.stderr new file mode 100644 index 000000000000..c6da3c6be645 --- /dev/null +++ b/tests/ui/if_same_then_else2.stderr @@ -0,0 +1,125 @@ +error: this `if` has identical blocks + --> $DIR/if_same_then_else2.rs:20:12 + | +LL | } else { + | ____________^ +LL | | //~ ERROR same body as `if` block +LL | | for _ in &[42] { +LL | | let foo: &Option<_> = &Some::(42); +... | +LL | | } +LL | | } + | |_____^ + | + = note: `-D clippy::if-same-then-else` implied by `-D warnings` +note: same as this + --> $DIR/if_same_then_else2.rs:11:13 + | +LL | if true { + | _____________^ +LL | | for _ in &[42] { +LL | | let foo: &Option<_> = &Some::(42); +LL | | if true { +... | +LL | | } +LL | | } else { + | |_____^ + +error: this `if` has identical blocks + --> $DIR/if_same_then_else2.rs:34:12 + | +LL | } else { + | ____________^ +LL | | //~ ERROR same body as `if` block +LL | | if let Some(a) = Some(42) {} +LL | | } + | |_____^ + | +note: same as this + --> $DIR/if_same_then_else2.rs:32:13 + | +LL | if true { + | _____________^ +LL | | if let Some(a) = Some(42) {} +LL | | } else { + | |_____^ + +error: this `if` has identical blocks + --> $DIR/if_same_then_else2.rs:41:12 + | +LL | } else { + | ____________^ +LL | | //~ ERROR same body as `if` block +LL | | if let (1, .., 3) = (1, 2, 3) {} +LL | | } + | |_____^ + | +note: same as this + --> $DIR/if_same_then_else2.rs:39:13 + | +LL | if true { + | _____________^ +LL | | if let (1, .., 3) = (1, 2, 3) {} +LL | | } else { + | |_____^ + +error: this `if` has identical blocks + --> $DIR/if_same_then_else2.rs:91:12 + | +LL | } else { + | ____________^ +LL | | //~ ERROR same body as `if` block +LL | | std::f32::NAN +LL | | }; + | |_____^ + | +note: same as this + --> $DIR/if_same_then_else2.rs:89:21 + | +LL | let _ = if true { + | _____________________^ +LL | | std::f32::NAN +LL | | } else { + | |_____^ + +error: this `if` has identical blocks + --> $DIR/if_same_then_else2.rs:98:12 + | +LL | } else { + | ____________^ +LL | | //~ ERROR same body as `if` block +LL | | Ok("foo")?; +LL | | } + | |_____^ + | +note: same as this + --> $DIR/if_same_then_else2.rs:96:13 + | +LL | if true { + | _____________^ +LL | | Ok("foo")?; +LL | | } else { + | |_____^ + +error: this `if` has identical blocks + --> $DIR/if_same_then_else2.rs:123:12 + | +LL | } else { + | ____________^ +LL | | let foo = ""; +LL | | return Ok(&foo[0..]); +LL | | } + | |_____^ + | +note: same as this + --> $DIR/if_same_then_else2.rs:120:20 + | +LL | } else if true { + | ____________________^ +LL | | let foo = ""; +LL | | return Ok(&foo[0..]); +LL | | } else { + | |_____^ + +error: aborting due to 6 previous errors + diff --git a/tests/ui/logic_bug.rs b/tests/ui/logic_bug.rs new file mode 100644 index 000000000000..b4163d776e73 --- /dev/null +++ b/tests/ui/logic_bug.rs @@ -0,0 +1,26 @@ +#![allow(unused, clippy::many_single_char_names)] +#![warn(clippy::logic_bug)] + +fn main() { + let a: bool = unimplemented!(); + let b: bool = unimplemented!(); + let c: bool = unimplemented!(); + let d: bool = unimplemented!(); + let e: bool = unimplemented!(); + let _ = a && b || a; + let _ = !(a && b); + let _ = false && a; + // don't lint on cfgs + let _ = cfg!(you_shall_not_not_pass) && a; + let _ = a || !b || !c || !d || !e; + let _ = !(a && b || c); +} + +fn equality_stuff() { + let a: i32 = unimplemented!(); + let b: i32 = unimplemented!(); + let _ = a == b && a != b; + let _ = a < b && a >= b; + let _ = a > b && a <= b; + let _ = a > b && a == b; +} diff --git a/tests/ui/logic_bug.stderr b/tests/ui/logic_bug.stderr new file mode 100644 index 000000000000..8f55e1c8ad85 --- /dev/null +++ b/tests/ui/logic_bug.stderr @@ -0,0 +1,63 @@ +error: this boolean expression contains a logic bug + --> $DIR/logic_bug.rs:10:13 + | +LL | let _ = a && b || a; + | ^^^^^^^^^^^ help: it would look like the following: `a` + | + = note: `-D clippy::logic-bug` implied by `-D warnings` +help: this expression can be optimized out by applying boolean operations to the outer expression + --> $DIR/logic_bug.rs:10:18 + | +LL | let _ = a && b || a; + | ^ + +error: this boolean expression contains a logic bug + --> $DIR/logic_bug.rs:12:13 + | +LL | let _ = false && a; + | ^^^^^^^^^^ help: it would look like the following: `false` + | +help: this expression can be optimized out by applying boolean operations to the outer expression + --> $DIR/logic_bug.rs:12:22 + | +LL | let _ = false && a; + | ^ + +error: this boolean expression contains a logic bug + --> $DIR/logic_bug.rs:22:13 + | +LL | let _ = a == b && a != b; + | ^^^^^^^^^^^^^^^^ help: it would look like the following: `false` + | +help: this expression can be optimized out by applying boolean operations to the outer expression + --> $DIR/logic_bug.rs:22:13 + | +LL | let _ = a == b && a != b; + | ^^^^^^ + +error: this boolean expression contains a logic bug + --> $DIR/logic_bug.rs:23:13 + | +LL | let _ = a < b && a >= b; + | ^^^^^^^^^^^^^^^ help: it would look like the following: `false` + | +help: this expression can be optimized out by applying boolean operations to the outer expression + --> $DIR/logic_bug.rs:23:13 + | +LL | let _ = a < b && a >= b; + | ^^^^^ + +error: this boolean expression contains a logic bug + --> $DIR/logic_bug.rs:24:13 + | +LL | let _ = a > b && a <= b; + | ^^^^^^^^^^^^^^^ help: it would look like the following: `false` + | +help: this expression can be optimized out by applying boolean operations to the outer expression + --> $DIR/logic_bug.rs:24:13 + | +LL | let _ = a > b && a <= b; + | ^^^^^ + +error: aborting due to 5 previous errors + diff --git a/tests/ui/nonminimal_bool.rs b/tests/ui/nonminimal_bool.rs new file mode 100644 index 000000000000..7ea154cb9b01 --- /dev/null +++ b/tests/ui/nonminimal_bool.rs @@ -0,0 +1,52 @@ +#![allow(unused, clippy::many_single_char_names)] +#![warn(clippy::nonminimal_bool)] + +fn main() { + let a: bool = unimplemented!(); + let b: bool = unimplemented!(); + let c: bool = unimplemented!(); + let d: bool = unimplemented!(); + let e: bool = unimplemented!(); + let _ = !true; + let _ = !false; + let _ = !!a; + let _ = false || a; + // don't lint on cfgs + let _ = cfg!(you_shall_not_not_pass) && a; + let _ = a || !b || !c || !d || !e; + let _ = !(!a && b); + let _ = !(!a || b); + let _ = !a && !(b && c); +} + +fn equality_stuff() { + let a: i32 = unimplemented!(); + let b: i32 = unimplemented!(); + let c: i32 = unimplemented!(); + let d: i32 = unimplemented!(); + let _ = a == b && c == 5 && a == b; + let _ = a == b || c == 5 || a == b; + let _ = a == b && c == 5 && b == a; + let _ = a != b || !(a != b || c == d); + let _ = a != b && !(a != b && c == d); +} + +fn issue3847(a: u32, b: u32) -> bool { + const THRESHOLD: u32 = 1_000; + + if a < THRESHOLD && b >= THRESHOLD || a >= THRESHOLD && b < THRESHOLD { + return false; + } + true +} + +fn issue4548() { + fn f(_i: u32, _j: u32) -> u32 { + unimplemented!(); + } + + let i = 0; + let j = 0; + + if i != j && f(i, j) != 0 || i == j && f(i, j) != 1 {} +} diff --git a/tests/ui/nonminimal_bool.stderr b/tests/ui/nonminimal_bool.stderr new file mode 100644 index 000000000000..d34d106cb2fb --- /dev/null +++ b/tests/ui/nonminimal_bool.stderr @@ -0,0 +1,111 @@ +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:10:13 + | +LL | let _ = !true; + | ^^^^^ help: try: `false` + | + = note: `-D clippy::nonminimal-bool` implied by `-D warnings` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:11:13 + | +LL | let _ = !false; + | ^^^^^^ help: try: `true` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:12:13 + | +LL | let _ = !!a; + | ^^^ help: try: `a` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:13:13 + | +LL | let _ = false || a; + | ^^^^^^^^^^ help: try: `a` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:17:13 + | +LL | let _ = !(!a && b); + | ^^^^^^^^^^ help: try: `a || !b` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:18:13 + | +LL | let _ = !(!a || b); + | ^^^^^^^^^^ help: try: `a && !b` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:19:13 + | +LL | let _ = !a && !(b && c); + | ^^^^^^^^^^^^^^^ help: try: `!(a || b && c)` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:27:13 + | +LL | let _ = a == b && c == 5 && a == b; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: try + | +LL | let _ = a == b && c == 5; + | ^^^^^^^^^^^^^^^^ +LL | let _ = !(a != b || c != 5); + | ^^^^^^^^^^^^^^^^^^^ + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:28:13 + | +LL | let _ = a == b || c == 5 || a == b; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: try + | +LL | let _ = a == b || c == 5; + | ^^^^^^^^^^^^^^^^ +LL | let _ = !(a != b && c != 5); + | ^^^^^^^^^^^^^^^^^^^ + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:29:13 + | +LL | let _ = a == b && c == 5 && b == a; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: try + | +LL | let _ = a == b && c == 5; + | ^^^^^^^^^^^^^^^^ +LL | let _ = !(a != b || c != 5); + | ^^^^^^^^^^^^^^^^^^^ + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:30:13 + | +LL | let _ = a != b || !(a != b || c == d); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: try + | +LL | let _ = a != b || c != d; + | ^^^^^^^^^^^^^^^^ +LL | let _ = !(a == b && c == d); + | ^^^^^^^^^^^^^^^^^^^ + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:31:13 + | +LL | let _ = a != b && !(a != b && c == d); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: try + | +LL | let _ = a != b && c != d; + | ^^^^^^^^^^^^^^^^ +LL | let _ = !(a == b || c == d); + | ^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 12 previous errors + diff --git a/tests/ui/booleans.rs b/tests/ui/nonminimal_bool_methods.rs similarity index 62% rename from tests/ui/booleans.rs rename to tests/ui/nonminimal_bool_methods.rs index ece20fb1eabb..4de48cd0879a 100644 --- a/tests/ui/booleans.rs +++ b/tests/ui/nonminimal_bool_methods.rs @@ -1,43 +1,6 @@ -#![warn(clippy::nonminimal_bool, clippy::logic_bug)] +#![allow(unused, clippy::many_single_char_names)] +#![warn(clippy::nonminimal_bool)] -#[allow(unused, clippy::many_single_char_names)] -fn main() { - let a: bool = unimplemented!(); - let b: bool = unimplemented!(); - let c: bool = unimplemented!(); - let d: bool = unimplemented!(); - let e: bool = unimplemented!(); - let _ = a && b || a; - let _ = !(a && b); - let _ = !true; - let _ = !false; - let _ = !!a; - let _ = false && a; - let _ = false || a; - // don't lint on cfgs - let _ = cfg!(you_shall_not_not_pass) && a; - let _ = a || !b || !c || !d || !e; - let _ = !(a && b || c); - let _ = !(!a && b); -} - -#[allow(unused, clippy::many_single_char_names)] -fn equality_stuff() { - let a: i32 = unimplemented!(); - let b: i32 = unimplemented!(); - let c: i32 = unimplemented!(); - let d: i32 = unimplemented!(); - let e: i32 = unimplemented!(); - let _ = a == b && a != b; - let _ = a == b && c == 5 && a == b; - let _ = a == b && c == 5 && b == a; - let _ = a < b && a >= b; - let _ = a > b && a <= b; - let _ = a > b && a == b; - let _ = a != b || !(a != b || c == d); -} - -#[allow(unused, clippy::many_single_char_names)] fn methods_with_negation() { let a: Option = unimplemented!(); let b: Result = unimplemented!(); @@ -51,6 +14,7 @@ fn methods_with_negation() { let _ = !b.is_ok(); let c = false; let _ = !(a.is_some() && !c); + let _ = !(a.is_some() || !c); let _ = !(!c ^ c) || !a.is_some(); let _ = (!c ^ c) || !a.is_some(); let _ = !c ^ c || !a.is_some(); @@ -143,22 +107,4 @@ fn dont_warn_for_negated_partial_ord_comparison() { let _ = !(a >= b); } -fn issue3847(a: u32, b: u32) -> bool { - const THRESHOLD: u32 = 1_000; - - if a < THRESHOLD && b >= THRESHOLD || a >= THRESHOLD && b < THRESHOLD { - return false; - } - true -} - -fn issue4548() { - fn f(_i: u32, _j: u32) -> u32 { - unimplemented!(); - } - - let i = 0; - let j = 0; - - if i != j && f(i, j) != 0 || i == j && f(i, j) != 1 {} -} +fn main() {} diff --git a/tests/ui/nonminimal_bool_methods.stderr b/tests/ui/nonminimal_bool_methods.stderr new file mode 100644 index 000000000000..a2df889d6230 --- /dev/null +++ b/tests/ui/nonminimal_bool_methods.stderr @@ -0,0 +1,82 @@ +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool_methods.rs:8:13 + | +LL | let _ = !a.is_some(); + | ^^^^^^^^^^^^ help: try: `a.is_none()` + | + = note: `-D clippy::nonminimal-bool` implied by `-D warnings` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool_methods.rs:10:13 + | +LL | let _ = !a.is_none(); + | ^^^^^^^^^^^^ help: try: `a.is_some()` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool_methods.rs:12:13 + | +LL | let _ = !b.is_err(); + | ^^^^^^^^^^^ help: try: `b.is_ok()` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool_methods.rs:14:13 + | +LL | let _ = !b.is_ok(); + | ^^^^^^^^^^ help: try: `b.is_err()` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool_methods.rs:16:13 + | +LL | let _ = !(a.is_some() && !c); + | ^^^^^^^^^^^^^^^^^^^^ help: try: `a.is_none() || c` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool_methods.rs:17:13 + | +LL | let _ = !(a.is_some() || !c); + | ^^^^^^^^^^^^^^^^^^^^ help: try: `a.is_none() && c` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool_methods.rs:18:26 + | +LL | let _ = !(!c ^ c) || !a.is_some(); + | ^^^^^^^^^^^^ help: try: `a.is_none()` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool_methods.rs:19:25 + | +LL | let _ = (!c ^ c) || !a.is_some(); + | ^^^^^^^^^^^^ help: try: `a.is_none()` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool_methods.rs:20:23 + | +LL | let _ = !c ^ c || !a.is_some(); + | ^^^^^^^^^^^^ help: try: `a.is_none()` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool_methods.rs:92:8 + | +LL | if !res.is_ok() {} + | ^^^^^^^^^^^^ help: try: `res.is_err()` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool_methods.rs:93:8 + | +LL | if !res.is_err() {} + | ^^^^^^^^^^^^^ help: try: `res.is_ok()` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool_methods.rs:96:8 + | +LL | if !res.is_some() {} + | ^^^^^^^^^^^^^^ help: try: `res.is_none()` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool_methods.rs:97:8 + | +LL | if !res.is_none() {} + | ^^^^^^^^^^^^^^ help: try: `res.is_some()` + +error: aborting due to 13 previous errors + diff --git a/tests/ui/ok_if_let.rs b/tests/ui/ok_if_let.rs deleted file mode 100644 index 61db31130527..000000000000 --- a/tests/ui/ok_if_let.rs +++ /dev/null @@ -1,23 +0,0 @@ -#![warn(clippy::if_let_some_result)] - -fn str_to_int(x: &str) -> i32 { - if let Some(y) = x.parse().ok() { - y - } else { - 0 - } -} - -fn str_to_int_ok(x: &str) -> i32 { - if let Ok(y) = x.parse() { - y - } else { - 0 - } -} - -fn main() { - let y = str_to_int("1"); - let z = str_to_int_ok("2"); - println!("{}{}", y, z); -} diff --git a/tests/ui/ok_if_let.stderr b/tests/ui/ok_if_let.stderr deleted file mode 100644 index e3e6c5c46343..000000000000 --- a/tests/ui/ok_if_let.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error: Matching on `Some` with `ok()` is redundant - --> $DIR/ok_if_let.rs:4:5 - | -LL | / if let Some(y) = x.parse().ok() { -LL | | y -LL | | } else { -LL | | 0 -LL | | } - | |_____^ - | - = note: `-D clippy::if-let-some-result` implied by `-D warnings` - = help: Consider matching on `Ok(y)` and removing the call to `ok` instead - -error: aborting due to previous error - diff --git a/tests/ui/patterns.fixed b/tests/ui/patterns.fixed index a443db7495dc..dfe27e193b97 100644 --- a/tests/ui/patterns.fixed +++ b/tests/ui/patterns.fixed @@ -1,7 +1,6 @@ // run-rustfix #![allow(unused)] #![warn(clippy::all)] -#![feature(slice_patterns)] fn main() { let v = Some(true); diff --git a/tests/ui/patterns.rs b/tests/ui/patterns.rs index 2c9f839ecf68..bd202fc04205 100644 --- a/tests/ui/patterns.rs +++ b/tests/ui/patterns.rs @@ -1,7 +1,6 @@ // run-rustfix #![allow(unused)] #![warn(clippy::all)] -#![feature(slice_patterns)] fn main() { let v = Some(true); diff --git a/tests/ui/patterns.stderr b/tests/ui/patterns.stderr index 784a3feaace6..f25e71e872b2 100644 --- a/tests/ui/patterns.stderr +++ b/tests/ui/patterns.stderr @@ -1,5 +1,5 @@ error: the `y @ _` pattern can be written as just `y` - --> $DIR/patterns.rs:11:9 + --> $DIR/patterns.rs:10:9 | LL | y @ _ => (), | ^^^^^ help: try: `y` diff --git a/tests/ui/unused_self.rs b/tests/ui/unused_self.rs index 9119c43c082c..711c1cd9d6c0 100644 --- a/tests/ui/unused_self.rs +++ b/tests/ui/unused_self.rs @@ -26,6 +26,24 @@ mod unused_self { } } +mod unused_self_allow { + struct A {} + + impl A { + // shouldn't trigger + #[allow(clippy::unused_self)] + fn unused_self_move(self) {} + } + + struct B {} + + // shouldn't trigger + #[allow(clippy::unused_self)] + impl B { + fn unused_self_move(self) {} + } +} + mod used_self { use std::pin::Pin;