diff options
57 files changed, 2076 insertions, 393 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index d82f970b8bf..7fd79deb56c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1632,6 +1632,7 @@ Released 2018-09-13 [`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 +[`await_holding_refcell_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_refcell_ref [`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 [`blacklisted_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#blacklisted_name @@ -1779,6 +1780,7 @@ Released 2018-09-13 [`large_digit_groups`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_digit_groups [`large_enum_variant`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant [`large_stack_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_arrays +[`large_types_passed_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_types_passed_by_value [`len_without_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_without_is_empty [`len_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_zero [`let_and_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return @@ -1793,6 +1795,7 @@ Released 2018-09-13 [`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn [`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy [`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive +[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains [`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic [`manual_strip`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_strip [`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap @@ -1841,6 +1844,7 @@ Released 2018-09-13 [`must_use_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#must_use_unit [`mut_from_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#mut_from_ref [`mut_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#mut_mut +[`mut_mutex_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#mut_mutex_lock [`mut_range_bound`]: https://rust-lang.github.io/rust-clippy/master/index.html#mut_range_bound [`mutable_key_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#mutable_key_type [`mutex_atomic`]: https://rust-lang.github.io/rust-clippy/master/index.html#mutex_atomic @@ -1936,6 +1940,7 @@ Released 2018-09-13 [`single_char_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern [`single_char_push_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_push_str [`single_component_path_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports +[`single_element_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_element_loop [`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match [`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else [`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next @@ -1979,6 +1984,7 @@ Released 2018-09-13 [`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err [`type_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity [`type_repetition_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds +[`undropped_manually_drops`]: https://rust-lang.github.io/rust-clippy/master/index.html#undropped_manually_drops [`unicode_not_nfc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unicode_not_nfc [`unimplemented`]: https://rust-lang.github.io/rust-clippy/master/index.html#unimplemented [`uninit_assumed_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_assumed_init diff --git a/clippy_lints/src/await_holding_lock.rs b/clippy_lints/src/await_holding_invalid.rs index 367534499fd..fcebb54c6c2 100644 --- a/clippy_lints/src/await_holding_lock.rs +++ b/clippy_lints/src/await_holding_invalid.rs @@ -45,13 +45,52 @@ declare_clippy_lint! { /// } /// ``` pub AWAIT_HOLDING_LOCK, - pedantic, + correctness, "Inside an async function, holding a MutexGuard while calling await" } -declare_lint_pass!(AwaitHoldingLock => [AWAIT_HOLDING_LOCK]); +declare_clippy_lint! { + /// **What it does:** Checks for calls to await while holding a + /// `RefCell` `Ref` or `RefMut`. + /// + /// **Why is this bad?** `RefCell` refs only check for exclusive mutable access + /// at runtime. Holding onto a `RefCell` ref across an `await` suspension point + /// risks panics from a mutable ref shared while other refs are outstanding. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust,ignore + /// use std::cell::RefCell; + /// + /// async fn foo(x: &RefCell<u32>) { + /// let b = x.borrow_mut()(); + /// *ref += 1; + /// bar.await; + /// } + /// ``` + /// + /// Use instead: + /// ```rust,ignore + /// use std::cell::RefCell; + /// + /// async fn foo(x: &RefCell<u32>) { + /// { + /// let b = x.borrow_mut(); + /// *ref += 1; + /// } + /// bar.await; + /// } + /// ``` + pub AWAIT_HOLDING_REFCELL_REF, + correctness, + "Inside an async function, holding a RefCell ref while calling await" +} -impl LateLintPass<'_> for AwaitHoldingLock { +declare_lint_pass!(AwaitHolding => [AWAIT_HOLDING_LOCK, AWAIT_HOLDING_REFCELL_REF]); + +impl LateLintPass<'_> for AwaitHolding { fn check_body(&mut self, cx: &LateContext<'_>, body: &'_ Body<'_>) { use AsyncGeneratorKind::{Block, Closure, Fn}; if let Some(GeneratorKind::Async(Block | Closure | Fn)) = body.generator_kind { @@ -78,6 +117,16 @@ fn check_interior_types(cx: &LateContext<'_>, ty_causes: &[GeneratorInteriorType "these are all the await points this lock is held through", ); } + if is_refcell_ref(cx, adt.did) { + span_lint_and_note( + cx, + AWAIT_HOLDING_REFCELL_REF, + ty_cause.span, + "this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await.", + ty_cause.scope_span.or(Some(span)), + "these are all the await points this ref is held through", + ); + } } } } @@ -90,3 +139,7 @@ fn is_mutex_guard(cx: &LateContext<'_>, def_id: DefId) -> bool { || match_def_path(cx, def_id, &paths::PARKING_LOT_RWLOCK_READ_GUARD) || match_def_path(cx, def_id, &paths::PARKING_LOT_RWLOCK_WRITE_GUARD) } + +fn is_refcell_ref(cx: &LateContext<'_>, def_id: DefId) -> bool { + match_def_path(cx, def_id, &paths::REFCELL_REF) || match_def_path(cx, def_id, &paths::REFCELL_REFMUT) +} diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index fd45a6da61c..9c0efef95de 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -579,9 +579,8 @@ fn is_mutable_pat(cx: &LateContext<'_>, pat: &hir::Pat<'_>, tys: &mut FxHashSet< if let hir::PatKind::Wild = pat.kind { return false; // ignore `_` patterns } - let def_id = pat.hir_id.owner.to_def_id(); - if cx.tcx.has_typeck_results(def_id) { - is_mutable_ty(cx, &cx.tcx.typeck(def_id.expect_local()).pat_ty(pat), pat.span, tys) + if cx.tcx.has_typeck_results(pat.hir_id.owner.to_def_id()) { + is_mutable_ty(cx, &cx.tcx.typeck(pat.hir_id.owner).pat_ty(pat), pat.span, tys) } else { false } @@ -694,11 +693,10 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for StaticMutVisitor<'a, 'tcx> { Call(_, args) | MethodCall(_, _, args, _) => { let mut tys = FxHashSet::default(); for arg in args { - let def_id = arg.hir_id.owner.to_def_id(); - if self.cx.tcx.has_typeck_results(def_id) + if self.cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id()) && is_mutable_ty( self.cx, - self.cx.tcx.typeck(def_id.expect_local()).expr_ty(arg), + self.cx.tcx.typeck(arg.hir_id.owner).expr_ty(arg), arg.span, &mut tys, ) diff --git a/clippy_lints/src/future_not_send.rs b/clippy_lints/src/future_not_send.rs index d2a322e1223..71a30d1c33d 100644 --- a/clippy_lints/src/future_not_send.rs +++ b/clippy_lints/src/future_not_send.rs @@ -92,13 +92,8 @@ impl<'tcx> LateLintPass<'tcx> for FutureNotSend { |db| { cx.tcx.infer_ctxt().enter(|infcx| { for FulfillmentError { obligation, .. } in send_errors { - infcx.maybe_note_obligation_cause_for_async_await( - db, - &obligation, - ); - if let Trait(trait_pred, _) = - obligation.predicate.skip_binders() - { + infcx.maybe_note_obligation_cause_for_async_await(db, &obligation); + if let Trait(trait_pred, _) = obligation.predicate.skip_binders() { db.note(&format!( "`{}` doesn't implement `{}`", trait_pred.self_ty(), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d4d2f92a6a6..b330b66776c 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -160,7 +160,7 @@ mod assign_ops; mod async_yields_async; mod atomic_ordering; mod attrs; -mod await_holding_lock; +mod await_holding_invalid; mod bit_mask; mod blacklisted_name; mod blocks_in_if_conditions; @@ -255,6 +255,7 @@ mod modulo_arithmetic; mod multiple_crate_versions; mod mut_key; mod mut_mut; +mod mut_mutex_lock; mod mut_reference; mod mutable_debug_assertion; mod mutex_atomic; @@ -278,6 +279,7 @@ mod overflow_check_conditional; mod panic_in_result_fn; mod panic_unimplemented; mod partialeq_ne_impl; +mod pass_by_ref_or_value; mod path_buf_push_overwrite; mod pattern_type_mismatch; mod precedence; @@ -311,9 +313,9 @@ mod to_string_in_display; mod trait_bounds; mod transmute; mod transmuting_null; -mod trivially_copy_pass_by_ref; mod try_err; mod types; +mod undropped_manually_drops; mod unicode; mod unit_return_expecting_ord; mod unnamed_address; @@ -509,7 +511,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &attrs::MISMATCHED_TARGET_OS, &attrs::UNKNOWN_CLIPPY_LINTS, &attrs::USELESS_ATTRIBUTE, - &await_holding_lock::AWAIT_HOLDING_LOCK, + &await_holding_invalid::AWAIT_HOLDING_LOCK, + &await_holding_invalid::AWAIT_HOLDING_REFCELL_REF, &bit_mask::BAD_BIT_MASK, &bit_mask::INEFFECTIVE_BIT_MASK, &bit_mask::VERBOSE_BIT_MASK, @@ -633,6 +636,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &loops::NEEDLESS_RANGE_LOOP, &loops::NEVER_LOOP, &loops::SAME_ITEM_PUSH, + &loops::SINGLE_ELEMENT_LOOP, &loops::WHILE_IMMUTABLE_CONDITION, &loops::WHILE_LET_LOOP, &loops::WHILE_LET_ON_ITERATOR, @@ -743,6 +747,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &multiple_crate_versions::MULTIPLE_CRATE_VERSIONS, &mut_key::MUTABLE_KEY_TYPE, &mut_mut::MUT_MUT, + &mut_mutex_lock::MUT_MUTEX_LOCK, &mut_reference::UNNECESSARY_MUT_PASSED, &mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL, &mutex_atomic::MUTEX_ATOMIC, @@ -776,6 +781,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &panic_unimplemented::UNIMPLEMENTED, &panic_unimplemented::UNREACHABLE, &partialeq_ne_impl::PARTIALEQ_NE_IMPL, + &pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE, + &pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF, &path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE, &pattern_type_mismatch::PATTERN_TYPE_MISMATCH, &precedence::PRECEDENCE, @@ -785,6 +792,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &ptr_eq::PTR_EQ, &ptr_offset_with_cast::PTR_OFFSET_WITH_CAST, &question_mark::QUESTION_MARK, + &ranges::MANUAL_RANGE_CONTAINS, &ranges::RANGE_MINUS_ONE, &ranges::RANGE_PLUS_ONE, &ranges::RANGE_ZIP_WITH_LEN, @@ -835,7 +843,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &transmute::USELESS_TRANSMUTE, &transmute::WRONG_TRANSMUTE, &transmuting_null::TRANSMUTING_NULL, - &trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF, &try_err::TRY_ERR, &types::ABSURD_EXTREME_COMPARISONS, &types::BORROWED_BOX, @@ -862,6 +869,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &types::UNIT_CMP, &types::UNNECESSARY_CAST, &types::VEC_BOX, + &undropped_manually_drops::UNDROPPED_MANUALLY_DROPS, &unicode::INVISIBLE_CHARACTERS, &unicode::NON_ASCII_LITERAL, &unicode::UNICODE_NOT_NFC, @@ -905,7 +913,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: ]); // end register lints, do not remove this comment, it’s used in `update_lints` - store.register_late_pass(|| box await_holding_lock::AwaitHoldingLock); + store.register_late_pass(|| box await_holding_invalid::AwaitHolding); store.register_late_pass(|| box serde_api::SerdeAPI); store.register_late_pass(|| box utils::internal_lints::CompilerLintFunctions::new()); store.register_late_pass(|| box utils::internal_lints::LintWithoutLintPass::default()); @@ -1009,11 +1017,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || box large_enum_variant::LargeEnumVariant::new(enum_variant_size_threshold)); store.register_late_pass(|| box explicit_write::ExplicitWrite); store.register_late_pass(|| box needless_pass_by_value::NeedlessPassByValue); - let trivially_copy_pass_by_ref = trivially_copy_pass_by_ref::TriviallyCopyPassByRef::new( + let pass_by_ref_or_value = pass_by_ref_or_value::PassByRefOrValue::new( conf.trivial_copy_size_limit, + conf.pass_by_value_size_limit, &sess.target, ); - store.register_late_pass(move || box trivially_copy_pass_by_ref); + store.register_late_pass(move || box pass_by_ref_or_value); store.register_late_pass(|| box try_err::TryErr); store.register_late_pass(|| box use_self::UseSelf); store.register_late_pass(|| box bytecount::ByteCount); @@ -1109,6 +1118,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box future_not_send::FutureNotSend); store.register_late_pass(|| box utils::internal_lints::CollapsibleCalls); store.register_late_pass(|| box if_let_mutex::IfLetMutex); + store.register_late_pass(|| box mut_mutex_lock::MutMutexLock); store.register_late_pass(|| box match_on_vec_items::MatchOnVecItems); store.register_early_pass(|| box manual_non_exhaustive::ManualNonExhaustive); store.register_late_pass(|| box manual_async_fn::ManualAsyncFn); @@ -1137,6 +1147,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || box disallowed_method::DisallowedMethod::new(&disallowed_methods)); store.register_early_pass(|| box asm_syntax::InlineAsmX86AttSyntax); store.register_early_pass(|| box asm_syntax::InlineAsmX86IntelSyntax); + store.register_late_pass(|| box undropped_manually_drops::UndroppedManuallyDrops); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec + /// or [`std::thread::yield_now`](https://doc.rust-lang.org/std/thread/fn.yield_now.html). + /// + /// For `no_std` targets, doing this is more complicated, especially because + /// `#[panic_handler]`s can't panic. To stop/pause the thread, you will + /// probably need to invoke some target-specific intrinsic. Examples include: + /// - [`x86_64::instructions::hlt`](https://docs.rs/x86_64/0.12.2/x86_64/instructions/fn.hlt.html) + /// - [`cortex_m::asm::wfi`](https://docs.rs/cortex-m/0.6.3/cortex_m/asm/fn.wfi.html) /// /// **Known problems:** None. /// @@ -452,6 +468,31 @@ declare_clippy_lint! { "the same item is pushed inside of a for loop" } +declare_clippy_lint! { + /// **What it does:** Checks whether a for loop has a single element. + /// + /// **Why is this bad?** There is no reason to have a loop of a + /// single element. + /// **Known problems:** None + /// + /// **Example:** + /// ```rust + /// let item1 = 2; + /// for item in &[item1] { + /// println!("{}", item); + /// } + /// ``` + /// could be written as + /// ```rust + /// let item1 = 2; + /// let item = &item1; + /// println!("{}", item); + /// ``` + pub SINGLE_ELEMENT_LOOP, + complexity, + "there is no reason to have a single element loop" +} + declare_lint_pass!(Loops => [ MANUAL_MEMCPY, NEEDLESS_RANGE_LOOP, @@ -469,6 +510,7 @@ declare_lint_pass!(Loops => [ MUT_RANGE_BOUND, WHILE_IMMUTABLE_CONDITION, SAME_ITEM_PUSH, + SINGLE_ELEMENT_LOOP, ]); impl<'tcx> LateLintPass<'tcx> for Loops { @@ -502,13 +544,15 @@ impl<'tcx> LateLintPass<'tcx> for Loops { // (even if the "match" or "if let" is used for declaration) if let ExprKind::Loop(ref block, _, LoopSource::Loop) = expr.kind { // also check for empty `loop {}` statements + // TODO(issue #6161): Enable for no_std crates (outside of #[panic_handler]) if block.stmts.is_empty() && block.expr.is_none() && !is_no_std_crate(cx.tcx.hir().krate()) { - span_lint( + span_lint_and_help( cx, EMPTY_LOOP, expr.span, - "empty `loop {}` detected. You may want to either use `panic!()` or add \ - `std::thread::sleep(..);` to the loop body.", + "empty `loop {}` wastes CPU cycles", + None, + "You should either use `panic!()` or add `std::thread::sleep(..);` to the loop body.", ); } @@ -777,6 +821,7 @@ fn check_for_loop<'tcx>( check_for_loop_arg(cx, pat, arg, expr); check_for_loop_over_map_kv(cx, pat, arg, body, expr); check_for_mut_range_bound(cx, arg, body); + check_for_single_element_loop(cx, pat, arg, body, expr); detect_same_item_push(cx, pat, arg, body, expr); } @@ -1866,6 +1911,43 @@ fn check_for_loop_over_map_kv<'tcx>( } } +fn check_for_single_element_loop<'tcx>( + cx: &LateContext<'tcx>, + pat: &'tcx Pat<'_>, + arg: &'tcx Expr<'_>, + body: &'tcx Expr<'_>, + expr: &'tcx Expr<'_>, +) { + if_chain! { + if let ExprKind::AddrOf(BorrowKind::Ref, _, ref arg_expr) = arg.kind; + if let PatKind::Binding(.., target, _) = pat.kind; + if let ExprKind::Array(ref arg_expr_list) = arg_expr.kind; + if let [arg_expression] = arg_expr_list; + if let ExprKind::Path(ref list_item) = arg_expression.kind; + if let Some(list_item_name) = single_segment_path(list_item).map(|ps| ps.ident.name); + if let ExprKind::Block(ref block, _) = body.kind; + if !block.stmts.is_empty(); + + then { + let for_span = get_span_of_entire_for_loop(expr); + let mut block_str = snippet(cx, block.span, "..").into_owned(); + block_str.remove(0); + block_str.pop(); + + + span_lint_and_sugg( + cx, + SINGLE_ELEMENT_LOOP, + for_span, + "for loop over a single element", + "try", + format!("{{\n{}let {} = &{};{}}}", " ".repeat(indent_of(cx, block.stmts[0].span).unwrap_or(0)), target.name, list_item_name, block_str), + Applicability::MachineApplicable + ) + } + } +} + struct MutatePairDelegate<'a, 'tcx> { cx: &'a LateContext<'tcx>, hir_id_low: Option<HirId>, @@ -1969,12 +2051,11 @@ fn check_for_mutation<'tcx>( span_low: None, span_high: None, }; - let def_id = body.hir_id.owner.to_def_id(); cx.tcx.infer_ctxt().enter(|infcx| { ExprUseVisitor::new( &mut delegate, &infcx, - def_id.expect_local(), + body.hir_id.owner, cx.param_env, cx.typeck_results(), ) diff --git a/clippy_lints/src/manual_unwrap_or.rs b/clippy_lints/src/manual_unwrap_or.rs index ddb8cc25077..22aa37e41fe 100644 --- a/clippy_lints/src/manual_unwrap_or.rs +++ b/clippy_lints/src/manual_unwrap_or.rs @@ -1,8 +1,9 @@ use crate::consts::constant_simple; use crate::utils; +use crate::utils::sugg; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{def, Arm, Expr, ExprKind, PatKind, QPath}; +use rustc_hir::{def, Arm, Expr, ExprKind, Pat, PatKind, QPath}; use rustc_lint::LintContext; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::lint::in_external_macro; @@ -10,7 +11,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { /// **What it does:** - /// Finds patterns that reimplement `Option::unwrap_or`. + /// Finds patterns that reimplement `Option::unwrap_or` or `Result::unwrap_or`. /// /// **Why is this bad?** /// Concise code helps focusing on behavior instead of boilerplate. @@ -33,7 +34,7 @@ declare_clippy_lint! { /// ``` pub MANUAL_UNWRAP_OR, complexity, - "finds patterns that can be encoded more concisely with `Option::unwrap_or`" + "finds patterns that can be encoded more concisely with `Option::unwrap_or` or `Result::unwrap_or`" } declare_lint_pass!(ManualUnwrapOr => [MANUAL_UNWRAP_OR]); @@ -43,32 +44,50 @@ impl LateLintPass<'_> for ManualUnwrapOr { if in_external_macro(cx.sess(), expr.span) { return; } - lint_option_unwrap_or_case(cx, expr); + lint_manual_unwrap_or(cx, expr); } } -fn lint_option_unwrap_or_case<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - fn applicable_none_arm<'a>(arms: &'a [Arm<'a>]) -> Option<&'a Arm<'a>> { +#[derive(Copy, Clone)] +enum Case { + Option, + Result, +} + +impl Case { + fn unwrap_fn_path(&self) -> &str { + match self { + Case::Option => "Option::unwrap_or", + Case::Result => "Result::unwrap_or", + } + } +} + +fn lint_manual_unwrap_or<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + fn applicable_or_arm<'a>(arms: &'a [Arm<'a>]) -> Option<&'a Arm<'a>> { if_chain! { if arms.len() == 2; if arms.iter().all(|arm| arm.guard.is_none()); - if let Some((idx, none_arm)) = arms.iter().enumerate().find(|(_, arm)| - if let PatKind::Path(ref qpath) = arm.pat.kind { - utils::match_qpath(qpath, &utils::paths::OPTION_NONE) - } else { - false + if let Some((idx, or_arm)) = arms.iter().enumerate().find(|(_, arm)| + match arm.pat.kind { + PatKind::Path(ref some_qpath) => + utils::match_qpath(some_qpath, &utils::paths::OPTION_NONE), + PatKind::TupleStruct(ref err_qpath, &[Pat { kind: PatKind::Wild, .. }], _) => + utils::match_qpath(err_qpath, &utils::paths::RESULT_ERR), + _ => false, } ); - let some_arm = &arms[1 - idx]; - if let PatKind::TupleStruct(ref some_qpath, &[some_binding], _) = some_arm.pat.kind; - if utils::match_qpath(some_qpath, &utils::paths::OPTION_SOME); - if let PatKind::Binding(_, binding_hir_id, ..) = some_binding.kind; - if let ExprKind::Path(QPath::Resolved(_, body_path)) = some_arm.body.kind; + let unwrap_arm = &arms[1 - idx]; + if let PatKind::TupleStruct(ref unwrap_qpath, &[unwrap_pat], _) = unwrap_arm.pat.kind; + if utils::match_qpath(unwrap_qpath, &utils::paths::OPTION_SOME) + || utils::match_qpath(unwrap_qpath, &utils::paths::RESULT_OK); + if let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind; + if let ExprKind::Path(QPath::Resolved(_, body_path)) = unwrap_arm.body.kind; if let def::Res::Local(body_path_hir_id) = body_path.res; if body_path_hir_id == binding_hir_id; - if !utils::usage::contains_return_break_continue_macro(none_arm.body); + if !utils::usage::contains_return_break_continue_macro(or_arm.body); then { - Some(none_arm) + Some(or_arm) } else { None } @@ -78,24 +97,29 @@ fn lint_option_unwrap_or_case<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tc if_chain! { if let ExprKind::Match(scrutinee, match_arms, _) = expr.kind; let ty = cx.typeck_results().expr_ty(scrutinee); - if utils::is_type_diagnostic_item(cx, ty, sym!(option_type)); - if let Some(none_arm) = applicable_none_arm(match_arms); - if let Some(scrutinee_snippet) = utils::snippet_opt(cx, scrutinee.span); - if let Some(none_body_snippet) = utils::snippet_opt(cx, none_arm.body.span); + if let Some(case) = if utils::is_type_diagnostic_item(cx, ty, sym!(option_type)) { + Some(Case::Option) + } else if utils::is_type_diagnostic_item(cx, ty, sym!(result_type)) { + Some(Case::Result) + } else { + None + }; + if let Some(or_arm) = applicable_or_arm(match_arms); + if let Some(or_body_snippet) = utils::snippet_opt(cx, or_arm.body.span); if let Some(indent) = utils::indent_of(cx, expr.span); - if constant_simple(cx, cx.typeck_results(), none_arm.body).is_some(); + if constant_simple(cx, cx.typeck_results(), or_arm.body).is_some(); then { - let reindented_none_body = - utils::reindent_multiline(none_body_snippet.into(), true, Some(indent)); + let reindented_or_body = + utils::reindent_multiline(or_body_snippet.into(), true, Some(indent)); utils::span_lint_and_sugg( cx, MANUAL_UNWRAP_OR, expr.span, - "this pattern reimplements `Option::unwrap_or`", + &format!("this pattern reimplements `{}`", case.unwrap_fn_path()), "replace with", format!( "{}.unwrap_or({})", - scrutinee_snippet, - reindented_none_body, + sugg::Sugg::hir(cx, scrutinee, "..").maybe_par(), + reindented_or_body, ), Applicability::MachineApplicable, ); diff --git a/clippy_lints/src/mut_mutex_lock.rs b/clippy_lints/src/mut_mutex_lock.rs new file mode 100644 index 00000000000..df1cecb328c --- /dev/null +++ b/clippy_lints/src/mut_mutex_lock.rs @@ -0,0 +1,68 @@ +use crate::utils::{is_type_diagnostic_item, span_lint_and_sugg}; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, Mutability}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// **What it does:** Checks for `&mut Mutex::lock` calls + /// + /// **Why is this bad?** `Mutex::lock` is less efficient than + /// calling `Mutex::get_mut`. In addition you also have a statically + /// guarantee that the mutex isn't locked, instead of just a runtime + /// guarantee. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// use std::sync::{Arc, Mutex}; + /// + /// let mut value_rc = Arc::new(Mutex::new(42_u8)); + /// let value_mutex = Arc::get_mut(&mut value_rc).unwrap(); + /// + /// let mut value = value_mutex.lock().unwrap(); + /// *value += 1; + /// ``` + /// Use instead: + /// ```rust + /// use std::sync::{Arc, Mutex}; + /// + /// let mut value_rc = Arc::new(Mutex::new(42_u8)); + /// let value_mutex = Arc::get_mut(&mut value_rc).unwrap(); + /// + /// let value = value_mutex.get_mut().unwrap(); + /// *value += 1; + /// ``` + pub MUT_MUTEX_LOCK, + style, + "`&mut Mutex::lock` does unnecessary locking" +} + +declare_lint_pass!(MutMutexLock => [MUT_MUTEX_LOCK]); + +impl<'tcx> LateLintPass<'tcx> for MutMutexLock { + fn check_expr(&mut self, cx: &LateContext<'tcx>, ex: &'tcx Expr<'tcx>) { + if_chain! { + if let ExprKind::MethodCall(path, method_span, args, _) = &ex.kind; + if path.ident.name == sym!(lock); + let ty = cx.typeck_results().expr_ty(&args[0]); + if let ty::Ref(_, inner_ty, Mutability::Mut) = ty.kind(); + if is_type_diagnostic_item(cx, inner_ty, sym!(mutex_type)); + then { + span_lint_and_sugg( + cx, + MUT_MUTEX_LOCK, + *method_span, + "calling `&mut Mutex::lock` unnecessarily locks an exclusive (mutable) reference", + "change this to", + "get_mut".to_owned(), + Applicability::MaybeIncorrect, + ); + } + } + } +} diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs index dc5aa669139..a799a644e97 100644 --- a/clippy_lints/src/needless_bool.rs +++ b/clippy_lints/src/needless_bool.rs @@ -3,7 +3,9 @@ //! This lint is **warn** by default use crate::utils::sugg::Sugg; -use crate::utils::{higher, parent_node_is_if_expr, snippet_with_applicability, span_lint, span_lint_and_sugg}; +use crate::utils::{ + higher, is_expn_of, parent_node_is_if_expr, snippet_with_applicability, span_lint, span_lint_and_sugg, +}; use if_chain::if_chain; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; @@ -233,6 +235,9 @@ fn check_comparison<'a, 'tcx>( cx.typeck_results().expr_ty(left_side), cx.typeck_results().expr_ty(right_side), ); + if is_expn_of(left_side.span, "cfg").is_some() || is_expn_of(right_side.span, "cfg").is_some() { + return; + } if l_ty.is_bool() && r_ty.is_bool() { let mut applicability = Applicability::MachineApplicable; @@ -295,7 +300,14 @@ fn suggest_bool_comparison<'a, 'tcx>( message: &str, conv_hint: impl FnOnce(Sugg<'a>) -> Sugg<'a>, ) { - let hint = Sugg::hir_with_applicability(cx, expr, "..", &mut applicability); + let hint = if expr.span.from_expansion() { + if applicability != Applicability::Unspecified { + applicability = Applicability::MaybeIncorrect; + } + Sugg::hir_with_macro_callsite(cx, expr, "..") + } else { + Sugg::hir_with_applicability(cx, expr, "..", &mut applicability) + }; span_lint_and_sugg( cx, BOOL_COMPARISON, diff --git a/clippy_lints/src/pass_by_ref_or_value.rs b/clippy_lints/src/pass_by_ref_or_value.rs new file mode 100644 index 00000000000..28816c3076d --- /dev/null +++ b/clippy_lints/src/pass_by_ref_or_value.rs @@ -0,0 +1,256 @@ +use std::cmp; + +use crate::utils::{is_copy, is_self_ty, snippet, span_lint_and_sugg}; +use if_chain::if_chain; +use rustc_ast::attr; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{BindingAnnotation, Body, FnDecl, HirId, ItemKind, MutTy, Mutability, Node, PatKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::Span; +use rustc_target::abi::LayoutOf; +use rustc_target::spec::abi::Abi; +use rustc_target::spec::Target; + +declare_clippy_lint! { + /// **What it does:** Checks for functions taking arguments by reference, where + /// the argument type is `Copy` and small enough to be more efficient to always + /// pass by value. + /// + /// **Why is this bad?** In many calling conventions instances of structs will + /// be passed through registers if they fit into two or less general purpose + /// registers. + /// + /// **Known problems:** This lint is target register size dependent, it is + /// limited to 32-bit to try and reduce portability problems between 32 and + /// 64-bit, but if you are compiling for 8 or 16-bit targets then the limit + /// will be different. + /// + /// The configuration option `trivial_copy_size_limit` can be set to override + /// this limit for a project. + /// + /// This lint attempts to allow passing arguments by reference if a reference + /// to that argument is returned. This is implemented by comparing the lifetime + /// of the argument and return value for equality. However, this can cause + /// false positives in cases involving multiple lifetimes that are bounded by + /// each other. + /// + /// **Example:** + /// + /// ```rust + /// // Bad + /// fn foo(v: &u32) {} + /// ``` + /// + /// ```rust + /// // Better + /// fn foo(v: u32) {} + /// ``` + pub TRIVIALLY_COPY_PASS_BY_REF, + pedantic, + "functions taking small copyable arguments by reference" +} + +declare_clippy_lint! { + /// **What it does:** Checks for functions taking arguments by value, where + /// the argument type is `Copy` and large enough to be worth considering + /// passing by reference. Does not trigger if the function is being exported, + /// because that might induce API breakage, if the parameter is declared as mutable, + /// or if the argument is a `self`. + /// + /// **Why is this bad?** Arguments passed by value might result in an unnecessary + /// shallow copy, taking up more space in the stack and requiring a call to + /// `memcpy`, which which can be expensive. + /// + /// **Example:** + /// + /// ```rust + /// #[derive(Clone, Copy)] + /// struct TooLarge([u8; 2048]); + /// + /// // Bad + /// fn foo(v: TooLarge) {} + /// ``` + /// ```rust + /// #[derive(Clone, Copy)] + /// struct TooLarge([u8; 2048]); + /// + /// // Good + /// fn foo(v: &TooLarge) {} + /// ``` + pub LARGE_TYPES_PASSED_BY_VALUE, + pedantic, + "functions taking large arguments by value" +} + +#[derive(Copy, Clone)] +pub struct PassByRefOrValue { + ref_min_size: u64, + value_max_size: u64, +} + +impl<'tcx> PassByRefOrValue { + pub fn new(ref_min_size: Option<u64>, value_max_size: u64, target: &Target) -> Self { + let ref_min_size = ref_min_size.unwrap_or_else(|| { + let bit_width = u64::from(target.pointer_width); + // Cap the calculated bit width at 32-bits to reduce + // portability problems between 32 and 64-bit targets + let bit_width = cmp::min(bit_width, 32); + #[allow(clippy::integer_division)] + let byte_width = bit_width / 8; + // Use a limit of 2 times the register byte width + byte_width * 2 + }); + + Self { + ref_min_size, + value_max_size, + } + } + + fn check_poly_fn(&mut self, cx: &LateContext<'tcx>, hir_id: HirId, decl: &FnDecl<'_>, span: Option<Span>) { + let fn_def_id = cx.tcx.hir().local_def_id(hir_id); + + let fn_sig = cx.tcx.fn_sig(fn_def_id); + let fn_sig = cx.tcx.erase_late_bound_regions(&fn_sig); + + let fn_body = cx.enclosing_body.map(|id| cx.tcx.hir().body(id)); + + for (index, (input, &ty)) in decl.inputs.iter().zip(fn_sig.inputs()).enumerate() { + // All spans generated from a proc-macro invocation are the same... + match span { + Some(s) if s == input.span => return, + _ => (), + } + + match ty.kind() { + ty::Ref(input_lt, ty, Mutability::Not) => { + // Use lifetimes to determine if we're returning a reference to the + // argument. In that case we can't switch to pass-by-value as the + // argument will not live long enough. + let output_lts = match *fn_sig.output().kind() { + ty::Ref(output_lt, _, _) => vec![output_lt], + ty::Adt(_, substs) => substs.regions().collect(), + _ => vec![], + }; + + if_chain! { + if !output_lts.contains(&input_lt); + if is_copy(cx, ty); + if let Some(size) = cx.layout_of(ty).ok().map(|l| l.size.bytes()); + if size <= self.ref_min_size; + if let hir::TyKind::Rptr(_, MutTy { ty: ref decl_ty, .. }) = input.kind; + then { + let value_type = if is_self_ty(decl_ty) { + "self".into() + } else { + snippet(cx, decl_ty.span, "_").into() + }; + span_lint_and_sugg( + cx, + TRIVIALLY_COPY_PASS_BY_REF, + input.span, + &format!("this argument ({} byte) is passed by reference, but would be more efficient if passed by value (limit: {} byte)", size, self.ref_min_size), + "consider passing by value instead", + value_type, + Applicability::Unspecified, + ); + } + } + }, + + ty::Adt(_, _) | ty::Array(_, _) | ty::Tuple(_) => { + // if function has a body and parameter is annotated with mut, ignore + if let Some(param) = fn_body.and_then(|body| body.params.get(index)) { + match param.pat.kind { + PatKind::Binding(BindingAnnotation::Unannotated, _, _, _) => {}, + _ => continue, + } + } + + if_chain! { + if !cx.access_levels.is_exported(hir_id); + if is_copy(cx, ty); + if !is_self_ty(input); + if let Some(size) = cx.layout_of(ty).ok().map(|l| l.size.bytes()); + if size > self.value_max_size; + then { + span_lint_and_sugg( + cx, + LARGE_TYPES_PASSED_BY_VALUE, + input.span, + &format!("this argument ({} byte) is passed by value, but might be more efficient if passed by reference (limit: {} byte)", size, self.value_max_size), + "consider passing by reference instead", + format!("&{}", snippet(cx, input.span, "_")), + Applicability::MaybeIncorrect, + ); + } + } + }, + + _ => {}, + } + } + } +} + +impl_lint_pass!(PassByRefOrValue => [TRIVIALLY_COPY_PASS_BY_REF, LARGE_TYPES_PASSED_BY_VALUE]); + +impl<'tcx> LateLintPass<'tcx> for PassByRefOrValue { + fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) { + if item.span.from_expansion() { + return; + } + + if let hir::TraitItemKind::Fn(method_sig, _) = &item.kind { + self.check_poly_fn(cx, item.hir_id, &*method_sig.decl, None); + } + } + + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + kind: FnKind<'tcx>, + decl: &'tcx FnDecl<'_>, + _body: &'tcx Body<'_>, + span: Span, + hir_id: HirId, + ) { + if span.from_expansion() { + return; + } + + match kind { + FnKind::ItemFn(.., header, _, attrs) => { + if header.abi != Abi::Rust { + return; + } + for a in attrs { + if let Some(meta_items) = a.meta_item_list() { + if a.has_name(sym!(proc_macro_derive)) + || (a.has_name(sym!(inline)) && attr::list_contains_name(&meta_items, sym!(always))) + { + return; + } + } + } + }, + FnKind::Method(..) => (), + FnKind::Closure(..) => return, + } + + // Exclude non-inherent impls + if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { + if matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), .. } | + ItemKind::Trait(..)) + { + return; + } + } + + self.check_poly_fn(cx, hir_id, decl, Some(span)); + } +} diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index cc492917b9d..de54711d851 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -2,15 +2,19 @@ use crate::consts::{constant, Constant}; use if_chain::if_chain; use rustc_ast::ast::RangeLimits; use rustc_errors::Applicability; -use rustc_hir::{BinOpKind, Expr, ExprKind, QPath}; +use rustc_hir::{BinOpKind, Expr, ExprKind, PathSegment, QPath}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::source_map::Spanned; +use rustc_span::source_map::{Span, Spanned}; +use rustc_span::symbol::Ident; use std::cmp::Ordering; use crate::utils::sugg::Sugg; -use crate::utils::{get_parent_expr, is_integer_const, snippet, snippet_opt, span_lint, span_lint_and_then}; +use crate::utils::{ + get_parent_expr, is_integer_const, single_segment_path, snippet, snippet_opt, snippet_with_applicability, + span_lint, span_lint_and_sugg, span_lint_and_then, +}; use crate::utils::{higher, SpanlessEq}; declare_clippy_lint! { @@ -128,43 +132,51 @@ declare_clippy_lint! { "reversing the limits of range expressions, resulting in empty ranges" } +declare_clippy_lint! { + /// **What it does:** Checks for expressions like `x >= 3 && x < 8` that could + /// be more readably expressed as `(3..8).contains(x)`. + /// + /// **Why is this bad?** `contains` expresses the intent better and has less + /// failure modes (such as fencepost errors or using `||` instead of `&&`). + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// // given + /// let x = 6; + /// + /// assert!(x >= 3 && x < 8); + /// ``` + /// Use instead: + /// ```rust + ///# let x = 6; + /// assert!((3..8).contains(&x)); + /// ``` + pub MANUAL_RANGE_CONTAINS, + style, + "manually reimplementing {`Range`, `RangeInclusive`}`::contains`" +} + declare_lint_pass!(Ranges => [ RANGE_ZIP_WITH_LEN, RANGE_PLUS_ONE, RANGE_MINUS_ONE, REVERSED_EMPTY_RANGES, + MANUAL_RANGE_CONTAINS, ]); impl<'tcx> LateLintPass<'tcx> for Ranges { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let ExprKind::MethodCall(ref path, _, ref args, _) = expr.kind { - let name = path.ident.as_str(); - if name == "zip" && args.len() == 2 { - let iter = &args[0].kind; - let zip_arg = &args[1]; - if_chain! { - // `.iter()` call - if let ExprKind::MethodCall(ref iter_path, _, ref iter_args , _) = *iter; - if iter_path.ident.name == sym!(iter); - // range expression in `.zip()` call: `0..x.len()` - if let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::range(zip_arg); - if is_integer_const(cx, start, 0); - // `.len()` call - if let ExprKind::MethodCall(ref len_path, _, ref len_args, _) = end.kind; - if len_path.ident.name == sym!(len) && len_args.len() == 1; - // `.iter()` and `.len()` called on same `Path` - if let ExprKind::Path(QPath::Resolved(_, ref iter_path)) = iter_args[0].kind; - if let ExprKind::Path(QPath::Resolved(_, ref len_path)) = len_args[0].kind; - if SpanlessEq::new(cx).eq_path_segments(&iter_path.segments, &len_path.segments); - then { - span_lint(cx, - RANGE_ZIP_WITH_LEN, - expr.span, - &format!("it is more idiomatic to use `{}.iter().enumerate()`", - snippet(cx, iter_args[0].span, "_"))); - } - } - } + match expr.kind { + ExprKind::MethodCall(ref path, _, ref args, _) => { + check_range_zip_with_len(cx, path, args, expr.span); + }, + ExprKind::Binary(ref op, ref l, ref r) => { + check_possible_range_contains(cx, op.node, l, r, expr.span); + }, + _ => {}, } check_exclusive_range_plus_one(cx, expr); @@ -173,6 +185,148 @@ impl<'tcx> LateLintPass<'tcx> for Ranges { } } +fn check_possible_range_contains(cx: &LateContext<'_>, op: BinOpKind, l: &Expr<'_>, r: &Expr<'_>, span: Span) { + let combine_and = match op { + BinOpKind::And | BinOpKind::BitAnd => true, + BinOpKind::Or | BinOpKind::BitOr => false, + _ => return, + }; + // value, name, order (higher/lower), inclusiveness + if let (Some((lval, lname, name_span, lval_span, lord, linc)), Some((rval, rname, _, rval_span, rord, rinc))) = + (check_range_bounds(cx, l), check_range_bounds(cx, r)) + { + // we only lint comparisons on the same name and with different + // direction + if lname != rname || lord == rord { + return; + } + let ord = Constant::partial_cmp(cx.tcx, cx.typeck_results().expr_ty(l), &lval, &rval); + if combine_and && ord == Some(rord) { + // order lower bound and upper bound + let (l_span, u_span, l_inc, u_inc) = if rord == Ordering::Less { + (lval_span, rval_span, linc, rinc) + } else { + (rval_span, lval_span, rinc, linc) + }; + // we only lint inclusive lower bounds + if !l_inc { + return; + } + let (range_type, range_op) = if u_inc { + ("RangeInclusive", "..=") + } else { + ("Range", "..") + }; + let mut applicability = Applicability::MachineApplicable; + let name = snippet_with_applicability(cx, name_span, "_", &mut applicability); + let lo = snippet_with_applicability(cx, l_span, "_", &mut applicability); + let hi = snippet_with_applicability(cx, u_span, "_", &mut applicability); + span_lint_and_sugg( + cx, + MANUAL_RANGE_CONTAINS, + span, + &format!("manual `{}::contains` implementation", range_type), + "use", + format!("({}{}{}).contains(&{})", lo, range_op, hi, name), + applicability, + ); + } else if !combine_and && ord == Some(lord) { + // `!_.contains(_)` + // order lower bound and upper bound + let (l_span, u_span, l_inc, u_inc) = if lord == Ordering::Less { + (lval_span, rval_span, linc, rinc) + } else { + (rval_span, lval_span, rinc, linc) + }; + if l_inc { + return; + } + let (range_type, range_op) = if u_inc { + ("Range", "..") + } else { + ("RangeInclusive", "..=") + }; + let mut applicability = Applicability::MachineApplicable; + let name = snippet_with_applicability(cx, name_span, "_", &mut applicability); + let lo = snippet_with_applicability(cx, l_span, "_", &mut applicability); + let hi = snippet_with_applicability(cx, u_span, "_", &mut applicability); + span_lint_and_sugg( + cx, + MANUAL_RANGE_CONTAINS, + span, + &format!("manual `!{}::contains` implementation", range_type), + "use", + format!("!({}{}{}).contains(&{})", lo, range_op, hi, name), + applicability, + ); + } + } +} + +fn check_range_bounds(cx: &LateContext<'_>, ex: &Expr<'_>) -> Option<(Constant, Ident, Span, Span, Ordering, bool)> { + if let ExprKind::Binary(ref op, ref l, ref r) = ex.kind { + let (inclusive, ordering) = match op.node { + BinOpKind::Gt => (false, Ordering::Greater), + BinOpKind::Ge => (true, Ordering::Greater), + BinOpKind::Lt => (false, Ordering::Less), + BinOpKind::Le => (true, Ordering::Less), + _ => return None, + }; + if let Some(id) = match_ident(l) { + if let Some((c, _)) = constant(cx, cx.typeck_results(), r) { + return Some((c, id, l.span, r.span, ordering, inclusive)); + } + } else if let Some(id) = match_ident(r) { + if let Some((c, _)) = constant(cx, cx.typeck_results(), l) { + return Some((c, id, r.span, l.span, ordering.reverse(), inclusive)); + } + } + } + None +} + +fn match_ident(e: &Expr<'_>) -> Option<Ident> { + if let ExprKind::Path(ref qpath) = e.kind { + if let Some(seg) = single_segment_path(qpath) { + if seg.args.is_none() { + return Some(seg.ident); + } + } + } + None +} + +fn check_range_zip_with_len(cx: &LateContext<'_>, path: &PathSegment<'_>, args: &[Expr<'_>], span: Span) { + let name = path.ident.as_str(); + if name == "zip" && args.len() == 2 { + let iter = &args[0].kind; + let zip_arg = &args[1]; + if_chain! { + // `.iter()` call + if let ExprKind::MethodCall(ref iter_path, _, ref iter_args, _) = *iter; + if iter_path.ident.name == sym!(iter); + // range expression in `.zip()` call: `0..x.len()` + if let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::range(zip_arg); + if is_integer_const(cx, start, 0); + // `.len()` call + if let ExprKind::MethodCall(ref len_path, _, ref len_args, _) = end.kind; + if len_path.ident.name == sym!(len) && len_args.len() == 1; + // `.iter()` and `.len()` called on same `Path` + if let ExprKind::Path(QPath::Resolved(_, ref iter_path)) = iter_args[0].kind; + if let ExprKind::Path(QPath::Resolved(_, ref len_path)) = len_args[0].kind; + if SpanlessEq::new(cx).eq_path_segments(&iter_path.segments, &len_path.segments); + then { + span_lint(cx, + RANGE_ZIP_WITH_LEN, + span, + &format!("it is more idiomatic to use `{}.iter().enumerate()`", + snippet(cx, iter_args[0].span, "_")) + ); + } + } + } +} + // exclusive range plus one: `x..(y+1)` fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) { if_chain! { diff --git a/clippy_lints/src/trivially_copy_pass_by_ref.rs b/clippy_lints/src/trivially_copy_pass_by_ref.rs deleted file mode 100644 index e90ea0fc200..00000000000 --- a/clippy_lints/src/trivially_copy_pass_by_ref.rs +++ /dev/null @@ -1,183 +0,0 @@ -use std::cmp; - -use crate::utils::{is_copy, is_self_ty, snippet, span_lint_and_sugg}; -use if_chain::if_chain; -use rustc_ast::attr; -use rustc_errors::Applicability; -use rustc_hir as hir; -use rustc_hir::intravisit::FnKind; -use rustc_hir::{Body, FnDecl, HirId, ItemKind, MutTy, Mutability, Node}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty; -use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::Span; -use rustc_target::abi::LayoutOf; -use rustc_target::spec::abi::Abi; -use rustc_target::spec::Target; - -declare_clippy_lint! { - /// **What it does:** Checks for functions taking arguments by reference, where - /// the argument type is `Copy` and small enough to be more efficient to always - /// pass by value. - /// - /// **Why is this bad?** In many calling conventions instances of structs will - /// be passed through registers if they fit into two or less general purpose - /// registers. - /// - /// **Known problems:** This lint is target register size dependent, it is - /// limited to 32-bit to try and reduce portability problems between 32 and - /// 64-bit, but if you are compiling for 8 or 16-bit targets then the limit - /// will be different. - /// - /// The configuration option `trivial_copy_size_limit` can be set to override - /// this limit for a project. - /// - /// This lint attempts to allow passing arguments by reference if a reference - /// to that argument is returned. This is implemented by comparing the lifetime - /// of the argument and return value for equality. However, this can cause - /// false positives in cases involving multiple lifetimes that are bounded by - /// each other. - /// - /// **Example:** - /// - /// ```rust - /// // Bad - /// fn foo(v: &u32) {} - /// ``` - /// - /// ```rust - /// // Better - /// fn foo(v: u32) {} - /// ``` - pub TRIVIALLY_COPY_PASS_BY_REF, - pedantic, - "functions taking small copyable arguments by reference" -} - -#[derive(Copy, Clone)] -pub struct TriviallyCopyPassByRef { - limit: u64, -} - -impl<'tcx> TriviallyCopyPassByRef { - pub fn new(limit: Option<u64>, target: &Target) -> Self { - let limit = limit.unwrap_or_else(|| { - let bit_width = u64::from(target.pointer_width); - // Cap the calculated bit width at 32-bits to reduce - // portability problems between 32 and 64-bit targets - let bit_width = cmp::min(bit_width, 32); - #[allow(clippy::integer_division)] - let byte_width = bit_width / 8; - // Use a limit of 2 times the register byte width - byte_width * 2 - }); - Self { limit } - } - - fn check_poly_fn(&mut self, cx: &LateContext<'tcx>, hir_id: HirId, decl: &FnDecl<'_>, span: Option<Span>) { - let fn_def_id = cx.tcx.hir().local_def_id(hir_id); - - let fn_sig = cx.tcx.fn_sig(fn_def_id); - let fn_sig = cx.tcx.erase_late_bound_regions(&fn_sig); - - // Use lifetimes to determine if we're returning a reference to the - // argument. In that case we can't switch to pass-by-value as the - // argument will not live long enough. - let output_lts = match *fn_sig.output().kind() { - ty::Ref(output_lt, _, _) => vec![output_lt], - ty::Adt(_, substs) => substs.regions().collect(), - _ => vec![], - }; - - for (input, &ty) in decl.inputs.iter().zip(fn_sig.inputs()) { - // All spans generated from a proc-macro invocation are the same... - match span { - Some(s) if s == input.span => return, - _ => (), - } - - if_chain! { - if let ty::Ref(input_lt, ty, Mutability::Not) = ty.kind(); - if !output_lts.contains(&input_lt); - if is_copy(cx, ty); - if let Some(size) = cx.layout_of(ty).ok().map(|l| l.size.bytes()); - if size <= self.limit; - if let hir::TyKind::Rptr(_, MutTy { ty: ref decl_ty, .. }) = input.kind; - then { - let value_type = if is_self_ty(decl_ty) { - "self".into() - } else { - snippet(cx, decl_ty.span, "_").into() - }; - span_lint_and_sugg( - cx, - TRIVIALLY_COPY_PASS_BY_REF, - input.span, - &format!("this argument ({} byte) is passed by reference, but would be more efficient if passed by value (limit: {} byte)", size, self.limit), - "consider passing by value instead", - value_type, - Applicability::Unspecified, - ); - } - } - } - } -} - -impl_lint_pass!(TriviallyCopyPassByRef => [TRIVIALLY_COPY_PASS_BY_REF]); - -impl<'tcx> LateLintPass<'tcx> for TriviallyCopyPassByRef { - fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) { - if item.span.from_expansion() { - return; - } - - if let hir::TraitItemKind::Fn(method_sig, _) = &item.kind { - self.check_poly_fn(cx, item.hir_id, &*method_sig.decl, None); - } - } - - fn check_fn( - &mut self, - cx: &LateContext<'tcx>, - kind: FnKind<'tcx>, - decl: &'tcx FnDecl<'_>, - _body: &'tcx Body<'_>, - span: Span, - hir_id: HirId, - ) { - if span.from_expansion() { - return; - } - - match kind { - FnKind::ItemFn(.., header, _, attrs) => { - if header.abi != Abi::Rust { - return; - } - for a in attrs { - if let Some(meta_items) = a.meta_item_list() { - if a.has_name(sym!(proc_macro_derive)) - || (a.has_name(sym!(inline)) && attr::list_contains_name(&meta_items, sym!(always))) - { - return; - } - } - } - }, - FnKind::Method(..) => (), - FnKind::Closure(..) => return, - } - - // Exclude non-inherent impls - if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { - if matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), .. } | - ItemKind::Trait(..)) - { - return; - } - } - - self.check_poly_fn(cx, hir_id, decl, Some(span)); - } -} diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 9a948af8bfc..6a33aaaaab2 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -11,7 +11,7 @@ use rustc_hir as hir; 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, Node, QPath, Stmt, StmtKind, + ImplItemKind, Item, ItemKind, Lifetime, Lit, Local, MatchSource, MutTy, Mutability, Node, QPath, Stmt, StmtKind, TraitFn, TraitItem, TraitItemKind, TyKind, UnOp, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -1224,7 +1224,8 @@ declare_clippy_lint! { } declare_clippy_lint! { - /// **What it does:** Checks for casts to the same type. + /// **What it does:** Checks for casts to the same type, casts of int literals to integer types + /// and casts of float literals to float types. /// /// **Why is this bad?** It's just unnecessary. /// @@ -1233,6 +1234,14 @@ declare_clippy_lint! { /// **Example:** /// ```rust /// let _ = 2i32 as i32; + /// let _ = 0.5 as f32; + /// ``` + /// + /// Better: + /// + /// ```rust + /// let _ = 2_i32; + /// let _ = 0.5_f32; /// ``` pub UNNECESSARY_CAST, complexity, @@ -1598,7 +1607,9 @@ impl<'tcx> LateLintPass<'tcx> for Casts { if let ExprKind::Cast(ref ex, _) = expr.kind { let (cast_from, cast_to) = (cx.typeck_results().expr_ty(ex), cx.typeck_results().expr_ty(expr)); lint_fn_to_numeric_cast(cx, expr, ex, cast_from, cast_to); - if let ExprKind::Lit(ref lit) = ex.kind { + if let Some(lit) = get_numeric_literal(ex) { + let literal_str = snippet_opt(cx, ex.span).unwrap_or_default(); + if_chain! { if let LitKind::Int(n, _) = lit.node; if let Some(src) = snippet_opt(cx, lit.span); @@ -1608,19 +1619,19 @@ impl<'tcx> LateLintPass<'tcx> for Casts { let to_nbits = fp_ty_mantissa_nbits(cast_to); if from_nbits != 0 && to_nbits != 0 && from_nbits <= to_nbits && num_lit.is_decimal(); then { - span_lint_and_sugg( - cx, - UNNECESSARY_CAST, - expr.span, - &format!("casting integer literal to `{}` is unnecessary", cast_to), - "try", - format!("{}_{}", n, cast_to), - Applicability::MachineApplicable, - ); + let literal_str = if is_unary_neg(ex) { format!("-{}", num_lit.integer) } else { num_lit.integer.into() }; + show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); return; } } + match lit.node { + LitKind::Int(_, LitIntType::Unsuffixed) if cast_to.is_integral() => { + show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); + }, + LitKind::Float(_, LitFloatType::Unsuffixed) if cast_to.is_floating_point() => { + show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); + }, LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed) => {}, _ => { if cast_from.kind() == cast_to.kind() && !in_external_macro(cx.sess(), expr.span) { @@ -1646,6 +1657,37 @@ impl<'tcx> LateLintPass<'tcx> for Casts { } } +fn is_unary_neg(expr: &Expr<'_>) -> bool { + matches!(expr.kind, ExprKind::Unary(UnOp::UnNeg, _)) +} + +fn get_numeric_literal<'e>(expr: &'e Expr<'e>) -> Option<&'e Lit> { + match expr.kind { + ExprKind::Lit(ref lit) => Some(lit), + ExprKind::Unary(UnOp::UnNeg, e) => { + if let ExprKind::Lit(ref lit) = e.kind { + Some(lit) + } else { + None + } + }, + _ => None, + } +} + +fn show_unnecessary_cast(cx: &LateContext<'_>, expr: &Expr<'_>, literal_str: &str, cast_from: Ty<'_>, cast_to: Ty<'_>) { + let literal_kind_name = if cast_from.is_integral() { "integer" } else { "float" }; + span_lint_and_sugg( + cx, + UNNECESSARY_CAST, + expr.span, + &format!("casting {} literal to `{}` is unnecessary", literal_kind_name, cast_to), + "try", + format!("{}_{}", literal_str, cast_to), + Applicability::MachineApplicable, + ); +} + fn lint_numeric_casts<'tcx>( cx: &LateContext<'tcx>, expr: &Expr<'tcx>, diff --git a/clippy_lints/src/undropped_manually_drops.rs b/clippy_lints/src/undropped_manually_drops.rs new file mode 100644 index 00000000000..5443f1601fc --- /dev/null +++ b/clippy_lints/src/undropped_manually_drops.rs @@ -0,0 +1,50 @@ +use crate::utils::{is_type_lang_item, match_function_call, paths, span_lint_and_help}; +use rustc_hir::{lang_items, Expr}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// **What it does:** Prevents the safe `std::mem::drop` function from being called on `std::mem::ManuallyDrop`. + /// + /// **Why is this bad?** The safe `drop` function does not drop the inner value of a `ManuallyDrop`. + /// + /// **Known problems:** Does not catch cases if the user binds `std::mem::drop` + /// to a different name and calls it that way. + /// + /// **Example:** + /// + /// ```rust + /// struct S; + /// drop(std::mem::ManuallyDrop::new(S)); + /// ``` + /// Use instead: + /// ```rust + /// struct S; + /// unsafe { + /// std::mem::ManuallyDrop::drop(&mut std::mem::ManuallyDrop::new(S)); + /// } + /// ``` + pub UNDROPPED_MANUALLY_DROPS, + correctness, + "use of safe `std::mem::drop` function to drop a std::mem::ManuallyDrop, which will not drop the inner value" +} + +declare_lint_pass!(UndroppedManuallyDrops => [UNDROPPED_MANUALLY_DROPS]); + +impl LateLintPass<'tcx> for UndroppedManuallyDrops { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if let Some(ref args) = match_function_call(cx, expr, &paths::DROP) { + let ty = cx.typeck_results().expr_ty(&args[0]); + if is_type_lang_item(cx, ty, lang_items::LangItem::ManuallyDrop) { + span_lint_and_help( + cx, + UNDROPPED_MANUALLY_DROPS, + expr.span, + "the inner value of this ManuallyDrop will not be dropped", + None, + "to drop a `ManuallyDrop<T>`, use std::mem::ManuallyDrop::drop", + ); + } + } + } +} diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index dd2fd0bb445..0ac8fff69f0 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -150,6 +150,8 @@ define_Conf! { (literal_representation_threshold, "literal_representation_threshold": u64, 16384), /// Lint: TRIVIALLY_COPY_PASS_BY_REF. The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by reference. (trivial_copy_size_limit, "trivial_copy_size_limit": Option<u64>, None), + /// Lint: LARGE_TYPE_PASS_BY_MOVE. The minimum size (in bytes) to consider a type for passing by reference instead of by value. + (pass_by_value_size_limit, "pass_by_value_size_limit": u64, 256), /// Lint: TOO_MANY_LINES. The maximum number of lines a function or method can have (too_many_lines_threshold, "too_many_lines_threshold": u64, 100), /// Lint: LARGE_STACK_ARRAYS, LARGE_CONST_ARRAYS. The maximum allowed size for arrays on the stack diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index a9d26d48b12..8297b9d128d 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -299,7 +299,7 @@ pub fn qpath_res(cx: &LateContext<'_>, qpath: &hir::QPath<'_>, id: hir::HirId) - hir::QPath::Resolved(_, path) => path.res, hir::QPath::TypeRelative(..) | hir::QPath::LangItem(..) => { if cx.tcx.has_typeck_results(id.owner.to_def_id()) { - cx.tcx.typeck(id.owner.to_def_id().expect_local()).qpath_res(qpath, id) + cx.tcx.typeck(id.owner).qpath_res(qpath, id) } else { Res::Err } diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 5e769c690a6..cd9b92efe58 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -93,6 +93,8 @@ pub const RANGE_ARGUMENT_TRAIT: [&str; 3] = ["core", "ops", "RangeBounds"]; pub const RC: [&str; 3] = ["alloc", "rc", "Rc"]; pub const RC_PTR_EQ: [&str; 4] = ["alloc", "rc", "Rc", "ptr_eq"]; pub const RECEIVER: [&str; 4] = ["std", "sync", "mpsc", "Receiver"]; +pub const REFCELL_REF: [&str; 3] = ["core", "cell", "Ref"]; +pub const REFCELL_REFMUT: [&str; 3] = ["core", "cell", "RefMut"]; pub const REGEX_BUILDER_NEW: [&str; 5] = ["regex", "re_builder", "unicode", "RegexBuilder", "new"]; pub const REGEX_BYTES_BUILDER_NEW: [&str; 5] = ["regex", "re_builder", "bytes", "RegexBuilder", "new"]; pub const REGEX_BYTES_NEW: [&str; 4] = ["regex", "re_bytes", "Regex", "new"]; diff --git a/clippy_lints/src/utils/usage.rs b/clippy_lints/src/utils/usage.rs index 2fd6046ebcf..8b327b2d467 100644 --- a/clippy_lints/src/utils/usage.rs +++ b/clippy_lints/src/utils/usage.rs @@ -19,12 +19,11 @@ pub fn mutated_variables<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) -> used_mutably: FxHashSet::default(), skip: false, }; - let def_id = expr.hir_id.owner.to_def_id(); cx.tcx.infer_ctxt().enter(|infcx| { ExprUseVisitor::new( &mut delegate, &infcx, - def_id.expect_local(), + expr.hir_id.owner, cx.param_env, cx.typeck_results(), ) diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 6301d623a2b..4bf77dae637 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -62,10 +62,17 @@ vec for further information)", diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 103ec27e7d7..a58e7e918e2 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -1,4 +1,4 @@ -error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `third-party` at line 5 column 1 +error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `third-party` at line 5 column 1 error: aborting due to previous error diff --git a/tests/ui/await_holding_refcell_ref.rs b/tests/ui/await_holding_refcell_ref.rs new file mode 100644 index 00000000000..88841597bb6 --- /dev/null +++ b/tests/ui/await_holding_refcell_ref.rs @@ -0,0 +1,86 @@ +// edition:2018 +#![warn(clippy::await_holding_refcell_ref)] + +use std::cell::RefCell; + +async fn bad(x: &RefCell<u32>) -> u32 { + let b = x.borrow(); + baz().await +} + +async fn bad_mut(x: &RefCell<u32>) -> u32 { + let b = x.borrow_mut(); + baz().await +} + +async fn good(x: &RefCell<u32>) -> u32 { + { + let b = x.borrow_mut(); + let y = *b + 1; + } + baz().await; + let b = x.borrow_mut(); + 47 +} + +async fn baz() -> u32 { + 42 +} + +async fn also_bad(x: &RefCell<u32>) -> u32 { + let first = baz().await; + + let b = x.borrow_mut(); + + let second = baz().await; + + let third = baz().await; + + first + second + third +} + +async fn less_bad(x: &RefCell<u32>) -> u32 { + let first = baz().await; + + let b = x.borrow_mut(); + + let second = baz().await; + + drop(b); + + let third = baz().await; + + first + second + third +} + +async fn not_good(x: &RefCell<u32>) -> u32 { + let first = baz().await; + + let second = { + let b = x.borrow_mut(); + baz().await + }; + + let third = baz().await; + + first + second + third +} + +#[allow(clippy::manual_async_fn)] +fn block_bad(x: &RefCell<u32>) -> impl std::future::Future<Output = u32> + '_ { + async move { + let b = x.borrow_mut(); + baz().await + } +} + +fn main() { + let rc = RefCell::new(100); + good(&rc); + bad(&rc); + bad_mut(&rc); + also_bad(&rc); + less_bad(&rc); + not_good(&rc); + block_bad(&rc); +} diff --git a/tests/ui/await_holding_refcell_ref.stderr b/tests/ui/await_holding_refcell_ref.stderr new file mode 100644 index 00000000000..b504f045491 --- /dev/null +++ b/tests/ui/await_holding_refcell_ref.stderr @@ -0,0 +1,95 @@ +error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await. + --> $DIR/await_holding_refcell_ref.rs:7:9 + | +LL | let b = x.borrow(); + | ^ + | + = note: `-D clippy::await-holding-refcell-ref` implied by `-D warnings` +note: these are all the await points this ref is held through + --> $DIR/await_holding_refcell_ref.rs:7:5 + | +LL | / let b = x.borrow(); +LL | | baz().await +LL | | } + | |_^ + +error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await. + --> $DIR/await_holding_refcell_ref.rs:12:9 + | +LL | let b = x.borrow_mut(); + | ^ + | +note: these are all the await points this ref is held through + --> $DIR/await_holding_refcell_ref.rs:12:5 + | +LL | / let b = x.borrow_mut(); +LL | | baz().await +LL | | } + | |_^ + +error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await. + --> $DIR/await_holding_refcell_ref.rs:33:9 + | +LL | let b = x.borrow_mut(); + | ^ + | +note: these are all the await points this ref is held through + --> $DIR/await_holding_refcell_ref.rs:33:5 + | +LL | / let b = x.borrow_mut(); +LL | | +LL | | let second = baz().await; +LL | | +... | +LL | | first + second + third +LL | | } + | |_^ + +error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await. + --> $DIR/await_holding_refcell_ref.rs:45:9 + | +LL | let b = x.borrow_mut(); + | ^ + | +note: these are all the await points this ref is held through + --> $DIR/await_holding_refcell_ref.rs:45:5 + | +LL | / let b = x.borrow_mut(); +LL | | +LL | | let second = baz().await; +LL | | +... | +LL | | first + second + third +LL | | } + | |_^ + +error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await. + --> $DIR/await_holding_refcell_ref.rs:60:13 + | +LL | let b = x.borrow_mut(); + | ^ + | +note: these are all the await points this ref is held through + --> $DIR/await_holding_refcell_ref.rs:60:9 + | +LL | / let b = x.borrow_mut(); +LL | | baz().await +LL | | }; + | |_____^ + +error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await. + --> $DIR/await_holding_refcell_ref.rs:72:13 + | +LL | let b = x.borrow_mut(); + | ^ + | +note: these are all the await points this ref is held through + --> $DIR/await_holding_refcell_ref.rs:72:9 + | +LL | / let b = x.borrow_mut(); +LL | | baz().await +LL | | } + | |_____^ + +error: aborting due to 6 previous errors + diff --git a/tests/ui/bool_comparison.fixed b/tests/ui/bool_comparison.fixed index 91211764759..5a012ff4d27 100644 --- a/tests/ui/bool_comparison.fixed +++ b/tests/ui/bool_comparison.fixed @@ -1,6 +1,7 @@ // run-rustfix -#[warn(clippy::bool_comparison)] +#![warn(clippy::bool_comparison)] + fn main() { let x = true; if x { @@ -127,3 +128,40 @@ fn issue4983() { if b == a {}; if !b == !a {}; } + +macro_rules! m { + ($func:ident) => { + $func() + }; +} + +fn func() -> bool { + true +} + +#[allow(dead_code)] +fn issue3973() { + // ok, don't lint on `cfg` invocation + if false == cfg!(feature = "debugging") {} + if cfg!(feature = "debugging") == false {} + if true == cfg!(feature = "debugging") {} + if cfg!(feature = "debugging") == true {} + + // lint, could be simplified + if !m!(func) {} + if !m!(func) {} + if m!(func) {} + if m!(func) {} + + // no lint with a variable + let is_debug = false; + if is_debug == cfg!(feature = "debugging") {} + if cfg!(feature = "debugging") == is_debug {} + if is_debug == m!(func) {} + if m!(func) == is_debug {} + let is_debug = true; + if is_debug == cfg!(feature = "debugging") {} + if cfg!(feature = "debugging") == is_debug {} + if is_debug == m!(func) {} + if m!(func) == is_debug {} +} diff --git a/tests/ui/bool_comparison.rs b/tests/ui/bool_comparison.rs index 01ee35859f0..c534bc25c20 100644 --- a/tests/ui/bool_comparison.rs +++ b/tests/ui/bool_comparison.rs @@ -1,6 +1,7 @@ // run-rustfix -#[warn(clippy::bool_comparison)] +#![warn(clippy::bool_comparison)] + fn main() { let x = true; if x == true { @@ -127,3 +128,40 @@ fn issue4983() { if b == a {}; if !b == !a {}; } + +macro_rules! m { + ($func:ident) => { + $func() + }; +} + +fn func() -> bool { + true +} + +#[allow(dead_code)] +fn issue3973() { + // ok, don't lint on `cfg` invocation + if false == cfg!(feature = "debugging") {} + if cfg!(feature = "debugging") == false {} + if true == cfg!(feature = "debugging") {} + if cfg!(feature = "debugging") == true {} + + // lint, could be simplified + if false == m!(func) {} + if m!(func) == false {} + if true == m!(func) {} + if m!(func) == true {} + + // no lint with a variable + let is_debug = false; + if is_debug == cfg!(feature = "debugging") {} + if cfg!(feature = "debugging") == is_debug {} + if is_debug == m!(func) {} + if m!(func) == is_debug {} + let is_debug = true; + if is_debug == cfg!(feature = "debugging") {} + if cfg!(feature = "debugging") == is_debug {} + if is_debug == m!(func) {} + if m!(func) == is_debug {} +} diff --git a/tests/ui/bool_comparison.stderr b/tests/ui/bool_comparison.stderr index 55d94b8257d..31522d4a525 100644 --- a/tests/ui/bool_comparison.stderr +++ b/tests/ui/bool_comparison.stderr @@ -1,5 +1,5 @@ error: equality checks against true are unnecessary - --> $DIR/bool_comparison.rs:6:8 + --> $DIR/bool_comparison.rs:7:8 | LL | if x == true { | ^^^^^^^^^ help: try simplifying it as shown: `x` @@ -7,106 +7,130 @@ LL | if x == true { = note: `-D clippy::bool-comparison` implied by `-D warnings` error: equality checks against false can be replaced by a negation - --> $DIR/bool_comparison.rs:11:8 + --> $DIR/bool_comparison.rs:12:8 | LL | if x == false { | ^^^^^^^^^^ help: try simplifying it as shown: `!x` error: equality checks against true are unnecessary - --> $DIR/bool_comparison.rs:16:8 + --> $DIR/bool_comparison.rs:17:8 | LL | if true == x { | ^^^^^^^^^ help: try simplifying it as shown: `x` error: equality checks against false can be replaced by a negation - --> $DIR/bool_comparison.rs:21:8 + --> $DIR/bool_comparison.rs:22:8 | LL | if false == x { | ^^^^^^^^^^ help: try simplifying it as shown: `!x` error: inequality checks against true can be replaced by a negation - --> $DIR/bool_comparison.rs:26:8 + --> $DIR/bool_comparison.rs:27:8 | LL | if x != true { | ^^^^^^^^^ help: try simplifying it as shown: `!x` error: inequality checks against false are unnecessary - --> $DIR/bool_comparison.rs:31:8 + --> $DIR/bool_comparison.rs:32:8 | LL | if x != false { | ^^^^^^^^^^ help: try simplifying it as shown: `x` error: inequality checks against true can be replaced by a negation - --> $DIR/bool_comparison.rs:36:8 + --> $DIR/bool_comparison.rs:37:8 | LL | if true != x { | ^^^^^^^^^ help: try simplifying it as shown: `!x` error: inequality checks against false are unnecessary - --> $DIR/bool_comparison.rs:41:8 + --> $DIR/bool_comparison.rs:42:8 | LL | if false != x { | ^^^^^^^^^^ help: try simplifying it as shown: `x` error: less than comparison against true can be replaced by a negation - --> $DIR/bool_comparison.rs:46:8 + --> $DIR/bool_comparison.rs:47:8 | LL | if x < true { | ^^^^^^^^ help: try simplifying it as shown: `!x` error: greater than checks against false are unnecessary - --> $DIR/bool_comparison.rs:51:8 + --> $DIR/bool_comparison.rs:52:8 | LL | if false < x { | ^^^^^^^^^ help: try simplifying it as shown: `x` error: greater than checks against false are unnecessary - --> $DIR/bool_comparison.rs:56:8 + --> $DIR/bool_comparison.rs:57:8 | LL | if x > false { | ^^^^^^^^^ help: try simplifying it as shown: `x` error: less than comparison against true can be replaced by a negation - --> $DIR/bool_comparison.rs:61:8 + --> $DIR/bool_comparison.rs:62:8 | LL | if true > x { | ^^^^^^^^ help: try simplifying it as shown: `!x` error: order comparisons between booleans can be simplified - --> $DIR/bool_comparison.rs:67:8 + --> $DIR/bool_comparison.rs:68:8 | LL | if x < y { | ^^^^^ help: try simplifying it as shown: `!x & y` error: order comparisons between booleans can be simplified - --> $DIR/bool_comparison.rs:72:8 + --> $DIR/bool_comparison.rs:73:8 | LL | if x > y { | ^^^^^ help: try simplifying it as shown: `x & !y` error: this comparison might be written more concisely - --> $DIR/bool_comparison.rs:120:8 + --> $DIR/bool_comparison.rs:121:8 | LL | if a == !b {}; | ^^^^^^^ help: try simplifying it as shown: `a != b` error: this comparison might be written more concisely - --> $DIR/bool_comparison.rs:121:8 + --> $DIR/bool_comparison.rs:122:8 | LL | if !a == b {}; | ^^^^^^^ help: try simplifying it as shown: `a != b` error: this comparison might be written more concisely - --> $DIR/bool_comparison.rs:125:8 + --> $DIR/bool_comparison.rs:126:8 | LL | if b == !a {}; | ^^^^^^^ help: try simplifying it as shown: `b != a` error: this comparison might be written more concisely - --> $DIR/bool_comparison.rs:126:8 + --> $DIR/bool_comparison.rs:127:8 | LL | if !b == a {}; | ^^^^^^^ help: try simplifying it as shown: `b != a` -error: aborting due to 18 previous errors +error: equality checks against false can be replaced by a negation + --> $DIR/bool_comparison.rs:151:8 + | +LL | if false == m!(func) {} + | ^^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `!m!(func)` + +error: equality checks against false can be replaced by a negation + --> $DIR/bool_comparison.rs:152:8 + | +LL | if m!(func) == false {} + | ^^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `!m!(func)` + +error: equality checks against true are unnecessary + --> $DIR/bool_comparison.rs:153:8 + | +LL | if true == m!(func) {} + | ^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `m!(func)` + +error: equality checks against true are unnecessary + --> $DIR/bool_comparison.rs:154:8 + | +LL | if m!(func) == true {} + | ^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `m!(func)` + +error: aborting due to 22 previous errors diff --git a/tests/ui/cast_size_32bit.stderr b/tests/ui/cast_size_32bit.stderr index 2eec51895f5..140676a5ffc 100644 --- a/tests/ui/cast_size_32bit.stderr +++ b/tests/ui/cast_size_32bit.stderr @@ -124,7 +124,7 @@ error: casting integer literal to `f64` is unnecessary --> $DIR/cast_size_32bit.rs:34:5 | LL | 3_999_999_999usize as f64; - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `3999999999_f64` + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `3_999_999_999_f64` | = note: `-D clippy::unnecessary-cast` implied by `-D warnings` diff --git a/tests/ui/crashes/ice-360.stderr b/tests/ui/crashes/ice-360.stderr index 84e31eaf2e9..bb03ce40355 100644 --- a/tests/ui/crashes/ice-360.stderr +++ b/tests/ui/crashes/ice-360.stderr @@ -12,13 +12,14 @@ LL | | } | = note: `-D clippy::while-let-loop` implied by `-D warnings` -error: empty `loop {}` detected. You may want to either use `panic!()` or add `std::thread::sleep(..);` to the loop body. +error: empty `loop {}` wastes CPU cycles --> $DIR/ice-360.rs:10:9 | LL | loop {} | ^^^^^^^ | = note: `-D clippy::empty-loop` implied by `-D warnings` + = help: You should either use `panic!()` or add `std::thread::sleep(..);` to the loop body. error: aborting due to 2 previous errors diff --git a/tests/ui/empty_loop.stderr b/tests/ui/empty_loop.stderr index e44c58ea770..fd3979f259a 100644 --- a/tests/ui/empty_loop.stderr +++ b/tests/ui/empty_loop.stderr @@ -1,22 +1,27 @@ -error: empty `loop {}` detected. You may want to either use `panic!()` or add `std::thread::sleep(..);` to the loop body. +error: empty `loop {}` wastes CPU cycles --> $DIR/empty_loop.rs:9:5 | LL | loop {} | ^^^^^^^ | = note: `-D clippy::empty-loop` implied by `-D warnings` + = help: You should either use `panic!()` or add `std::thread::sleep(..);` to the loop body. -error: empty `loop {}` detected. You may want to either use `panic!()` or add `std::thread::sleep(..);` to the loop body. +error: empty `loop {}` wastes CPU cycles --> $DIR/empty_loop.rs:11:9 | LL | loop {} | ^^^^^^^ + | + = help: You should either use `panic!()` or add `std::thread::sleep(..);` to the loop body. -error: empty `loop {}` detected. You may want to either use `panic!()` or add `std::thread::sleep(..);` to the loop body. +error: empty `loop {}` wastes CPU cycles --> $DIR/empty_loop.rs:15:9 | LL | 'inner: loop {} | ^^^^^^^^^^^^^^^ + | + = help: You should either use `panic!()` or add `std::thread::sleep(..);` to the loop body. error: aborting due to 3 previous errors diff --git a/tests/ui/issue-3746.rs b/tests/ui/empty_loop_no_std.rs index 879d1d5d916..879d1d5d916 100644 --- a/tests/ui/issue-3746.rs +++ b/tests/ui/empty_loop_no_std.rs diff --git a/tests/ui/eq_op.rs b/tests/ui/eq_op.rs index 272b0900a31..4e09d19ea21 100644 --- a/tests/ui/eq_op.rs +++ b/tests/ui/eq_op.rs @@ -6,6 +6,7 @@ #[allow(clippy::no_effect, unused_variables, clippy::unnecessary_operation, clippy::short_circuit_statement)] #[allow(clippy::nonminimal_bool)] #[allow(unused)] +#[allow(clippy::unnecessary_cast)] fn main() { // simple values and comparisons 1 == 1; diff --git a/tests/ui/eq_op.stderr b/tests/ui/eq_op.stderr index 5b80e6078ee..ad81b35a766 100644 --- a/tests/ui/eq_op.stderr +++ b/tests/ui/eq_op.stderr @@ -1,5 +1,5 @@ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:11:5 + --> $DIR/eq_op.rs:12:5 | LL | 1 == 1; | ^^^^^^ @@ -7,157 +7,157 @@ LL | 1 == 1; = note: `-D clippy::eq-op` implied by `-D warnings` error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:12:5 + --> $DIR/eq_op.rs:13:5 | LL | "no" == "no"; | ^^^^^^^^^^^^ error: equal expressions as operands to `!=` - --> $DIR/eq_op.rs:14:5 + --> $DIR/eq_op.rs:15:5 | LL | false != false; | ^^^^^^^^^^^^^^ error: equal expressions as operands to `<` - --> $DIR/eq_op.rs:15:5 + --> $DIR/eq_op.rs:16:5 | LL | 1.5 < 1.5; | ^^^^^^^^^ error: equal expressions as operands to `>=` - --> $DIR/eq_op.rs:16:5 + --> $DIR/eq_op.rs:17:5 | LL | 1u64 >= 1u64; | ^^^^^^^^^^^^ error: equal expressions as operands to `&` - --> $DIR/eq_op.rs:19:5 + --> $DIR/eq_op.rs:20:5 | LL | (1 as u64) & (1 as u64); | ^^^^^^^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `^` - --> $DIR/eq_op.rs:20:5 + --> $DIR/eq_op.rs:21:5 | LL | 1 ^ ((((((1)))))); | ^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `<` - --> $DIR/eq_op.rs:23:5 + --> $DIR/eq_op.rs:24:5 | LL | (-(2) < -(2)); | ^^^^^^^^^^^^^ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:24:5 + --> $DIR/eq_op.rs:25:5 | LL | ((1 + 1) & (1 + 1) == (1 + 1) & (1 + 1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `&` - --> $DIR/eq_op.rs:24:6 + --> $DIR/eq_op.rs:25:6 | LL | ((1 + 1) & (1 + 1) == (1 + 1) & (1 + 1)); | ^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `&` - --> $DIR/eq_op.rs:24:27 + --> $DIR/eq_op.rs:25:27 | LL | ((1 + 1) & (1 + 1) == (1 + 1) & (1 + 1)); | ^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:25:5 + --> $DIR/eq_op.rs:26:5 | LL | (1 * 2) + (3 * 4) == 1 * 2 + 3 * 4; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `!=` - --> $DIR/eq_op.rs:28:5 + --> $DIR/eq_op.rs:29:5 | LL | ([1] != [1]); | ^^^^^^^^^^^^ error: equal expressions as operands to `!=` - --> $DIR/eq_op.rs:29:5 + --> $DIR/eq_op.rs:30:5 | LL | ((1, 2) != (1, 2)); | ^^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:33:5 + --> $DIR/eq_op.rs:34:5 | LL | 1 + 1 == 2; | ^^^^^^^^^^ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:34:5 + --> $DIR/eq_op.rs:35:5 | LL | 1 - 1 == 0; | ^^^^^^^^^^ error: equal expressions as operands to `-` - --> $DIR/eq_op.rs:34:5 + --> $DIR/eq_op.rs:35:5 | LL | 1 - 1 == 0; | ^^^^^ error: equal expressions as operands to `-` - --> $DIR/eq_op.rs:36:5 + --> $DIR/eq_op.rs:37:5 | LL | 1 - 1; | ^^^^^ error: equal expressions as operands to `/` - --> $DIR/eq_op.rs:37:5 + --> $DIR/eq_op.rs:38:5 | LL | 1 / 1; | ^^^^^ error: equal expressions as operands to `&&` - --> $DIR/eq_op.rs:38:5 + --> $DIR/eq_op.rs:39:5 | LL | true && true; | ^^^^^^^^^^^^ error: equal expressions as operands to `||` - --> $DIR/eq_op.rs:40:5 + --> $DIR/eq_op.rs:41:5 | LL | true || true; | ^^^^^^^^^^^^ error: equal expressions as operands to `&&` - --> $DIR/eq_op.rs:46:5 + --> $DIR/eq_op.rs:47:5 | LL | a == b && b == a; | ^^^^^^^^^^^^^^^^ error: equal expressions as operands to `&&` - --> $DIR/eq_op.rs:47:5 + --> $DIR/eq_op.rs:48:5 | LL | a != b && b != a; | ^^^^^^^^^^^^^^^^ error: equal expressions as operands to `&&` - --> $DIR/eq_op.rs:48:5 + --> $DIR/eq_op.rs:49:5 | LL | a < b && b > a; | ^^^^^^^^^^^^^^ error: equal expressions as operands to `&&` - --> $DIR/eq_op.rs:49:5 + --> $DIR/eq_op.rs:50:5 | LL | a <= b && b >= a; | ^^^^^^^^^^^^^^^^ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:52:5 + --> $DIR/eq_op.rs:53:5 | LL | a == a; | ^^^^^^ error: equal expressions as operands to `/` - --> $DIR/eq_op.rs:62:20 + --> $DIR/eq_op.rs:63:20 | LL | const D: u32 = A / A; | ^^^^^ diff --git a/tests/ui/if_same_then_else2.rs b/tests/ui/if_same_then_else2.rs index 3cc21809264..8d54f75b5d1 100644 --- a/tests/ui/if_same_then_else2.rs +++ b/tests/ui/if_same_then_else2.rs @@ -3,7 +3,8 @@ clippy::blacklisted_name, clippy::collapsible_if, clippy::ifs_same_cond, - clippy::needless_return + clippy::needless_return, + clippy::single_element_loop )] fn if_same_then_else2() -> Result<&'static str, ()> { diff --git a/tests/ui/if_same_then_else2.stderr b/tests/ui/if_same_then_else2.stderr index f5d087fe128..da2be6c8aa5 100644 --- a/tests/ui/if_same_then_else2.stderr +++ b/tests/ui/if_same_then_else2.stderr @@ -1,5 +1,5 @@ error: this `if` has identical blocks - --> $DIR/if_same_then_else2.rs:19:12 + --> $DIR/if_same_then_else2.rs:20:12 | LL | } else { | ____________^ @@ -13,7 +13,7 @@ LL | | } | = note: `-D clippy::if-same-then-else` implied by `-D warnings` note: same as this - --> $DIR/if_same_then_else2.rs:10:13 + --> $DIR/if_same_then_else2.rs:11:13 | LL | if true { | _____________^ @@ -26,7 +26,7 @@ LL | | } else { | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else2.rs:33:12 + --> $DIR/if_same_then_else2.rs:34:12 | LL | } else { | ____________^ @@ -36,7 +36,7 @@ LL | | } | |_____^ | note: same as this - --> $DIR/if_same_then_else2.rs:31:13 + --> $DIR/if_same_then_else2.rs:32:13 | LL | if true { | _____________^ @@ -45,7 +45,7 @@ LL | | } else { | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else2.rs:40:12 + --> $DIR/if_same_then_else2.rs:41:12 | LL | } else { | ____________^ @@ -55,7 +55,7 @@ LL | | } | |_____^ | note: same as this - --> $DIR/if_same_then_else2.rs:38:13 + --> $DIR/if_same_then_else2.rs:39:13 | LL | if true { | _____________^ @@ -64,7 +64,7 @@ LL | | } else { | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else2.rs:90:12 + --> $DIR/if_same_then_else2.rs:91:12 | LL | } else { | ____________^ @@ -74,7 +74,7 @@ LL | | }; | |_____^ | note: same as this - --> $DIR/if_same_then_else2.rs:88:21 + --> $DIR/if_same_then_else2.rs:89:21 | LL | let _ = if true { | _____________________^ @@ -83,7 +83,7 @@ LL | | } else { | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else2.rs:97:12 + --> $DIR/if_same_then_else2.rs:98:12 | LL | } else { | ____________^ @@ -93,7 +93,7 @@ LL | | } | |_____^ | note: same as this - --> $DIR/if_same_then_else2.rs:95:13 + --> $DIR/if_same_then_else2.rs:96:13 | LL | if true { | _____________^ @@ -102,7 +102,7 @@ LL | | } else { | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else2.rs:122:12 + --> $DIR/if_same_then_else2.rs:123:12 | LL | } else { | ____________^ @@ -112,7 +112,7 @@ LL | | } | |_____^ | note: same as this - --> $DIR/if_same_then_else2.rs:119:20 + --> $DIR/if_same_then_else2.rs:120:20 | LL | } else if true { | ____________________^ diff --git a/tests/ui/large_types_passed_by_value.rs b/tests/ui/large_types_passed_by_value.rs new file mode 100644 index 00000000000..e4a2e9df4d7 --- /dev/null +++ b/tests/ui/large_types_passed_by_value.rs @@ -0,0 +1,66 @@ +// normalize-stderr-test "\(\d+ byte\)" -> "(N byte)" +// normalize-stderr-test "\(limit: \d+ byte\)" -> "(limit: N byte)" + +#![warn(clippy::large_types_passed_by_value)] + +pub struct Large([u8; 2048]); + +#[derive(Clone, Copy)] +pub struct LargeAndCopy([u8; 2048]); + +pub struct Small([u8; 4]); + +#[derive(Clone, Copy)] +pub struct SmallAndCopy([u8; 4]); + +fn small(a: Small, b: SmallAndCopy) {} +fn not_copy(a: Large) {} +fn by_ref(a: &Large, b: &LargeAndCopy) {} +fn mutable(mut a: LargeAndCopy) {} +fn bad(a: LargeAndCopy) {} +pub fn bad_but_pub(a: LargeAndCopy) {} + +impl LargeAndCopy { + fn self_is_ok(self) {} + fn other_is_not_ok(self, other: LargeAndCopy) {} + fn unless_other_can_change(self, mut other: LargeAndCopy) {} + pub fn or_were_in_public(self, other: LargeAndCopy) {} +} + +trait LargeTypeDevourer { + fn devoure_array(&self, array: [u8; 6666]); + fn devoure_tuple(&self, tup: (LargeAndCopy, LargeAndCopy)); + fn devoure_array_and_tuple_wow(&self, array: [u8; 6666], tup: (LargeAndCopy, LargeAndCopy)); +} + +pub trait PubLargeTypeDevourer { + fn devoure_array_in_public(&self, array: [u8; 6666]); +} + +struct S {} +impl LargeTypeDevourer for S { + fn devoure_array(&self, array: [u8; 6666]) { + todo!(); + } + fn devoure_tuple(&self, tup: (LargeAndCopy, LargeAndCopy)) { + todo!(); + } + fn devoure_array_and_tuple_wow(&self, array: [u8; 6666], tup: (LargeAndCopy, LargeAndCopy)) { + todo!(); + } +} + +#[inline(always)] +fn foo_always(x: LargeAndCopy) { + todo!(); +} +#[inline(never)] +fn foo_never(x: LargeAndCopy) { + todo!(); +} +#[inline] +fn foo(x: LargeAndCopy) { + todo!(); +} + +fn main() {} diff --git a/tests/ui/large_types_passed_by_value.stderr b/tests/ui/large_types_passed_by_value.stderr new file mode 100644 index 00000000000..5f42dcfb9b5 --- /dev/null +++ b/tests/ui/large_types_passed_by_value.stderr @@ -0,0 +1,52 @@ +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:20:11 + | +LL | fn bad(a: LargeAndCopy) {} + | ^^^^^^^^^^^^ help: consider passing by reference instead: `&LargeAndCopy` + | + = note: `-D clippy::large-types-passed-by-value` implied by `-D warnings` + +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:25:37 + | +LL | fn other_is_not_ok(self, other: LargeAndCopy) {} + | ^^^^^^^^^^^^ help: consider passing by reference instead: `&LargeAndCopy` + +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:31:36 + | +LL | fn devoure_array(&self, array: [u8; 6666]); + | ^^^^^^^^^^ help: consider passing by reference instead: `&[u8; 6666]` + +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:32:34 + | +LL | fn devoure_tuple(&self, tup: (LargeAndCopy, LargeAndCopy)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider passing by reference instead: `&(LargeAndCopy, LargeAndCopy)` + +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:33:50 + | +LL | fn devoure_array_and_tuple_wow(&self, array: [u8; 6666], tup: (LargeAndCopy, LargeAndCopy)); + | ^^^^^^^^^^ help: consider passing by reference instead: `&[u8; 6666]` + +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:33:67 + | +LL | fn devoure_array_and_tuple_wow(&self, array: [u8; 6666], tup: (LargeAndCopy, LargeAndCopy)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider passing by reference instead: `&(LargeAndCopy, LargeAndCopy)` + +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:58:17 + | +LL | fn foo_never(x: LargeAndCopy) { + | ^^^^^^^^^^^^ help: consider passing by reference instead: `&LargeAndCopy` + +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:62:11 + | +LL | fn foo(x: LargeAndCopy) { + | ^^^^^^^^^^^^ help: consider passing by reference instead: `&LargeAndCopy` + +error: aborting due to 8 previous errors + diff --git a/tests/ui/manual_unwrap_or.fixed b/tests/ui/manual_unwrap_or.fixed index a8736f1e6ef..5aa5a43cb92 100644 --- a/tests/ui/manual_unwrap_or.fixed +++ b/tests/ui/manual_unwrap_or.fixed @@ -1,7 +1,8 @@ // run-rustfix #![allow(dead_code)] +#![allow(unused_variables)] -fn unwrap_or() { +fn option_unwrap_or() { // int case Some(1).unwrap_or(42); @@ -65,4 +66,74 @@ fn unwrap_or() { }; } +fn result_unwrap_or() { + // int case + Ok::<i32, &str>(1).unwrap_or(42); + + // int case, scrutinee is a binding + let a = Ok::<i32, &str>(1); + a.unwrap_or(42); + + // int case, suggestion must surround Result expr with parenthesis + (Ok(1) as Result<i32, &str>).unwrap_or(42); + + // method call case, suggestion must not surround Result expr `s.method()` with parenthesis + struct S {} + impl S { + fn method(self) -> Option<i32> { + Some(42) + } + } + let s = S {}; + s.method().unwrap_or(42); + + // int case reversed + Ok::<i32, &str>(1).unwrap_or(42); + + // richer none expr + Ok::<i32, &str>(1).unwrap_or(1 + 42); + + // multiline case + #[rustfmt::skip] + Ok::<i32, &str>(1).unwrap_or({ + 42 + 42 + + 42 + 42 + 42 + + 42 + 42 + 42 + }); + + // string case + Ok::<&str, &str>("Bob").unwrap_or("Alice"); + + // don't lint + match Ok::<i32, &str>(1) { + Ok(i) => i + 2, + Err(_) => 42, + }; + match Ok::<i32, &str>(1) { + Ok(i) => i, + Err(_) => return, + }; + for j in 0..4 { + match Ok::<i32, &str>(j) { + Ok(i) => i, + Err(_) => continue, + }; + match Ok::<i32, &str>(j) { + Ok(i) => i, + Err(_) => break, + }; + } + + // don't lint, Err value is used + match Ok::<&str, &str>("Alice") { + Ok(s) => s, + Err(s) => s, + }; + // could lint, but unused_variables takes care of it + match Ok::<&str, &str>("Alice") { + Ok(s) => s, + Err(s) => "Bob", + }; +} + fn main() {} diff --git a/tests/ui/manual_unwrap_or.rs b/tests/ui/manual_unwrap_or.rs index bede8cffc32..df534031f54 100644 --- a/tests/ui/manual_unwrap_or.rs +++ b/tests/ui/manual_unwrap_or.rs @@ -1,7 +1,8 @@ // run-rustfix #![allow(dead_code)] +#![allow(unused_variables)] -fn unwrap_or() { +fn option_unwrap_or() { // int case match Some(1) { Some(i) => i, @@ -80,4 +81,98 @@ fn unwrap_or() { }; } +fn result_unwrap_or() { + // int case + match Ok::<i32, &str>(1) { + Ok(i) => i, + Err(_) => 42, + }; + + // int case, scrutinee is a binding + let a = Ok::<i32, &str>(1); + match a { + Ok(i) => i, + Err(_) => 42, + }; + + // int case, suggestion must surround Result expr with parenthesis + match Ok(1) as Result<i32, &str> { + Ok(i) => i, + Err(_) => 42, + }; + + // method call case, suggestion must not surround Result expr `s.method()` with parenthesis + struct S {} + impl S { + fn method(self) -> Option<i32> { + Some(42) + } + } + let s = S {}; + match s.method() { + Some(i) => i, + None => 42, + }; + + // int case reversed + match Ok::<i32, &str>(1) { + Err(_) => 42, + Ok(i) => i, + }; + + // richer none expr + match Ok::<i32, &str>(1) { + Ok(i) => i, + Err(_) => 1 + 42, + }; + + // multiline case + #[rustfmt::skip] + match Ok::<i32, &str>(1) { + Ok(i) => i, + Err(_) => { + 42 + 42 + + 42 + 42 + 42 + + 42 + 42 + 42 + } + }; + + // string case + match Ok::<&str, &str>("Bob") { + Ok(i) => i, + Err(_) => "Alice", + }; + + // don't lint + match Ok::<i32, &str>(1) { + Ok(i) => i + 2, + Err(_) => 42, + }; + match Ok::<i32, &str>(1) { + Ok(i) => i, + Err(_) => return, + }; + for j in 0..4 { + match Ok::<i32, &str>(j) { + Ok(i) => i, + Err(_) => continue, + }; + match Ok::<i32, &str>(j) { + Ok(i) => i, + Err(_) => break, + }; + } + + // don't lint, Err value is used + match Ok::<&str, &str>("Alice") { + Ok(s) => s, + Err(s) => s, + }; + // could lint, but unused_variables takes care of it + match Ok::<&str, &str>("Alice") { + Ok(s) => s, + Err(s) => "Bob", + }; +} + fn main() {} diff --git a/tests/ui/manual_unwrap_or.stderr b/tests/ui/manual_unwrap_or.stderr index 674f2952635..fc174c4c270 100644 --- a/tests/ui/manual_unwrap_or.stderr +++ b/tests/ui/manual_unwrap_or.stderr @@ -1,5 +1,5 @@ error: this pattern reimplements `Option::unwrap_or` - --> $DIR/manual_unwrap_or.rs:6:5 + --> $DIR/manual_unwrap_or.rs:7:5 | LL | / match Some(1) { LL | | Some(i) => i, @@ -10,7 +10,7 @@ LL | | }; = note: `-D clippy::manual-unwrap-or` implied by `-D warnings` error: this pattern reimplements `Option::unwrap_or` - --> $DIR/manual_unwrap_or.rs:12:5 + --> $DIR/manual_unwrap_or.rs:13:5 | LL | / match Some(1) { LL | | None => 42, @@ -19,7 +19,7 @@ LL | | }; | |_____^ help: replace with: `Some(1).unwrap_or(42)` error: this pattern reimplements `Option::unwrap_or` - --> $DIR/manual_unwrap_or.rs:18:5 + --> $DIR/manual_unwrap_or.rs:19:5 | LL | / match Some(1) { LL | | Some(i) => i, @@ -28,7 +28,7 @@ LL | | }; | |_____^ help: replace with: `Some(1).unwrap_or(1 + 42)` error: this pattern reimplements `Option::unwrap_or` - --> $DIR/manual_unwrap_or.rs:25:5 + --> $DIR/manual_unwrap_or.rs:26:5 | LL | / match Some(1) { LL | | Some(i) => i, @@ -49,7 +49,7 @@ LL | }); | error: this pattern reimplements `Option::unwrap_or` - --> $DIR/manual_unwrap_or.rs:35:5 + --> $DIR/manual_unwrap_or.rs:36:5 | LL | / match Some("Bob") { LL | | Some(i) => i, @@ -57,5 +57,89 @@ LL | | None => "Alice", LL | | }; | |_____^ help: replace with: `Some("Bob").unwrap_or("Alice")` -error: aborting due to 5 previous errors +error: this pattern reimplements `Result::unwrap_or` + --> $DIR/manual_unwrap_or.rs:86:5 + | +LL | / match Ok::<i32, &str>(1) { +LL | | Ok(i) => i, +LL | | Err(_) => 42, +LL | | }; + | |_____^ help: replace with: `Ok::<i32, &str>(1).unwrap_or(42)` + +error: this pattern reimplements `Result::unwrap_or` + --> $DIR/manual_unwrap_or.rs:93:5 + | +LL | / match a { +LL | | Ok(i) => i, +LL | | Err(_) => 42, +LL | | }; + | |_____^ help: replace with: `a.unwrap_or(42)` + +error: this pattern reimplements `Result::unwrap_or` + --> $DIR/manual_unwrap_or.rs:99:5 + | +LL | / match Ok(1) as Result<i32, &str> { +LL | | Ok(i) => i, +LL | | Err(_) => 42, +LL | | }; + | |_____^ help: replace with: `(Ok(1) as Result<i32, &str>).unwrap_or(42)` + +error: this pattern reimplements `Option::unwrap_or` + --> $DIR/manual_unwrap_or.rs:112:5 + | +LL | / match s.method() { +LL | | Some(i) => i, +LL | | None => 42, +LL | | }; + | |_____^ help: replace with: `s.method().unwrap_or(42)` + +error: this pattern reimplements `Result::unwrap_or` + --> $DIR/manual_unwrap_or.rs:118:5 + | +LL | / match Ok::<i32, &str>(1) { +LL | | Err(_) => 42, +LL | | Ok(i) => i, +LL | | }; + | |_____^ help: replace with: `Ok::<i32, &str>(1).unwrap_or(42)` + +error: this pattern reimplements `Result::unwrap_or` + --> $DIR/manual_unwrap_or.rs:124:5 + | +LL | / match Ok::<i32, &str>(1) { +LL | | Ok(i) => i, +LL | | Err(_) => 1 + 42, +LL | | }; + | |_____^ help: replace with: `Ok::<i32, &str>(1).unwrap_or(1 + 42)` + +error: this pattern reimplements `Result::unwrap_or` + --> $DIR/manual_unwrap_or.rs:131:5 + | +LL | / match Ok::<i32, &str>(1) { +LL | | Ok(i) => i, +LL | | Err(_) => { +LL | | 42 + 42 +... | +LL | | } +LL | | }; + | |_____^ + | +help: replace with + | +LL | Ok::<i32, &str>(1).unwrap_or({ +LL | 42 + 42 +LL | + 42 + 42 + 42 +LL | + 42 + 42 + 42 +LL | }); + | + +error: this pattern reimplements `Result::unwrap_or` + --> $DIR/manual_unwrap_or.rs:141:5 + | +LL | / match Ok::<&str, &str>("Bob") { +LL | | Ok(i) => i, +LL | | Err(_) => "Alice", +LL | | }; + | |_____^ help: replace with: `Ok::<&str, &str>("Bob").unwrap_or("Alice")` + +error: aborting due to 13 previous errors diff --git a/tests/ui/mut_mutex_lock.fixed b/tests/ui/mut_mutex_lock.fixed new file mode 100644 index 00000000000..36bc52e3374 --- /dev/null +++ b/tests/ui/mut_mutex_lock.fixed @@ -0,0 +1,21 @@ +// run-rustfix +#![allow(dead_code, unused_mut)] +#![warn(clippy::mut_mutex_lock)] + +use std::sync::{Arc, Mutex}; + +fn mut_mutex_lock() { + let mut value_rc = Arc::new(Mutex::new(42_u8)); + let value_mutex = Arc::get_mut(&mut value_rc).unwrap(); + + let mut value = value_mutex.get_mut().unwrap(); + *value += 1; +} + +fn no_owned_mutex_lock() { + let mut value_rc = Arc::new(Mutex::new(42_u8)); + let mut value = value_rc.lock().unwrap(); + *value += 1; +} + +fn main() {} diff --git a/tests/ui/mut_mutex_lock.rs b/tests/ui/mut_mutex_lock.rs new file mode 100644 index 00000000000..ea60df5ae1b --- /dev/null +++ b/tests/ui/mut_mutex_lock.rs @@ -0,0 +1,21 @@ +// run-rustfix +#![allow(dead_code, unused_mut)] +#![warn(clippy::mut_mutex_lock)] + +use std::sync::{Arc, Mutex}; + +fn mut_mutex_lock() { + let mut value_rc = Arc::new(Mutex::new(42_u8)); + let value_mutex = Arc::get_mut(&mut value_rc).unwrap(); + + let mut value = value_mutex.lock().unwrap(); + *value += 1; +} + +fn no_owned_mutex_lock() { + let mut value_rc = Arc::new(Mutex::new(42_u8)); + let mut value = value_rc.lock().unwrap(); + *value += 1; +} + +fn main() {} diff --git a/tests/ui/mut_mutex_lock.stderr b/tests/ui/mut_mutex_lock.stderr new file mode 100644 index 00000000000..21c1b3486ca --- /dev/null +++ b/tests/ui/mut_mutex_lock.stderr @@ -0,0 +1,10 @@ +error: calling `&mut Mutex::lock` unnecessarily locks an exclusive (mutable) reference + --> $DIR/mut_mutex_lock.rs:11:33 + | +LL | let mut value = value_mutex.lock().unwrap(); + | ^^^^ help: change this to: `get_mut` + | + = note: `-D clippy::mut-mutex-lock` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/tests/ui/needless_lifetimes.rs b/tests/ui/needless_lifetimes.rs index d482d466e44..6001ef37eb7 100644 --- a/tests/ui/needless_lifetimes.rs +++ b/tests/ui/needless_lifetimes.rs @@ -357,4 +357,15 @@ mod nested_elision_sites { } } +mod issue6159 { + use std::ops::Deref; + pub fn apply_deref<'a, T, F, R>(x: &'a T, f: F) -> R + where + T: Deref, + F: FnOnce(&'a T::Target) -> R, + { + f(x.deref()) + } +} + fn main() {} diff --git a/tests/ui/range_contains.fixed b/tests/ui/range_contains.fixed new file mode 100644 index 00000000000..632a6592a28 --- /dev/null +++ b/tests/ui/range_contains.fixed @@ -0,0 +1,41 @@ +// run-rustfix + +#[warn(clippy::manual_range_contains)] +#[allow(unused)] +#[allow(clippy::no_effect)] +#[allow(clippy::short_circuit_statement)] +#[allow(clippy::unnecessary_operation)] +fn main() { + let x = 9_u32; + + // order shouldn't matter + (8..12).contains(&x); + (21..42).contains(&x); + (1..100).contains(&x); + + // also with inclusive ranges + (9..=99).contains(&x); + (1..=33).contains(&x); + (1..=999).contains(&x); + + // and the outside + !(8..12).contains(&x); + !(21..42).contains(&x); + !(1..100).contains(&x); + + // also with the outside of inclusive ranges + !(9..=99).contains(&x); + !(1..=33).contains(&x); + !(1..=999).contains(&x); + + // not a range.contains + x > 8 && x < 12; // lower bound not inclusive + x < 8 && x <= 12; // same direction + x >= 12 && 12 >= x; // same bounds + x < 8 && x > 12; // wrong direction + + x <= 8 || x >= 12; + x >= 8 || x >= 12; + x < 12 || 12 < x; + x >= 8 || x <= 12; +} diff --git a/tests/ui/range_contains.rs b/tests/ui/range_contains.rs new file mode 100644 index 00000000000..6af0d034ef6 --- /dev/null +++ b/tests/ui/range_contains.rs @@ -0,0 +1,41 @@ +// run-rustfix + +#[warn(clippy::manual_range_contains)] +#[allow(unused)] +#[allow(clippy::no_effect)] +#[allow(clippy::short_circuit_statement)] +#[allow(clippy::unnecessary_operation)] +fn main() { + let x = 9_u32; + + // order shouldn't matter + x >= 8 && x < 12; + x < 42 && x >= 21; + 100 > x && 1 <= x; + + // also with inclusive ranges + x >= 9 && x <= 99; + x <= 33 && x >= 1; + 999 >= x && 1 <= x; + + // and the outside + x < 8 || x >= 12; + x >= 42 || x < 21; + 100 <= x || 1 > x; + + // also with the outside of inclusive ranges + x < 9 || x > 99; + x > 33 || x < 1; + 999 < x || 1 > x; + + // not a range.contains + x > 8 && x < 12; // lower bound not inclusive + x < 8 && x <= 12; // same direction + x >= 12 && 12 >= x; // same bounds + x < 8 && x > 12; // wrong direction + + x <= 8 || x >= 12; + x >= 8 || x >= 12; + x < 12 || 12 < x; + x >= 8 || x <= 12; +} diff --git a/tests/ui/range_contains.stderr b/tests/ui/range_contains.stderr new file mode 100644 index 00000000000..69b009eafc3 --- /dev/null +++ b/tests/ui/range_contains.stderr @@ -0,0 +1,76 @@ +error: manual `Range::contains` implementation + --> $DIR/range_contains.rs:12:5 + | +LL | x >= 8 && x < 12; + | ^^^^^^^^^^^^^^^^ help: use: `(8..12).contains(&x)` + | + = note: `-D clippy::manual-range-contains` implied by `-D warnings` + +error: manual `Range::contains` implementation + --> $DIR/range_contains.rs:13:5 + | +LL | x < 42 && x >= 21; + | ^^^^^^^^^^^^^^^^^ help: use: `(21..42).contains(&x)` + +error: manual `Range::contains` implementation + --> $DIR/range_contains.rs:14:5 + | +LL | 100 > x && 1 <= x; + | ^^^^^^^^^^^^^^^^^ help: use: `(1..100).contains(&x)` + +error: manual `RangeInclusive::contains` implementation + --> $DIR/range_contains.rs:17:5 + | +LL | x >= 9 && x <= 99; + | ^^^^^^^^^^^^^^^^^ help: use: `(9..=99).contains(&x)` + +error: manual `RangeInclusive::contains` implementation + --> $DIR/range_contains.rs:18:5 + | +LL | x <= 33 && x >= 1; + | ^^^^^^^^^^^^^^^^^ help: use: `(1..=33).contains(&x)` + +error: manual `RangeInclusive::contains` implementation + --> $DIR/range_contains.rs:19:5 + | +LL | 999 >= x && 1 <= x; + | ^^^^^^^^^^^^^^^^^^ help: use: `(1..=999).contains(&x)` + +error: manual `!Range::contains` implementation + --> $DIR/range_contains.rs:22:5 + | +LL | x < 8 || x >= 12; + | ^^^^^^^^^^^^^^^^ help: use: `!(8..12).contains(&x)` + +error: manual `!Range::contains` implementation + --> $DIR/range_contains.rs:23:5 + | +LL | x >= 42 || x < 21; + | ^^^^^^^^^^^^^^^^^ help: use: `!(21..42).contains(&x)` + +error: manual `!Range::contains` implementation + --> $DIR/range_contains.rs:24:5 + | +LL | 100 <= x || 1 > x; + | ^^^^^^^^^^^^^^^^^ help: use: `!(1..100).contains(&x)` + +error: manual `!RangeInclusive::contains` implementation + --> $DIR/range_contains.rs:27:5 + | +LL | x < 9 || x > 99; + | ^^^^^^^^^^^^^^^ help: use: `!(9..=99).contains(&x)` + +error: manual `!RangeInclusive::contains` implementation + --> $DIR/range_contains.rs:28:5 + | +LL | x > 33 || x < 1; + | ^^^^^^^^^^^^^^^ help: use: `!(1..=33).contains(&x)` + +error: manual `!RangeInclusive::contains` implementation + --> $DIR/range_contains.rs:29:5 + | +LL | 999 < x || 1 > x; + | ^^^^^^^^^^^^^^^^ help: use: `!(1..=999).contains(&x)` + +error: aborting due to 12 previous errors + diff --git a/tests/ui/single_element_loop.fixed b/tests/ui/single_element_loop.fixed new file mode 100644 index 00000000000..8ca068293a6 --- /dev/null +++ b/tests/ui/single_element_loop.fixed @@ -0,0 +1,11 @@ +// run-rustfix +// Tests from for_loop.rs that don't have suggestions + +#[warn(clippy::single_element_loop)] +fn main() { + let item1 = 2; + { + let item = &item1; + println!("{}", item); + } +} diff --git a/tests/ui/single_element_loop.rs b/tests/ui/single_element_loop.rs new file mode 100644 index 00000000000..57e9336a31f --- /dev/null +++ b/tests/ui/single_element_loop.rs @@ -0,0 +1,10 @@ +// run-rustfix +// Tests from for_loop.rs that don't have suggestions + +#[warn(clippy::single_element_loop)] +fn main() { + let item1 = 2; + for item in &[item1] { + println!("{}", item); + } +} diff --git a/tests/ui/single_element_loop.stderr b/tests/ui/single_element_loop.stderr new file mode 100644 index 00000000000..90be1dc3283 --- /dev/null +++ b/tests/ui/single_element_loop.stderr @@ -0,0 +1,19 @@ +error: for loop over a single element + --> $DIR/single_element_loop.rs:7:5 + | +LL | / for item in &[item1] { +LL | | println!("{}", item); +LL | | } + | |_____^ + | + = note: `-D clippy::single-element-loop` implied by `-D warnings` +help: try + | +LL | { +LL | let item = &item1; +LL | println!("{}", item); +LL | } + | + +error: aborting due to previous error + diff --git a/tests/ui/temporary_assignment.rs b/tests/ui/temporary_assignment.rs index b4a931043b0..ac4c1bc6597 100644 --- a/tests/ui/temporary_assignment.rs +++ b/tests/ui/temporary_assignment.rs @@ -1,5 +1,4 @@ #![warn(clippy::temporary_assignment)] -#![allow(const_item_mutation)] use std::ops::{Deref, DerefMut}; diff --git a/tests/ui/temporary_assignment.stderr b/tests/ui/temporary_assignment.stderr index 4cc32c79f05..7d79901a28d 100644 --- a/tests/ui/temporary_assignment.stderr +++ b/tests/ui/temporary_assignment.stderr @@ -1,5 +1,5 @@ error: assignment to temporary - --> $DIR/temporary_assignment.rs:48:5 + --> $DIR/temporary_assignment.rs:47:5 | LL | Struct { field: 0 }.field = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -7,7 +7,7 @@ LL | Struct { field: 0 }.field = 1; = note: `-D clippy::temporary-assignment` implied by `-D warnings` error: assignment to temporary - --> $DIR/temporary_assignment.rs:49:5 + --> $DIR/temporary_assignment.rs:48:5 | LL | / MultiStruct { LL | | structure: Struct { field: 0 }, @@ -17,13 +17,13 @@ LL | | .field = 1; | |______________^ error: assignment to temporary - --> $DIR/temporary_assignment.rs:54:5 + --> $DIR/temporary_assignment.rs:53:5 | LL | ArrayStruct { array: [0] }.array[0] = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: assignment to temporary - --> $DIR/temporary_assignment.rs:55:5 + --> $DIR/temporary_assignment.rs:54:5 | LL | (0, 0).0 = 1; | ^^^^^^^^^^^^ diff --git a/tests/ui/undropped_manually_drops.rs b/tests/ui/undropped_manually_drops.rs new file mode 100644 index 00000000000..f4cfc92e1cd --- /dev/null +++ b/tests/ui/undropped_manually_drops.rs @@ -0,0 +1,26 @@ +#![warn(clippy::undropped_manually_drops)] + +struct S; + +fn main() { + let f = std::mem::drop; + let g = std::mem::ManuallyDrop::drop; + let mut manual1 = std::mem::ManuallyDrop::new(S); + let mut manual2 = std::mem::ManuallyDrop::new(S); + let mut manual3 = std::mem::ManuallyDrop::new(S); + let mut manual4 = std::mem::ManuallyDrop::new(S); + + // These lines will not drop `S` and should be linted + drop(std::mem::ManuallyDrop::new(S)); + drop(manual1); + + // FIXME: this line is not linted, though it should be + f(manual2); + + // These lines will drop `S` and should be okay. + unsafe { + std::mem::ManuallyDrop::drop(&mut std::mem::ManuallyDrop::new(S)); + std::mem::ManuallyDrop::drop(&mut manual3); + g(&mut manual4); + } +} diff --git a/tests/ui/undropped_manually_drops.stderr b/tests/ui/undropped_manually_drops.stderr new file mode 100644 index 00000000000..2ac0fe98697 --- /dev/null +++ b/tests/ui/undropped_manually_drops.stderr @@ -0,0 +1,19 @@ +error: the inner value of this ManuallyDrop will not be dropped + --> $DIR/undropped_manually_drops.rs:14:5 + | +LL | drop(std::mem::ManuallyDrop::new(S)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::undropped-manually-drops` implied by `-D warnings` + = help: to drop a `ManuallyDrop<T>`, use std::mem::ManuallyDrop::drop + +error: the inner value of this ManuallyDrop will not be dropped + --> $DIR/undropped_manually_drops.rs:15:5 + | +LL | drop(manual1); + | ^^^^^^^^^^^^^ + | + = help: to drop a `ManuallyDrop<T>`, use std::mem::ManuallyDrop::drop + +error: aborting due to 2 previous errors + diff --git a/tests/ui/unnecessary_cast_fixable.fixed b/tests/ui/unnecessary_cast_fixable.fixed index fb89a9fce3d..350da4965d1 100644 --- a/tests/ui/unnecessary_cast_fixable.fixed +++ b/tests/ui/unnecessary_cast_fixable.fixed @@ -8,16 +8,31 @@ fn main() { 100_f32; 100_f64; 100_f64; + let _ = -100_f32; + let _ = -100_f64; + let _ = -100_f64; // Should not trigger #[rustfmt::skip] let v = vec!(1); &v as &[i32]; - 1.0 as f64; - 1 as u64; 0x10 as f32; 0o10 as f32; 0b10 as f32; 0x11 as f64; 0o11 as f64; 0b11 as f64; + + 1_u32; + 0x10_i32; + 0b10_usize; + 0o73_u16; + 1_000_000_000_u32; + + 1.0_f64; + 0.5_f32; + + 1.0 as u16; + + let _ = -1_i32; + let _ = -1.0_f32; } diff --git a/tests/ui/unnecessary_cast_fixable.rs b/tests/ui/unnecessary_cast_fixable.rs index 4a0c8620dc1..ad2fb2e6289 100644 --- a/tests/ui/unnecessary_cast_fixable.rs +++ b/tests/ui/unnecessary_cast_fixable.rs @@ -8,16 +8,31 @@ fn main() { 100 as f32; 100 as f64; 100_i32 as f64; + let _ = -100 as f32; + let _ = -100 as f64; + let _ = -100_i32 as f64; // Should not trigger #[rustfmt::skip] let v = vec!(1); &v as &[i32]; - 1.0 as f64; - 1 as u64; 0x10 as f32; 0o10 as f32; 0b10 as f32; 0x11 as f64; 0o11 as f64; 0b11 as f64; + + 1 as u32; + 0x10 as i32; + 0b10 as usize; + 0o73 as u16; + 1_000_000_000 as u32; + + 1.0 as f64; + 0.5 as f32; + + 1.0 as u16; + + let _ = -1 as i32; + let _ = -1.0 as f32; } diff --git a/tests/ui/unnecessary_cast_fixable.stderr b/tests/ui/unnecessary_cast_fixable.stderr index 8ff1e5dea60..5a210fc8909 100644 --- a/tests/ui/unnecessary_cast_fixable.stderr +++ b/tests/ui/unnecessary_cast_fixable.stderr @@ -18,5 +18,77 @@ error: casting integer literal to `f64` is unnecessary LL | 100_i32 as f64; | ^^^^^^^^^^^^^^ help: try: `100_f64` -error: aborting due to 3 previous errors +error: casting integer literal to `f32` is unnecessary + --> $DIR/unnecessary_cast_fixable.rs:11:13 + | +LL | let _ = -100 as f32; + | ^^^^^^^^^^^ help: try: `-100_f32` + +error: casting integer literal to `f64` is unnecessary + --> $DIR/unnecessary_cast_fixable.rs:12:13 + | +LL | let _ = -100 as f64; + | ^^^^^^^^^^^ help: try: `-100_f64` + +error: casting integer literal to `f64` is unnecessary + --> $DIR/unnecessary_cast_fixable.rs:13:13 + | +LL | let _ = -100_i32 as f64; + | ^^^^^^^^^^^^^^^ help: try: `-100_f64` + +error: casting integer literal to `u32` is unnecessary + --> $DIR/unnecessary_cast_fixable.rs:25:5 + | +LL | 1 as u32; + | ^^^^^^^^ help: try: `1_u32` + +error: casting integer literal to `i32` is unnecessary + --> $DIR/unnecessary_cast_fixable.rs:26:5 + | +LL | 0x10 as i32; + | ^^^^^^^^^^^ help: try: `0x10_i32` + +error: casting integer literal to `usize` is unnecessary + --> $DIR/unnecessary_cast_fixable.rs:27:5 + | +LL | 0b10 as usize; + | ^^^^^^^^^^^^^ help: try: `0b10_usize` + +error: casting integer literal to `u16` is unnecessary + --> $DIR/unnecessary_cast_fixable.rs:28:5 + | +LL | 0o73 as u16; + | ^^^^^^^^^^^ help: try: `0o73_u16` + +error: casting integer literal to `u32` is unnecessary + --> $DIR/unnecessary_cast_fixable.rs:29:5 + | +LL | 1_000_000_000 as u32; + | ^^^^^^^^^^^^^^^^^^^^ help: try: `1_000_000_000_u32` + +error: casting float literal to `f64` is unnecessary + --> $DIR/unnecessary_cast_fixable.rs:31:5 + | +LL | 1.0 as f64; + | ^^^^^^^^^^ help: try: `1.0_f64` + +error: casting float literal to `f32` is unnecessary + --> $DIR/unnecessary_cast_fixable.rs:32:5 + | +LL | 0.5 as f32; + | ^^^^^^^^^^ help: try: `0.5_f32` + +error: casting integer literal to `i32` is unnecessary + --> $DIR/unnecessary_cast_fixable.rs:36:13 + | +LL | let _ = -1 as i32; + | ^^^^^^^^^ help: try: `-1_i32` + +error: casting float literal to `f32` is unnecessary + --> $DIR/unnecessary_cast_fixable.rs:37:13 + | +LL | let _ = -1.0 as f32; + | ^^^^^^^^^^^ help: try: `-1.0_f32` + +error: aborting due to 15 previous errors diff --git a/util/dev b/util/dev deleted file mode 100755 index 319de217e0d..00000000000 --- a/util/dev +++ /dev/null @@ -1,7 +0,0 @@ -#!/bin/sh -CARGO_TARGET_DIR=$(pwd)/target/ -export CARGO_TARGET_DIR - -echo 'Deprecated! `util/dev` usage is deprecated, please use `cargo dev` instead.' - -cd clippy_dev && cargo run -- "$@" |