diff --git a/.cargo/config b/.cargo/config index 84ae36a46d7..688473f2f9b 100644 --- a/.cargo/config +++ b/.cargo/config @@ -1,9 +1,10 @@ [alias] uitest = "test --test compile-test" -dev = "run --target-dir clippy_dev/target --package clippy_dev --bin clippy_dev --manifest-path clippy_dev/Cargo.toml --" -lintcheck = "run --target-dir lintcheck/target --package lintcheck --bin lintcheck --manifest-path lintcheck/Cargo.toml -- " +dev = "run --package clippy_dev --bin clippy_dev --manifest-path clippy_dev/Cargo.toml --" +lintcheck = "run --package lintcheck --bin lintcheck --manifest-path lintcheck/Cargo.toml -- " collect-metadata = "test --test dogfood --features metadata-collector-lint -- run_metadata_collection_lint --ignored" [build] # -Zbinary-dep-depinfo allows us to track which rlib files to use for compiling UI tests rustflags = ["-Zunstable-options", "-Zbinary-dep-depinfo"] +target-dir = "target" diff --git a/.github/ISSUE_TEMPLATE/blank_issue.md b/.github/ISSUE_TEMPLATE/blank_issue.md index 2891d5e5da1..866303a1f9f 100644 --- a/.github/ISSUE_TEMPLATE/blank_issue.md +++ b/.github/ISSUE_TEMPLATE/blank_issue.md @@ -8,7 +8,7 @@ about: Create a blank issue. Additional labels can be added to this issue by including the following command (without the space after the @ symbol): -`@rustbot label +<label>` +@ rustbot label +<label> Common labels for this issue type are: * C-an-interesting-project diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 87c18cdee66..119a498fb99 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -36,7 +36,7 @@ LLVM version: 10.0 Additional labels can be added to this issue by including the following command (without the space after the @ symbol): -`@rustbot label +<label>` +@ rustbot label +<label> Common labels for this issue type are: * `I-suggestion-causes-error` diff --git a/.github/ISSUE_TEMPLATE/false_positive.md b/.github/ISSUE_TEMPLATE/false_positive.md index 4170b9ff2db..82158e02f08 100644 --- a/.github/ISSUE_TEMPLATE/false_positive.md +++ b/.github/ISSUE_TEMPLATE/false_positive.md @@ -37,7 +37,7 @@ LLVM version: 10.0 Additional labels can be added to this issue by including the following command (without the space after the @ symbol): -`@rustbot label +<label>` +@ rustbot label +<label> Common labels for this issue type are: * I-suggestion-causes-error diff --git a/CHANGELOG.md b/CHANGELOG.md index f5ac2f7c9f8..58ea0f9ab9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,81 @@ document. ## Unreleased / In Rust Nightly -[74d1561...master](https://github.com/rust-lang/rust-clippy/compare/74d1561...master) +[7bfc26e...master](https://github.com/rust-lang/rust-clippy/compare/7bfc26e...master) + +## Rust 1.56 + +Current beta, release 2021-10-21 + +[74d1561...7bfc26e](https://github.com/rust-lang/rust-clippy/compare/74d1561...7bfc26e) + +### New Lints + +* [`unwrap_or_else_default`] + [#7516](https://github.com/rust-lang/rust-clippy/pull/7516) + +### Enhancements + +* [`needless_continue`]: Now also lints in `loop { continue; }` case + [#7477](https://github.com/rust-lang/rust-clippy/pull/7477) +* [`disallowed_type`]: Now also primitive types can be disallowed + [#7488](https://github.com/rust-lang/rust-clippy/pull/7488) +* [`manual_swap`]: Now also lints on xor swaps + [#7506](https://github.com/rust-lang/rust-clippy/pull/7506) +* [`map_flatten`]: Now also lints on the `Result` type + [#7522](https://github.com/rust-lang/rust-clippy/pull/7522) +* [`no_effect`]: Now also lints on inclusive ranges + [#7556](https://github.com/rust-lang/rust-clippy/pull/7556) + +### False Positive Fixes + +* [`nonstandard_macro_braces`]: No longer lints on similar named nested macros + [#7478](https://github.com/rust-lang/rust-clippy/pull/7478) +* [`too_many_lines`]: No longer lints in closures to avoid duplicated diagnostics + [#7534](https://github.com/rust-lang/rust-clippy/pull/7534) +* [`similar_names`]: No longer complains about `iter` and `item` being too + similar [#7546](https://github.com/rust-lang/rust-clippy/pull/7546) + +### Suggestion Fixes/Improvements + +* [`similar_names`]: No longer suggests to insert or add an underscore as a fix + [#7221](https://github.com/rust-lang/rust-clippy/pull/7221) +* [`new_without_default`]: No longer shows the full qualified type path when + suggesting adding a `Default` implementation + [#7493](https://github.com/rust-lang/rust-clippy/pull/7493) +* [`while_let_on_iterator`]: Now suggests re-borrowing mutable references + [#7520](https://github.com/rust-lang/rust-clippy/pull/7520) +* [`extend_with_drain`]: Improve code suggestion for mutable and immutable + references [#7533](https://github.com/rust-lang/rust-clippy/pull/7533) +* [`trivially_copy_pass_by_ref`]: Now properly handles `Self` type + [#7535](https://github.com/rust-lang/rust-clippy/pull/7535) +* [`never_loop`]: Now suggests using `if let` instead of a `for` loop when + applicable [#7541](https://github.com/rust-lang/rust-clippy/pull/7541) + +### Documentation Improvements + +* Clippy now uses a lint to generate its lint documentation. [Lints all the way + down](https://en.wikipedia.org/wiki/Turtles_all_the_way_down). + [#7502](https://github.com/rust-lang/rust-clippy/pull/7502) +* Reworked Clippy's website: + [#7172](https://github.com/rust-lang/rust-clippy/issues/7172) + [#7279](https://github.com/rust-lang/rust-clippy/pull/7279) + * Added applicability information about lints + * Added a link to jump into the implementation + * Improved loading times + * Adapted some styling +* `cargo clippy --help` now also explains the `--fix` and `--no-deps` flag + [#7492](https://github.com/rust-lang/rust-clippy/pull/7492) +* [`unnested_or_patterns`]: Removed `or_patterns` feature gate in the code + example [#7507](https://github.com/rust-lang/rust-clippy/pull/7507) + +### New Lints + +* Renamed Lint: `if_let_some_result` is now called [`match_result_ok`]. Now also handles `while let` case. ## Rust 1.55 -Current beta, release 2021-09-09 +Current stable, released 2021-09-09 [3ae8faf...74d1561](https://github.com/rust-lang/rust-clippy/compare/3ae8faf...74d1561) @@ -126,21 +196,9 @@ Current beta, release 2021-09-09 * [`use_self`] [#7428](https://github.com/rust-lang/rust-clippy/pull/7428) -### Documentation Improvements - -* Reworked Clippy's website: - [#7279](https://github.com/rust-lang/rust-clippy/pull/7279) - [#7172](https://github.com/rust-lang/rust-clippy/issues/7172) - * Added applicability information about lints - * Added a link to jump into the implementation - * Improved loading times - * Adapted some styling -* Clippy now uses a lint to generate its documentation - [#7298](https://github.com/rust-lang/rust-clippy/pull/7298) - ## Rust 1.54 -Current stable, released 2021-07-29 +Released 2021-07-29 [7c7683c...3ae8faf](https://github.com/rust-lang/rust-clippy/compare/7c7683c...3ae8faf) @@ -1050,7 +1108,7 @@ Released 2020-11-19 [#5913](https://github.com/rust-lang/rust-clippy/pull/5913) * Add example of false positive to [`ptr_arg`] docs. [#5885](https://github.com/rust-lang/rust-clippy/pull/5885) -* [`box_vec`], [`vec_box`] and [`borrowed_box`]: add link to the documentation of `Box` +* [`box_vec`](https://rust-lang.github.io/rust-clippy/master/index.html#box_collection), [`vec_box`] and [`borrowed_box`]: add link to the documentation of `Box` [#6023](https://github.com/rust-lang/rust-clippy/pull/6023) ## Rust 1.47 @@ -1491,7 +1549,7 @@ Released 2020-03-12 * `unknown_clippy_lints` [#4963](https://github.com/rust-lang/rust-clippy/pull/4963) * [`explicit_into_iter_loop`] [#4978](https://github.com/rust-lang/rust-clippy/pull/4978) * [`useless_attribute`] [#5022](https://github.com/rust-lang/rust-clippy/pull/5022) -* [`if_let_some_result`] [#5032](https://github.com/rust-lang/rust-clippy/pull/5032) +* `if_let_some_result` [#5032](https://github.com/rust-lang/rust-clippy/pull/5032) ### ICE fixes @@ -2570,7 +2628,7 @@ Released 2018-09-13 [`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison [`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const [`borrowed_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrowed_box -[`box_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_vec +[`box_collection`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_collection [`boxed_local`]: https://rust-lang.github.io/rust-clippy/master/index.html#boxed_local [`branches_sharing_code`]: https://rust-lang.github.io/rust-clippy/master/index.html#branches_sharing_code [`builtin_type_shadow`]: https://rust-lang.github.io/rust-clippy/master/index.html#builtin_type_shadow @@ -2685,9 +2743,9 @@ Released 2018-09-13 [`identity_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_op [`if_let_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_mutex [`if_let_redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_redundant_pattern_matching -[`if_let_some_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_some_result [`if_not_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else [`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else +[`if_then_panic`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_panic [`if_then_some_else_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none [`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond [`implicit_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_clone @@ -2722,6 +2780,7 @@ Released 2018-09-13 [`iter_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_count [`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_not_returning_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_not_returning_iterator [`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 @@ -2773,6 +2832,7 @@ Released 2018-09-13 [`match_on_vec_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_on_vec_items [`match_overlapping_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_overlapping_arm [`match_ref_pats`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_ref_pats +[`match_result_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_result_ok [`match_same_arms`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms [`match_single_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding [`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm @@ -2905,6 +2965,7 @@ Released 2018-09-13 [`reversed_empty_ranges`]: https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges [`same_functions_in_if_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_functions_in_if_condition [`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push +[`same_name_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_name_method [`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some [`self_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_assignment [`self_named_constructors`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_constructors diff --git a/Cargo.toml b/Cargo.toml index 40aaa5924df..ba3ed3053ac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ license = "MIT OR Apache-2.0" keywords = ["clippy", "lint", "plugin"] categories = ["development-tools", "development-tools::cargo-plugins"] build = "build.rs" -edition = "2018" +edition = "2021" publish = false [[bin]] diff --git a/README.md b/README.md index aaf404eadea..822335fc65f 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the | `clippy::style` | code that should be written in a more idiomatic way | **warn** | | `clippy::complexity` | code that does something simple but in a complex way | **warn** | | `clippy::perf` | code that can be written to run faster | **warn** | -| `clippy::pedantic` | lints which are rather strict or might have false positives | allow | +| `clippy::pedantic` | lints which are rather strict or have occasional false positives | allow | | `clippy::nursery` | new lints that are still under development | allow | | `clippy::cargo` | lints for the cargo manifest | allow | diff --git a/clippy_dev/Cargo.toml b/clippy_dev/Cargo.toml index d7d2655026b..4a13a452409 100644 --- a/clippy_dev/Cargo.toml +++ b/clippy_dev/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "clippy_dev" version = "0.0.1" -edition = "2018" +edition = "2021" [dependencies] bytecount = "0.6" diff --git a/clippy_dev/src/bless.rs b/clippy_dev/src/bless.rs index c4fa0a9aca7..daf0fcc993b 100644 --- a/clippy_dev/src/bless.rs +++ b/clippy_dev/src/bless.rs @@ -1,7 +1,6 @@ //! `bless` updates the reference files in the repo with changed output files //! from the last test run. -use std::env; use std::ffi::OsStr; use std::fs; use std::lazy::SyncLazy; @@ -10,17 +9,9 @@ use walkdir::WalkDir; use crate::clippy_project_root; -// NOTE: this is duplicated with tests/cargo/mod.rs What to do? -pub static CARGO_TARGET_DIR: SyncLazy<PathBuf> = SyncLazy::new(|| match env::var_os("CARGO_TARGET_DIR") { - Some(v) => v.into(), - None => env::current_dir().unwrap().join("target"), -}); - static CLIPPY_BUILD_TIME: SyncLazy<Option<std::time::SystemTime>> = SyncLazy::new(|| { - let profile = env::var("PROFILE").unwrap_or_else(|_| "debug".to_string()); - let mut path = PathBuf::from(&**CARGO_TARGET_DIR); - path.push(profile); - path.push("cargo-clippy"); + let mut path = std::env::current_exe().unwrap(); + path.set_file_name("cargo-clippy"); fs::metadata(path).ok()?.modified().ok() }); @@ -94,10 +85,7 @@ fn updated_since_clippy_build(path: &Path) -> Option<bool> { } fn build_dir() -> PathBuf { - let profile = env::var("PROFILE").unwrap_or_else(|_| "debug".to_string()); - let mut path = PathBuf::new(); - path.push(CARGO_TARGET_DIR.clone()); - path.push(profile); - path.push("test_build_base"); + let mut path = std::env::current_exe().unwrap(); + path.set_file_name("test"); path } diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index e59175a55e1..7900dc6d041 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -6,7 +6,7 @@ repository = "https://github.com/rust-lang/rust-clippy" readme = "README.md" license = "MIT OR Apache-2.0" keywords = ["clippy", "lint", "plugin"] -edition = "2018" +edition = "2021" [dependencies] cargo_metadata = "0.12" diff --git a/clippy_lints/src/derivable_impls.rs b/clippy_lints/src/derivable_impls.rs index b4c4ca016aa..15252ef96cd 100644 --- a/clippy_lints/src/derivable_impls.rs +++ b/clippy_lints/src/derivable_impls.rs @@ -2,10 +2,9 @@ use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::{in_macro, is_automatically_derived, is_default_equivalent, remove_blocks}; use rustc_hir::{ def::{DefKind, Res}, - Body, Expr, ExprKind, Impl, ImplItemKind, Item, ItemKind, Node, QPath, + Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, TyKind, }; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::TypeFoldable; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -68,6 +67,7 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls { if let ItemKind::Impl(Impl { of_trait: Some(ref trait_ref), items: [child], + self_ty, .. }) = item.kind; if let attrs = cx.tcx.hir().attrs(item.hir_id()); @@ -80,9 +80,18 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls { if let ImplItemKind::Fn(_, b) = &impl_item.kind; if let Body { value: func_expr, .. } = cx.tcx.hir().body(*b); if let Some(adt_def) = cx.tcx.type_of(item.def_id).ty_adt_def(); + if !attrs.iter().any(|attr| attr.doc_str().is_some()); + if let child_attrs = cx.tcx.hir().attrs(impl_item_hir); + if !child_attrs.iter().any(|attr| attr.doc_str().is_some()); then { - if cx.tcx.type_of(item.def_id).definitely_has_param_types_or_consts(cx.tcx) { - return; + if let TyKind::Path(QPath::Resolved(_, p)) = self_ty.kind { + if let Some(PathSegment { args: Some(a), .. }) = p.segments.last() { + for arg in a.args { + if !matches!(arg, GenericArg::Lifetime(_)) { + return; + } + } + } } let should_emit = match remove_blocks(func_expr).kind { ExprKind::Tup(fields) => fields.iter().all(|e| is_default_equivalent(cx, e)), diff --git a/clippy_lints/src/disallowed_method.rs b/clippy_lints/src/disallowed_method.rs index 7069cb4198c..1167b26c8f1 100644 --- a/clippy_lints/src/disallowed_method.rs +++ b/clippy_lints/src/disallowed_method.rs @@ -1,33 +1,44 @@ -use clippy_utils::diagnostics::span_lint; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::fn_def_id; -use rustc_data_structures::fx::FxHashSet; -use rustc_hir::{def::Res, def_id::DefId, Crate, Expr}; +use rustc_hir::{def::Res, def_id::DefIdMap, Crate, Expr}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::Symbol; + +use crate::utils::conf; declare_clippy_lint! { /// ### What it does /// Denies the configured methods and functions in clippy.toml /// /// ### Why is this bad? - /// Some methods are undesirable in certain contexts, - /// and it's beneficial to lint for them as needed. + /// Some methods are undesirable in certain contexts, and it's beneficial to + /// lint for them as needed. /// /// ### Example /// An example clippy.toml configuration: /// ```toml /// # clippy.toml - /// disallowed-methods = ["std::vec::Vec::leak", "std::time::Instant::now"] + /// disallowed-methods = [ + /// # Can use a string as the path of the disallowed method. + /// "std::boxed::Box::new", + /// # Can also use an inline table with a `path` key. + /// { path = "std::time::Instant::now" }, + /// # When using an inline table, can add a `reason` for why the method + /// # is disallowed. + /// { path = "std::vec::Vec::leak", reason = "no leaking memory" }, + /// ] /// ``` /// /// ```rust,ignore /// // Example code where clippy issues a warning /// let xs = vec![1, 2, 3, 4]; /// xs.leak(); // Vec::leak is disallowed in the config. + /// // The diagnostic contains the message "no leaking memory". /// /// let _now = Instant::now(); // Instant::now is disallowed in the config. + /// + /// let _box = Box::new(3); // Box::new is disallowed in the config. /// ``` /// /// Use instead: @@ -43,18 +54,15 @@ declare_clippy_lint! { #[derive(Clone, Debug)] pub struct DisallowedMethod { - disallowed: FxHashSet<Vec<Symbol>>, - def_ids: FxHashSet<(DefId, Vec<Symbol>)>, + conf_disallowed: Vec<conf::DisallowedMethod>, + disallowed: DefIdMap<Option<String>>, } impl DisallowedMethod { - pub fn new(disallowed: &FxHashSet<String>) -> Self { + pub fn new(conf_disallowed: Vec<conf::DisallowedMethod>) -> Self { Self { - disallowed: disallowed - .iter() - .map(|s| s.split("::").map(|seg| Symbol::intern(seg)).collect::<Vec<_>>()) - .collect(), - def_ids: FxHashSet::default(), + conf_disallowed, + disallowed: DefIdMap::default(), } } } @@ -63,32 +71,36 @@ impl_lint_pass!(DisallowedMethod => [DISALLOWED_METHOD]); impl<'tcx> LateLintPass<'tcx> for DisallowedMethod { fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) { - for path in &self.disallowed { - let segs = path.iter().map(ToString::to_string).collect::<Vec<_>>(); - if let Res::Def(_, id) = clippy_utils::path_to_res(cx, &segs.iter().map(String::as_str).collect::<Vec<_>>()) - { - self.def_ids.insert((id, path.clone())); + for conf in &self.conf_disallowed { + let (path, reason) = match conf { + conf::DisallowedMethod::Simple(path) => (path, None), + conf::DisallowedMethod::WithReason { path, reason } => ( + path, + reason.as_ref().map(|reason| format!("{} (from clippy.toml)", reason)), + ), + }; + let segs: Vec<_> = path.split("::").collect(); + if let Res::Def(_, id) = clippy_utils::path_to_res(cx, &segs) { + self.disallowed.insert(id, reason); } } } fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let Some(def_id) = fn_def_id(cx, expr) { - if self.def_ids.iter().any(|(id, _)| def_id == *id) { - let func_path = cx.get_def_path(def_id); - let func_path_string = func_path - .into_iter() - .map(Symbol::to_ident_string) - .collect::<Vec<_>>() - .join("::"); - - span_lint( - cx, - DISALLOWED_METHOD, - expr.span, - &format!("use of a disallowed method `{}`", func_path_string), - ); + let def_id = match fn_def_id(cx, expr) { + Some(def_id) => def_id, + None => return, + }; + let reason = match self.disallowed.get(&def_id) { + Some(reason) => reason, + None => return, + }; + let func_path = cx.tcx.def_path_str(def_id); + let msg = format!("use of a disallowed method `{}`", func_path); + span_lint_and_then(cx, DISALLOWED_METHOD, expr.span, &msg, |diag| { + if let Some(reason) = reason { + diag.note(reason); } - } + }); } } diff --git a/clippy_lints/src/disallowed_type.rs b/clippy_lints/src/disallowed_type.rs index e627168b932..6c861fb33a9 100644 --- a/clippy_lints/src/disallowed_type.rs +++ b/clippy_lints/src/disallowed_type.rs @@ -48,7 +48,7 @@ impl DisallowedType { Self { disallowed: disallowed .iter() - .map(|s| s.split("::").map(|seg| Symbol::intern(seg)).collect::<Vec<_>>()) + .map(|s| s.split("::").map(Symbol::intern).collect::<Vec<_>>()) .collect(), def_ids: FxHashSet::default(), prim_tys: FxHashSet::default(), diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs index 6bbac6d9a24..090be73af3b 100644 --- a/clippy_lints/src/escape.rs +++ b/clippy_lints/src/escape.rs @@ -91,8 +91,11 @@ impl<'tcx> LateLintPass<'tcx> for BoxedLocal { if trait_item.id.hir_id() == hir_id { // be sure we have `self` parameter in this function if let AssocItemKind::Fn { has_self: true } = trait_item.kind { - trait_self_ty = - Some(TraitRef::identity(cx.tcx, trait_item.id.def_id.to_def_id()).self_ty().skip_binder()); + trait_self_ty = Some( + TraitRef::identity(cx.tcx, trait_item.id.def_id.to_def_id()) + .self_ty() + .skip_binder(), + ); } } } diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs index f6a64a8ca60..9df92cc5b64 100644 --- a/clippy_lints/src/eta_reduction.rs +++ b/clippy_lints/src/eta_reduction.rs @@ -1,16 +1,16 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use clippy_utils::higher::VecArgs; use clippy_utils::source::snippet_opt; -use clippy_utils::ty::{implements_trait, type_is_unsafe_function}; use clippy_utils::usage::UsedAfterExprVisitor; -use clippy_utils::{get_enclosing_loop_or_closure, higher}; -use clippy_utils::{is_adjusted, iter_input_pats}; +use clippy_utils::{get_enclosing_loop_or_closure, higher, path_to_local_id}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{def_id, Expr, ExprKind, Param, PatKind, QPath}; -use rustc_lint::{LateContext, LateLintPass, LintContext}; -use rustc_middle::lint::in_external_macro; -use rustc_middle::ty::{self, ClosureKind, Ty}; +use rustc_hir::def_id::DefId; +use rustc_hir::{Expr, ExprKind, Param, PatKind, Unsafety}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow}; +use rustc_middle::ty::subst::Subst; +use rustc_middle::ty::{self, ClosureKind, Ty, TypeFoldable}; use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { @@ -52,12 +52,6 @@ declare_clippy_lint! { /// ### Why is this bad? /// It's unnecessary to create the closure. /// - /// ### Known problems - /// [#3071](https://github.com/rust-lang/rust-clippy/issues/3071), - /// [#3942](https://github.com/rust-lang/rust-clippy/issues/3942), - /// [#4002](https://github.com/rust-lang/rust-clippy/issues/4002) - /// - /// /// ### Example /// ```rust,ignore /// Some('a').map(|s| s.to_uppercase()); @@ -75,32 +69,16 @@ declare_lint_pass!(EtaReduction => [REDUNDANT_CLOSURE, REDUNDANT_CLOSURE_FOR_MET impl<'tcx> LateLintPass<'tcx> for EtaReduction { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if in_external_macro(cx.sess(), expr.span) { + if expr.span.from_expansion() { return; } - - match expr.kind { - ExprKind::Call(_, args) | ExprKind::MethodCall(_, _, args, _) => { - for arg in args { - // skip `foo(macro!())` - if arg.span.ctxt() == expr.span.ctxt() { - check_closure(cx, arg); - } - } - }, - _ => (), - } - } -} - -fn check_closure<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let ExprKind::Closure(_, decl, eid, _, _) = expr.kind { - let body = cx.tcx.hir().body(eid); - let ex = &body.value; - - if ex.span.ctxt() != expr.span.ctxt() { - if decl.inputs.is_empty() { - if let Some(VecArgs::Vec(&[])) = higher::VecArgs::hir(cx, ex) { + let body = match expr.kind { + ExprKind::Closure(_, _, id, _, _) => cx.tcx.hir().body(id), + _ => return, + }; + if body.value.span.from_expansion() { + if body.params.is_empty() { + if let Some(VecArgs::Vec(&[])) = higher::VecArgs::hir(cx, &body.value) { // replace `|| vec![]` with `Vec::new` span_lint_and_sugg( cx, @@ -117,33 +95,30 @@ fn check_closure<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { return; } + let closure_ty = cx.typeck_results().expr_ty(expr); + if_chain!( - if let ExprKind::Call(caller, args) = ex.kind; - - if let ExprKind::Path(_) = caller.kind; - - // Not the same number of arguments, there is no way the closure is the same as the function return; - if args.len() == decl.inputs.len(); - - // Are the expression or the arguments type-adjusted? Then we need the closure - if !(is_adjusted(cx, ex) || args.iter().any(|arg| is_adjusted(cx, arg))); - - let fn_ty = cx.typeck_results().expr_ty(caller); - - if matches!(fn_ty.kind(), ty::FnDef(_, _) | ty::FnPtr(_) | ty::Closure(_, _)); - - if !type_is_unsafe_function(cx, fn_ty); - - if compare_inputs(&mut iter_input_pats(decl, body), &mut args.iter()); - + if let ExprKind::Call(callee, args) = body.value.kind; + if let ExprKind::Path(_) = callee.kind; + if check_inputs(cx, body.params, args); + let callee_ty = cx.typeck_results().expr_ty_adjusted(callee); + let call_ty = cx.typeck_results().type_dependent_def_id(body.value.hir_id) + .map_or(callee_ty, |id| cx.tcx.type_of(id)); + if check_sig(cx, closure_ty, call_ty); + let substs = cx.typeck_results().node_substs(callee.hir_id); + // This fixes some false positives that I don't entirely understand + if substs.is_empty() || !cx.typeck_results().expr_ty(expr).has_late_bound_regions(); + // A type param function ref like `T::f` is not 'static, however + // it is if cast like `T::f as fn()`. This seems like a rustc bug. + if !substs.types().any(|t| matches!(t.kind(), ty::Param(_))); then { span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure", |diag| { - if let Some(mut snippet) = snippet_opt(cx, caller.span) { + if let Some(mut snippet) = snippet_opt(cx, callee.span) { if_chain! { - if let ty::Closure(_, substs) = fn_ty.kind(); + if let ty::Closure(_, substs) = callee_ty.peel_refs().kind(); if let ClosureKind::FnMut = substs.as_closure().kind(); - if UsedAfterExprVisitor::is_found(cx, caller) - || get_enclosing_loop_or_closure(cx.tcx, expr).is_some(); + if get_enclosing_loop_or_closure(cx.tcx, expr).is_some() + || UsedAfterExprVisitor::is_found(cx, callee); then { // Mutable closure is used after current expr; we cannot consume it. @@ -162,110 +137,79 @@ fn check_closure<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { ); if_chain!( - if let ExprKind::MethodCall(path, _, args, _) = ex.kind; - - // Not the same number of arguments, there is no way the closure is the same as the function return; - if args.len() == decl.inputs.len(); - - // Are the expression or the arguments type-adjusted? Then we need the closure - if !(is_adjusted(cx, ex) || args.iter().skip(1).any(|arg| is_adjusted(cx, arg))); - - let method_def_id = cx.typeck_results().type_dependent_def_id(ex.hir_id).unwrap(); - if !type_is_unsafe_function(cx, cx.tcx.type_of(method_def_id)); - - if compare_inputs(&mut iter_input_pats(decl, body), &mut args.iter()); - - if let Some(name) = get_ufcs_type_name(cx, method_def_id, &args[0]); - + if let ExprKind::MethodCall(path, _, args, _) = body.value.kind; + if check_inputs(cx, body.params, args); + let method_def_id = cx.typeck_results().type_dependent_def_id(body.value.hir_id).unwrap(); + let substs = cx.typeck_results().node_substs(body.value.hir_id); + let call_ty = cx.tcx.type_of(method_def_id).subst(cx.tcx, substs); + if check_sig(cx, closure_ty, call_ty); then { - span_lint_and_sugg( - cx, - REDUNDANT_CLOSURE_FOR_METHOD_CALLS, - expr.span, - "redundant closure", - "replace the closure with the method itself", - format!("{}::{}", name, path.ident.name), - Applicability::MachineApplicable, - ); + span_lint_and_then(cx, REDUNDANT_CLOSURE_FOR_METHOD_CALLS, expr.span, "redundant closure", |diag| { + let name = get_ufcs_type_name(cx, method_def_id); + diag.span_suggestion( + expr.span, + "replace the closure with the method itself", + format!("{}::{}", name, path.ident.name), + Applicability::MachineApplicable, + ); + }) } ); } } -/// Tries to determine the type for universal function call to be used instead of the closure -fn get_ufcs_type_name(cx: &LateContext<'_>, method_def_id: def_id::DefId, self_arg: &Expr<'_>) -> Option<String> { - let expected_type_of_self = &cx.tcx.fn_sig(method_def_id).inputs_and_output().skip_binder()[0]; - let actual_type_of_self = &cx.typeck_results().node_type(self_arg.hir_id); - - if let Some(trait_id) = cx.tcx.trait_of_item(method_def_id) { - if match_borrow_depth(expected_type_of_self, actual_type_of_self) - && implements_trait(cx, actual_type_of_self, trait_id, &[]) - { - return Some(cx.tcx.def_path_str(trait_id)); - } +fn check_inputs(cx: &LateContext<'_>, params: &[Param<'_>], call_args: &[Expr<'_>]) -> bool { + if params.len() != call_args.len() { + return false; } - - cx.tcx.impl_of_method(method_def_id).and_then(|_| { - //a type may implicitly implement other type's methods (e.g. Deref) - if match_types(expected_type_of_self, actual_type_of_self) { - Some(get_type_name(cx, actual_type_of_self)) - } else { - None + std::iter::zip(params, call_args).all(|(param, arg)| { + match param.pat.kind { + PatKind::Binding(_, id, ..) if path_to_local_id(arg, id) => {}, + _ => return false, + } + match *cx.typeck_results().expr_adjustments(arg) { + [] => true, + [Adjustment { + kind: Adjust::Deref(None), + .. + }, Adjustment { + kind: Adjust::Borrow(AutoBorrow::Ref(_, mu2)), + .. + }] => { + // re-borrow with the same mutability is allowed + let ty = cx.typeck_results().expr_ty(arg); + matches!(*ty.kind(), ty::Ref(.., mu1) if mu1 == mu2.into()) + }, + _ => false, } }) } -fn match_borrow_depth(lhs: Ty<'_>, rhs: Ty<'_>) -> bool { - match (&lhs.kind(), &rhs.kind()) { - (ty::Ref(_, t1, mut1), ty::Ref(_, t2, mut2)) => mut1 == mut2 && match_borrow_depth(t1, t2), - (l, r) => !matches!((l, r), (ty::Ref(_, _, _), _) | (_, ty::Ref(_, _, _))), +fn check_sig<'tcx>(cx: &LateContext<'tcx>, closure_ty: Ty<'tcx>, call_ty: Ty<'tcx>) -> bool { + let call_sig = call_ty.fn_sig(cx.tcx); + if call_sig.unsafety() == Unsafety::Unsafe { + return false; } + if !closure_ty.has_late_bound_regions() { + return true; + } + let substs = match closure_ty.kind() { + ty::Closure(_, substs) => substs, + _ => return false, + }; + let closure_sig = cx.tcx.signature_unclosure(substs.as_closure().sig(), Unsafety::Normal); + cx.tcx.erase_late_bound_regions(closure_sig) == cx.tcx.erase_late_bound_regions(call_sig) } -fn match_types(lhs: Ty<'_>, rhs: Ty<'_>) -> bool { - match (&lhs.kind(), &rhs.kind()) { - (ty::Bool, ty::Bool) - | (ty::Char, ty::Char) - | (ty::Int(_), ty::Int(_)) - | (ty::Uint(_), ty::Uint(_)) - | (ty::Str, ty::Str) => true, - (ty::Ref(_, t1, mut1), ty::Ref(_, t2, mut2)) => mut1 == mut2 && match_types(t1, t2), - (ty::Array(t1, _), ty::Array(t2, _)) | (ty::Slice(t1), ty::Slice(t2)) => match_types(t1, t2), - (ty::Adt(def1, _), ty::Adt(def2, _)) => def1 == def2, - (_, _) => false, - } -} - -fn get_type_name(cx: &LateContext<'_>, ty: Ty<'_>) -> String { - match ty.kind() { - ty::Adt(t, _) => cx.tcx.def_path_str(t.did), - ty::Ref(_, r, _) => get_type_name(cx, r), - _ => ty.to_string(), - } -} - -fn compare_inputs( - closure_inputs: &mut dyn Iterator<Item = &Param<'_>>, - call_args: &mut dyn Iterator<Item = &Expr<'_>>, -) -> bool { - for (closure_input, function_arg) in closure_inputs.zip(call_args) { - if let PatKind::Binding(_, _, ident, _) = closure_input.pat.kind { - // XXXManishearth Should I be checking the binding mode here? - if let ExprKind::Path(QPath::Resolved(None, p)) = function_arg.kind { - if p.segments.len() != 1 { - // If it's a proper path, it can't be a local variable - return false; - } - if p.segments[0].ident.name != ident.name { - // The two idents should be the same - return false; - } - } else { - return false; +fn get_ufcs_type_name(cx: &LateContext<'_>, method_def_id: DefId) -> String { + match cx.tcx.associated_item(method_def_id).container { + ty::TraitContainer(def_id) => cx.tcx.def_path_str(def_id), + ty::ImplContainer(def_id) => { + let ty = cx.tcx.type_of(def_id); + match ty.kind() { + ty::Adt(adt, _) => cx.tcx.def_path_str(adt.did), + _ => ty.to_string(), } - } else { - return false; - } + }, } - true } diff --git a/clippy_lints/src/float_literal.rs b/clippy_lints/src/float_literal.rs index a3d70f31f00..1e8a5bd7d34 100644 --- a/clippy_lints/src/float_literal.rs +++ b/clippy_lints/src/float_literal.rs @@ -111,7 +111,7 @@ impl<'tcx> LateLintPass<'tcx> for FloatLiteral { Applicability::MachineApplicable, ); } - } else if digits > max as usize && sym_str != float_str { + } else if digits > max as usize && float_str.len() < sym_str.len() { span_lint_and_sugg( cx, EXCESSIVE_PRECISION, diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index 863c606f5a9..508cac33848 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -69,8 +69,8 @@ impl<'tcx> LateLintPass<'tcx> for UselessFormat { ty::Str => true, _ => false, }; - if format_args.args.iter().all(|e| is_display_arg(e)); - if format_args.fmt_expr.map_or(true, |e| check_unformatted(e)); + if format_args.args.iter().all(is_display_arg); + if format_args.fmt_expr.map_or(true, check_unformatted); then { let is_new_string = match value.kind { ExprKind::Binary(..) => true, diff --git a/clippy_lints/src/formatting.rs b/clippy_lints/src/formatting.rs index 4dd0ffe77ea..b4f186525c5 100644 --- a/clippy_lints/src/formatting.rs +++ b/clippy_lints/src/formatting.rs @@ -286,34 +286,39 @@ fn check_array(cx: &EarlyContext<'_>, expr: &Expr) { } fn check_missing_else(cx: &EarlyContext<'_>, first: &Expr, second: &Expr) { - if !differing_macro_contexts(first.span, second.span) - && !first.span.from_expansion() - && is_if(first) - && (is_block(second) || is_if(second)) - { - // where the else would be + if_chain! { + if !differing_macro_contexts(first.span, second.span); + if !first.span.from_expansion(); + if let ExprKind::If(cond_expr, ..) = &first.kind; + if is_block(second) || is_if(second); + + // Proc-macros can give weird spans. Make sure this is actually an `if`. + if let Some(if_snip) = snippet_opt(cx, first.span.until(cond_expr.span)); + if if_snip.starts_with("if"); + + // If there is a line break between the two expressions, don't lint. + // If there is a non-whitespace character, this span came from a proc-macro. let else_span = first.span.between(second.span); + if let Some(else_snippet) = snippet_opt(cx, else_span); + if !else_snippet.chars().any(|c| c == '\n' || !c.is_whitespace()); + then { + let (looks_like, next_thing) = if is_if(second) { + ("an `else if`", "the second `if`") + } else { + ("an `else {..}`", "the next block") + }; - if let Some(else_snippet) = snippet_opt(cx, else_span) { - if !else_snippet.contains('\n') { - let (looks_like, next_thing) = if is_if(second) { - ("an `else if`", "the second `if`") - } else { - ("an `else {..}`", "the next block") - }; - - span_lint_and_note( - cx, - SUSPICIOUS_ELSE_FORMATTING, - else_span, - &format!("this looks like {} but the `else` is missing", looks_like), - None, - &format!( - "to remove this lint, add the missing `else` or add a new line before {}", - next_thing, - ), - ); - } + span_lint_and_note( + cx, + SUSPICIOUS_ELSE_FORMATTING, + else_span, + &format!("this looks like {} but the `else` is missing", looks_like), + None, + &format!( + "to remove this lint, add the missing `else` or add a new line before {}", + next_thing, + ), + ); } } } diff --git a/clippy_lints/src/if_then_panic.rs b/clippy_lints/src/if_then_panic.rs new file mode 100644 index 00000000000..ee575c81a8b --- /dev/null +++ b/clippy_lints/src/if_then_panic.rs @@ -0,0 +1,97 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::higher::PanicExpn; +use clippy_utils::is_expn_of; +use clippy_utils::source::snippet_with_applicability; +use rustc_errors::Applicability; +use rustc_hir::{Block, Expr, ExprKind, StmtKind, UnOp}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// Detects `if`-then-`panic!` that can be replaced with `assert!`. + /// + /// ### Why is this bad? + /// `assert!` is simpler than `if`-then-`panic!`. + /// + /// ### Example + /// ```rust + /// let sad_people: Vec<&str> = vec![]; + /// if !sad_people.is_empty() { + /// panic!("there are sad people: {:?}", sad_people); + /// } + /// ``` + /// Use instead: + /// ```rust + /// let sad_people: Vec<&str> = vec![]; + /// assert!(sad_people.is_empty(), "there are sad people: {:?}", sad_people); + /// ``` + pub IF_THEN_PANIC, + style, + "`panic!` and only a `panic!` in `if`-then statement" +} + +declare_lint_pass!(IfThenPanic => [IF_THEN_PANIC]); + +impl LateLintPass<'_> for IfThenPanic { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + if_chain! { + if let Expr { + kind: ExprKind:: If(cond, Expr { + kind: ExprKind::Block( + Block { + stmts: [stmt], + .. + }, + _), + .. + }, None), + .. + } = &expr; + if is_expn_of(stmt.span, "panic").is_some(); + if !matches!(cond.kind, ExprKind::Let(_, _, _)); + if let StmtKind::Semi(semi) = stmt.kind; + if !cx.tcx.sess.source_map().is_multiline(cond.span); + + then { + let span = if let Some(panic_expn) = PanicExpn::parse(semi) { + match *panic_expn.format_args.value_args { + [] => panic_expn.format_args.format_string_span, + [.., last] => panic_expn.format_args.format_string_span.to(last.span), + } + } else { + if_chain! { + if let ExprKind::Block(block, _) = semi.kind; + if let Some(init) = block.expr; + if let ExprKind::Call(_, [format_args]) = init.kind; + + then { + format_args.span + } else { + return + } + } + }; + let mut applicability = Applicability::MachineApplicable; + let sugg = snippet_with_applicability(cx, span, "..", &mut applicability); + + let cond_sugg = + if let ExprKind::DropTemps(Expr{kind: ExprKind::Unary(UnOp::Not, not_expr), ..}) = cond.kind { + snippet_with_applicability(cx, not_expr.span, "..", &mut applicability).to_string() + } else { + format!("!{}", snippet_with_applicability(cx, cond.span, "..", &mut applicability)) + }; + + span_lint_and_sugg( + cx, + IF_THEN_PANIC, + expr.span, + "only a `panic!` in `if`-then statement", + "try", + format!("assert!({}, {});", cond_sugg, sugg), + Applicability::MachineApplicable, + ); + } + } + } +} diff --git a/clippy_lints/src/iter_not_returning_iterator.rs b/clippy_lints/src/iter_not_returning_iterator.rs new file mode 100644 index 00000000000..6c779348ca2 --- /dev/null +++ b/clippy_lints/src/iter_not_returning_iterator.rs @@ -0,0 +1,64 @@ +use clippy_utils::{diagnostics::span_lint, return_ty, ty::implements_trait}; +use rustc_hir::{ImplItem, ImplItemKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::kw; +use rustc_span::symbol::sym; + +declare_clippy_lint! { + /// ### What it does + /// Detects methods named `iter` or `iter_mut` that do not have a return type that implements `Iterator`. + /// + /// ### Why is this bad? + /// Methods named `iter` or `iter_mut` conventionally return an `Iterator`. + /// + /// ### Example + /// ```rust + /// // `String` does not implement `Iterator` + /// struct Data {} + /// impl Data { + /// fn iter(&self) -> String { + /// todo!() + /// } + /// } + /// ``` + /// Use instead: + /// ```rust + /// use std::str::Chars; + /// struct Data {} + /// impl Data { + /// fn iter(&self) -> Chars<'static> { + /// todo!() + /// } + /// } + /// ``` + pub ITER_NOT_RETURNING_ITERATOR, + pedantic, + "methods named `iter` or `iter_mut` that do not return an `Iterator`" +} + +declare_lint_pass!(IterNotReturningIterator => [ITER_NOT_RETURNING_ITERATOR]); + +impl LateLintPass<'_> for IterNotReturningIterator { + fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'tcx>) { + let name: &str = &impl_item.ident.name.as_str(); + if_chain! { + if let ImplItemKind::Fn(fn_sig, _) = &impl_item.kind; + let ret_ty = return_ty(cx, impl_item.hir_id()); + if matches!(name, "iter" | "iter_mut"); + if let [param] = cx.tcx.fn_arg_names(impl_item.def_id); + if param.name == kw::SelfLower; + if let Some(iter_trait_id) = cx.tcx.get_diagnostic_item(sym::Iterator); + if !implements_trait(cx, ret_ty, iter_trait_id, &[]); + + then { + span_lint( + cx, + ITER_NOT_RETURNING_ITERATOR, + fn_sig.span, + &format!("this method is named `{}` but its return type does not implement `Iterator`", name), + ); + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index acc78840bb9..89724917482 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -225,8 +225,8 @@ mod future_not_send; mod get_last_with_len; mod identity_op; mod if_let_mutex; -mod if_let_some_result; mod if_not_else; +mod if_then_panic; mod if_then_some_else_none; mod implicit_hasher; mod implicit_return; @@ -241,6 +241,7 @@ mod int_plus_one; mod integer_division; mod invalid_upcast_comparisons; mod items_after_statements; +mod iter_not_returning_iterator; mod large_const_arrays; mod large_enum_variant; mod large_stack_arrays; @@ -262,6 +263,7 @@ mod map_clone; mod map_err_ignore; mod map_unit_fn; mod match_on_vec_items; +mod match_result_ok; mod matches; mod mem_discriminant; mod mem_forget; @@ -330,6 +332,7 @@ mod reference; mod regex; mod repeat_once; mod returns; +mod same_name_method; mod self_assignment; mod self_named_constructors; mod semicolon_if_nothing_returned; @@ -655,8 +658,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: get_last_with_len::GET_LAST_WITH_LEN, identity_op::IDENTITY_OP, if_let_mutex::IF_LET_MUTEX, - if_let_some_result::IF_LET_SOME_RESULT, if_not_else::IF_NOT_ELSE, + if_then_panic::IF_THEN_PANIC, if_then_some_else_none::IF_THEN_SOME_ELSE_NONE, implicit_hasher::IMPLICIT_HASHER, implicit_return::IMPLICIT_RETURN, @@ -674,6 +677,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: integer_division::INTEGER_DIVISION, invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS, items_after_statements::ITEMS_AFTER_STATEMENTS, + iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR, large_const_arrays::LARGE_CONST_ARRAYS, large_enum_variant::LARGE_ENUM_VARIANT, large_stack_arrays::LARGE_STACK_ARRAYS, @@ -723,6 +727,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: map_unit_fn::OPTION_MAP_UNIT_FN, map_unit_fn::RESULT_MAP_UNIT_FN, match_on_vec_items::MATCH_ON_VEC_ITEMS, + match_result_ok::MATCH_RESULT_OK, matches::INFALLIBLE_DESTRUCTURING_MATCH, matches::MATCH_AS_REF, matches::MATCH_BOOL, @@ -908,6 +913,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: repeat_once::REPEAT_ONCE, returns::LET_AND_RETURN, returns::NEEDLESS_RETURN, + same_name_method::SAME_NAME_METHOD, self_assignment::SELF_ASSIGNMENT, self_named_constructors::SELF_NAMED_CONSTRUCTORS, semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED, @@ -952,7 +958,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: transmuting_null::TRANSMUTING_NULL, try_err::TRY_ERR, types::BORROWED_BOX, - types::BOX_VEC, + types::BOX_COLLECTION, types::LINKEDLIST, types::OPTION_OPTION, types::RC_BUFFER, @@ -1051,6 +1057,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(panic_unimplemented::UNIMPLEMENTED), LintId::of(panic_unimplemented::UNREACHABLE), LintId::of(pattern_type_mismatch::PATTERN_TYPE_MISMATCH), + LintId::of(same_name_method::SAME_NAME_METHOD), LintId::of(shadow::SHADOW_REUSE), LintId::of(shadow::SHADOW_SAME), LintId::of(strings::STRING_ADD), @@ -1104,6 +1111,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(infinite_iter::MAYBE_INFINITE_ITER), LintId::of(invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS), LintId::of(items_after_statements::ITEMS_AFTER_STATEMENTS), + LintId::of(iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR), LintId::of(large_stack_arrays::LARGE_STACK_ARRAYS), LintId::of(let_underscore::LET_UNDERSCORE_DROP), LintId::of(literal_representation::LARGE_DIGIT_GROUPS), @@ -1126,6 +1134,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(methods::INEFFICIENT_TO_STRING), LintId::of(methods::MAP_FLATTEN), LintId::of(methods::MAP_UNWRAP_OR), + LintId::of(misc::FLOAT_CMP), LintId::of(misc::USED_UNDERSCORE_BINDING), LintId::of(misc_early::UNSEPARATED_LITERAL_SUFFIX), LintId::of(mut_mut::MUT_MUT), @@ -1134,6 +1143,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(needless_continue::NEEDLESS_CONTINUE), LintId::of(needless_for_each::NEEDLESS_FOR_EACH), LintId::of(needless_pass_by_value::NEEDLESS_PASS_BY_VALUE), + LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES), LintId::of(non_expressive_names::SIMILAR_NAMES), LintId::of(pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE), LintId::of(pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF), @@ -1249,7 +1259,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(get_last_with_len::GET_LAST_WITH_LEN), LintId::of(identity_op::IDENTITY_OP), LintId::of(if_let_mutex::IF_LET_MUTEX), - LintId::of(if_let_some_result::IF_LET_SOME_RESULT), + LintId::of(if_then_panic::IF_THEN_PANIC), LintId::of(indexing_slicing::OUT_OF_BOUNDS_INDEXING), LintId::of(infinite_iter::INFINITE_ITER), LintId::of(inherent_to_string::INHERENT_TO_STRING), @@ -1292,6 +1302,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(map_clone::MAP_CLONE), LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN), LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN), + LintId::of(match_result_ok::MATCH_RESULT_OK), LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(matches::MATCH_AS_REF), LintId::of(matches::MATCH_LIKE_MATCHES_MACRO), @@ -1358,7 +1369,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(minmax::MIN_MAX), LintId::of(misc::CMP_NAN), LintId::of(misc::CMP_OWNED), - LintId::of(misc::FLOAT_CMP), LintId::of(misc::MODULO_ONE), LintId::of(misc::SHORT_CIRCUIT_STATEMENT), LintId::of(misc::TOPLEVEL_REF_ARG), @@ -1390,7 +1400,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(non_copy_const::BORROW_INTERIOR_MUTABLE_CONST), LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST), LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), - LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES), LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS), LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS), LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP), @@ -1448,7 +1457,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(transmuting_null::TRANSMUTING_NULL), LintId::of(try_err::TRY_ERR), LintId::of(types::BORROWED_BOX), - LintId::of(types::BOX_VEC), + LintId::of(types::BOX_COLLECTION), LintId::of(types::REDUNDANT_ALLOCATION), LintId::of(types::TYPE_COMPLEXITY), LintId::of(types::VEC_BOX), @@ -1504,7 +1513,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(functions::DOUBLE_MUST_USE), LintId::of(functions::MUST_USE_UNIT), LintId::of(functions::RESULT_UNIT_ERR), - LintId::of(if_let_some_result::IF_LET_SOME_RESULT), + LintId::of(if_then_panic::IF_THEN_PANIC), LintId::of(inherent_to_string::INHERENT_TO_STRING), LintId::of(len_zero::COMPARISON_TO_EMPTY), LintId::of(len_zero::LEN_WITHOUT_IS_EMPTY), @@ -1520,6 +1529,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(manual_map::MANUAL_MAP), LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE), LintId::of(map_clone::MAP_CLONE), + LintId::of(match_result_ok::MATCH_RESULT_OK), LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(matches::MATCH_LIKE_MATCHES_MACRO), LintId::of(matches::MATCH_OVERLAPPING_ARM), @@ -1564,7 +1574,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(non_copy_const::BORROW_INTERIOR_MUTABLE_CONST), LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST), LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), - LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES), LintId::of(ptr::CMP_NULL), LintId::of(ptr::PTR_ARG), LintId::of(ptr_eq::PTR_EQ), @@ -1724,7 +1733,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(methods::ZST_OFFSET), LintId::of(minmax::MIN_MAX), LintId::of(misc::CMP_NAN), - LintId::of(misc::FLOAT_CMP), LintId::of(misc::MODULO_ONE), LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS), LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS), @@ -1787,7 +1795,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(redundant_clone::REDUNDANT_CLONE), LintId::of(slow_vector_initialization::SLOW_VECTOR_INITIALIZATION), LintId::of(stable_sort_primitive::STABLE_SORT_PRIMITIVE), - LintId::of(types::BOX_VEC), + LintId::of(types::BOX_COLLECTION), LintId::of(types::REDUNDANT_ALLOCATION), LintId::of(vec::USELESS_VEC), LintId::of(vec_init_then_push::VEC_INIT_THEN_PUSH), @@ -1920,6 +1928,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(move || Box::new(unnested_or_patterns::UnnestedOrPatterns::new(msrv))); store.register_late_pass(|| Box::new(size_of_in_element_count::SizeOfInElementCount)); + store.register_late_pass(|| Box::new(same_name_method::SameNameMethod)); store.register_late_pass(|| Box::new(map_clone::MapClone)); store.register_late_pass(|| Box::new(map_err_ignore::MapErrIgnore)); store.register_late_pass(|| Box::new(shadow::Shadow)); @@ -1952,7 +1961,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(empty_enum::EmptyEnum)); store.register_late_pass(|| Box::new(absurd_extreme_comparisons::AbsurdExtremeComparisons)); store.register_late_pass(|| Box::new(invalid_upcast_comparisons::InvalidUpcastComparisons)); - store.register_late_pass(|| Box::new(regex::Regex::default())); + store.register_late_pass(|| Box::new(regex::Regex)); store.register_late_pass(|| Box::new(copies::CopyAndPaste)); store.register_late_pass(|| Box::new(copy_iterator::CopyIterator)); store.register_late_pass(|| Box::new(format::UselessFormat)); @@ -1976,7 +1985,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(missing_doc::MissingDoc::new())); store.register_late_pass(|| Box::new(missing_inline::MissingInline)); store.register_late_pass(move || Box::new(exhaustive_items::ExhaustiveItems)); - store.register_late_pass(|| Box::new(if_let_some_result::OkIfLet)); + store.register_late_pass(|| Box::new(match_result_ok::MatchResultOk)); store.register_late_pass(|| Box::new(partialeq_ne_impl::PartialEqNeImpl)); store.register_late_pass(|| Box::new(unused_io_amount::UnusedIoAmount)); let enum_variant_size_threshold = conf.enum_variant_size_threshold; @@ -2105,8 +2114,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(float_equality_without_abs::FloatEqualityWithoutAbs)); store.register_late_pass(|| Box::new(semicolon_if_nothing_returned::SemicolonIfNothingReturned)); store.register_late_pass(|| Box::new(async_yields_async::AsyncYieldsAsync)); - let disallowed_methods = conf.disallowed_methods.iter().cloned().collect::<FxHashSet<_>>(); - store.register_late_pass(move || Box::new(disallowed_method::DisallowedMethod::new(&disallowed_methods))); + let disallowed_methods = conf.disallowed_methods.clone(); + store.register_late_pass(move || Box::new(disallowed_method::DisallowedMethod::new(disallowed_methods.clone()))); store.register_early_pass(|| Box::new(asm_syntax::InlineAsmX86AttSyntax)); store.register_early_pass(|| Box::new(asm_syntax::InlineAsmX86IntelSyntax)); store.register_late_pass(|| Box::new(undropped_manually_drops::UndroppedManuallyDrops)); @@ -2131,6 +2140,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(strlen_on_c_strings::StrlenOnCStrings)); store.register_late_pass(move || Box::new(self_named_constructors::SelfNamedConstructors)); store.register_late_pass(move || Box::new(feature_name::FeatureName)); + store.register_late_pass(move || Box::new(iter_not_returning_iterator::IterNotReturningIterator)); + store.register_late_pass(move || Box::new(if_then_panic::IfThenPanic)); } #[rustfmt::skip] @@ -2186,6 +2197,7 @@ pub fn register_renamed(ls: &mut rustc_lint::LintStore) { ls.register_renamed("clippy::cyclomatic_complexity", "clippy::cognitive_complexity"); ls.register_renamed("clippy::const_static_lifetime", "clippy::redundant_static_lifetimes"); ls.register_renamed("clippy::option_and_then_some", "clippy::bind_instead_of_map"); + ls.register_renamed("clippy::box_vec", "clippy::box_collection"); ls.register_renamed("clippy::block_in_if_condition_expr", "clippy::blocks_in_if_conditions"); ls.register_renamed("clippy::block_in_if_condition_stmt", "clippy::blocks_in_if_conditions"); ls.register_renamed("clippy::option_map_unwrap_or", "clippy::map_unwrap_or"); @@ -2200,6 +2212,7 @@ pub fn register_renamed(ls: &mut rustc_lint::LintStore) { ls.register_renamed("clippy::identity_conversion", "clippy::useless_conversion"); ls.register_renamed("clippy::zero_width_space", "clippy::invisible_characters"); ls.register_renamed("clippy::single_char_push_str", "clippy::single_char_add_str"); + ls.register_renamed("clippy::if_let_some_result", "clippy::match_result_ok"); // uplifted lints ls.register_renamed("clippy::invalid_ref", "invalid_value"); diff --git a/clippy_lints/src/loops/for_kv_map.rs b/clippy_lints/src/loops/for_kv_map.rs index 12ffe7a1364..dd60e460d21 100644 --- a/clippy_lints/src/loops/for_kv_map.rs +++ b/clippy_lints/src/loops/for_kv_map.rs @@ -10,12 +10,7 @@ use rustc_middle::ty; use rustc_span::sym; /// Checks for the `FOR_KV_MAP` lint. -pub(super) fn check<'tcx>( - cx: &LateContext<'tcx>, - pat: &'tcx Pat<'_>, - arg: &'tcx Expr<'_>, - body: &'tcx Expr<'_>, -) { +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, arg: &'tcx Expr<'_>, body: &'tcx Expr<'_>) { let pat_span = pat.span; if let PatKind::Tuple(pat, _) = pat.kind { diff --git a/clippy_lints/src/loops/while_let_loop.rs b/clippy_lints/src/loops/while_let_loop.rs index 1848f5b5de2..4dcd5c87722 100644 --- a/clippy_lints/src/loops/while_let_loop.rs +++ b/clippy_lints/src/loops/while_let_loop.rs @@ -65,7 +65,7 @@ fn extract_first_expr<'tcx>(block: &Block<'tcx>) -> Option<&'tcx Expr<'tcx>> { fn is_simple_break_expr(expr: &Expr<'_>) -> bool { match expr.kind { ExprKind::Break(dest, ref passed_expr) if dest.label.is_none() && passed_expr.is_none() => true, - ExprKind::Block(b, _) => extract_first_expr(b).map_or(false, |subexpr| is_simple_break_expr(subexpr)), + ExprKind::Block(b, _) => extract_first_expr(b).map_or(false, is_simple_break_expr), _ => false, } } diff --git a/clippy_lints/src/loops/while_let_on_iterator.rs b/clippy_lints/src/loops/while_let_on_iterator.rs index 79527e3bfa9..b390476a664 100644 --- a/clippy_lints/src/loops/while_let_on_iterator.rs +++ b/clippy_lints/src/loops/while_let_on_iterator.rs @@ -8,7 +8,7 @@ use clippy_utils::{ use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor}; -use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, Mutability, PatKind, QPath, UnOp}; +use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, PatKind, QPath, UnOp}; use rustc_lint::LateContext; use rustc_span::{symbol::sym, Span, Symbol}; @@ -47,13 +47,8 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // If the iterator is a field or the iterator is accessed after the loop is complete it needs to be // borrowed mutably. TODO: If the struct can be partially moved from and the struct isn't used // afterwards a mutable borrow of a field isn't necessary. - let ref_mut = if !iter_expr.fields.is_empty() || needs_mutable_borrow(cx, &iter_expr, loop_expr) { - if cx.typeck_results().node_type(iter_expr.hir_id).ref_mutability() == Some(Mutability::Mut) { - // Reborrow for mutable references. It may not be possible to get a mutable reference here. - "&mut *" - } else { - "&mut " - } + let by_ref = if !iter_expr.fields.is_empty() || needs_mutable_borrow(cx, &iter_expr, loop_expr) { + ".by_ref()" } else { "" }; @@ -65,7 +60,7 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { expr.span.with_hi(scrutinee_expr.span.hi()), "this loop could be written as a `for` loop", "try", - format!("for {} in {}{}", loop_var, ref_mut, iterator), + format!("for {} in {}{}", loop_var, iterator, by_ref), applicability, ); } @@ -74,8 +69,6 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { struct IterExpr { /// The span of the whole expression, not just the path and fields stored here. span: Span, - /// The HIR id of the whole expression, not just the path and fields stored here. - hir_id: HirId, /// The fields used, in order of child to parent. fields: Vec<Symbol>, /// The path being used. @@ -86,14 +79,12 @@ struct IterExpr { /// the expression might have side effects. fn try_parse_iter_expr(cx: &LateContext<'_>, mut e: &Expr<'_>) -> Option<IterExpr> { let span = e.span; - let hir_id = e.hir_id; let mut fields = Vec::new(); loop { match e.kind { ExprKind::Path(ref path) => { break Some(IterExpr { span, - hir_id, fields, path: cx.qpath_res(path, e.hir_id), }); diff --git a/clippy_lints/src/macro_use.rs b/clippy_lints/src/macro_use.rs index 41e6ad12d05..aff6b3853a4 100644 --- a/clippy_lints/src/macro_use.rs +++ b/clippy_lints/src/macro_use.rs @@ -63,29 +63,24 @@ impl MacroUseImports { fn push_unique_macro(&mut self, cx: &LateContext<'_>, span: Span) { let call_site = span.source_callsite(); let name = snippet(cx, cx.sess().source_map().span_until_char(call_site, '!'), "_"); - if let Some(_callee) = span.source_callee() { - if !self.collected.contains(&call_site) { - let name = if name.contains("::") { - name.split("::").last().unwrap().to_string() - } else { - name.to_string() - }; + if span.source_callee().is_some() && !self.collected.contains(&call_site) { + let name = if name.contains("::") { + name.split("::").last().unwrap().to_string() + } else { + name.to_string() + }; - self.mac_refs.push(MacroRefData::new(name)); - self.collected.insert(call_site); - } + self.mac_refs.push(MacroRefData::new(name)); + self.collected.insert(call_site); } } fn push_unique_macro_pat_ty(&mut self, cx: &LateContext<'_>, span: Span) { let call_site = span.source_callsite(); let name = snippet(cx, cx.sess().source_map().span_until_char(call_site, '!'), "_"); - if let Some(_callee) = span.source_callee() { - if !self.collected.contains(&call_site) { - self.mac_refs - .push(MacroRefData::new(name.to_string())); - self.collected.insert(call_site); - } + if span.source_callee().is_some() && !self.collected.contains(&call_site) { + self.mac_refs.push(MacroRefData::new(name.to_string())); + self.collected.insert(call_site); } } } diff --git a/clippy_lints/src/if_let_some_result.rs b/clippy_lints/src/match_result_ok.rs similarity index 66% rename from clippy_lints/src/if_let_some_result.rs rename to clippy_lints/src/match_result_ok.rs index adcd78ed0d4..c7de06f0815 100644 --- a/clippy_lints/src/if_let_some_result.rs +++ b/clippy_lints/src/match_result_ok.rs @@ -12,40 +12,51 @@ use rustc_span::sym; declare_clippy_lint! { /// ### What it does - ///* Checks for unnecessary `ok()` in if let. + /// Checks for unnecessary `ok()` in `while let`. /// /// ### Why is this bad? - /// Calling `ok()` in if let is unnecessary, instead match + /// Calling `ok()` in `while let` is unnecessary, instead match /// on `Ok(pat)` /// /// ### Example /// ```ignore - /// for i in iter { - /// if let Some(value) = i.parse().ok() { - /// vec.push(value) - /// } + /// while let Some(value) = iter.next().ok() { + /// vec.push(value) /// } - /// ``` - /// Could be written: /// - /// ```ignore - /// for i in iter { - /// if let Ok(value) = i.parse() { - /// vec.push(value) - /// } + /// if let Some(valie) = iter.next().ok() { + /// vec.push(value) /// } /// ``` - pub IF_LET_SOME_RESULT, + /// Use instead: + /// ```ignore + /// while let Ok(value) = iter.next() { + /// vec.push(value) + /// } + /// + /// if let Ok(value) = iter.next() { + /// vec.push_value) + /// } + /// ``` + pub MATCH_RESULT_OK, style, - "usage of `ok()` in `if let Some(pat)` statements is unnecessary, match on `Ok(pat)` instead" + "usage of `ok()` in `let Some(pat)` statements is unnecessary, match on `Ok(pat)` instead" } -declare_lint_pass!(OkIfLet => [IF_LET_SOME_RESULT]); +declare_lint_pass!(MatchResultOk => [MATCH_RESULT_OK]); -impl<'tcx> LateLintPass<'tcx> for OkIfLet { +impl<'tcx> LateLintPass<'tcx> for MatchResultOk { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if_chain! { //begin checking variables - if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(cx, expr); + let (let_pat, let_expr, ifwhile) = + if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(cx, expr) { + (let_pat, let_expr, "if") + } else if let Some(higher::WhileLet { let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) { + (let_pat, let_expr, "while") + } else { + return; + }; + + if_chain! { if let ExprKind::MethodCall(_, ok_span, [ref result_types_0, ..], _) = let_expr.kind; //check is expr.ok() has type Result<T,E>.ok(, _) if let PatKind::TupleStruct(QPath::Resolved(_, x), y, _) = let_pat.kind; //get operation if method_chain_args(let_expr, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized; @@ -53,17 +64,19 @@ impl<'tcx> LateLintPass<'tcx> for OkIfLet { if rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_path(x, false)) == "Some"; then { + let mut applicability = Applicability::MachineApplicable; let some_expr_string = snippet_with_applicability(cx, y[0].span, "", &mut applicability); let trimmed_ok = snippet_with_applicability(cx, let_expr.span.until(ok_span), "", &mut applicability); let sugg = format!( - "if let Ok({}) = {}", + "{} let Ok({}) = {}", + ifwhile, some_expr_string, trimmed_ok.trim().trim_end_matches('.'), ); span_lint_and_sugg( cx, - IF_LET_SOME_RESULT, + MATCH_RESULT_OK, expr.span.with_hi(let_expr.span.hi()), "matching on `Some` with `ok()` is redundant", &format!("consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string), diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 2f1ff567e84..d878fbc35fd 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -183,8 +183,8 @@ declare_clippy_lint! { /// ```rust /// let x = 5; /// match x { - /// 1...10 => println!("1 ... 10"), - /// 5...15 => println!("5 ... 15"), + /// 1..=10 => println!("1 ... 10"), + /// 5..=15 => println!("5 ... 15"), /// _ => (), /// } /// ``` diff --git a/clippy_lints/src/methods/manual_split_once.rs b/clippy_lints/src/methods/manual_split_once.rs index 00167650324..55688677e1d 100644 --- a/clippy_lints/src/methods/manual_split_once.rs +++ b/clippy_lints/src/methods/manual_split_once.rs @@ -17,32 +17,25 @@ pub(super) fn check(cx: &LateContext<'_>, method_name: &str, expr: &Expr<'_>, se } let ctxt = expr.span.ctxt(); - let usage = match parse_iter_usage(cx, ctxt, cx.tcx.hir().parent_iter(expr.hir_id)) { + let (method_name, msg, reverse) = if method_name == "splitn" { + ("split_once", "manual implementation of `split_once`", false) + } else { + ("rsplit_once", "manual implementation of `rsplit_once`", true) + }; + let usage = match parse_iter_usage(cx, ctxt, cx.tcx.hir().parent_iter(expr.hir_id), reverse) { Some(x) => x, None => return, }; - let (method_name, msg) = if method_name == "splitn" { - ("split_once", "manual implementation of `split_once`") - } else { - ("rsplit_once", "manual implementation of `rsplit_once`") - }; let mut app = Applicability::MachineApplicable; let self_snip = snippet_with_context(cx, self_arg.span, ctxt, "..", &mut app).0; let pat_snip = snippet_with_context(cx, pat_arg.span, ctxt, "..", &mut app).0; - match usage.kind { + let sugg = match usage.kind { IterUsageKind::NextTuple => { - span_lint_and_sugg( - cx, - MANUAL_SPLIT_ONCE, - usage.span, - msg, - "try this", - format!("{}.{}({})", self_snip, method_name, pat_snip), - app, - ); + format!("{}.{}({})", self_snip, method_name, pat_snip) }, + IterUsageKind::RNextTuple => format!("{}.{}({}).map(|(x, y)| (y, x))", self_snip, method_name, pat_snip), IterUsageKind::Next => { let self_deref = { let adjust = cx.typeck_results().expr_adjustments(self_arg); @@ -58,7 +51,7 @@ pub(super) fn check(cx: &LateContext<'_>, method_name: &str, expr: &Expr<'_>, se "*".repeat(adjust.len() - 2) } }; - let sugg = if usage.unwrap_kind.is_some() { + if usage.unwrap_kind.is_some() { format!( "{}.{}({}).map_or({}{}, |x| x.0)", &self_snip, method_name, pat_snip, self_deref, &self_snip @@ -68,9 +61,7 @@ pub(super) fn check(cx: &LateContext<'_>, method_name: &str, expr: &Expr<'_>, se "Some({}.{}({}).map_or({}{}, |x| x.0))", &self_snip, method_name, pat_snip, self_deref, &self_snip ) - }; - - span_lint_and_sugg(cx, MANUAL_SPLIT_ONCE, usage.span, msg, "try this", sugg, app); + } }, IterUsageKind::Second => { let access_str = match usage.unwrap_kind { @@ -78,23 +69,18 @@ pub(super) fn check(cx: &LateContext<'_>, method_name: &str, expr: &Expr<'_>, se Some(UnwrapKind::QuestionMark) => "?.1", None => ".map(|x| x.1)", }; - span_lint_and_sugg( - cx, - MANUAL_SPLIT_ONCE, - usage.span, - msg, - "try this", - format!("{}.{}({}){}", self_snip, method_name, pat_snip, access_str), - app, - ); + format!("{}.{}({}){}", self_snip, method_name, pat_snip, access_str) }, - } + }; + + span_lint_and_sugg(cx, MANUAL_SPLIT_ONCE, usage.span, msg, "try this", sugg, app); } enum IterUsageKind { Next, Second, NextTuple, + RNextTuple, } enum UnwrapKind { @@ -108,10 +94,12 @@ struct IterUsage { span: Span, } +#[allow(clippy::too_many_lines)] fn parse_iter_usage( cx: &LateContext<'tcx>, ctxt: SyntaxContext, mut iter: impl Iterator<Item = (HirId, Node<'tcx>)>, + reverse: bool, ) -> Option<IterUsage> { let (kind, span) = match iter.next() { Some((_, Node::Expr(e))) if e.span.ctxt() == ctxt => { @@ -124,20 +112,30 @@ fn parse_iter_usage( let iter_id = cx.tcx.get_diagnostic_item(sym::Iterator)?; match (&*name.ident.as_str(), args) { - ("next", []) if cx.tcx.trait_of_item(did) == Some(iter_id) => (IterUsageKind::Next, e.span), + ("next", []) if cx.tcx.trait_of_item(did) == Some(iter_id) => { + if reverse { + (IterUsageKind::Second, e.span) + } else { + (IterUsageKind::Next, e.span) + } + }, ("next_tuple", []) => { - if_chain! { + return if_chain! { if match_def_path(cx, did, &paths::ITERTOOLS_NEXT_TUPLE); if let ty::Adt(adt_def, subs) = cx.typeck_results().expr_ty(e).kind(); if cx.tcx.is_diagnostic_item(sym::option_type, adt_def.did); if let ty::Tuple(subs) = subs.type_at(0).kind(); if subs.len() == 2; then { - return Some(IterUsage { kind: IterUsageKind::NextTuple, span: e.span, unwrap_kind: None }); + Some(IterUsage { + kind: if reverse { IterUsageKind::RNextTuple } else { IterUsageKind::NextTuple }, + span: e.span, + unwrap_kind: None + }) } else { - return None; + None } - } + }; }, ("nth" | "skip", [idx_expr]) if cx.tcx.trait_of_item(did) == Some(iter_id) => { if let Some((Constant::Int(idx), _)) = constant(cx, cx.typeck_results(), idx_expr) { @@ -158,7 +156,7 @@ fn parse_iter_usage( } } }; - match idx { + match if reverse { idx ^ 1 } else { idx } { 0 => (IterUsageKind::Next, span), 1 => (IterUsageKind::Second, span), _ => return None, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 8a699f13f2e..2025056ac94 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -264,6 +264,8 @@ declare_clippy_lint! { /// The method signature is controlled by the trait and often `&self` is required for all types that implement the trait /// (see e.g. the `std::string::ToString` trait). /// + /// Clippy allows `Pin<&Self>` and `Pin<&mut Self>` if `&self` and `&mut self` is required. + /// /// Please find more info here: /// https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv /// diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 538fa4e1678..0f32cd9164e 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -113,7 +113,7 @@ declare_clippy_lint! { /// if (y - x).abs() > error_margin { } /// ``` pub FLOAT_CMP, - correctness, + pedantic, "using `==` or `!=` on float values instead of comparing difference with an epsilon" } diff --git a/clippy_lints/src/mut_key.rs b/clippy_lints/src/mut_key.rs index 2c7681c45a4..cb17e4dbfd0 100644 --- a/clippy_lints/src/mut_key.rs +++ b/clippy_lints/src/mut_key.rs @@ -3,7 +3,7 @@ use clippy_utils::trait_ref_of_method; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::TypeFoldable; -use rustc_middle::ty::{Adt, Array, RawPtr, Ref, Slice, Tuple, Ty, TypeAndMut}; +use rustc_middle::ty::{Adt, Array, Ref, Slice, Tuple, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; use rustc_span::symbol::sym; @@ -19,10 +19,29 @@ declare_clippy_lint! { /// so having types with interior mutability is a bad idea. /// /// ### Known problems - /// It's correct to use a struct, that contains interior mutability - /// as a key, when its `Hash` implementation doesn't access any of the interior mutable types. - /// However, this lint is unable to recognize this, so it causes a false positive in theses cases. - /// The `bytes` crate is a great example of this. + /// + /// #### False Positives + /// It's correct to use a struct that contains interior mutability as a key, when its + /// implementation of `Hash` or `Ord` doesn't access any of the interior mutable types. + /// However, this lint is unable to recognize this, so it will often cause false positives in + /// theses cases. The `bytes` crate is a great example of this. + /// + /// #### False Negatives + /// For custom `struct`s/`enum`s, this lint is unable to check for interior mutability behind + /// indirection. For example, `struct BadKey<'a>(&'a Cell<usize>)` will be seen as immutable + /// and cause a false negative if its implementation of `Hash`/`Ord` accesses the `Cell`. + /// + /// This lint does check a few cases for indirection. Firstly, using some standard library + /// types (`Option`, `Result`, `Box`, `Rc`, `Arc`, `Vec`, `VecDeque`, `BTreeMap` and + /// `BTreeSet`) directly as keys (e.g. in `HashMap<Box<Cell<usize>>, ()>`) **will** trigger the + /// lint, because the impls of `Hash`/`Ord` for these types directly call `Hash`/`Ord` on their + /// contained type. + /// + /// Secondly, the implementations of `Hash` and `Ord` for raw pointers (`*const T` or `*mut T`) + /// apply only to the **address** of the contained value. Therefore, interior mutability + /// behind raw pointers (e.g. in `HashSet<*mut Cell<usize>>`) can't impact the value of `Hash` + /// or `Ord`, and therefore will not trigger this link. For more info, see issue + /// [#6745](https://github.com/rust-lang/rust-clippy/issues/6745). /// /// ### Example /// ```rust @@ -103,30 +122,52 @@ fn check_sig<'tcx>(cx: &LateContext<'tcx>, item_hir_id: hir::HirId, decl: &hir:: fn check_ty<'tcx>(cx: &LateContext<'tcx>, span: Span, ty: Ty<'tcx>) { let ty = ty.peel_refs(); if let Adt(def, substs) = ty.kind() { - if [sym::hashmap_type, sym::BTreeMap, sym::hashset_type, sym::BTreeMap] + let is_keyed_type = [sym::hashmap_type, sym::BTreeMap, sym::hashset_type, sym::BTreeSet] .iter() - .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def.did)) - && is_mutable_type(cx, substs.type_at(0), span) - { + .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def.did)); + if is_keyed_type && is_interior_mutable_type(cx, substs.type_at(0), span) { span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type"); } } } -fn is_mutable_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, span: Span) -> bool { +/// Determines if a type contains interior mutability which would affect its implementation of +/// [`Hash`] or [`Ord`]. +fn is_interior_mutable_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, span: Span) -> bool { match *ty.kind() { - RawPtr(TypeAndMut { ty: inner_ty, mutbl }) | Ref(_, inner_ty, mutbl) => { - mutbl == hir::Mutability::Mut || is_mutable_type(cx, inner_ty, span) - }, - Slice(inner_ty) => is_mutable_type(cx, inner_ty, span), + Ref(_, inner_ty, mutbl) => mutbl == hir::Mutability::Mut || is_interior_mutable_type(cx, inner_ty, span), + Slice(inner_ty) => is_interior_mutable_type(cx, inner_ty, span), Array(inner_ty, size) => { - size.try_eval_usize(cx.tcx, cx.param_env).map_or(true, |u| u != 0) && is_mutable_type(cx, inner_ty, span) + size.try_eval_usize(cx.tcx, cx.param_env).map_or(true, |u| u != 0) + && is_interior_mutable_type(cx, inner_ty, span) }, - Tuple(..) => ty.tuple_fields().any(|ty| is_mutable_type(cx, ty, span)), - Adt(..) => { - !ty.has_escaping_bound_vars() - && cx.tcx.layout_of(cx.param_env.and(ty)).is_ok() - && !ty.is_freeze(cx.tcx.at(span), cx.param_env) + Tuple(..) => ty.tuple_fields().any(|ty| is_interior_mutable_type(cx, ty, span)), + Adt(def, substs) => { + // Special case for collections in `std` who's impl of `Hash` or `Ord` delegates to + // that of their type parameters. Note: we don't include `HashSet` and `HashMap` + // because they have no impl for `Hash` or `Ord`. + let is_std_collection = [ + sym::option_type, + sym::result_type, + sym::LinkedList, + sym::vec_type, + sym::vecdeque_type, + sym::BTreeMap, + sym::BTreeSet, + sym::Rc, + sym::Arc, + ] + .iter() + .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def.did)); + let is_box = Some(def.did) == cx.tcx.lang_items().owned_box(); + if is_std_collection || is_box { + // The type is mutable if any of its type parameters are + substs.types().any(|ty| is_interior_mutable_type(cx, ty, span)) + } else { + !ty.has_escaping_bound_vars() + && cx.tcx.layout_of(cx.param_env.and(ty)).is_ok() + && !ty.is_freeze(cx.tcx.at(span), cx.param_env) + } }, _ => false, } diff --git a/clippy_lints/src/needless_borrow.rs b/clippy_lints/src/needless_borrow.rs index ba8f9446af8..1b2495d764d 100644 --- a/clippy_lints/src/needless_borrow.rs +++ b/clippy_lints/src/needless_borrow.rs @@ -104,7 +104,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow { if e.span.from_expansion() { return; } - if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, inner) = e.kind { + if let ExprKind::AddrOf(BorrowKind::Ref, mutability, inner) = e.kind { if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty(inner).kind() { for adj3 in cx.typeck_results().expr_adjustments(e).windows(3) { if let [Adjustment { @@ -116,14 +116,20 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow { .. }] = *adj3 { + let help_msg_ty = if matches!(mutability, Mutability::Not) { + format!("&{}", ty) + } else { + format!("&mut {}", ty) + }; + span_lint_and_then( cx, NEEDLESS_BORROW, e.span, &format!( - "this expression borrows a reference (`&{}`) that is immediately dereferenced \ + "this expression borrows a reference (`{}`) that is immediately dereferenced \ by the compiler", - ty + help_msg_ty ), |diag| { if let Some(snippet) = snippet_opt(cx, inner.span) { diff --git a/clippy_lints/src/non_expressive_names.rs b/clippy_lints/src/non_expressive_names.rs index 2ffc00b449d..5b254bc8133 100644 --- a/clippy_lints/src/non_expressive_names.rs +++ b/clippy_lints/src/non_expressive_names.rs @@ -43,7 +43,7 @@ declare_clippy_lint! { /// let (a, b, c, d, e, f, g) = (...); /// ``` pub MANY_SINGLE_CHAR_NAMES, - style, + pedantic, "too many single character bindings" } diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index c0d1f1eb6e6..d696e17d656 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -372,7 +372,7 @@ fn check_fn(cx: &LateContext<'_>, decl: &FnDecl<'_>, fn_id: HirId, opt_body_id: for (_, ref mutbl, ref argspan) in decl .inputs .iter() - .filter_map(|ty| get_rptr_lm(ty)) + .filter_map(get_rptr_lm) .filter(|&(lt, _, _)| lt.name == out.name) { if *mutbl == Mutability::Mut { diff --git a/clippy_lints/src/regex.rs b/clippy_lints/src/regex.rs index 947c25ac880..5d08aee1e5f 100644 --- a/clippy_lints/src/regex.rs +++ b/clippy_lints/src/regex.rs @@ -5,7 +5,7 @@ use if_chain::if_chain; use rustc_ast::ast::{LitKind, StrStyle}; use rustc_hir::{BorrowKind, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::{BytePos, Span}; use std::convert::TryFrom; @@ -51,10 +51,7 @@ declare_clippy_lint! { "trivial regular expressions" } -#[derive(Clone, Default)] -pub struct Regex {} - -impl_lint_pass!(Regex => [INVALID_REGEX, TRIVIAL_REGEX]); +declare_lint_pass!(Regex => [INVALID_REGEX, TRIVIAL_REGEX]); impl<'tcx> LateLintPass<'tcx> for Regex { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 341b5a61631..ae85b7087e7 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -11,6 +11,7 @@ use rustc_middle::hir::map::Map; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::subst::GenericArgKind; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::hygiene::DesugaringKind; use rustc_span::source_map::Span; use rustc_span::sym; @@ -199,7 +200,9 @@ fn check_final_expr<'tcx>( check_block_return(cx, ifblock); } if let Some(else_clause) = else_clause_opt { - check_final_expr(cx, else_clause, None, RetReplacement::Empty); + if expr.span.desugaring_kind() != Some(DesugaringKind::LetElse) { + check_final_expr(cx, else_clause, None, RetReplacement::Empty); + } } }, // a match expr, check all arms diff --git a/clippy_lints/src/same_name_method.rs b/clippy_lints/src/same_name_method.rs new file mode 100644 index 00000000000..014898e6dab --- /dev/null +++ b/clippy_lints/src/same_name_method.rs @@ -0,0 +1,160 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use rustc_data_structures::fx::FxHashMap; +use rustc_hir::def::{DefKind, Res}; +use rustc_hir::{Crate, Impl, ItemKind, Node, Path, QPath, TraitRef, TyKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::AssocKind; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::Symbol; +use rustc_span::Span; +use std::collections::{BTreeMap, BTreeSet}; + +declare_clippy_lint! { + /// ### What it does + /// It lints if a struct has two method with same time: + /// one from a trait, another not from trait. + /// + /// ### Why is this bad? + /// Confusing. + /// + /// ### Example + /// ```rust + /// trait T { + /// fn foo(&self) {} + /// } + /// + /// struct S; + /// + /// impl T for S { + /// fn foo(&self) {} + /// } + /// + /// impl S { + /// fn foo(&self) {} + /// } + /// ``` + pub SAME_NAME_METHOD, + restriction, + "two method with same name" +} + +declare_lint_pass!(SameNameMethod => [SAME_NAME_METHOD]); + +struct ExistingName { + impl_methods: BTreeMap<Symbol, Span>, + trait_methods: BTreeMap<Symbol, Vec<Span>>, +} + +impl<'tcx> LateLintPass<'tcx> for SameNameMethod { + fn check_crate_post(&mut self, cx: &LateContext<'tcx>, krate: &'tcx Crate<'tcx>) { + let mut map = FxHashMap::<Res, ExistingName>::default(); + + for item in krate.items() { + if let ItemKind::Impl(Impl { + items, + of_trait, + self_ty, + .. + }) = &item.kind + { + if let TyKind::Path(QPath::Resolved(_, Path { res, .. })) = self_ty.kind { + if !map.contains_key(res) { + map.insert( + *res, + ExistingName { + impl_methods: BTreeMap::new(), + trait_methods: BTreeMap::new(), + }, + ); + } + let existing_name = map.get_mut(res).unwrap(); + + match of_trait { + Some(trait_ref) => { + let mut methods_in_trait: BTreeSet<Symbol> = if_chain! { + if let Some(Node::TraitRef(TraitRef { path, .. })) = + cx.tcx.hir().find(trait_ref.hir_ref_id); + if let Res::Def(DefKind::Trait, did) = path.res; + then{ + // FIXME: if + // `rustc_middle::ty::assoc::AssocItems::items` is public, + // we can iterate its keys instead of `in_definition_order`, + // which's more efficient + cx.tcx + .associated_items(did) + .in_definition_order() + .filter(|assoc_item| { + matches!(assoc_item.kind, AssocKind::Fn) + }) + .map(|assoc_item| assoc_item.ident.name) + .collect() + }else{ + BTreeSet::new() + } + }; + + let mut check_trait_method = |method_name: Symbol, trait_method_span: Span| { + if let Some(impl_span) = existing_name.impl_methods.get(&method_name) { + span_lint_and_then( + cx, + SAME_NAME_METHOD, + *impl_span, + "method's name is same to an existing method in a trait", + |diag| { + diag.span_note( + trait_method_span, + &format!("existing `{}` defined here", method_name), + ); + }, + ); + } + if let Some(v) = existing_name.trait_methods.get_mut(&method_name) { + v.push(trait_method_span); + } else { + existing_name.trait_methods.insert(method_name, vec![trait_method_span]); + } + }; + + for impl_item_ref in (*items).iter().filter(|impl_item_ref| { + matches!(impl_item_ref.kind, rustc_hir::AssocItemKind::Fn { .. }) + }) { + let method_name = impl_item_ref.ident.name; + methods_in_trait.remove(&method_name); + check_trait_method(method_name, impl_item_ref.span); + } + + for method_name in methods_in_trait { + check_trait_method(method_name, item.span); + } + }, + None => { + for impl_item_ref in (*items).iter().filter(|impl_item_ref| { + matches!(impl_item_ref.kind, rustc_hir::AssocItemKind::Fn { .. }) + }) { + let method_name = impl_item_ref.ident.name; + let impl_span = impl_item_ref.span; + if let Some(trait_spans) = existing_name.trait_methods.get(&method_name) { + span_lint_and_then( + cx, + SAME_NAME_METHOD, + impl_span, + "method's name is same to an existing method in a trait", + |diag| { + // TODO should we `span_note` on every trait? + // iterate on trait_spans? + diag.span_note( + trait_spans[0], + &format!("existing `{}` defined here", method_name), + ); + }, + ); + } + existing_name.impl_methods.insert(method_name, impl_span); + } + }, + } + } + } + } + } +} diff --git a/clippy_lints/src/types/box_collection.rs b/clippy_lints/src/types/box_collection.rs new file mode 100644 index 00000000000..b28da29c91c --- /dev/null +++ b/clippy_lints/src/types/box_collection.rs @@ -0,0 +1,50 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::is_ty_param_diagnostic_item; +use rustc_hir::{self as hir, def_id::DefId, QPath}; +use rustc_lint::LateContext; +use rustc_span::symbol::sym; + +use super::BOX_COLLECTION; + +pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_>, def_id: DefId) -> bool { + if_chain! { + if Some(def_id) == cx.tcx.lang_items().owned_box(); + if let Some(item_type) = get_std_collection(cx, qpath); + then { + let generic = if item_type == "String" { + "" + } else { + "<..>" + }; + span_lint_and_help( + cx, + BOX_COLLECTION, + hir_ty.span, + &format!( + "you seem to be trying to use `Box<{outer}{generic}>`. Consider using just `{outer}{generic}`", + outer=item_type, + generic = generic), + None, + &format!( + "`{outer}{generic}` is already on the heap, `Box<{outer}{generic}>` makes an extra allocation", + outer=item_type, + generic = generic) + ); + true + } else { + false + } + } +} + +fn get_std_collection(cx: &LateContext<'_>, qpath: &QPath<'_>) -> Option<&'static str> { + if is_ty_param_diagnostic_item(cx, qpath, sym::vec_type).is_some() { + Some("Vec") + } else if is_ty_param_diagnostic_item(cx, qpath, sym::string_type).is_some() { + Some("String") + } else if is_ty_param_diagnostic_item(cx, qpath, sym::hashmap_type).is_some() { + Some("HashMap") + } else { + None + } +} diff --git a/clippy_lints/src/types/box_vec.rs b/clippy_lints/src/types/box_vec.rs deleted file mode 100644 index d8b1953457c..00000000000 --- a/clippy_lints/src/types/box_vec.rs +++ /dev/null @@ -1,25 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::is_ty_param_diagnostic_item; -use rustc_hir::{self as hir, def_id::DefId, QPath}; -use rustc_lint::LateContext; -use rustc_span::symbol::sym; - -use super::BOX_VEC; - -pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_>, def_id: DefId) -> bool { - if Some(def_id) == cx.tcx.lang_items().owned_box() - && is_ty_param_diagnostic_item(cx, qpath, sym::vec_type).is_some() - { - span_lint_and_help( - cx, - BOX_VEC, - hir_ty.span, - "you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>`", - None, - "`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation", - ); - true - } else { - false - } -} diff --git a/clippy_lints/src/types/mod.rs b/clippy_lints/src/types/mod.rs index 9588de8459c..bbe07db5358 100644 --- a/clippy_lints/src/types/mod.rs +++ b/clippy_lints/src/types/mod.rs @@ -1,5 +1,5 @@ mod borrowed_box; -mod box_vec; +mod box_collection; mod linked_list; mod option_option; mod rc_buffer; @@ -21,12 +21,12 @@ use rustc_span::source_map::Span; declare_clippy_lint! { /// ### What it does - /// Checks for use of `Box<Vec<_>>` anywhere in the code. + /// Checks for use of `Box<T>` where T is a collection such as Vec anywhere in the code. /// Check the [Box documentation](https://doc.rust-lang.org/std/boxed/index.html) for more information. /// /// ### Why is this bad? - /// `Vec` already keeps its contents in a separate area on - /// the heap. So if you `Box` it, you just add another level of indirection + /// Collections already keeps their contents in a separate area on + /// the heap. So if you `Box` them, you just add another level of indirection /// without any benefit whatsoever. /// /// ### Example @@ -43,7 +43,7 @@ declare_clippy_lint! { /// values: Vec<Foo>, /// } /// ``` - pub BOX_VEC, + pub BOX_COLLECTION, perf, "usage of `Box<Vec<T>>`, vector elements are already on the heap" } @@ -298,7 +298,7 @@ pub struct Types { avoid_breaking_exported_api: bool, } -impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, RC_MUTEX, TYPE_COMPLEXITY]); +impl_lint_pass!(Types => [BOX_COLLECTION, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, RC_MUTEX, TYPE_COMPLEXITY]); impl<'tcx> LateLintPass<'tcx> for Types { fn check_fn(&mut self, cx: &LateContext<'_>, _: FnKind<'_>, decl: &FnDecl<'_>, _: &Body<'_>, _: Span, id: HirId) { @@ -447,7 +447,7 @@ impl Types { // in `clippy_lints::utils::conf.rs` let mut triggered = false; - triggered |= box_vec::check(cx, hir_ty, qpath, def_id); + triggered |= box_collection::check(cx, hir_ty, qpath, def_id); triggered |= redundant_allocation::check(cx, hir_ty, qpath, def_id); triggered |= rc_buffer::check(cx, hir_ty, qpath, def_id); triggered |= vec_box::check(cx, hir_ty, qpath, def_id, self.vec_box_size_threshold); diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 8651fa6fcf9..1e0447239be 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -15,6 +15,14 @@ pub struct Rename { pub rename: String, } +/// A single disallowed method, used by the `DISALLOWED_METHOD` lint. +#[derive(Clone, Debug, Deserialize)] +#[serde(untagged)] +pub enum DisallowedMethod { + Simple(String), + WithReason { path: String, reason: Option<String> }, +} + /// Conf with parse errors #[derive(Default)] pub struct TryConf { @@ -128,7 +136,7 @@ macro_rules! define_Conf { } define_Conf! { - /// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION, BOX_VEC, REDUNDANT_ALLOCATION, RC_BUFFER, VEC_BOX, OPTION_OPTION, LINKEDLIST, RC_MUTEX. + /// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION, BOX_COLLECTION, REDUNDANT_ALLOCATION, RC_BUFFER, VEC_BOX, OPTION_OPTION, LINKEDLIST, RC_MUTEX. /// /// Suppress lints whenever the suggested change would cause breakage for other crates. (avoid_breaking_exported_api: bool = true), @@ -243,7 +251,7 @@ define_Conf! { /// Lint: DISALLOWED_METHOD. /// /// The list of disallowed methods, written as fully qualified paths. - (disallowed_methods: Vec<String> = Vec::new()), + (disallowed_methods: Vec<crate::utils::conf::DisallowedMethod> = Vec::new()), /// Lint: DISALLOWED_TYPE. /// /// The list of disallowed types, written as fully qualified paths. diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 756c33d70c2..3e2a4e9748d 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -561,9 +561,7 @@ declare_lint_pass!(ProduceIce => [PRODUCE_ICE]); impl EarlyLintPass for ProduceIce { fn check_fn(&mut self, _: &EarlyContext<'_>, fn_kind: FnKind<'_>, _: Span, _: NodeId) { - if is_trigger_fn(fn_kind) { - panic!("Would you like some help with that?"); - } + assert!(!is_trigger_fn(fn_kind), "Would you like some help with that?"); } } @@ -894,7 +892,7 @@ impl<'tcx> LateLintPass<'tcx> for InvalidPaths { }).collect(); if !check_path(cx, &path[..]); then { - span_lint(cx, CLIPPY_LINTS_INTERNAL, item.span, "invalid path"); + span_lint(cx, INVALID_PATHS, item.span, "invalid path"); } } } @@ -1224,5 +1222,10 @@ fn if_chain_local_span(cx: &LateContext<'_>, local: &Local<'_>, if_chain_span: S let sm = cx.sess().source_map(); let span = sm.span_extend_to_prev_str(span, "let", false); let span = sm.span_extend_to_next_char(span, ';', false); - Span::new(span.lo() - BytePos(3), span.hi() + BytePos(1), span.ctxt()) + Span::new( + span.lo() - BytePos(3), + span.hi() + BytePos(1), + span.ctxt(), + span.parent(), + ) } diff --git a/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/clippy_lints/src/utils/internal_lints/metadata_collector.rs index 188d0419c39..0d27874b7af 100644 --- a/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -298,6 +298,7 @@ pub struct ClippyConfiguration { default: String, lints: Vec<String>, doc: String, + #[allow(dead_code)] deprecation_reason: Option<&'static str>, } diff --git a/clippy_utils/Cargo.toml b/clippy_utils/Cargo.toml index 7c24e830e71..e7fca3ae5d4 100644 --- a/clippy_utils/Cargo.toml +++ b/clippy_utils/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "clippy_utils" version = "0.1.57" -edition = "2018" +edition = "2021" publish = false [dependencies] diff --git a/clippy_utils/src/ast_utils.rs b/clippy_utils/src/ast_utils.rs index 133f6c29f7d..2fa98831c77 100644 --- a/clippy_utils/src/ast_utils.rs +++ b/clippy_utils/src/ast_utils.rs @@ -46,15 +46,12 @@ pub fn eq_pat(l: &Pat, r: &Pat) -> bool { | (Ref(l, Mutability::Not), Ref(r, Mutability::Not)) | (Ref(l, Mutability::Mut), Ref(r, Mutability::Mut)) => eq_pat(l, r), (Tuple(l), Tuple(r)) | (Slice(l), Slice(r)) => over(l, r, |l, r| eq_pat(l, r)), - (Path(lq, lp), Path(rq, rp)) => both(lq, rq, |l, r| eq_qself(l, r)) && eq_path(lp, rp), + (Path(lq, lp), Path(rq, rp)) => both(lq, rq, eq_qself) && eq_path(lp, rp), (TupleStruct(lqself, lp, lfs), TupleStruct(rqself, rp, rfs)) => { eq_maybe_qself(lqself, rqself) && eq_path(lp, rp) && over(lfs, rfs, |l, r| eq_pat(l, r)) }, (Struct(lqself, lp, lfs, lr), Struct(rqself, rp, rfs, rr)) => { - lr == rr - && eq_maybe_qself(lqself, rqself) - && eq_path(lp, rp) - && unordered_over(lfs, rfs, |lf, rf| eq_field_pat(lf, rf)) + lr == rr && eq_maybe_qself(lqself, rqself) && eq_path(lp, rp) && unordered_over(lfs, rfs, eq_field_pat) }, (Or(ls), Or(rs)) => unordered_over(ls, rs, |l, r| eq_pat(l, r)), (MacCall(l), MacCall(r)) => eq_mac_call(l, r), @@ -76,7 +73,7 @@ pub fn eq_field_pat(l: &PatField, r: &PatField) -> bool { l.is_placeholder == r.is_placeholder && eq_id(l.ident, r.ident) && eq_pat(&l.pat, &r.pat) - && over(&l.attrs, &r.attrs, |l, r| eq_attr(l, r)) + && over(&l.attrs, &r.attrs, eq_attr) } pub fn eq_qself(l: &QSelf, r: &QSelf) -> bool { @@ -92,7 +89,7 @@ pub fn eq_maybe_qself(l: &Option<QSelf>, r: &Option<QSelf>) -> bool { } pub fn eq_path(l: &Path, r: &Path) -> bool { - over(&l.segments, &r.segments, |l, r| eq_path_seg(l, r)) + over(&l.segments, &r.segments, eq_path_seg) } pub fn eq_path_seg(l: &PathSegment, r: &PathSegment) -> bool { @@ -101,9 +98,7 @@ pub fn eq_path_seg(l: &PathSegment, r: &PathSegment) -> bool { pub fn eq_generic_args(l: &GenericArgs, r: &GenericArgs) -> bool { match (l, r) { - (GenericArgs::AngleBracketed(l), GenericArgs::AngleBracketed(r)) => { - over(&l.args, &r.args, |l, r| eq_angle_arg(l, r)) - }, + (GenericArgs::AngleBracketed(l), GenericArgs::AngleBracketed(r)) => over(&l.args, &r.args, eq_angle_arg), (GenericArgs::Parenthesized(l), GenericArgs::Parenthesized(r)) => { over(&l.inputs, &r.inputs, |l, r| eq_ty(l, r)) && eq_fn_ret_ty(&l.output, &r.output) }, @@ -142,7 +137,7 @@ pub fn eq_struct_rest(l: &StructRest, r: &StructRest) -> bool { pub fn eq_expr(l: &Expr, r: &Expr) -> bool { use ExprKind::*; - if !over(&l.attrs, &r.attrs, |l, r| eq_attr(l, r)) { + if !over(&l.attrs, &r.attrs, eq_attr) { return false; } match (&l.kind, &r.kind) { @@ -173,20 +168,20 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool { (Assign(l1, l2, _), Assign(r1, r2, _)) | (Index(l1, l2), Index(r1, r2)) => eq_expr(l1, r1) && eq_expr(l2, r2), (AssignOp(lo, lp, lv), AssignOp(ro, rp, rv)) => lo.node == ro.node && eq_expr(lp, rp) && eq_expr(lv, rv), (Field(lp, lf), Field(rp, rf)) => eq_id(*lf, *rf) && eq_expr(lp, rp), - (Match(ls, la), Match(rs, ra)) => eq_expr(ls, rs) && over(la, ra, |l, r| eq_arm(l, r)), + (Match(ls, la), Match(rs, ra)) => eq_expr(ls, rs) && over(la, ra, eq_arm), (Closure(lc, la, lm, lf, lb, _), Closure(rc, ra, rm, rf, rb, _)) => { lc == rc && la.is_async() == ra.is_async() && lm == rm && eq_fn_decl(lf, rf) && eq_expr(lb, rb) }, (Async(lc, _, lb), Async(rc, _, rb)) => lc == rc && eq_block(lb, rb), (Range(lf, lt, ll), Range(rf, rt, rl)) => ll == rl && eq_expr_opt(lf, rf) && eq_expr_opt(lt, rt), (AddrOf(lbk, lm, le), AddrOf(rbk, rm, re)) => lbk == rbk && lm == rm && eq_expr(le, re), - (Path(lq, lp), Path(rq, rp)) => both(lq, rq, |l, r| eq_qself(l, r)) && eq_path(lp, rp), + (Path(lq, lp), Path(rq, rp)) => both(lq, rq, eq_qself) && eq_path(lp, rp), (MacCall(l), MacCall(r)) => eq_mac_call(l, r), (Struct(lse), Struct(rse)) => { eq_maybe_qself(&lse.qself, &rse.qself) && eq_path(&lse.path, &rse.path) && eq_struct_rest(&lse.rest, &rse.rest) - && unordered_over(&lse.fields, &rse.fields, |l, r| eq_field(l, r)) + && unordered_over(&lse.fields, &rse.fields, eq_field) }, _ => false, } @@ -196,7 +191,7 @@ pub fn eq_field(l: &ExprField, r: &ExprField) -> bool { l.is_placeholder == r.is_placeholder && eq_id(l.ident, r.ident) && eq_expr(&l.expr, &r.expr) - && over(&l.attrs, &r.attrs, |l, r| eq_attr(l, r)) + && over(&l.attrs, &r.attrs, eq_attr) } pub fn eq_arm(l: &Arm, r: &Arm) -> bool { @@ -204,7 +199,7 @@ pub fn eq_arm(l: &Arm, r: &Arm) -> bool { && eq_pat(&l.pat, &r.pat) && eq_expr(&l.body, &r.body) && eq_expr_opt(&l.guard, &r.guard) - && over(&l.attrs, &r.attrs, |l, r| eq_attr(l, r)) + && over(&l.attrs, &r.attrs, eq_attr) } pub fn eq_label(l: &Option<Label>, r: &Option<Label>) -> bool { @@ -212,7 +207,7 @@ pub fn eq_label(l: &Option<Label>, r: &Option<Label>) -> bool { } pub fn eq_block(l: &Block, r: &Block) -> bool { - l.rules == r.rules && over(&l.stmts, &r.stmts, |l, r| eq_stmt(l, r)) + l.rules == r.rules && over(&l.stmts, &r.stmts, eq_stmt) } pub fn eq_stmt(l: &Stmt, r: &Stmt) -> bool { @@ -222,13 +217,13 @@ pub fn eq_stmt(l: &Stmt, r: &Stmt) -> bool { eq_pat(&l.pat, &r.pat) && both(&l.ty, &r.ty, |l, r| eq_ty(l, r)) && eq_local_kind(&l.kind, &r.kind) - && over(&l.attrs, &r.attrs, |l, r| eq_attr(l, r)) + && over(&l.attrs, &r.attrs, eq_attr) }, (Item(l), Item(r)) => eq_item(l, r, eq_item_kind), (Expr(l), Expr(r)) | (Semi(l), Semi(r)) => eq_expr(l, r), (Empty, Empty) => true, (MacCall(l), MacCall(r)) => { - l.style == r.style && eq_mac_call(&l.mac, &r.mac) && over(&l.attrs, &r.attrs, |l, r| eq_attr(l, r)) + l.style == r.style && eq_mac_call(&l.mac, &r.mac) && over(&l.attrs, &r.attrs, eq_attr) }, _ => false, } @@ -245,10 +240,7 @@ pub fn eq_local_kind(l: &LocalKind, r: &LocalKind) -> bool { } pub fn eq_item<K>(l: &Item<K>, r: &Item<K>, mut eq_kind: impl FnMut(&K, &K) -> bool) -> bool { - eq_id(l.ident, r.ident) - && over(&l.attrs, &r.attrs, |l, r| eq_attr(l, r)) - && eq_vis(&l.vis, &r.vis) - && eq_kind(&l.kind, &r.kind) + eq_id(l.ident, r.ident) && over(&l.attrs, &r.attrs, eq_attr) && eq_vis(&l.vis, &r.vis) && eq_kind(&l.kind, &r.kind) } pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool { @@ -272,18 +264,15 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool { } }, (ForeignMod(l), ForeignMod(r)) => { - both(&l.abi, &r.abi, |l, r| eq_str_lit(l, r)) - && over(&l.items, &r.items, |l, r| eq_item(l, r, eq_foreign_item_kind)) + both(&l.abi, &r.abi, eq_str_lit) && over(&l.items, &r.items, |l, r| eq_item(l, r, eq_foreign_item_kind)) }, (TyAlias(box TyAliasKind(ld, lg, lb, lt)), TyAlias(box TyAliasKind(rd, rg, rb, rt))) => { eq_defaultness(*ld, *rd) && eq_generics(lg, rg) - && over(lb, rb, |l, r| eq_generic_bound(l, r)) + && over(lb, rb, eq_generic_bound) && both(lt, rt, |l, r| eq_ty(l, r)) }, - (Enum(le, lg), Enum(re, rg)) => { - over(&le.variants, &re.variants, |l, r| eq_variant(l, r)) && eq_generics(lg, rg) - }, + (Enum(le, lg), Enum(re, rg)) => over(&le.variants, &re.variants, eq_variant) && eq_generics(lg, rg), (Struct(lv, lg), Struct(rv, rg)) | (Union(lv, lg), Union(rv, rg)) => { eq_variant_data(lv, rv) && eq_generics(lg, rg) }, @@ -291,10 +280,10 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool { la == ra && matches!(lu, Unsafe::No) == matches!(ru, Unsafe::No) && eq_generics(lg, rg) - && over(lb, rb, |l, r| eq_generic_bound(l, r)) + && over(lb, rb, eq_generic_bound) && over(li, ri, |l, r| eq_item(l, r, eq_assoc_item_kind)) }, - (TraitAlias(lg, lb), TraitAlias(rg, rb)) => eq_generics(lg, rg) && over(lb, rb, |l, r| eq_generic_bound(l, r)), + (TraitAlias(lg, lb), TraitAlias(rg, rb)) => eq_generics(lg, rg) && over(lb, rb, eq_generic_bound), ( Impl(box ImplKind { unsafety: lu, @@ -342,7 +331,7 @@ pub fn eq_foreign_item_kind(l: &ForeignItemKind, r: &ForeignItemKind) -> bool { (TyAlias(box TyAliasKind(ld, lg, lb, lt)), TyAlias(box TyAliasKind(rd, rg, rb, rt))) => { eq_defaultness(*ld, *rd) && eq_generics(lg, rg) - && over(lb, rb, |l, r| eq_generic_bound(l, r)) + && over(lb, rb, eq_generic_bound) && both(lt, rt, |l, r| eq_ty(l, r)) }, (MacCall(l), MacCall(r)) => eq_mac_call(l, r), @@ -360,7 +349,7 @@ pub fn eq_assoc_item_kind(l: &AssocItemKind, r: &AssocItemKind) -> bool { (TyAlias(box TyAliasKind(ld, lg, lb, lt)), TyAlias(box TyAliasKind(rd, rg, rb, rt))) => { eq_defaultness(*ld, *rd) && eq_generics(lg, rg) - && over(lb, rb, |l, r| eq_generic_bound(l, r)) + && over(lb, rb, eq_generic_bound) && both(lt, rt, |l, r| eq_ty(l, r)) }, (MacCall(l), MacCall(r)) => eq_mac_call(l, r), @@ -370,7 +359,7 @@ pub fn eq_assoc_item_kind(l: &AssocItemKind, r: &AssocItemKind) -> bool { pub fn eq_variant(l: &Variant, r: &Variant) -> bool { l.is_placeholder == r.is_placeholder - && over(&l.attrs, &r.attrs, |l, r| eq_attr(l, r)) + && over(&l.attrs, &r.attrs, eq_attr) && eq_vis(&l.vis, &r.vis) && eq_id(l.ident, r.ident) && eq_variant_data(&l.data, &r.data) @@ -381,14 +370,14 @@ pub fn eq_variant_data(l: &VariantData, r: &VariantData) -> bool { use VariantData::*; match (l, r) { (Unit(_), Unit(_)) => true, - (Struct(l, _), Struct(r, _)) | (Tuple(l, _), Tuple(r, _)) => over(l, r, |l, r| eq_struct_field(l, r)), + (Struct(l, _), Struct(r, _)) | (Tuple(l, _), Tuple(r, _)) => over(l, r, eq_struct_field), _ => false, } } pub fn eq_struct_field(l: &FieldDef, r: &FieldDef) -> bool { l.is_placeholder == r.is_placeholder - && over(&l.attrs, &r.attrs, |l, r| eq_attr(l, r)) + && over(&l.attrs, &r.attrs, eq_attr) && eq_vis(&l.vis, &r.vis) && both(&l.ident, &r.ident, |l, r| eq_id(*l, *r)) && eq_ty(&l.ty, &r.ty) @@ -406,7 +395,7 @@ pub fn eq_fn_header(l: &FnHeader, r: &FnHeader) -> bool { } pub fn eq_generics(l: &Generics, r: &Generics) -> bool { - over(&l.params, &r.params, |l, r| eq_generic_param(l, r)) + over(&l.params, &r.params, eq_generic_param) && over(&l.where_clause.predicates, &r.where_clause.predicates, |l, r| { eq_where_predicate(l, r) }) @@ -419,10 +408,10 @@ pub fn eq_where_predicate(l: &WherePredicate, r: &WherePredicate) -> bool { over(&l.bound_generic_params, &r.bound_generic_params, |l, r| { eq_generic_param(l, r) }) && eq_ty(&l.bounded_ty, &r.bounded_ty) - && over(&l.bounds, &r.bounds, |l, r| eq_generic_bound(l, r)) + && over(&l.bounds, &r.bounds, eq_generic_bound) }, (RegionPredicate(l), RegionPredicate(r)) => { - eq_id(l.lifetime.ident, r.lifetime.ident) && over(&l.bounds, &r.bounds, |l, r| eq_generic_bound(l, r)) + eq_id(l.lifetime.ident, r.lifetime.ident) && over(&l.bounds, &r.bounds, eq_generic_bound) }, (EqPredicate(l), EqPredicate(r)) => eq_ty(&l.lhs_ty, &r.lhs_ty) && eq_ty(&l.rhs_ty, &r.rhs_ty), _ => false, @@ -469,7 +458,7 @@ pub fn eq_fn_decl(l: &FnDecl, r: &FnDecl) -> bool { l.is_placeholder == r.is_placeholder && eq_pat(&l.pat, &r.pat) && eq_ty(&l.ty, &r.ty) - && over(&l.attrs, &r.attrs, |l, r| eq_attr(l, r)) + && over(&l.attrs, &r.attrs, eq_attr) }) } @@ -496,13 +485,13 @@ pub fn eq_ty(l: &Ty, r: &Ty) -> bool { (BareFn(l), BareFn(r)) => { l.unsafety == r.unsafety && eq_ext(&l.ext, &r.ext) - && over(&l.generic_params, &r.generic_params, |l, r| eq_generic_param(l, r)) + && over(&l.generic_params, &r.generic_params, eq_generic_param) && eq_fn_decl(&l.decl, &r.decl) }, (Tup(l), Tup(r)) => over(l, r, |l, r| eq_ty(l, r)), - (Path(lq, lp), Path(rq, rp)) => both(lq, rq, |l, r| eq_qself(l, r)) && eq_path(lp, rp), - (TraitObject(lg, ls), TraitObject(rg, rs)) => ls == rs && over(lg, rg, |l, r| eq_generic_bound(l, r)), - (ImplTrait(_, lg), ImplTrait(_, rg)) => over(lg, rg, |l, r| eq_generic_bound(l, r)), + (Path(lq, lp), Path(rq, rp)) => both(lq, rq, eq_qself) && eq_path(lp, rp), + (TraitObject(lg, ls), TraitObject(rg, rs)) => ls == rs && over(lg, rg, eq_generic_bound), + (ImplTrait(_, lg), ImplTrait(_, rg)) => over(lg, rg, eq_generic_bound), (Typeof(l), Typeof(r)) => eq_expr(&l.value, &r.value), (MacCall(l), MacCall(r)) => eq_mac_call(l, r), _ => false, @@ -533,7 +522,7 @@ pub fn eq_generic_param(l: &GenericParam, r: &GenericParam) -> bool { use GenericParamKind::*; l.is_placeholder == r.is_placeholder && eq_id(l.ident, r.ident) - && over(&l.bounds, &r.bounds, |l, r| eq_generic_bound(l, r)) + && over(&l.bounds, &r.bounds, eq_generic_bound) && match (&l.kind, &r.kind) { (Lifetime, Lifetime) => true, (Type { default: l }, Type { default: r }) => both(l, r, |l, r| eq_ty(l, r)), @@ -548,10 +537,10 @@ pub fn eq_generic_param(l: &GenericParam, r: &GenericParam) -> bool { kw_span: _, default: rd, }, - ) => eq_ty(lt, rt) && both(ld, rd, |ld, rd| eq_anon_const(ld, rd)), + ) => eq_ty(lt, rt) && both(ld, rd, eq_anon_const), _ => false, } - && over(&l.attrs, &r.attrs, |l, r| eq_attr(l, r)) + && over(&l.attrs, &r.attrs, eq_attr) } pub fn eq_generic_bound(l: &GenericBound, r: &GenericBound) -> bool { @@ -568,7 +557,7 @@ pub fn eq_assoc_constraint(l: &AssocTyConstraint, r: &AssocTyConstraint) -> bool eq_id(l.ident, r.ident) && match (&l.kind, &r.kind) { (Equality { ty: l }, Equality { ty: r }) => eq_ty(l, r), - (Bound { bounds: l }, Bound { bounds: r }) => over(l, r, |l, r| eq_generic_bound(l, r)), + (Bound { bounds: l }, Bound { bounds: r }) => over(l, r, eq_generic_bound), _ => false, } } diff --git a/clippy_utils/src/eager_or_lazy.rs b/clippy_utils/src/eager_or_lazy.rs index 29e2559fc6d..9650294fc7b 100644 --- a/clippy_utils/src/eager_or_lazy.rs +++ b/clippy_utils/src/eager_or_lazy.rs @@ -27,10 +27,9 @@ fn identify_some_pure_patterns(expr: &Expr<'_>) -> bool { match expr.kind { ExprKind::Lit(..) | ExprKind::ConstBlock(..) | ExprKind::Path(..) | ExprKind::Field(..) => true, ExprKind::AddrOf(_, _, addr_of_expr) => identify_some_pure_patterns(addr_of_expr), - ExprKind::Tup(tup_exprs) => tup_exprs.iter().all(|expr| identify_some_pure_patterns(expr)), + ExprKind::Tup(tup_exprs) => tup_exprs.iter().all(identify_some_pure_patterns), ExprKind::Struct(_, fields, expr) => { - fields.iter().all(|f| identify_some_pure_patterns(f.expr)) - && expr.map_or(true, |e| identify_some_pure_patterns(e)) + fields.iter().all(|f| identify_some_pure_patterns(f.expr)) && expr.map_or(true, identify_some_pure_patterns) }, ExprKind::Call( &Expr { @@ -45,7 +44,7 @@ fn identify_some_pure_patterns(expr: &Expr<'_>) -> bool { .. }, args, - ) => args.iter().all(|expr| identify_some_pure_patterns(expr)), + ) => args.iter().all(identify_some_pure_patterns), ExprKind::Block( &Block { stmts, diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs index e6c06224999..ba4d50bf744 100644 --- a/clippy_utils/src/higher.rs +++ b/clippy_utils/src/higher.rs @@ -602,3 +602,33 @@ pub fn is_from_for_desugar(local: &hir::Local<'_>) -> bool { false } + +/// A parsed `panic!` expansion +pub struct PanicExpn<'tcx> { + /// Span of `panic!(..)` + pub call_site: Span, + /// Inner `format_args!` expansion + pub format_args: FormatArgsExpn<'tcx>, +} + +impl PanicExpn<'tcx> { + /// Parses an expanded `panic!` invocation + pub fn parse(expr: &'tcx Expr<'tcx>) -> Option<Self> { + if_chain! { + if let ExprKind::Block(block, _) = expr.kind; + if let Some(init) = block.expr; + if let ExprKind::Call(_, [format_args]) = init.kind; + let expn_data = expr.span.ctxt().outer_expn_data(); + if let ExprKind::AddrOf(_, _, format_args) = format_args.kind; + if let Some(format_args) = FormatArgsExpn::parse(format_args); + then { + Some(PanicExpn { + call_site: expn_data.call_site, + format_args, + }) + } else { + None + } + } + } +} diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 6e9a1de21ee..7438b6eabf9 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -540,7 +540,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { std::mem::discriminant(&b.rules).hash(&mut self.s); } - #[allow(clippy::many_single_char_names, clippy::too_many_lines)] + #[allow(clippy::too_many_lines)] pub fn hash_expr(&mut self, e: &Expr<'_>) { let simple_const = self .maybe_typeck_results diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 80be4350c3c..7a8208c12c0 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -68,6 +68,7 @@ pub const IO_WRITE: [&str; 3] = ["std", "io", "Write"]; pub const IPADDR_V4: [&str; 5] = ["std", "net", "ip", "IpAddr", "V4"]; pub const IPADDR_V6: [&str; 5] = ["std", "net", "ip", "IpAddr", "V6"]; pub const ITER_REPEAT: [&str; 5] = ["core", "iter", "sources", "repeat", "repeat"]; +#[allow(clippy::invalid_paths)] // internal lints do not know about all external crates pub const ITERTOOLS_NEXT_TUPLE: [&str; 3] = ["itertools", "Itertools", "next_tuple"]; #[cfg(feature = "internal-lints")] pub const KW_MODULE: [&str; 3] = ["rustc_span", "symbol", "kw"]; diff --git a/clippy_utils/src/qualify_min_const_fn.rs b/clippy_utils/src/qualify_min_const_fn.rs index e2f2e2008bb..238728f090f 100644 --- a/clippy_utils/src/qualify_min_const_fn.rs +++ b/clippy_utils/src/qualify_min_const_fn.rs @@ -192,9 +192,8 @@ fn check_rvalue(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, rvalue: &Rv )) } }, - Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf, _) => Ok(()), + Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf, _) | Rvalue::ShallowInitBox(_, _) => Ok(()), Rvalue::NullaryOp(NullOp::Box, _) => Err((span, "heap allocations are not allowed in const fn".into())), - Rvalue::ShallowInitBox(_, _) => Ok(()), Rvalue::UnaryOp(_, operand) => { let ty = operand.ty(body, tcx); if ty.is_integral() || ty.is_bool() { diff --git a/lintcheck/src/main.rs b/lintcheck/src/main.rs index f1e03ba4296..97d3794fb84 100644 --- a/lintcheck/src/main.rs +++ b/lintcheck/src/main.rs @@ -15,6 +15,8 @@ use std::{ env, fmt, fs::write, path::{Path, PathBuf}, + thread, + time::Duration, }; use clap::{App, Arg, ArgMatches}; @@ -109,6 +111,22 @@ impl std::fmt::Display for ClippyWarning { } } +fn get(path: &str) -> Result<ureq::Response, ureq::Error> { + const MAX_RETRIES: u8 = 4; + let mut retries = 0; + loop { + match ureq::get(path).call() { + Ok(res) => return Ok(res), + Err(e) if retries >= MAX_RETRIES => return Err(e), + Err(ureq::Error::Transport(e)) => eprintln!("Error: {}", e), + Err(e) => return Err(e), + } + eprintln!("retrying in {} seconds...", retries); + thread::sleep(Duration::from_secs(retries as u64)); + retries += 1; + } +} + impl CrateSource { /// Makes the sources available on the disk for clippy to check. /// Clones a git repo and checks out the specified commit or downloads a crate from crates.io or @@ -129,7 +147,7 @@ impl CrateSource { if !krate_file_path.is_file() { // create a file path to download and write the crate data into let mut krate_dest = std::fs::File::create(&krate_file_path).unwrap(); - let mut krate_req = ureq::get(&url).call().unwrap().into_reader(); + let mut krate_req = get(&url).unwrap().into_reader(); // copy the crate into the file std::io::copy(&mut krate_req, &mut krate_dest).unwrap(); diff --git a/rust-toolchain b/rust-toolchain index 92bde3423a2..660401ff28c 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2021-09-08" +channel = "nightly-2021-09-28" components = ["llvm-tools-preview", "rustc-dev", "rust-src"] diff --git a/src/main.rs b/src/main.rs index 7589f42a7b4..7ebdd947893 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,7 +4,6 @@ use rustc_tools_util::VersionInfo; use std::env; -use std::ffi::OsString; use std::path::PathBuf; use std::process::{self, Command}; @@ -14,7 +13,7 @@ Usage: cargo clippy [options] [--] [<opts>...] Common options: - --no-deps Run Clippy only on the given crate, without linting the dependencies + --no-deps Run Clippy only on the given crate, without linting the dependencies --fix Automatically apply lint suggestions. This flag implies `--no-deps` -h, --help Print this message -V, --version Print version info and exit @@ -116,22 +115,6 @@ impl ClippyCmd { path } - fn target_dir() -> Option<(&'static str, OsString)> { - env::var_os("CLIPPY_DOGFOOD") - .map(|_| { - env::var_os("CARGO_MANIFEST_DIR").map_or_else( - || std::ffi::OsString::from("clippy_dogfood"), - |d| { - std::path::PathBuf::from(d) - .join("target") - .join("dogfood") - .into_os_string() - }, - ) - }) - .map(|p| ("CARGO_TARGET_DIR", p)) - } - fn into_std_cmd(self) -> Command { let mut cmd = Command::new("cargo"); let clippy_args: String = self @@ -141,7 +124,6 @@ impl ClippyCmd { .collect(); cmd.env("RUSTC_WORKSPACE_WRAPPER", Self::path()) - .envs(ClippyCmd::target_dir()) .env("CLIPPY_ARGS", clippy_args) .arg(self.cargo_subcommand) .args(&self.args); diff --git a/tests/cargo/mod.rs b/tests/cargo/mod.rs index a8f3e3145f6..4dbe71e4b6a 100644 --- a/tests/cargo/mod.rs +++ b/tests/cargo/mod.rs @@ -1,25 +1,3 @@ -use std::env; -use std::lazy::SyncLazy; -use std::path::PathBuf; - -pub static CARGO_TARGET_DIR: SyncLazy<PathBuf> = SyncLazy::new(|| match env::var_os("CARGO_TARGET_DIR") { - Some(v) => v.into(), - None => env::current_dir().unwrap().join("target"), -}); - -pub static TARGET_LIB: SyncLazy<PathBuf> = SyncLazy::new(|| { - if let Some(path) = option_env!("TARGET_LIBS") { - path.into() - } else { - let mut dir = CARGO_TARGET_DIR.clone(); - if let Some(target) = env::var_os("CARGO_BUILD_TARGET") { - dir.push(target); - } - dir.push(env!("PROFILE")); - dir - } -}); - #[must_use] pub fn is_rustc_test_suite() -> bool { option_env!("RUSTC_TEST_SUITE").is_some() diff --git a/tests/compile-test.rs b/tests/compile-test.rs index c63c47690d5..d7596f6ff0c 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -1,5 +1,4 @@ #![feature(test)] // compiletest_rs requires this attribute -#![feature(once_cell)] #![cfg_attr(feature = "deny-warnings", deny(warnings))] #![warn(rust_2018_idioms, unused_lifetimes)] @@ -46,14 +45,6 @@ extern crate quote; #[allow(unused_extern_crates)] extern crate syn; -fn host_lib() -> PathBuf { - option_env!("HOST_LIBS").map_or(cargo::CARGO_TARGET_DIR.join(env!("PROFILE")), PathBuf::from) -} - -fn clippy_driver_path() -> PathBuf { - option_env!("CLIPPY_DRIVER_PATH").map_or(cargo::TARGET_LIB.join("clippy-driver"), PathBuf::from) -} - /// Produces a string with an `--extern` flag for all UI test crate /// dependencies. /// @@ -99,12 +90,14 @@ fn extern_flags() -> String { .copied() .filter(|n| !crates.contains_key(n)) .collect(); - if !not_found.is_empty() { - panic!("dependencies not found in depinfo: {:?}", not_found); - } + assert!( + not_found.is_empty(), + "dependencies not found in depinfo: {:?}", + not_found + ); crates .into_iter() - .map(|(name, path)| format!("--extern {}={} ", name, path)) + .map(|(name, path)| format!(" --extern {}={}", name, path)) .collect() } @@ -120,19 +113,29 @@ fn default_config() -> compiletest::Config { config.run_lib_path = path.clone(); config.compile_lib_path = path; } + let current_exe_path = std::env::current_exe().unwrap(); + let deps_path = current_exe_path.parent().unwrap(); + let profile_path = deps_path.parent().unwrap(); // Using `-L dependency={}` enforces that external dependencies are added with `--extern`. // This is valuable because a) it allows us to monitor what external dependencies are used // and b) it ensures that conflicting rlibs are resolved properly. + let host_libs = option_env!("HOST_LIBS") + .map(|p| format!(" -L dependency={}", Path::new(p).join("deps").display())) + .unwrap_or_default(); config.target_rustcflags = Some(format!( - "--emit=metadata -L dependency={} -L dependency={} -Dwarnings -Zui-testing {}", - host_lib().join("deps").display(), - cargo::TARGET_LIB.join("deps").display(), + "--emit=metadata -Dwarnings -Zui-testing -L dependency={}{}{}", + deps_path.display(), + host_libs, extern_flags(), )); - config.build_base = host_lib().join("test_build_base"); - config.rustc_path = clippy_driver_path(); + config.build_base = profile_path.join("test"); + config.rustc_path = profile_path.join(if cfg!(windows) { + "clippy-driver.exe" + } else { + "clippy-driver" + }); config } diff --git a/tests/dogfood.rs b/tests/dogfood.rs index 54f452172de..a37cdfed126 100644 --- a/tests/dogfood.rs +++ b/tests/dogfood.rs @@ -15,7 +15,12 @@ use std::process::Command; mod cargo; -static CLIPPY_PATH: SyncLazy<PathBuf> = SyncLazy::new(|| cargo::TARGET_LIB.join("cargo-clippy")); +static CLIPPY_PATH: SyncLazy<PathBuf> = SyncLazy::new(|| { + let mut path = std::env::current_exe().unwrap(); + assert!(path.pop()); // deps + path.set_file_name("cargo-clippy"); + path +}); #[test] fn dogfood_clippy() { @@ -28,7 +33,6 @@ fn dogfood_clippy() { let mut command = Command::new(&*CLIPPY_PATH); command .current_dir(root_dir) - .env("CLIPPY_DOGFOOD", "1") .env("CARGO_INCREMENTAL", "0") .arg("clippy") .arg("--all-targets") @@ -74,7 +78,6 @@ fn test_no_deps_ignores_path_deps_in_workspaces() { // Make sure that with the `--no-deps` argument Clippy does not run on `path_dep`. let output = Command::new(&*CLIPPY_PATH) .current_dir(&cwd) - .env("CLIPPY_DOGFOOD", "1") .env("CARGO_INCREMENTAL", "0") .arg("clippy") .args(&["-p", "subcrate"]) @@ -94,7 +97,6 @@ fn test_no_deps_ignores_path_deps_in_workspaces() { // Test that without the `--no-deps` argument, `path_dep` is linted. let output = Command::new(&*CLIPPY_PATH) .current_dir(&cwd) - .env("CLIPPY_DOGFOOD", "1") .env("CARGO_INCREMENTAL", "0") .arg("clippy") .args(&["-p", "subcrate"]) @@ -121,7 +123,6 @@ fn test_no_deps_ignores_path_deps_in_workspaces() { let successful_build = || { let output = Command::new(&*CLIPPY_PATH) .current_dir(&cwd) - .env("CLIPPY_DOGFOOD", "1") .env("CARGO_INCREMENTAL", "0") .arg("clippy") .args(&["-p", "subcrate"]) @@ -223,7 +224,6 @@ fn run_clippy_for_project(project: &str) { command .current_dir(root_dir.join(project)) - .env("CLIPPY_DOGFOOD", "1") .env("CARGO_INCREMENTAL", "0") .arg("clippy") .arg("--all-targets") diff --git a/tests/integration.rs b/tests/integration.rs index 7e3eff3c732..c64425fa01a 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -74,8 +74,11 @@ fn integration_test() { panic!("incompatible crate versions"); } else if stderr.contains("failed to run `rustc` to learn about target-specific information") { panic!("couldn't find librustc_driver, consider setting `LD_LIBRARY_PATH`"); - } else if stderr.contains("toolchain") && stderr.contains("is not installed") { - panic!("missing required toolchain"); + } else { + assert!( + !stderr.contains("toolchain") || !stderr.contains("is not installed"), + "missing required toolchain" + ); } match output.status.code() { diff --git a/tests/ui-internal/invalid_paths.stderr b/tests/ui-internal/invalid_paths.stderr index bd69d661b71..20aa81b98a0 100644 --- a/tests/ui-internal/invalid_paths.stderr +++ b/tests/ui-internal/invalid_paths.stderr @@ -4,7 +4,7 @@ error: invalid path LL | pub const BAD_CRATE_PATH: [&str; 2] = ["bad", "path"]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: `-D clippy::clippy-lints-internal` implied by `-D warnings` + = note: `-D clippy::invalid-paths` implied by `-D warnings` error: invalid path --> $DIR/invalid_paths.rs:20:5 diff --git a/tests/ui-toml/toml_disallowed_method/clippy.toml b/tests/ui-toml/toml_disallowed_method/clippy.toml index a3245da6825..f1d4a4619c5 100644 --- a/tests/ui-toml/toml_disallowed_method/clippy.toml +++ b/tests/ui-toml/toml_disallowed_method/clippy.toml @@ -1,5 +1,8 @@ disallowed-methods = [ + # just a string is shorthand for path only "std::iter::Iterator::sum", - "regex::Regex::is_match", - "regex::Regex::new" + # can give path and reason with an inline table + { path = "regex::Regex::is_match", reason = "no matching allowed" }, + # can use an inline table but omit reason + { path = "regex::Regex::new" }, ] diff --git a/tests/ui-toml/toml_disallowed_method/conf_disallowed_method.stderr b/tests/ui-toml/toml_disallowed_method/conf_disallowed_method.stderr index 2b628c67fa7..38123220a43 100644 --- a/tests/ui-toml/toml_disallowed_method/conf_disallowed_method.stderr +++ b/tests/ui-toml/toml_disallowed_method/conf_disallowed_method.stderr @@ -1,4 +1,4 @@ -error: use of a disallowed method `regex::re_unicode::Regex::new` +error: use of a disallowed method `regex::Regex::new` --> $DIR/conf_disallowed_method.rs:7:14 | LL | let re = Regex::new(r"ab.*c").unwrap(); @@ -6,13 +6,15 @@ LL | let re = Regex::new(r"ab.*c").unwrap(); | = note: `-D clippy::disallowed-method` implied by `-D warnings` -error: use of a disallowed method `regex::re_unicode::Regex::is_match` +error: use of a disallowed method `regex::Regex::is_match` --> $DIR/conf_disallowed_method.rs:8:5 | LL | re.is_match("abc"); | ^^^^^^^^^^^^^^^^^^ + | + = note: no matching allowed (from clippy.toml) -error: use of a disallowed method `core::iter::traits::iterator::Iterator::sum` +error: use of a disallowed method `std::iter::Iterator::sum` --> $DIR/conf_disallowed_method.rs:11:5 | LL | a.iter().sum::<i32>(); diff --git a/tests/ui-toml/toml_trivially_copy/test.rs b/tests/ui-toml/toml_trivially_copy/test.rs index 19019a25416..fb0e226f3aa 100644 --- a/tests/ui-toml/toml_trivially_copy/test.rs +++ b/tests/ui-toml/toml_trivially_copy/test.rs @@ -2,7 +2,6 @@ // normalize-stderr-test "\(limit: \d+ byte\)" -> "(limit: N byte)" #![deny(clippy::trivially_copy_pass_by_ref)] -#![allow(clippy::many_single_char_names)] #[derive(Copy, Clone)] struct Foo(u8); diff --git a/tests/ui-toml/toml_trivially_copy/test.stderr b/tests/ui-toml/toml_trivially_copy/test.stderr index 912761a8f00..b3ef5928e8e 100644 --- a/tests/ui-toml/toml_trivially_copy/test.stderr +++ b/tests/ui-toml/toml_trivially_copy/test.stderr @@ -1,5 +1,5 @@ error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/test.rs:15:11 + --> $DIR/test.rs:14:11 | LL | fn bad(x: &u16, y: &Foo) {} | ^^^^ help: consider passing by value instead: `u16` @@ -11,7 +11,7 @@ LL | #![deny(clippy::trivially_copy_pass_by_ref)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/test.rs:15:20 + --> $DIR/test.rs:14:20 | LL | fn bad(x: &u16, y: &Foo) {} | ^^^^ help: consider passing by value instead: `Foo` diff --git a/tests/ui/auxiliary/proc_macro_suspicious_else_formatting.rs b/tests/ui/auxiliary/proc_macro_suspicious_else_formatting.rs new file mode 100644 index 00000000000..26c88489b03 --- /dev/null +++ b/tests/ui/auxiliary/proc_macro_suspicious_else_formatting.rs @@ -0,0 +1,75 @@ +// compile-flags: --emit=link +// no-prefer-dynamic + +#![crate_type = "proc-macro"] + +extern crate proc_macro; +use proc_macro::{token_stream, Delimiter, Group, Ident, Span, TokenStream, TokenTree}; +use std::iter::FromIterator; + +fn read_ident(iter: &mut token_stream::IntoIter) -> Ident { + match iter.next() { + Some(TokenTree::Ident(i)) => i, + _ => panic!("expected ident"), + } +} + +#[proc_macro_derive(DeriveBadSpan)] +pub fn derive_bad_span(input: TokenStream) -> TokenStream { + let mut input = input.into_iter(); + assert_eq!(read_ident(&mut input).to_string(), "struct"); + let ident = read_ident(&mut input); + let mut tys = match input.next() { + Some(TokenTree::Group(g)) if g.delimiter() == Delimiter::Parenthesis => g.stream().into_iter(), + _ => panic!(), + }; + let field1 = read_ident(&mut tys); + tys.next(); + let field2 = read_ident(&mut tys); + + <TokenStream as FromIterator<TokenTree>>::from_iter( + [ + Ident::new("impl", Span::call_site()).into(), + ident.into(), + Group::new( + Delimiter::Brace, + <TokenStream as FromIterator<TokenTree>>::from_iter( + [ + Ident::new("fn", Span::call_site()).into(), + Ident::new("_foo", Span::call_site()).into(), + Group::new(Delimiter::Parenthesis, TokenStream::new()).into(), + Group::new( + Delimiter::Brace, + <TokenStream as FromIterator<TokenTree>>::from_iter( + [ + Ident::new("if", field1.span()).into(), + Ident::new("true", field1.span()).into(), + { + let mut group = Group::new(Delimiter::Brace, TokenStream::new()); + group.set_span(field1.span()); + group.into() + }, + Ident::new("if", field2.span()).into(), + Ident::new("true", field2.span()).into(), + { + let mut group = Group::new(Delimiter::Brace, TokenStream::new()); + group.set_span(field2.span()); + group.into() + }, + ] + .iter() + .cloned(), + ), + ) + .into(), + ] + .iter() + .cloned(), + ), + ) + .into(), + ] + .iter() + .cloned(), + ) +} diff --git a/tests/ui/box_vec.rs b/tests/ui/box_collection.rs similarity index 86% rename from tests/ui/box_vec.rs rename to tests/ui/box_collection.rs index 1d6366972da..e00f061f28a 100644 --- a/tests/ui/box_vec.rs +++ b/tests/ui/box_collection.rs @@ -6,6 +6,8 @@ unused )] +use std::collections::HashMap; + macro_rules! boxit { ($init:expr, $x:ty) => { let _: Box<$x> = Box::new($init); @@ -15,6 +17,7 @@ macro_rules! boxit { fn test_macro() { boxit!(Vec::new(), Vec<u8>); } + fn test(foo: Box<Vec<bool>>) {} fn test2(foo: Box<dyn Fn(Vec<u32>)>) { @@ -22,6 +25,10 @@ fn test2(foo: Box<dyn Fn(Vec<u32>)>) { foo(vec![1, 2, 3]) } +fn test3(foo: Box<String>) {} + +fn test4(foo: Box<HashMap<String, String>>) {} + fn test_local_not_linted() { let _: Box<Vec<bool>>; } @@ -29,6 +36,7 @@ fn test_local_not_linted() { // All of these test should be allowed because they are part of the // public api and `avoid_breaking_exported_api` is `false` by default. pub fn pub_test(foo: Box<Vec<bool>>) {} + pub fn pub_test_ret() -> Box<Vec<bool>> { Box::new(Vec::new()) } diff --git a/tests/ui/box_collection.stderr b/tests/ui/box_collection.stderr new file mode 100644 index 00000000000..6de85d05a99 --- /dev/null +++ b/tests/ui/box_collection.stderr @@ -0,0 +1,27 @@ +error: you seem to be trying to use `Box<Vec<..>>`. Consider using just `Vec<..>` + --> $DIR/box_collection.rs:21:14 + | +LL | fn test(foo: Box<Vec<bool>>) {} + | ^^^^^^^^^^^^^^ + | + = note: `-D clippy::box-collection` implied by `-D warnings` + = help: `Vec<..>` is already on the heap, `Box<Vec<..>>` makes an extra allocation + +error: you seem to be trying to use `Box<String>`. Consider using just `String` + --> $DIR/box_collection.rs:28:15 + | +LL | fn test3(foo: Box<String>) {} + | ^^^^^^^^^^^ + | + = help: `String` is already on the heap, `Box<String>` makes an extra allocation + +error: you seem to be trying to use `Box<HashMap<..>>`. Consider using just `HashMap<..>` + --> $DIR/box_collection.rs:30:15 + | +LL | fn test4(foo: Box<HashMap<String, String>>) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: `HashMap<..>` is already on the heap, `Box<HashMap<..>>` makes an extra allocation + +error: aborting due to 3 previous errors + diff --git a/tests/ui/box_vec.stderr b/tests/ui/box_vec.stderr deleted file mode 100644 index 58c1f13fb87..00000000000 --- a/tests/ui/box_vec.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error: you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>` - --> $DIR/box_vec.rs:18:14 - | -LL | fn test(foo: Box<Vec<bool>>) {} - | ^^^^^^^^^^^^^^ - | - = note: `-D clippy::box-vec` implied by `-D warnings` - = help: `Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation - -error: aborting due to previous error - diff --git a/tests/ui/default_trait_access.fixed b/tests/ui/default_trait_access.fixed index f1f9c123dc8..9114d8754dc 100644 --- a/tests/ui/default_trait_access.fixed +++ b/tests/ui/default_trait_access.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused_imports,dead_code)] +#![allow(unused_imports, dead_code)] #![deny(clippy::default_trait_access)] use std::default; diff --git a/tests/ui/default_trait_access.rs b/tests/ui/default_trait_access.rs index 7f3dfc7f013..8a5f0d6a749 100644 --- a/tests/ui/default_trait_access.rs +++ b/tests/ui/default_trait_access.rs @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused_imports,dead_code)] +#![allow(unused_imports, dead_code)] #![deny(clippy::default_trait_access)] use std::default; diff --git a/tests/ui/deref_addrof.fixed b/tests/ui/deref_addrof.fixed index 0795900558b..d4832daa689 100644 --- a/tests/ui/deref_addrof.fixed +++ b/tests/ui/deref_addrof.fixed @@ -9,7 +9,7 @@ fn get_reference(n: &usize) -> &usize { n } -#[allow(clippy::many_single_char_names, clippy::double_parens)] +#[allow(clippy::double_parens)] #[allow(unused_variables, unused_parens)] fn main() { let a = 10; diff --git a/tests/ui/deref_addrof.rs b/tests/ui/deref_addrof.rs index 60c4318601b..be7cc669b5b 100644 --- a/tests/ui/deref_addrof.rs +++ b/tests/ui/deref_addrof.rs @@ -9,7 +9,7 @@ fn get_reference(n: &usize) -> &usize { n } -#[allow(clippy::many_single_char_names, clippy::double_parens)] +#[allow(clippy::double_parens)] #[allow(unused_variables, unused_parens)] fn main() { let a = 10; diff --git a/tests/ui/derivable_impls.rs b/tests/ui/derivable_impls.rs index 336a743de72..ebbc0c77e32 100644 --- a/tests/ui/derivable_impls.rs +++ b/tests/ui/derivable_impls.rs @@ -167,4 +167,44 @@ impl Default for WithoutSelfParan { } } +// https://github.com/rust-lang/rust-clippy/issues/7655 + +pub struct SpecializedImpl2<T> { + v: Vec<T>, +} + +impl Default for SpecializedImpl2<String> { + fn default() -> Self { + Self { v: Vec::new() } + } +} + +// https://github.com/rust-lang/rust-clippy/issues/7654 + +pub struct Color { + pub r: u8, + pub g: u8, + pub b: u8, +} + +/// `#000000` +impl Default for Color { + fn default() -> Self { + Color { r: 0, g: 0, b: 0 } + } +} + +pub struct Color2 { + pub r: u8, + pub g: u8, + pub b: u8, +} + +impl Default for Color2 { + /// `#000000` + fn default() -> Self { + Self { r: 0, g: 0, b: 0 } + } +} + fn main() {} diff --git a/tests/ui/eq_op.rs b/tests/ui/eq_op.rs index 7ab23320db6..707b449f82e 100644 --- a/tests/ui/eq_op.rs +++ b/tests/ui/eq_op.rs @@ -2,7 +2,7 @@ #[rustfmt::skip] #[warn(clippy::eq_op)] -#[allow(clippy::identity_op, clippy::double_parens, clippy::many_single_char_names)] +#[allow(clippy::identity_op, clippy::double_parens)] #[allow(clippy::no_effect, unused_variables, clippy::unnecessary_operation, clippy::short_circuit_statement)] #[allow(clippy::nonminimal_bool)] #[allow(unused)] diff --git a/tests/ui/eta.fixed b/tests/ui/eta.fixed index 91b837f9a85..1de79667f55 100644 --- a/tests/ui/eta.fixed +++ b/tests/ui/eta.fixed @@ -4,7 +4,6 @@ unused, clippy::no_effect, clippy::redundant_closure_call, - clippy::many_single_char_names, clippy::needless_pass_by_value, clippy::option_map_unit_fn )] @@ -14,7 +13,7 @@ clippy::needless_borrow )] -use std::path::PathBuf; +use std::path::{Path, PathBuf}; macro_rules! mac { () => { @@ -30,19 +29,18 @@ macro_rules! closure_mac { fn main() { let a = Some(1u8).map(foo); - meta(foo); let c = Some(1u8).map(|a| {1+2; foo}(a)); true.then(|| mac!()); // don't lint function in macro expansion Some(1).map(closure_mac!()); // don't lint closure in macro expansion let _: Option<Vec<u8>> = true.then(std::vec::Vec::new); // special case vec! - let d = Some(1u8).map(|a| foo((|b| foo2(b))(a))); //is adjusted? - all(&[1, 2, 3], &2, |x, y| below(x, y)); //is adjusted + let d = Some(1u8).map(|a| foo(foo2(a))); //is adjusted? + all(&[1, 2, 3], &2, below); //is adjusted unsafe { Some(1u8).map(|a| unsafe_fn(a)); // unsafe fn } // See #815 - let e = Some(1u8).map(|a| divergent(a)); + let e = Some(1u8).map(divergent); let e = Some(1u8).map(generic); let e = Some(1u8).map(generic); // See #515 @@ -90,24 +88,17 @@ impl<'a> std::ops::Deref for TestStruct<'a> { fn test_redundant_closures_containing_method_calls() { let i = 10; let e = Some(TestStruct { some_ref: &i }).map(TestStruct::foo); - let e = Some(TestStruct { some_ref: &i }).map(TestStruct::foo); let e = Some(TestStruct { some_ref: &i }).map(TestTrait::trait_foo); let e = Some(TestStruct { some_ref: &i }).map(|a| a.trait_foo_ref()); - let e = Some(TestStruct { some_ref: &i }).map(TestTrait::trait_foo); - let e = Some(&mut vec![1, 2, 3]).map(std::vec::Vec::clear); let e = Some(&mut vec![1, 2, 3]).map(std::vec::Vec::clear); unsafe { let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo_unsafe()); } let e = Some("str").map(std::string::ToString::to_string); - let e = Some("str").map(str::to_string); - let e = Some('a').map(char::to_uppercase); let e = Some('a').map(char::to_uppercase); let e: std::vec::Vec<usize> = vec!['a', 'b', 'c'].iter().map(|c| c.len_utf8()).collect(); let e: std::vec::Vec<char> = vec!['a', 'b', 'c'].iter().map(char::to_ascii_uppercase).collect(); - let e: std::vec::Vec<char> = vec!['a', 'b', 'c'].iter().map(char::to_ascii_uppercase).collect(); - let p = Some(PathBuf::new()); - let e = p.as_ref().and_then(|s| s.to_str()); + let e = Some(PathBuf::new()).as_ref().and_then(|s| s.to_str()); let c = Some(TestStruct { some_ref: &i }) .as_ref() .map(|c| c.to_ascii_uppercase()); @@ -119,10 +110,6 @@ fn test_redundant_closures_containing_method_calls() { t.iter().filter(|x| x.trait_foo_ref()); t.iter().map(|x| x.trait_foo_ref()); } - - let mut some = Some(|x| x * x); - let arr = [Ok(1), Err(2)]; - let _: Vec<_> = arr.iter().map(|x| x.map_err(|e| some.take().unwrap()(e))).collect(); } struct Thunk<T>(Box<dyn FnMut() -> T>); @@ -145,13 +132,6 @@ fn foobar() { thunk.unwrap() } -fn meta<F>(f: F) -where - F: Fn(u8), -{ - f(1u8) -} - fn foo(_: u8) {} fn foo2(_: u8) -> u8 { @@ -180,7 +160,7 @@ fn generic<T>(_: T) -> u8 { } fn passes_fn_mut(mut x: Box<dyn FnMut()>) { - requires_fn_once(|| x()); + requires_fn_once(x); } fn requires_fn_once<T: FnOnce()>(_: T) {} @@ -236,3 +216,35 @@ fn mutable_closure_in_loop() { Some(1).map(&mut closure); } } + +fn late_bound_lifetimes() { + fn take_asref_path<P: AsRef<Path>>(path: P) {} + + fn map_str<F>(thunk: F) + where + F: FnOnce(&str), + { + } + + fn map_str_to_path<F>(thunk: F) + where + F: FnOnce(&str) -> &Path, + { + } + map_str(|s| take_asref_path(s)); + map_str_to_path(std::convert::AsRef::as_ref); +} + +mod type_param_bound { + trait Trait { + fn fun(); + } + + fn take<T: 'static>(_: T) {} + + fn test<X: Trait>() { + // don't lint, but it's questionable that rust requires a cast + take(|| X::fun()); + take(X::fun as fn()); + } +} diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs index 1b53700289d..86abd347baa 100644 --- a/tests/ui/eta.rs +++ b/tests/ui/eta.rs @@ -4,7 +4,6 @@ unused, clippy::no_effect, clippy::redundant_closure_call, - clippy::many_single_char_names, clippy::needless_pass_by_value, clippy::option_map_unit_fn )] @@ -14,7 +13,7 @@ clippy::needless_borrow )] -use std::path::PathBuf; +use std::path::{Path, PathBuf}; macro_rules! mac { () => { @@ -30,7 +29,6 @@ macro_rules! closure_mac { fn main() { let a = Some(1u8).map(|a| foo(a)); - meta(|a| foo(a)); let c = Some(1u8).map(|a| {1+2; foo}(a)); true.then(|| mac!()); // don't lint function in macro expansion Some(1).map(closure_mac!()); // don't lint closure in macro expansion @@ -90,24 +88,17 @@ impl<'a> std::ops::Deref for TestStruct<'a> { fn test_redundant_closures_containing_method_calls() { let i = 10; let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo()); - let e = Some(TestStruct { some_ref: &i }).map(TestStruct::foo); let e = Some(TestStruct { some_ref: &i }).map(|a| a.trait_foo()); let e = Some(TestStruct { some_ref: &i }).map(|a| a.trait_foo_ref()); - let e = Some(TestStruct { some_ref: &i }).map(TestTrait::trait_foo); let e = Some(&mut vec![1, 2, 3]).map(|v| v.clear()); - let e = Some(&mut vec![1, 2, 3]).map(std::vec::Vec::clear); unsafe { let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo_unsafe()); } let e = Some("str").map(|s| s.to_string()); - let e = Some("str").map(str::to_string); let e = Some('a').map(|s| s.to_uppercase()); - let e = Some('a').map(char::to_uppercase); let e: std::vec::Vec<usize> = vec!['a', 'b', 'c'].iter().map(|c| c.len_utf8()).collect(); let e: std::vec::Vec<char> = vec!['a', 'b', 'c'].iter().map(|c| c.to_ascii_uppercase()).collect(); - let e: std::vec::Vec<char> = vec!['a', 'b', 'c'].iter().map(char::to_ascii_uppercase).collect(); - let p = Some(PathBuf::new()); - let e = p.as_ref().and_then(|s| s.to_str()); + let e = Some(PathBuf::new()).as_ref().and_then(|s| s.to_str()); let c = Some(TestStruct { some_ref: &i }) .as_ref() .map(|c| c.to_ascii_uppercase()); @@ -119,10 +110,6 @@ fn test_redundant_closures_containing_method_calls() { t.iter().filter(|x| x.trait_foo_ref()); t.iter().map(|x| x.trait_foo_ref()); } - - let mut some = Some(|x| x * x); - let arr = [Ok(1), Err(2)]; - let _: Vec<_> = arr.iter().map(|x| x.map_err(|e| some.take().unwrap()(e))).collect(); } struct Thunk<T>(Box<dyn FnMut() -> T>); @@ -145,13 +132,6 @@ fn foobar() { thunk.unwrap() } -fn meta<F>(f: F) -where - F: Fn(u8), -{ - f(1u8) -} - fn foo(_: u8) {} fn foo2(_: u8) -> u8 { @@ -236,3 +216,35 @@ fn mutable_closure_in_loop() { Some(1).map(|n| closure(n)); } } + +fn late_bound_lifetimes() { + fn take_asref_path<P: AsRef<Path>>(path: P) {} + + fn map_str<F>(thunk: F) + where + F: FnOnce(&str), + { + } + + fn map_str_to_path<F>(thunk: F) + where + F: FnOnce(&str) -> &Path, + { + } + map_str(|s| take_asref_path(s)); + map_str_to_path(|s| s.as_ref()); +} + +mod type_param_bound { + trait Trait { + fn fun(); + } + + fn take<T: 'static>(_: T) {} + + fn test<X: Trait>() { + // don't lint, but it's questionable that rust requires a cast + take(|| X::fun()); + take(X::fun as fn()); + } +} diff --git a/tests/ui/eta.stderr b/tests/ui/eta.stderr index 28da8941346..8092f04c3fc 100644 --- a/tests/ui/eta.stderr +++ b/tests/ui/eta.stderr @@ -1,5 +1,5 @@ error: redundant closure - --> $DIR/eta.rs:32:27 + --> $DIR/eta.rs:31:27 | LL | let a = Some(1u8).map(|a| foo(a)); | ^^^^^^^^^^ help: replace the closure with the function itself: `foo` @@ -7,19 +7,19 @@ LL | let a = Some(1u8).map(|a| foo(a)); = note: `-D clippy::redundant-closure` implied by `-D warnings` error: redundant closure - --> $DIR/eta.rs:33:10 - | -LL | meta(|a| foo(a)); - | ^^^^^^^^^^ help: replace the closure with the function itself: `foo` - -error: redundant closure - --> $DIR/eta.rs:37:40 + --> $DIR/eta.rs:35:40 | LL | let _: Option<Vec<u8>> = true.then(|| vec![]); // special case vec! | ^^^^^^^^^ help: replace the closure with `Vec::new`: `std::vec::Vec::new` +error: redundant closure + --> $DIR/eta.rs:36:35 + | +LL | let d = Some(1u8).map(|a| foo((|b| foo2(b))(a))); //is adjusted? + | ^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo2` + error: this expression borrows a reference (`&u8`) that is immediately dereferenced by the compiler - --> $DIR/eta.rs:39:21 + --> $DIR/eta.rs:37:21 | LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted | ^^^ help: change this to: `&2` @@ -27,13 +27,25 @@ LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted = note: `-D clippy::needless-borrow` implied by `-D warnings` error: redundant closure - --> $DIR/eta.rs:46:27 + --> $DIR/eta.rs:37:26 + | +LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted + | ^^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `below` + +error: redundant closure + --> $DIR/eta.rs:43:27 + | +LL | let e = Some(1u8).map(|a| divergent(a)); + | ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `divergent` + +error: redundant closure + --> $DIR/eta.rs:44:27 | LL | let e = Some(1u8).map(|a| generic(a)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `generic` error: redundant closure - --> $DIR/eta.rs:92:51 + --> $DIR/eta.rs:90:51 | LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo()); | ^^^^^^^^^^^ help: replace the closure with the method itself: `TestStruct::foo` @@ -41,70 +53,82 @@ LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo()); = note: `-D clippy::redundant-closure-for-method-calls` implied by `-D warnings` error: redundant closure - --> $DIR/eta.rs:94:51 + --> $DIR/eta.rs:91:51 | LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.trait_foo()); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `TestTrait::trait_foo` error: redundant closure - --> $DIR/eta.rs:97:42 + --> $DIR/eta.rs:93:42 | LL | let e = Some(&mut vec![1, 2, 3]).map(|v| v.clear()); | ^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::vec::Vec::clear` error: redundant closure - --> $DIR/eta.rs:102:29 + --> $DIR/eta.rs:97:29 | LL | let e = Some("str").map(|s| s.to_string()); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::string::ToString::to_string` error: redundant closure - --> $DIR/eta.rs:104:27 + --> $DIR/eta.rs:98:27 | LL | let e = Some('a').map(|s| s.to_uppercase()); | ^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_uppercase` error: redundant closure - --> $DIR/eta.rs:107:65 + --> $DIR/eta.rs:100:65 | LL | let e: std::vec::Vec<char> = vec!['a', 'b', 'c'].iter().map(|c| c.to_ascii_uppercase()).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_ascii_uppercase` error: redundant closure - --> $DIR/eta.rs:190:27 + --> $DIR/eta.rs:163:22 + | +LL | requires_fn_once(|| x()); + | ^^^^^^ help: replace the closure with the function itself: `x` + +error: redundant closure + --> $DIR/eta.rs:170:27 | LL | let a = Some(1u8).map(|a| foo_ptr(a)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo_ptr` error: redundant closure - --> $DIR/eta.rs:195:27 + --> $DIR/eta.rs:175:27 | LL | let a = Some(1u8).map(|a| closure(a)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `closure` error: redundant closure - --> $DIR/eta.rs:227:28 + --> $DIR/eta.rs:207:28 | LL | x.into_iter().for_each(|x| add_to_res(x)); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res` error: redundant closure - --> $DIR/eta.rs:228:28 + --> $DIR/eta.rs:208:28 | LL | y.into_iter().for_each(|x| add_to_res(x)); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res` error: redundant closure - --> $DIR/eta.rs:229:28 + --> $DIR/eta.rs:209:28 | LL | z.into_iter().for_each(|x| add_to_res(x)); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `add_to_res` error: redundant closure - --> $DIR/eta.rs:236:21 + --> $DIR/eta.rs:216:21 | LL | Some(1).map(|n| closure(n)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut closure` -error: aborting due to 17 previous errors +error: redundant closure + --> $DIR/eta.rs:235:21 + | +LL | map_str_to_path(|s| s.as_ref()); + | ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::convert::AsRef::as_ref` + +error: aborting due to 21 previous errors diff --git a/tests/ui/eval_order_dependence.rs b/tests/ui/eval_order_dependence.rs index d742856bc41..8e6a32b7be3 100644 --- a/tests/ui/eval_order_dependence.rs +++ b/tests/ui/eval_order_dependence.rs @@ -4,7 +4,6 @@ #[allow( unused_assignments, unused_variables, - clippy::many_single_char_names, clippy::no_effect, dead_code, clippy::blacklisted_name diff --git a/tests/ui/eval_order_dependence.stderr b/tests/ui/eval_order_dependence.stderr index 35eb85e95a3..4f611e308e1 100644 --- a/tests/ui/eval_order_dependence.stderr +++ b/tests/ui/eval_order_dependence.stderr @@ -1,48 +1,48 @@ error: unsequenced read of `x` - --> $DIR/eval_order_dependence.rs:17:9 + --> $DIR/eval_order_dependence.rs:16:9 | LL | } + x; | ^ | = note: `-D clippy::eval-order-dependence` implied by `-D warnings` note: whether read occurs before this write depends on evaluation order - --> $DIR/eval_order_dependence.rs:15:9 + --> $DIR/eval_order_dependence.rs:14:9 | LL | x = 1; | ^^^^^ error: unsequenced read of `x` - --> $DIR/eval_order_dependence.rs:20:5 + --> $DIR/eval_order_dependence.rs:19:5 | LL | x += { | ^ | note: whether read occurs before this write depends on evaluation order - --> $DIR/eval_order_dependence.rs:21:9 + --> $DIR/eval_order_dependence.rs:20:9 | LL | x = 20; | ^^^^^^ error: unsequenced read of `x` - --> $DIR/eval_order_dependence.rs:33:12 + --> $DIR/eval_order_dependence.rs:32:12 | LL | a: x, | ^ | note: whether read occurs before this write depends on evaluation order - --> $DIR/eval_order_dependence.rs:35:13 + --> $DIR/eval_order_dependence.rs:34:13 | LL | x = 6; | ^^^^^ error: unsequenced read of `x` - --> $DIR/eval_order_dependence.rs:42:9 + --> $DIR/eval_order_dependence.rs:41:9 | LL | x += { | ^ | note: whether read occurs before this write depends on evaluation order - --> $DIR/eval_order_dependence.rs:43:13 + --> $DIR/eval_order_dependence.rs:42:13 | LL | x = 20; | ^^^^^^ diff --git a/tests/ui/excessive_precision.fixed b/tests/ui/excessive_precision.fixed index bf0325fec79..90376620a9f 100644 --- a/tests/ui/excessive_precision.fixed +++ b/tests/ui/excessive_precision.fixed @@ -17,8 +17,8 @@ fn main() { const BAD32_3: f32 = 0.1; const BAD32_EDGE: f32 = 1.000_001; - const BAD64_1: f64 = 0.123_456_789_012_345_66_f64; - const BAD64_2: f64 = 0.123_456_789_012_345_66; + const BAD64_1: f64 = 0.123_456_789_012_345_67f64; + const BAD64_2: f64 = 0.123_456_789_012_345_67; const BAD64_3: f64 = 0.1; // Literal as param @@ -37,9 +37,9 @@ fn main() { let bad32_suf: f32 = 1.123_456_8_f32; let bad32_inf = 1.123_456_8_f32; - let bad64: f64 = 0.123_456_789_012_345_66; - let bad64_suf: f64 = 0.123_456_789_012_345_66_f64; - let bad64_inf = 0.123_456_789_012_345_66; + let bad64: f64 = 0.123_456_789_012_345_67; + let bad64_suf: f64 = 0.123_456_789_012_345_67f64; + let bad64_inf = 0.123_456_789_012_345_67; // Vectors let good_vec32: Vec<f32> = vec![0.123_456]; diff --git a/tests/ui/excessive_precision.stderr b/tests/ui/excessive_precision.stderr index 599773f2f70..e59c20c30b4 100644 --- a/tests/ui/excessive_precision.stderr +++ b/tests/ui/excessive_precision.stderr @@ -24,18 +24,6 @@ error: float has excessive precision LL | const BAD32_EDGE: f32 = 1.000_000_9; | ^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.000_001` -error: float has excessive precision - --> $DIR/excessive_precision.rs:20:26 - | -LL | const BAD64_1: f64 = 0.123_456_789_012_345_67f64; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_66_f64` - -error: float has excessive precision - --> $DIR/excessive_precision.rs:21:26 - | -LL | const BAD64_2: f64 = 0.123_456_789_012_345_67; - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_66` - error: float has excessive precision --> $DIR/excessive_precision.rs:22:26 | @@ -66,24 +54,6 @@ error: float has excessive precision LL | let bad32_inf = 1.123_456_789_f32; | ^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.123_456_8_f32` -error: float has excessive precision - --> $DIR/excessive_precision.rs:40:22 - | -LL | let bad64: f64 = 0.123_456_789_012_345_67; - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_66` - -error: float has excessive precision - --> $DIR/excessive_precision.rs:41:26 - | -LL | let bad64_suf: f64 = 0.123_456_789_012_345_67f64; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_66_f64` - -error: float has excessive precision - --> $DIR/excessive_precision.rs:42:21 - | -LL | let bad64_inf = 0.123_456_789_012_345_67; - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `0.123_456_789_012_345_66` - error: float has excessive precision --> $DIR/excessive_precision.rs:48:36 | @@ -108,5 +78,5 @@ error: float has excessive precision LL | let bad_bige32: f32 = 1.123_456_788_888E-10; | ^^^^^^^^^^^^^^^^^^^^^ help: consider changing the type or truncating it to: `1.123_456_8E-10` -error: aborting due to 18 previous errors +error: aborting due to 13 previous errors diff --git a/tests/ui/explicit_deref_methods.fixed b/tests/ui/explicit_deref_methods.fixed index 51d0468e47c..48e2aae75d0 100644 --- a/tests/ui/explicit_deref_methods.fixed +++ b/tests/ui/explicit_deref_methods.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)] +#![allow(unused_variables, clippy::clone_double_ref)] #![warn(clippy::explicit_deref_methods)] use std::ops::{Deref, DerefMut}; diff --git a/tests/ui/explicit_deref_methods.rs b/tests/ui/explicit_deref_methods.rs index 680664bd4f6..d8c8c0c5ca3 100644 --- a/tests/ui/explicit_deref_methods.rs +++ b/tests/ui/explicit_deref_methods.rs @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)] +#![allow(unused_variables, clippy::clone_double_ref)] #![warn(clippy::explicit_deref_methods)] use std::ops::{Deref, DerefMut}; diff --git a/tests/ui/fallible_impl_from.rs b/tests/ui/fallible_impl_from.rs index 5d5af4e4632..495cd97e05e 100644 --- a/tests/ui/fallible_impl_from.rs +++ b/tests/ui/fallible_impl_from.rs @@ -1,4 +1,5 @@ #![deny(clippy::fallible_impl_from)] +#![allow(clippy::if_then_panic)] // docs example struct Foo(i32); diff --git a/tests/ui/fallible_impl_from.stderr b/tests/ui/fallible_impl_from.stderr index 64c8ea85727..8b8054586e6 100644 --- a/tests/ui/fallible_impl_from.stderr +++ b/tests/ui/fallible_impl_from.stderr @@ -1,5 +1,5 @@ error: consider implementing `TryFrom` instead - --> $DIR/fallible_impl_from.rs:5:1 + --> $DIR/fallible_impl_from.rs:6:1 | LL | / impl From<String> for Foo { LL | | fn from(s: String) -> Self { @@ -15,13 +15,13 @@ LL | #![deny(clippy::fallible_impl_from)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ = help: `From` is intended for infallible conversions only. Use `TryFrom` if there's a possibility for the conversion to fail note: potential failure(s) - --> $DIR/fallible_impl_from.rs:7:13 + --> $DIR/fallible_impl_from.rs:8:13 | LL | Foo(s.parse().unwrap()) | ^^^^^^^^^^^^^^^^^^ error: consider implementing `TryFrom` instead - --> $DIR/fallible_impl_from.rs:26:1 + --> $DIR/fallible_impl_from.rs:27:1 | LL | / impl From<usize> for Invalid { LL | | fn from(i: usize) -> Invalid { @@ -34,14 +34,14 @@ LL | | } | = help: `From` is intended for infallible conversions only. Use `TryFrom` if there's a possibility for the conversion to fail note: potential failure(s) - --> $DIR/fallible_impl_from.rs:29:13 + --> $DIR/fallible_impl_from.rs:30:13 | LL | panic!(); | ^^^^^^^^^ = note: this error originates in the macro `$crate::panic::panic_2015` (in Nightly builds, run with -Z macro-backtrace for more info) error: consider implementing `TryFrom` instead - --> $DIR/fallible_impl_from.rs:35:1 + --> $DIR/fallible_impl_from.rs:36:1 | LL | / impl From<Option<String>> for Invalid { LL | | fn from(s: Option<String>) -> Invalid { @@ -54,7 +54,7 @@ LL | | } | = help: `From` is intended for infallible conversions only. Use `TryFrom` if there's a possibility for the conversion to fail note: potential failure(s) - --> $DIR/fallible_impl_from.rs:37:17 + --> $DIR/fallible_impl_from.rs:38:17 | LL | let s = s.unwrap(); | ^^^^^^^^^^ @@ -68,7 +68,7 @@ LL | panic!("{:?}", s); = note: this error originates in the macro `$crate::panic::panic_2015` (in Nightly builds, run with -Z macro-backtrace for more info) error: consider implementing `TryFrom` instead - --> $DIR/fallible_impl_from.rs:53:1 + --> $DIR/fallible_impl_from.rs:54:1 | LL | / impl<'a> From<&'a mut <Box<u32> as ProjStrTrait>::ProjString> for Invalid { LL | | fn from(s: &'a mut <Box<u32> as ProjStrTrait>::ProjString) -> Invalid { @@ -81,7 +81,7 @@ LL | | } | = help: `From` is intended for infallible conversions only. Use `TryFrom` if there's a possibility for the conversion to fail note: potential failure(s) - --> $DIR/fallible_impl_from.rs:55:12 + --> $DIR/fallible_impl_from.rs:56:12 | LL | if s.parse::<u32>().ok().unwrap() != 42 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/float_cmp.rs b/tests/ui/float_cmp.rs index ad5d1a09c03..a34458b9419 100644 --- a/tests/ui/float_cmp.rs +++ b/tests/ui/float_cmp.rs @@ -4,8 +4,7 @@ clippy::no_effect, clippy::op_ref, clippy::unnecessary_operation, - clippy::cast_lossless, - clippy::many_single_char_names + clippy::cast_lossless )] use std::ops::Add; diff --git a/tests/ui/float_cmp.stderr b/tests/ui/float_cmp.stderr index cb5b68b2e95..9cc1f1b75ed 100644 --- a/tests/ui/float_cmp.stderr +++ b/tests/ui/float_cmp.stderr @@ -1,5 +1,5 @@ error: strict comparison of `f32` or `f64` - --> $DIR/float_cmp.rs:58:5 + --> $DIR/float_cmp.rs:57:5 | LL | ONE as f64 != 2.0; | ^^^^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(ONE as f64 - 2.0).abs() > error_margin` @@ -8,7 +8,7 @@ LL | ONE as f64 != 2.0; = note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin` error: strict comparison of `f32` or `f64` - --> $DIR/float_cmp.rs:63:5 + --> $DIR/float_cmp.rs:62:5 | LL | x == 1.0; | ^^^^^^^^ help: consider comparing them within some margin of error: `(x - 1.0).abs() < error_margin` @@ -16,7 +16,7 @@ LL | x == 1.0; = note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin` error: strict comparison of `f32` or `f64` - --> $DIR/float_cmp.rs:66:5 + --> $DIR/float_cmp.rs:65:5 | LL | twice(x) != twice(ONE as f64); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(twice(x) - twice(ONE as f64)).abs() > error_margin` @@ -24,7 +24,7 @@ LL | twice(x) != twice(ONE as f64); = note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin` error: strict comparison of `f32` or `f64` - --> $DIR/float_cmp.rs:86:5 + --> $DIR/float_cmp.rs:85:5 | LL | NON_ZERO_ARRAY[i] == NON_ZERO_ARRAY[j]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(NON_ZERO_ARRAY[i] - NON_ZERO_ARRAY[j]).abs() < error_margin` @@ -32,7 +32,7 @@ LL | NON_ZERO_ARRAY[i] == NON_ZERO_ARRAY[j]; = note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin` error: strict comparison of `f32` or `f64` arrays - --> $DIR/float_cmp.rs:91:5 + --> $DIR/float_cmp.rs:90:5 | LL | a1 == a2; | ^^^^^^^^ @@ -40,7 +40,7 @@ LL | a1 == a2; = note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin` error: strict comparison of `f32` or `f64` - --> $DIR/float_cmp.rs:92:5 + --> $DIR/float_cmp.rs:91:5 | LL | a1[0] == a2[0]; | ^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(a1[0] - a2[0]).abs() < error_margin` diff --git a/tests/ui/for_loop_fixable.fixed b/tests/ui/for_loop_fixable.fixed index f44928d4083..f0e4835415f 100644 --- a/tests/ui/for_loop_fixable.fixed +++ b/tests/ui/for_loop_fixable.fixed @@ -29,7 +29,7 @@ impl Unrelated { clippy::unnecessary_mut_passed, clippy::similar_names )] -#[allow(clippy::many_single_char_names, unused_variables)] +#[allow(unused_variables)] fn main() { let mut vec = vec![1, 2, 3, 4]; diff --git a/tests/ui/for_loop_fixable.rs b/tests/ui/for_loop_fixable.rs index 5b1eb3ee4dc..1edef175fb9 100644 --- a/tests/ui/for_loop_fixable.rs +++ b/tests/ui/for_loop_fixable.rs @@ -29,7 +29,7 @@ impl Unrelated { clippy::unnecessary_mut_passed, clippy::similar_names )] -#[allow(clippy::many_single_char_names, unused_variables)] +#[allow(unused_variables)] fn main() { let mut vec = vec![1, 2, 3, 4]; diff --git a/tests/ui/if_let_some_result.fixed b/tests/ui/if_let_some_result.fixed deleted file mode 100644 index 1bddc47721e..00000000000 --- a/tests/ui/if_let_some_result.fixed +++ /dev/null @@ -1,28 +0,0 @@ -// run-rustfix - -#![warn(clippy::if_let_some_result)] -#![allow(dead_code)] - -fn str_to_int(x: &str) -> i32 { - if let Ok(y) = x.parse() { y } else { 0 } -} - -fn str_to_int_ok(x: &str) -> i32 { - if let Ok(y) = x.parse() { y } else { 0 } -} - -#[rustfmt::skip] -fn strange_some_no_else(x: &str) -> i32 { - { - if let Ok(y) = x . parse() { - return y; - }; - 0 - } -} - -fn negative() { - while let Some(1) = "".parse().ok() {} -} - -fn main() {} diff --git a/tests/ui/if_let_some_result.rs b/tests/ui/if_let_some_result.rs deleted file mode 100644 index d4a52ec9881..00000000000 --- a/tests/ui/if_let_some_result.rs +++ /dev/null @@ -1,28 +0,0 @@ -// run-rustfix - -#![warn(clippy::if_let_some_result)] -#![allow(dead_code)] - -fn str_to_int(x: &str) -> i32 { - if let Some(y) = x.parse().ok() { y } else { 0 } -} - -fn str_to_int_ok(x: &str) -> i32 { - if let Ok(y) = x.parse() { y } else { 0 } -} - -#[rustfmt::skip] -fn strange_some_no_else(x: &str) -> i32 { - { - if let Some(y) = x . parse() . ok () { - return y; - }; - 0 - } -} - -fn negative() { - while let Some(1) = "".parse().ok() {} -} - -fn main() {} diff --git a/tests/ui/if_then_panic.fixed b/tests/ui/if_then_panic.fixed new file mode 100644 index 00000000000..fc57ae0dfa5 --- /dev/null +++ b/tests/ui/if_then_panic.fixed @@ -0,0 +1,34 @@ +// run-rustfix +#![warn(clippy::if_then_panic)] + +fn main() { + let a = vec![1, 2, 3]; + let c = Some(2); + if !a.is_empty() + && a.len() == 3 + && c != None + && !a.is_empty() + && a.len() == 3 + && !a.is_empty() + && a.len() == 3 + && !a.is_empty() + && a.len() == 3 + { + panic!("qaqaq{:?}", a); + } + assert!(a.is_empty(), "qaqaq{:?}", a); + assert!(a.is_empty(), "qwqwq"); + if a.len() == 3 { + println!("qwq"); + println!("qwq"); + println!("qwq"); + } + if let Some(b) = c { + panic!("orz {}", b); + } + if a.len() == 3 { + panic!("qaqaq"); + } else { + println!("qwq"); + } +} diff --git a/tests/ui/if_then_panic.rs b/tests/ui/if_then_panic.rs new file mode 100644 index 00000000000..d1ac93d8d41 --- /dev/null +++ b/tests/ui/if_then_panic.rs @@ -0,0 +1,38 @@ +// run-rustfix +#![warn(clippy::if_then_panic)] + +fn main() { + let a = vec![1, 2, 3]; + let c = Some(2); + if !a.is_empty() + && a.len() == 3 + && c != None + && !a.is_empty() + && a.len() == 3 + && !a.is_empty() + && a.len() == 3 + && !a.is_empty() + && a.len() == 3 + { + panic!("qaqaq{:?}", a); + } + if !a.is_empty() { + panic!("qaqaq{:?}", a); + } + if !a.is_empty() { + panic!("qwqwq"); + } + if a.len() == 3 { + println!("qwq"); + println!("qwq"); + println!("qwq"); + } + if let Some(b) = c { + panic!("orz {}", b); + } + if a.len() == 3 { + panic!("qaqaq"); + } else { + println!("qwq"); + } +} diff --git a/tests/ui/if_then_panic.stderr b/tests/ui/if_then_panic.stderr new file mode 100644 index 00000000000..b92c9bdf674 --- /dev/null +++ b/tests/ui/if_then_panic.stderr @@ -0,0 +1,20 @@ +error: only a `panic!` in `if`-then statement + --> $DIR/if_then_panic.rs:19:5 + | +LL | / if !a.is_empty() { +LL | | panic!("qaqaq{:?}", a); +LL | | } + | |_____^ help: try: `assert!(a.is_empty(), "qaqaq{:?}", a);` + | + = note: `-D clippy::if-then-panic` implied by `-D warnings` + +error: only a `panic!` in `if`-then statement + --> $DIR/if_then_panic.rs:22:5 + | +LL | / if !a.is_empty() { +LL | | panic!("qwqwq"); +LL | | } + | |_____^ help: try: `assert!(a.is_empty(), "qwqwq");` + +error: aborting due to 2 previous errors + diff --git a/tests/ui/infinite_loop.rs b/tests/ui/infinite_loop.rs index e518b2677b7..e86bd7bcf4f 100644 --- a/tests/ui/infinite_loop.rs +++ b/tests/ui/infinite_loop.rs @@ -16,7 +16,6 @@ fn foob() -> bool { unimplemented!() } -#[allow(clippy::many_single_char_names)] fn immutable_condition() { // Should warn when all vars mentioned are immutable let y = 0; diff --git a/tests/ui/infinite_loop.stderr b/tests/ui/infinite_loop.stderr index 2736753c14b..69309b0da87 100644 --- a/tests/ui/infinite_loop.stderr +++ b/tests/ui/infinite_loop.stderr @@ -1,5 +1,5 @@ error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:23:11 + --> $DIR/infinite_loop.rs:22:11 | LL | while y < 10 { | ^^^^^^ @@ -8,7 +8,7 @@ LL | while y < 10 { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:28:11 + --> $DIR/infinite_loop.rs:27:11 | LL | while y < 10 && x < 3 { | ^^^^^^^^^^^^^^^ @@ -16,7 +16,7 @@ LL | while y < 10 && x < 3 { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:35:11 + --> $DIR/infinite_loop.rs:34:11 | LL | while !cond { | ^^^^^ @@ -24,7 +24,7 @@ LL | while !cond { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:79:11 + --> $DIR/infinite_loop.rs:78:11 | LL | while i < 3 { | ^^^^^ @@ -32,7 +32,7 @@ LL | while i < 3 { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:84:11 + --> $DIR/infinite_loop.rs:83:11 | LL | while i < 3 && j > 0 { | ^^^^^^^^^^^^^^ @@ -40,7 +40,7 @@ LL | while i < 3 && j > 0 { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:88:11 + --> $DIR/infinite_loop.rs:87:11 | LL | while i < 3 { | ^^^^^ @@ -48,7 +48,7 @@ LL | while i < 3 { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:103:11 + --> $DIR/infinite_loop.rs:102:11 | LL | while i < 3 { | ^^^^^ @@ -56,7 +56,7 @@ LL | while i < 3 { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:108:11 + --> $DIR/infinite_loop.rs:107:11 | LL | while i < 3 { | ^^^^^ @@ -64,7 +64,7 @@ LL | while i < 3 { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:174:15 + --> $DIR/infinite_loop.rs:173:15 | LL | while self.count < n { | ^^^^^^^^^^^^^^ @@ -72,7 +72,7 @@ LL | while self.count < n { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:182:11 + --> $DIR/infinite_loop.rs:181:11 | LL | while y < 10 { | ^^^^^^ @@ -82,7 +82,7 @@ LL | while y < 10 { = help: rewrite it as `if cond { loop { } }` error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:189:11 + --> $DIR/infinite_loop.rs:188:11 | LL | while y < 10 { | ^^^^^^ diff --git a/tests/ui/inherent_to_string.rs b/tests/ui/inherent_to_string.rs index 6e65fdbd04e..aeb0a0c1e2e 100644 --- a/tests/ui/inherent_to_string.rs +++ b/tests/ui/inherent_to_string.rs @@ -1,6 +1,5 @@ #![warn(clippy::inherent_to_string)] #![deny(clippy::inherent_to_string_shadow_display)] -#![allow(clippy::many_single_char_names)] use std::fmt; diff --git a/tests/ui/inherent_to_string.stderr b/tests/ui/inherent_to_string.stderr index f5fcc193b4d..4f331f5bec9 100644 --- a/tests/ui/inherent_to_string.stderr +++ b/tests/ui/inherent_to_string.stderr @@ -1,5 +1,5 @@ error: implementation of inherent method `to_string(&self) -> String` for type `A` - --> $DIR/inherent_to_string.rs:21:5 + --> $DIR/inherent_to_string.rs:20:5 | LL | / fn to_string(&self) -> String { LL | | "A.to_string()".to_string() @@ -10,7 +10,7 @@ LL | | } = help: implement trait `Display` for type `A` instead error: type `C` implements inherent method `to_string(&self) -> String` which shadows the implementation of `Display` - --> $DIR/inherent_to_string.rs:45:5 + --> $DIR/inherent_to_string.rs:44:5 | LL | / fn to_string(&self) -> String { LL | | "C.to_string()".to_string() diff --git a/tests/ui/iter_not_returning_iterator.rs b/tests/ui/iter_not_returning_iterator.rs new file mode 100644 index 00000000000..377f760b3c4 --- /dev/null +++ b/tests/ui/iter_not_returning_iterator.rs @@ -0,0 +1,47 @@ +#![warn(clippy::iter_not_returning_iterator)] + +struct Data { + begin: u32, +} + +struct Counter { + count: u32, +} + +impl Data { + fn iter(&self) -> Counter { + todo!() + } + + fn iter_mut(&self) -> Counter { + todo!() + } +} + +struct Data2 { + begin: u32, +} + +struct Counter2 { + count: u32, +} + +impl Data2 { + fn iter(&self) -> Counter2 { + todo!() + } + + fn iter_mut(&self) -> Counter2 { + todo!() + } +} + +impl Iterator for Counter { + type Item = u32; + + fn next(&mut self) -> Option<Self::Item> { + todo!() + } +} + +fn main() {} diff --git a/tests/ui/iter_not_returning_iterator.stderr b/tests/ui/iter_not_returning_iterator.stderr new file mode 100644 index 00000000000..2273cd0be66 --- /dev/null +++ b/tests/ui/iter_not_returning_iterator.stderr @@ -0,0 +1,16 @@ +error: this method is named `iter` but its return type does not implement `Iterator` + --> $DIR/iter_not_returning_iterator.rs:30:5 + | +LL | fn iter(&self) -> Counter2 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::iter-not-returning-iterator` implied by `-D warnings` + +error: this method is named `iter_mut` but its return type does not implement `Iterator` + --> $DIR/iter_not_returning_iterator.rs:34:5 + | +LL | fn iter_mut(&self) -> Counter2 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/tests/ui/logic_bug.rs b/tests/ui/logic_bug.rs index a01c6ef99db..4eaa2dd98eb 100644 --- a/tests/ui/logic_bug.rs +++ b/tests/ui/logic_bug.rs @@ -1,4 +1,4 @@ -#![allow(unused, clippy::many_single_char_names, clippy::diverging_sub_expression)] +#![allow(unused, clippy::diverging_sub_expression)] #![warn(clippy::logic_bug)] fn main() { diff --git a/tests/ui/manual_split_once.fixed b/tests/ui/manual_split_once.fixed index 3a0332939d4..992baf1f185 100644 --- a/tests/ui/manual_split_once.fixed +++ b/tests/ui/manual_split_once.fixed @@ -36,6 +36,12 @@ fn main() { // Don't lint, slices don't have `split_once` let _ = [0, 1, 2].splitn(2, |&x| x == 1).nth(1).unwrap(); + + // `rsplitn` gives the results in the reverse order of `rsplit_once` + let _ = "key=value".rsplit_once('=').unwrap().1; + let _ = "key=value".rsplit_once('=').map_or("key=value", |x| x.0); + let _ = "key=value".rsplit_once('=').map(|x| x.1); + let (_, _) = "key=value".rsplit_once('=').map(|(x, y)| (y, x)).unwrap(); } fn _msrv_1_51() { diff --git a/tests/ui/manual_split_once.rs b/tests/ui/manual_split_once.rs index e6093b63fe8..4f92ab6b812 100644 --- a/tests/ui/manual_split_once.rs +++ b/tests/ui/manual_split_once.rs @@ -36,6 +36,12 @@ fn main() { // Don't lint, slices don't have `split_once` let _ = [0, 1, 2].splitn(2, |&x| x == 1).nth(1).unwrap(); + + // `rsplitn` gives the results in the reverse order of `rsplit_once` + let _ = "key=value".rsplitn(2, '=').next().unwrap(); + let _ = "key=value".rsplitn(2, '=').nth(1).unwrap(); + let _ = "key=value".rsplitn(2, '=').nth(0); + let (_, _) = "key=value".rsplitn(2, '=').next_tuple().unwrap(); } fn _msrv_1_51() { diff --git a/tests/ui/manual_split_once.stderr b/tests/ui/manual_split_once.stderr index 4f15196b469..7bea2303d92 100644 --- a/tests/ui/manual_split_once.stderr +++ b/tests/ui/manual_split_once.stderr @@ -72,11 +72,35 @@ error: manual implementation of `split_once` LL | let _ = s.splitn(2, "key=value").skip(1).next()?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.split_once("key=value")?.1` +error: manual implementation of `rsplit_once` + --> $DIR/manual_split_once.rs:41:13 + | +LL | let _ = "key=value".rsplitn(2, '=').next().unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".rsplit_once('=').unwrap().1` + +error: manual implementation of `rsplit_once` + --> $DIR/manual_split_once.rs:42:13 + | +LL | let _ = "key=value".rsplitn(2, '=').nth(1).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".rsplit_once('=').map_or("key=value", |x| x.0)` + +error: manual implementation of `rsplit_once` + --> $DIR/manual_split_once.rs:43:13 + | +LL | let _ = "key=value".rsplitn(2, '=').nth(0); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".rsplit_once('=').map(|x| x.1)` + +error: manual implementation of `rsplit_once` + --> $DIR/manual_split_once.rs:44:18 + | +LL | let (_, _) = "key=value".rsplitn(2, '=').next_tuple().unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".rsplit_once('=').map(|(x, y)| (y, x))` + error: manual implementation of `split_once` - --> $DIR/manual_split_once.rs:49:13 + --> $DIR/manual_split_once.rs:55:13 | LL | let _ = "key=value".splitn(2, '=').nth(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".split_once('=').unwrap().1` -error: aborting due to 13 previous errors +error: aborting due to 17 previous errors diff --git a/tests/ui/many_single_char_names.rs b/tests/ui/many_single_char_names.rs index 80800e48724..65769819110 100644 --- a/tests/ui/many_single_char_names.rs +++ b/tests/ui/many_single_char_names.rs @@ -1,4 +1,4 @@ -#[warn(clippy::many_single_char_names)] +#![warn(clippy::many_single_char_names)] fn bla() { let a: i32; diff --git a/tests/ui/map_flatten.fixed b/tests/ui/map_flatten.fixed index 18846c898da..fec3a95edd6 100644 --- a/tests/ui/map_flatten.fixed +++ b/tests/ui/map_flatten.fixed @@ -4,6 +4,7 @@ #![allow(clippy::let_underscore_drop)] #![allow(clippy::missing_docs_in_private_items)] #![allow(clippy::map_identity)] +#![allow(clippy::redundant_closure)] #![allow(clippy::unnecessary_wraps)] #![feature(result_flattening)] diff --git a/tests/ui/map_flatten.rs b/tests/ui/map_flatten.rs index 01db27876da..aa1f76e335a 100644 --- a/tests/ui/map_flatten.rs +++ b/tests/ui/map_flatten.rs @@ -4,6 +4,7 @@ #![allow(clippy::let_underscore_drop)] #![allow(clippy::missing_docs_in_private_items)] #![allow(clippy::map_identity)] +#![allow(clippy::redundant_closure)] #![allow(clippy::unnecessary_wraps)] #![feature(result_flattening)] diff --git a/tests/ui/map_flatten.stderr b/tests/ui/map_flatten.stderr index 38457c8ea4d..bcd2047e6fa 100644 --- a/tests/ui/map_flatten.stderr +++ b/tests/ui/map_flatten.stderr @@ -1,5 +1,5 @@ error: called `map(..).flatten()` on an `Iterator` - --> $DIR/map_flatten.rs:17:46 + --> $DIR/map_flatten.rs:18:46 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id)` @@ -7,37 +7,37 @@ LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().coll = note: `-D clippy::map-flatten` implied by `-D warnings` error: called `map(..).flatten()` on an `Iterator` - --> $DIR/map_flatten.rs:18:46 + --> $DIR/map_flatten.rs:19:46 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_ref).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id_ref)` error: called `map(..).flatten()` on an `Iterator` - --> $DIR/map_flatten.rs:19:46 + --> $DIR/map_flatten.rs:20:46 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id_closure)` error: called `map(..).flatten()` on an `Iterator` - --> $DIR/map_flatten.rs:20:46 + --> $DIR/map_flatten.rs:21:46 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(|x| x.checked_add(1))` error: called `map(..).flatten()` on an `Iterator` - --> $DIR/map_flatten.rs:23:46 + --> $DIR/map_flatten.rs:24:46 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `.flat_map(|x| 0..x)` error: called `map(..).flatten()` on an `Option` - --> $DIR/map_flatten.rs:26:39 + --> $DIR/map_flatten.rs:27:39 | LL | let _: Option<_> = (Some(Some(1))).map(|x| x).flatten(); | ^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `.and_then(|x| x)` error: called `map(..).flatten()` on an `Result` - --> $DIR/map_flatten.rs:29:41 + --> $DIR/map_flatten.rs:30:41 | LL | let _: Result<_, &str> = (Ok(Ok(1))).map(|x| x).flatten(); | ^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `.and_then(|x| x)` diff --git a/tests/ui/match_result_ok.fixed b/tests/ui/match_result_ok.fixed new file mode 100644 index 00000000000..d4760a97567 --- /dev/null +++ b/tests/ui/match_result_ok.fixed @@ -0,0 +1,63 @@ +// run-rustfix + +#![warn(clippy::match_result_ok)] +#![allow(clippy::boxed_local)] +#![allow(dead_code)] + +// Checking `if` cases + +fn str_to_int(x: &str) -> i32 { + if let Ok(y) = x.parse() { y } else { 0 } +} + +fn str_to_int_ok(x: &str) -> i32 { + if let Ok(y) = x.parse() { y } else { 0 } +} + +#[rustfmt::skip] +fn strange_some_no_else(x: &str) -> i32 { + { + if let Ok(y) = x . parse() { + return y; + }; + 0 + } +} + +// Checking `while` cases + +struct Wat { + counter: i32, +} + +impl Wat { + fn next(&mut self) -> Result<i32, &str> { + self.counter += 1; + if self.counter < 5 { + Ok(self.counter) + } else { + Err("Oh no") + } + } +} + +fn base_1(x: i32) { + let mut wat = Wat { counter: x }; + while let Ok(a) = wat.next() { + println!("{}", a); + } +} + +fn base_2(x: i32) { + let mut wat = Wat { counter: x }; + while let Ok(a) = wat.next() { + println!("{}", a); + } +} + +fn base_3(test_func: Box<Result<i32, &str>>) { + // Expected to stay as is + while let Some(_b) = test_func.ok() {} +} + +fn main() {} diff --git a/tests/ui/match_result_ok.rs b/tests/ui/match_result_ok.rs new file mode 100644 index 00000000000..0b818723d98 --- /dev/null +++ b/tests/ui/match_result_ok.rs @@ -0,0 +1,63 @@ +// run-rustfix + +#![warn(clippy::match_result_ok)] +#![allow(clippy::boxed_local)] +#![allow(dead_code)] + +// Checking `if` cases + +fn str_to_int(x: &str) -> i32 { + if let Some(y) = x.parse().ok() { y } else { 0 } +} + +fn str_to_int_ok(x: &str) -> i32 { + if let Ok(y) = x.parse() { y } else { 0 } +} + +#[rustfmt::skip] +fn strange_some_no_else(x: &str) -> i32 { + { + if let Some(y) = x . parse() . ok () { + return y; + }; + 0 + } +} + +// Checking `while` cases + +struct Wat { + counter: i32, +} + +impl Wat { + fn next(&mut self) -> Result<i32, &str> { + self.counter += 1; + if self.counter < 5 { + Ok(self.counter) + } else { + Err("Oh no") + } + } +} + +fn base_1(x: i32) { + let mut wat = Wat { counter: x }; + while let Some(a) = wat.next().ok() { + println!("{}", a); + } +} + +fn base_2(x: i32) { + let mut wat = Wat { counter: x }; + while let Ok(a) = wat.next() { + println!("{}", a); + } +} + +fn base_3(test_func: Box<Result<i32, &str>>) { + // Expected to stay as is + while let Some(_b) = test_func.ok() {} +} + +fn main() {} diff --git a/tests/ui/if_let_some_result.stderr b/tests/ui/match_result_ok.stderr similarity index 56% rename from tests/ui/if_let_some_result.stderr rename to tests/ui/match_result_ok.stderr index bc3a5e7698d..cc3bc8c76ff 100644 --- a/tests/ui/if_let_some_result.stderr +++ b/tests/ui/match_result_ok.stderr @@ -1,17 +1,17 @@ error: matching on `Some` with `ok()` is redundant - --> $DIR/if_let_some_result.rs:7:5 + --> $DIR/match_result_ok.rs:10:5 | LL | if let Some(y) = x.parse().ok() { y } else { 0 } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: `-D clippy::if-let-some-result` implied by `-D warnings` + = note: `-D clippy::match-result-ok` implied by `-D warnings` help: consider matching on `Ok(y)` and removing the call to `ok` instead | LL | if let Ok(y) = x.parse() { y } else { 0 } | ~~~~~~~~~~~~~~~~~~~~~~~~ error: matching on `Some` with `ok()` is redundant - --> $DIR/if_let_some_result.rs:17:9 + --> $DIR/match_result_ok.rs:20:9 | LL | if let Some(y) = x . parse() . ok () { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -21,5 +21,16 @@ help: consider matching on `Ok(y)` and removing the call to `ok` instead LL | if let Ok(y) = x . parse() { | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -error: aborting due to 2 previous errors +error: matching on `Some` with `ok()` is redundant + --> $DIR/match_result_ok.rs:46:5 + | +LL | while let Some(a) = wat.next().ok() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider matching on `Ok(a)` and removing the call to `ok` instead + | +LL | while let Ok(a) = wat.next() { + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to 3 previous errors diff --git a/tests/ui/match_single_binding.fixed b/tests/ui/match_single_binding.fixed index 30bf6402253..b4ec525ada0 100644 --- a/tests/ui/match_single_binding.fixed +++ b/tests/ui/match_single_binding.fixed @@ -1,7 +1,7 @@ // run-rustfix #![warn(clippy::match_single_binding)] -#![allow(unused_variables, clippy::many_single_char_names, clippy::toplevel_ref_arg)] +#![allow(unused_variables, clippy::toplevel_ref_arg)] struct Point { x: i32, diff --git a/tests/ui/match_single_binding.rs b/tests/ui/match_single_binding.rs index d8bb80d8b96..e04c4018b98 100644 --- a/tests/ui/match_single_binding.rs +++ b/tests/ui/match_single_binding.rs @@ -1,7 +1,7 @@ // run-rustfix #![warn(clippy::match_single_binding)] -#![allow(unused_variables, clippy::many_single_char_names, clippy::toplevel_ref_arg)] +#![allow(unused_variables, clippy::toplevel_ref_arg)] struct Point { x: i32, diff --git a/tests/ui/mut_key.rs b/tests/ui/mut_key.rs index 2d227e6654c..1c0ba664580 100644 --- a/tests/ui/mut_key.rs +++ b/tests/ui/mut_key.rs @@ -1,6 +1,9 @@ -use std::collections::{HashMap, HashSet}; +use std::cell::Cell; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::hash::{Hash, Hasher}; +use std::rc::Rc; use std::sync::atomic::{AtomicUsize, Ordering::Relaxed}; +use std::sync::Arc; struct Key(AtomicUsize); @@ -31,11 +34,19 @@ fn should_not_take_this_arg(m: &mut HashMap<Key, usize>, _n: usize) -> HashSet<K fn this_is_ok(_m: &mut HashMap<usize, Key>) {} +// Raw pointers are hashed by the address they point to, so it doesn't matter if they point to a +// type with interior mutability. See: +// - clippy issue: https://github.com/rust-lang/rust-clippy/issues/6745 +// - std lib: https://github.com/rust-lang/rust/blob/1.54.0/library/core/src/hash/mod.rs#L717-L736 +// So these are OK: +fn raw_ptr_is_ok(_m: &mut HashMap<*const Key, ()>) {} +fn raw_mut_ptr_is_ok(_m: &mut HashMap<*mut Key, ()>) {} + #[allow(unused)] trait Trait { type AssociatedType; - fn trait_fn(&self, set: std::collections::HashSet<Self::AssociatedType>); + fn trait_fn(&self, set: HashSet<Self::AssociatedType>); } fn generics_are_ok_too<K>(_m: &mut HashSet<K>) { @@ -52,4 +63,23 @@ fn main() { tuples::<Key>(&mut HashMap::new()); tuples::<()>(&mut HashMap::new()); tuples_bad::<()>(&mut HashMap::new()); + + raw_ptr_is_ok(&mut HashMap::new()); + raw_mut_ptr_is_ok(&mut HashMap::new()); + + let _map = HashMap::<Cell<usize>, usize>::new(); + let _map = HashMap::<&mut Cell<usize>, usize>::new(); + let _map = HashMap::<&mut usize, usize>::new(); + // Collection types from `std` who's impl of `Hash` or `Ord` delegate their type parameters + let _map = HashMap::<Vec<Cell<usize>>, usize>::new(); + let _map = HashMap::<BTreeMap<Cell<usize>, ()>, usize>::new(); + let _map = HashMap::<BTreeMap<(), Cell<usize>>, usize>::new(); + let _map = HashMap::<BTreeSet<Cell<usize>>, usize>::new(); + let _map = HashMap::<Option<Cell<usize>>, usize>::new(); + let _map = HashMap::<Option<Vec<Cell<usize>>>, usize>::new(); + let _map = HashMap::<Result<&mut usize, ()>, usize>::new(); + // Smart pointers from `std` who's impl of `Hash` or `Ord` delegate their type parameters + let _map = HashMap::<Box<Cell<usize>>, usize>::new(); + let _map = HashMap::<Rc<Cell<usize>>, usize>::new(); + let _map = HashMap::<Arc<Cell<usize>>, usize>::new(); } diff --git a/tests/ui/mut_key.stderr b/tests/ui/mut_key.stderr index a8460b06ca6..25dd029b16e 100644 --- a/tests/ui/mut_key.stderr +++ b/tests/ui/mut_key.stderr @@ -1,5 +1,5 @@ error: mutable key type - --> $DIR/mut_key.rs:27:32 + --> $DIR/mut_key.rs:30:32 | LL | fn should_not_take_this_arg(m: &mut HashMap<Key, usize>, _n: usize) -> HashSet<Key> { | ^^^^^^^^^^^^^^^^^^^^^^^^ @@ -7,22 +7,100 @@ LL | fn should_not_take_this_arg(m: &mut HashMap<Key, usize>, _n: usize) -> Hash = note: `-D clippy::mutable-key-type` implied by `-D warnings` error: mutable key type - --> $DIR/mut_key.rs:27:72 + --> $DIR/mut_key.rs:30:72 | LL | fn should_not_take_this_arg(m: &mut HashMap<Key, usize>, _n: usize) -> HashSet<Key> { | ^^^^^^^^^^^^ error: mutable key type - --> $DIR/mut_key.rs:28:5 + --> $DIR/mut_key.rs:31:5 | LL | let _other: HashMap<Key, bool> = HashMap::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: mutable key type - --> $DIR/mut_key.rs:47:22 + --> $DIR/mut_key.rs:58:22 | LL | fn tuples_bad<U>(_m: &mut HashMap<(Key, U), bool>) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 4 previous errors +error: mutable key type + --> $DIR/mut_key.rs:70:5 + | +LL | let _map = HashMap::<Cell<usize>, usize>::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: mutable key type + --> $DIR/mut_key.rs:71:5 + | +LL | let _map = HashMap::<&mut Cell<usize>, usize>::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: mutable key type + --> $DIR/mut_key.rs:72:5 + | +LL | let _map = HashMap::<&mut usize, usize>::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: mutable key type + --> $DIR/mut_key.rs:74:5 + | +LL | let _map = HashMap::<Vec<Cell<usize>>, usize>::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: mutable key type + --> $DIR/mut_key.rs:75:5 + | +LL | let _map = HashMap::<BTreeMap<Cell<usize>, ()>, usize>::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: mutable key type + --> $DIR/mut_key.rs:76:5 + | +LL | let _map = HashMap::<BTreeMap<(), Cell<usize>>, usize>::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: mutable key type + --> $DIR/mut_key.rs:77:5 + | +LL | let _map = HashMap::<BTreeSet<Cell<usize>>, usize>::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: mutable key type + --> $DIR/mut_key.rs:78:5 + | +LL | let _map = HashMap::<Option<Cell<usize>>, usize>::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: mutable key type + --> $DIR/mut_key.rs:79:5 + | +LL | let _map = HashMap::<Option<Vec<Cell<usize>>>, usize>::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: mutable key type + --> $DIR/mut_key.rs:80:5 + | +LL | let _map = HashMap::<Result<&mut usize, ()>, usize>::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: mutable key type + --> $DIR/mut_key.rs:82:5 + | +LL | let _map = HashMap::<Box<Cell<usize>>, usize>::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: mutable key type + --> $DIR/mut_key.rs:83:5 + | +LL | let _map = HashMap::<Rc<Cell<usize>>, usize>::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: mutable key type + --> $DIR/mut_key.rs:84:5 + | +LL | let _map = HashMap::<Arc<Cell<usize>>, usize>::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 17 previous errors diff --git a/tests/ui/needless_borrow.fixed b/tests/ui/needless_borrow.fixed index a87171dc3f2..42c2bb9f414 100644 --- a/tests/ui/needless_borrow.fixed +++ b/tests/ui/needless_borrow.fixed @@ -1,17 +1,16 @@ // run-rustfix -#![allow(clippy::needless_borrowed_reference)] - -fn x(y: &i32) -> i32 { - *y -} - #[warn(clippy::all, clippy::needless_borrow)] #[allow(unused_variables)] fn main() { let a = 5; - let b = x(&a); - let c = x(&a); + let _ = x(&a); // no warning + let _ = x(&a); // warn + + let mut b = 5; + mut_ref(&mut b); // no warning + mut_ref(&mut b); // warn + let s = &String::from("hi"); let s_ident = f(&s); // should not error, because `&String` implements Copy, but `String` does not let g_val = g(&Vec::new()); // should not error, because `&Vec<T>` derefs to `&[T]` @@ -29,6 +28,15 @@ fn main() { }; } +#[allow(clippy::needless_borrowed_reference)] +fn x(y: &i32) -> i32 { + *y +} + +fn mut_ref(y: &mut i32) { + *y = 5; +} + fn f<T: Copy>(y: &T) -> T { *y } diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index 059dc8ceac3..31977416bc7 100644 --- a/tests/ui/needless_borrow.rs +++ b/tests/ui/needless_borrow.rs @@ -1,17 +1,16 @@ // run-rustfix -#![allow(clippy::needless_borrowed_reference)] - -fn x(y: &i32) -> i32 { - *y -} - #[warn(clippy::all, clippy::needless_borrow)] #[allow(unused_variables)] fn main() { let a = 5; - let b = x(&a); - let c = x(&&a); + let _ = x(&a); // no warning + let _ = x(&&a); // warn + + let mut b = 5; + mut_ref(&mut b); // no warning + mut_ref(&mut &mut b); // warn + let s = &String::from("hi"); let s_ident = f(&s); // should not error, because `&String` implements Copy, but `String` does not let g_val = g(&Vec::new()); // should not error, because `&Vec<T>` derefs to `&[T]` @@ -29,6 +28,15 @@ fn main() { }; } +#[allow(clippy::needless_borrowed_reference)] +fn x(y: &i32) -> i32 { + *y +} + +fn mut_ref(y: &mut i32) { + *y = 5; +} + fn f<T: Copy>(y: &T) -> T { *y } diff --git a/tests/ui/needless_borrow.stderr b/tests/ui/needless_borrow.stderr index 6eddf26db06..012d62e2871 100644 --- a/tests/ui/needless_borrow.stderr +++ b/tests/ui/needless_borrow.stderr @@ -1,16 +1,22 @@ error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:14:15 + --> $DIR/needless_borrow.rs:8:15 | -LL | let c = x(&&a); +LL | let _ = x(&&a); // warn | ^^^ help: change this to: `&a` | = note: `-D clippy::needless-borrow` implied by `-D warnings` +error: this expression borrows a reference (`&mut i32`) that is immediately dereferenced by the compiler + --> $DIR/needless_borrow.rs:12:13 + | +LL | mut_ref(&mut &mut b); // warn + | ^^^^^^^^^^^ help: change this to: `&mut b` + error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:27:15 + --> $DIR/needless_borrow.rs:26:15 | LL | 46 => &&a, | ^^^ help: change this to: `&a` -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors diff --git a/tests/ui/needless_pass_by_value.rs b/tests/ui/needless_pass_by_value.rs index 7a9ba55590d..5a35b100afe 100644 --- a/tests/ui/needless_pass_by_value.rs +++ b/tests/ui/needless_pass_by_value.rs @@ -3,7 +3,6 @@ dead_code, clippy::single_match, clippy::redundant_pattern_matching, - clippy::many_single_char_names, clippy::option_option, clippy::redundant_clone )] diff --git a/tests/ui/needless_pass_by_value.stderr b/tests/ui/needless_pass_by_value.stderr index 2f61ba241c4..d960c86a9f0 100644 --- a/tests/ui/needless_pass_by_value.stderr +++ b/tests/ui/needless_pass_by_value.stderr @@ -1,5 +1,5 @@ error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:18:23 + --> $DIR/needless_pass_by_value.rs:17:23 | LL | fn foo<T: Default>(v: Vec<T>, w: Vec<T>, mut x: Vec<T>, y: Vec<T>) -> Vec<T> { | ^^^^^^ help: consider changing the type to: `&[T]` @@ -7,55 +7,55 @@ LL | fn foo<T: Default>(v: Vec<T>, w: Vec<T>, mut x: Vec<T>, y: Vec<T>) -> Vec<T = note: `-D clippy::needless-pass-by-value` implied by `-D warnings` error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:32:11 + --> $DIR/needless_pass_by_value.rs:31:11 | LL | fn bar(x: String, y: Wrapper) { | ^^^^^^ help: consider changing the type to: `&str` error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:32:22 + --> $DIR/needless_pass_by_value.rs:31:22 | LL | fn bar(x: String, y: Wrapper) { | ^^^^^^^ help: consider taking a reference instead: `&Wrapper` error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:38:71 + --> $DIR/needless_pass_by_value.rs:37:71 | LL | fn test_borrow_trait<T: Borrow<str>, U: AsRef<str>, V>(t: T, u: U, v: V) { | ^ help: consider taking a reference instead: `&V` error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:50:18 + --> $DIR/needless_pass_by_value.rs:49:18 | LL | fn test_match(x: Option<Option<String>>, y: Option<Option<String>>) { | ^^^^^^^^^^^^^^^^^^^^^^ help: consider taking a reference instead: `&Option<Option<String>>` error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:63:24 + --> $DIR/needless_pass_by_value.rs:62:24 | LL | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) { | ^^^^^^^ help: consider taking a reference instead: `&Wrapper` error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:63:36 + --> $DIR/needless_pass_by_value.rs:62:36 | LL | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) { | ^^^^^^^ help: consider taking a reference instead: `&Wrapper` error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:79:49 + --> $DIR/needless_pass_by_value.rs:78:49 | LL | fn test_blanket_ref<T: Foo, S: Serialize>(_foo: T, _serializable: S) {} | ^ help: consider taking a reference instead: `&T` error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:81:18 + --> $DIR/needless_pass_by_value.rs:80:18 | LL | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) { | ^^^^^^ help: consider taking a reference instead: `&String` error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:81:29 + --> $DIR/needless_pass_by_value.rs:80:29 | LL | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) { | ^^^^^^ @@ -70,13 +70,13 @@ LL | let _ = t.to_string(); | ~~~~~~~~~~~~~ error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:81:40 + --> $DIR/needless_pass_by_value.rs:80:40 | LL | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) { | ^^^^^^^^ help: consider taking a reference instead: `&Vec<i32>` error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:81:53 + --> $DIR/needless_pass_by_value.rs:80:53 | LL | fn issue_2114(s: String, t: String, u: Vec<i32>, v: Vec<i32>) { | ^^^^^^^^ @@ -91,85 +91,85 @@ LL | let _ = v.to_owned(); | ~~~~~~~~~~~~ error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:94:12 + --> $DIR/needless_pass_by_value.rs:93:12 | LL | s: String, | ^^^^^^ help: consider changing the type to: `&str` error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:95:12 + --> $DIR/needless_pass_by_value.rs:94:12 | LL | t: String, | ^^^^^^ help: consider taking a reference instead: `&String` error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:104:23 + --> $DIR/needless_pass_by_value.rs:103:23 | LL | fn baz(&self, _u: U, _s: Self) {} | ^ help: consider taking a reference instead: `&U` error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:104:30 + --> $DIR/needless_pass_by_value.rs:103:30 | LL | fn baz(&self, _u: U, _s: Self) {} | ^^^^ help: consider taking a reference instead: `&Self` error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:126:24 + --> $DIR/needless_pass_by_value.rs:125:24 | LL | fn bar_copy(x: u32, y: CopyWrapper) { | ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper` | help: consider marking this type as `Copy` - --> $DIR/needless_pass_by_value.rs:124:1 + --> $DIR/needless_pass_by_value.rs:123:1 | LL | struct CopyWrapper(u32); | ^^^^^^^^^^^^^^^^^^^^^^^^ error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:132:29 + --> $DIR/needless_pass_by_value.rs:131:29 | LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) { | ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper` | help: consider marking this type as `Copy` - --> $DIR/needless_pass_by_value.rs:124:1 + --> $DIR/needless_pass_by_value.rs:123:1 | LL | struct CopyWrapper(u32); | ^^^^^^^^^^^^^^^^^^^^^^^^ error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:132:45 + --> $DIR/needless_pass_by_value.rs:131:45 | LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) { | ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper` | help: consider marking this type as `Copy` - --> $DIR/needless_pass_by_value.rs:124:1 + --> $DIR/needless_pass_by_value.rs:123:1 | LL | struct CopyWrapper(u32); | ^^^^^^^^^^^^^^^^^^^^^^^^ error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:132:61 + --> $DIR/needless_pass_by_value.rs:131:61 | LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) { | ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper` | help: consider marking this type as `Copy` - --> $DIR/needless_pass_by_value.rs:124:1 + --> $DIR/needless_pass_by_value.rs:123:1 | LL | struct CopyWrapper(u32); | ^^^^^^^^^^^^^^^^^^^^^^^^ error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:144:40 + --> $DIR/needless_pass_by_value.rs:143:40 | LL | fn some_fun<'b, S: Bar<'b, ()>>(_item: S) {} | ^ help: consider taking a reference instead: `&S` error: this argument is passed by value, but not consumed in the function body - --> $DIR/needless_pass_by_value.rs:149:20 + --> $DIR/needless_pass_by_value.rs:148:20 | LL | fn more_fun(_item: impl Club<'static, i32>) {} | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider taking a reference instead: `&impl Club<'static, i32>` diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed index 5c4fd466c04..37efa6274df 100644 --- a/tests/ui/needless_return.fixed +++ b/tests/ui/needless_return.fixed @@ -1,13 +1,9 @@ // run-rustfix // edition:2018 +#![feature(let_else)] #![allow(unused)] -#![allow( - clippy::if_same_then_else, - clippy::single_match, - clippy::branches_sharing_code, - clippy::needless_bool -)] +#![allow(clippy::if_same_then_else, clippy::single_match, clippy::needless_bool)] #![warn(clippy::needless_return)] macro_rules! the_answer { @@ -207,4 +203,8 @@ async fn async_test_return_in_macro() { needed_return!(0); } +fn let_else() { + let Some(1) = Some(1) else { return }; +} + fn main() {} diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs index 34811db7413..cbf384ac9e4 100644 --- a/tests/ui/needless_return.rs +++ b/tests/ui/needless_return.rs @@ -1,13 +1,9 @@ // run-rustfix // edition:2018 +#![feature(let_else)] #![allow(unused)] -#![allow( - clippy::if_same_then_else, - clippy::single_match, - clippy::branches_sharing_code, - clippy::needless_bool -)] +#![allow(clippy::if_same_then_else, clippy::single_match, clippy::needless_bool)] #![warn(clippy::needless_return)] macro_rules! the_answer { @@ -207,4 +203,8 @@ async fn async_test_return_in_macro() { needed_return!(0); } +fn let_else() { + let Some(1) = Some(1) else { return }; +} + fn main() {} diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr index 74dda971fda..7ce7028bbae 100644 --- a/tests/ui/needless_return.stderr +++ b/tests/ui/needless_return.stderr @@ -1,5 +1,5 @@ error: unneeded `return` statement - --> $DIR/needless_return.rs:24:5 + --> $DIR/needless_return.rs:20:5 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` @@ -7,107 +7,113 @@ LL | return true; = note: `-D clippy::needless-return` implied by `-D warnings` error: unneeded `return` statement - --> $DIR/needless_return.rs:28:5 + --> $DIR/needless_return.rs:24:5 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:33:9 + --> $DIR/needless_return.rs:29:9 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:35:9 + --> $DIR/needless_return.rs:31:9 | LL | return false; | ^^^^^^^^^^^^^ help: remove `return`: `false` error: unneeded `return` statement - --> $DIR/needless_return.rs:41:17 + --> $DIR/needless_return.rs:37:17 | LL | true => return false, | ^^^^^^^^^^^^ help: remove `return`: `false` error: unneeded `return` statement - --> $DIR/needless_return.rs:43:13 + --> $DIR/needless_return.rs:39:13 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:50:9 + --> $DIR/needless_return.rs:46:9 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:52:16 + --> $DIR/needless_return.rs:48:16 | LL | let _ = || return true; | ^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:60:5 + --> $DIR/needless_return.rs:56:5 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:65:9 + --> $DIR/needless_return.rs:61:9 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:67:9 + --> $DIR/needless_return.rs:63:9 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:74:14 + --> $DIR/needless_return.rs:70:14 | LL | _ => return, | ^^^^^^ help: replace `return` with an empty block: `{}` error: unneeded `return` statement - --> $DIR/needless_return.rs:89:9 + --> $DIR/needless_return.rs:85:9 | LL | return String::from("test"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::from("test")` error: unneeded `return` statement - --> $DIR/needless_return.rs:91:9 + --> $DIR/needless_return.rs:87:9 | LL | return String::new(); | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()` error: unneeded `return` statement - --> $DIR/needless_return.rs:112:32 + --> $DIR/needless_return.rs:108:32 | LL | bar.unwrap_or_else(|_| return) | ^^^^^^ help: replace `return` with an empty block: `{}` error: unneeded `return` statement - --> $DIR/needless_return.rs:117:13 + --> $DIR/needless_return.rs:113:13 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:119:20 + --> $DIR/needless_return.rs:115:20 | LL | let _ = || return; | ^^^^^^ help: replace `return` with an empty block: `{}` error: unneeded `return` statement - --> $DIR/needless_return.rs:125:32 + --> $DIR/needless_return.rs:121:32 | LL | res.unwrap_or_else(|_| return Foo) | ^^^^^^^^^^ help: remove `return`: `Foo` +error: unneeded `return` statement + --> $DIR/needless_return.rs:130:5 + | +LL | return true; + | ^^^^^^^^^^^^ help: remove `return`: `true` + error: unneeded `return` statement --> $DIR/needless_return.rs:134:5 | @@ -115,79 +121,73 @@ LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:138:5 - | -LL | return true; - | ^^^^^^^^^^^^ help: remove `return`: `true` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:143:9 + --> $DIR/needless_return.rs:139:9 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:145:9 + --> $DIR/needless_return.rs:141:9 | LL | return false; | ^^^^^^^^^^^^^ help: remove `return`: `false` error: unneeded `return` statement - --> $DIR/needless_return.rs:151:17 + --> $DIR/needless_return.rs:147:17 | LL | true => return false, | ^^^^^^^^^^^^ help: remove `return`: `false` error: unneeded `return` statement - --> $DIR/needless_return.rs:153:13 + --> $DIR/needless_return.rs:149:13 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:160:9 + --> $DIR/needless_return.rs:156:9 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:162:16 + --> $DIR/needless_return.rs:158:16 | LL | let _ = || return true; | ^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:170:5 + --> $DIR/needless_return.rs:166:5 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:175:9 + --> $DIR/needless_return.rs:171:9 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:177:9 + --> $DIR/needless_return.rs:173:9 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:184:14 + --> $DIR/needless_return.rs:180:14 | LL | _ => return, | ^^^^^^ help: replace `return` with an empty block: `{}` error: unneeded `return` statement - --> $DIR/needless_return.rs:199:9 + --> $DIR/needless_return.rs:195:9 | LL | return String::from("test"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::from("test")` error: unneeded `return` statement - --> $DIR/needless_return.rs:201:9 + --> $DIR/needless_return.rs:197:9 | LL | return String::new(); | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()` diff --git a/tests/ui/nonminimal_bool.rs b/tests/ui/nonminimal_bool.rs index 971be26278f..fa5743c1155 100644 --- a/tests/ui/nonminimal_bool.rs +++ b/tests/ui/nonminimal_bool.rs @@ -1,4 +1,4 @@ -#![allow(unused, clippy::many_single_char_names, clippy::diverging_sub_expression)] +#![allow(unused, clippy::diverging_sub_expression)] #![warn(clippy::nonminimal_bool)] fn main() { diff --git a/tests/ui/nonminimal_bool_methods.rs b/tests/ui/nonminimal_bool_methods.rs index 90758740290..d0a289b7ea4 100644 --- a/tests/ui/nonminimal_bool_methods.rs +++ b/tests/ui/nonminimal_bool_methods.rs @@ -1,4 +1,4 @@ -#![allow(unused, clippy::many_single_char_names, clippy::diverging_sub_expression)] +#![allow(unused, clippy::diverging_sub_expression)] #![warn(clippy::nonminimal_bool)] fn methods_with_negation() { diff --git a/tests/ui/op_ref.rs b/tests/ui/op_ref.rs index 6605c967c8e..ab9c4d34c88 100644 --- a/tests/ui/op_ref.rs +++ b/tests/ui/op_ref.rs @@ -1,6 +1,5 @@ #![allow(unused_variables, clippy::blacklisted_name)] #![warn(clippy::op_ref)] -#![allow(clippy::many_single_char_names)] use std::collections::HashSet; use std::ops::BitAnd; diff --git a/tests/ui/op_ref.stderr b/tests/ui/op_ref.stderr index 821099d8779..992417084bd 100644 --- a/tests/ui/op_ref.stderr +++ b/tests/ui/op_ref.stderr @@ -1,5 +1,5 @@ error: needlessly taken reference of both operands - --> $DIR/op_ref.rs:12:15 + --> $DIR/op_ref.rs:11:15 | LL | let foo = &5 - &6; | ^^^^^^^ @@ -11,7 +11,7 @@ LL | let foo = 5 - 6; | ~ ~ error: taken reference of right operand - --> $DIR/op_ref.rs:57:13 + --> $DIR/op_ref.rs:56:13 | LL | let z = x & &y; | ^^^^-- diff --git a/tests/ui/overflow_check_conditional.rs b/tests/ui/overflow_check_conditional.rs index 84332040dba..5db75f5291b 100644 --- a/tests/ui/overflow_check_conditional.rs +++ b/tests/ui/overflow_check_conditional.rs @@ -1,4 +1,3 @@ -#![allow(clippy::many_single_char_names)] #![warn(clippy::overflow_check_conditional)] fn main() { diff --git a/tests/ui/overflow_check_conditional.stderr b/tests/ui/overflow_check_conditional.stderr index 19e843c2c0a..1b8b146b60a 100644 --- a/tests/ui/overflow_check_conditional.stderr +++ b/tests/ui/overflow_check_conditional.stderr @@ -1,5 +1,5 @@ error: you are trying to use classic C overflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:8:8 + --> $DIR/overflow_check_conditional.rs:7:8 | LL | if a + b < a {} | ^^^^^^^^^ @@ -7,43 +7,43 @@ LL | if a + b < a {} = note: `-D clippy::overflow-check-conditional` implied by `-D warnings` error: you are trying to use classic C overflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:9:8 + --> $DIR/overflow_check_conditional.rs:8:8 | LL | if a > a + b {} | ^^^^^^^^^ error: you are trying to use classic C overflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:10:8 + --> $DIR/overflow_check_conditional.rs:9:8 | LL | if a + b < b {} | ^^^^^^^^^ error: you are trying to use classic C overflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:11:8 + --> $DIR/overflow_check_conditional.rs:10:8 | LL | if b > a + b {} | ^^^^^^^^^ error: you are trying to use classic C underflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:12:8 + --> $DIR/overflow_check_conditional.rs:11:8 | LL | if a - b > b {} | ^^^^^^^^^ error: you are trying to use classic C underflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:13:8 + --> $DIR/overflow_check_conditional.rs:12:8 | LL | if b < a - b {} | ^^^^^^^^^ error: you are trying to use classic C underflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:14:8 + --> $DIR/overflow_check_conditional.rs:13:8 | LL | if a - b > a {} | ^^^^^^^^^ error: you are trying to use classic C underflow conditions that will fail in Rust - --> $DIR/overflow_check_conditional.rs:15:8 + --> $DIR/overflow_check_conditional.rs:14:8 | LL | if a < a - b {} | ^^^^^^^^^ diff --git a/tests/ui/ptr_arg.rs b/tests/ui/ptr_arg.rs index 06370dfce65..99e6d2aad8d 100644 --- a/tests/ui/ptr_arg.rs +++ b/tests/ui/ptr_arg.rs @@ -1,4 +1,9 @@ -#![allow(unused, clippy::many_single_char_names, clippy::redundant_clone)] +#![allow( + unused, + clippy::many_single_char_names, + clippy::redundant_clone, + clippy::if_then_panic +)] #![warn(clippy::ptr_arg)] use std::borrow::Cow; diff --git a/tests/ui/ptr_arg.stderr b/tests/ui/ptr_arg.stderr index 64594eb870c..42183447ead 100644 --- a/tests/ui/ptr_arg.stderr +++ b/tests/ui/ptr_arg.stderr @@ -1,5 +1,5 @@ error: writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used with non-Vec-based slices - --> $DIR/ptr_arg.rs:7:14 + --> $DIR/ptr_arg.rs:12:14 | LL | fn do_vec(x: &Vec<i64>) { | ^^^^^^^^^ help: change this to: `&[i64]` @@ -7,25 +7,25 @@ LL | fn do_vec(x: &Vec<i64>) { = note: `-D clippy::ptr-arg` implied by `-D warnings` error: writing `&String` instead of `&str` involves a new object where a slice will do - --> $DIR/ptr_arg.rs:16:14 + --> $DIR/ptr_arg.rs:21:14 | LL | fn do_str(x: &String) { | ^^^^^^^ help: change this to: `&str` error: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do - --> $DIR/ptr_arg.rs:25:15 + --> $DIR/ptr_arg.rs:30:15 | LL | fn do_path(x: &PathBuf) { | ^^^^^^^^ help: change this to: `&Path` error: writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used with non-Vec-based slices - --> $DIR/ptr_arg.rs:38:18 + --> $DIR/ptr_arg.rs:43:18 | LL | fn do_vec(x: &Vec<i64>); | ^^^^^^^^^ help: change this to: `&[i64]` error: writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used with non-Vec-based slices - --> $DIR/ptr_arg.rs:51:14 + --> $DIR/ptr_arg.rs:56:14 | LL | fn cloned(x: &Vec<u8>) -> Vec<u8> { | ^^^^^^^^ @@ -44,7 +44,7 @@ LL | x.to_owned() | error: writing `&String` instead of `&str` involves a new object where a slice will do - --> $DIR/ptr_arg.rs:60:18 + --> $DIR/ptr_arg.rs:65:18 | LL | fn str_cloned(x: &String) -> String { | ^^^^^^^ @@ -67,7 +67,7 @@ LL | x.to_string() | error: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do - --> $DIR/ptr_arg.rs:68:19 + --> $DIR/ptr_arg.rs:73:19 | LL | fn path_cloned(x: &PathBuf) -> PathBuf { | ^^^^^^^^ @@ -90,7 +90,7 @@ LL | x.to_path_buf() | error: writing `&String` instead of `&str` involves a new object where a slice will do - --> $DIR/ptr_arg.rs:76:44 + --> $DIR/ptr_arg.rs:81:44 | LL | fn false_positive_capacity(x: &Vec<u8>, y: &String) { | ^^^^^^^ @@ -109,13 +109,13 @@ LL | let c = y; | ~ error: using a reference to `Cow` is not recommended - --> $DIR/ptr_arg.rs:90:25 + --> $DIR/ptr_arg.rs:95:25 | LL | fn test_cow_with_ref(c: &Cow<[i32]>) {} | ^^^^^^^^^^^ help: change this to: `&[i32]` error: writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used with non-Vec-based slices - --> $DIR/ptr_arg.rs:143:21 + --> $DIR/ptr_arg.rs:148:21 | LL | fn foo_vec(vec: &Vec<u8>) { | ^^^^^^^^ @@ -134,7 +134,7 @@ LL | let _ = vec.to_owned().clone(); | ~~~~~~~~~~~~~~ error: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do - --> $DIR/ptr_arg.rs:148:23 + --> $DIR/ptr_arg.rs:153:23 | LL | fn foo_path(path: &PathBuf) { | ^^^^^^^^ @@ -153,7 +153,7 @@ LL | let _ = path.to_path_buf().clone(); | ~~~~~~~~~~~~~~~~~~ error: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do - --> $DIR/ptr_arg.rs:153:21 + --> $DIR/ptr_arg.rs:158:21 | LL | fn foo_str(str: &PathBuf) { | ^^^^^^^^ diff --git a/tests/ui/repeat_once.fixed b/tests/ui/repeat_once.fixed index a637c22fbcd..dc197e50300 100644 --- a/tests/ui/repeat_once.fixed +++ b/tests/ui/repeat_once.fixed @@ -1,6 +1,6 @@ // run-rustfix #![warn(clippy::repeat_once)] -#[allow(unused, clippy::many_single_char_names, clippy::redundant_clone)] +#[allow(unused, clippy::redundant_clone)] fn main() { const N: usize = 1; let s = "str"; diff --git a/tests/ui/repeat_once.rs b/tests/ui/repeat_once.rs index d99ca1b5b55..0ec5127117c 100644 --- a/tests/ui/repeat_once.rs +++ b/tests/ui/repeat_once.rs @@ -1,6 +1,6 @@ // run-rustfix #![warn(clippy::repeat_once)] -#[allow(unused, clippy::many_single_char_names, clippy::redundant_clone)] +#[allow(unused, clippy::redundant_clone)] fn main() { const N: usize = 1; let s = "str"; diff --git a/tests/ui/same_name_method.rs b/tests/ui/same_name_method.rs new file mode 100644 index 00000000000..12e10ba6c49 --- /dev/null +++ b/tests/ui/same_name_method.rs @@ -0,0 +1,111 @@ +#![warn(clippy::same_name_method)] +#![allow(dead_code, non_camel_case_types)] + +trait T1 { + fn foo() {} +} + +trait T2 { + fn foo() {} +} + +mod should_lint { + + mod test_basic_case { + use crate::T1; + + struct S; + + impl S { + fn foo() {} + } + + impl T1 for S { + fn foo() {} + } + } + + mod test_derive { + + #[derive(Clone)] + struct S; + + impl S { + fn clone() {} + } + } + + mod with_generic { + use crate::T1; + + struct S<U>(U); + + impl<U> S<U> { + fn foo() {} + } + + impl<U: Copy> T1 for S<U> { + fn foo() {} + } + } + + mod default_method { + use crate::T1; + + struct S; + + impl S { + fn foo() {} + } + + impl T1 for S {} + } + + mod mulitply_conflicit_trait { + use crate::{T1, T2}; + + struct S; + + impl S { + fn foo() {} + } + + impl T1 for S {} + + impl T2 for S {} + } +} + +mod should_not_lint { + + mod not_lint_two_trait_method { + use crate::{T1, T2}; + + struct S; + + impl T1 for S { + fn foo() {} + } + + impl T2 for S { + fn foo() {} + } + } + + mod only_lint_on_method { + trait T3 { + type foo; + } + + struct S; + + impl S { + fn foo() {} + } + impl T3 for S { + type foo = usize; + } + } +} + +fn main() {} diff --git a/tests/ui/same_name_method.stderr b/tests/ui/same_name_method.stderr new file mode 100644 index 00000000000..0f9139b41b9 --- /dev/null +++ b/tests/ui/same_name_method.stderr @@ -0,0 +1,64 @@ +error: method's name is same to an existing method in a trait + --> $DIR/same_name_method.rs:20:13 + | +LL | fn foo() {} + | ^^^^^^^^^^^ + | + = note: `-D clippy::same-name-method` implied by `-D warnings` +note: existing `foo` defined here + --> $DIR/same_name_method.rs:24:13 + | +LL | fn foo() {} + | ^^^^^^^^^^^ + +error: method's name is same to an existing method in a trait + --> $DIR/same_name_method.rs:44:13 + | +LL | fn foo() {} + | ^^^^^^^^^^^ + | +note: existing `foo` defined here + --> $DIR/same_name_method.rs:48:13 + | +LL | fn foo() {} + | ^^^^^^^^^^^ + +error: method's name is same to an existing method in a trait + --> $DIR/same_name_method.rs:58:13 + | +LL | fn foo() {} + | ^^^^^^^^^^^ + | +note: existing `foo` defined here + --> $DIR/same_name_method.rs:61:9 + | +LL | impl T1 for S {} + | ^^^^^^^^^^^^^^^^ + +error: method's name is same to an existing method in a trait + --> $DIR/same_name_method.rs:70:13 + | +LL | fn foo() {} + | ^^^^^^^^^^^ + | +note: existing `foo` defined here + --> $DIR/same_name_method.rs:73:9 + | +LL | impl T1 for S {} + | ^^^^^^^^^^^^^^^^ + +error: method's name is same to an existing method in a trait + --> $DIR/same_name_method.rs:34:13 + | +LL | fn clone() {} + | ^^^^^^^^^^^^^ + | +note: existing `clone` defined here + --> $DIR/same_name_method.rs:30:18 + | +LL | #[derive(Clone)] + | ^^^^^ + = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 5 previous errors + diff --git a/tests/ui/semicolon_if_nothing_returned.rs b/tests/ui/semicolon_if_nothing_returned.rs index 79ba7402f1f..9644a232968 100644 --- a/tests/ui/semicolon_if_nothing_returned.rs +++ b/tests/ui/semicolon_if_nothing_returned.rs @@ -1,4 +1,5 @@ #![warn(clippy::semicolon_if_nothing_returned)] +#![allow(clippy::redundant_closure)] #![feature(label_break_value)] fn get_unit() {} @@ -30,8 +31,8 @@ fn unsafe_checks_error() { use std::ptr; let mut s = MaybeUninit::<String>::uninit(); - let _d = || unsafe { - ptr::drop_in_place(s.as_mut_ptr()) + let _d = || unsafe { + ptr::drop_in_place(s.as_mut_ptr()) }; } diff --git a/tests/ui/semicolon_if_nothing_returned.stderr b/tests/ui/semicolon_if_nothing_returned.stderr index e88ebe2ad35..78813e7cc1c 100644 --- a/tests/ui/semicolon_if_nothing_returned.stderr +++ b/tests/ui/semicolon_if_nothing_returned.stderr @@ -1,5 +1,5 @@ error: consider adding a `;` to the last statement for consistent formatting - --> $DIR/semicolon_if_nothing_returned.rs:8:5 + --> $DIR/semicolon_if_nothing_returned.rs:9:5 | LL | println!("Hello") | ^^^^^^^^^^^^^^^^^ help: add a `;` here: `println!("Hello");` @@ -7,27 +7,27 @@ LL | println!("Hello") = note: `-D clippy::semicolon-if-nothing-returned` implied by `-D warnings` error: consider adding a `;` to the last statement for consistent formatting - --> $DIR/semicolon_if_nothing_returned.rs:12:5 + --> $DIR/semicolon_if_nothing_returned.rs:13:5 | LL | get_unit() | ^^^^^^^^^^ help: add a `;` here: `get_unit();` error: consider adding a `;` to the last statement for consistent formatting - --> $DIR/semicolon_if_nothing_returned.rs:17:5 + --> $DIR/semicolon_if_nothing_returned.rs:18:5 | LL | y = x + 1 | ^^^^^^^^^ help: add a `;` here: `y = x + 1;` error: consider adding a `;` to the last statement for consistent formatting - --> $DIR/semicolon_if_nothing_returned.rs:23:9 + --> $DIR/semicolon_if_nothing_returned.rs:24:9 | LL | hello() | ^^^^^^^ help: add a `;` here: `hello();` error: consider adding a `;` to the last statement for consistent formatting - --> $DIR/semicolon_if_nothing_returned.rs:34:9 + --> $DIR/semicolon_if_nothing_returned.rs:35:9 | -LL | ptr::drop_in_place(s.as_mut_ptr()) +LL | ptr::drop_in_place(s.as_mut_ptr()) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add a `;` here: `ptr::drop_in_place(s.as_mut_ptr());` error: aborting due to 5 previous errors diff --git a/tests/ui/suspicious_else_formatting.rs b/tests/ui/suspicious_else_formatting.rs index 547615b10d9..be8bc22bf98 100644 --- a/tests/ui/suspicious_else_formatting.rs +++ b/tests/ui/suspicious_else_formatting.rs @@ -1,5 +1,10 @@ +// aux-build:proc_macro_suspicious_else_formatting.rs + #![warn(clippy::suspicious_else_formatting)] +extern crate proc_macro_suspicious_else_formatting; +use proc_macro_suspicious_else_formatting::DeriveBadSpan; + fn foo() -> bool { true } @@ -103,3 +108,7 @@ fn main() { { } } + +// #7650 - Don't lint. Proc-macro using bad spans for `if` expressions. +#[derive(DeriveBadSpan)] +struct _Foo(u32, u32); diff --git a/tests/ui/suspicious_else_formatting.stderr b/tests/ui/suspicious_else_formatting.stderr index d8d67b4138a..d1db195cbb8 100644 --- a/tests/ui/suspicious_else_formatting.stderr +++ b/tests/ui/suspicious_else_formatting.stderr @@ -1,5 +1,5 @@ error: this looks like an `else {..}` but the `else` is missing - --> $DIR/suspicious_else_formatting.rs:11:6 + --> $DIR/suspicious_else_formatting.rs:16:6 | LL | } { | ^ @@ -8,7 +8,7 @@ LL | } { = note: to remove this lint, add the missing `else` or add a new line before the next block error: this looks like an `else if` but the `else` is missing - --> $DIR/suspicious_else_formatting.rs:15:6 + --> $DIR/suspicious_else_formatting.rs:20:6 | LL | } if foo() { | ^ @@ -16,7 +16,7 @@ LL | } if foo() { = note: to remove this lint, add the missing `else` or add a new line before the second `if` error: this looks like an `else if` but the `else` is missing - --> $DIR/suspicious_else_formatting.rs:22:10 + --> $DIR/suspicious_else_formatting.rs:27:10 | LL | } if foo() { | ^ @@ -24,7 +24,7 @@ LL | } if foo() { = note: to remove this lint, add the missing `else` or add a new line before the second `if` error: this looks like an `else if` but the `else` is missing - --> $DIR/suspicious_else_formatting.rs:30:10 + --> $DIR/suspicious_else_formatting.rs:35:10 | LL | } if foo() { | ^ @@ -32,7 +32,7 @@ LL | } if foo() { = note: to remove this lint, add the missing `else` or add a new line before the second `if` error: this is an `else {..}` but the formatting might hide it - --> $DIR/suspicious_else_formatting.rs:39:6 + --> $DIR/suspicious_else_formatting.rs:44:6 | LL | } else | ______^ @@ -42,7 +42,7 @@ LL | | { = note: to remove this lint, remove the `else` or remove the new line between `else` and `{..}` error: this is an `else if` but the formatting might hide it - --> $DIR/suspicious_else_formatting.rs:51:6 + --> $DIR/suspicious_else_formatting.rs:56:6 | LL | } else | ______^ @@ -52,7 +52,7 @@ LL | | if foo() { // the span of the above error should continue here = note: to remove this lint, remove the `else` or remove the new line between `else` and `if` error: this is an `else if` but the formatting might hide it - --> $DIR/suspicious_else_formatting.rs:56:6 + --> $DIR/suspicious_else_formatting.rs:61:6 | LL | } | ______^ @@ -63,7 +63,7 @@ LL | | if foo() { // the span of the above error should continue here = note: to remove this lint, remove the `else` or remove the new line between `else` and `if` error: this is an `else {..}` but the formatting might hide it - --> $DIR/suspicious_else_formatting.rs:83:6 + --> $DIR/suspicious_else_formatting.rs:88:6 | LL | } | ______^ @@ -75,7 +75,7 @@ LL | | { = note: to remove this lint, remove the `else` or remove the new line between `else` and `{..}` error: this is an `else {..}` but the formatting might hide it - --> $DIR/suspicious_else_formatting.rs:91:6 + --> $DIR/suspicious_else_formatting.rs:96:6 | LL | } | ______^ diff --git a/tests/ui/trivially_copy_pass_by_ref.rs b/tests/ui/trivially_copy_pass_by_ref.rs index 1a0123803a3..ea3dce17081 100644 --- a/tests/ui/trivially_copy_pass_by_ref.rs +++ b/tests/ui/trivially_copy_pass_by_ref.rs @@ -2,11 +2,7 @@ // normalize-stderr-test "\(limit: \d+ byte\)" -> "(limit: N byte)" #![deny(clippy::trivially_copy_pass_by_ref)] -#![allow( - clippy::many_single_char_names, - clippy::blacklisted_name, - clippy::redundant_field_names -)] +#![allow(clippy::blacklisted_name, clippy::redundant_field_names)] #[derive(Copy, Clone)] struct Foo(u32); diff --git a/tests/ui/trivially_copy_pass_by_ref.stderr b/tests/ui/trivially_copy_pass_by_ref.stderr index 9c4c49ceac4..a88d35f3ea5 100644 --- a/tests/ui/trivially_copy_pass_by_ref.stderr +++ b/tests/ui/trivially_copy_pass_by_ref.stderr @@ -1,5 +1,5 @@ error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:51:11 + --> $DIR/trivially_copy_pass_by_ref.rs:47:11 | LL | fn bad(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` @@ -11,97 +11,97 @@ LL | #![deny(clippy::trivially_copy_pass_by_ref)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:51:20 + --> $DIR/trivially_copy_pass_by_ref.rs:47:20 | LL | fn bad(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:51:29 + --> $DIR/trivially_copy_pass_by_ref.rs:47:29 | LL | fn bad(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:58:12 + --> $DIR/trivially_copy_pass_by_ref.rs:54:12 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^^ help: consider passing by value instead: `self` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:58:22 + --> $DIR/trivially_copy_pass_by_ref.rs:54:22 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:58:31 + --> $DIR/trivially_copy_pass_by_ref.rs:54:31 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:58:40 + --> $DIR/trivially_copy_pass_by_ref.rs:54:40 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:60:16 + --> $DIR/trivially_copy_pass_by_ref.rs:56:16 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:60:25 + --> $DIR/trivially_copy_pass_by_ref.rs:56:25 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:60:34 + --> $DIR/trivially_copy_pass_by_ref.rs:56:34 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:62:35 + --> $DIR/trivially_copy_pass_by_ref.rs:58:35 | LL | fn bad_issue7518(self, other: &Self) {} | ^^^^^ help: consider passing by value instead: `Self` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:74:16 + --> $DIR/trivially_copy_pass_by_ref.rs:70:16 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:74:25 + --> $DIR/trivially_copy_pass_by_ref.rs:70:25 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:74:34 + --> $DIR/trivially_copy_pass_by_ref.rs:70:34 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:78:34 + --> $DIR/trivially_copy_pass_by_ref.rs:74:34 | LL | fn trait_method(&self, _foo: &Foo); | ^^^^ help: consider passing by value instead: `Foo` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:110:21 + --> $DIR/trivially_copy_pass_by_ref.rs:106:21 | LL | fn foo_never(x: &i32) { | ^^^^ help: consider passing by value instead: `i32` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:115:15 + --> $DIR/trivially_copy_pass_by_ref.rs:111:15 | LL | fn foo(x: &i32) { | ^^^^ help: consider passing by value instead: `i32` diff --git a/tests/ui/while_let_on_iterator.fixed b/tests/ui/while_let_on_iterator.fixed index cdcdd808c94..f5a34190902 100644 --- a/tests/ui/while_let_on_iterator.fixed +++ b/tests/ui/while_let_on_iterator.fixed @@ -188,7 +188,7 @@ fn issue6491() { // Used in outer loop, needs &mut let mut it = 1..40; while let Some(n) = it.next() { - for m in &mut it { + for m in it.by_ref() { if m % 10 == 0 { break; } @@ -219,7 +219,7 @@ fn issue6491() { // Used after the loop, needs &mut. let mut it = 1..40; - for m in &mut it { + for m in it.by_ref() { if m % 10 == 0 { break; } @@ -236,7 +236,7 @@ fn issue6231() { let mut it = 1..40; let mut opt = Some(0); while let Some(n) = opt.take().or_else(|| it.next()) { - for m in &mut it { + for m in it.by_ref() { if n % 10 == 0 { break; } @@ -251,7 +251,7 @@ fn issue1924() { impl<T: Iterator<Item = u32>> S<T> { fn f(&mut self) -> Option<u32> { // Used as a field. - for i in &mut self.0 { + for i in self.0.by_ref() { if !(3..=7).contains(&i) { return Some(i); } @@ -283,7 +283,7 @@ fn issue1924() { } } // This one is fine, a different field is borrowed - for i in &mut self.0.0.0 { + for i in self.0.0.0.by_ref() { if i == 1 { return self.0.1.take(); } else { @@ -312,7 +312,7 @@ fn issue1924() { // Needs &mut, field of the iterator is accessed after the loop let mut it = S2(1..40, 0); - for n in &mut it { + for n in it.by_ref() { if n == 0 { break; } @@ -324,7 +324,7 @@ fn issue7249() { let mut it = 0..10; let mut x = || { // Needs &mut, the closure can be called multiple times - for x in &mut it { + for x in it.by_ref() { if x % 2 == 0 { break; } @@ -338,7 +338,7 @@ fn issue7510() { let mut it = 0..10; let it = &mut it; // Needs to reborrow `it` as the binding isn't mutable - for x in &mut *it { + for x in it.by_ref() { if x % 2 == 0 { break; } @@ -349,7 +349,7 @@ fn issue7510() { let mut it = 0..10; let it = S(&mut it); // Needs to reborrow `it.0` as the binding isn't mutable - for x in &mut *it.0 { + for x in it.0.by_ref() { if x % 2 == 0 { break; } diff --git a/tests/ui/while_let_on_iterator.stderr b/tests/ui/while_let_on_iterator.stderr index ff9b08996da..5e2fce4491a 100644 --- a/tests/ui/while_let_on_iterator.stderr +++ b/tests/ui/while_let_on_iterator.stderr @@ -46,7 +46,7 @@ error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:191:9 | LL | while let Some(m) = it.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in &mut it` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:202:5 @@ -70,19 +70,19 @@ error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:222:9 | LL | while let Some(m) = it.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in &mut it` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:239:9 | LL | while let Some(m) = it.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in &mut it` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for m in it.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:254:13 | LL | while let Some(i) = self.0.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in &mut self.0` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in self.0.by_ref()` error: manual `!RangeInclusive::contains` implementation --> $DIR/while_let_on_iterator.rs:255:20 @@ -96,31 +96,31 @@ error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:286:13 | LL | while let Some(i) = self.0.0.0.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in &mut self.0.0.0` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for i in self.0.0.0.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:315:5 | LL | while let Some(n) = it.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for n in &mut it` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for n in it.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:327:9 | LL | while let Some(x) = it.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut it` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:341:5 | LL | while let Some(x) = it.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut *it` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:352:5 | LL | while let Some(x) = it.0.next() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut *it.0` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in it.0.by_ref()` error: this loop could be written as a `for` loop --> $DIR/while_let_on_iterator.rs:371:5 diff --git a/tests/ui/wrong_self_convention2.rs b/tests/ui/wrong_self_convention2.rs index 501bc1e6a85..0d827c1feb3 100644 --- a/tests/ui/wrong_self_convention2.rs +++ b/tests/ui/wrong_self_convention2.rs @@ -68,3 +68,40 @@ mod issue7179 { fn as_byte_slice(slice: &[Self]) -> &[u8]; } } + +mod issue3414 { + struct CellLikeThing<T>(T); + + impl<T> CellLikeThing<T> { + // don't trigger + fn into_inner(this: Self) -> T { + this.0 + } + } + + impl<T> std::ops::Deref for CellLikeThing<T> { + type Target = T; + + fn deref(&self) -> &T { + &self.0 + } + } +} + +// don't trigger +mod issue4546 { + use std::pin::Pin; + + struct S; + impl S { + pub fn as_mut(self: Pin<&mut Self>) {} + + pub fn as_other_thingy(self: Pin<&Self>) {} + + pub fn is_other_thingy(self: Pin<&Self>) {} + + pub fn to_mut(self: Pin<&mut Self>) {} + + pub fn to_other_thingy(self: Pin<&Self>) {} + } +}