From: flip1995 Date: Thu, 10 Sep 2020 15:47:07 +0000 (+0200) Subject: Merge commit '5034d47f721ff4c3a3ff2aca9ef2ef3e1d067f9f' into clippyup X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=ca6c695320e2ee35b70d7ff3ebfbc747946c7a3d;hp=25b2f4861222d6507598149f576e7d25dc308c8c;p=rust.git Merge commit '5034d47f721ff4c3a3ff2aca9ef2ef3e1d067f9f' into clippyup --- diff --git a/src/tools/clippy/CHANGELOG.md b/src/tools/clippy/CHANGELOG.md index 137b561028a..64f9379b303 100644 --- a/src/tools/clippy/CHANGELOG.md +++ b/src/tools/clippy/CHANGELOG.md @@ -6,11 +6,113 @@ document. ## Unreleased / In Rust Nightly -[c2c07fa...master](https://github.com/rust-lang/rust-clippy/compare/c2c07fa...master) +[09bd400...master](https://github.com/rust-lang/rust-clippy/compare/09bd400...master) + +## Rust 1.47 + +Current beta, release 2020-10-08 + +[c2c07fa...09bd400](https://github.com/rust-lang/rust-clippy/compare/c2c07fa...09bd400) + +### New lints + +* [`derive_ord_xor_partial_ord`] [#5848](https://github.com/rust-lang/rust-clippy/pull/5848) +* [`trait_duplication_in_bounds`] [#5852](https://github.com/rust-lang/rust-clippy/pull/5852) +* [`map_identity`] [#5694](https://github.com/rust-lang/rust-clippy/pull/5694) +* [`unit_return_expecting_ord`] [#5737](https://github.com/rust-lang/rust-clippy/pull/5737) +* [`pattern_type_mismatch`] [#4841](https://github.com/rust-lang/rust-clippy/pull/4841) +* [`repeat_once`] [#5773](https://github.com/rust-lang/rust-clippy/pull/5773) +* [`same_item_push`] [#5825](https://github.com/rust-lang/rust-clippy/pull/5825) +* [`needless_arbitrary_self_type`] [#5869](https://github.com/rust-lang/rust-clippy/pull/5869) +* [`match_like_matches_macro`] [#5769](https://github.com/rust-lang/rust-clippy/pull/5769) +* [`stable_sort_primitive`] [#5809](https://github.com/rust-lang/rust-clippy/pull/5809) +* [`blanket_clippy_restriction_lints`] [#5750](https://github.com/rust-lang/rust-clippy/pull/5750) +* [`option_if_let_else`] [#5301](https://github.com/rust-lang/rust-clippy/pull/5301) + +### Moves and Deprecations + +* Deprecate [`regex_macro`] lint + [#5760](https://github.com/rust-lang/rust-clippy/pull/5760) +* Move [`range_minus_one`] to `pedantic` + [#5752](https://github.com/rust-lang/rust-clippy/pull/5752) + +### Enhancements + +* Improve [`needless_collect`] by catching `collect` calls followed by `iter` or `into_iter` calls + [#5837](https://github.com/rust-lang/rust-clippy/pull/5837) +* [`panic`], [`todo`], [`unimplemented`] and [`unreachable`] now detect calls with formatting + [#5811](https://github.com/rust-lang/rust-clippy/pull/5811) +* Detect more cases of [`suboptimal_flops`] and [`imprecise_flops`] + [#5443](https://github.com/rust-lang/rust-clippy/pull/5443) +* Handle asymmetrical implementations of `PartialEq` in [`cmp_owned`] + [#5701](https://github.com/rust-lang/rust-clippy/pull/5701) +* Make it possible to allow [`unsafe_derive_deserialize`] + [#5870](https://github.com/rust-lang/rust-clippy/pull/5870) +* Catch `ord.min(a).max(b)` where a < b in [`min_max`] + [#5871](https://github.com/rust-lang/rust-clippy/pull/5871) +* Make [`clone_on_copy`] suggestion machine applicable + [#5745](https://github.com/rust-lang/rust-clippy/pull/5745) +* Enable [`len_zero`] on ranges now that `is_empty` is stable on them + [#5961](https://github.com/rust-lang/rust-clippy/pull/5961) + +### False Positive Fixes + +* Avoid triggering [`or_fun_call`] with const fns that take no arguments + [#5889](https://github.com/rust-lang/rust-clippy/pull/5889) +* Fix [`redundant_closure_call`] false positive for closures that have multiple calls + [#5800](https://github.com/rust-lang/rust-clippy/pull/5800) +* Don't lint cases involving `ManuallyDrop` in [`redundant_clone`] + [#5824](https://github.com/rust-lang/rust-clippy/pull/5824) +* Treat a single expression the same as a single statement in the 2nd arm of a match in [`single_match_else`] + [#5771](https://github.com/rust-lang/rust-clippy/pull/5771) +* Don't trigger [`unnested_or_patterns`] if the feature `or_patterns` is not enabled + [#5758](https://github.com/rust-lang/rust-clippy/pull/5758) +* Avoid linting if key borrows in [`unnecessary_sort_by`] + [#5756](https://github.com/rust-lang/rust-clippy/pull/5756) +* Consider `Try` impl for `Poll` when generating suggestions in [`try_err`] + [#5857](https://github.com/rust-lang/rust-clippy/pull/5857) +* Take input lifetimes into account in `manual_async_fn` + [#5859](https://github.com/rust-lang/rust-clippy/pull/5859) +* Fix multiple false positives in [`type_repetition_in_bounds`] and add a configuration option + [#5761](https://github.com/rust-lang/rust-clippy/pull/5761) +* Limit the [`suspicious_arithmetic_impl`] lint to one binary operation + [#5820](https://github.com/rust-lang/rust-clippy/pull/5820) + +### Suggestion Fixes/Improvements + +* Improve readability of [`shadow_unrelated`] suggestion by truncating the RHS snippet + [#5788](https://github.com/rust-lang/rust-clippy/pull/5788) +* Suggest `filter_map` instead of `flat_map` when mapping to `Option` in [`map_flatten`] + [#5846](https://github.com/rust-lang/rust-clippy/pull/5846) +* Ensure suggestion is shown correctly for long method call chains in [`iter_nth_zero`] + [#5793](https://github.com/rust-lang/rust-clippy/pull/5793) +* Drop borrow operator in suggestions of [`redundant_pattern_matching`] + [#5815](https://github.com/rust-lang/rust-clippy/pull/5815) +* Add suggestion for [`iter_skip_next`] + [#5843](https://github.com/rust-lang/rust-clippy/pull/5843) +* Improve [`collapsible_if`] fix suggestion + [#5732](https://github.com/rust-lang/rust-clippy/pull/5732) + +### ICE Fixes + +* Fix ICE caused by [`needless_collect`] + [#5877](https://github.com/rust-lang/rust-clippy/pull/5877) +* Fix ICE caused by [`unnested_or_patterns`] + [#5784](https://github.com/rust-lang/rust-clippy/pull/5784) + +### Documentation Improvements + +* Fix grammar of [`await_holding_lock`] documentation + [#5748](https://github.com/rust-lang/rust-clippy/pull/5748) + +### Others + +* Make lints adhere to the rustc dev guide + [#5888](https://github.com/rust-lang/rust-clippy/pull/5888) ## Rust 1.46 -Current beta, release 2020-08-27 +Current stable, released 2020-08-27 [7ea7cd1...c2c07fa](https://github.com/rust-lang/rust-clippy/compare/7ea7cd1...c2c07fa) @@ -72,7 +174,7 @@ Current beta, release 2020-08-27 ## Rust 1.45 -Current stable, released 2020-07-16 +Released 2020-07-16 [891e1a8...7ea7cd1](https://github.com/rust-lang/rust-clippy/compare/891e1a8...7ea7cd1) @@ -1410,6 +1512,7 @@ Released 2018-09-13 [`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants [`assign_op_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern [`assign_ops`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_ops +[`async_yields_async`]: https://rust-lang.github.io/rust-clippy/master/index.html#async_yields_async [`await_holding_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock [`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask [`bind_instead_of_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map @@ -1444,6 +1547,7 @@ Released 2018-09-13 [`collapsible_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if [`comparison_chain`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain [`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator +[`create_dir`]: https://rust-lang.github.io/rust-clippy/master/index.html#create_dir [`crosspointer_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#crosspointer_transmute [`dbg_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#dbg_macro [`debug_assert_with_mut_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#debug_assert_with_mut_call diff --git a/src/tools/clippy/clippy_lints/src/async_yields_async.rs b/src/tools/clippy/clippy_lints/src/async_yields_async.rs new file mode 100644 index 00000000000..88d9d3b5a26 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/async_yields_async.rs @@ -0,0 +1,86 @@ +use crate::utils::{implements_trait, snippet, span_lint_and_then}; +use rustc_errors::Applicability; +use rustc_hir::{AsyncGeneratorKind, Body, BodyId, ExprKind, GeneratorKind, QPath}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// **What it does:** Checks for async blocks that yield values of types + /// that can themselves be awaited. + /// + /// **Why is this bad?** An await is likely missing. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// async fn foo() {} + /// + /// fn bar() { + /// let x = async { + /// foo() + /// }; + /// } + /// ``` + /// Use instead: + /// ```rust + /// async fn foo() {} + /// + /// fn bar() { + /// let x = async { + /// foo().await + /// }; + /// } + /// ``` + pub ASYNC_YIELDS_ASYNC, + correctness, + "async blocks that return a type that can be awaited" +} + +declare_lint_pass!(AsyncYieldsAsync => [ASYNC_YIELDS_ASYNC]); + +impl<'tcx> LateLintPass<'tcx> for AsyncYieldsAsync { + fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'_>) { + use AsyncGeneratorKind::{Block, Closure}; + // For functions, with explicitly defined types, don't warn. + // XXXkhuey maybe we should? + if let Some(GeneratorKind::Async(Block | Closure)) = body.generator_kind { + if let Some(future_trait_def_id) = cx.tcx.lang_items().future_trait() { + let body_id = BodyId { + hir_id: body.value.hir_id, + }; + let def_id = cx.tcx.hir().body_owner_def_id(body_id); + let typeck_results = cx.tcx.typeck(def_id); + let expr_ty = typeck_results.expr_ty(&body.value); + + if implements_trait(cx, expr_ty, future_trait_def_id, &[]) { + let return_expr_span = match &body.value.kind { + // XXXkhuey there has to be a better way. + ExprKind::Block(block, _) => block.expr.map(|e| e.span), + ExprKind::Path(QPath::Resolved(_, path)) => Some(path.span), + _ => None, + }; + if let Some(return_expr_span) = return_expr_span { + span_lint_and_then( + cx, + ASYNC_YIELDS_ASYNC, + return_expr_span, + "an async construct yields a type which is itself awaitable", + |db| { + db.span_label(body.value.span, "outer async construct"); + db.span_label(return_expr_span, "awaitable value not awaited"); + db.span_suggestion( + return_expr_span, + "consider awaiting this value", + format!("{}.await", snippet(cx, return_expr_span, "..")), + Applicability::MaybeIncorrect, + ); + }, + ); + } + } + } + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/attrs.rs b/src/tools/clippy/clippy_lints/src/attrs.rs index cfcc1b3c5f3..c8f153e7201 100644 --- a/src/tools/clippy/clippy_lints/src/attrs.rs +++ b/src/tools/clippy/clippy_lints/src/attrs.rs @@ -71,8 +71,9 @@ /// **What it does:** Checks for `extern crate` and `use` items annotated with /// lint attributes. /// - /// This lint permits `#[allow(unused_imports)]`, `#[allow(deprecated)]` and - /// `#[allow(unreachable_pub)]` on `use` items and `#[allow(unused_imports)]` on + /// This lint permits `#[allow(unused_imports)]`, `#[allow(deprecated)]`, + /// `#[allow(unreachable_pub)]`, `#[allow(clippy::wildcard_imports)]` and + /// `#[allow(clippy::enum_glob_use)]` on `use` items and `#[allow(unused_imports)]` on /// `extern crate` items with a `#[macro_use]` attribute. /// /// **Why is this bad?** Lint attributes have no effect on crate imports. Most @@ -318,7 +319,8 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { if let Some(ident) = attr.ident() { match &*ident.as_str() { "allow" | "warn" | "deny" | "forbid" => { - // permit `unused_imports`, `deprecated` and `unreachable_pub` for `use` items + // permit `unused_imports`, `deprecated`, `unreachable_pub`, + // `clippy::wildcard_imports`, and `clippy::enum_glob_use` for `use` items // and `unused_imports` for `extern crate` items with `macro_use` for lint in lint_list { match item.kind { @@ -327,6 +329,9 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { || is_word(lint, sym!(deprecated)) || is_word(lint, sym!(unreachable_pub)) || is_word(lint, sym!(unused)) + || extract_clippy_lint(lint) + .map_or(false, |s| s == "wildcard_imports") + || extract_clippy_lint(lint).map_or(false, |s| s == "enum_glob_use") { return; } @@ -387,24 +392,25 @@ fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_> } } -fn check_clippy_lint_names(cx: &LateContext<'_>, ident: &str, items: &[NestedMetaItem]) { - fn extract_name(lint: &NestedMetaItem) -> Option { - if_chain! { - if let Some(meta_item) = lint.meta_item(); - if meta_item.path.segments.len() > 1; - if let tool_name = meta_item.path.segments[0].ident; - if tool_name.as_str() == "clippy"; - let lint_name = meta_item.path.segments.last().unwrap().ident.name; - then { - return Some(lint_name.as_str()); - } +/// Returns the lint name if it is clippy lint. +fn extract_clippy_lint(lint: &NestedMetaItem) -> Option { + if_chain! { + if let Some(meta_item) = lint.meta_item(); + if meta_item.path.segments.len() > 1; + if let tool_name = meta_item.path.segments[0].ident; + if tool_name.as_str() == "clippy"; + let lint_name = meta_item.path.segments.last().unwrap().ident.name; + then { + return Some(lint_name.as_str()); } - None } + None +} +fn check_clippy_lint_names(cx: &LateContext<'_>, ident: &str, items: &[NestedMetaItem]) { let lint_store = cx.lints(); for lint in items { - if let Some(lint_name) = extract_name(lint) { + if let Some(lint_name) = extract_clippy_lint(lint) { if let CheckLintNameResult::Tool(Err((None, _))) = lint_store.check_lint_name(&lint_name, Some(sym!(clippy))) { diff --git a/src/tools/clippy/clippy_lints/src/create_dir.rs b/src/tools/clippy/clippy_lints/src/create_dir.rs new file mode 100644 index 00000000000..4002fb655a5 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/create_dir.rs @@ -0,0 +1,51 @@ +use crate::utils::{match_def_path, paths, snippet, span_lint_and_sugg}; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// **What it does:** Checks usage of `std::fs::create_dir` and suggest using `std::fs::create_dir_all` instead. + /// + /// **Why is this bad?** Sometimes `std::fs::crate_dir` is mistakenly chosen over `std::fs::create_dir_all`. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// std::fs::create_dir("foo"); + /// ``` + /// Use instead: + /// ```rust + /// std::fs::create_dir_all("foo"); + /// ``` + pub CREATE_DIR, + restriction, + "calling `std::fs::create_dir` instead of `std::fs::create_dir_all`" +} + +declare_lint_pass!(CreateDir => [CREATE_DIR]); + +impl LateLintPass<'_> for CreateDir { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + if_chain! { + if let ExprKind::Call(ref func, ref args) = expr.kind; + if let ExprKind::Path(ref path) = func.kind; + if let Some(def_id) = cx.qpath_res(path, func.hir_id).opt_def_id(); + if match_def_path(cx, def_id, &paths::STD_FS_CREATE_DIR); + then { + span_lint_and_sugg( + cx, + CREATE_DIR, + expr.span, + "calling `std::fs::create_dir` where there may be a better way", + "consider calling `std::fs::create_dir_all` instead", + format!("create_dir_all({})", snippet(cx, args[0].span, "..")), + Applicability::MaybeIncorrect, + ) + } + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/default_trait_access.rs b/src/tools/clippy/clippy_lints/src/default_trait_access.rs index 1654df56a9a..3048436d9a7 100644 --- a/src/tools/clippy/clippy_lints/src/default_trait_access.rs +++ b/src/tools/clippy/clippy_lints/src/default_trait_access.rs @@ -38,37 +38,23 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if let ExprKind::Path(ref qpath) = path.kind; if let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id(); if match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD); + // Detect and ignore ::default() because these calls do explicitly name the type. + if let QPath::Resolved(None, _path) = qpath; then { - match qpath { - QPath::Resolved(..) => { - if_chain! { - // Detect and ignore ::default() because these calls do - // explicitly name the type. - if let ExprKind::Call(ref method, ref _args) = expr.kind; - if let ExprKind::Path(ref p) = method.kind; - if let QPath::Resolved(Some(_ty), _path) = p; - then { - return; - } - } - - // TODO: Work out a way to put "whatever the imported way of referencing - // this type in this file" rather than a fully-qualified type. - let expr_ty = cx.typeck_results().expr_ty(expr); - if let ty::Adt(..) = expr_ty.kind() { - let replacement = format!("{}::default()", expr_ty); - span_lint_and_sugg( - cx, - DEFAULT_TRAIT_ACCESS, - expr.span, - &format!("calling `{}` is more clear than this expression", replacement), - "try", - replacement, - Applicability::Unspecified, // First resolve the TODO above - ); - } - }, - QPath::TypeRelative(..) | QPath::LangItem(..) => {}, + let expr_ty = cx.typeck_results().expr_ty(expr); + if let ty::Adt(def, ..) = expr_ty.kind() { + // TODO: Work out a way to put "whatever the imported way of referencing + // this type in this file" rather than a fully-qualified type. + let replacement = format!("{}::default()", cx.tcx.def_path_str(def.did)); + span_lint_and_sugg( + cx, + DEFAULT_TRAIT_ACCESS, + expr.span, + &format!("calling `{}` is more clear than this expression", replacement), + "try", + replacement, + Applicability::Unspecified, // First resolve the TODO above + ); } } } diff --git a/src/tools/clippy/clippy_lints/src/functions.rs b/src/tools/clippy/clippy_lints/src/functions.rs index 89fde1d509d..50b39cf4ea7 100644 --- a/src/tools/clippy/clippy_lints/src/functions.rs +++ b/src/tools/clippy/clippy_lints/src/functions.rs @@ -374,7 +374,12 @@ fn check_line_number(self, cx: &LateContext<'_>, span: Span, body: &'tcx hir::Bo } if line_count > self.max_lines { - span_lint(cx, TOO_MANY_LINES, span, "this function has a large number of lines") + span_lint( + cx, + TOO_MANY_LINES, + span, + &format!("this function has too many lines ({}/{})", line_count, self.max_lines), + ) } } diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index 577ce6523b4..2020ef78509 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -154,6 +154,7 @@ macro_rules! declare_clippy_lint { mod as_conversions; mod assertions_on_constants; mod assign_ops; +mod async_yields_async; mod atomic_ordering; mod attrs; mod await_holding_lock; @@ -169,6 +170,7 @@ macro_rules! declare_clippy_lint { mod comparison_chain; mod copies; mod copy_iterator; +mod create_dir; mod dbg_macro; mod default_trait_access; mod dereference; @@ -483,6 +485,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &assertions_on_constants::ASSERTIONS_ON_CONSTANTS, &assign_ops::ASSIGN_OP_PATTERN, &assign_ops::MISREFACTORED_ASSIGN_OP, + &async_yields_async::ASYNC_YIELDS_ASYNC, &atomic_ordering::INVALID_ATOMIC_ORDERING, &attrs::BLANKET_CLIPPY_RESTRICTION_LINTS, &attrs::DEPRECATED_CFG_ATTR, @@ -511,6 +514,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &copies::MATCH_SAME_ARMS, &copies::SAME_FUNCTIONS_IN_IF_CONDITION, ©_iterator::COPY_ITERATOR, + &create_dir::CREATE_DIR, &dbg_macro::DBG_MACRO, &default_trait_access::DEFAULT_TRAIT_ACCESS, &dereference::EXPLICIT_DEREF_METHODS, @@ -1042,6 +1046,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| box items_after_statements::ItemsAfterStatements); store.register_early_pass(|| box precedence::Precedence); store.register_early_pass(|| box needless_continue::NeedlessContinue); + store.register_late_pass(|| box create_dir::CreateDir); store.register_early_pass(|| box needless_arbitrary_self_type::NeedlessArbitrarySelfType); store.register_early_pass(|| box redundant_static_lifetimes::RedundantStaticLifetimes); store.register_late_pass(|| box cargo_common_metadata::CargoCommonMetadata); @@ -1099,11 +1104,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box unwrap_in_result::UnwrapInResult); store.register_late_pass(|| box self_assignment::SelfAssignment); store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs); + store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), LintId::of(&arithmetic::INTEGER_ARITHMETIC), LintId::of(&as_conversions::AS_CONVERSIONS), + LintId::of(&create_dir::CREATE_DIR), LintId::of(&dbg_macro::DBG_MACRO), LintId::of(&else_if_without_else::ELSE_IF_WITHOUT_ELSE), LintId::of(&exit::EXIT), @@ -1232,6 +1239,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&assertions_on_constants::ASSERTIONS_ON_CONSTANTS), LintId::of(&assign_ops::ASSIGN_OP_PATTERN), LintId::of(&assign_ops::MISREFACTORED_ASSIGN_OP), + LintId::of(&async_yields_async::ASYNC_YIELDS_ASYNC), LintId::of(&atomic_ordering::INVALID_ATOMIC_ORDERING), LintId::of(&attrs::BLANKET_CLIPPY_RESTRICTION_LINTS), LintId::of(&attrs::DEPRECATED_CFG_ATTR), @@ -1675,6 +1683,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_group(true, "clippy::correctness", Some("clippy_correctness"), vec![ LintId::of(&approx_const::APPROX_CONSTANT), + LintId::of(&async_yields_async::ASYNC_YIELDS_ASYNC), LintId::of(&atomic_ordering::INVALID_ATOMIC_ORDERING), LintId::of(&attrs::DEPRECATED_SEMVER), LintId::of(&attrs::MISMATCHED_TARGET_OS), diff --git a/src/tools/clippy/clippy_lints/src/loops.rs b/src/tools/clippy/clippy_lints/src/loops.rs index 604a97e3c08..6c54c07869a 100644 --- a/src/tools/clippy/clippy_lints/src/loops.rs +++ b/src/tools/clippy/clippy_lints/src/loops.rs @@ -1131,6 +1131,27 @@ fn detect_same_item_push<'tcx>( body: &'tcx Expr<'_>, _: &'tcx Expr<'_>, ) { + fn emit_lint(cx: &LateContext<'_>, vec: &Expr<'_>, pushed_item: &Expr<'_>) { + let vec_str = snippet_with_macro_callsite(cx, vec.span, ""); + let item_str = snippet_with_macro_callsite(cx, pushed_item.span, ""); + + span_lint_and_help( + cx, + SAME_ITEM_PUSH, + vec.span, + "it looks like the same item is being pushed into this Vec", + None, + &format!( + "try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})", + item_str, vec_str, item_str + ), + ) + } + + if !matches!(pat.kind, PatKind::Wild) { + return; + } + // Determine whether it is safe to lint the body let mut same_item_push_visitor = SameItemPushVisitor { should_lint: true, @@ -1140,43 +1161,50 @@ fn detect_same_item_push<'tcx>( walk_expr(&mut same_item_push_visitor, body); if same_item_push_visitor.should_lint { if let Some((vec, pushed_item)) = same_item_push_visitor.vec_push { - // Make sure that the push does not involve possibly mutating values - if let PatKind::Wild = pat.kind { - let vec_str = snippet_with_macro_callsite(cx, vec.span, ""); - let item_str = snippet_with_macro_callsite(cx, pushed_item.span, ""); - if let ExprKind::Path(ref qpath) = pushed_item.kind { - if_chain! { - if let Res::Local(hir_id) = qpath_res(cx, qpath, pushed_item.hir_id); - let node = cx.tcx.hir().get(hir_id); - if let Node::Binding(pat) = node; - if let PatKind::Binding(bind_ann, ..) = pat.kind; - if !matches!(bind_ann, BindingAnnotation::RefMut | BindingAnnotation::Mutable); - then { - span_lint_and_help( - cx, - SAME_ITEM_PUSH, - vec.span, - "it looks like the same item is being pushed into this Vec", - None, - &format!( - "try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})", - item_str, vec_str, item_str - ), - ) + let vec_ty = cx.typeck_results().expr_ty(vec); + let ty = vec_ty.walk().nth(1).unwrap().expect_ty(); + if cx + .tcx + .lang_items() + .clone_trait() + .map_or(false, |id| implements_trait(cx, ty, id, &[])) + { + // Make sure that the push does not involve possibly mutating values + match pushed_item.kind { + ExprKind::Path(ref qpath) => { + match qpath_res(cx, qpath, pushed_item.hir_id) { + // immutable bindings that are initialized with literal or constant + Res::Local(hir_id) => { + if_chain! { + let node = cx.tcx.hir().get(hir_id); + if let Node::Binding(pat) = node; + if let PatKind::Binding(bind_ann, ..) = pat.kind; + if !matches!(bind_ann, BindingAnnotation::RefMut | BindingAnnotation::Mutable); + let parent_node = cx.tcx.hir().get_parent_node(hir_id); + if let Some(Node::Local(parent_let_expr)) = cx.tcx.hir().find(parent_node); + if let Some(init) = parent_let_expr.init; + then { + match init.kind { + // immutable bindings that are initialized with literal + ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item), + // immutable bindings that are initialized with constant + ExprKind::Path(ref path) => { + if let Res::Def(DefKind::Const, ..) = qpath_res(cx, path, init.hir_id) { + emit_lint(cx, vec, pushed_item); + } + } + _ => {}, + } + } + } + }, + // constant + Res::Def(DefKind::Const, ..) => emit_lint(cx, vec, pushed_item), + _ => {}, } - } - } else if mutated_variables(pushed_item, cx).map_or(false, |mutvars| mutvars.is_empty()) { - span_lint_and_help( - cx, - SAME_ITEM_PUSH, - vec.span, - "it looks like the same item is being pushed into this Vec", - None, - &format!( - "try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})", - item_str, vec_str, item_str - ), - ) + }, + ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item), + _ => {}, } } } diff --git a/src/tools/clippy/clippy_lints/src/map_unit_fn.rs b/src/tools/clippy/clippy_lints/src/map_unit_fn.rs index 1f9ae8c931a..076ef235b8b 100644 --- a/src/tools/clippy/clippy_lints/src/map_unit_fn.rs +++ b/src/tools/clippy/clippy_lints/src/map_unit_fn.rs @@ -9,7 +9,7 @@ declare_clippy_lint! { /// **What it does:** Checks for usage of `option.map(f)` where f is a function - /// or closure that returns the unit type. + /// or closure that returns the unit type `()`. /// /// **Why is this bad?** Readability, this can be written more clearly with /// an if let statement @@ -51,7 +51,7 @@ declare_clippy_lint! { /// **What it does:** Checks for usage of `result.map(f)` where f is a function - /// or closure that returns the unit type. + /// or closure that returns the unit type `()`. /// /// **Why is this bad?** Readability, this can be written more clearly with /// an if let statement @@ -197,7 +197,7 @@ fn let_binding_name(cx: &LateContext<'_>, var_arg: &hir::Expr<'_>) -> String { #[must_use] fn suggestion_msg(function_type: &str, map_type: &str) -> String { format!( - "called `map(f)` on an `{0}` value where `f` is a {1} that returns the unit type", + "called `map(f)` on an `{0}` value where `f` is a {1} that returns the unit type `()`", map_type, function_type ) } diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs index a7a3d675156..ba69c8266b1 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mod.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs @@ -1324,20 +1324,20 @@ } declare_clippy_lint! { - /// **What it does:** Warns when using push_str with a single-character string literal, - /// and push with a char would work fine. + /// **What it does:** Warns when using `push_str` with a single-character string literal, + /// and `push` with a `char` would work fine. /// - /// **Why is this bad?** It's less clear that we are pushing a single character + /// **Why is this bad?** It's less clear that we are pushing a single character. /// /// **Known problems:** None /// /// **Example:** - /// ``` + /// ```rust /// let mut string = String::new(); /// string.push_str("R"); /// ``` /// Could be written as - /// ``` + /// ```rust /// let mut string = String::new(); /// string.push('R'); /// ``` diff --git a/src/tools/clippy/clippy_lints/src/temporary_assignment.rs b/src/tools/clippy/clippy_lints/src/temporary_assignment.rs index 2b6ddadd4c1..fb891866364 100644 --- a/src/tools/clippy/clippy_lints/src/temporary_assignment.rs +++ b/src/tools/clippy/clippy_lints/src/temporary_assignment.rs @@ -21,11 +21,8 @@ "assignments to temporaries" } -fn is_temporary(_cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - match &expr.kind { - ExprKind::Struct(..) | ExprKind::Tup(..) => true, - _ => false, - } +fn is_temporary(expr: &Expr<'_>) -> bool { + matches!(&expr.kind, ExprKind::Struct(..) | ExprKind::Tup(..)) } declare_lint_pass!(TemporaryAssignment => [TEMPORARY_ASSIGNMENT]); @@ -37,7 +34,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { while let ExprKind::Field(f, _) | ExprKind::Index(f, _) = &base.kind { base = f; } - if is_temporary(cx, base) && !is_adjusted(cx, base) { + if is_temporary(base) && !is_adjusted(cx, base) { span_lint(cx, TEMPORARY_ASSIGNMENT, expr.span, "assignment to temporary"); } } diff --git a/src/tools/clippy/clippy_lints/src/transmute.rs b/src/tools/clippy/clippy_lints/src/transmute.rs index 87c5408c785..c75adb62f25 100644 --- a/src/tools/clippy/clippy_lints/src/transmute.rs +++ b/src/tools/clippy/clippy_lints/src/transmute.rs @@ -331,8 +331,9 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { if let Some(def_id) = cx.qpath_res(qpath, path_expr.hir_id).opt_def_id(); if match_def_path(cx, def_id, &paths::TRANSMUTE); then { - // Avoid suggesting from/to bits in const contexts. + // Avoid suggesting from/to bits and dereferencing raw pointers in const contexts. // See https://github.com/rust-lang/rust/issues/73736 for progress on making them `const fn`. + // And see https://github.com/rust-lang/rust/issues/51911 for dereferencing raw pointers. let const_context = in_constant(cx, e.hir_id); let from_ty = cx.typeck_results().expr_ty(&args[0]); @@ -486,7 +487,8 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { Applicability::Unspecified, ); } else { - if cx.tcx.erase_regions(&from_ty) != cx.tcx.erase_regions(&to_ty) { + if (cx.tcx.erase_regions(&from_ty) != cx.tcx.erase_regions(&to_ty)) + && !const_context { span_lint_and_then( cx, TRANSMUTE_PTR_TO_PTR, diff --git a/src/tools/clippy/clippy_lints/src/types.rs b/src/tools/clippy/clippy_lints/src/types.rs index c82deaa43b2..6c6188d61ad 100644 --- a/src/tools/clippy/clippy_lints/src/types.rs +++ b/src/tools/clippy/clippy_lints/src/types.rs @@ -11,8 +11,8 @@ use rustc_hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitorMap, Visitor}; use rustc_hir::{ BinOpKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericParamKind, HirId, ImplItem, - ImplItemKind, Item, ItemKind, Lifetime, Local, MatchSource, MutTy, Mutability, QPath, Stmt, StmtKind, TraitFn, - TraitItem, TraitItemKind, TyKind, UnOp, + ImplItemKind, Item, ItemKind, Lifetime, Local, MatchSource, MutTy, Mutability, Node, QPath, Stmt, StmtKind, + TraitFn, TraitItem, TraitItemKind, TyKind, UnOp, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::hir::map::Map; @@ -31,12 +31,13 @@ use crate::utils::{ clip, comparisons, differing_macro_contexts, higher, in_constant, indent_of, int_bits, is_type_diagnostic_item, last_path_segment, match_def_path, match_path, method_chain_args, multispan_sugg, numeric_literal::NumericLiteral, - qpath_res, sext, snippet, snippet_block_with_applicability, snippet_opt, snippet_with_applicability, - snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, unsext, + qpath_res, reindent_multiline, sext, snippet, snippet_opt, snippet_with_applicability, snippet_with_macro_callsite, + span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, unsext, }; declare_clippy_lint! { /// **What it does:** Checks for use of `Box>` 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 @@ -65,6 +66,7 @@ declare_clippy_lint! { /// **What it does:** Checks for use of `Vec>` where T: Sized 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` its contents, you just add another level of indirection. @@ -167,6 +169,7 @@ declare_clippy_lint! { /// **What it does:** Checks for use of `&Box` anywhere in the code. + /// Check the [Box documentation](https://doc.rust-lang.org/std/boxed/index.html) for more information. /// /// **Why is this bad?** Any `&Box` can also be a `&T`, which is more /// general. @@ -802,6 +805,45 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { } } +fn fmt_stmts_and_call( + cx: &LateContext<'_>, + call_expr: &Expr<'_>, + call_snippet: &str, + args_snippets: &[impl AsRef], + non_empty_block_args_snippets: &[impl AsRef], +) -> String { + let call_expr_indent = indent_of(cx, call_expr.span).unwrap_or(0); + let call_snippet_with_replacements = args_snippets + .iter() + .fold(call_snippet.to_owned(), |acc, arg| acc.replacen(arg.as_ref(), "()", 1)); + + let mut stmts_and_call = non_empty_block_args_snippets + .iter() + .map(|it| it.as_ref().to_owned()) + .collect::>(); + stmts_and_call.push(call_snippet_with_replacements); + stmts_and_call = stmts_and_call + .into_iter() + .map(|v| reindent_multiline(v.into(), true, Some(call_expr_indent)).into_owned()) + .collect(); + + let mut stmts_and_call_snippet = stmts_and_call.join(&format!("{}{}", ";\n", " ".repeat(call_expr_indent))); + // expr is not in a block statement or result expression position, wrap in a block + let parent_node = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(call_expr.hir_id)); + if !matches!(parent_node, Some(Node::Block(_))) && !matches!(parent_node, Some(Node::Stmt(_))) { + let block_indent = call_expr_indent + 4; + stmts_and_call_snippet = + reindent_multiline(stmts_and_call_snippet.into(), true, Some(block_indent)).into_owned(); + stmts_and_call_snippet = format!( + "{{\n{}{}\n{}}}", + " ".repeat(block_indent), + &stmts_and_call_snippet, + " ".repeat(call_expr_indent) + ); + } + stmts_and_call_snippet +} + fn lint_unit_args(cx: &LateContext<'_>, expr: &Expr<'_>, args_to_recover: &[&Expr<'_>]) { let mut applicability = Applicability::MachineApplicable; let (singular, plural) = if args_to_recover.len() > 1 { @@ -844,43 +886,52 @@ fn lint_unit_args(cx: &LateContext<'_>, expr: &Expr<'_>, args_to_recover: &[&Exp Applicability::MaybeIncorrect, ); or = "or "; + applicability = Applicability::MaybeIncorrect; }); - let sugg = args_to_recover + + let arg_snippets: Vec = args_to_recover + .iter() + .filter_map(|arg| snippet_opt(cx, arg.span)) + .collect(); + let arg_snippets_without_empty_blocks: Vec = args_to_recover .iter() .filter(|arg| !is_empty_block(arg)) - .enumerate() - .map(|(i, arg)| { - let indent = if i == 0 { - 0 - } else { - indent_of(cx, expr.span).unwrap_or(0) - }; - format!( - "{}{};", - " ".repeat(indent), - snippet_block_with_applicability(cx, arg.span, "..", Some(expr.span), &mut applicability) - ) - }) - .collect::>(); - let mut and = ""; - if !sugg.is_empty() { - let plural = if sugg.len() > 1 { "s" } else { "" }; - db.span_suggestion( - expr.span.with_hi(expr.span.lo()), - &format!("{}move the expression{} in front of the call...", or, plural), - format!("{}\n", sugg.join("\n")), - applicability, + .filter_map(|arg| snippet_opt(cx, arg.span)) + .collect(); + + if let Some(call_snippet) = snippet_opt(cx, expr.span) { + let sugg = fmt_stmts_and_call( + cx, + expr, + &call_snippet, + &arg_snippets, + &arg_snippets_without_empty_blocks, ); - and = "...and " + + if arg_snippets_without_empty_blocks.is_empty() { + db.multipart_suggestion( + &format!("use {}unit literal{} instead", singular, plural), + args_to_recover + .iter() + .map(|arg| (arg.span, "()".to_string())) + .collect::>(), + applicability, + ); + } else { + let plural = arg_snippets_without_empty_blocks.len() > 1; + let empty_or_s = if plural { "s" } else { "" }; + let it_or_them = if plural { "them" } else { "it" }; + db.span_suggestion( + expr.span, + &format!( + "{}move the expression{} in front of the call and replace {} with the unit literal `()`", + or, empty_or_s, it_or_them + ), + sugg, + applicability, + ); + } } - db.multipart_suggestion( - &format!("{}use {}unit literal{} instead", and, singular, plural), - args_to_recover - .iter() - .map(|arg| (arg.span, "()".to_string())) - .collect::>(), - applicability, - ); }, ); } @@ -2055,6 +2106,7 @@ fn partial_cmp(&self, other: &Self) -> Option { }) } } + impl Ord for FullInt { #[must_use] fn cmp(&self, other: &Self) -> Ordering { diff --git a/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs b/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs index 8b00d29acb5..9b6a9075a29 100644 --- a/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs +++ b/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs @@ -1,5 +1,4 @@ use crate::utils; -use crate::utils::paths; use crate::utils::sugg::Sugg; use if_chain::if_chain; use rustc_errors::Applicability; @@ -171,12 +170,22 @@ fn mirrored_exprs( } fn detect_lint(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { + // NOTE: Vectors of references are not supported. In order to avoid hitting https://github.com/rust-lang/rust/issues/34162, + // (different unnamed lifetimes for closure arg and return type) we need to make sure the suggested + // closure parameter is not a reference in case we suggest `Reverse`. Trying to destructure more + // than one level of references would add some extra complexity as we would have to compensate + // in the closure body. + if_chain! { if let ExprKind::MethodCall(name_ident, _, args, _) = &expr.kind; if let name = name_ident.ident.name.to_ident_string(); if name == "sort_by" || name == "sort_unstable_by"; if let [vec, Expr { kind: ExprKind::Closure(_, _, closure_body_id, _, _), .. }] = args; - if utils::match_type(cx, &cx.typeck_results().expr_ty(vec), &paths::VEC); + let vec_ty = cx.typeck_results().expr_ty(vec); + if utils::is_type_diagnostic_item(cx, vec_ty, sym!(vec_type)); + let ty = vec_ty.walk().nth(1).unwrap().expect_ty(); // T in Vec + if !matches!(&ty.kind(), ty::Ref(..)); + if utils::is_copy(cx, ty); if let closure_body = cx.tcx.hir().body(*closure_body_id); if let &[ Param { pat: Pat { kind: PatKind::Binding(_, _, left_ident, _), .. }, ..}, diff --git a/src/tools/clippy/clippy_lints/src/utils/ast_utils.rs b/src/tools/clippy/clippy_lints/src/utils/ast_utils.rs index 3c3f8b26e3a..fa8dd210eba 100644 --- a/src/tools/clippy/clippy_lints/src/utils/ast_utils.rs +++ b/src/tools/clippy/clippy_lints/src/utils/ast_utils.rs @@ -191,7 +191,9 @@ pub fn eq_stmt(l: &Stmt, r: &Stmt) -> bool { (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)), + (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)) + }, _ => false, } } diff --git a/src/tools/clippy/clippy_lints/src/utils/conf.rs b/src/tools/clippy/clippy_lints/src/utils/conf.rs index 292dbd7ad6b..9c5a12ea9c8 100644 --- a/src/tools/clippy/clippy_lints/src/utils/conf.rs +++ b/src/tools/clippy/clippy_lints/src/utils/conf.rs @@ -122,7 +122,7 @@ fn $config() -> $Ty { "IPv4", "IPv6", "ClojureScript", "CoffeeScript", "JavaScript", "PureScript", "TypeScript", "NaN", "NaNs", - "OAuth", + "OAuth", "GraphQL", "OCaml", "OpenGL", "OpenMP", "OpenSSH", "OpenSSL", "OpenStreetMap", "TensorFlow", diff --git a/src/tools/clippy/clippy_lints/src/utils/mod.rs b/src/tools/clippy/clippy_lints/src/utils/mod.rs index 45add9ab284..3ebbfed6456 100644 --- a/src/tools/clippy/clippy_lints/src/utils/mod.rs +++ b/src/tools/clippy/clippy_lints/src/utils/mod.rs @@ -19,6 +19,7 @@ pub mod ptr; pub mod sugg; pub mod usage; + pub use self::attrs::*; pub use self::diagnostics::*; pub use self::hir_utils::{both, eq_expr_value, over, SpanlessEq, SpanlessHash}; @@ -108,6 +109,7 @@ pub fn in_macro(span: Span) -> bool { false } } + // If the snippet is empty, it's an attribute that was inserted during macro // expansion and we want to ignore those, because they could come from external // sources that the user has no control over. @@ -571,7 +573,7 @@ pub fn snippet_block<'a, T: LintContext>( ) -> Cow<'a, str> { let snip = snippet(cx, span, default); let indent = indent_relative_to.and_then(|s| indent_of(cx, s)); - trim_multiline(snip, true, indent) + reindent_multiline(snip, true, indent) } /// Same as `snippet_block`, but adapts the applicability level by the rules of @@ -585,7 +587,7 @@ pub fn snippet_block_with_applicability<'a, T: LintContext>( ) -> Cow<'a, str> { let snip = snippet_with_applicability(cx, span, default, applicability); let indent = indent_relative_to.and_then(|s| indent_of(cx, s)); - trim_multiline(snip, true, indent) + reindent_multiline(snip, true, indent) } /// Returns a new Span that extends the original Span to the first non-whitespace char of the first @@ -661,16 +663,16 @@ pub fn expr_block<'a, T: LintContext>( } } -/// Trim indentation from a multiline string with possibility of ignoring the -/// first line. -fn trim_multiline(s: Cow<'_, str>, ignore_first: bool, indent: Option) -> Cow<'_, str> { - let s_space = trim_multiline_inner(s, ignore_first, indent, ' '); - let s_tab = trim_multiline_inner(s_space, ignore_first, indent, '\t'); - trim_multiline_inner(s_tab, ignore_first, indent, ' ') +/// Reindent a multiline string with possibility of ignoring the first line. +#[allow(clippy::needless_pass_by_value)] +pub fn reindent_multiline(s: Cow<'_, str>, ignore_first: bool, indent: Option) -> Cow<'_, str> { + let s_space = reindent_multiline_inner(&s, ignore_first, indent, ' '); + let s_tab = reindent_multiline_inner(&s_space, ignore_first, indent, '\t'); + reindent_multiline_inner(&s_tab, ignore_first, indent, ' ').into() } -fn trim_multiline_inner(s: Cow<'_, str>, ignore_first: bool, indent: Option, ch: char) -> Cow<'_, str> { - let mut x = s +fn reindent_multiline_inner(s: &str, ignore_first: bool, indent: Option, ch: char) -> String { + let x = s .lines() .skip(ignore_first as usize) .filter_map(|l| { @@ -683,26 +685,20 @@ fn trim_multiline_inner(s: Cow<'_, str>, ignore_first: bool, indent: Option 0 { - Cow::Owned( - s.lines() - .enumerate() - .map(|(i, l)| { - if (ignore_first && i == 0) || l.is_empty() { - l - } else { - l.split_at(x).1 - } - }) - .collect::>() - .join("\n"), - ) - } else { - s - } + let indent = indent.unwrap_or(0); + s.lines() + .enumerate() + .map(|(i, l)| { + if (ignore_first && i == 0) || l.is_empty() { + l.to_owned() + } else if x > indent { + l.split_at(x - indent).1.to_owned() + } else { + " ".repeat(indent - x) + l + } + }) + .collect::>() + .join("\n") } /// Gets the parent expression, if any –- this is useful to constrain a lint. @@ -899,7 +895,7 @@ fn has_no_arguments(cx: &LateContext<'_>, def_id: DefId) -> bool { return match res { def::Res::Def(DefKind::Variant | DefKind::Ctor(..), ..) => true, // FIXME: check the constness of the arguments, see https://github.com/rust-lang/rust-clippy/pull/5682#issuecomment-638681210 - def::Res::Def(DefKind::Fn, def_id) if has_no_arguments(cx, def_id) => { + def::Res::Def(DefKind::Fn | DefKind::AssocFn, def_id) if has_no_arguments(cx, def_id) => { const_eval::is_const_fn(cx.tcx, def_id) }, def::Res::Def(_, def_id) => cx.tcx.is_promotable_const_fn(def_id), @@ -1432,7 +1428,7 @@ pub fn is_slice_of_primitives(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option false, }; @@ -1474,26 +1470,26 @@ macro_rules! unwrap_cargo_metadata { #[cfg(test)] mod test { - use super::{trim_multiline, without_block_comments}; + use super::{reindent_multiline, without_block_comments}; #[test] - fn test_trim_multiline_single_line() { - assert_eq!("", trim_multiline("".into(), false, None)); - assert_eq!("...", trim_multiline("...".into(), false, None)); - assert_eq!("...", trim_multiline(" ...".into(), false, None)); - assert_eq!("...", trim_multiline("\t...".into(), false, None)); - assert_eq!("...", trim_multiline("\t\t...".into(), false, None)); + fn test_reindent_multiline_single_line() { + assert_eq!("", reindent_multiline("".into(), false, None)); + assert_eq!("...", reindent_multiline("...".into(), false, None)); + assert_eq!("...", reindent_multiline(" ...".into(), false, None)); + assert_eq!("...", reindent_multiline("\t...".into(), false, None)); + assert_eq!("...", reindent_multiline("\t\t...".into(), false, None)); } #[test] #[rustfmt::skip] - fn test_trim_multiline_block() { + fn test_reindent_multiline_block() { assert_eq!("\ if x { y } else { z - }", trim_multiline(" if x { + }", reindent_multiline(" if x { y } else { z @@ -1503,7 +1499,7 @@ fn test_trim_multiline_block() { \ty } else { \tz - }", trim_multiline(" if x { + }", reindent_multiline(" if x { \ty } else { \tz @@ -1512,14 +1508,14 @@ fn test_trim_multiline_block() { #[test] #[rustfmt::skip] - fn test_trim_multiline_empty_line() { + fn test_reindent_multiline_empty_line() { assert_eq!("\ if x { y } else { z - }", trim_multiline(" if x { + }", reindent_multiline(" if x { y } else { @@ -1527,6 +1523,22 @@ fn test_trim_multiline_empty_line() { }".into(), false, None)); } + #[test] + #[rustfmt::skip] + fn test_reindent_multiline_lines_deeper() { + assert_eq!("\ + if x { + y + } else { + z + }", reindent_multiline("\ + if x { + y + } else { + z + }".into(), true, Some(8))); + } + #[test] fn test_without_block_comments_lines_without_block_comments() { let result = without_block_comments(vec!["/*", "", "*/"]); diff --git a/src/tools/clippy/clippy_lints/src/utils/paths.rs b/src/tools/clippy/clippy_lints/src/utils/paths.rs index d44854aefe9..65320d6a0e0 100644 --- a/src/tools/clippy/clippy_lints/src/utils/paths.rs +++ b/src/tools/clippy/clippy_lints/src/utils/paths.rs @@ -110,6 +110,7 @@ pub const STDERR: [&str; 4] = ["std", "io", "stdio", "stderr"]; pub const STDOUT: [&str; 4] = ["std", "io", "stdio", "stdout"]; pub const STD_CONVERT_IDENTITY: [&str; 3] = ["std", "convert", "identity"]; +pub const STD_FS_CREATE_DIR: [&str; 3] = ["std", "fs", "create_dir"]; pub const STD_MEM_TRANSMUTE: [&str; 3] = ["std", "mem", "transmute"]; pub const STD_PTR_NULL: [&str; 3] = ["std", "ptr", "null"]; pub const STRING_AS_MUT_STR: [&str; 4] = ["alloc", "string", "String", "as_mut_str"]; diff --git a/src/tools/clippy/clippy_lints/src/utils/sugg.rs b/src/tools/clippy/clippy_lints/src/utils/sugg.rs index 2955f8d8e59..811fde388d1 100644 --- a/src/tools/clippy/clippy_lints/src/utils/sugg.rs +++ b/src/tools/clippy/clippy_lints/src/utils/sugg.rs @@ -132,7 +132,11 @@ fn hir_from_snippet(expr: &hir::Expr<'_>, snippet: Cow<'a, str>) -> Self { pub fn ast(cx: &EarlyContext<'_>, expr: &ast::Expr, default: &'a str) -> Self { use rustc_ast::ast::RangeLimits; - let snippet = snippet(cx, expr.span, default); + let snippet = if expr.span.from_expansion() { + snippet_with_macro_callsite(cx, expr.span, default) + } else { + snippet(cx, expr.span, default) + }; match expr.kind { ast::ExprKind::AddrOf(..) diff --git a/src/tools/clippy/src/lintlist/mod.rs b/src/tools/clippy/src/lintlist/mod.rs index 687fac7baa8..6697835e950 100644 --- a/src/tools/clippy/src/lintlist/mod.rs +++ b/src/tools/clippy/src/lintlist/mod.rs @@ -52,6 +52,13 @@ deprecation: None, module: "assign_ops", }, + Lint { + name: "async_yields_async", + group: "correctness", + desc: "async blocks that return a type that can be awaited", + deprecation: None, + module: "async_yields_async", + }, Lint { name: "await_holding_lock", group: "pedantic", @@ -290,6 +297,13 @@ deprecation: None, module: "copy_iterator", }, + Lint { + name: "create_dir", + group: "restriction", + desc: "calling `std::fs::create_dir` instead of `std::fs::create_dir_all`", + deprecation: None, + module: "create_dir", + }, Lint { name: "crosspointer_transmute", group: "complexity", diff --git a/src/tools/clippy/tests/ui-toml/functions_maxlines/test.stderr b/src/tools/clippy/tests/ui-toml/functions_maxlines/test.stderr index fb12257021a..a27ce945ca5 100644 --- a/src/tools/clippy/tests/ui-toml/functions_maxlines/test.stderr +++ b/src/tools/clippy/tests/ui-toml/functions_maxlines/test.stderr @@ -1,4 +1,4 @@ -error: this function has a large number of lines +error: this function has too many lines (2/1) --> $DIR/test.rs:18:1 | LL | / fn too_many_lines() { @@ -9,7 +9,7 @@ LL | | } | = note: `-D clippy::too-many-lines` implied by `-D warnings` -error: this function has a large number of lines +error: this function has too many lines (2/1) --> $DIR/test.rs:38:1 | LL | / fn comment_before_code() { diff --git a/src/tools/clippy/tests/ui/async_yields_async.fixed b/src/tools/clippy/tests/ui/async_yields_async.fixed new file mode 100644 index 00000000000..9b1a7ac3ba9 --- /dev/null +++ b/src/tools/clippy/tests/ui/async_yields_async.fixed @@ -0,0 +1,68 @@ +// run-rustfix +// edition:2018 + +#![feature(async_closure)] +#![warn(clippy::async_yields_async)] + +use core::future::Future; +use core::pin::Pin; +use core::task::{Context, Poll}; + +struct CustomFutureType; + +impl Future for CustomFutureType { + type Output = u8; + + fn poll(self: Pin<&mut Self>, _: &mut Context) -> Poll { + Poll::Ready(3) + } +} + +fn custom_future_type_ctor() -> CustomFutureType { + CustomFutureType +} + +async fn f() -> CustomFutureType { + // Don't warn for functions since you have to explicitly declare their + // return types. + CustomFutureType +} + +#[rustfmt::skip] +fn main() { + let _f = { + 3 + }; + let _g = async { + 3 + }; + let _h = async { + async { + 3 + }.await + }; + let _i = async { + CustomFutureType.await + }; + let _i = async || { + 3 + }; + let _j = async || { + async { + 3 + }.await + }; + let _k = async || { + CustomFutureType.await + }; + let _l = async || CustomFutureType.await; + let _m = async || { + println!("I'm bored"); + // Some more stuff + + // Finally something to await + CustomFutureType.await + }; + let _n = async || custom_future_type_ctor(); + let _o = async || f(); +} diff --git a/src/tools/clippy/tests/ui/async_yields_async.rs b/src/tools/clippy/tests/ui/async_yields_async.rs new file mode 100644 index 00000000000..731c094edb4 --- /dev/null +++ b/src/tools/clippy/tests/ui/async_yields_async.rs @@ -0,0 +1,68 @@ +// run-rustfix +// edition:2018 + +#![feature(async_closure)] +#![warn(clippy::async_yields_async)] + +use core::future::Future; +use core::pin::Pin; +use core::task::{Context, Poll}; + +struct CustomFutureType; + +impl Future for CustomFutureType { + type Output = u8; + + fn poll(self: Pin<&mut Self>, _: &mut Context) -> Poll { + Poll::Ready(3) + } +} + +fn custom_future_type_ctor() -> CustomFutureType { + CustomFutureType +} + +async fn f() -> CustomFutureType { + // Don't warn for functions since you have to explicitly declare their + // return types. + CustomFutureType +} + +#[rustfmt::skip] +fn main() { + let _f = { + 3 + }; + let _g = async { + 3 + }; + let _h = async { + async { + 3 + } + }; + let _i = async { + CustomFutureType + }; + let _i = async || { + 3 + }; + let _j = async || { + async { + 3 + } + }; + let _k = async || { + CustomFutureType + }; + let _l = async || CustomFutureType; + let _m = async || { + println!("I'm bored"); + // Some more stuff + + // Finally something to await + CustomFutureType + }; + let _n = async || custom_future_type_ctor(); + let _o = async || f(); +} diff --git a/src/tools/clippy/tests/ui/async_yields_async.stderr b/src/tools/clippy/tests/ui/async_yields_async.stderr new file mode 100644 index 00000000000..17d0c375106 --- /dev/null +++ b/src/tools/clippy/tests/ui/async_yields_async.stderr @@ -0,0 +1,96 @@ +error: an async construct yields a type which is itself awaitable + --> $DIR/async_yields_async.rs:40:9 + | +LL | let _h = async { + | ____________________- +LL | | async { + | |_________^ +LL | || 3 +LL | || } + | ||_________^ awaitable value not awaited +LL | | }; + | |_____- outer async construct + | + = note: `-D clippy::async-yields-async` implied by `-D warnings` +help: consider awaiting this value + | +LL | async { +LL | 3 +LL | }.await + | + +error: an async construct yields a type which is itself awaitable + --> $DIR/async_yields_async.rs:45:9 + | +LL | let _i = async { + | ____________________- +LL | | CustomFutureType + | | ^^^^^^^^^^^^^^^^ + | | | + | | awaitable value not awaited + | | help: consider awaiting this value: `CustomFutureType.await` +LL | | }; + | |_____- outer async construct + +error: an async construct yields a type which is itself awaitable + --> $DIR/async_yields_async.rs:51:9 + | +LL | let _j = async || { + | _______________________- +LL | | async { + | |_________^ +LL | || 3 +LL | || } + | ||_________^ awaitable value not awaited +LL | | }; + | |_____- outer async construct + | +help: consider awaiting this value + | +LL | async { +LL | 3 +LL | }.await + | + +error: an async construct yields a type which is itself awaitable + --> $DIR/async_yields_async.rs:56:9 + | +LL | let _k = async || { + | _______________________- +LL | | CustomFutureType + | | ^^^^^^^^^^^^^^^^ + | | | + | | awaitable value not awaited + | | help: consider awaiting this value: `CustomFutureType.await` +LL | | }; + | |_____- outer async construct + +error: an async construct yields a type which is itself awaitable + --> $DIR/async_yields_async.rs:58:23 + | +LL | let _l = async || CustomFutureType; + | ^^^^^^^^^^^^^^^^ + | | + | outer async construct + | awaitable value not awaited + | help: consider awaiting this value: `CustomFutureType.await` + +error: an async construct yields a type which is itself awaitable + --> $DIR/async_yields_async.rs:64:9 + | +LL | let _m = async || { + | _______________________- +LL | | println!("I'm bored"); +LL | | // Some more stuff +LL | | +LL | | // Finally something to await +LL | | CustomFutureType + | | ^^^^^^^^^^^^^^^^ + | | | + | | awaitable value not awaited + | | help: consider awaiting this value: `CustomFutureType.await` +LL | | }; + | |_____- outer async construct + +error: aborting due to 6 previous errors + diff --git a/src/tools/clippy/tests/ui/collapsible_if.fixed b/src/tools/clippy/tests/ui/collapsible_if.fixed index 561283fc8e7..efd4187947b 100644 --- a/src/tools/clippy/tests/ui/collapsible_if.fixed +++ b/src/tools/clippy/tests/ui/collapsible_if.fixed @@ -135,4 +135,7 @@ fn main() { if truth() {} } } + + // Fix #5962 + if matches!(true, true) && matches!(true, true) {} } diff --git a/src/tools/clippy/tests/ui/collapsible_if.rs b/src/tools/clippy/tests/ui/collapsible_if.rs index dc9d9b451c0..657f32d38a3 100644 --- a/src/tools/clippy/tests/ui/collapsible_if.rs +++ b/src/tools/clippy/tests/ui/collapsible_if.rs @@ -149,4 +149,9 @@ fn truth() -> bool { true } if truth() {} } } + + // Fix #5962 + if matches!(true, true) { + if matches!(true, true) {} + } } diff --git a/src/tools/clippy/tests/ui/collapsible_if.stderr b/src/tools/clippy/tests/ui/collapsible_if.stderr index f56dd65b9dd..acd1ec3f2ca 100644 --- a/src/tools/clippy/tests/ui/collapsible_if.stderr +++ b/src/tools/clippy/tests/ui/collapsible_if.stderr @@ -118,5 +118,13 @@ LL | println!("Hello world!"); LL | } | -error: aborting due to 7 previous errors +error: this `if` statement can be collapsed + --> $DIR/collapsible_if.rs:154:5 + | +LL | / if matches!(true, true) { +LL | | if matches!(true, true) {} +LL | | } + | |_____^ help: collapse nested if block: `if matches!(true, true) && matches!(true, true) {}` + +error: aborting due to 8 previous errors diff --git a/src/tools/clippy/tests/ui/create_dir.fixed b/src/tools/clippy/tests/ui/create_dir.fixed new file mode 100644 index 00000000000..8ed53a56ac0 --- /dev/null +++ b/src/tools/clippy/tests/ui/create_dir.fixed @@ -0,0 +1,17 @@ +// run-rustfix +#![allow(unused_must_use)] +#![warn(clippy::create_dir)] + +use std::fs::create_dir_all; + +fn create_dir() {} + +fn main() { + // Should be warned + create_dir_all("foo"); + create_dir_all("bar").unwrap(); + + // Shouldn't be warned + create_dir(); + std::fs::create_dir_all("foobar"); +} diff --git a/src/tools/clippy/tests/ui/create_dir.rs b/src/tools/clippy/tests/ui/create_dir.rs new file mode 100644 index 00000000000..19c8fc24ba2 --- /dev/null +++ b/src/tools/clippy/tests/ui/create_dir.rs @@ -0,0 +1,17 @@ +// run-rustfix +#![allow(unused_must_use)] +#![warn(clippy::create_dir)] + +use std::fs::create_dir_all; + +fn create_dir() {} + +fn main() { + // Should be warned + std::fs::create_dir("foo"); + std::fs::create_dir("bar").unwrap(); + + // Shouldn't be warned + create_dir(); + std::fs::create_dir_all("foobar"); +} diff --git a/src/tools/clippy/tests/ui/create_dir.stderr b/src/tools/clippy/tests/ui/create_dir.stderr new file mode 100644 index 00000000000..67298fc4709 --- /dev/null +++ b/src/tools/clippy/tests/ui/create_dir.stderr @@ -0,0 +1,16 @@ +error: calling `std::fs::create_dir` where there may be a better way + --> $DIR/create_dir.rs:11:5 + | +LL | std::fs::create_dir("foo"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `std::fs::create_dir_all` instead: `create_dir_all("foo")` + | + = note: `-D clippy::create-dir` implied by `-D warnings` + +error: calling `std::fs::create_dir` where there may be a better way + --> $DIR/create_dir.rs:12:5 + | +LL | std::fs::create_dir("bar").unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling `std::fs::create_dir_all` instead: `create_dir_all("bar")` + +error: aborting due to 2 previous errors + diff --git a/src/tools/clippy/tests/ui/default_trait_access.fixed b/src/tools/clippy/tests/ui/default_trait_access.fixed new file mode 100644 index 00000000000..d05567a3f82 --- /dev/null +++ b/src/tools/clippy/tests/ui/default_trait_access.fixed @@ -0,0 +1,106 @@ +// run-rustfix + +#![allow(unused_imports)] +#![deny(clippy::default_trait_access)] + +use std::default; +use std::default::Default as D2; +use std::string; + +fn main() { + let s1: String = std::string::String::default(); + + let s2 = String::default(); + + let s3: String = std::string::String::default(); + + let s4: String = std::string::String::default(); + + let s5 = string::String::default(); + + let s6: String = std::string::String::default(); + + let s7 = std::string::String::default(); + + let s8: String = DefaultFactory::make_t_badly(); + + let s9: String = DefaultFactory::make_t_nicely(); + + let s10 = DerivedDefault::default(); + + let s11: GenericDerivedDefault = GenericDerivedDefault::default(); + + let s12 = GenericDerivedDefault::::default(); + + let s13 = TupleDerivedDefault::default(); + + let s14: TupleDerivedDefault = TupleDerivedDefault::default(); + + let s15: ArrayDerivedDefault = ArrayDerivedDefault::default(); + + let s16 = ArrayDerivedDefault::default(); + + let s17: TupleStructDerivedDefault = TupleStructDerivedDefault::default(); + + let s18 = TupleStructDerivedDefault::default(); + + let s19 = ::default(); + + println!( + "[{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}], [{:?}]", + s1, + s2, + s3, + s4, + s5, + s6, + s7, + s8, + s9, + s10, + s11, + s12, + s13, + s14, + s15, + s16, + s17, + s18, + s19, + ); +} + +struct DefaultFactory; + +impl DefaultFactory { + pub fn make_t_badly() -> T { + Default::default() + } + + pub fn make_t_nicely() -> T { + T::default() + } +} + +#[derive(Debug, Default)] +struct DerivedDefault { + pub s: String, +} + +#[derive(Debug, Default)] +struct GenericDerivedDefault { + pub s: T, +} + +#[derive(Debug, Default)] +struct TupleDerivedDefault { + pub s: (String, String), +} + +#[derive(Debug, Default)] +struct ArrayDerivedDefault { + pub s: [String; 10], +} + +#[derive(Debug, Default)] +struct TupleStructDerivedDefault(String); diff --git a/src/tools/clippy/tests/ui/default_trait_access.rs b/src/tools/clippy/tests/ui/default_trait_access.rs index 2f1490a7036..447e70c0bbb 100644 --- a/src/tools/clippy/tests/ui/default_trait_access.rs +++ b/src/tools/clippy/tests/ui/default_trait_access.rs @@ -1,4 +1,7 @@ -#![warn(clippy::default_trait_access)] +// run-rustfix + +#![allow(unused_imports)] +#![deny(clippy::default_trait_access)] use std::default; use std::default::Default as D2; diff --git a/src/tools/clippy/tests/ui/default_trait_access.stderr b/src/tools/clippy/tests/ui/default_trait_access.stderr index 26b2057548b..df8a5b94ddc 100644 --- a/src/tools/clippy/tests/ui/default_trait_access.stderr +++ b/src/tools/clippy/tests/ui/default_trait_access.stderr @@ -1,49 +1,53 @@ error: calling `std::string::String::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:8:22 + --> $DIR/default_trait_access.rs:11:22 | LL | let s1: String = Default::default(); | ^^^^^^^^^^^^^^^^^^ help: try: `std::string::String::default()` | - = note: `-D clippy::default-trait-access` implied by `-D warnings` +note: the lint level is defined here + --> $DIR/default_trait_access.rs:4:9 + | +LL | #![deny(clippy::default_trait_access)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: calling `std::string::String::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:12:22 + --> $DIR/default_trait_access.rs:15:22 | LL | let s3: String = D2::default(); | ^^^^^^^^^^^^^ help: try: `std::string::String::default()` error: calling `std::string::String::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:14:22 + --> $DIR/default_trait_access.rs:17:22 | LL | let s4: String = std::default::Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::string::String::default()` error: calling `std::string::String::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:18:22 + --> $DIR/default_trait_access.rs:21:22 | LL | let s6: String = default::Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::string::String::default()` -error: calling `GenericDerivedDefault::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:28:46 +error: calling `GenericDerivedDefault::default()` is more clear than this expression + --> $DIR/default_trait_access.rs:31:46 | LL | let s11: GenericDerivedDefault = Default::default(); - | ^^^^^^^^^^^^^^^^^^ help: try: `GenericDerivedDefault::default()` + | ^^^^^^^^^^^^^^^^^^ help: try: `GenericDerivedDefault::default()` error: calling `TupleDerivedDefault::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:34:36 + --> $DIR/default_trait_access.rs:37:36 | LL | let s14: TupleDerivedDefault = Default::default(); | ^^^^^^^^^^^^^^^^^^ help: try: `TupleDerivedDefault::default()` error: calling `ArrayDerivedDefault::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:36:36 + --> $DIR/default_trait_access.rs:39:36 | LL | let s15: ArrayDerivedDefault = Default::default(); | ^^^^^^^^^^^^^^^^^^ help: try: `ArrayDerivedDefault::default()` error: calling `TupleStructDerivedDefault::default()` is more clear than this expression - --> $DIR/default_trait_access.rs:40:42 + --> $DIR/default_trait_access.rs:43:42 | LL | let s17: TupleStructDerivedDefault = Default::default(); | ^^^^^^^^^^^^^^^^^^ help: try: `TupleStructDerivedDefault::default()` diff --git a/src/tools/clippy/tests/ui/doc.rs b/src/tools/clippy/tests/ui/doc.rs index 77620c857e6..68c5d32846f 100644 --- a/src/tools/clippy/tests/ui/doc.rs +++ b/src/tools/clippy/tests/ui/doc.rs @@ -49,6 +49,16 @@ fn test_emphasis() { fn test_units() { } +/// This tests allowed identifiers. +/// DirectX +/// ECMAScript +/// OAuth GraphQL +/// TeX LaTeX BibTeX BibLaTeX +/// CamelCase (see also #2395) +/// be_sure_we_got_to_the_end_of_it +fn test_allowed() { +} + /// This test has [a link_with_underscores][chunked-example] inside it. See #823. /// See also [the issue tracker](https://github.com/rust-lang/rust-clippy/search?q=clippy::doc_markdown&type=Issues) /// on GitHub (which is a camel-cased word, but is OK). And here is another [inline link][inline_link]. @@ -168,9 +178,6 @@ fn issue_1920() {} /// Not ok: http://www.unicode.org/reports/tr9/#Reordering_Resolved_Levels fn issue_1832() {} -/// Ok: CamelCase (It should not be surrounded by backticks) -fn issue_2395() {} - /// An iterator over mycrate::Collection's values. /// It should not lint a `'static` lifetime in ticks. fn issue_2210() {} diff --git a/src/tools/clippy/tests/ui/doc.stderr b/src/tools/clippy/tests/ui/doc.stderr index ae9bb394cb9..23fca43590b 100644 --- a/src/tools/clippy/tests/ui/doc.stderr +++ b/src/tools/clippy/tests/ui/doc.stderr @@ -54,131 +54,137 @@ error: you should put `be_sure_we_got_to_the_end_of_it` between ticks in the doc LL | /// be_sure_we_got_to_the_end_of_it | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +error: you should put `be_sure_we_got_to_the_end_of_it` between ticks in the documentation + --> $DIR/doc.rs:58:5 + | +LL | /// be_sure_we_got_to_the_end_of_it + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + error: you should put `link_with_underscores` between ticks in the documentation - --> $DIR/doc.rs:52:22 + --> $DIR/doc.rs:62:22 | LL | /// This test has [a link_with_underscores][chunked-example] inside it. See #823. | ^^^^^^^^^^^^^^^^^^^^^ error: you should put `inline_link2` between ticks in the documentation - --> $DIR/doc.rs:55:21 + --> $DIR/doc.rs:65:21 | LL | /// It can also be [inline_link2]. | ^^^^^^^^^^^^ error: you should put `be_sure_we_got_to_the_end_of_it` between ticks in the documentation - --> $DIR/doc.rs:65:5 + --> $DIR/doc.rs:75:5 | LL | /// be_sure_we_got_to_the_end_of_it | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: you should put `CamelCaseThing` between ticks in the documentation - --> $DIR/doc.rs:73:8 + --> $DIR/doc.rs:83:8 | LL | /// ## CamelCaseThing | ^^^^^^^^^^^^^^ error: you should put `CamelCaseThing` between ticks in the documentation - --> $DIR/doc.rs:76:7 + --> $DIR/doc.rs:86:7 | LL | /// # CamelCaseThing | ^^^^^^^^^^^^^^ error: you should put `CamelCaseThing` between ticks in the documentation - --> $DIR/doc.rs:78:22 + --> $DIR/doc.rs:88:22 | LL | /// Not a title #897 CamelCaseThing | ^^^^^^^^^^^^^^ error: you should put `be_sure_we_got_to_the_end_of_it` between ticks in the documentation - --> $DIR/doc.rs:79:5 + --> $DIR/doc.rs:89:5 | LL | /// be_sure_we_got_to_the_end_of_it | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: you should put `be_sure_we_got_to_the_end_of_it` between ticks in the documentation - --> $DIR/doc.rs:86:5 + --> $DIR/doc.rs:96:5 | LL | /// be_sure_we_got_to_the_end_of_it | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: you should put `be_sure_we_got_to_the_end_of_it` between ticks in the documentation - --> $DIR/doc.rs:99:5 + --> $DIR/doc.rs:109:5 | LL | /// be_sure_we_got_to_the_end_of_it | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: you should put `FooBar` between ticks in the documentation - --> $DIR/doc.rs:110:43 + --> $DIR/doc.rs:120:43 | LL | /** E.g., serialization of an empty list: FooBar | ^^^^^^ error: you should put `BarQuz` between ticks in the documentation - --> $DIR/doc.rs:115:5 + --> $DIR/doc.rs:125:5 | LL | And BarQuz too. | ^^^^^^ error: you should put `be_sure_we_got_to_the_end_of_it` between ticks in the documentation - --> $DIR/doc.rs:116:1 + --> $DIR/doc.rs:126:1 | LL | be_sure_we_got_to_the_end_of_it | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: you should put `FooBar` between ticks in the documentation - --> $DIR/doc.rs:121:43 + --> $DIR/doc.rs:131:43 | LL | /** E.g., serialization of an empty list: FooBar | ^^^^^^ error: you should put `BarQuz` between ticks in the documentation - --> $DIR/doc.rs:126:5 + --> $DIR/doc.rs:136:5 | LL | And BarQuz too. | ^^^^^^ error: you should put `be_sure_we_got_to_the_end_of_it` between ticks in the documentation - --> $DIR/doc.rs:127:1 + --> $DIR/doc.rs:137:1 | LL | be_sure_we_got_to_the_end_of_it | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: you should put `be_sure_we_got_to_the_end_of_it` between ticks in the documentation - --> $DIR/doc.rs:138:5 + --> $DIR/doc.rs:148:5 | LL | /// be_sure_we_got_to_the_end_of_it | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: you should put bare URLs between `<`/`>` or make a proper Markdown link - --> $DIR/doc.rs:165:13 + --> $DIR/doc.rs:175:13 | LL | /// Not ok: http://www.unicode.org | ^^^^^^^^^^^^^^^^^^^^^^ error: you should put bare URLs between `<`/`>` or make a proper Markdown link - --> $DIR/doc.rs:166:13 + --> $DIR/doc.rs:176:13 | LL | /// Not ok: https://www.unicode.org | ^^^^^^^^^^^^^^^^^^^^^^^ error: you should put bare URLs between `<`/`>` or make a proper Markdown link - --> $DIR/doc.rs:167:13 + --> $DIR/doc.rs:177:13 | LL | /// Not ok: http://www.unicode.org/ | ^^^^^^^^^^^^^^^^^^^^^^ error: you should put bare URLs between `<`/`>` or make a proper Markdown link - --> $DIR/doc.rs:168:13 + --> $DIR/doc.rs:178:13 | LL | /// Not ok: http://www.unicode.org/reports/tr9/#Reordering_Resolved_Levels | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: you should put `mycrate::Collection` between ticks in the documentation - --> $DIR/doc.rs:174:22 + --> $DIR/doc.rs:181:22 | LL | /// An iterator over mycrate::Collection's values. | ^^^^^^^^^^^^^^^^^^^ -error: aborting due to 30 previous errors +error: aborting due to 31 previous errors diff --git a/src/tools/clippy/tests/ui/functions_maxlines.stderr b/src/tools/clippy/tests/ui/functions_maxlines.stderr index c640c82d6d7..dc6c8ba2f15 100644 --- a/src/tools/clippy/tests/ui/functions_maxlines.stderr +++ b/src/tools/clippy/tests/ui/functions_maxlines.stderr @@ -1,4 +1,4 @@ -error: this function has a large number of lines +error: this function has too many lines (102/100) --> $DIR/functions_maxlines.rs:58:1 | LL | / fn bad_lines() { diff --git a/src/tools/clippy/tests/ui/option_map_unit_fn_fixable.stderr b/src/tools/clippy/tests/ui/option_map_unit_fn_fixable.stderr index 1312c70b6d5..d7d45ef9b0b 100644 --- a/src/tools/clippy/tests/ui/option_map_unit_fn_fixable.stderr +++ b/src/tools/clippy/tests/ui/option_map_unit_fn_fixable.stderr @@ -1,4 +1,4 @@ -error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:38:5 | LL | x.field.map(do_nothing); @@ -8,7 +8,7 @@ LL | x.field.map(do_nothing); | = note: `-D clippy::option-map-unit-fn` implied by `-D warnings` -error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:40:5 | LL | x.field.map(do_nothing); @@ -16,7 +16,7 @@ LL | x.field.map(do_nothing); | | | help: try this: `if let Some(x_field) = x.field { do_nothing(x_field) }` -error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:42:5 | LL | x.field.map(diverge); @@ -24,7 +24,7 @@ LL | x.field.map(diverge); | | | help: try this: `if let Some(x_field) = x.field { diverge(x_field) }` -error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:48:5 | LL | x.field.map(|value| x.do_option_nothing(value + captured)); @@ -32,7 +32,7 @@ LL | x.field.map(|value| x.do_option_nothing(value + captured)); | | | help: try this: `if let Some(value) = x.field { x.do_option_nothing(value + captured) }` -error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:50:5 | LL | x.field.map(|value| { x.do_option_plus_one(value + captured); }); @@ -40,7 +40,7 @@ LL | x.field.map(|value| { x.do_option_plus_one(value + captured); }); | | | help: try this: `if let Some(value) = x.field { x.do_option_plus_one(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:53:5 | LL | x.field.map(|value| do_nothing(value + captured)); @@ -48,7 +48,7 @@ LL | x.field.map(|value| do_nothing(value + captured)); | | | help: try this: `if let Some(value) = x.field { do_nothing(value + captured) }` -error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:55:5 | LL | x.field.map(|value| { do_nothing(value + captured) }); @@ -56,7 +56,7 @@ LL | x.field.map(|value| { do_nothing(value + captured) }); | | | help: try this: `if let Some(value) = x.field { do_nothing(value + captured) }` -error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:57:5 | LL | x.field.map(|value| { do_nothing(value + captured); }); @@ -64,7 +64,7 @@ LL | x.field.map(|value| { do_nothing(value + captured); }); | | | help: try this: `if let Some(value) = x.field { do_nothing(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:59:5 | LL | x.field.map(|value| { { do_nothing(value + captured); } }); @@ -72,7 +72,7 @@ LL | x.field.map(|value| { { do_nothing(value + captured); } }); | | | help: try this: `if let Some(value) = x.field { do_nothing(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:62:5 | LL | x.field.map(|value| diverge(value + captured)); @@ -80,7 +80,7 @@ LL | x.field.map(|value| diverge(value + captured)); | | | help: try this: `if let Some(value) = x.field { diverge(value + captured) }` -error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:64:5 | LL | x.field.map(|value| { diverge(value + captured) }); @@ -88,7 +88,7 @@ LL | x.field.map(|value| { diverge(value + captured) }); | | | help: try this: `if let Some(value) = x.field { diverge(value + captured) }` -error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:66:5 | LL | x.field.map(|value| { diverge(value + captured); }); @@ -96,7 +96,7 @@ LL | x.field.map(|value| { diverge(value + captured); }); | | | help: try this: `if let Some(value) = x.field { diverge(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:68:5 | LL | x.field.map(|value| { { diverge(value + captured); } }); @@ -104,7 +104,7 @@ LL | x.field.map(|value| { { diverge(value + captured); } }); | | | help: try this: `if let Some(value) = x.field { diverge(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:73:5 | LL | x.field.map(|value| { let y = plus_one(value + captured); }); @@ -112,7 +112,7 @@ LL | x.field.map(|value| { let y = plus_one(value + captured); }); | | | help: try this: `if let Some(value) = x.field { let y = plus_one(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:75:5 | LL | x.field.map(|value| { plus_one(value + captured); }); @@ -120,7 +120,7 @@ LL | x.field.map(|value| { plus_one(value + captured); }); | | | help: try this: `if let Some(value) = x.field { plus_one(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:77:5 | LL | x.field.map(|value| { { plus_one(value + captured); } }); @@ -128,7 +128,7 @@ LL | x.field.map(|value| { { plus_one(value + captured); } }); | | | help: try this: `if let Some(value) = x.field { plus_one(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:80:5 | LL | x.field.map(|ref value| { do_nothing(value + captured) }); @@ -136,7 +136,7 @@ LL | x.field.map(|ref value| { do_nothing(value + captured) }); | | | help: try this: `if let Some(ref value) = x.field { do_nothing(value + captured) }` -error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type +error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:82:5 | LL | option().map(do_nothing);} diff --git a/src/tools/clippy/tests/ui/or_fun_call.fixed b/src/tools/clippy/tests/ui/or_fun_call.fixed index 67faa8bd4a0..5fb568672d3 100644 --- a/src/tools/clippy/tests/ui/or_fun_call.fixed +++ b/src/tools/clippy/tests/ui/or_fun_call.fixed @@ -58,12 +58,6 @@ fn or_fun_call() { let without_default = Some(Foo); without_default.unwrap_or_else(Foo::new); - let mut map = HashMap::::new(); - map.entry(42).or_insert_with(String::new); - - let mut btree = BTreeMap::::new(); - btree.entry(42).or_insert_with(String::new); - let stringy = Some(String::from("")); let _ = stringy.unwrap_or_else(|| "".to_owned()); @@ -122,6 +116,17 @@ pub fn skip_const_fn_with_no_args() { Some(42) } let _ = None.or(foo()); + + // See issue #5693. + let mut map = std::collections::HashMap::new(); + map.insert(1, vec![1]); + map.entry(1).or_insert(vec![]); + + let mut map = HashMap::::new(); + map.entry(42).or_insert(String::new()); + + let mut btree = BTreeMap::::new(); + btree.entry(42).or_insert(String::new()); } fn main() {} diff --git a/src/tools/clippy/tests/ui/or_fun_call.rs b/src/tools/clippy/tests/ui/or_fun_call.rs index 9867e2eedcf..737b0f7e55b 100644 --- a/src/tools/clippy/tests/ui/or_fun_call.rs +++ b/src/tools/clippy/tests/ui/or_fun_call.rs @@ -58,12 +58,6 @@ fn make() -> T { let without_default = Some(Foo); without_default.unwrap_or(Foo::new()); - let mut map = HashMap::::new(); - map.entry(42).or_insert(String::new()); - - let mut btree = BTreeMap::::new(); - btree.entry(42).or_insert(String::new()); - let stringy = Some(String::from("")); let _ = stringy.unwrap_or("".to_owned()); @@ -122,6 +116,17 @@ const fn foo() -> Option { Some(42) } let _ = None.or(foo()); + + // See issue #5693. + let mut map = std::collections::HashMap::new(); + map.insert(1, vec![1]); + map.entry(1).or_insert(vec![]); + + let mut map = HashMap::::new(); + map.entry(42).or_insert(String::new()); + + let mut btree = BTreeMap::::new(); + btree.entry(42).or_insert(String::new()); } fn main() {} diff --git a/src/tools/clippy/tests/ui/or_fun_call.stderr b/src/tools/clippy/tests/ui/or_fun_call.stderr index bc5978b538f..b8a436993f3 100644 --- a/src/tools/clippy/tests/ui/or_fun_call.stderr +++ b/src/tools/clippy/tests/ui/or_fun_call.stderr @@ -60,35 +60,23 @@ error: use of `unwrap_or` followed by a function call LL | without_default.unwrap_or(Foo::new()); | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(Foo::new)` -error: use of `or_insert` followed by a function call - --> $DIR/or_fun_call.rs:62:19 - | -LL | map.entry(42).or_insert(String::new()); - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)` - -error: use of `or_insert` followed by a function call - --> $DIR/or_fun_call.rs:65:21 - | -LL | btree.entry(42).or_insert(String::new()); - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)` - error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:68:21 + --> $DIR/or_fun_call.rs:62:21 | LL | let _ = stringy.unwrap_or("".to_owned()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "".to_owned())` error: use of `or` followed by a function call - --> $DIR/or_fun_call.rs:93:35 + --> $DIR/or_fun_call.rs:87:35 | LL | let _ = Some("a".to_string()).or(Some("b".to_string())); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some("b".to_string()))` error: use of `or` followed by a function call - --> $DIR/or_fun_call.rs:97:10 + --> $DIR/or_fun_call.rs:91:10 | LL | .or(Some(Bar(b, Duration::from_secs(2)))); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some(Bar(b, Duration::from_secs(2))))` -error: aborting due to 15 previous errors +error: aborting due to 13 previous errors diff --git a/src/tools/clippy/tests/ui/result_map_unit_fn_fixable.stderr b/src/tools/clippy/tests/ui/result_map_unit_fn_fixable.stderr index 467e00263cd..4f3a8c6b792 100644 --- a/src/tools/clippy/tests/ui/result_map_unit_fn_fixable.stderr +++ b/src/tools/clippy/tests/ui/result_map_unit_fn_fixable.stderr @@ -1,4 +1,4 @@ -error: called `map(f)` on an `Result` value where `f` is a function that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a function that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:35:5 | LL | x.field.map(do_nothing); @@ -8,7 +8,7 @@ LL | x.field.map(do_nothing); | = note: `-D clippy::result-map-unit-fn` implied by `-D warnings` -error: called `map(f)` on an `Result` value where `f` is a function that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a function that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:37:5 | LL | x.field.map(do_nothing); @@ -16,7 +16,7 @@ LL | x.field.map(do_nothing); | | | help: try this: `if let Ok(x_field) = x.field { do_nothing(x_field) }` -error: called `map(f)` on an `Result` value where `f` is a function that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a function that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:39:5 | LL | x.field.map(diverge); @@ -24,7 +24,7 @@ LL | x.field.map(diverge); | | | help: try this: `if let Ok(x_field) = x.field { diverge(x_field) }` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:45:5 | LL | x.field.map(|value| x.do_result_nothing(value + captured)); @@ -32,7 +32,7 @@ LL | x.field.map(|value| x.do_result_nothing(value + captured)); | | | help: try this: `if let Ok(value) = x.field { x.do_result_nothing(value + captured) }` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:47:5 | LL | x.field.map(|value| { x.do_result_plus_one(value + captured); }); @@ -40,7 +40,7 @@ LL | x.field.map(|value| { x.do_result_plus_one(value + captured); }); | | | help: try this: `if let Ok(value) = x.field { x.do_result_plus_one(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:50:5 | LL | x.field.map(|value| do_nothing(value + captured)); @@ -48,7 +48,7 @@ LL | x.field.map(|value| do_nothing(value + captured)); | | | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured) }` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:52:5 | LL | x.field.map(|value| { do_nothing(value + captured) }); @@ -56,7 +56,7 @@ LL | x.field.map(|value| { do_nothing(value + captured) }); | | | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured) }` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:54:5 | LL | x.field.map(|value| { do_nothing(value + captured); }); @@ -64,7 +64,7 @@ LL | x.field.map(|value| { do_nothing(value + captured); }); | | | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:56:5 | LL | x.field.map(|value| { { do_nothing(value + captured); } }); @@ -72,7 +72,7 @@ LL | x.field.map(|value| { { do_nothing(value + captured); } }); | | | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:59:5 | LL | x.field.map(|value| diverge(value + captured)); @@ -80,7 +80,7 @@ LL | x.field.map(|value| diverge(value + captured)); | | | help: try this: `if let Ok(value) = x.field { diverge(value + captured) }` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:61:5 | LL | x.field.map(|value| { diverge(value + captured) }); @@ -88,7 +88,7 @@ LL | x.field.map(|value| { diverge(value + captured) }); | | | help: try this: `if let Ok(value) = x.field { diverge(value + captured) }` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:63:5 | LL | x.field.map(|value| { diverge(value + captured); }); @@ -96,7 +96,7 @@ LL | x.field.map(|value| { diverge(value + captured); }); | | | help: try this: `if let Ok(value) = x.field { diverge(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:65:5 | LL | x.field.map(|value| { { diverge(value + captured); } }); @@ -104,7 +104,7 @@ LL | x.field.map(|value| { { diverge(value + captured); } }); | | | help: try this: `if let Ok(value) = x.field { diverge(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:70:5 | LL | x.field.map(|value| { let y = plus_one(value + captured); }); @@ -112,7 +112,7 @@ LL | x.field.map(|value| { let y = plus_one(value + captured); }); | | | help: try this: `if let Ok(value) = x.field { let y = plus_one(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:72:5 | LL | x.field.map(|value| { plus_one(value + captured); }); @@ -120,7 +120,7 @@ LL | x.field.map(|value| { plus_one(value + captured); }); | | | help: try this: `if let Ok(value) = x.field { plus_one(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:74:5 | LL | x.field.map(|value| { { plus_one(value + captured); } }); @@ -128,7 +128,7 @@ LL | x.field.map(|value| { { plus_one(value + captured); } }); | | | help: try this: `if let Ok(value) = x.field { plus_one(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_fixable.rs:77:5 | LL | x.field.map(|ref value| { do_nothing(value + captured) }); diff --git a/src/tools/clippy/tests/ui/result_map_unit_fn_unfixable.stderr b/src/tools/clippy/tests/ui/result_map_unit_fn_unfixable.stderr index b23cc608621..88e4efdb0f0 100644 --- a/src/tools/clippy/tests/ui/result_map_unit_fn_unfixable.stderr +++ b/src/tools/clippy/tests/ui/result_map_unit_fn_unfixable.stderr @@ -1,4 +1,4 @@ -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_unfixable.rs:23:5 | LL | x.field.map(|value| { do_nothing(value); do_nothing(value) }); @@ -8,7 +8,7 @@ LL | x.field.map(|value| { do_nothing(value); do_nothing(value) }); | = note: `-D clippy::result-map-unit-fn` implied by `-D warnings` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_unfixable.rs:25:5 | LL | x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) }); @@ -16,7 +16,7 @@ LL | x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) | | | help: try this: `if let Ok(value) = x.field { ... }` -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_unfixable.rs:29:5 | LL | x.field.map(|value| { @@ -30,7 +30,7 @@ LL | || }); | |_______| | -error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` --> $DIR/result_map_unit_fn_unfixable.rs:33:5 | LL | x.field.map(|value| { do_nothing(value); do_nothing(value); }); @@ -38,7 +38,7 @@ LL | x.field.map(|value| { do_nothing(value); do_nothing(value); }); | | | help: try this: `if let Ok(value) = x.field { ... }` -error: called `map(f)` on an `Result` value where `f` is a function that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a function that returns the unit type `()` --> $DIR/result_map_unit_fn_unfixable.rs:37:5 | LL | "12".parse::().map(diverge); @@ -46,7 +46,7 @@ LL | "12".parse::().map(diverge); | | | help: try this: `if let Ok(a) = "12".parse::() { diverge(a) }` -error: called `map(f)` on an `Result` value where `f` is a function that returns the unit type +error: called `map(f)` on an `Result` value where `f` is a function that returns the unit type `()` --> $DIR/result_map_unit_fn_unfixable.rs:43:5 | LL | y.map(do_nothing); diff --git a/src/tools/clippy/tests/ui/same_item_push.rs b/src/tools/clippy/tests/ui/same_item_push.rs index bfe27e02044..a37c8782ec3 100644 --- a/src/tools/clippy/tests/ui/same_item_push.rs +++ b/src/tools/clippy/tests/ui/same_item_push.rs @@ -1,5 +1,7 @@ #![warn(clippy::same_item_push)] +const VALUE: u8 = 7; + fn mutate_increment(x: &mut u8) -> u8 { *x += 1; *x @@ -9,65 +11,81 @@ fn increment(x: u8) -> u8 { x + 1 } -fn main() { - // Test for basic case - let mut spaces = Vec::with_capacity(10); - for _ in 0..10 { - spaces.push(vec![b' ']); - } +fn fun() -> usize { + 42 +} - let mut vec2: Vec = Vec::new(); +fn main() { + // ** linted cases ** + let mut vec: Vec = Vec::new(); let item = 2; for _ in 5..=20 { - vec2.push(item); + vec.push(item); } - let mut vec3: Vec = Vec::new(); + let mut vec: Vec = Vec::new(); for _ in 0..15 { let item = 2; - vec3.push(item); + vec.push(item); } - let mut vec4: Vec = Vec::new(); + let mut vec: Vec = Vec::new(); for _ in 0..15 { - vec4.push(13); + vec.push(13); + } + + let mut vec = Vec::new(); + for _ in 0..20 { + vec.push(VALUE); + } + + let mut vec = Vec::new(); + let item = VALUE; + for _ in 0..20 { + vec.push(item); + } + + // ** non-linted cases ** + let mut spaces = Vec::with_capacity(10); + for _ in 0..10 { + spaces.push(vec![b' ']); } // Suggestion should not be given as pushed variable can mutate - let mut vec5: Vec = Vec::new(); + let mut vec: Vec = Vec::new(); let mut item: u8 = 2; for _ in 0..30 { - vec5.push(mutate_increment(&mut item)); + vec.push(mutate_increment(&mut item)); } - let mut vec6: Vec = Vec::new(); + let mut vec: Vec = Vec::new(); let mut item: u8 = 2; let mut item2 = &mut mutate_increment(&mut item); for _ in 0..30 { - vec6.push(mutate_increment(item2)); + vec.push(mutate_increment(item2)); } - let mut vec7: Vec = Vec::new(); + let mut vec: Vec = Vec::new(); for (a, b) in [0, 1, 4, 9, 16].iter().enumerate() { - vec7.push(a); + vec.push(a); } - let mut vec8: Vec = Vec::new(); + let mut vec: Vec = Vec::new(); for i in 0..30 { - vec8.push(increment(i)); + vec.push(increment(i)); } - let mut vec9: Vec = Vec::new(); + let mut vec: Vec = Vec::new(); for i in 0..30 { - vec9.push(i + i * i); + vec.push(i + i * i); } // Suggestion should not be given as there are multiple pushes that are not the same - let mut vec10: Vec = Vec::new(); + let mut vec: Vec = Vec::new(); let item: u8 = 2; for _ in 0..30 { - vec10.push(item); - vec10.push(item * 2); + vec.push(item); + vec.push(item * 2); } // Suggestion should not be given as Vec is not involved @@ -82,16 +100,52 @@ struct A { for i in 0..30 { vec_a.push(A { kind: i }); } - let mut vec12: Vec = Vec::new(); + let mut vec: Vec = Vec::new(); for a in vec_a { - vec12.push(2u8.pow(a.kind)); + vec.push(2u8.pow(a.kind)); } // Fix #5902 - let mut vec13: Vec = Vec::new(); + let mut vec: Vec = Vec::new(); let mut item = 0; for _ in 0..10 { - vec13.push(item); + vec.push(item); item += 10; } + + // Fix #5979 + let mut vec: Vec = Vec::new(); + for _ in 0..10 { + vec.push(std::fs::File::open("foobar").unwrap()); + } + // Fix #5979 + #[derive(Clone)] + struct S {} + + trait T {} + impl T for S {} + + let mut vec: Vec> = Vec::new(); + for _ in 0..10 { + vec.push(Box::new(S {})); + } + + // Fix #5985 + let mut vec = Vec::new(); + let item = 42; + let item = fun(); + for _ in 0..20 { + vec.push(item); + } + + // Fix #5985 + let mut vec = Vec::new(); + let key = 1; + for _ in 0..20 { + let item = match key { + 1 => 10, + _ => 0, + }; + vec.push(item); + } } diff --git a/src/tools/clippy/tests/ui/same_item_push.stderr b/src/tools/clippy/tests/ui/same_item_push.stderr index ddc5d48cd41..d9ffa15780a 100644 --- a/src/tools/clippy/tests/ui/same_item_push.stderr +++ b/src/tools/clippy/tests/ui/same_item_push.stderr @@ -1,35 +1,43 @@ error: it looks like the same item is being pushed into this Vec - --> $DIR/same_item_push.rs:16:9 + --> $DIR/same_item_push.rs:23:9 | -LL | spaces.push(vec![b' ']); - | ^^^^^^ +LL | vec.push(item); + | ^^^ | = note: `-D clippy::same-item-push` implied by `-D warnings` - = help: try using vec![vec![b' '];SIZE] or spaces.resize(NEW_SIZE, vec![b' ']) + = help: try using vec![item;SIZE] or vec.resize(NEW_SIZE, item) error: it looks like the same item is being pushed into this Vec - --> $DIR/same_item_push.rs:22:9 + --> $DIR/same_item_push.rs:29:9 | -LL | vec2.push(item); - | ^^^^ +LL | vec.push(item); + | ^^^ | - = help: try using vec![item;SIZE] or vec2.resize(NEW_SIZE, item) + = help: try using vec![item;SIZE] or vec.resize(NEW_SIZE, item) error: it looks like the same item is being pushed into this Vec - --> $DIR/same_item_push.rs:28:9 + --> $DIR/same_item_push.rs:34:9 | -LL | vec3.push(item); - | ^^^^ +LL | vec.push(13); + | ^^^ | - = help: try using vec![item;SIZE] or vec3.resize(NEW_SIZE, item) + = help: try using vec![13;SIZE] or vec.resize(NEW_SIZE, 13) error: it looks like the same item is being pushed into this Vec - --> $DIR/same_item_push.rs:33:9 + --> $DIR/same_item_push.rs:39:9 | -LL | vec4.push(13); - | ^^^^ +LL | vec.push(VALUE); + | ^^^ | - = help: try using vec![13;SIZE] or vec4.resize(NEW_SIZE, 13) + = help: try using vec![VALUE;SIZE] or vec.resize(NEW_SIZE, VALUE) -error: aborting due to 4 previous errors +error: it looks like the same item is being pushed into this Vec + --> $DIR/same_item_push.rs:45:9 + | +LL | vec.push(item); + | ^^^ + | + = help: try using vec![item;SIZE] or vec.resize(NEW_SIZE, item) + +error: aborting due to 5 previous errors diff --git a/src/tools/clippy/tests/ui/temporary_assignment.rs b/src/tools/clippy/tests/ui/temporary_assignment.rs index d6f56d40c5d..b4a931043b0 100644 --- a/src/tools/clippy/tests/ui/temporary_assignment.rs +++ b/src/tools/clippy/tests/ui/temporary_assignment.rs @@ -54,11 +54,6 @@ fn main() { ArrayStruct { array: [0] }.array[0] = 1; (0, 0).0 = 1; - A.0 = 2; - B.field = 2; - C.structure.field = 2; - D.array[0] = 2; - // no error s.field = 1; t.0 = 1; diff --git a/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs b/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs index 0d8a322f2b2..26b03bdc740 100644 --- a/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs +++ b/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs @@ -51,4 +51,12 @@ fn transmute_ptr_to_ptr() { let _: &GenericParam<&LifetimeParam<'static>> = unsafe { std::mem::transmute(&GenericParam { t: &lp }) }; } +// dereferencing raw pointers in const contexts, should not lint as it's unstable (issue 5959) +const _: &() = { + struct ZST; + let zst = &ZST; + + unsafe { std::mem::transmute::<&'static ZST, &'static ()>(zst) } +}; + fn main() {} diff --git a/src/tools/clippy/tests/ui/unit_arg.rs b/src/tools/clippy/tests/ui/unit_arg.rs index 2992abae775..fec115ff29d 100644 --- a/src/tools/clippy/tests/ui/unit_arg.rs +++ b/src/tools/clippy/tests/ui/unit_arg.rs @@ -1,5 +1,11 @@ #![warn(clippy::unit_arg)] -#![allow(clippy::no_effect, unused_must_use, unused_variables)] +#![allow( + clippy::no_effect, + unused_must_use, + unused_variables, + clippy::unused_unit, + clippy::or_fun_call +)] use std::fmt::Debug; @@ -47,6 +53,11 @@ fn bad() { foo(3); }, ); + // here Some(foo(2)) isn't the top level statement expression, wrap the suggestion in a block + None.or(Some(foo(2))); + // in this case, the suggestion can be inlined, no need for a surrounding block + // foo(()); foo(()) instead of { foo(()); foo(()) } + foo(foo(())) } fn ok() { diff --git a/src/tools/clippy/tests/ui/unit_arg.stderr b/src/tools/clippy/tests/ui/unit_arg.stderr index 56f6a855dfa..90fee3aab23 100644 --- a/src/tools/clippy/tests/ui/unit_arg.stderr +++ b/src/tools/clippy/tests/ui/unit_arg.stderr @@ -1,5 +1,5 @@ error: passing a unit value to a function - --> $DIR/unit_arg.rs:23:5 + --> $DIR/unit_arg.rs:29:5 | LL | / foo({ LL | | 1; @@ -11,34 +11,28 @@ help: remove the semicolon from the last statement in the block | LL | 1 | -help: or move the expression in front of the call... +help: or move the expression in front of the call and replace it with the unit literal `()` | LL | { LL | 1; LL | }; - | -help: ...and use a unit literal instead - | LL | foo(()); - | ^^ + | error: passing a unit value to a function - --> $DIR/unit_arg.rs:26:5 + --> $DIR/unit_arg.rs:32:5 | LL | foo(foo(1)); | ^^^^^^^^^^^ | -help: move the expression in front of the call... +help: move the expression in front of the call and replace it with the unit literal `()` | LL | foo(1); - | -help: ...and use a unit literal instead - | LL | foo(()); - | ^^ + | error: passing a unit value to a function - --> $DIR/unit_arg.rs:27:5 + --> $DIR/unit_arg.rs:33:5 | LL | / foo({ LL | | foo(1); @@ -50,20 +44,17 @@ help: remove the semicolon from the last statement in the block | LL | foo(2) | -help: or move the expression in front of the call... +help: or move the expression in front of the call and replace it with the unit literal `()` | LL | { LL | foo(1); LL | foo(2); LL | }; - | -help: ...and use a unit literal instead - | LL | foo(()); - | ^^ + | error: passing a unit value to a function - --> $DIR/unit_arg.rs:32:5 + --> $DIR/unit_arg.rs:38:5 | LL | / b.bar({ LL | | 1; @@ -74,35 +65,29 @@ help: remove the semicolon from the last statement in the block | LL | 1 | -help: or move the expression in front of the call... +help: or move the expression in front of the call and replace it with the unit literal `()` | LL | { LL | 1; LL | }; - | -help: ...and use a unit literal instead - | LL | b.bar(()); - | ^^ + | error: passing unit values to a function - --> $DIR/unit_arg.rs:35:5 + --> $DIR/unit_arg.rs:41:5 | LL | taking_multiple_units(foo(0), foo(1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -help: move the expressions in front of the call... +help: move the expressions in front of the call and replace them with the unit literal `()` | LL | foo(0); LL | foo(1); - | -help: ...and use unit literals instead - | LL | taking_multiple_units((), ()); - | ^^ ^^ + | error: passing unit values to a function - --> $DIR/unit_arg.rs:36:5 + --> $DIR/unit_arg.rs:42:5 | LL | / taking_multiple_units(foo(0), { LL | | foo(1); @@ -114,21 +99,18 @@ help: remove the semicolon from the last statement in the block | LL | foo(2) | -help: or move the expressions in front of the call... +help: or move the expressions in front of the call and replace them with the unit literal `()` | LL | foo(0); LL | { LL | foo(1); LL | foo(2); LL | }; - | -help: ...and use unit literals instead - | LL | taking_multiple_units((), ()); - | ^^ ^^ + | error: passing unit values to a function - --> $DIR/unit_arg.rs:40:5 + --> $DIR/unit_arg.rs:46:5 | LL | / taking_multiple_units( LL | | { @@ -147,7 +129,7 @@ help: remove the semicolon from the last statement in the block | LL | foo(3) | -help: or move the expressions in front of the call... +help: or move the expressions in front of the call and replace them with the unit literal `()` | LL | { LL | foo(0); @@ -156,26 +138,44 @@ LL | }; LL | { LL | foo(2); ... -help: ...and use unit literals instead + +error: passing a unit value to a function + --> $DIR/unit_arg.rs:57:13 + | +LL | None.or(Some(foo(2))); + | ^^^^^^^^^^^^ | -LL | (), -LL | (), +help: move the expression in front of the call and replace it with the unit literal `()` + | +LL | None.or({ +LL | foo(2); +LL | Some(()) +LL | }); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:82:5 + --> $DIR/unit_arg.rs:60:5 | -LL | Some(foo(1)) +LL | foo(foo(())) | ^^^^^^^^^^^^ | -help: move the expression in front of the call... +help: move the expression in front of the call and replace it with the unit literal `()` | -LL | foo(1); +LL | foo(()); +LL | foo(()) | -help: ...and use a unit literal instead + +error: passing a unit value to a function + --> $DIR/unit_arg.rs:93:5 | +LL | Some(foo(1)) + | ^^^^^^^^^^^^ + | +help: move the expression in front of the call and replace it with the unit literal `()` + | +LL | foo(1); LL | Some(()) - | ^^ + | -error: aborting due to 8 previous errors +error: aborting due to 10 previous errors diff --git a/src/tools/clippy/tests/ui/unit_arg_empty_blocks.stderr b/src/tools/clippy/tests/ui/unit_arg_empty_blocks.stderr index bb58483584b..456b12a2c6b 100644 --- a/src/tools/clippy/tests/ui/unit_arg_empty_blocks.stderr +++ b/src/tools/clippy/tests/ui/unit_arg_empty_blocks.stderr @@ -22,14 +22,11 @@ error: passing unit values to a function LL | taking_two_units({}, foo(0)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -help: move the expression in front of the call... +help: move the expression in front of the call and replace it with the unit literal `()` | LL | foo(0); - | -help: ...and use unit literals instead - | LL | taking_two_units((), ()); - | ^^ ^^ + | error: passing unit values to a function --> $DIR/unit_arg_empty_blocks.rs:18:5 @@ -37,15 +34,12 @@ error: passing unit values to a function LL | taking_three_units({}, foo(0), foo(1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -help: move the expressions in front of the call... +help: move the expressions in front of the call and replace them with the unit literal `()` | LL | foo(0); LL | foo(1); - | -help: ...and use unit literals instead - | LL | taking_three_units((), (), ()); - | ^^ ^^ ^^ + | error: aborting due to 4 previous errors diff --git a/src/tools/clippy/tests/ui/unnecessary_sort_by.fixed b/src/tools/clippy/tests/ui/unnecessary_sort_by.fixed index 31c2ba0f9c5..ad0d0387db0 100644 --- a/src/tools/clippy/tests/ui/unnecessary_sort_by.fixed +++ b/src/tools/clippy/tests/ui/unnecessary_sort_by.fixed @@ -25,17 +25,25 @@ fn unnecessary_sort_by() { vec.sort_by(|_, b| b.cmp(&5)); vec.sort_by(|_, b| b.cmp(c)); vec.sort_unstable_by(|a, _| a.cmp(c)); + + // Ignore vectors of references + let mut vec: Vec<&&&isize> = vec![&&&3, &&&6, &&&1, &&&2, &&&5]; + vec.sort_by(|a, b| (***a).abs().cmp(&(***b).abs())); + vec.sort_unstable_by(|a, b| (***a).abs().cmp(&(***b).abs())); + vec.sort_by(|a, b| b.cmp(a)); + vec.sort_unstable_by(|a, b| b.cmp(a)); } -// Should not be linted to avoid hitting https://github.com/rust-lang/rust/issues/34162 +// Do not suggest returning a reference to the closure parameter of `Vec::sort_by_key` mod issue_5754 { - struct Test(String); + #[derive(Clone, Copy)] + struct Test(usize); #[derive(PartialOrd, Ord, PartialEq, Eq)] - struct Wrapper<'a>(&'a str); + struct Wrapper<'a>(&'a usize); impl Test { - fn name(&self) -> &str { + fn name(&self) -> &usize { &self.0 } @@ -60,7 +68,33 @@ mod issue_5754 { } } +// `Vec::sort_by_key` closure parameter is `F: FnMut(&T) -> K` +// The suggestion is destructuring T and we know T is not a reference, so test that non-Copy T are +// not linted. +mod issue_6001 { + struct Test(String); + + impl Test { + // Return an owned type so that we don't hit the fix for 5754 + fn name(&self) -> String { + self.0.clone() + } + } + + pub fn test() { + let mut args: Vec = vec![]; + + // Forward + args.sort_by(|a, b| a.name().cmp(&b.name())); + args.sort_unstable_by(|a, b| a.name().cmp(&b.name())); + // Reverse + args.sort_by(|a, b| b.name().cmp(&a.name())); + args.sort_unstable_by(|a, b| b.name().cmp(&a.name())); + } +} + fn main() { unnecessary_sort_by(); issue_5754::test(); + issue_6001::test(); } diff --git a/src/tools/clippy/tests/ui/unnecessary_sort_by.rs b/src/tools/clippy/tests/ui/unnecessary_sort_by.rs index a3c8ae468ed..9746f6e6849 100644 --- a/src/tools/clippy/tests/ui/unnecessary_sort_by.rs +++ b/src/tools/clippy/tests/ui/unnecessary_sort_by.rs @@ -25,17 +25,25 @@ fn id(x: isize) -> isize { vec.sort_by(|_, b| b.cmp(&5)); vec.sort_by(|_, b| b.cmp(c)); vec.sort_unstable_by(|a, _| a.cmp(c)); + + // Ignore vectors of references + let mut vec: Vec<&&&isize> = vec![&&&3, &&&6, &&&1, &&&2, &&&5]; + vec.sort_by(|a, b| (***a).abs().cmp(&(***b).abs())); + vec.sort_unstable_by(|a, b| (***a).abs().cmp(&(***b).abs())); + vec.sort_by(|a, b| b.cmp(a)); + vec.sort_unstable_by(|a, b| b.cmp(a)); } -// Should not be linted to avoid hitting https://github.com/rust-lang/rust/issues/34162 +// Do not suggest returning a reference to the closure parameter of `Vec::sort_by_key` mod issue_5754 { - struct Test(String); + #[derive(Clone, Copy)] + struct Test(usize); #[derive(PartialOrd, Ord, PartialEq, Eq)] - struct Wrapper<'a>(&'a str); + struct Wrapper<'a>(&'a usize); impl Test { - fn name(&self) -> &str { + fn name(&self) -> &usize { &self.0 } @@ -60,7 +68,33 @@ pub fn test() { } } +// `Vec::sort_by_key` closure parameter is `F: FnMut(&T) -> K` +// The suggestion is destructuring T and we know T is not a reference, so test that non-Copy T are +// not linted. +mod issue_6001 { + struct Test(String); + + impl Test { + // Return an owned type so that we don't hit the fix for 5754 + fn name(&self) -> String { + self.0.clone() + } + } + + pub fn test() { + let mut args: Vec = vec![]; + + // Forward + args.sort_by(|a, b| a.name().cmp(&b.name())); + args.sort_unstable_by(|a, b| a.name().cmp(&b.name())); + // Reverse + args.sort_by(|a, b| b.name().cmp(&a.name())); + args.sort_unstable_by(|a, b| b.name().cmp(&a.name())); + } +} + fn main() { unnecessary_sort_by(); issue_5754::test(); + issue_6001::test(); } diff --git a/src/tools/clippy/tests/ui/useless_attribute.fixed b/src/tools/clippy/tests/ui/useless_attribute.fixed index b222e2f7976..a5fcde768f1 100644 --- a/src/tools/clippy/tests/ui/useless_attribute.fixed +++ b/src/tools/clippy/tests/ui/useless_attribute.fixed @@ -49,6 +49,14 @@ mod a { pub use self::b::C; } +// don't lint on clippy::wildcard_imports for `use` items +#[allow(clippy::wildcard_imports)] +pub use std::io::prelude::*; + +// don't lint on clippy::enum_glob_use for `use` items +#[allow(clippy::enum_glob_use)] +pub use std::cmp::Ordering::*; + fn test_indented_attr() { #![allow(clippy::almost_swapped)] use std::collections::HashSet; diff --git a/src/tools/clippy/tests/ui/useless_attribute.rs b/src/tools/clippy/tests/ui/useless_attribute.rs index 3422eace4ab..0396d39e3d5 100644 --- a/src/tools/clippy/tests/ui/useless_attribute.rs +++ b/src/tools/clippy/tests/ui/useless_attribute.rs @@ -49,6 +49,14 @@ pub struct C {} pub use self::b::C; } +// don't lint on clippy::wildcard_imports for `use` items +#[allow(clippy::wildcard_imports)] +pub use std::io::prelude::*; + +// don't lint on clippy::enum_glob_use for `use` items +#[allow(clippy::enum_glob_use)] +pub use std::cmp::Ordering::*; + fn test_indented_attr() { #[allow(clippy::almost_swapped)] use std::collections::HashSet; diff --git a/src/tools/clippy/tests/ui/useless_attribute.stderr b/src/tools/clippy/tests/ui/useless_attribute.stderr index 57ba976730c..d0194e4bbbe 100644 --- a/src/tools/clippy/tests/ui/useless_attribute.stderr +++ b/src/tools/clippy/tests/ui/useless_attribute.stderr @@ -13,7 +13,7 @@ LL | #[cfg_attr(feature = "cargo-clippy", allow(dead_code))] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: if you just forgot a `!`, use: `#![cfg_attr(feature = "cargo-clippy", allow(dead_code)` error: useless lint attribute - --> $DIR/useless_attribute.rs:53:5 + --> $DIR/useless_attribute.rs:61:5 | LL | #[allow(clippy::almost_swapped)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: if you just forgot a `!`, use: `#![allow(clippy::almost_swapped)]`