diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ac9057199ff..a4a184480fba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,88 @@ document. ## Unreleased / In Rust Nightly -[891e1a8...master](https://github.com/rust-lang/rust-clippy/compare/891e1a8...master) +[7ea7cd1...master](https://github.com/rust-lang/rust-clippy/compare/7ea7cd1...master) + +## Rust 1.45 + +Current beta, release 2020-07-16 + +[891e1a8...7ea7cd1](https://github.com/rust-lang/rust-clippy/compare/891e1a8...7ea7cd1) + +### New lints + +* [`match_wildcard_for_single_variants`] [#5582](https://github.com/rust-lang/rust-clippy/pull/5582) +* [`unsafe_derive_deserialize`] [#5493](https://github.com/rust-lang/rust-clippy/pull/5493) +* [`if_let_mutex`] [#5332](https://github.com/rust-lang/rust-clippy/pull/5332) +* [`mismatched_target_os`] [#5506](https://github.com/rust-lang/rust-clippy/pull/5506) +* [`await_holding_lock`] [#5439](https://github.com/rust-lang/rust-clippy/pull/5439) +* [`match_on_vec_items`] [#5522](https://github.com/rust-lang/rust-clippy/pull/5522) +* [`manual_async_fn`] [#5576](https://github.com/rust-lang/rust-clippy/pull/5576) +* [`reversed_empty_ranges`] [#5583](https://github.com/rust-lang/rust-clippy/pull/5583) +* [`manual_non_exhaustive`] [#5550](https://github.com/rust-lang/rust-clippy/pull/5550) + +### Moves and Deprecations + +* Downgrade [`match_bool`] to pedantic [#5408](https://github.com/rust-lang/rust-clippy/pull/5408) +* Downgrade [`match_wild_err_arm`] to pedantic and update help messages. [#5622](https://github.com/rust-lang/rust-clippy/pull/5622) +* Downgrade [`useless_let_if_seq`] to nursery. [#5599](https://github.com/rust-lang/rust-clippy/pull/5599) +* Generalize `option_and_then_some` and rename to [`bind_instead_of_map`]. [#5529](https://github.com/rust-lang/rust-clippy/pull/5529) +* Rename `identity_conversion` to [`useless_conversion`]. [#5568](https://github.com/rust-lang/rust-clippy/pull/5568) +* Merge `block_in_if_condition_expr` and `block_in_if_condition_stmt` into [`blocks_in_if_conditions`]. +[#5563](https://github.com/rust-lang/rust-clippy/pull/5563) +* Merge `option_map_unwrap_or`, `option_map_unwrap_or_else` and `result_map_unwrap_or_else` into [`map_unwrap_or`]. +[#5563](https://github.com/rust-lang/rust-clippy/pull/5563) +* Merge `option_unwrap_used` and `result_unwrap_used` into [`unwrap_used`]. +[#5563](https://github.com/rust-lang/rust-clippy/pull/5563) +* Merge `option_expect_used` and `result_expect_used` into [`expect_used`]. +[#5563](https://github.com/rust-lang/rust-clippy/pull/5563) +* Merge `for_loop_over_option` and `for_loop_over_result` into [`for_loops_over_fallibles`]. +[#5563](https://github.com/rust-lang/rust-clippy/pull/5563) + +### Enhancements + +* Avoid running cargo lints when not enabled to improve performance. [#5505](https://github.com/rust-lang/rust-clippy/pull/5505) +* Extend [`useless_conversion`] with `TryFrom` and `TryInto`. [#5631](https://github.com/rust-lang/rust-clippy/pull/5631) +* Lint also in type parameters and where clauses in [`unused_unit`]. [#5592](https://github.com/rust-lang/rust-clippy/pull/5592) +* Do not suggest deriving `Default` in [`new_without_default`]. [#5616](https://github.com/rust-lang/rust-clippy/pull/5616) + +### False Positive Fixes + +* [`while_let_on_iterator`] [#5525](https://github.com/rust-lang/rust-clippy/pull/5525) +* [`empty_line_after_outer_attr`] [#5609](https://github.com/rust-lang/rust-clippy/pull/5609) +* [`unnecessary_unwrap`] [#5558](https://github.com/rust-lang/rust-clippy/pull/5558) +* [`comparison_chain`] [#5596](https://github.com/rust-lang/rust-clippy/pull/5596) +* Don't trigger [`used_underscore_binding`] in await desugaring. [#5535](https://github.com/rust-lang/rust-clippy/pull/5535) +* Don't trigger [`borrowed_box`] on mutable references. [#5491](https://github.com/rust-lang/rust-clippy/pull/5491) +* Allow `1 << 0` in [`identity_op`]. [#5602](https://github.com/rust-lang/rust-clippy/pull/5602) +* Allow `use super::*;` glob imports in [`wildcard_imports`]. [#5564](https://github.com/rust-lang/rust-clippy/pull/5564) +* Whitelist more words in [`doc_markdown`]. [#5611](https://github.com/rust-lang/rust-clippy/pull/5611) +* Skip dev and build deps in [`multiple_crate_versions`]. [#5636](https://github.com/rust-lang/rust-clippy/pull/5636) +* Honor `allow` attribute on arguments in [`ptr_arg`]. [#5647](https://github.com/rust-lang/rust-clippy/pull/5647) +* Honor lint level attributes for [`redundant_field_names`], [`just_underscores_and_digits`], [`many_single_char_names`] +and [`similar_names`]. [#5651](https://github.com/rust-lang/rust-clippy/pull/5651) +* Ignore calls to `len` in [`or_fun_call`]. [#4429](https://github.com/rust-lang/rust-clippy/pull/4429) + +### Suggestion Improvements + +* Simplify suggestions in [`manual_memcpy`]. [#5536](https://github.com/rust-lang/rust-clippy/pull/5536) +* Fix suggestion in [`redundant_pattern_matching`] for macros. [#5511](https://github.com/rust-lang/rust-clippy/pull/5511) +* Avoid suggesting `copied()` for mutable references in [`map_clone`]. [#5530](https://github.com/rust-lang/rust-clippy/pull/5530) +* Improve help message for [`clone_double_ref`]. [#5547](https://github.com/rust-lang/rust-clippy/pull/5547) + +### ICE Fixes + +* Fix ICE caused in unwrap module. [#5590](https://github.com/rust-lang/rust-clippy/pull/5590) +* Fix ICE on rustc test issue-69020-assoc-const-arith-overflow.rs [#5499](https://github.com/rust-lang/rust-clippy/pull/5499) + +### Documentation + +* Clarify the documentation of [`unnecessary_mut_passed`]. [#5639](https://github.com/rust-lang/rust-clippy/pull/5639) +* Extend example for [`unneeded_field_pattern`]. [#5541](https://github.com/rust-lang/rust-clippy/pull/5541) ## Rust 1.44 -Current beta, release 2020-06-04 +Current stable, released 2020-06-04 [204bb9b...891e1a8](https://github.com/rust-lang/rust-clippy/compare/204bb9b...891e1a8) @@ -93,7 +170,7 @@ Current beta, release 2020-06-04 ## Rust 1.43 -Current stable, released 2020-04-23 +Released 2020-04-23 [4ee1206...204bb9b](https://github.com/rust-lang/rust-clippy/compare/4ee1206...204bb9b) @@ -1401,6 +1478,7 @@ Released 2018-09-13 [`items_after_statements`]: https://rust-lang.github.io/rust-clippy/master/index.html#items_after_statements [`iter_cloned_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_cloned_collect [`iter_next_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_loop +[`iter_next_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_slice [`iter_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth [`iter_nth_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth_zero [`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next @@ -1601,6 +1679,7 @@ Released 2018-09-13 [`unnecessary_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fold [`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation +[`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by [`unnecessary_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap [`unneeded_field_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_field_pattern [`unneeded_wildcard_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_wildcard_pattern @@ -1630,6 +1709,7 @@ Released 2018-09-13 [`useless_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_transmute [`useless_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_vec [`vec_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_box +[`vec_resize_to_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_resize_to_zero [`verbose_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_bit_mask [`verbose_file_reads`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_file_reads [`vtable_address_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#vtable_address_comparisons diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0f47ac98fd20..9f7bdcb1be7e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -12,14 +12,16 @@ anything, feel free to ask questions on issues or visit the `#clippy` on [Discor All contributors are expected to follow the [Rust Code of Conduct]. -* [Getting started](#getting-started) - * [Finding something to fix/improve](#finding-something-to-fiximprove) -* [Writing code](#writing-code) -* [How Clippy works](#how-clippy-works) -* [Fixing nightly build failures](#fixing-build-failures-caused-by-rust) -* [Issue and PR Triage](#issue-and-pr-triage) -* [Bors and Homu](#bors-and-homu) -* [Contributions](#contributions) +- [Contributing to Clippy](#contributing-to-clippy) + - [Getting started](#getting-started) + - [Finding something to fix/improve](#finding-something-to-fiximprove) + - [Writing code](#writing-code) + - [Getting code-completion for rustc internals to work](#getting-code-completion-for-rustc-internals-to-work) + - [How Clippy works](#how-clippy-works) + - [Fixing build failures caused by Rust](#fixing-build-failures-caused-by-rust) + - [Issue and PR triage](#issue-and-pr-triage) + - [Bors and Homu](#bors-and-homu) + - [Contributions](#contributions) [Discord]: https://discord.gg/rust-lang [Rust Code of Conduct]: https://www.rust-lang.org/policies/code-of-conduct @@ -91,6 +93,24 @@ quick read. [rfc_stability]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#stability-guarantees [rfc_lint_cats]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#lint-audit-and-categories +## Getting code-completion for rustc internals to work + +Unfortunately, [`rust-analyzer`][ra_homepage] does not (yet?) understand how Clippy uses compiler-internals +using `extern crate` and it also needs to be able to read the source files of the rustc-compiler which are not +available via a `rustup` component at the time of writing. +To work around this, you need to have a copy of the [rustc-repo][rustc_repo] available which can be obtained via +`git clone https://github.com/rust-lang/rust/`. +Then you can run a `cargo dev` command to automatically make Clippy use the rustc-repo via path-dependencies +which rust-analyzer will be able to understand. +Run `cargo dev ra-setup --repo-path ` where `` is an absolute path to the rustc repo +you just cloned. +The command will add path-dependencies pointing towards rustc-crates inside the rustc repo to +Clippys `Cargo.toml`s and should allow rust-analyzer to understand most of the types that Clippy uses. +Just make sure to remove the dependencies again before finally making a pull request! + +[ra_homepage]: https://rust-analyzer.github.io/ +[rustc_repo]: https://github.com/rust-lang/rust/ + ## How Clippy works [`clippy_lints/src/lib.rs`][lint_crate_entry] imports all the different lint modules and registers in the [`LintStore`]. diff --git a/Cargo.toml b/Cargo.toml index 6999b6bd7404..836897927b01 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,7 +37,7 @@ tempfile = { version = "3.1.0", optional = true } lazy_static = "1.0" [dev-dependencies] -cargo_metadata = "0.9.0" +cargo_metadata = "0.9.1" compiletest_rs = { version = "0.5.0", features = ["tmp"] } tester = "0.7" lazy_static = "1.0" diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 6fdd282c6849..5baa31d5cde0 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -11,6 +11,7 @@ use walkdir::WalkDir; pub mod fmt; pub mod new_lint; +pub mod ra_setup; pub mod stderr_length_check; pub mod update_lints; @@ -400,7 +401,7 @@ fn test_replace_region_no_changes() { changed: false, new_lines: "123\n456\n789".to_string(), }; - let result = replace_region_in_text(text, r#"^\s*123$"#, r#"^\s*456"#, false, || vec![]); + let result = replace_region_in_text(text, r#"^\s*123$"#, r#"^\s*456"#, false, Vec::new); assert_eq!(expected, result); } diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index d99235f7c07a..281037ae37c9 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -1,7 +1,7 @@ #![cfg_attr(feature = "deny-warnings", deny(warnings))] use clap::{App, Arg, SubCommand}; -use clippy_dev::{fmt, new_lint, stderr_length_check, update_lints}; +use clippy_dev::{fmt, new_lint, ra_setup, stderr_length_check, update_lints}; fn main() { let matches = App::new("Clippy developer tooling") @@ -87,6 +87,19 @@ fn main() { SubCommand::with_name("limit_stderr_length") .about("Ensures that stderr files do not grow longer than a certain amount of lines."), ) + .subcommand( + SubCommand::with_name("ra-setup") + .about("Alter dependencies so rust-analyzer can find rustc internals") + .arg( + Arg::with_name("rustc-repo-path") + .long("repo-path") + .short("r") + .help("The path to a rustc repo that will be used for setting the dependencies") + .takes_value(true) + .value_name("path") + .required(true), + ), + ) .get_matches(); match matches.subcommand() { @@ -115,6 +128,7 @@ fn main() { ("limit_stderr_length", _) => { stderr_length_check::check(); }, + ("ra-setup", Some(matches)) => ra_setup::run(matches.value_of("rustc-repo-path")), _ => {}, } } diff --git a/clippy_dev/src/new_lint.rs b/clippy_dev/src/new_lint.rs index c0b2dac2f60f..1e032a7bc20c 100644 --- a/clippy_dev/src/new_lint.rs +++ b/clippy_dev/src/new_lint.rs @@ -147,6 +147,8 @@ fn get_manifest_contents(lint_name: &str, hint: &str) -> String { name = "{}" version = "0.1.0" publish = false + +[workspace] "#, hint, lint_name ) diff --git a/clippy_dev/src/ra_setup.rs b/clippy_dev/src/ra_setup.rs new file mode 100644 index 000000000000..8617445c8a60 --- /dev/null +++ b/clippy_dev/src/ra_setup.rs @@ -0,0 +1,90 @@ +#![allow(clippy::filter_map)] + +use std::fs; +use std::fs::File; +use std::io::prelude::*; +use std::path::PathBuf; + +// This module takes an absolute path to a rustc repo and alters the dependencies to point towards +// the respective rustc subcrates instead of using extern crate xyz. +// This allows rust analyzer to analyze rustc internals and show proper information inside clippy +// code. See https://github.com/rust-analyzer/rust-analyzer/issues/3517 and https://github.com/rust-lang/rust-clippy/issues/5514 for details + +pub fn run(rustc_path: Option<&str>) { + // we can unwrap here because the arg is required here + let rustc_path = PathBuf::from(rustc_path.unwrap()); + assert!(rustc_path.is_dir(), "path is not a directory"); + let rustc_source_basedir = rustc_path.join("src"); + assert!( + rustc_source_basedir.is_dir(), + "are you sure the path leads to a rustc repo?" + ); + + let clippy_root_manifest = fs::read_to_string("Cargo.toml").expect("failed to read ./Cargo.toml"); + let clippy_root_lib_rs = fs::read_to_string("src/driver.rs").expect("failed to read ./src/driver.rs"); + inject_deps_into_manifest( + &rustc_source_basedir, + "Cargo.toml", + &clippy_root_manifest, + &clippy_root_lib_rs, + ) + .expect("Failed to inject deps into ./Cargo.toml"); + + let clippy_lints_manifest = + fs::read_to_string("clippy_lints/Cargo.toml").expect("failed to read ./clippy_lints/Cargo.toml"); + let clippy_lints_lib_rs = + fs::read_to_string("clippy_lints/src/lib.rs").expect("failed to read ./clippy_lints/src/lib.rs"); + inject_deps_into_manifest( + &rustc_source_basedir, + "clippy_lints/Cargo.toml", + &clippy_lints_manifest, + &clippy_lints_lib_rs, + ) + .expect("Failed to inject deps into ./clippy_lints/Cargo.toml"); +} + +fn inject_deps_into_manifest( + rustc_source_dir: &PathBuf, + manifest_path: &str, + cargo_toml: &str, + lib_rs: &str, +) -> std::io::Result<()> { + let extern_crates = lib_rs + .lines() + // get the deps + .filter(|line| line.starts_with("extern crate")) + // we have something like "extern crate foo;", we only care about the "foo" + // ↓ ↓ + // extern crate rustc_middle; + .map(|s| &s[13..(s.len() - 1)]); + + let new_deps = extern_crates.map(|dep| { + // format the dependencies that are going to be put inside the Cargo.toml + format!( + "{dep} = {{ path = \"{source_path}/lib{dep}\" }}\n", + dep = dep, + source_path = rustc_source_dir.display() + ) + }); + + // format a new [dependencies]-block with the new deps we need to inject + let mut all_deps = String::from("[dependencies]\n"); + new_deps.for_each(|dep_line| { + all_deps.push_str(&dep_line); + }); + + // replace "[dependencies]" with + // [dependencies] + // dep1 = { path = ... } + // dep2 = { path = ... } + // etc + let new_manifest = cargo_toml.replacen("[dependencies]\n", &all_deps, 1); + + // println!("{}", new_manifest); + let mut file = File::create(manifest_path)?; + file.write_all(new_manifest.as_bytes())?; + + println!("Dependency paths injected: {}", manifest_path); + + Ok(()) +} diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index 76baf27fb2db..e959c1a65112 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -17,11 +17,11 @@ keywords = ["clippy", "lint", "plugin"] edition = "2018" [dependencies] -cargo_metadata = "0.9.0" +cargo_metadata = "0.9.1" if_chain = "1.0.0" itertools = "0.9" lazy_static = "1.0.2" -pulldown-cmark = { version = "0.7", default-features = false } +pulldown-cmark = { version = "0.7.1", default-features = false } quine-mc_cluskey = "0.2.2" regex-syntax = "0.6" serde = { version = "1.0", features = ["derive"] } diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs index 05e2650d0b71..13e61fe98bac 100644 --- a/clippy_lints/src/assign_ops.rs +++ b/clippy_lints/src/assign_ops.rs @@ -24,7 +24,11 @@ declare_clippy_lint! { /// let mut a = 5; /// let b = 0; /// // ... + /// // Bad /// a = a + b; + /// + /// // Good + /// a += b; /// ``` pub ASSIGN_OP_PATTERN, style, diff --git a/clippy_lints/src/cargo_common_metadata.rs b/clippy_lints/src/cargo_common_metadata.rs index 16b46423c8f0..c40a387d2979 100644 --- a/clippy_lints/src/cargo_common_metadata.rs +++ b/clippy_lints/src/cargo_common_metadata.rs @@ -36,13 +36,9 @@ declare_clippy_lint! { "common metadata is defined in `Cargo.toml`" } -fn warning(cx: &LateContext<'_, '_>, message: &str) { - span_lint(cx, CARGO_COMMON_METADATA, DUMMY_SP, message); -} - fn missing_warning(cx: &LateContext<'_, '_>, package: &cargo_metadata::Package, field: &str) { let message = format!("package `{}` is missing `{}` metadata", package.name, field); - warning(cx, &message); + span_lint(cx, CARGO_COMMON_METADATA, DUMMY_SP, &message); } fn is_empty_str(value: &Option) -> bool { @@ -66,12 +62,7 @@ impl LateLintPass<'_, '_> for CargoCommonMetadata { return; } - let metadata = if let Ok(metadata) = cargo_metadata::MetadataCommand::new().no_deps().exec() { - metadata - } else { - warning(cx, "could not read cargo metadata"); - return; - }; + let metadata = unwrap_cargo_metadata!(cx, CARGO_COMMON_METADATA, false); for package in metadata.packages { if is_empty_vec(&package.authors) { diff --git a/clippy_lints/src/checked_conversions.rs b/clippy_lints/src/checked_conversions.rs index d9776dd50a83..e845ef99c7cc 100644 --- a/clippy_lints/src/checked_conversions.rs +++ b/clippy_lints/src/checked_conversions.rs @@ -58,24 +58,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CheckedConversions { } }; - if_chain! { - if let Some(cv) = result; - if let Some(to_type) = cv.to_type; - - then { + if let Some(cv) = result { + if let Some(to_type) = cv.to_type { let mut applicability = Applicability::MachineApplicable; - let snippet = snippet_with_applicability(cx, cv.expr_to_cast.span, "_", &mut - applicability); + let snippet = snippet_with_applicability(cx, cv.expr_to_cast.span, "_", &mut applicability); span_lint_and_sugg( cx, CHECKED_CONVERSIONS, item.span, "Checked cast can be simplified.", "try", - format!("{}::try_from({}).is_ok()", - to_type, - snippet), - applicability + format!("{}::try_from({}).is_ok()", to_type, snippet), + applicability, ); } } @@ -184,7 +178,7 @@ fn check_upper_bound<'tcx>(expr: &'tcx Expr<'tcx>) -> Option> { if_chain! { if let ExprKind::Binary(ref op, ref left, ref right) = &expr.kind; if let Some((candidate, check)) = normalize_le_ge(op, left, right); - if let Some((from, to)) = get_types_from_cast(check, MAX_VALUE, INTS); + if let Some((from, to)) = get_types_from_cast(check, INTS, "max_value", "MAX"); then { Conversion::try_new(candidate, from, to) @@ -224,7 +218,7 @@ fn check_lower_bound_zero<'a>(candidate: &'a Expr<'_>, check: &'a Expr<'_>) -> O /// Check for `expr >= (to_type::MIN as from_type)` fn check_lower_bound_min<'a>(candidate: &'a Expr<'_>, check: &'a Expr<'_>) -> Option> { - if let Some((from, to)) = get_types_from_cast(check, MIN_VALUE, SINTS) { + if let Some((from, to)) = get_types_from_cast(check, SINTS, "min_value", "MIN") { Conversion::try_new(candidate, from, to) } else { None @@ -232,10 +226,16 @@ fn check_lower_bound_min<'a>(candidate: &'a Expr<'_>, check: &'a Expr<'_>) -> Op } /// Tries to extract the from- and to-type from a cast expression -fn get_types_from_cast<'a>(expr: &'a Expr<'_>, func: &'a str, types: &'a [&str]) -> Option<(&'a str, &'a str)> { - // `to_type::maxmin_value() as from_type` +fn get_types_from_cast<'a>( + expr: &'a Expr<'_>, + types: &'a [&str], + func: &'a str, + assoc_const: &'a str, +) -> Option<(&'a str, &'a str)> { + // `to_type::max_value() as from_type` + // or `to_type::MAX as from_type` let call_from_cast: Option<(&Expr<'_>, &str)> = if_chain! { - // to_type::maxmin_value(), from_type + // to_type::max_value(), from_type if let ExprKind::Cast(ref limit, ref from_type) = &expr.kind; if let TyKind::Path(ref from_type_path) = &from_type.kind; if let Some(from_sym) = int_ty_to_sym(from_type_path); @@ -247,17 +247,17 @@ fn get_types_from_cast<'a>(expr: &'a Expr<'_>, func: &'a str, types: &'a [&str]) } }; - // `from_type::from(to_type::maxmin_value())` + // `from_type::from(to_type::max_value())` let limit_from: Option<(&Expr<'_>, &str)> = call_from_cast.or_else(|| { if_chain! { - // `from_type::from, to_type::maxmin_value()` + // `from_type::from, to_type::max_value()` if let ExprKind::Call(ref from_func, ref args) = &expr.kind; - // `to_type::maxmin_value()` + // `to_type::max_value()` if args.len() == 1; if let limit = &args[0]; // `from_type::from` if let ExprKind::Path(ref path) = &from_func.kind; - if let Some(from_sym) = get_implementing_type(path, INTS, FROM); + if let Some(from_sym) = get_implementing_type(path, INTS, "from"); then { Some((limit, from_sym)) @@ -268,22 +268,26 @@ fn get_types_from_cast<'a>(expr: &'a Expr<'_>, func: &'a str, types: &'a [&str]) }); if let Some((limit, from_type)) = limit_from { - if_chain! { - if let ExprKind::Call(ref fun_name, _) = &limit.kind; - // `to_type, maxmin_value` - if let ExprKind::Path(ref path) = &fun_name.kind; - // `to_type` - if let Some(to_type) = get_implementing_type(path, types, func); - - then { - Some((from_type, to_type)) - } else { - None - } + match limit.kind { + // `from_type::from(_)` + ExprKind::Call(path, _) => { + if let ExprKind::Path(ref path) = path.kind { + // `to_type` + if let Some(to_type) = get_implementing_type(path, types, func) { + return Some((from_type, to_type)); + } + } + }, + // `to_type::MAX` + ExprKind::Path(ref path) => { + if let Some(to_type) = get_implementing_type(path, types, assoc_const) { + return Some((from_type, to_type)); + } + }, + _ => {}, } - } else { - None - } + }; + None } /// Gets the type which implements the called function @@ -336,10 +340,6 @@ fn normalize_le_ge<'a>(op: &BinOp, left: &'a Expr<'a>, right: &'a Expr<'a>) -> O } // Constants -const FROM: &str = "from"; -const MAX_VALUE: &str = "max_value"; -const MIN_VALUE: &str = "min_value"; - const UINTS: &[&str] = &["u8", "u16", "u32", "u64", "usize"]; const SINTS: &[&str] = &["i8", "i16", "i32", "i64", "isize"]; const INTS: &[&str] = &["u8", "u16", "u32", "u64", "usize", "i8", "i16", "i32", "i64", "isize"]; diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index 66722786eab4..b6d50bdfa146 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -1,9 +1,9 @@ -use crate::utils::{get_parent_expr, higher, if_sequence, same_tys, snippet, span_lint_and_note, span_lint_and_then}; +use crate::utils::{get_parent_expr, higher, if_sequence, snippet, span_lint_and_note, span_lint_and_then}; use crate::utils::{SpanlessEq, SpanlessHash}; use rustc_data_structures::fx::FxHashMap; use rustc_hir::{Arm, Block, Expr, ExprKind, MatchSource, Pat, PatKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::Ty; +use rustc_middle::ty::{Ty, TyS}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::symbol::Symbol; use std::collections::hash_map::Entry; @@ -242,15 +242,11 @@ fn lint_same_fns_in_if_cond(cx: &LateContext<'_, '_>, conds: &[&Expr<'_>]) { /// Implementation of `MATCH_SAME_ARMS`. fn lint_match_arms<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &Expr<'_>) { - fn same_bindings<'tcx>( - cx: &LateContext<'_, 'tcx>, - lhs: &FxHashMap>, - rhs: &FxHashMap>, - ) -> bool { + fn same_bindings<'tcx>(lhs: &FxHashMap>, rhs: &FxHashMap>) -> bool { lhs.len() == rhs.len() && lhs .iter() - .all(|(name, l_ty)| rhs.get(name).map_or(false, |r_ty| same_tys(cx, l_ty, r_ty))) + .all(|(name, l_ty)| rhs.get(name).map_or(false, |r_ty| TyS::same_type(l_ty, r_ty))) } if let ExprKind::Match(_, ref arms, MatchSource::Normal) = expr.kind { @@ -269,7 +265,7 @@ fn lint_match_arms<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &Expr<'_>) { (min_index..=max_index).all(|index| arms[index].guard.is_none()) && SpanlessEq::new(cx).eq_expr(&lhs.body, &rhs.body) && // all patterns should have the same bindings - same_bindings(cx, &bindings(cx, &lhs.pat), &bindings(cx, &rhs.pat)) + same_bindings(&bindings(cx, &lhs.pat), &bindings(cx, &rhs.pat)) }; let indexed_arms: Vec<(usize, &Arm<'_>)> = arms.iter().enumerate().collect(); diff --git a/clippy_lints/src/double_parens.rs b/clippy_lints/src/double_parens.rs index 7f2ff8b9b26f..05517f6f9f0c 100644 --- a/clippy_lints/src/double_parens.rs +++ b/clippy_lints/src/double_parens.rs @@ -13,10 +13,24 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust + /// // Bad + /// fn simple_double_parens() -> i32 { + /// ((0)) + /// } + /// + /// // Good + /// fn simple_no_parens() -> i32 { + /// 0 + /// } + /// + /// // or + /// /// # fn foo(bar: usize) {} - /// ((0)); + /// // Bad /// foo((0)); - /// ((1, 2)); + /// + /// // Good + /// foo(0); /// ``` pub DOUBLE_PARENS, complexity, diff --git a/clippy_lints/src/drop_bounds.rs b/clippy_lints/src/drop_bounds.rs index f49668082791..5a7f759486ed 100644 --- a/clippy_lints/src/drop_bounds.rs +++ b/clippy_lints/src/drop_bounds.rs @@ -27,6 +27,10 @@ declare_clippy_lint! { /// ```rust /// fn foo() {} /// ``` + /// Could be written as: + /// ```rust + /// fn foo() {} + /// ``` pub DROP_BOUNDS, correctness, "Bounds of the form `T: Drop` are useless" diff --git a/clippy_lints/src/duration_subsec.rs b/clippy_lints/src/duration_subsec.rs index b35a8facf8b9..afefa2506381 100644 --- a/clippy_lints/src/duration_subsec.rs +++ b/clippy_lints/src/duration_subsec.rs @@ -22,8 +22,14 @@ declare_clippy_lint! { /// ```rust /// # use std::time::Duration; /// let dur = Duration::new(5, 0); + /// + /// // Bad /// let _micros = dur.subsec_nanos() / 1_000; /// let _millis = dur.subsec_nanos() / 1_000_000; + /// + /// // Good + /// let _micros = dur.subsec_micros(); + /// let _millis = dur.subsec_millis(); /// ``` pub DURATION_SUBSEC, complexity, diff --git a/clippy_lints/src/enum_variants.rs b/clippy_lints/src/enum_variants.rs index a5871cf0cd4d..cb0fd59a2d40 100644 --- a/clippy_lints/src/enum_variants.rs +++ b/clippy_lints/src/enum_variants.rs @@ -25,31 +25,47 @@ declare_clippy_lint! { /// BattenbergCake, /// } /// ``` + /// Could be written as: + /// ```rust + /// enum Cake { + /// BlackForest, + /// Hummingbird, + /// Battenberg, + /// } + /// ``` pub ENUM_VARIANT_NAMES, style, "enums where all variants share a prefix/postfix" } declare_clippy_lint! { - /// **What it does:** Detects enumeration variants that are prefixed or suffixed - /// by the same characters. + /// **What it does:** Detects public enumeration variants that are + /// prefixed or suffixed by the same characters. /// - /// **Why is this bad?** Enumeration variant names should specify their variant, + /// **Why is this bad?** Public enumeration variant names should specify their variant, /// not repeat the enumeration name. /// /// **Known problems:** None. /// /// **Example:** /// ```rust - /// enum Cake { + /// pub enum Cake { /// BlackForestCake, /// HummingbirdCake, /// BattenbergCake, /// } /// ``` + /// Could be written as: + /// ```rust + /// pub enum Cake { + /// BlackForest, + /// Hummingbird, + /// Battenberg, + /// } + /// ``` pub PUB_ENUM_VARIANT_NAMES, pedantic, - "enums where all variants share a prefix/postfix" + "public enums where all variants share a prefix/postfix" } declare_clippy_lint! { @@ -66,6 +82,12 @@ declare_clippy_lint! { /// struct BlackForestCake; /// } /// ``` + /// Could be written as: + /// ```rust + /// mod cake { + /// struct BlackForest; + /// } + /// ``` pub MODULE_NAME_REPETITIONS, pedantic, "type names prefixed/postfixed with their containing module's name" diff --git a/clippy_lints/src/eq_op.rs b/clippy_lints/src/eq_op.rs index 4e1c1f131405..d7819d737ea0 100644 --- a/clippy_lints/src/eq_op.rs +++ b/clippy_lints/src/eq_op.rs @@ -39,7 +39,11 @@ declare_clippy_lint! { /// /// **Example:** /// ```ignore + /// // Bad /// &x == y + /// + /// // Good + /// x == *y /// ``` pub OP_REF, style, diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs index 1ec60a0e6e67..7227683aa5ac 100644 --- a/clippy_lints/src/escape.rs +++ b/clippy_lints/src/escape.rs @@ -28,9 +28,16 @@ declare_clippy_lint! { /// **Example:** /// ```rust /// # fn foo(bar: usize) {} + /// + /// // Bad /// let x = Box::new(1); /// foo(*x); /// println!("{}", *x); + /// + /// // Good + /// let x = 1; + /// foo(x); + /// println!("{}", x); /// ``` pub BOXED_LOCAL, perf, diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs index e3e1136b6769..d093025fd3d7 100644 --- a/clippy_lints/src/eta_reduction.rs +++ b/clippy_lints/src/eta_reduction.rs @@ -26,7 +26,11 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust,ignore + /// // Bad /// xs.map(|x| foo(x)) + /// + /// // Good + /// xs.map(foo) /// ``` /// where `foo(_)` is a plain function that takes the exact argument type of /// `x`. diff --git a/clippy_lints/src/eval_order_dependence.rs b/clippy_lints/src/eval_order_dependence.rs index 5206266ccf2a..74144fb299de 100644 --- a/clippy_lints/src/eval_order_dependence.rs +++ b/clippy_lints/src/eval_order_dependence.rs @@ -21,11 +21,20 @@ declare_clippy_lint! { /// **Example:** /// ```rust /// let mut x = 0; + /// + /// // Bad /// let a = { /// x = 1; /// 1 /// } + x; /// // Unclear whether a is 1 or 2. + /// + /// // Good + /// let tmp = { + /// x = 1; + /// 1 + /// }; + /// let a = tmp + x; /// ``` pub EVAL_ORDER_DEPENDENCE, complexity, diff --git a/clippy_lints/src/fallible_impl_from.rs b/clippy_lints/src/fallible_impl_from.rs index 17639cc2a064..92812816461c 100644 --- a/clippy_lints/src/fallible_impl_from.rs +++ b/clippy_lints/src/fallible_impl_from.rs @@ -20,12 +20,31 @@ declare_clippy_lint! { /// **Example:** /// ```rust /// struct Foo(i32); + /// + /// // Bad /// impl From for Foo { /// fn from(s: String) -> Self { /// Foo(s.parse().unwrap()) /// } /// } /// ``` + /// + /// ```rust + /// // Good + /// struct Foo(i32); + /// + /// use std::convert::TryFrom; + /// impl TryFrom for Foo { + /// type Error = (); + /// fn try_from(s: String) -> Result { + /// if let Ok(parsed) = s.parse() { + /// Ok(Foo(parsed)) + /// } else { + /// Err(()) + /// } + /// } + /// } + /// ``` pub FALLIBLE_IMPL_FROM, nursery, "Warn on impls of `From<..>` that contain `panic!()` or `unwrap()`" @@ -120,7 +139,7 @@ fn lint_impl_body<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, impl_span: Span, impl_it move |diag| { diag.help( "`From` is intended for infallible conversions only. \ - Use `TryFrom` if there's a possibility for the conversion to fail."); + Use `TryFrom` if there's a possibility for the conversion to fail."); diag.span_note(fpu.result, "potential failure(s)"); }); } diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index 86317fb8bd5c..3a912d928375 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -28,7 +28,6 @@ declare_clippy_lint! { /// **Example:** /// /// ```rust - /// /// let a = 3f32; /// let _ = a.powf(1.0 / 3.0); /// let _ = (1.0 + a).ln(); @@ -38,7 +37,6 @@ declare_clippy_lint! { /// is better expressed as /// /// ```rust - /// /// let a = 3f32; /// let _ = a.cbrt(); /// let _ = a.ln_1p(); diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index 5b092526ce4f..1530538aa7d1 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -25,9 +25,13 @@ declare_clippy_lint! { /// /// **Examples:** /// ```rust + /// + /// // Bad /// # let foo = "foo"; - /// format!("foo"); /// format!("{}", foo); + /// + /// // Good + /// format!("foo"); /// ``` pub USELESS_FORMAT, complexity, diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index c24a24733d7f..325b6cf32a3d 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -49,11 +49,11 @@ declare_clippy_lint! { /// **Known problems:** None. /// /// **Example:** - /// ``` rust + /// ```rust /// fn im_too_long() { - /// println!(""); - /// // ... 100 more LoC - /// println!(""); + /// println!(""); + /// // ... 100 more LoC + /// println!(""); /// } /// ``` pub TOO_MANY_LINES, @@ -79,10 +79,16 @@ declare_clippy_lint! { /// `some_argument.get_raw_ptr()`). /// /// **Example:** - /// ```rust + /// ```rust,ignore + /// // Bad /// pub fn foo(x: *const u8) { /// println!("{}", unsafe { *x }); /// } + /// + /// // Good + /// pub unsafe fn foo(x: *const u8) { + /// println!("{}", unsafe { *x }); + /// } /// ``` pub NOT_UNSAFE_PTR_ARG_DEREF, correctness, diff --git a/clippy_lints/src/implicit_saturating_sub.rs b/clippy_lints/src/implicit_saturating_sub.rs index 155a93de4fac..fdaf37e5e08f 100644 --- a/clippy_lints/src/implicit_saturating_sub.rs +++ b/clippy_lints/src/implicit_saturating_sub.rs @@ -25,13 +25,6 @@ declare_clippy_lint! { /// if i != 0 { /// i -= 1; /// } - /// ``` - /// Use instead: - /// ```rust - /// let end: u32 = 10; - /// let start: u32 = 5; - /// - /// let mut i: u32 = end - start; /// /// // Good /// i = i.saturating_sub(1); diff --git a/clippy_lints/src/int_plus_one.rs b/clippy_lints/src/int_plus_one.rs index d5dbd495680b..e91fb0c2f27c 100644 --- a/clippy_lints/src/int_plus_one.rs +++ b/clippy_lints/src/int_plus_one.rs @@ -10,7 +10,6 @@ use crate::utils::{snippet_opt, span_lint_and_sugg}; declare_clippy_lint! { /// **What it does:** Checks for usage of `x >= y + 1` or `x - 1 >= y` (and `<=`) in a block /// - /// /// **Why is this bad?** Readability -- better to use `> y` instead of `>= y + 1`. /// /// **Known problems:** None. diff --git a/clippy_lints/src/integer_division.rs b/clippy_lints/src/integer_division.rs index fe34d33fe652..d537ef3f3238 100644 --- a/clippy_lints/src/integer_division.rs +++ b/clippy_lints/src/integer_division.rs @@ -15,10 +15,13 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust - /// fn main() { - /// let x = 3 / 2; - /// println!("{}", x); - /// } + /// // Bad + /// let x = 3 / 2; + /// println!("{}", x); + /// + /// // Good + /// let x = 3f32 / 2f32; + /// println!("{}", x); /// ``` pub INTEGER_DIVISION, restriction, diff --git a/clippy_lints/src/items_after_statements.rs b/clippy_lints/src/items_after_statements.rs index e7062b7c16bb..c8576bcfcb44 100644 --- a/clippy_lints/src/items_after_statements.rs +++ b/clippy_lints/src/items_after_statements.rs @@ -16,6 +16,7 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust + /// // Bad /// fn foo() { /// println!("cake"); /// } @@ -28,6 +29,21 @@ declare_clippy_lint! { /// foo(); // prints "foo" /// } /// ``` + /// + /// ```rust + /// // Good + /// fn foo() { + /// println!("cake"); + /// } + /// + /// fn main() { + /// fn foo() { + /// println!("foo"); + /// } + /// foo(); // prints "foo" + /// foo(); // prints "foo" + /// } + /// ``` pub ITEMS_AFTER_STATEMENTS, pedantic, "blocks where an item comes after a statement" diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index 2ec0b5a8d6fb..f5bfede75a76 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -1,4 +1,4 @@ -use crate::utils::{get_item_name, snippet_with_applicability, span_lint, span_lint_and_sugg, walk_ptrs_ty}; +use crate::utils::{get_item_name, higher, snippet_with_applicability, span_lint, span_lint_and_sugg, walk_ptrs_ty}; use rustc_ast::ast::LitKind; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; @@ -259,6 +259,17 @@ fn check_len( /// Checks if this type has an `is_empty` method. fn has_is_empty(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { + /// Special case ranges until `range_is_empty` is stabilized. See issue 3807. + fn should_skip_range(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { + higher::range(cx, expr).map_or(false, |_| { + !cx.tcx + .features() + .declared_lib_features + .iter() + .any(|(name, _)| name.as_str() == "range_is_empty") + }) + } + /// Gets an `AssocItem` and return true if it matches `is_empty(self)`. fn is_is_empty(cx: &LateContext<'_, '_>, item: &ty::AssocItem) -> bool { if let ty::AssocKind::Fn = item.kind { @@ -284,6 +295,10 @@ fn has_is_empty(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { }) } + if should_skip_range(cx, expr) { + return false; + } + let ty = &walk_ptrs_ty(cx.tables.expr_ty(expr)); match ty.kind { ty::Dynamic(ref tt, ..) => { diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index b7d928d249f9..6f8923b26609 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -318,6 +318,7 @@ mod try_err; mod types; mod unicode; mod unnamed_address; +mod unnecessary_sort_by; mod unsafe_removed_from_name; mod unused_io_amount; mod unused_self; @@ -325,6 +326,7 @@ mod unwrap; mod use_self; mod useless_conversion; mod vec; +mod vec_resize_to_zero; mod verbose_file_reads; mod wildcard_dependencies; mod wildcard_imports; @@ -664,6 +666,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &methods::INTO_ITER_ON_REF, &methods::ITERATOR_STEP_BY_ZERO, &methods::ITER_CLONED_COLLECT, + &methods::ITER_NEXT_SLICE, &methods::ITER_NTH, &methods::ITER_NTH_ZERO, &methods::ITER_SKIP_NEXT, @@ -832,6 +835,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &unicode::ZERO_WIDTH_SPACE, &unnamed_address::FN_ADDRESS_COMPARISONS, &unnamed_address::VTABLE_ADDRESS_COMPARISONS, + &unnecessary_sort_by::UNNECESSARY_SORT_BY, &unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME, &unused_io_amount::UNUSED_IO_AMOUNT, &unused_self::UNUSED_SELF, @@ -847,6 +851,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &utils::internal_lints::OUTER_EXPN_EXPN_DATA, &utils::internal_lints::PRODUCE_ICE, &vec::USELESS_VEC, + &vec_resize_to_zero::VEC_RESIZE_TO_ZERO, &verbose_file_reads::VERBOSE_FILE_READS, &wildcard_dependencies::WILDCARD_DEPENDENCIES, &wildcard_imports::ENUM_GLOB_USE, @@ -994,6 +999,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box ptr_offset_with_cast::PtrOffsetWithCast); store.register_late_pass(|| box redundant_clone::RedundantClone); store.register_late_pass(|| box slow_vector_initialization::SlowVectorInit); + store.register_late_pass(|| box unnecessary_sort_by::UnnecessarySortBy); store.register_late_pass(|| box types::RefToMut); store.register_late_pass(|| box assertions_on_constants::AssertionsOnConstants); store.register_late_pass(|| box missing_const_for_fn::MissingConstForFn); @@ -1062,6 +1068,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| box manual_non_exhaustive::ManualNonExhaustive); store.register_late_pass(|| box manual_async_fn::ManualAsyncFn); store.register_early_pass(|| box redundant_field_names::RedundantFieldNames); + store.register_late_pass(|| box vec_resize_to_zero::VecResizeToZero); let single_char_binding_names_threshold = conf.single_char_binding_names_threshold; store.register_early_pass(move || box non_expressive_names::NonExpressiveNames { single_char_binding_names_threshold, @@ -1164,6 +1171,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::CAST_POSSIBLE_TRUNCATION), LintId::of(&types::CAST_POSSIBLE_WRAP), LintId::of(&types::CAST_PRECISION_LOSS), + LintId::of(&types::CAST_PTR_ALIGNMENT), LintId::of(&types::CAST_SIGN_LOSS), LintId::of(&types::IMPLICIT_HASHER), LintId::of(&types::INVALID_UPCAST_COMPARISONS), @@ -1303,6 +1311,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&methods::INTO_ITER_ON_REF), LintId::of(&methods::ITERATOR_STEP_BY_ZERO), LintId::of(&methods::ITER_CLONED_COLLECT), + LintId::of(&methods::ITER_NEXT_SLICE), LintId::of(&methods::ITER_NTH), LintId::of(&methods::ITER_NTH_ZERO), LintId::of(&methods::ITER_SKIP_NEXT), @@ -1410,7 +1419,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::ABSURD_EXTREME_COMPARISONS), LintId::of(&types::BORROWED_BOX), LintId::of(&types::BOX_VEC), - LintId::of(&types::CAST_PTR_ALIGNMENT), LintId::of(&types::CAST_REF_TO_MUT), LintId::of(&types::CHAR_LIT_AS_U8), LintId::of(&types::FN_TO_NUMERIC_CAST), @@ -1424,12 +1432,14 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&unicode::ZERO_WIDTH_SPACE), LintId::of(&unnamed_address::FN_ADDRESS_COMPARISONS), LintId::of(&unnamed_address::VTABLE_ADDRESS_COMPARISONS), + LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY), LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME), LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT), LintId::of(&unwrap::PANICKING_UNWRAP), LintId::of(&unwrap::UNNECESSARY_UNWRAP), LintId::of(&useless_conversion::USELESS_CONVERSION), LintId::of(&vec::USELESS_VEC), + LintId::of(&vec_resize_to_zero::VEC_RESIZE_TO_ZERO), LintId::of(&write::PRINTLN_EMPTY_STRING), LintId::of(&write::PRINT_LITERAL), LintId::of(&write::PRINT_WITH_NEWLINE), @@ -1483,6 +1493,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&methods::CHARS_NEXT_CMP), LintId::of(&methods::INTO_ITER_ON_REF), LintId::of(&methods::ITER_CLONED_COLLECT), + LintId::of(&methods::ITER_NEXT_SLICE), LintId::of(&methods::ITER_NTH_ZERO), LintId::of(&methods::ITER_SKIP_NEXT), LintId::of(&methods::MANUAL_SATURATING_ARITHMETIC), @@ -1604,6 +1615,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::UNIT_ARG), LintId::of(&types::UNNECESSARY_CAST), LintId::of(&types::VEC_BOX), + LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY), LintId::of(&unwrap::UNNECESSARY_UNWRAP), LintId::of(&useless_conversion::USELESS_CONVERSION), LintId::of(&zero_div_zero::ZERO_DIVIDED_BY_ZERO), @@ -1669,7 +1681,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&transmute::WRONG_TRANSMUTE), LintId::of(&transmuting_null::TRANSMUTING_NULL), LintId::of(&types::ABSURD_EXTREME_COMPARISONS), - LintId::of(&types::CAST_PTR_ALIGNMENT), LintId::of(&types::CAST_REF_TO_MUT), LintId::of(&types::UNIT_CMP), LintId::of(&unicode::ZERO_WIDTH_SPACE), @@ -1677,6 +1688,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&unnamed_address::VTABLE_ADDRESS_COMPARISONS), LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT), LintId::of(&unwrap::PANICKING_UNWRAP), + LintId::of(&vec_resize_to_zero::VEC_RESIZE_TO_ZERO), ]); store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![ diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index ec7c4531ed71..7ba43562d7d4 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -24,7 +24,11 @@ declare_clippy_lint! { /// **Example:** /// /// ```rust + /// // Bad /// let x: u64 = 61864918973511; + /// + /// // Good + /// let x: u64 = 61_864_918_973_511; /// ``` pub UNREADABLE_LITERAL, pedantic, @@ -44,7 +48,11 @@ declare_clippy_lint! { /// **Example:** /// /// ```rust + /// // Probably mistyped /// 2_32; + /// + /// // Good + /// 2_i32; /// ``` pub MISTYPED_LITERAL_SUFFIXES, correctness, @@ -63,7 +71,11 @@ declare_clippy_lint! { /// **Example:** /// /// ```rust + /// // Bad /// let x: u64 = 618_64_9189_73_511; + /// + /// // Good + /// let x: u64 = 61_864_918_973_511; /// ``` pub INCONSISTENT_DIGIT_GROUPING, style, diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 38a5829b3f74..57c62d739640 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -8,7 +8,7 @@ use crate::utils::{ multispan_sugg, snippet, snippet_opt, snippet_with_applicability, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, SpanlessEq, }; -use crate::utils::{is_type_diagnostic_item, qpath_res, same_tys, sugg}; +use crate::utils::{is_type_diagnostic_item, qpath_res, sugg}; use if_chain::if_chain; use rustc_ast::ast; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -24,10 +24,10 @@ use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::hir::map::Map; use rustc_middle::lint::in_external_macro; use rustc_middle::middle::region; -use rustc_middle::ty::{self, Ty}; +use rustc_middle::ty::{self, Ty, TyS}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; -use rustc_span::BytePos; +use rustc_span::symbol::Symbol; use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, Place, PlaceBase}; use std::iter::{once, Iterator}; use std::mem; @@ -1256,7 +1256,7 @@ fn check_for_loop_arg(cx: &LateContext<'_, '_>, pat: &Pat<'_>, arg: &Expr<'_>, e } else if method_name == "into_iter" && match_trait_method(cx, arg, &paths::INTO_ITERATOR) { let receiver_ty = cx.tables.expr_ty(&args[0]); let receiver_ty_adjusted = cx.tables.expr_ty_adjusted(&args[0]); - if same_tys(cx, receiver_ty, receiver_ty_adjusted) { + if TyS::same_type(receiver_ty, receiver_ty_adjusted) { let mut applicability = Applicability::MachineApplicable; let object = snippet_with_applicability(cx, args[0].span, "_", &mut applicability); span_lint_and_sugg( @@ -1277,7 +1277,7 @@ fn check_for_loop_arg(cx: &LateContext<'_, '_>, pat: &Pat<'_>, arg: &Expr<'_>, e mutbl: Mutability::Not, }, ); - if same_tys(cx, receiver_ty_adjusted, ref_receiver_ty) { + if TyS::same_type(receiver_ty_adjusted, ref_receiver_ty) { lint_iter_method(cx, args, arg, method_name) } } @@ -2381,32 +2381,32 @@ fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'a, ' match_type(cx, ty, &paths::BTREEMAP) || is_type_diagnostic_item(cx, ty, sym!(hashmap_type)) { if method.ident.name == sym!(len) { - let span = shorten_needless_collect_span(expr); + let span = shorten_span(expr, sym!(collect)); span_lint_and_sugg( cx, NEEDLESS_COLLECT, span, NEEDLESS_COLLECT_MSG, "replace with", - ".count()".to_string(), + "count()".to_string(), Applicability::MachineApplicable, ); } if method.ident.name == sym!(is_empty) { - let span = shorten_needless_collect_span(expr); + let span = shorten_span(expr, sym!(iter)); span_lint_and_sugg( cx, NEEDLESS_COLLECT, span, NEEDLESS_COLLECT_MSG, "replace with", - ".next().is_none()".to_string(), + "get(0).is_none()".to_string(), Applicability::MachineApplicable, ); } if method.ident.name == sym!(contains) { let contains_arg = snippet(cx, args[1].span, "??"); - let span = shorten_needless_collect_span(expr); + let span = shorten_span(expr, sym!(collect)); span_lint_and_then( cx, NEEDLESS_COLLECT, @@ -2422,7 +2422,7 @@ fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'a, ' span, "replace with", format!( - ".any(|{}| x == {})", + "any(|{}| x == {})", arg, pred ), Applicability::MachineApplicable, @@ -2435,13 +2435,13 @@ fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'a, ' } } -fn shorten_needless_collect_span(expr: &Expr<'_>) -> Span { - if_chain! { - if let ExprKind::MethodCall(_, _, ref args) = expr.kind; - if let ExprKind::MethodCall(_, ref span, _) = args[0].kind; - then { - return expr.span.with_lo(span.lo() - BytePos(1)); +fn shorten_span(expr: &Expr<'_>, target_fn_name: Symbol) -> Span { + let mut current_expr = expr; + while let ExprKind::MethodCall(ref path, ref span, ref args) = current_expr.kind { + if path.ident.name == target_fn_name { + return expr.span.with_lo(span.lo()); } + current_expr = &args[0]; } unreachable!() } diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 94380acfcfd4..6d7af45a4722 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -36,10 +36,17 @@ declare_clippy_lint! { /// ```rust /// # fn bar(stool: &str) {} /// # let x = Some("abc"); + /// + /// // Bad /// match x { /// Some(ref foo) => bar(foo), /// _ => (), /// } + /// + /// // Good + /// if let Some(ref foo) = x { + /// bar(foo); + /// } /// ``` pub SINGLE_MATCH, style, @@ -97,11 +104,19 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust,ignore + /// // Bad /// match x { /// &A(ref y) => foo(y), /// &B => bar(), /// _ => frob(&x), /// } + /// + /// // Good + /// match *x { + /// A(ref y) => foo(y), + /// B => bar(), + /// _ => frob(x), + /// } /// ``` pub MATCH_REF_PATS, style, @@ -197,10 +212,15 @@ declare_clippy_lint! { /// **Example:** /// ```rust /// let x: Option<()> = None; + /// + /// // Bad /// let r: Option<&()> = match x { /// None => None, /// Some(ref v) => Some(v), /// }; + /// + /// // Good + /// let r: Option<&()> = x.as_ref(); /// ``` pub MATCH_AS_REF, complexity, @@ -219,10 +239,18 @@ declare_clippy_lint! { /// ```rust /// # enum Foo { A(usize), B(usize) } /// # let x = Foo::B(1); + /// + /// // Bad /// match x { /// Foo::A(_) => {}, /// _ => {}, /// } + /// + /// // Good + /// match x { + /// Foo::A(_) => {}, + /// Foo::B(_) => {}, + /// } /// ``` pub WILDCARD_ENUM_MATCH_ARM, restriction, @@ -242,16 +270,14 @@ declare_clippy_lint! { /// ```rust /// # enum Foo { A, B, C } /// # let x = Foo::B; + /// // Bad /// match x { /// Foo::A => {}, /// Foo::B => {}, /// _ => {}, /// } - /// ``` - /// Use instead: - /// ```rust - /// # enum Foo { A, B, C } - /// # let x = Foo::B; + /// + /// // Good /// match x { /// Foo::A => {}, /// Foo::B => {}, @@ -273,10 +299,17 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust + /// // Bad /// match "foo" { /// "a" => {}, /// "bar" | _ => {}, /// } + /// + /// // Good + /// match "foo" { + /// "a" => {}, + /// _ => {}, + /// } /// ``` pub WILDCARD_IN_OR_PATTERNS, complexity, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 52ca962e7ef9..78d69690c2d7 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -18,7 +18,7 @@ use rustc_lint::{LateContext, LateLintPass, Lint, LintContext}; use rustc_middle::hir::map::Map; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::subst::GenericArgKind; -use rustc_middle::ty::{self, Ty}; +use rustc_middle::ty::{self, Ty, TyS}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; use rustc_span::symbol::{sym, SymbolStr}; @@ -26,12 +26,12 @@ use rustc_span::symbol::{sym, SymbolStr}; use crate::consts::{constant, Constant}; use crate::utils::usage::mutated_variables; use crate::utils::{ - get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, implements_trait, in_macro, is_copy, + get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait, in_macro, is_copy, is_ctor_or_promotable_const_function, is_expn_of, is_type_diagnostic_item, iter_input_pats, last_path_segment, match_def_path, match_qpath, match_trait_method, match_type, match_var, method_calls, method_chain_args, paths, - remove_blocks, return_ty, same_tys, single_segment_path, snippet, snippet_with_applicability, - snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, - span_lint_and_then, sugg, walk_ptrs_ty, walk_ptrs_ty_depth, SpanlessEq, + remove_blocks, return_ty, single_segment_path, snippet, snippet_with_applicability, snippet_with_macro_callsite, + span_lint, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then, sugg, walk_ptrs_ty, + walk_ptrs_ty_depth, SpanlessEq, }; declare_clippy_lint! { @@ -218,7 +218,12 @@ declare_clippy_lint! { /// **Example:** /// ```rust /// # let x = Ok::<_, ()>(()); - /// x.ok().expect("why did I do this again?") + /// + /// // Bad + /// x.ok().expect("why did I do this again?"); + /// + /// // Good + /// x.expect("why did I do this again?"); /// ``` pub OK_EXPECT, style, @@ -273,8 +278,12 @@ declare_clippy_lint! { /// **Example:** /// ```rust /// # let opt = Some(1); - /// opt.map_or(None, |a| Some(a + 1)) - /// # ; + /// + /// // Bad + /// opt.map_or(None, |a| Some(a + 1)); + /// + /// // Good + /// opt.and_then(|a| Some(a + 1)); /// ``` pub OPTION_MAP_OR_NONE, style, @@ -390,14 +399,19 @@ declare_clippy_lint! { /// **What it does:** Checks for usage of `_.map(_).flatten(_)`, /// /// **Why is this bad?** Readability, this can be written more concisely as a - /// single method call. + /// single method call using `_.flat_map(_)` /// /// **Known problems:** /// /// **Example:** /// ```rust /// let vec = vec![vec![1]]; + /// + /// // Bad /// vec.iter().map(|x| x.iter()).flatten(); + /// + /// // Good + /// vec.iter().flat_map(|x| x.iter()); /// ``` pub MAP_FLATTEN, pedantic, @@ -417,7 +431,16 @@ declare_clippy_lint! { /// **Example:** /// ```rust /// let vec = vec![1]; + /// + /// // Bad /// vec.iter().filter(|x| **x == 0).map(|x| *x * 2); + /// + /// // Good + /// vec.iter().filter_map(|x| if *x == 0 { + /// Some(*x * 2) + /// } else { + /// None + /// }); /// ``` pub FILTER_MAP, pedantic, @@ -634,7 +657,12 @@ declare_clippy_lint! { /// ```rust /// # use std::rc::Rc; /// let x = Rc::new(1); + /// + /// // Bad /// x.clone(); + /// + /// // Good + /// Rc::clone(&x); /// ``` pub CLONE_ON_REF_PTR, restriction, @@ -741,7 +769,12 @@ declare_clippy_lint! { /// **Known problems:** Does not catch multi-byte unicode characters. /// /// **Example:** - /// `_.split("x")` could be `_.split('x')` + /// ```rust,ignore + /// // Bad + /// _.split("x"); + /// + /// // Good + /// _.split('x'); pub SINGLE_CHAR_PATTERN, perf, "using a single-character str where a char could be used, e.g., `_.split(\"x\")`" @@ -964,8 +997,8 @@ declare_clippy_lint! { } declare_clippy_lint! { - /// **What it does:** Checks for usage of `.chars().last()` or - /// `.chars().next_back()` on a `str` to check if it ends with a given char. + /// **What it does:** Checks for usage of `_.chars().last()` or + /// `_.chars().next_back()` on a `str` to check if it ends with a given char. /// /// **Why is this bad?** Readability, this can be written more concisely as /// `_.ends_with(_)`. @@ -975,8 +1008,12 @@ declare_clippy_lint! { /// **Example:** /// ```rust /// # let name = "_"; - /// name.chars().last() == Some('_') || name.chars().next_back() == Some('-') - /// # ; + /// + /// // Bad + /// name.chars().last() == Some('_') || name.chars().next_back() == Some('-'); + /// + /// // Good + /// name.ends_with('_') || name.ends_with('-'); /// ``` pub CHARS_LAST_CMP, style, @@ -1044,17 +1081,15 @@ declare_clippy_lint! { /// **Example:** /// ```rust /// let _ = (0..3).filter_map(|x| if x > 2 { Some(x) } else { None }); - /// ``` - /// As there is no transformation of the argument this could be written as: - /// ```rust + /// + /// // As there is no transformation of the argument this could be written as: /// let _ = (0..3).filter(|&x| x > 2); /// ``` /// /// ```rust /// let _ = (0..4).filter_map(|x| Some(x + 1)); - /// ``` - /// As there is no conditional check on the argument this could be written as: - /// ```rust + /// + /// // As there is no conditional check on the argument this could be written as: /// let _ = (0..4).map(|x| x + 1); /// ``` pub UNNECESSARY_FILTER_MAP, @@ -1075,7 +1110,11 @@ declare_clippy_lint! { /// **Example:** /// /// ```rust + /// // Bad /// let _ = (&vec![3, 4, 5]).into_iter(); + /// + /// // Good + /// let _ = (&vec![3, 4, 5]).iter(); /// ``` pub INTO_ITER_ON_REF, style, @@ -1242,6 +1281,32 @@ declare_clippy_lint! { "using `as_ref().map(Deref::deref)`, which is more succinctly expressed as `as_deref()`" } +declare_clippy_lint! { + /// **What it does:** Checks for usage of `iter().next()` on a Slice or an Array + /// + /// **Why is this bad?** These can be shortened into `.get()` + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust + /// # let a = [1, 2, 3]; + /// # let b = vec![1, 2, 3]; + /// a[2..].iter().next(); + /// b.iter().next(); + /// ``` + /// should be written as: + /// ```rust + /// # let a = [1, 2, 3]; + /// # let b = vec![1, 2, 3]; + /// a.get(2); + /// b.get(0); + /// ``` + pub ITER_NEXT_SLICE, + style, + "using `.iter().next()` on a sliced array, which can be shortened to just `.get()`" +} + declare_lint_pass!(Methods => [ UNWRAP_USED, EXPECT_USED, @@ -1273,6 +1338,7 @@ declare_lint_pass!(Methods => [ FIND_MAP, MAP_FLATTEN, ITERATOR_STEP_BY_ZERO, + ITER_NEXT_SLICE, ITER_NTH, ITER_NTH_ZERO, ITER_SKIP_NEXT, @@ -1320,6 +1386,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { }, ["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]), ["next", "skip_while"] => lint_skip_while_next(cx, expr, arg_lists[1]), + ["next", "iter"] => lint_iter_next(cx, expr, arg_lists[1]), ["map", "filter"] => lint_filter_map(cx, expr, arg_lists[1], arg_lists[0]), ["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]), ["next", "filter_map"] => lint_filter_map_next(cx, expr, arg_lists[1]), @@ -1481,7 +1548,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { let contains_self_ty = |ty: Ty<'tcx>| { ty.walk().any(|inner| match inner.unpack() { - GenericArgKind::Type(inner_ty) => same_tys(cx, self_ty, inner_ty), + GenericArgKind::Type(inner_ty) => TyS::same_type(self_ty, inner_ty), GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false, }) @@ -1508,7 +1575,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { } } - if name == "new" && !same_tys(cx, ret_ty, self_ty) { + if name == "new" && !TyS::same_type(ret_ty, self_ty) { span_lint( cx, NEW_RET_NO_SELF, @@ -2199,6 +2266,60 @@ fn lint_step_by<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &hir::Expr<'_>, args } } +fn lint_iter_next<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<'_>, iter_args: &'tcx [hir::Expr<'_>]) { + let caller_expr = &iter_args[0]; + + // Skip lint if the `iter().next()` expression is a for loop argument, + // since it is already covered by `&loops::ITER_NEXT_LOOP` + let mut parent_expr_opt = get_parent_expr(cx, expr); + while let Some(parent_expr) = parent_expr_opt { + if higher::for_loop(parent_expr).is_some() { + return; + } + parent_expr_opt = get_parent_expr(cx, parent_expr); + } + + if derefs_to_slice(cx, caller_expr, cx.tables.expr_ty(caller_expr)).is_some() { + // caller is a Slice + if_chain! { + if let hir::ExprKind::Index(ref caller_var, ref index_expr) = &caller_expr.kind; + if let Some(higher::Range { start: Some(start_expr), end: None, limits: ast::RangeLimits::HalfOpen }) + = higher::range(cx, index_expr); + if let hir::ExprKind::Lit(ref start_lit) = &start_expr.kind; + if let ast::LitKind::Int(start_idx, _) = start_lit.node; + then { + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + ITER_NEXT_SLICE, + expr.span, + "Using `.iter().next()` on a Slice without end index.", + "try calling", + format!("{}.get({})", snippet_with_applicability(cx, caller_var.span, "..", &mut applicability), start_idx), + applicability, + ); + } + } + } else if is_type_diagnostic_item(cx, cx.tables.expr_ty(caller_expr), sym!(vec_type)) + || matches!(&walk_ptrs_ty(cx.tables.expr_ty(caller_expr)).kind, ty::Array(_, _)) + { + // caller is a Vec or an Array + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + ITER_NEXT_SLICE, + expr.span, + "Using `.iter().next()` on an array", + "try calling", + format!( + "{}.get(0)", + snippet_with_applicability(cx, caller_expr.span, "..", &mut applicability) + ), + applicability, + ); + } +} + fn lint_iter_nth<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, expr: &hir::Expr<'_>, diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index e1d524c2231e..51282ab93ef6 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -38,10 +38,16 @@ declare_clippy_lint! { /// dereferences, e.g., changing `*x` to `x` within the function. /// /// **Example:** - /// ```rust + /// ```rust,ignore + /// // Bad /// fn foo(ref x: u8) -> bool { /// true /// } + /// + /// // Good + /// fn foo(x: &u8) -> bool { + /// true + /// } /// ``` pub TOPLEVEL_REF_ARG, style, @@ -60,7 +66,11 @@ declare_clippy_lint! { /// ```rust /// # let x = 1.0; /// + /// // Bad /// if x == f32::NAN { } + /// + /// // Good + /// if x.is_nan() { } /// ``` pub CMP_NAN, correctness, @@ -83,8 +93,15 @@ declare_clippy_lint! { /// ```rust /// let x = 1.2331f64; /// let y = 1.2332f64; + /// + /// // Bad /// if y == 1.23f64 { } /// if y != x {} // where both are floats + /// + /// // Good + /// let error = 0.01f64; // Use an epsilon for comparison + /// if (y - 1.23f64).abs() < error { } + /// if (y - x).abs() > error { } /// ``` pub FLOAT_CMP, correctness, @@ -191,7 +208,11 @@ declare_clippy_lint! { /// **Example:** /// /// ```rust + /// // Bad /// let a = 0 as *const u32; + /// + /// // Good + /// let a = std::ptr::null::(); /// ``` pub ZERO_PTR, style, @@ -214,7 +235,13 @@ declare_clippy_lint! { /// ```rust /// let x: f64 = 1.0; /// const ONE: f64 = 1.00; - /// x == ONE; // where both are floats + /// + /// // Bad + /// if x == ONE { } // where both are floats + /// + /// // Good + /// let error = 0.1f64; // Use an epsilon for comparison + /// if (x - ONE).abs() < error { } /// ``` pub FLOAT_CMP_CONST, restriction, diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index 552222eba2ee..ad39e59d0678 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -59,7 +59,11 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust + /// // Bad /// fn foo(a: i32, _a: i32) {} + /// + /// // Good + /// fn bar(a: i32, _b: i32) {} /// ``` pub DUPLICATE_UNDERSCORE_ARGUMENT, style, @@ -77,7 +81,11 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust,ignore - /// (|| 42)() + /// // Bad + /// let a = (|| 42)() + /// + /// // Good + /// let a = 42 /// ``` pub REDUNDANT_CLOSURE_CALL, complexity, @@ -112,7 +120,11 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust + /// // Bad /// let y = 0x1a9BAcD; + /// + /// // Good + /// let y = 0x1A9BACD; /// ``` pub MIXED_CASE_HEX_LITERALS, style, @@ -129,7 +141,11 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust + /// // Bad /// let y = 123832i32; + /// + /// // Good + /// let y = 123832_i32; /// ``` pub UNSEPARATED_LITERAL_SUFFIX, pedantic, @@ -207,9 +223,16 @@ declare_clippy_lint! { /// ```rust /// # let v = Some("abc"); /// + /// // Bad + /// match v { + /// Some(x) => (), + /// y @ _ => (), + /// } + /// + /// // Good /// match v { /// Some(x) => (), - /// y @ _ => (), // easier written as `y`, + /// y => (), /// } /// ``` pub REDUNDANT_PATTERN, @@ -235,16 +258,13 @@ declare_clippy_lint! { /// # struct TupleStruct(u32, u32, u32); /// # let t = TupleStruct(1, 2, 3); /// + /// // Bad /// match t { /// TupleStruct(0, .., _) => (), /// _ => (), /// } - /// ``` - /// can be written as - /// ```rust - /// # struct TupleStruct(u32, u32, u32); - /// # let t = TupleStruct(1, 2, 3); /// + /// // Good /// match t { /// TupleStruct(0, ..) => (), /// _ => (), diff --git a/clippy_lints/src/multiple_crate_versions.rs b/clippy_lints/src/multiple_crate_versions.rs index b6770804e180..6c42014b4c8a 100644 --- a/clippy_lints/src/multiple_crate_versions.rs +++ b/clippy_lints/src/multiple_crate_versions.rs @@ -7,7 +7,7 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::DUMMY_SP; -use cargo_metadata::{DependencyKind, MetadataCommand, Node, Package, PackageId}; +use cargo_metadata::{DependencyKind, Node, Package, PackageId}; use if_chain::if_chain; use itertools::Itertools; @@ -42,13 +42,7 @@ impl LateLintPass<'_, '_> for MultipleCrateVersions { return; } - let metadata = if let Ok(metadata) = MetadataCommand::new().exec() { - metadata - } else { - span_lint(cx, MULTIPLE_CRATE_VERSIONS, DUMMY_SP, "could not read cargo metadata"); - return; - }; - + let metadata = unwrap_cargo_metadata!(cx, MULTIPLE_CRATE_VERSIONS, true); let local_name = cx.tcx.crate_name(LOCAL_CRATE).as_str(); let mut packages = metadata.packages; packages.sort_by(|a, b| a.name.cmp(&b.name)); diff --git a/clippy_lints/src/mut_reference.rs b/clippy_lints/src/mut_reference.rs index 67a1ac78a677..58a8e1a1064a 100644 --- a/clippy_lints/src/mut_reference.rs +++ b/clippy_lints/src/mut_reference.rs @@ -16,7 +16,11 @@ declare_clippy_lint! { /// /// **Example:** /// ```ignore + /// // Bad /// my_vec.push(&mut value) + /// + /// // Good + /// my_vec.push(&value) /// ``` pub UNNECESSARY_MUT_PASSED, style, diff --git a/clippy_lints/src/mutex_atomic.rs b/clippy_lints/src/mutex_atomic.rs index 4e1a8be4892e..78b15afc5a7f 100644 --- a/clippy_lints/src/mutex_atomic.rs +++ b/clippy_lints/src/mutex_atomic.rs @@ -22,9 +22,15 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust + /// # let y = true; + /// + /// // Bad /// # use std::sync::Mutex; - /// # let y = 1; /// let x = Mutex::new(&y); + /// + /// // Good + /// # use std::sync::atomic::AtomicBool; + /// let x = AtomicBool::new(y); /// ``` pub MUTEX_ATOMIC, perf, @@ -46,6 +52,10 @@ declare_clippy_lint! { /// ```rust /// # use std::sync::Mutex; /// let x = Mutex::new(0usize); + /// + /// // Good + /// # use std::sync::atomic::AtomicUsize; + /// let x = AtomicUsize::new(0usize); /// ``` pub MUTEX_INTEGER, nursery, diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs index efa77db822dd..15b129fa0980 100644 --- a/clippy_lints/src/needless_bool.rs +++ b/clippy_lints/src/needless_bool.rs @@ -15,8 +15,7 @@ use rustc_span::Span; declare_clippy_lint! { /// **What it does:** Checks for expressions of the form `if c { true } else { - /// false }` - /// (or vice versa) and suggest using the condition directly. + /// false }` (or vice versa) and suggests using the condition directly. /// /// **Why is this bad?** Redundant code. /// diff --git a/clippy_lints/src/needless_borrow.rs b/clippy_lints/src/needless_borrow.rs index 9ee875d7516e..5880d1d61020 100644 --- a/clippy_lints/src/needless_borrow.rs +++ b/clippy_lints/src/needless_borrow.rs @@ -18,12 +18,16 @@ declare_clippy_lint! { /// **Why is this bad?** Suggests that the receiver of the expression borrows /// the expression. /// + /// **Known problems:** None. + /// /// **Example:** /// ```rust + /// // Bad /// let x: &i32 = &&&&&&5; - /// ``` /// - /// **Known problems:** None. + /// // Good + /// let x: &i32 = &5; + /// ``` pub NEEDLESS_BORROW, nursery, "taking a reference that is going to be automatically dereferenced" diff --git a/clippy_lints/src/needless_continue.rs b/clippy_lints/src/needless_continue.rs index 28183810df48..a971d041ca66 100644 --- a/clippy_lints/src/needless_continue.rs +++ b/clippy_lints/src/needless_continue.rs @@ -424,7 +424,7 @@ fn erode_from_back(s: &str) -> String { } fn span_of_first_expr_in_block(block: &ast::Block) -> Option { - block.stmts.iter().next().map(|stmt| stmt.span) + block.stmts.get(0).map(|stmt| stmt.span) } #[cfg(test)] diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index 218b0d27f748..fbdf927b8247 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/clippy_lints/src/needless_pass_by_value.rs @@ -40,9 +40,8 @@ declare_clippy_lint! { /// assert_eq!(v.len(), 42); /// } /// ``` - /// + /// should be /// ```rust - /// // should be /// fn foo(v: &[i32]) { /// assert_eq!(v.len(), 42); /// } @@ -173,13 +172,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue { !preds.is_empty() && { let ty_empty_region = cx.tcx.mk_imm_ref(cx.tcx.lifetimes.re_root_empty, ty); preds.iter().all(|t| { - let ty_params = &t - .skip_binder() - .trait_ref - .substs - .iter() - .skip(1) - .collect::>(); + let ty_params = &t.skip_binder().trait_ref.substs.iter().skip(1).collect::>(); implements_trait(cx, ty_empty_region, t.def_id(), ty_params) }) }, diff --git a/clippy_lints/src/needless_update.rs b/clippy_lints/src/needless_update.rs index 4b2586877e56..d866bab2f642 100644 --- a/clippy_lints/src/needless_update.rs +++ b/clippy_lints/src/needless_update.rs @@ -21,6 +21,16 @@ declare_clippy_lint! { /// # z: i32, /// # } /// # let zero_point = Point { x: 0, y: 0, z: 0 }; + /// + /// // Bad + /// Point { + /// x: 1, + /// y: 1, + /// z: 1, + /// ..zero_point + /// }; + /// + /// // Ok /// Point { /// x: 1, /// y: 1, diff --git a/clippy_lints/src/new_without_default.rs b/clippy_lints/src/new_without_default.rs index e556e5d59c18..dd236535c18a 100644 --- a/clippy_lints/src/new_without_default.rs +++ b/clippy_lints/src/new_without_default.rs @@ -1,13 +1,13 @@ use crate::utils::paths; use crate::utils::sugg::DiagnosticBuilderExt; -use crate::utils::{get_trait_def_id, return_ty, same_tys, span_lint_hir_and_then}; +use crate::utils::{get_trait_def_id, return_ty, span_lint_hir_and_then}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::HirIdSet; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; -use rustc_middle::ty::Ty; +use rustc_middle::ty::{Ty, TyS}; use rustc_session::{declare_tool_lint, impl_lint_pass}; declare_clippy_lint! { @@ -93,7 +93,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault { let self_def_id = cx.tcx.hir().local_def_id(cx.tcx.hir().get_parent_item(id)); let self_ty = cx.tcx.type_of(self_def_id); if_chain! { - if same_tys(cx, self_ty, return_ty(cx, id)); + if TyS::same_type(self_ty, return_ty(cx, id)); if let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT); then { if self.impling_types.is_none() { diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index 4eac571f9662..c77b44e0c99c 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -47,7 +47,11 @@ declare_clippy_lint! { /// /// **Example:** /// ```ignore + /// // Bad /// fn foo(&Vec) { .. } + /// + /// // Good + /// fn foo(&[u32]) { .. } /// ``` pub PTR_ARG, style, @@ -65,9 +69,15 @@ declare_clippy_lint! { /// /// **Example:** /// ```ignore + /// // Bad /// if x == ptr::null { /// .. /// } + /// + /// // Good + /// if x.is_null() { + /// .. + /// } /// ``` pub CMP_NULL, style, @@ -76,19 +86,16 @@ declare_clippy_lint! { declare_clippy_lint! { /// **What it does:** This lint checks for functions that take immutable - /// references and return - /// mutable ones. + /// references and return mutable ones. /// /// **Why is this bad?** This is trivially unsound, as one can create two - /// mutable references - /// from the same (immutable!) source. This - /// [error](https://github.com/rust-lang/rust/issues/39465) + /// mutable references from the same (immutable!) source. + /// This [error](https://github.com/rust-lang/rust/issues/39465) /// actually lead to an interim Rust release 1.15.1. /// /// **Known problems:** To be on the conservative side, if there's at least one - /// mutable reference - /// with the output lifetime, this lint will not trigger. In practice, this - /// case is unlikely anyway. + /// mutable reference with the output lifetime, this lint will not trigger. + /// In practice, this case is unlikely anyway. /// /// **Example:** /// ```ignore diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index ea654467b866..e4361b00fb4c 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -88,7 +88,7 @@ impl QuestionMark { replacement_str, applicability, ) - } + } } } } diff --git a/clippy_lints/src/reference.rs b/clippy_lints/src/reference.rs index d5797468e9d5..fe457aad50e3 100644 --- a/clippy_lints/src/reference.rs +++ b/clippy_lints/src/reference.rs @@ -16,8 +16,13 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust,ignore + /// // Bad /// let a = f(*&mut b); /// let c = *&d; + /// + /// // Good + /// let a = f(b); + /// let c = d; /// ``` pub DEREF_ADDROF, complexity, diff --git a/clippy_lints/src/regex.rs b/clippy_lints/src/regex.rs index 30084e3e1ffc..a2c35c426734 100644 --- a/clippy_lints/src/regex.rs +++ b/clippy_lints/src/regex.rs @@ -86,11 +86,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Regex { if let Some(span) = is_expn_of(expr.span, "regex"); then { if !self.spans.contains(&span) { - span_lint(cx, - REGEX_MACRO, - span, - "`regex!(_)` found. \ - Please use `Regex::new(_)`, which is faster for now."); + span_lint( + cx, + REGEX_MACRO, + span, + "`regex!(_)` found. \ + Please use `Regex::new(_)`, which is faster for now." + ); self.spans.insert(span); } self.last = Some(block.hir_id); diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs index 11360b0ef849..68c36f918918 100644 --- a/clippy_lints/src/shadow.rs +++ b/clippy_lints/src/shadow.rs @@ -25,7 +25,12 @@ declare_clippy_lint! { /// **Example:** /// ```rust /// # let x = 1; + /// + /// // Bad /// let x = &x; + /// + /// // Good + /// let y = &x; // use different variable name /// ``` pub SHADOW_SAME, restriction, @@ -77,7 +82,12 @@ declare_clippy_lint! { /// # let y = 1; /// # let z = 2; /// let x = y; + /// + /// // Bad /// let x = z; // shadows the earlier binding + /// + /// // Good + /// let w = z; // use different variable name /// ``` pub SHADOW_UNRELATED, pedantic, diff --git a/clippy_lints/src/single_component_path_imports.rs b/clippy_lints/src/single_component_path_imports.rs index 8d767a7fec88..2e853e8301d6 100644 --- a/clippy_lints/src/single_component_path_imports.rs +++ b/clippy_lints/src/single_component_path_imports.rs @@ -16,7 +16,7 @@ declare_clippy_lint! { /// /// **Example:** /// - /// ```rust, ignore + /// ```rust,ignore /// use regex; /// /// fn main() { @@ -24,7 +24,7 @@ declare_clippy_lint! { /// } /// ``` /// Better as - /// ```rust, ignore + /// ```rust,ignore /// fn main() { /// regex::Regex::new(r"^\d{4}-\d{2}-\d{2}$").unwrap(); /// } diff --git a/clippy_lints/src/slow_vector_initialization.rs b/clippy_lints/src/slow_vector_initialization.rs index fb3706be1c21..a7c4f2c2291f 100644 --- a/clippy_lints/src/slow_vector_initialization.rs +++ b/clippy_lints/src/slow_vector_initialization.rs @@ -22,11 +22,17 @@ declare_clippy_lint! { /// ```rust /// # use core::iter::repeat; /// # let len = 4; + /// + /// // Bad /// let mut vec1 = Vec::with_capacity(len); /// vec1.resize(len, 0); /// /// let mut vec2 = Vec::with_capacity(len); - /// vec2.extend(repeat(0).take(len)) + /// vec2.extend(repeat(0).take(len)); + /// + /// // Good + /// let mut vec1 = vec![0; len]; + /// let mut vec2 = vec![0; len]; /// ``` pub SLOW_VECTOR_INITIALIZATION, perf, diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index 2c51271e312d..f84566ef707a 100644 --- a/clippy_lints/src/strings.rs +++ b/clippy_lints/src/strings.rs @@ -24,6 +24,10 @@ declare_clippy_lint! { /// ```rust /// let mut x = "Hello".to_owned(); /// x = x + ", World"; + /// + /// // More readable + /// x += ", World"; + /// x.push_str(", World"); /// ``` pub STRING_ADD_ASSIGN, pedantic, @@ -69,7 +73,11 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust + /// // Bad /// let bs = "a byte string".as_bytes(); + /// + /// // Good + /// let bs = b"a byte string"; /// ``` pub STRING_LIT_AS_BYTES, style, diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 6ed9ff22e466..bc5fe44b30f8 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -10,14 +10,14 @@ use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitorMap, Visitor}; use rustc_hir::{ - BinOpKind, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericParamKind, HirId, ImplItem, + BinOpKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericParamKind, HirId, ImplItem, ImplItemKind, Item, ItemKind, Lifetime, Local, MatchSource, MutTy, Mutability, QPath, Stmt, StmtKind, TraitFn, TraitItem, TraitItemKind, TyKind, UnOp, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::hir::map::Map; use rustc_middle::lint::in_external_macro; -use rustc_middle::ty::{self, InferTy, Ty, TyCtxt, TypeckTables}; +use rustc_middle::ty::{self, InferTy, Ty, TyCtxt, TyS, TypeckTables}; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use rustc_span::hygiene::{ExpnKind, MacroKind}; use rustc_span::source_map::Span; @@ -29,10 +29,10 @@ use rustc_typeck::hir_ty_to_ty; use crate::consts::{constant, Constant}; use crate::utils::paths; use crate::utils::{ - clip, comparisons, differing_macro_contexts, higher, in_constant, int_bits, is_type_diagnostic_item, + clip, comparisons, differing_macro_contexts, higher, in_constant, indent_of, int_bits, is_type_diagnostic_item, last_path_segment, match_def_path, match_path, method_chain_args, multispan_sugg, numeric_literal::NumericLiteral, - qpath_res, same_tys, sext, snippet, snippet_opt, snippet_with_applicability, snippet_with_macro_callsite, - span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, unsext, + qpath_res, sext, snippet, snippet_block_with_applicability, snippet_opt, snippet_with_applicability, + snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, unsext, }; declare_clippy_lint! { @@ -779,24 +779,22 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnitArg { match expr.kind { ExprKind::Call(_, args) | ExprKind::MethodCall(_, _, args) => { - for arg in args { - if is_unit(cx.tables.expr_ty(arg)) && !is_unit_literal(arg) { - if let ExprKind::Match(.., match_source) = &arg.kind { - if *match_source == MatchSource::TryDesugar { - continue; + let args_to_recover = args + .iter() + .filter(|arg| { + if is_unit(cx.tables.expr_ty(arg)) && !is_unit_literal(arg) { + if let ExprKind::Match(.., MatchSource::TryDesugar) = &arg.kind { + false + } else { + true } + } else { + false } - - span_lint_and_sugg( - cx, - UNIT_ARG, - arg.span, - "passing a unit value to a function", - "if you intended to pass a unit value, use a unit literal instead", - "()".to_string(), - Applicability::MaybeIncorrect, - ); - } + }) + .collect::>(); + if !args_to_recover.is_empty() { + lint_unit_args(cx, expr, &args_to_recover); } }, _ => (), @@ -804,6 +802,101 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnitArg { } } +fn lint_unit_args(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args_to_recover: &[&Expr<'_>]) { + let mut applicability = Applicability::MachineApplicable; + let (singular, plural) = if args_to_recover.len() > 1 { + ("", "s") + } else { + ("a ", "") + }; + span_lint_and_then( + cx, + UNIT_ARG, + expr.span, + &format!("passing {}unit value{} to a function", singular, plural), + |db| { + let mut or = ""; + args_to_recover + .iter() + .filter_map(|arg| { + if_chain! { + if let ExprKind::Block(block, _) = arg.kind; + if block.expr.is_none(); + if let Some(last_stmt) = block.stmts.iter().last(); + if let StmtKind::Semi(last_expr) = last_stmt.kind; + if let Some(snip) = snippet_opt(cx, last_expr.span); + then { + Some(( + last_stmt.span, + snip, + )) + } + else { + None + } + } + }) + .for_each(|(span, sugg)| { + db.span_suggestion( + span, + "remove the semicolon from the last statement in the block", + sugg, + Applicability::MaybeIncorrect, + ); + or = "or "; + }); + let sugg = args_to_recover + .iter() + .filter(|arg| !is_empty_block(arg)) + .enumerate() + .map(|(i, arg)| { + let indent = if i == 0 { + 0 + } else { + indent_of(cx, expr.span).unwrap_or(0) + }; + format!( + "{}{};", + " ".repeat(indent), + snippet_block_with_applicability(cx, arg.span, "..", Some(expr.span), &mut applicability) + ) + }) + .collect::>(); + let mut and = ""; + if !sugg.is_empty() { + let plural = if sugg.len() > 1 { "s" } else { "" }; + db.span_suggestion( + expr.span.with_hi(expr.span.lo()), + &format!("{}move the expression{} in front of the call...", or, plural), + format!("{}\n", sugg.join("\n")), + applicability, + ); + and = "...and " + } + db.multipart_suggestion( + &format!("{}use {}unit literal{} instead", and, singular, plural), + args_to_recover + .iter() + .map(|arg| (arg.span, "()".to_string())) + .collect::>(), + applicability, + ); + }, + ); +} + +fn is_empty_block(expr: &Expr<'_>) -> bool { + matches!( + expr.kind, + ExprKind::Block( + Block { + stmts: &[], expr: None, .. + }, + _, + ) + ) +} + fn is_questionmark_desugar_marked_call(expr: &Expr<'_>) -> bool { use rustc_span::hygiene::DesugaringKind; if let ExprKind::Call(ref callee, _) = expr.kind { @@ -974,7 +1067,8 @@ declare_clippy_lint! { /// behavior. /// /// **Known problems:** Using `std::ptr::read_unaligned` and `std::ptr::write_unaligned` or similar - /// on the resulting pointer is fine. + /// on the resulting pointer is fine. Is over-zealous: Casts with manual alignment checks or casts like + /// u64-> u8 -> u16 can be fine. Miri is able to do a more in-depth analysis. /// /// **Example:** /// ```rust @@ -982,7 +1076,7 @@ declare_clippy_lint! { /// let _ = (&mut 1u8 as *mut u8) as *mut u16; /// ``` pub CAST_PTR_ALIGNMENT, - correctness, + pedantic, "cast from a pointer to a more-strictly-aligned pointer" } @@ -2462,7 +2556,7 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for ImplicitHasherConstructorVisitor<'a, 'b, 't if let ExprKind::Path(QPath::TypeRelative(ref ty, ref method)) = fun.kind; if let TyKind::Path(QPath::Resolved(None, ty_path)) = ty.kind; then { - if !same_tys(self.cx, self.target.ty(), self.body.expr_ty(e)) { + if !TyS::same_type(self.target.ty(), self.body.expr_ty(e)) { return; } diff --git a/clippy_lints/src/unnecessary_sort_by.rs b/clippy_lints/src/unnecessary_sort_by.rs new file mode 100644 index 000000000000..33d8331c2923 --- /dev/null +++ b/clippy_lints/src/unnecessary_sort_by.rs @@ -0,0 +1,267 @@ +use crate::utils; +use crate::utils::paths; +use crate::utils::sugg::Sugg; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, Mutability, Param, Pat, PatKind, Path, PathSegment, QPath}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::Ident; + +declare_clippy_lint! { + /// **What it does:** + /// Detects when people use `Vec::sort_by` and pass in a function + /// which compares the two arguments, either directly or indirectly. + /// + /// **Why is this bad?** + /// It is more clear to use `Vec::sort_by_key` (or `Vec::sort` if + /// possible) than to use `Vec::sort_by` and and a more complicated + /// closure. + /// + /// **Known problems:** + /// If the suggested `Vec::sort_by_key` uses Reverse and it isn't + /// imported by a use statement in the current frame, then a `use` + /// statement that imports it will need to be added (which this lint + /// can't do). + /// + /// **Example:** + /// + /// ```rust + /// # struct A; + /// # impl A { fn foo(&self) {} } + /// # let mut vec: Vec = Vec::new(); + /// vec.sort_by(|a, b| a.foo().cmp(&b.foo())); + /// ``` + /// Use instead: + /// ```rust + /// # struct A; + /// # impl A { fn foo(&self) {} } + /// # let mut vec: Vec = Vec::new(); + /// vec.sort_by_key(|a| a.foo()); + /// ``` + pub UNNECESSARY_SORT_BY, + complexity, + "Use of `Vec::sort_by` when `Vec::sort_by_key` or `Vec::sort` would be clearer" +} + +declare_lint_pass!(UnnecessarySortBy => [UNNECESSARY_SORT_BY]); + +enum LintTrigger { + Sort(SortDetection), + SortByKey(SortByKeyDetection), +} + +struct SortDetection { + vec_name: String, + unstable: bool, +} + +struct SortByKeyDetection { + vec_name: String, + closure_arg: String, + closure_body: String, + reverse: bool, + unstable: bool, +} + +/// Detect if the two expressions are mirrored (identical, except one +/// contains a and the other replaces it with b) +fn mirrored_exprs( + cx: &LateContext<'_, '_>, + a_expr: &Expr<'_>, + a_ident: &Ident, + b_expr: &Expr<'_>, + b_ident: &Ident, +) -> bool { + match (&a_expr.kind, &b_expr.kind) { + // Two boxes with mirrored contents + (ExprKind::Box(left_expr), ExprKind::Box(right_expr)) => { + mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident) + }, + // Two arrays with mirrored contents + (ExprKind::Array(left_exprs), ExprKind::Array(right_exprs)) => left_exprs + .iter() + .zip(right_exprs.iter()) + .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)), + // The two exprs are function calls. + // Check to see that the function itself and its arguments are mirrored + (ExprKind::Call(left_expr, left_args), ExprKind::Call(right_expr, right_args)) => { + mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident) + && left_args + .iter() + .zip(right_args.iter()) + .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)) + }, + // The two exprs are method calls. + // Check to see that the function is the same and the arguments are mirrored + // This is enough because the receiver of the method is listed in the arguments + (ExprKind::MethodCall(left_segment, _, left_args), ExprKind::MethodCall(right_segment, _, right_args)) => { + left_segment.ident == right_segment.ident + && left_args + .iter() + .zip(right_args.iter()) + .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)) + }, + // Two tuples with mirrored contents + (ExprKind::Tup(left_exprs), ExprKind::Tup(right_exprs)) => left_exprs + .iter() + .zip(right_exprs.iter()) + .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)), + // Two binary ops, which are the same operation and which have mirrored arguments + (ExprKind::Binary(left_op, left_left, left_right), ExprKind::Binary(right_op, right_left, right_right)) => { + left_op.node == right_op.node + && mirrored_exprs(cx, left_left, a_ident, right_left, b_ident) + && mirrored_exprs(cx, left_right, a_ident, right_right, b_ident) + }, + // Two unary ops, which are the same operation and which have the same argument + (ExprKind::Unary(left_op, left_expr), ExprKind::Unary(right_op, right_expr)) => { + left_op == right_op && mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident) + }, + // The two exprs are literals of some kind + (ExprKind::Lit(left_lit), ExprKind::Lit(right_lit)) => left_lit.node == right_lit.node, + (ExprKind::Cast(left, _), ExprKind::Cast(right, _)) => mirrored_exprs(cx, left, a_ident, right, b_ident), + (ExprKind::DropTemps(left_block), ExprKind::DropTemps(right_block)) => { + mirrored_exprs(cx, left_block, a_ident, right_block, b_ident) + }, + (ExprKind::Field(left_expr, left_ident), ExprKind::Field(right_expr, right_ident)) => { + left_ident.name == right_ident.name && mirrored_exprs(cx, left_expr, a_ident, right_expr, right_ident) + }, + // Two paths: either one is a and the other is b, or they're identical to each other + ( + ExprKind::Path(QPath::Resolved( + _, + Path { + segments: left_segments, + .. + }, + )), + ExprKind::Path(QPath::Resolved( + _, + Path { + segments: right_segments, + .. + }, + )), + ) => { + (left_segments + .iter() + .zip(right_segments.iter()) + .all(|(left, right)| left.ident == right.ident) + && left_segments + .iter() + .all(|seg| &seg.ident != a_ident && &seg.ident != b_ident)) + || (left_segments.len() == 1 + && &left_segments[0].ident == a_ident + && right_segments.len() == 1 + && &right_segments[0].ident == b_ident) + }, + // Matching expressions, but one or both is borrowed + ( + ExprKind::AddrOf(left_kind, Mutability::Not, left_expr), + ExprKind::AddrOf(right_kind, Mutability::Not, right_expr), + ) => left_kind == right_kind && mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident), + (_, ExprKind::AddrOf(_, Mutability::Not, right_expr)) => { + mirrored_exprs(cx, a_expr, a_ident, right_expr, b_ident) + }, + (ExprKind::AddrOf(_, Mutability::Not, left_expr), _) => mirrored_exprs(cx, left_expr, a_ident, b_expr, b_ident), + _ => false, + } +} + +fn detect_lint(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> Option { + if_chain! { + if let ExprKind::MethodCall(name_ident, _, args) = &expr.kind; + if let name = name_ident.ident.name.to_ident_string(); + if name == "sort_by" || name == "sort_unstable_by"; + if let [vec, Expr { kind: ExprKind::Closure(_, _, closure_body_id, _, _), .. }] = args; + if utils::match_type(cx, &cx.tables.expr_ty(vec), &paths::VEC); + if let closure_body = cx.tcx.hir().body(*closure_body_id); + if let &[ + Param { pat: Pat { kind: PatKind::Binding(_, _, left_ident, _), .. }, ..}, + Param { pat: Pat { kind: PatKind::Binding(_, _, right_ident, _), .. }, .. } + ] = &closure_body.params; + if let ExprKind::MethodCall(method_path, _, [ref left_expr, ref right_expr]) = &closure_body.value.kind; + if method_path.ident.name.to_ident_string() == "cmp"; + then { + let (closure_body, closure_arg, reverse) = if mirrored_exprs( + &cx, + &left_expr, + &left_ident, + &right_expr, + &right_ident + ) { + (Sugg::hir(cx, &left_expr, "..").to_string(), left_ident.name.to_string(), false) + } else if mirrored_exprs(&cx, &left_expr, &right_ident, &right_expr, &left_ident) { + (Sugg::hir(cx, &left_expr, "..").to_string(), right_ident.name.to_string(), true) + } else { + return None; + }; + let vec_name = Sugg::hir(cx, &args[0], "..").to_string(); + let unstable = name == "sort_unstable_by"; + if_chain! { + if let ExprKind::Path(QPath::Resolved(_, Path { + segments: [PathSegment { ident: left_name, .. }], .. + })) = &left_expr.kind; + if left_name == left_ident; + then { + Some(LintTrigger::Sort(SortDetection { vec_name, unstable })) + } + else { + Some(LintTrigger::SortByKey(SortByKeyDetection { + vec_name, + unstable, + closure_arg, + closure_body, + reverse + })) + } + } + } else { + None + } + } +} + +impl LateLintPass<'_, '_> for UnnecessarySortBy { + fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &Expr<'_>) { + match detect_lint(cx, expr) { + Some(LintTrigger::SortByKey(trigger)) => utils::span_lint_and_sugg( + cx, + UNNECESSARY_SORT_BY, + expr.span, + "use Vec::sort_by_key here instead", + "try", + format!( + "{}.sort{}_by_key(|&{}| {})", + trigger.vec_name, + if trigger.unstable { "_unstable" } else { "" }, + trigger.closure_arg, + if trigger.reverse { + format!("Reverse({})", trigger.closure_body) + } else { + trigger.closure_body.to_string() + }, + ), + if trigger.reverse { + Applicability::MaybeIncorrect + } else { + Applicability::MachineApplicable + }, + ), + Some(LintTrigger::Sort(trigger)) => utils::span_lint_and_sugg( + cx, + UNNECESSARY_SORT_BY, + expr.span, + "use Vec::sort here instead", + "try", + format!( + "{}.sort{}()", + trigger.vec_name, + if trigger.unstable { "_unstable" } else { "" }, + ), + Applicability::MachineApplicable, + ), + None => {}, + } + } +} diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs index 7fa97b246991..141035a980ad 100644 --- a/clippy_lints/src/useless_conversion.rs +++ b/clippy_lints/src/useless_conversion.rs @@ -1,12 +1,12 @@ use crate::utils::{ - is_type_diagnostic_item, match_def_path, match_trait_method, paths, same_tys, snippet, snippet_with_macro_callsite, + is_type_diagnostic_item, match_def_path, match_trait_method, paths, snippet, snippet_with_macro_callsite, span_lint_and_help, span_lint_and_sugg, }; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, HirId, MatchSource}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty; +use rustc_middle::ty::{self, TyS}; use rustc_session::{declare_tool_lint, impl_lint_pass}; declare_clippy_lint! { @@ -65,7 +65,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion { if match_trait_method(cx, e, &paths::INTO) && &*name.ident.as_str() == "into" { let a = cx.tables.expr_ty(e); let b = cx.tables.expr_ty(&args[0]); - if same_tys(cx, a, b) { + if TyS::same_type(a, b) { let sugg = snippet_with_macro_callsite(cx, args[0].span, "").to_string(); span_lint_and_sugg( cx, @@ -81,7 +81,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion { if match_trait_method(cx, e, &paths::INTO_ITERATOR) && &*name.ident.as_str() == "into_iter" { let a = cx.tables.expr_ty(e); let b = cx.tables.expr_ty(&args[0]); - if same_tys(cx, a, b) { + if TyS::same_type(a, b) { let sugg = snippet(cx, args[0].span, "").into_owned(); span_lint_and_sugg( cx, @@ -101,7 +101,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion { if is_type_diagnostic_item(cx, a, sym!(result_type)); if let ty::Adt(_, substs) = a.kind; if let Some(a_type) = substs.types().next(); - if same_tys(cx, a_type, b); + if TyS::same_type(a_type, b); then { span_lint_and_help( @@ -131,7 +131,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion { if is_type_diagnostic_item(cx, a, sym!(result_type)); if let ty::Adt(_, substs) = a.kind; if let Some(a_type) = substs.types().next(); - if same_tys(cx, a_type, b); + if TyS::same_type(a_type, b); then { let hint = format!("consider removing `{}()`", snippet(cx, path.span, "TryFrom::try_from")); @@ -148,7 +148,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion { if_chain! { if match_def_path(cx, def_id, &paths::FROM_FROM); - if same_tys(cx, a, b); + if TyS::same_type(a, b); then { let sugg = snippet(cx, args[0].span.source_callsite(), "").into_owned(); diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 6dd8fef7e82d..06638e7187ba 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -40,7 +40,7 @@ use rustc_hir::{ use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::{LateContext, Level, Lint, LintContext}; use rustc_middle::hir::map::Map; -use rustc_middle::ty::{self, layout::IntegerExt, subst::GenericArg, Binder, Ty, TyCtxt, TypeFoldable}; +use rustc_middle::ty::{self, layout::IntegerExt, subst::GenericArg, Ty, TyCtxt, TypeFoldable}; use rustc_span::hygiene::{ExpnKind, MacroKind}; use rustc_span::source_map::original_sp; use rustc_span::symbol::{self, kw, Symbol}; @@ -165,8 +165,8 @@ pub fn match_trait_method(cx: &LateContext<'_, '_>, expr: &Expr<'_>, path: &[&st /// Checks if an expression references a variable of the given name. pub fn match_var(expr: &Expr<'_>, var: Name) -> bool { if let ExprKind::Path(QPath::Resolved(None, ref path)) = expr.kind { - if path.segments.len() == 1 && path.segments[0].ident.name == var { - return true; + if let [p] = path.segments { + return p.ident.name == var; } } false @@ -181,8 +181,7 @@ pub fn last_path_segment<'tcx>(path: &QPath<'tcx>) -> &'tcx PathSegment<'tcx> { pub fn single_segment_path<'tcx>(path: &QPath<'tcx>) -> Option<&'tcx PathSegment<'tcx>> { match *path { - QPath::Resolved(_, ref path) if path.segments.len() == 1 => Some(&path.segments[0]), - QPath::Resolved(..) => None, + QPath::Resolved(_, ref path) => path.segments.get(0), QPath::TypeRelative(_, ref seg) => Some(seg), } } @@ -201,9 +200,12 @@ pub fn match_qpath(path: &QPath<'_>, segments: &[&str]) -> bool { QPath::Resolved(_, ref path) => match_path(path, segments), QPath::TypeRelative(ref ty, ref segment) => match ty.kind { TyKind::Path(ref inner_path) => { - !segments.is_empty() - && match_qpath(inner_path, &segments[..(segments.len() - 1)]) - && segment.ident.name.as_str() == segments[segments.len() - 1] + if let [prefix @ .., end] = segments { + if match_qpath(inner_path, prefix) { + return segment.ident.name.as_str() == *end; + } + } + false }, _ => false, }, @@ -882,20 +884,6 @@ pub fn return_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, fn_item: hir::HirId) -> T cx.tcx.erase_late_bound_regions(&ret_ty) } -/// Checks if two types are the same. -/// -/// This discards any lifetime annotations, too. -// -// FIXME: this works correctly for lifetimes bounds (`for <'a> Foo<'a>` == -// `for <'b> Foo<'b>`, but not for type parameters). -pub fn same_tys<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, a: Ty<'tcx>, b: Ty<'tcx>) -> bool { - let a = cx.tcx.erase_late_bound_regions(&Binder::bind(a)); - let b = cx.tcx.erase_late_bound_regions(&Binder::bind(b)); - cx.tcx - .infer_ctxt() - .enter(|infcx| infcx.can_eq(cx.param_env, a, b).is_ok()) -} - /// Returns `true` if the given type is an `unsafe` function. pub fn type_is_unsafe_function<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool { match ty.kind { @@ -1408,6 +1396,24 @@ pub fn run_lints(cx: &LateContext<'_, '_>, lints: &[&'static Lint], id: HirId) - }) } +#[macro_export] +macro_rules! unwrap_cargo_metadata { + ($cx: ident, $lint: ident, $deps: expr) => {{ + let mut command = cargo_metadata::MetadataCommand::new(); + if !$deps { + command.no_deps(); + } + + match command.exec() { + Ok(metadata) => metadata, + Err(err) => { + span_lint($cx, $lint, DUMMY_SP, &format!("could not read cargo metadata: {}", err)); + return; + }, + } + }}; +} + #[cfg(test)] mod test { use super::{trim_multiline, without_block_comments}; diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 779da7e6bf23..3b7e9739211b 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -138,5 +138,6 @@ pub const VEC_AS_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_slice"]; pub const VEC_DEQUE: [&str; 4] = ["alloc", "collections", "vec_deque", "VecDeque"]; pub const VEC_FROM_ELEM: [&str; 3] = ["alloc", "vec", "from_elem"]; pub const VEC_NEW: [&str; 4] = ["alloc", "vec", "Vec", "new"]; +pub const VEC_RESIZE: [&str; 4] = ["alloc", "vec", "Vec", "resize"]; pub const WEAK_ARC: [&str; 3] = ["alloc", "sync", "Weak"]; pub const WEAK_RC: [&str; 3] = ["alloc", "rc", "Weak"]; diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs index 1174f4215774..a8d4c7620b1e 100644 --- a/clippy_lints/src/vec.rs +++ b/clippy_lints/src/vec.rs @@ -17,8 +17,14 @@ declare_clippy_lint! { /// **Known problems:** None. /// /// **Example:** - /// ```rust,ignore - /// foo(&vec![1, 2]) + /// ```rust + /// # fn foo(my_vec: &[u8]) {} + /// + /// // Bad + /// foo(&vec![1, 2]); + /// + /// // Good + /// foo(&[1, 2]); /// ``` pub USELESS_VEC, perf, diff --git a/clippy_lints/src/vec_resize_to_zero.rs b/clippy_lints/src/vec_resize_to_zero.rs new file mode 100644 index 000000000000..86cbfa8203d5 --- /dev/null +++ b/clippy_lints/src/vec_resize_to_zero.rs @@ -0,0 +1,59 @@ +use crate::utils::span_lint_and_then; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::source_map::Spanned; + +use crate::utils::{match_def_path, paths}; +use rustc_ast::ast::LitKind; +use rustc_hir as hir; + +declare_clippy_lint! { + /// **What it does:** Finds occurences of `Vec::resize(0, an_int)` + /// + /// **Why is this bad?** This is probably an argument inversion mistake. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust + /// vec!(1, 2, 3, 4, 5).resize(0, 5) + /// ``` + pub VEC_RESIZE_TO_ZERO, + correctness, + "emptying a vector with `resize(0, an_int)` instead of `clear()` is probably an argument inversion mistake" +} + +declare_lint_pass!(VecResizeToZero => [VEC_RESIZE_TO_ZERO]); + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for VecResizeToZero { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { + if_chain! { + if let hir::ExprKind::MethodCall(path_segment, _, ref args) = expr.kind; + if let Some(method_def_id) = cx.tables.type_dependent_def_id(expr.hir_id); + if match_def_path(cx, method_def_id, &paths::VEC_RESIZE) && args.len() == 3; + if let ExprKind::Lit(Spanned { node: LitKind::Int(0, _), .. }) = args[1].kind; + if let ExprKind::Lit(Spanned { node: LitKind::Int(..), .. }) = args[2].kind; + then { + let method_call_span = expr.span.with_lo(path_segment.ident.span.lo()); + span_lint_and_then( + cx, + VEC_RESIZE_TO_ZERO, + expr.span, + "emptying a vector with `resize`", + |db| { + db.help("the arguments may be inverted..."); + db.span_suggestion( + method_call_span, + "...or you can empty the vector with", + "clear()".to_string(), + Applicability::MaybeIncorrect, + ); + }, + ); + } + } + } +} diff --git a/clippy_lints/src/verbose_file_reads.rs b/clippy_lints/src/verbose_file_reads.rs index 4d8d4438d881..7247518e19b9 100644 --- a/clippy_lints/src/verbose_file_reads.rs +++ b/clippy_lints/src/verbose_file_reads.rs @@ -9,6 +9,7 @@ declare_clippy_lint! { /// /// **Why is this bad?** `fs::{read, read_to_string}` provide the same functionality when `buf` is empty with fewer imports and no intermediate values. /// See also: [fs::read docs](https://doc.rust-lang.org/std/fs/fn.read.html), [fs::read_to_string docs](https://doc.rust-lang.org/std/fs/fn.read_to_string.html) + /// /// **Known problems:** None. /// /// **Example:** diff --git a/clippy_lints/src/wildcard_dependencies.rs b/clippy_lints/src/wildcard_dependencies.rs index d8d48eb15358..511518082bec 100644 --- a/clippy_lints/src/wildcard_dependencies.rs +++ b/clippy_lints/src/wildcard_dependencies.rs @@ -34,12 +34,7 @@ impl LateLintPass<'_, '_> for WildcardDependencies { return; } - let metadata = if let Ok(metadata) = cargo_metadata::MetadataCommand::new().no_deps().exec() { - metadata - } else { - span_lint(cx, WILDCARD_DEPENDENCIES, DUMMY_SP, "could not read cargo metadata"); - return; - }; + let metadata = unwrap_cargo_metadata!(cx, WILDCARD_DEPENDENCIES, false); for dep in &metadata.packages[0].dependencies { // VersionReq::any() does not work diff --git a/clippy_lints/src/wildcard_imports.rs b/clippy_lints/src/wildcard_imports.rs index 32d9a45c37d7..b637253bd026 100644 --- a/clippy_lints/src/wildcard_imports.rs +++ b/clippy_lints/src/wildcard_imports.rs @@ -19,8 +19,14 @@ declare_clippy_lint! { /// still around. /// /// **Example:** - /// ```rust + /// ```rust,ignore + /// // Bad /// use std::cmp::Ordering::*; + /// foo(Less); + /// + /// // Good + /// use std::cmp::Ordering; + /// foo(Ordering::Less) /// ``` pub ENUM_GLOB_USE, pedantic, @@ -60,15 +66,15 @@ declare_clippy_lint! { /// /// **Example:** /// - /// Bad: /// ```rust,ignore + /// // Bad /// use crate1::*; /// /// foo(); /// ``` /// - /// Good: /// ```rust,ignore + /// // Good /// use crate1::foo; /// /// foo(); diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index cb769b5a2ce9..732f4b28e06e 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -23,7 +23,11 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust + /// // Bad /// println!(""); + /// + /// // Good + /// println!(); /// ``` pub PRINTLN_EMPTY_STRING, style, @@ -32,8 +36,7 @@ declare_clippy_lint! { declare_clippy_lint! { /// **What it does:** This lint warns when you use `print!()` with a format - /// string that - /// ends in a newline. + /// string that ends in a newline. /// /// **Why is this bad?** You should use `println!()` instead, which appends the /// newline. @@ -125,7 +128,12 @@ declare_clippy_lint! { /// ```rust /// # use std::fmt::Write; /// # let mut buf = String::new(); + /// + /// // Bad /// writeln!(buf, ""); + /// + /// // Good + /// writeln!(buf); /// ``` pub WRITELN_EMPTY_STRING, style, @@ -147,7 +155,12 @@ declare_clippy_lint! { /// # use std::fmt::Write; /// # let mut buf = String::new(); /// # let name = "World"; + /// + /// // Bad /// write!(buf, "Hello {}!\n", name); + /// + /// // Good + /// writeln!(buf, "Hello {}!", name); /// ``` pub WRITE_WITH_NEWLINE, style, @@ -168,7 +181,12 @@ declare_clippy_lint! { /// ```rust /// # use std::fmt::Write; /// # let mut buf = String::new(); + /// + /// // Bad /// writeln!(buf, "{}", "foo"); + /// + /// // Good + /// writeln!(buf, "foo"); /// ``` pub WRITE_LITERAL, style, @@ -279,12 +297,11 @@ impl EarlyLintPass for Write { if let (Some(fmt_str), expr) = self.check_tts(cx, &mac.args.inner_tokens(), true) { if fmt_str.symbol == Symbol::intern("") { let mut applicability = Applicability::MachineApplicable; - let suggestion = match expr { - Some(expr) => snippet_with_applicability(cx, expr.span, "v", &mut applicability), - None => { - applicability = Applicability::HasPlaceholders; - Cow::Borrowed("v") - }, + let suggestion = if let Some(e) = expr { + snippet_with_applicability(cx, e.span, "v", &mut applicability) + } else { + applicability = Applicability::HasPlaceholders; + Cow::Borrowed("v") }; span_lint_and_sugg( diff --git a/clippy_lints/src/zero_div_zero.rs b/clippy_lints/src/zero_div_zero.rs index fb4700d8743f..0820385e01bb 100644 --- a/clippy_lints/src/zero_div_zero.rs +++ b/clippy_lints/src/zero_div_zero.rs @@ -14,7 +14,11 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust - /// 0.0f32 / 0.0; + /// // Bad + /// let nan = 0.0f32 / 0.0; + /// + /// // Good + /// let nan = f32::NAN; /// ``` pub ZERO_DIVIDED_BY_ZERO, complexity, diff --git a/doc/changelog_update.md b/doc/changelog_update.md index 0b80cce6d23e..497264649573 100644 --- a/doc/changelog_update.md +++ b/doc/changelog_update.md @@ -18,7 +18,7 @@ been very rare that Clippy changes were included in a patch release. ### 1. Finding the relevant Clippy commits -Each Rust release ships with its own version of Clippy. The Clippy submodule can +Each Rust release ships with its own version of Clippy. The Clippy subtree can be found in the `tools` directory of the Rust repository. Depending on the current time and what exactly you want to update, the following @@ -32,8 +32,10 @@ bullet points might be helpful: need to select the Rust release tag from the dropdown and then check the commit of the Clippy directory: - ![Explanation of how to find the commit hash](https://user-images.githubusercontent.com/2042399/62846160-1f8b0480-bcce-11e9-9da8-7964ca034e7a.png) - +To find the commit hash, issue the following command when in a `rust-lang/rust` checkout: +``` +git log --oneline -- src/tools/clippy/ | grep -o "Merge commit '[a-f0-9]*' into .*" | head -1 | sed -e "s/Merge commit '\([a-f0-9]*\)' into .*/\1/g" +``` ### 2. Fetching the PRs between those commits @@ -74,5 +76,5 @@ relevant commit ranges. [changelog]: https://github.com/rust-lang/rust-clippy/blob/master/CHANGELOG.md [forge]: https://forge.rust-lang.org/ -[rust_master_tools]: https://github.com/rust-lang/rust/tree/master/src/tools -[rust_beta_tools]: https://github.com/rust-lang/rust/tree/beta/src/tools +[rust_master_tools]: https://github.com/rust-lang/rust/tree/master/src/tools/clippy +[rust_beta_tools]: https://github.com/rust-lang/rust/tree/beta/src/tools/clippy diff --git a/doc/common_tools_writing_lints.md b/doc/common_tools_writing_lints.md index ed33b37c6bd1..dbc434505947 100644 --- a/doc/common_tools_writing_lints.md +++ b/doc/common_tools_writing_lints.md @@ -4,7 +4,9 @@ You may need following tooltips to catch up with common operations. - [Common tools for writing lints](#common-tools-for-writing-lints) - [Retrieving the type of an expression](#retrieving-the-type-of-an-expression) + - [Checking if an expression is calling a specific method](#checking-if-an-expr-is-calling-a-specific-method) - [Checking if a type implements a specific trait](#checking-if-a-type-implements-a-specific-trait) + - [Checking if a type defines a method](#checking-if-a-type-defines-a-method) - [Dealing with macros](#dealing-with-macros) Useful Rustc dev guide links: @@ -49,6 +51,26 @@ Two noticeable items here: - `tables` is [`TypeckTables`][TypeckTables] and is created by type checking step, it includes useful information such as types of expressions, ways to resolve methods and so on. +# Checking if an expr is calling a specific method + +Starting with an `expr`, you can check whether it is calling a specific method `some_method`: + +```rust +impl LateLintPass<'_, '_> for MyStructLint { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<'_>) { + if_chain! { + // Check our expr is calling a method + if let hir::ExprKind::MethodCall(path, _, _args) = &expr.kind; + // Check the name of this method is `some_method` + if path.ident.name == sym!(some_method); + then { + // ... + } + } + } +} +``` + # Checking if a type implements a specific trait There are two ways to do this, depending if the target trait is part of lang items. @@ -83,6 +105,32 @@ A list of defined paths for Clippy can be found in [paths.rs][paths] We access lang items through the type context `tcx`. `tcx` is of type [`TyCtxt`][TyCtxt] and is defined in the `rustc_middle` crate. +# Checking if a type defines a specific method + +To check if our type defines a method called `some_method`: + +```rust +use crate::utils::{is_type_diagnostic_item, return_ty}; + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MyTypeImpl { + fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, impl_item: &'tcx ImplItem<'_>) { + if_chain! { + // Check if item is a method/function + if let ImplItemKind::Fn(ref signature, _) = impl_item.kind; + // Check the method is named `some_method` + if impl_item.ident.name == sym!(some_method); + // We can also check it has a parameter `self` + if signature.decl.implicit_self.has_implicit_self(); + // We can go further and even check if its return type is `String` + if is_type_diagnostic_item(cx, return_ty(cx, impl_item.hir_id), sym!(string_type)); + then { + // ... + } + } + } +} +``` + # Dealing with macros There are several helpers in Clippy's utils to deal with macros: diff --git a/doc/release.md b/doc/release.md index 9d69fa8a7f69..391952ea6b14 100644 --- a/doc/release.md +++ b/doc/release.md @@ -7,11 +7,11 @@ Clippy is released together with stable Rust releases. The dates for these releases can be found at the [Rust Forge]. This document explains the necessary steps to create a Clippy release. -1. [Find the Clippy commit](#find-the-clippy-commit) -2. [Tag the stable commit](#tag-the-stable-commit) -3. [Update `CHANGELOG.md`](#update-changelogmd) -4. [Remerge the `beta` branch](#remerge-the-beta-branch) -5. [Update the `beta` branch](#update-the-beta-branch) +1. [Remerge the `beta` branch](#remerge-the-beta-branch) +2. [Update the `beta` branch](#update-the-beta-branch) +3. [Find the Clippy commit](#find-the-clippy-commit) +4. [Tag the stable commit](#tag-the-stable-commit) +5. [Update `CHANGELOG.md`](#update-changelogmd) _NOTE: This document is for stable Rust releases, not for point releases. For point releases, step 1. and 2. should be enough._ @@ -19,43 +19,6 @@ point releases, step 1. and 2. should be enough._ [Rust Forge]: https://forge.rust-lang.org/ -## Find the Clippy commit - -The first step is to tag the Clippy commit, that is included in the stable Rust -release. This commit can be found in the Rust repository. - -```bash -# Assuming the current directory corresponds to the Rust repository -$ git fetch upstream # `upstream` is the `rust-lang/rust` remote -$ git checkout 1.XX.0 # XX should be exchanged with the corresponding version -$ git submodule update -$ SHA=$(git submodule status src/tools/clippy | awk '{print $1}') -``` - - -## Tag the stable commit - -After finding the Clippy commit, it can be tagged with the release number. - -```bash -# Assuming the current directory corresponds to the Clippy repository -$ git checkout $SHA -$ git tag rust-1.XX.0 # XX should be exchanged with the corresponding version -$ git push upstream master --tags # `upstream` is the `rust-lang/rust-clippy` remote -``` - -After this, the release should be available on the Clippy [release page]. - -[release page]: https://github.com/rust-lang/rust-clippy/releases - - -## Update `CHANGELOG.md` - -For this see the document on [how to update the changelog]. - -[how to update the changelog]: https://github.com/rust-lang/rust-clippy/blob/master/doc/changelog_update.md - - ## Remerge the `beta` branch This step is only necessary, if since the last release something was backported @@ -76,7 +39,7 @@ If this command outputs `master`, this step is **not** necessary. ```bash # Assuming `HEAD` is the current `master` branch of rust-lang/rust-clippy $ git checkout -b backport_remerge -$ git merge beta +$ git merge upstream/beta $ git diff # This diff has to be empty, otherwise something with the remerge failed $ git push origin backport_remerge # This can be pushed to your fork ``` @@ -96,8 +59,7 @@ determined. ```bash # Assuming the current directory corresponds to the Rust repository $ git checkout beta -$ git submodule update -$ BETA_SHA=$(git submodule status src/tools/clippy | awk '{print $1}') +$ BETA_SHA=$(git log --oneline -- src/tools/clippy/ | grep -o "Merge commit '[a-f0-9]*' into .*" | head -1 | sed -e "s/Merge commit '\([a-f0-9]*\)' into .*/\1/g") ``` After finding the Clippy commit, the `beta` branch in the Clippy repository can @@ -109,3 +71,39 @@ $ git checkout beta $ git rebase $BETA_SHA $ git push upstream beta ``` + + +## Find the Clippy commit + +The first step is to tag the Clippy commit, that is included in the stable Rust +release. This commit can be found in the Rust repository. + +```bash +# Assuming the current directory corresponds to the Rust repository +$ git fetch upstream # `upstream` is the `rust-lang/rust` remote +$ git checkout 1.XX.0 # XX should be exchanged with the corresponding version +$ SHA=$(git log --oneline -- src/tools/clippy/ | grep -o "Merge commit '[a-f0-9]*' into .*" | head -1 | sed -e "s/Merge commit '\([a-f0-9]*\)' into .*/\1/g") +``` + + +## Tag the stable commit + +After finding the Clippy commit, it can be tagged with the release number. + +```bash +# Assuming the current directory corresponds to the Clippy repository +$ git checkout $SHA +$ git tag rust-1.XX.0 # XX should be exchanged with the corresponding version +$ git push upstream master --tags # `upstream` is the `rust-lang/rust-clippy` remote +``` + +After this, the release should be available on the Clippy [release page]. + +[release page]: https://github.com/rust-lang/rust-clippy/releases + + +## Update `CHANGELOG.md` + +For this see the document on [how to update the changelog]. + +[how to update the changelog]: https://github.com/rust-lang/rust-clippy/blob/master/doc/changelog_update.md diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index f63301c7db0a..d5d07ccb2eb0 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -166,7 +166,7 @@ pub static ref ALL_LINTS: Vec = vec![ }, Lint { name: "cast_ptr_alignment", - group: "correctness", + group: "pedantic", desc: "cast from a pointer to a more-strictly-aligned pointer", deprecation: None, module: "types", @@ -934,6 +934,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "loops", }, + Lint { + name: "iter_next_slice", + group: "style", + desc: "using `.iter().next()` on a sliced array, which can be shortened to just `.get()`", + deprecation: None, + module: "methods", + }, Lint { name: "iter_nth", group: "perf", @@ -1735,7 +1742,7 @@ pub static ref ALL_LINTS: Vec = vec![ Lint { name: "pub_enum_variant_names", group: "pedantic", - desc: "enums where all variants share a prefix/postfix", + desc: "public enums where all variants share a prefix/postfix", deprecation: None, module: "enum_variants", }, @@ -2292,6 +2299,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "no_effect", }, + Lint { + name: "unnecessary_sort_by", + group: "complexity", + desc: "Use of `Vec::sort_by` when `Vec::sort_by_key` or `Vec::sort` would be clearer", + deprecation: None, + module: "unnecessary_sort_by", + }, Lint { name: "unnecessary_unwrap", group: "complexity", @@ -2460,6 +2474,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "types", }, + Lint { + name: "vec_resize_to_zero", + group: "correctness", + desc: "emptying a vector with `resize(0, an_int)` instead of `clear()` is probably an argument inversion mistake", + deprecation: None, + module: "vec_resize_to_zero", + }, Lint { name: "verbose_bit_mask", group: "style", diff --git a/tests/compile-test.rs b/tests/compile-test.rs index 1c4914a470c9..11b3f69a828c 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -153,9 +153,6 @@ fn run_ui_toml(config: &mut compiletest::Config) { } fn run_ui_cargo(config: &mut compiletest::Config) { - if cargo::is_rustc_test_suite() { - return; - } fn run_tests( config: &compiletest::Config, filter: &Option, @@ -184,8 +181,15 @@ fn run_ui_cargo(config: &mut compiletest::Config) { } let src_path = case.path().join("src"); - env::set_current_dir(&src_path)?; + // When switching between branches, if the previous branch had a test + // that the current branch does not have, the directory is not removed + // because an ignored Cargo.lock file exists. + if !src_path.exists() { + continue; + } + + env::set_current_dir(&src_path)?; for file in fs::read_dir(&src_path)? { let file = file?; if file.file_type()?.is_dir() { diff --git a/tests/ui-cargo/cargo_common_metadata/fail/Cargo.toml b/tests/ui-cargo/cargo_common_metadata/fail/Cargo.toml index c64adcf7c013..ae0a60329962 100644 --- a/tests/ui-cargo/cargo_common_metadata/fail/Cargo.toml +++ b/tests/ui-cargo/cargo_common_metadata/fail/Cargo.toml @@ -2,3 +2,5 @@ name = "cargo_common_metadata" version = "0.1.0" publish = false + +[workspace] diff --git a/tests/ui-cargo/cargo_common_metadata/pass/Cargo.toml b/tests/ui-cargo/cargo_common_metadata/pass/Cargo.toml index c8233f328bb0..737e84e963c9 100644 --- a/tests/ui-cargo/cargo_common_metadata/pass/Cargo.toml +++ b/tests/ui-cargo/cargo_common_metadata/pass/Cargo.toml @@ -9,3 +9,5 @@ readme = "README.md" license = "MIT OR Apache-2.0" keywords = ["metadata", "lint", "clippy"] categories = ["development-tools::testing"] + +[workspace] diff --git a/tests/ui-cargo/multiple_crate_versions/5041_allow_dev_build/Cargo.toml b/tests/ui-cargo/multiple_crate_versions/5041_allow_dev_build/Cargo.toml index 72731fbc75d0..278bebbbd9e8 100644 --- a/tests/ui-cargo/multiple_crate_versions/5041_allow_dev_build/Cargo.toml +++ b/tests/ui-cargo/multiple_crate_versions/5041_allow_dev_build/Cargo.toml @@ -5,6 +5,8 @@ name = "multiple_crate_versions" version = "0.1.0" publish = false +[workspace] + # One of the versions of winapi is only a dev dependency: allowed [dependencies] ctrlc = "=3.1.0" diff --git a/tests/ui-cargo/multiple_crate_versions/fail/Cargo.toml b/tests/ui-cargo/multiple_crate_versions/fail/Cargo.toml index 3a94b723f3fd..4f97b0113340 100644 --- a/tests/ui-cargo/multiple_crate_versions/fail/Cargo.toml +++ b/tests/ui-cargo/multiple_crate_versions/fail/Cargo.toml @@ -3,6 +3,8 @@ name = "multiple_crate_versions" version = "0.1.0" publish = false +[workspace] + [dependencies] ctrlc = "=3.1.0" ansi_term = "=0.11.0" diff --git a/tests/ui-cargo/multiple_crate_versions/pass/Cargo.toml b/tests/ui-cargo/multiple_crate_versions/pass/Cargo.toml index a9b06420b333..b4b49bb369ac 100644 --- a/tests/ui-cargo/multiple_crate_versions/pass/Cargo.toml +++ b/tests/ui-cargo/multiple_crate_versions/pass/Cargo.toml @@ -3,6 +3,8 @@ name = "cargo_common_metadata" version = "0.1.0" publish = false +[workspace] + [dependencies] regex = "1.3.7" serde = "1.0.110" diff --git a/tests/ui-cargo/wildcard_dependencies/fail/Cargo.toml b/tests/ui-cargo/wildcard_dependencies/fail/Cargo.toml index fd2a34148568..3e1a02cbb3c5 100644 --- a/tests/ui-cargo/wildcard_dependencies/fail/Cargo.toml +++ b/tests/ui-cargo/wildcard_dependencies/fail/Cargo.toml @@ -3,5 +3,7 @@ name = "wildcard_dependencies" version = "0.1.0" publish = false +[workspace] + [dependencies] regex = "*" diff --git a/tests/ui-cargo/wildcard_dependencies/pass/Cargo.toml b/tests/ui-cargo/wildcard_dependencies/pass/Cargo.toml index 38cb139146e0..f844cab09ba7 100644 --- a/tests/ui-cargo/wildcard_dependencies/pass/Cargo.toml +++ b/tests/ui-cargo/wildcard_dependencies/pass/Cargo.toml @@ -3,5 +3,7 @@ name = "wildcard_dependencies" version = "0.1.0" publish = false +[workspace] + [dependencies] regex = "1" diff --git a/tests/ui/checked_conversions.fixed b/tests/ui/checked_conversions.fixed index 7febd6f37613..12290db3dcf5 100644 --- a/tests/ui/checked_conversions.fixed +++ b/tests/ui/checked_conversions.fixed @@ -1,106 +1,76 @@ // run-rustfix +#![allow( + clippy::cast_lossless, + // Int::max_value will be deprecated in the future + deprecated, +)] #![warn(clippy::checked_conversions)] -#![allow(clippy::cast_lossless)] -#![allow(dead_code)] + use std::convert::TryFrom; // Positive tests // Signed to unsigned -fn i64_to_u32(value: i64) -> Option { - if u32::try_from(value).is_ok() { - Some(value as u32) - } else { - None - } +pub fn i64_to_u32(value: i64) { + let _ = u32::try_from(value).is_ok(); + let _ = u32::try_from(value).is_ok(); } -fn i64_to_u16(value: i64) -> Option { - if u16::try_from(value).is_ok() { - Some(value as u16) - } else { - None - } +pub fn i64_to_u16(value: i64) { + let _ = u16::try_from(value).is_ok(); + let _ = u16::try_from(value).is_ok(); } -fn isize_to_u8(value: isize) -> Option { - if u8::try_from(value).is_ok() { - Some(value as u8) - } else { - None - } +pub fn isize_to_u8(value: isize) { + let _ = u8::try_from(value).is_ok(); + let _ = u8::try_from(value).is_ok(); } // Signed to signed -fn i64_to_i32(value: i64) -> Option { - if i32::try_from(value).is_ok() { - Some(value as i32) - } else { - None - } +pub fn i64_to_i32(value: i64) { + let _ = i32::try_from(value).is_ok(); + let _ = i32::try_from(value).is_ok(); } -fn i64_to_i16(value: i64) -> Option { - if i16::try_from(value).is_ok() { - Some(value as i16) - } else { - None - } +pub fn i64_to_i16(value: i64) { + let _ = i16::try_from(value).is_ok(); + let _ = i16::try_from(value).is_ok(); } // Unsigned to X -fn u32_to_i32(value: u32) -> Option { - if i32::try_from(value).is_ok() { - Some(value as i32) - } else { - None - } +pub fn u32_to_i32(value: u32) { + let _ = i32::try_from(value).is_ok(); + let _ = i32::try_from(value).is_ok(); } -fn usize_to_isize(value: usize) -> isize { - if isize::try_from(value).is_ok() && value as i32 == 5 { - 5 - } else { - 1 - } +pub fn usize_to_isize(value: usize) { + let _ = isize::try_from(value).is_ok() && value as i32 == 5; + let _ = isize::try_from(value).is_ok() && value as i32 == 5; } -fn u32_to_u16(value: u32) -> isize { - if u16::try_from(value).is_ok() && value as i32 == 5 { - 5 - } else { - 1 - } +pub fn u32_to_u16(value: u32) { + let _ = u16::try_from(value).is_ok() && value as i32 == 5; + let _ = u16::try_from(value).is_ok() && value as i32 == 5; } // Negative tests -fn no_i64_to_i32(value: i64) -> Option { - if value <= (i32::max_value() as i64) && value >= 0 { - Some(value as i32) - } else { - None - } +pub fn no_i64_to_i32(value: i64) { + let _ = value <= (i32::max_value() as i64) && value >= 0; + let _ = value <= (i32::MAX as i64) && value >= 0; } -fn no_isize_to_u8(value: isize) -> Option { - if value <= (u8::max_value() as isize) && value >= (u8::min_value() as isize) { - Some(value as u8) - } else { - None - } +pub fn no_isize_to_u8(value: isize) { + let _ = value <= (u8::max_value() as isize) && value >= (u8::min_value() as isize); + let _ = value <= (u8::MAX as isize) && value >= (u8::MIN as isize); } -fn i8_to_u8(value: i8) -> Option { - if value >= 0 { - Some(value as u8) - } else { - None - } +pub fn i8_to_u8(value: i8) { + let _ = value >= 0; } fn main() {} diff --git a/tests/ui/checked_conversions.rs b/tests/ui/checked_conversions.rs index a643354e2438..895a301e8212 100644 --- a/tests/ui/checked_conversions.rs +++ b/tests/ui/checked_conversions.rs @@ -1,106 +1,76 @@ // run-rustfix +#![allow( + clippy::cast_lossless, + // Int::max_value will be deprecated in the future + deprecated, +)] #![warn(clippy::checked_conversions)] -#![allow(clippy::cast_lossless)] -#![allow(dead_code)] + use std::convert::TryFrom; // Positive tests // Signed to unsigned -fn i64_to_u32(value: i64) -> Option { - if value <= (u32::max_value() as i64) && value >= 0 { - Some(value as u32) - } else { - None - } +pub fn i64_to_u32(value: i64) { + let _ = value <= (u32::max_value() as i64) && value >= 0; + let _ = value <= (u32::MAX as i64) && value >= 0; } -fn i64_to_u16(value: i64) -> Option { - if value <= i64::from(u16::max_value()) && value >= 0 { - Some(value as u16) - } else { - None - } +pub fn i64_to_u16(value: i64) { + let _ = value <= i64::from(u16::max_value()) && value >= 0; + let _ = value <= i64::from(u16::MAX) && value >= 0; } -fn isize_to_u8(value: isize) -> Option { - if value <= (u8::max_value() as isize) && value >= 0 { - Some(value as u8) - } else { - None - } +pub fn isize_to_u8(value: isize) { + let _ = value <= (u8::max_value() as isize) && value >= 0; + let _ = value <= (u8::MAX as isize) && value >= 0; } // Signed to signed -fn i64_to_i32(value: i64) -> Option { - if value <= (i32::max_value() as i64) && value >= (i32::min_value() as i64) { - Some(value as i32) - } else { - None - } +pub fn i64_to_i32(value: i64) { + let _ = value <= (i32::max_value() as i64) && value >= (i32::min_value() as i64); + let _ = value <= (i32::MAX as i64) && value >= (i32::MIN as i64); } -fn i64_to_i16(value: i64) -> Option { - if value <= i64::from(i16::max_value()) && value >= i64::from(i16::min_value()) { - Some(value as i16) - } else { - None - } +pub fn i64_to_i16(value: i64) { + let _ = value <= i64::from(i16::max_value()) && value >= i64::from(i16::min_value()); + let _ = value <= i64::from(i16::MAX) && value >= i64::from(i16::MIN); } // Unsigned to X -fn u32_to_i32(value: u32) -> Option { - if value <= i32::max_value() as u32 { - Some(value as i32) - } else { - None - } +pub fn u32_to_i32(value: u32) { + let _ = value <= i32::max_value() as u32; + let _ = value <= i32::MAX as u32; } -fn usize_to_isize(value: usize) -> isize { - if value <= isize::max_value() as usize && value as i32 == 5 { - 5 - } else { - 1 - } +pub fn usize_to_isize(value: usize) { + let _ = value <= isize::max_value() as usize && value as i32 == 5; + let _ = value <= isize::MAX as usize && value as i32 == 5; } -fn u32_to_u16(value: u32) -> isize { - if value <= u16::max_value() as u32 && value as i32 == 5 { - 5 - } else { - 1 - } +pub fn u32_to_u16(value: u32) { + let _ = value <= u16::max_value() as u32 && value as i32 == 5; + let _ = value <= u16::MAX as u32 && value as i32 == 5; } // Negative tests -fn no_i64_to_i32(value: i64) -> Option { - if value <= (i32::max_value() as i64) && value >= 0 { - Some(value as i32) - } else { - None - } +pub fn no_i64_to_i32(value: i64) { + let _ = value <= (i32::max_value() as i64) && value >= 0; + let _ = value <= (i32::MAX as i64) && value >= 0; } -fn no_isize_to_u8(value: isize) -> Option { - if value <= (u8::max_value() as isize) && value >= (u8::min_value() as isize) { - Some(value as u8) - } else { - None - } +pub fn no_isize_to_u8(value: isize) { + let _ = value <= (u8::max_value() as isize) && value >= (u8::min_value() as isize); + let _ = value <= (u8::MAX as isize) && value >= (u8::MIN as isize); } -fn i8_to_u8(value: i8) -> Option { - if value >= 0 { - Some(value as u8) - } else { - None - } +pub fn i8_to_u8(value: i8) { + let _ = value >= 0; } fn main() {} diff --git a/tests/ui/checked_conversions.stderr b/tests/ui/checked_conversions.stderr index f678f009621f..648ba3ccd01d 100644 --- a/tests/ui/checked_conversions.stderr +++ b/tests/ui/checked_conversions.stderr @@ -1,52 +1,100 @@ error: Checked cast can be simplified. - --> $DIR/checked_conversions.rs:13:8 + --> $DIR/checked_conversions.rs:17:13 | -LL | if value <= (u32::max_value() as i64) && value >= 0 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u32::try_from(value).is_ok()` +LL | let _ = value <= (u32::max_value() as i64) && value >= 0; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u32::try_from(value).is_ok()` | = note: `-D clippy::checked-conversions` implied by `-D warnings` error: Checked cast can be simplified. - --> $DIR/checked_conversions.rs:21:8 + --> $DIR/checked_conversions.rs:18:13 | -LL | if value <= i64::from(u16::max_value()) && value >= 0 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()` +LL | let _ = value <= (u32::MAX as i64) && value >= 0; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u32::try_from(value).is_ok()` error: Checked cast can be simplified. - --> $DIR/checked_conversions.rs:29:8 + --> $DIR/checked_conversions.rs:22:13 | -LL | if value <= (u8::max_value() as isize) && value >= 0 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u8::try_from(value).is_ok()` +LL | let _ = value <= i64::from(u16::max_value()) && value >= 0; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()` error: Checked cast can be simplified. - --> $DIR/checked_conversions.rs:39:8 + --> $DIR/checked_conversions.rs:23:13 | -LL | if value <= (i32::max_value() as i64) && value >= (i32::min_value() as i64) { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()` +LL | let _ = value <= i64::from(u16::MAX) && value >= 0; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()` error: Checked cast can be simplified. - --> $DIR/checked_conversions.rs:47:8 + --> $DIR/checked_conversions.rs:27:13 | -LL | if value <= i64::from(i16::max_value()) && value >= i64::from(i16::min_value()) { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i16::try_from(value).is_ok()` +LL | let _ = value <= (u8::max_value() as isize) && value >= 0; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u8::try_from(value).is_ok()` error: Checked cast can be simplified. - --> $DIR/checked_conversions.rs:57:8 + --> $DIR/checked_conversions.rs:28:13 | -LL | if value <= i32::max_value() as u32 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()` +LL | let _ = value <= (u8::MAX as isize) && value >= 0; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u8::try_from(value).is_ok()` error: Checked cast can be simplified. - --> $DIR/checked_conversions.rs:65:8 + --> $DIR/checked_conversions.rs:34:13 | -LL | if value <= isize::max_value() as usize && value as i32 == 5 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `isize::try_from(value).is_ok()` +LL | let _ = value <= (i32::max_value() as i64) && value >= (i32::min_value() as i64); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()` error: Checked cast can be simplified. - --> $DIR/checked_conversions.rs:73:8 + --> $DIR/checked_conversions.rs:35:13 | -LL | if value <= u16::max_value() as u32 && value as i32 == 5 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()` +LL | let _ = value <= (i32::MAX as i64) && value >= (i32::MIN as i64); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()` -error: aborting due to 8 previous errors +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:39:13 + | +LL | let _ = value <= i64::from(i16::max_value()) && value >= i64::from(i16::min_value()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i16::try_from(value).is_ok()` + +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:40:13 + | +LL | let _ = value <= i64::from(i16::MAX) && value >= i64::from(i16::MIN); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i16::try_from(value).is_ok()` + +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:46:13 + | +LL | let _ = value <= i32::max_value() as u32; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()` + +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:47:13 + | +LL | let _ = value <= i32::MAX as u32; + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()` + +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:51:13 + | +LL | let _ = value <= isize::max_value() as usize && value as i32 == 5; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `isize::try_from(value).is_ok()` + +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:52:13 + | +LL | let _ = value <= isize::MAX as usize && value as i32 == 5; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `isize::try_from(value).is_ok()` + +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:56:13 + | +LL | let _ = value <= u16::max_value() as u32 && value as i32 == 5; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()` + +error: Checked cast can be simplified. + --> $DIR/checked_conversions.rs:57:13 + | +LL | let _ = value <= u16::MAX as u32 && value as i32 == 5; + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()` + +error: aborting due to 16 previous errors diff --git a/tests/ui/checked_conversions.stdout b/tests/ui/checked_conversions.stdout deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/tests/ui/crashes/ice-3969.rs b/tests/ui/crashes/ice-3969.rs new file mode 100644 index 000000000000..4feab7910b74 --- /dev/null +++ b/tests/ui/crashes/ice-3969.rs @@ -0,0 +1,51 @@ +// https://github.com/rust-lang/rust-clippy/issues/3969 +// used to crash: error: internal compiler error: +// src/librustc_traits/normalize_erasing_regions.rs:43: could not fully normalize `::Item test from rustc ./ui/trivial-bounds/trivial-bounds-inconsistent.rs + +// Check that tautalogically false bounds are accepted, and are used +// in type inference. +#![feature(trivial_bounds)] +#![allow(unused)] + +trait A {} + +impl A for i32 {} + +struct Dst { + x: X, +} + +struct TwoStrs(str, str) +where + str: Sized; + +fn unsized_local() +where + for<'a> Dst: Sized, +{ + let x: Dst = *(Box::new(Dst { x: 1 }) as Box>); +} + +fn return_str() -> str +where + str: Sized, +{ + *"Sized".to_string().into_boxed_str() +} + +fn use_op(s: String) -> String +where + String: ::std::ops::Neg, +{ + -s +} + +fn use_for() +where + i32: Iterator, +{ + for _ in 2i32 {} +} + +fn main() {} diff --git a/tests/ui/crashes/ice-3969.stderr b/tests/ui/crashes/ice-3969.stderr new file mode 100644 index 000000000000..923db0664a71 --- /dev/null +++ b/tests/ui/crashes/ice-3969.stderr @@ -0,0 +1,22 @@ +error: trait objects without an explicit `dyn` are deprecated + --> $DIR/ice-3969.rs:25:17 + | +LL | for<'a> Dst: Sized, + | ^^^^^^ help: use `dyn`: `dyn A + 'a` + | + = note: `-D bare-trait-objects` implied by `-D warnings` + +error: trait objects without an explicit `dyn` are deprecated + --> $DIR/ice-3969.rs:27:16 + | +LL | let x: Dst = *(Box::new(Dst { x: 1 }) as Box>); + | ^ help: use `dyn`: `dyn A` + +error: trait objects without an explicit `dyn` are deprecated + --> $DIR/ice-3969.rs:27:57 + | +LL | let x: Dst = *(Box::new(Dst { x: 1 }) as Box>); + | ^ help: use `dyn`: `dyn A` + +error: aborting due to 3 previous errors + diff --git a/tests/ui/crashes/regressions.rs b/tests/ui/crashes/regressions.rs index 623ae51f9f08..3d5063d1a3a7 100644 --- a/tests/ui/crashes/regressions.rs +++ b/tests/ui/crashes/regressions.rs @@ -6,4 +6,8 @@ pub fn foo(bar: *const u8) { println!("{:#p}", bar); } +// Regression test for https://github.com/rust-lang/rust-clippy/issues/4917 +/// ::new()).iter(); //~ WARN equivalent to .iter() let _ = std::path::Path::new("12/34").iter(); //~ WARN equivalent to .iter() let _ = std::path::PathBuf::from("12/34").iter(); //~ ERROR equivalent to .iter() + + let _ = (&[1, 2, 3]).iter().next(); //~ WARN equivalent to .iter() } diff --git a/tests/ui/into_iter_on_ref.rs b/tests/ui/into_iter_on_ref.rs index 94bc1689619a..416056d3fdb9 100644 --- a/tests/ui/into_iter_on_ref.rs +++ b/tests/ui/into_iter_on_ref.rs @@ -40,4 +40,6 @@ fn main() { let _ = (&HashSet::::new()).into_iter(); //~ WARN equivalent to .iter() let _ = std::path::Path::new("12/34").into_iter(); //~ WARN equivalent to .iter() let _ = std::path::PathBuf::from("12/34").into_iter(); //~ ERROR equivalent to .iter() + + let _ = (&[1, 2, 3]).into_iter().next(); //~ WARN equivalent to .iter() } diff --git a/tests/ui/into_iter_on_ref.stderr b/tests/ui/into_iter_on_ref.stderr index 80e2d104f824..1cd6400b0195 100644 --- a/tests/ui/into_iter_on_ref.stderr +++ b/tests/ui/into_iter_on_ref.stderr @@ -156,5 +156,11 @@ error: this `.into_iter()` call is equivalent to `.iter()` and will not move the LL | let _ = std::path::PathBuf::from("12/34").into_iter(); //~ ERROR equivalent to .iter() | ^^^^^^^^^ help: call directly: `iter` -error: aborting due to 26 previous errors +error: this `.into_iter()` call is equivalent to `.iter()` and will not move the `array` + --> $DIR/into_iter_on_ref.rs:44:26 + | +LL | let _ = (&[1, 2, 3]).into_iter().next(); //~ WARN equivalent to .iter() + | ^^^^^^^^^ help: call directly: `iter` + +error: aborting due to 27 previous errors diff --git a/tests/ui/iter_next_slice.fixed b/tests/ui/iter_next_slice.fixed new file mode 100644 index 000000000000..79c1db87ac3c --- /dev/null +++ b/tests/ui/iter_next_slice.fixed @@ -0,0 +1,24 @@ +// run-rustfix +#![warn(clippy::iter_next_slice)] + +fn main() { + // test code goes here + let s = [1, 2, 3]; + let v = vec![1, 2, 3]; + + s.get(0); + // Should be replaced by s.get(0) + + s.get(2); + // Should be replaced by s.get(2) + + v.get(5); + // Should be replaced by v.get(5) + + v.get(0); + // Should be replaced by v.get(0) + + let o = Some(5); + o.iter().next(); + // Shouldn't be linted since this is not a Slice or an Array +} diff --git a/tests/ui/iter_next_slice.rs b/tests/ui/iter_next_slice.rs new file mode 100644 index 000000000000..ef9a55f3d997 --- /dev/null +++ b/tests/ui/iter_next_slice.rs @@ -0,0 +1,24 @@ +// run-rustfix +#![warn(clippy::iter_next_slice)] + +fn main() { + // test code goes here + let s = [1, 2, 3]; + let v = vec![1, 2, 3]; + + s.iter().next(); + // Should be replaced by s.get(0) + + s[2..].iter().next(); + // Should be replaced by s.get(2) + + v[5..].iter().next(); + // Should be replaced by v.get(5) + + v.iter().next(); + // Should be replaced by v.get(0) + + let o = Some(5); + o.iter().next(); + // Shouldn't be linted since this is not a Slice or an Array +} diff --git a/tests/ui/iter_next_slice.stderr b/tests/ui/iter_next_slice.stderr new file mode 100644 index 000000000000..bbf61df0cda6 --- /dev/null +++ b/tests/ui/iter_next_slice.stderr @@ -0,0 +1,28 @@ +error: Using `.iter().next()` on an array + --> $DIR/iter_next_slice.rs:9:5 + | +LL | s.iter().next(); + | ^^^^^^^^^^^^^^^ help: try calling: `s.get(0)` + | + = note: `-D clippy::iter-next-slice` implied by `-D warnings` + +error: Using `.iter().next()` on a Slice without end index. + --> $DIR/iter_next_slice.rs:12:5 + | +LL | s[2..].iter().next(); + | ^^^^^^^^^^^^^^^^^^^^ help: try calling: `s.get(2)` + +error: Using `.iter().next()` on a Slice without end index. + --> $DIR/iter_next_slice.rs:15:5 + | +LL | v[5..].iter().next(); + | ^^^^^^^^^^^^^^^^^^^^ help: try calling: `v.get(5)` + +error: Using `.iter().next()` on an array + --> $DIR/iter_next_slice.rs:18:5 + | +LL | v.iter().next(); + | ^^^^^^^^^^^^^^^ help: try calling: `v.get(0)` + +error: aborting due to 4 previous errors + diff --git a/tests/ui/len_zero.fixed b/tests/ui/len_zero.fixed index 624e5ef8fcf1..a29b832eb601 100644 --- a/tests/ui/len_zero.fixed +++ b/tests/ui/len_zero.fixed @@ -141,3 +141,11 @@ fn main() { fn test_slice(b: &[u8]) { if !b.is_empty() {} } + +mod issue_3807 { + // Avoid suggesting changes to ranges if the user did not enable `range_is_empty`. + // See https://github.com/rust-lang/rust/issues/48111#issuecomment-445132965 + fn no_suggestion() { + let _ = (0..42).len() == 0; + } +} diff --git a/tests/ui/len_zero.rs b/tests/ui/len_zero.rs index 7fba971cfd88..8fd0093f4fdb 100644 --- a/tests/ui/len_zero.rs +++ b/tests/ui/len_zero.rs @@ -141,3 +141,11 @@ fn main() { fn test_slice(b: &[u8]) { if b.len() != 0 {} } + +mod issue_3807 { + // Avoid suggesting changes to ranges if the user did not enable `range_is_empty`. + // See https://github.com/rust-lang/rust/issues/48111#issuecomment-445132965 + fn no_suggestion() { + let _ = (0..42).len() == 0; + } +} diff --git a/tests/ui/len_zero_ranges.fixed b/tests/ui/len_zero_ranges.fixed new file mode 100644 index 000000000000..7da26f8ff4d4 --- /dev/null +++ b/tests/ui/len_zero_ranges.fixed @@ -0,0 +1,14 @@ +// run-rustfix + +#![feature(range_is_empty)] +#![warn(clippy::len_zero)] +#![allow(unused)] + +mod issue_3807 { + // With the feature enabled, `is_empty` should be suggested + fn suggestion_is_fine() { + let _ = (0..42).is_empty(); + } +} + +fn main() {} diff --git a/tests/ui/len_zero_ranges.rs b/tests/ui/len_zero_ranges.rs new file mode 100644 index 000000000000..be7b4244bc06 --- /dev/null +++ b/tests/ui/len_zero_ranges.rs @@ -0,0 +1,14 @@ +// run-rustfix + +#![feature(range_is_empty)] +#![warn(clippy::len_zero)] +#![allow(unused)] + +mod issue_3807 { + // With the feature enabled, `is_empty` should be suggested + fn suggestion_is_fine() { + let _ = (0..42).len() == 0; + } +} + +fn main() {} diff --git a/tests/ui/len_zero_ranges.stderr b/tests/ui/len_zero_ranges.stderr new file mode 100644 index 000000000000..6e5fa41fb08a --- /dev/null +++ b/tests/ui/len_zero_ranges.stderr @@ -0,0 +1,10 @@ +error: length comparison to zero + --> $DIR/len_zero_ranges.rs:10:17 + | +LL | let _ = (0..42).len() == 0; + | ^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `(0..42).is_empty()` + | + = note: `-D clippy::len-zero` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/tests/ui/needless_collect.fixed b/tests/ui/needless_collect.fixed index b4227eaf2f8b..be37dc16b9a3 100644 --- a/tests/ui/needless_collect.fixed +++ b/tests/ui/needless_collect.fixed @@ -9,7 +9,7 @@ use std::collections::{BTreeSet, HashMap, HashSet}; fn main() { let sample = [1; 5]; let len = sample.iter().count(); - if sample.iter().next().is_none() { + if sample.get(0).is_none() { // Empty } sample.iter().cloned().any(|x| x == 1); diff --git a/tests/ui/needless_collect.stderr b/tests/ui/needless_collect.stderr index 8884c8e16129..9113aad90dd7 100644 --- a/tests/ui/needless_collect.stderr +++ b/tests/ui/needless_collect.stderr @@ -1,28 +1,28 @@ error: avoid using `collect()` when not needed - --> $DIR/needless_collect.rs:11:28 + --> $DIR/needless_collect.rs:11:29 | LL | let len = sample.iter().collect::>().len(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.count()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()` | = note: `-D clippy::needless-collect` implied by `-D warnings` error: avoid using `collect()` when not needed - --> $DIR/needless_collect.rs:12:21 + --> $DIR/needless_collect.rs:12:15 | LL | if sample.iter().collect::>().is_empty() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.next().is_none()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `get(0).is_none()` error: avoid using `collect()` when not needed - --> $DIR/needless_collect.rs:15:27 + --> $DIR/needless_collect.rs:15:28 | LL | sample.iter().cloned().collect::>().contains(&1); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.any(|x| x == 1)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == 1)` error: avoid using `collect()` when not needed - --> $DIR/needless_collect.rs:16:34 + --> $DIR/needless_collect.rs:16:35 | LL | sample.iter().map(|x| (x, x)).collect::>().len(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.count()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()` error: aborting due to 4 previous errors diff --git a/tests/ui/string_lit_as_bytes.fixed b/tests/ui/string_lit_as_bytes.fixed index 7ad272ade5f9..ccf8f61c4a92 100644 --- a/tests/ui/string_lit_as_bytes.fixed +++ b/tests/ui/string_lit_as_bytes.fixed @@ -14,6 +14,8 @@ fn str_lit_as_bytes() { let strify = stringify!(foobar).as_bytes(); + let current_version = env!("CARGO_PKG_VERSION").as_bytes(); + let includestr = include_bytes!("entry_unfixable.rs"); let _ = b"string with newline\t\n"; diff --git a/tests/ui/string_lit_as_bytes.rs b/tests/ui/string_lit_as_bytes.rs index 1bf4538b7c94..178df08e249e 100644 --- a/tests/ui/string_lit_as_bytes.rs +++ b/tests/ui/string_lit_as_bytes.rs @@ -14,6 +14,8 @@ fn str_lit_as_bytes() { let strify = stringify!(foobar).as_bytes(); + let current_version = env!("CARGO_PKG_VERSION").as_bytes(); + let includestr = include_str!("entry_unfixable.rs").as_bytes(); let _ = "string with newline\t\n".as_bytes(); diff --git a/tests/ui/string_lit_as_bytes.stderr b/tests/ui/string_lit_as_bytes.stderr index ff6e3346dfc7..99c512354d58 100644 --- a/tests/ui/string_lit_as_bytes.stderr +++ b/tests/ui/string_lit_as_bytes.stderr @@ -13,13 +13,13 @@ LL | let bs = r###"raw string with 3# plus " ""###.as_bytes(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using a byte string literal instead: `br###"raw string with 3# plus " ""###` error: calling `as_bytes()` on `include_str!(..)` - --> $DIR/string_lit_as_bytes.rs:17:22 + --> $DIR/string_lit_as_bytes.rs:19:22 | LL | let includestr = include_str!("entry_unfixable.rs").as_bytes(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `include_bytes!(..)` instead: `include_bytes!("entry_unfixable.rs")` error: calling `as_bytes()` on a string literal - --> $DIR/string_lit_as_bytes.rs:19:13 + --> $DIR/string_lit_as_bytes.rs:21:13 | LL | let _ = "string with newline/t/n".as_bytes(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using a byte string literal instead: `b"string with newline/t/n"` diff --git a/tests/ui/unit_arg.fixed b/tests/ui/unit_arg.fixed deleted file mode 100644 index a739cf7ad814..000000000000 --- a/tests/ui/unit_arg.fixed +++ /dev/null @@ -1,64 +0,0 @@ -// run-rustfix -#![warn(clippy::unit_arg)] -#![allow(unused_braces, clippy::no_effect, unused_must_use)] - -use std::fmt::Debug; - -fn foo(t: T) { - println!("{:?}", t); -} - -fn foo3(t1: T1, t2: T2, t3: T3) { - println!("{:?}, {:?}, {:?}", t1, t2, t3); -} - -struct Bar; - -impl Bar { - fn bar(&self, t: T) { - println!("{:?}", t); - } -} - -fn bad() { - foo(()); - foo(()); - foo(()); - foo(()); - foo3((), 2, 2); - let b = Bar; - b.bar(()); -} - -fn ok() { - foo(()); - foo(1); - foo({ 1 }); - foo3("a", 3, vec![3]); - let b = Bar; - b.bar({ 1 }); - b.bar(()); - question_mark(); -} - -fn question_mark() -> Result<(), ()> { - Ok(Ok(())?)?; - Ok(Ok(()))??; - Ok(()) -} - -#[allow(dead_code)] -mod issue_2945 { - fn unit_fn() -> Result<(), i32> { - Ok(()) - } - - fn fallible() -> Result<(), i32> { - Ok(unit_fn()?) - } -} - -fn main() { - bad(); - ok(); -} diff --git a/tests/ui/unit_arg.rs b/tests/ui/unit_arg.rs index d90c49f79de6..2992abae775b 100644 --- a/tests/ui/unit_arg.rs +++ b/tests/ui/unit_arg.rs @@ -1,6 +1,5 @@ -// run-rustfix #![warn(clippy::unit_arg)] -#![allow(unused_braces, clippy::no_effect, unused_must_use)] +#![allow(clippy::no_effect, unused_must_use, unused_variables)] use std::fmt::Debug; @@ -21,7 +20,6 @@ impl Bar { } fn bad() { - foo({}); foo({ 1; }); @@ -30,11 +28,25 @@ fn bad() { foo(1); foo(2); }); - foo3({}, 2, 2); let b = Bar; b.bar({ 1; }); + taking_multiple_units(foo(0), foo(1)); + taking_multiple_units(foo(0), { + foo(1); + foo(2); + }); + taking_multiple_units( + { + foo(0); + foo(1); + }, + { + foo(2); + foo(3); + }, + ); } fn ok() { @@ -65,6 +77,13 @@ mod issue_2945 { } } +#[allow(dead_code)] +fn returning_expr() -> Option<()> { + Some(foo(1)) +} + +fn taking_multiple_units(a: (), b: ()) {} + fn main() { bad(); ok(); diff --git a/tests/ui/unit_arg.stderr b/tests/ui/unit_arg.stderr index 21ccc684ea9d..56f6a855dfa5 100644 --- a/tests/ui/unit_arg.stderr +++ b/tests/ui/unit_arg.stderr @@ -1,79 +1,181 @@ error: passing a unit value to a function - --> $DIR/unit_arg.rs:24:9 + --> $DIR/unit_arg.rs:23:5 | -LL | foo({}); - | ^^ +LL | / foo({ +LL | | 1; +LL | | }); + | |______^ | = note: `-D clippy::unit-arg` implied by `-D warnings` -help: if you intended to pass a unit value, use a unit literal instead +help: remove the semicolon from the last statement in the block | -LL | foo(()); - | ^^ - -error: passing a unit value to a function - --> $DIR/unit_arg.rs:25:9 +LL | 1 | -LL | foo({ - | _________^ -LL | | 1; -LL | | }); - | |_____^ +help: or move the expression in front of the call... | -help: if you intended to pass a unit value, use a unit literal instead +LL | { +LL | 1; +LL | }; + | +help: ...and use a unit literal instead | LL | foo(()); | ^^ error: passing a unit value to a function - --> $DIR/unit_arg.rs:28:9 + --> $DIR/unit_arg.rs:26:5 | LL | foo(foo(1)); - | ^^^^^^ + | ^^^^^^^^^^^ + | +help: move the expression in front of the call... | -help: if you intended to pass a unit value, use a unit literal instead +LL | foo(1); + | +help: ...and use a unit literal instead | LL | foo(()); | ^^ error: passing a unit value to a function - --> $DIR/unit_arg.rs:29:9 + --> $DIR/unit_arg.rs:27:5 | -LL | foo({ - | _________^ +LL | / foo({ LL | | foo(1); LL | | foo(2); LL | | }); - | |_____^ + | |______^ + | +help: remove the semicolon from the last statement in the block + | +LL | foo(2) | -help: if you intended to pass a unit value, use a unit literal instead +help: or move the expression in front of the call... + | +LL | { +LL | foo(1); +LL | foo(2); +LL | }; + | +help: ...and use a unit literal instead | LL | foo(()); | ^^ error: passing a unit value to a function - --> $DIR/unit_arg.rs:33:10 + --> $DIR/unit_arg.rs:32:5 | -LL | foo3({}, 2, 2); - | ^^ +LL | / b.bar({ +LL | | 1; +LL | | }); + | |______^ | -help: if you intended to pass a unit value, use a unit literal instead +help: remove the semicolon from the last statement in the block | -LL | foo3((), 2, 2); - | ^^ +LL | 1 + | +help: or move the expression in front of the call... + | +LL | { +LL | 1; +LL | }; + | +help: ...and use a unit literal instead + | +LL | b.bar(()); + | ^^ -error: passing a unit value to a function - --> $DIR/unit_arg.rs:35:11 +error: passing unit values to a function + --> $DIR/unit_arg.rs:35:5 | -LL | b.bar({ - | ___________^ -LL | | 1; +LL | taking_multiple_units(foo(0), foo(1)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: move the expressions in front of the call... + | +LL | foo(0); +LL | foo(1); + | +help: ...and use unit literals instead + | +LL | taking_multiple_units((), ()); + | ^^ ^^ + +error: passing unit values to a function + --> $DIR/unit_arg.rs:36:5 + | +LL | / taking_multiple_units(foo(0), { +LL | | foo(1); +LL | | foo(2); LL | | }); + | |______^ + | +help: remove the semicolon from the last statement in the block + | +LL | foo(2) + | +help: or move the expressions in front of the call... + | +LL | foo(0); +LL | { +LL | foo(1); +LL | foo(2); +LL | }; + | +help: ...and use unit literals instead + | +LL | taking_multiple_units((), ()); + | ^^ ^^ + +error: passing unit values to a function + --> $DIR/unit_arg.rs:40:5 + | +LL | / taking_multiple_units( +LL | | { +LL | | foo(0); +LL | | foo(1); +... | +LL | | }, +LL | | ); | |_____^ | -help: if you intended to pass a unit value, use a unit literal instead +help: remove the semicolon from the last statement in the block | -LL | b.bar(()); - | ^^ +LL | foo(1) + | +help: remove the semicolon from the last statement in the block + | +LL | foo(3) + | +help: or move the expressions in front of the call... + | +LL | { +LL | foo(0); +LL | foo(1); +LL | }; +LL | { +LL | foo(2); + ... +help: ...and use unit literals instead + | +LL | (), +LL | (), + | + +error: passing a unit value to a function + --> $DIR/unit_arg.rs:82:5 + | +LL | Some(foo(1)) + | ^^^^^^^^^^^^ + | +help: move the expression in front of the call... + | +LL | foo(1); + | +help: ...and use a unit literal instead + | +LL | Some(()) + | ^^ -error: aborting due to 6 previous errors +error: aborting due to 8 previous errors diff --git a/tests/ui/unit_arg_empty_blocks.rs b/tests/ui/unit_arg_empty_blocks.rs new file mode 100644 index 000000000000..18a31eb3deee --- /dev/null +++ b/tests/ui/unit_arg_empty_blocks.rs @@ -0,0 +1,26 @@ +#![warn(clippy::unit_arg)] +#![allow(clippy::no_effect, unused_must_use, unused_variables)] + +use std::fmt::Debug; + +fn foo(t: T) { + println!("{:?}", t); +} + +fn foo3(t1: T1, t2: T2, t3: T3) { + println!("{:?}, {:?}, {:?}", t1, t2, t3); +} + +fn bad() { + foo({}); + foo3({}, 2, 2); + taking_two_units({}, foo(0)); + taking_three_units({}, foo(0), foo(1)); +} + +fn taking_two_units(a: (), b: ()) {} +fn taking_three_units(a: (), b: (), c: ()) {} + +fn main() { + bad(); +} diff --git a/tests/ui/unit_arg_empty_blocks.stderr b/tests/ui/unit_arg_empty_blocks.stderr new file mode 100644 index 000000000000..bb58483584b3 --- /dev/null +++ b/tests/ui/unit_arg_empty_blocks.stderr @@ -0,0 +1,51 @@ +error: passing a unit value to a function + --> $DIR/unit_arg_empty_blocks.rs:15:5 + | +LL | foo({}); + | ^^^^--^ + | | + | help: use a unit literal instead: `()` + | + = note: `-D clippy::unit-arg` implied by `-D warnings` + +error: passing a unit value to a function + --> $DIR/unit_arg_empty_blocks.rs:16:5 + | +LL | foo3({}, 2, 2); + | ^^^^^--^^^^^^^ + | | + | help: use a unit literal instead: `()` + +error: passing unit values to a function + --> $DIR/unit_arg_empty_blocks.rs:17:5 + | +LL | taking_two_units({}, foo(0)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: move the expression in front of the call... + | +LL | foo(0); + | +help: ...and use unit literals instead + | +LL | taking_two_units((), ()); + | ^^ ^^ + +error: passing unit values to a function + --> $DIR/unit_arg_empty_blocks.rs:18:5 + | +LL | taking_three_units({}, foo(0), foo(1)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: move the expressions in front of the call... + | +LL | foo(0); +LL | foo(1); + | +help: ...and use unit literals instead + | +LL | taking_three_units((), (), ()); + | ^^ ^^ ^^ + +error: aborting due to 4 previous errors + diff --git a/tests/ui/unnecessary_sort_by.fixed b/tests/ui/unnecessary_sort_by.fixed new file mode 100644 index 000000000000..779fd57707ad --- /dev/null +++ b/tests/ui/unnecessary_sort_by.fixed @@ -0,0 +1,26 @@ +// run-rustfix + +use std::cmp::Reverse; + +fn id(x: isize) -> isize { + x +} + +fn main() { + let mut vec: Vec = vec![3, 6, 1, 2, 5]; + // Forward examples + vec.sort(); + vec.sort_unstable(); + vec.sort_by_key(|&a| (a + 5).abs()); + vec.sort_unstable_by_key(|&a| id(-a)); + // Reverse examples + vec.sort_by_key(|&b| Reverse(b)); + vec.sort_by_key(|&b| Reverse((b + 5).abs())); + vec.sort_unstable_by_key(|&b| Reverse(id(-b))); + // Negative examples (shouldn't be changed) + let c = &7; + vec.sort_by(|a, b| (b - a).cmp(&(a - b))); + vec.sort_by(|_, b| b.cmp(&5)); + vec.sort_by(|_, b| b.cmp(c)); + vec.sort_unstable_by(|a, _| a.cmp(c)); +} diff --git a/tests/ui/unnecessary_sort_by.rs b/tests/ui/unnecessary_sort_by.rs new file mode 100644 index 000000000000..0485a5630afe --- /dev/null +++ b/tests/ui/unnecessary_sort_by.rs @@ -0,0 +1,26 @@ +// run-rustfix + +use std::cmp::Reverse; + +fn id(x: isize) -> isize { + x +} + +fn main() { + let mut vec: Vec = vec![3, 6, 1, 2, 5]; + // Forward examples + vec.sort_by(|a, b| a.cmp(b)); + vec.sort_unstable_by(|a, b| a.cmp(b)); + vec.sort_by(|a, b| (a + 5).abs().cmp(&(b + 5).abs())); + vec.sort_unstable_by(|a, b| id(-a).cmp(&id(-b))); + // Reverse examples + vec.sort_by(|a, b| b.cmp(a)); + vec.sort_by(|a, b| (b + 5).abs().cmp(&(a + 5).abs())); + vec.sort_unstable_by(|a, b| id(-b).cmp(&id(-a))); + // Negative examples (shouldn't be changed) + let c = &7; + vec.sort_by(|a, b| (b - a).cmp(&(a - b))); + vec.sort_by(|_, b| b.cmp(&5)); + vec.sort_by(|_, b| b.cmp(c)); + vec.sort_unstable_by(|a, _| a.cmp(c)); +} diff --git a/tests/ui/unnecessary_sort_by.stderr b/tests/ui/unnecessary_sort_by.stderr new file mode 100644 index 000000000000..903b6e5099ce --- /dev/null +++ b/tests/ui/unnecessary_sort_by.stderr @@ -0,0 +1,46 @@ +error: use Vec::sort here instead + --> $DIR/unnecessary_sort_by.rs:12:5 + | +LL | vec.sort_by(|a, b| a.cmp(b)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort()` + | + = note: `-D clippy::unnecessary-sort-by` implied by `-D warnings` + +error: use Vec::sort here instead + --> $DIR/unnecessary_sort_by.rs:13:5 + | +LL | vec.sort_unstable_by(|a, b| a.cmp(b)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable()` + +error: use Vec::sort_by_key here instead + --> $DIR/unnecessary_sort_by.rs:14:5 + | +LL | vec.sort_by(|a, b| (a + 5).abs().cmp(&(b + 5).abs())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|&a| (a + 5).abs())` + +error: use Vec::sort_by_key here instead + --> $DIR/unnecessary_sort_by.rs:15:5 + | +LL | vec.sort_unstable_by(|a, b| id(-a).cmp(&id(-b))); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|&a| id(-a))` + +error: use Vec::sort_by_key here instead + --> $DIR/unnecessary_sort_by.rs:17:5 + | +LL | vec.sort_by(|a, b| b.cmp(a)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|&b| Reverse(b))` + +error: use Vec::sort_by_key here instead + --> $DIR/unnecessary_sort_by.rs:18:5 + | +LL | vec.sort_by(|a, b| (b + 5).abs().cmp(&(a + 5).abs())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|&b| Reverse((b + 5).abs()))` + +error: use Vec::sort_by_key here instead + --> $DIR/unnecessary_sort_by.rs:19:5 + | +LL | vec.sort_unstable_by(|a, b| id(-b).cmp(&id(-a))); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|&b| Reverse(id(-b)))` + +error: aborting due to 7 previous errors + diff --git a/tests/ui/vec_resize_to_zero.rs b/tests/ui/vec_resize_to_zero.rs new file mode 100644 index 000000000000..0263e2f5f20c --- /dev/null +++ b/tests/ui/vec_resize_to_zero.rs @@ -0,0 +1,15 @@ +#![warn(clippy::vec_resize_to_zero)] + +fn main() { + // applicable here + vec![1, 2, 3, 4, 5].resize(0, 5); + + // not applicable + vec![1, 2, 3, 4, 5].resize(2, 5); + + // applicable here, but only implemented for integer litterals for now + vec!["foo", "bar", "baz"].resize(0, "bar"); + + // not applicable + vec!["foo", "bar", "baz"].resize(2, "bar") +} diff --git a/tests/ui/vec_resize_to_zero.stderr b/tests/ui/vec_resize_to_zero.stderr new file mode 100644 index 000000000000..feb846298c65 --- /dev/null +++ b/tests/ui/vec_resize_to_zero.stderr @@ -0,0 +1,13 @@ +error: emptying a vector with `resize` + --> $DIR/vec_resize_to_zero.rs:5:5 + | +LL | vec![1, 2, 3, 4, 5].resize(0, 5); + | ^^^^^^^^^^^^^^^^^^^^------------ + | | + | help: ...or you can empty the vector with: `clear()` + | + = note: `-D clippy::vec-resize-to-zero` implied by `-D warnings` + = help: the arguments may be inverted... + +error: aborting due to previous error + diff --git a/util/dev b/util/dev deleted file mode 100755 index 319de217e0d9..000000000000 --- a/util/dev +++ /dev/null @@ -1,7 +0,0 @@ -#!/bin/sh -CARGO_TARGET_DIR=$(pwd)/target/ -export CARGO_TARGET_DIR - -echo 'Deprecated! `util/dev` usage is deprecated, please use `cargo dev` instead.' - -cd clippy_dev && cargo run -- "$@"