diff --git a/CHANGELOG.md b/CHANGELOG.md index 6357511cc..71671273c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4844,6 +4844,7 @@ Released 2018-09-13 [`field_reassign_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#field_reassign_with_default [`filetype_is_file`]: https://rust-lang.github.io/rust-clippy/master/index.html#filetype_is_file [`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map +[`filter_map_bool_then`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_bool_then [`filter_map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_identity [`filter_map_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next [`filter_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_next @@ -4889,12 +4890,14 @@ Released 2018-09-13 [`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else [`if_then_some_else_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none [`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond +[`ignored_unit_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#ignored_unit_patterns [`impl_trait_in_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#impl_trait_in_params [`implicit_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_clone [`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher [`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return [`implicit_saturating_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_saturating_add [`implicit_saturating_sub`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_saturating_sub +[`impossible_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#impossible_comparisons [`imprecise_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#imprecise_flops [`inconsistent_digit_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping [`inconsistent_struct_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_struct_constructor @@ -5189,6 +5192,7 @@ Released 2018-09-13 [`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure [`redundant_closure_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_call [`redundant_closure_for_method_calls`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_for_method_calls +[`redundant_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_comparisons [`redundant_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_else [`redundant_feature_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_feature_names [`redundant_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index 43eaccdf5..fca750faf 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -41,7 +41,7 @@ fn main() { matches.get_one::("type").map(String::as_str), matches.get_flag("msrv"), ) { - Ok(_) => update_lints::update(update_lints::UpdateMode::Change), + Ok(()) => update_lints::update(update_lints::UpdateMode::Change), Err(e) => eprintln!("Unable to create lint: {e}"), } }, diff --git a/clippy_dev/src/setup/vscode.rs b/clippy_dev/src/setup/vscode.rs index dbcdc9b59..204f4af2c 100644 --- a/clippy_dev/src/setup/vscode.rs +++ b/clippy_dev/src/setup/vscode.rs @@ -47,7 +47,7 @@ fn check_install_precondition(force_override: bool) -> bool { } } else { match fs::create_dir(vs_dir_path) { - Ok(_) => { + Ok(()) => { println!("info: created `{VSCODE_DIR}` directory for clippy"); }, Err(err) => { diff --git a/clippy_lints/src/casts/ptr_as_ptr.rs b/clippy_lints/src/casts/ptr_as_ptr.rs index 15ffb00da..181dbcf6e 100644 --- a/clippy_lints/src/casts/ptr_as_ptr.rs +++ b/clippy_lints/src/casts/ptr_as_ptr.rs @@ -1,9 +1,7 @@ -use std::borrow::Cow; - use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::msrvs::{self, Msrv}; +use clippy_utils::source::snippet_with_applicability; use clippy_utils::sugg::Sugg; -use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, Mutability, TyKind}; use rustc_lint::LateContext; @@ -16,33 +14,41 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, msrv: &Msrv) { return; } - if_chain! { - if let ExprKind::Cast(cast_expr, cast_to_hir_ty) = expr.kind; - let (cast_from, cast_to) = (cx.typeck_results().expr_ty(cast_expr), cx.typeck_results().expr_ty(expr)); - if let ty::RawPtr(TypeAndMut { mutbl: from_mutbl, .. }) = cast_from.kind(); - if let ty::RawPtr(TypeAndMut { ty: to_pointee_ty, mutbl: to_mutbl }) = cast_to.kind(); - if matches!((from_mutbl, to_mutbl), - (Mutability::Not, Mutability::Not) | (Mutability::Mut, Mutability::Mut)); + if let ExprKind::Cast(cast_expr, cast_to_hir_ty) = expr.kind + && let (cast_from, cast_to) = (cx.typeck_results().expr_ty(cast_expr), cx.typeck_results().expr_ty(expr)) + && let ty::RawPtr(TypeAndMut { mutbl: from_mutbl, .. }) = cast_from.kind() + && let ty::RawPtr(TypeAndMut { ty: to_pointee_ty, mutbl: to_mutbl }) = cast_to.kind() + && matches!((from_mutbl, to_mutbl), + (Mutability::Not, Mutability::Not) | (Mutability::Mut, Mutability::Mut)) // The `U` in `pointer::cast` have to be `Sized` // as explained here: https://github.com/rust-lang/rust/issues/60602. - if to_pointee_ty.is_sized(cx.tcx, cx.param_env); - then { - let mut applicability = Applicability::MachineApplicable; - let cast_expr_sugg = Sugg::hir_with_applicability(cx, cast_expr, "_", &mut applicability); - let turbofish = match &cast_to_hir_ty.kind { - TyKind::Infer => Cow::Borrowed(""), - TyKind::Ptr(mut_ty) if matches!(mut_ty.ty.kind, TyKind::Infer) => Cow::Borrowed(""), - _ => Cow::Owned(format!("::<{to_pointee_ty}>")), - }; - span_lint_and_sugg( - cx, - PTR_AS_PTR, - expr.span, - "`as` casting between raw pointers without changing its mutability", - "try `pointer::cast`, a safer alternative", - format!("{}.cast{turbofish}()", cast_expr_sugg.maybe_par()), - applicability, - ); - } + && to_pointee_ty.is_sized(cx.tcx, cx.param_env) + { + let mut app = Applicability::MachineApplicable; + let cast_expr_sugg = Sugg::hir_with_applicability(cx, cast_expr, "_", &mut app); + let turbofish = match &cast_to_hir_ty.kind { + TyKind::Infer => String::new(), + TyKind::Ptr(mut_ty) => { + if matches!(mut_ty.ty.kind, TyKind::Infer) { + String::new() + } else { + format!( + "::<{}>", + snippet_with_applicability(cx, mut_ty.ty.span, "/* type */", &mut app) + ) + } + }, + _ => return, + }; + + span_lint_and_sugg( + cx, + PTR_AS_PTR, + expr.span, + "`as` casting between raw pointers without changing its mutability", + "try `pointer::cast`, a safer alternative", + format!("{}.cast{turbofish}()", cast_expr_sugg.maybe_par()), + app, + ); } } diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 9a9998cca..db114abfc 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -203,6 +203,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::if_let_mutex::IF_LET_MUTEX_INFO, crate::if_not_else::IF_NOT_ELSE_INFO, crate::if_then_some_else_none::IF_THEN_SOME_ELSE_NONE_INFO, + crate::ignored_unit_patterns::IGNORED_UNIT_PATTERNS_INFO, crate::implicit_hasher::IMPLICIT_HASHER_INFO, crate::implicit_return::IMPLICIT_RETURN_INFO, crate::implicit_saturating_add::IMPLICIT_SATURATING_ADD_INFO, @@ -337,6 +338,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::EXPECT_USED_INFO, crate::methods::EXTEND_WITH_DRAIN_INFO, crate::methods::FILETYPE_IS_FILE_INFO, + crate::methods::FILTER_MAP_BOOL_THEN_INFO, crate::methods::FILTER_MAP_IDENTITY_INFO, crate::methods::FILTER_MAP_NEXT_INFO, crate::methods::FILTER_NEXT_INFO, @@ -516,6 +518,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::operators::FLOAT_CMP_CONST_INFO, crate::operators::FLOAT_EQUALITY_WITHOUT_ABS_INFO, crate::operators::IDENTITY_OP_INFO, + crate::operators::IMPOSSIBLE_COMPARISONS_INFO, crate::operators::INEFFECTIVE_BIT_MASK_INFO, crate::operators::INTEGER_DIVISION_INFO, crate::operators::MISREFACTORED_ASSIGN_OP_INFO, @@ -524,6 +527,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::operators::NEEDLESS_BITWISE_BOOL_INFO, crate::operators::OP_REF_INFO, crate::operators::PTR_EQ_INFO, + crate::operators::REDUNDANT_COMPARISONS_INFO, crate::operators::SELF_ASSIGNMENT_INFO, crate::operators::VERBOSE_BIT_MASK_INFO, crate::option_env_unwrap::OPTION_ENV_UNWRAP_INFO, diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index 9900dbdee..d3311792c 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -1,6 +1,4 @@ -use clippy_utils::diagnostics::{ - span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then, -}; +use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::ty::{implements_trait, implements_trait_with_env, is_copy}; use clippy_utils::{is_lint_allowed, match_def_path, paths}; use if_chain::if_chain; @@ -8,15 +6,14 @@ use rustc_errors::Applicability; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{walk_expr, walk_fn, walk_item, FnKind, Visitor}; use rustc_hir::{ - self as hir, BlockCheckMode, BodyId, Expr, ExprKind, FnDecl, Impl, Item, ItemKind, - UnsafeSource, Unsafety, + self as hir, BlockCheckMode, BodyId, Expr, ExprKind, FnDecl, Impl, Item, ItemKind, UnsafeSource, Unsafety, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::nested_filter; use rustc_middle::traits::Reveal; use rustc_middle::ty::{ - self, ClauseKind, GenericArgKind, GenericParamDefKind, ImplPolarity, ParamEnv, - ToPredicate, TraitPredicate, Ty, TyCtxt, + self, ClauseKind, GenericArgKind, GenericParamDefKind, ImplPolarity, ParamEnv, ToPredicate, TraitPredicate, Ty, + TyCtxt, }; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::def_id::LocalDefId; @@ -207,10 +204,13 @@ declare_lint_pass!(Derive => [ impl<'tcx> LateLintPass<'tcx> for Derive { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { - if let ItemKind::Impl(Impl { of_trait: Some(ref trait_ref), .. }) = item.kind { + if let ItemKind::Impl(Impl { + of_trait: Some(ref trait_ref), + .. + }) = item.kind + { let ty = cx.tcx.type_of(item.owner_id).instantiate_identity(); - let is_automatically_derived = - cx.tcx.has_attr(item.owner_id, sym::automatically_derived); + let is_automatically_derived = cx.tcx.has_attr(item.owner_id, sym::automatically_derived); check_hash_peq(cx, item.span, trait_ref, ty, is_automatically_derived); check_ord_partial_ord(cx, item.span, trait_ref, ty, is_automatically_derived); @@ -327,12 +327,7 @@ fn check_ord_partial_ord<'tcx>( } /// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint. -fn check_copy_clone<'tcx>( - cx: &LateContext<'tcx>, - item: &Item<'_>, - trait_ref: &hir::TraitRef<'_>, - ty: Ty<'tcx>, -) { +fn check_copy_clone<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &hir::TraitRef<'_>, ty: Ty<'tcx>) { let clone_id = match cx.tcx.lang_items().clone_trait() { Some(id) if trait_ref.trait_def_id() == Some(id) => id, _ => return, @@ -350,9 +345,10 @@ fn check_copy_clone<'tcx>( if !is_copy(cx, ty) { if ty_subs.non_erasable_generics().next().is_some() { let has_copy_impl = cx.tcx.all_local_trait_impls(()).get(©_id).map_or(false, |impls| { - impls - .iter() - .any(|&id| matches!(cx.tcx.type_of(id).instantiate_identity().kind(), ty::Adt(adt, _) if ty_adt.did() == adt.did())) + impls.iter().any(|&id| { + matches!(cx.tcx.type_of(id).instantiate_identity().kind(), ty::Adt(adt, _) + if ty_adt.did() == adt.did()) + }) }); if !has_copy_impl { return; @@ -431,14 +427,7 @@ struct UnsafeVisitor<'a, 'tcx> { impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> { type NestedFilter = nested_filter::All; - fn visit_fn( - &mut self, - kind: FnKind<'tcx>, - decl: &'tcx FnDecl<'_>, - body_id: BodyId, - _: Span, - id: LocalDefId, - ) { + fn visit_fn(&mut self, kind: FnKind<'tcx>, decl: &'tcx FnDecl<'_>, body_id: BodyId, _: Span, id: LocalDefId) { if self.has_unsafe { return; } @@ -474,12 +463,7 @@ impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> { } /// Implementation of the `DERIVE_PARTIAL_EQ_WITHOUT_EQ` lint. -fn check_partial_eq_without_eq<'tcx>( - cx: &LateContext<'tcx>, - span: Span, - trait_ref: &hir::TraitRef<'_>, - ty: Ty<'tcx>, -) { +fn check_partial_eq_without_eq<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_ref: &hir::TraitRef<'_>, ty: Ty<'tcx>) { if_chain! { if let ty::Adt(adt, args) = ty.kind(); if cx.tcx.visibility(adt.did()).is_public(); diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index 2c4d93e33..e29ab634c 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -716,10 +716,7 @@ fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) { let sm = Lrc::new(SourceMap::new(FilePathMapping::empty())); let fallback_bundle = rustc_errors::fallback_fluent_bundle(rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), false); - let emitter = EmitterWriter::new( - Box::new(io::sink()), - fallback_bundle, - ); + let emitter = EmitterWriter::new(Box::new(io::sink()), fallback_bundle); let handler = Handler::with_emitter(Box::new(emitter)).disable_warnings(); let sess = ParseSess::with_span_handler(handler, sm); diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs index ba7957b0d..38066503c 100644 --- a/clippy_lints/src/eta_reduction.rs +++ b/clippy_lints/src/eta_reduction.rs @@ -10,8 +10,8 @@ use rustc_hir::{BindingAnnotation, Expr, ExprKind, FnRetTy, Param, PatKind, QPat use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{ - self, Binder, ClosureArgs, ClosureKind, EarlyBinder, FnSig, GenericArg, GenericArgKind, - GenericArgsRef, ImplPolarity, List, Region, RegionKind, Ty, TypeVisitableExt, TypeckResults, + self, Binder, ClosureArgs, ClosureKind, EarlyBinder, FnSig, GenericArg, GenericArgKind, GenericArgsRef, + ImplPolarity, List, Region, RegionKind, Ty, TypeVisitableExt, TypeckResults, }; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::symbol::sym; diff --git a/clippy_lints/src/ignored_unit_patterns.rs b/clippy_lints/src/ignored_unit_patterns.rs new file mode 100644 index 000000000..c635120b8 --- /dev/null +++ b/clippy_lints/src/ignored_unit_patterns.rs @@ -0,0 +1,52 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use hir::PatKind; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// Checks for usage of `_` in patterns of type `()`. + /// + /// ### Why is this bad? + /// Matching with `()` explicitly instead of `_` outlines + /// the fact that the pattern contains no data. Also it + /// would detect a type change that `_` would ignore. + /// + /// ### Example + /// ```rust + /// match std::fs::create_dir("tmp-work-dir") { + /// Ok(_) => println!("Working directory created"), + /// Err(s) => eprintln!("Could not create directory: {s}"), + /// } + /// ``` + /// Use instead: + /// ```rust + /// match std::fs::create_dir("tmp-work-dir") { + /// Ok(()) => println!("Working directory created"), + /// Err(s) => eprintln!("Could not create directory: {s}"), + /// } + /// ``` + #[clippy::version = "1.73.0"] + pub IGNORED_UNIT_PATTERNS, + pedantic, + "suggest replacing `_` by `()` in patterns where appropriate" +} +declare_lint_pass!(IgnoredUnitPatterns => [IGNORED_UNIT_PATTERNS]); + +impl<'tcx> LateLintPass<'tcx> for IgnoredUnitPatterns { + fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx hir::Pat<'tcx>) { + if matches!(pat.kind, PatKind::Wild) && cx.typeck_results().pat_ty(pat).is_unit() { + span_lint_and_sugg( + cx, + IGNORED_UNIT_PATTERNS, + pat.span, + "matching over `()` is more explicit", + "use `()` instead of `_`", + String::from("()"), + Applicability::MachineApplicable, + ); + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 9d6096ccb..358004cf4 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -147,6 +147,7 @@ mod future_not_send; mod if_let_mutex; mod if_not_else; mod if_then_some_else_none; +mod ignored_unit_patterns; mod implicit_hasher; mod implicit_return; mod implicit_saturating_add; @@ -1093,6 +1094,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: }) }); store.register_late_pass(|_| Box::new(redundant_locals::RedundantLocals)); + store.register_late_pass(|_| Box::new(ignored_unit_patterns::IgnoredUnitPatterns)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/loops/never_loop.rs b/clippy_lints/src/loops/never_loop.rs index dd77c69ef..cc19ac55e 100644 --- a/clippy_lints/src/loops/never_loop.rs +++ b/clippy_lints/src/loops/never_loop.rs @@ -162,7 +162,9 @@ fn never_loop_expr<'tcx>( ExprKind::Binary(_, e1, e2) | ExprKind::Assign(e1, e2, _) | ExprKind::AssignOp(_, e1, e2) - | ExprKind::Index(e1, e2, _) => never_loop_expr_all(cx, &mut [e1, e2].iter().copied(), ignore_ids, main_loop_id), + | ExprKind::Index(e1, e2, _) => { + never_loop_expr_all(cx, &mut [e1, e2].iter().copied(), ignore_ids, main_loop_id) + }, ExprKind::Loop(b, _, _, _) => { // Break can come from the inner loop so remove them. absorb_break(never_loop_block(cx, b, ignore_ids, main_loop_id)) diff --git a/clippy_lints/src/manual_float_methods.rs b/clippy_lints/src/manual_float_methods.rs index f48a5d9d2..88db7ae6a 100644 --- a/clippy_lints/src/manual_float_methods.rs +++ b/clippy_lints/src/manual_float_methods.rs @@ -84,7 +84,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualFloatMethods { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if !in_external_macro(cx.sess(), expr.span) && ( - matches!(cx.tcx.constness(cx.tcx.hir().enclosing_body_owner(expr.hir_id)), Constness::NotConst) + matches!(cx.tcx.constness(cx.tcx.hir().enclosing_body_owner(expr.hir_id)), Constness::NotConst) || cx.tcx.features().active(sym!(const_float_classify)) ) && let ExprKind::Binary(kind, lhs, rhs) = expr.kind && let ExprKind::Binary(lhs_kind, lhs_lhs, lhs_rhs) = lhs.kind diff --git a/clippy_lints/src/matches/redundant_guards.rs b/clippy_lints/src/matches/redundant_guards.rs index 6383326aa..29af48123 100644 --- a/clippy_lints/src/matches/redundant_guards.rs +++ b/clippy_lints/src/matches/redundant_guards.rs @@ -2,6 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::path_to_local; use clippy_utils::source::snippet_with_applicability; use clippy_utils::visitors::{for_each_expr, is_local_used}; +use rustc_ast::LitKind; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; use rustc_hir::{Arm, BinOpKind, Expr, ExprKind, Guard, MatchSource, Node, Pat, PatKind}; @@ -160,6 +161,11 @@ fn emit_redundant_guards<'tcx>( } /// Checks if the given `Expr` can also be represented as a `Pat`. +/// +/// All literals generally also work as patterns, however float literals are special. +/// They are currently (as of 2023/08/08) still allowed in patterns, but that will become +/// an error in the future, and rustc already actively warns against this (see rust#41620), +/// so we don't consider those as usable within patterns for linting purposes. fn expr_can_be_pat(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { for_each_expr(expr, |expr| { if match expr.kind { @@ -177,8 +183,8 @@ fn expr_can_be_pat(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { ExprKind::AddrOf(..) | ExprKind::Array(..) | ExprKind::Tup(..) - | ExprKind::Struct(..) - | ExprKind::Lit(..) => true, + | ExprKind::Struct(..) => true, + ExprKind::Lit(lit) if !matches!(lit.node, LitKind::Float(..)) => true, _ => false, } { return ControlFlow::Continue(()); diff --git a/clippy_lints/src/methods/expect_used.rs b/clippy_lints/src/methods/expect_used.rs deleted file mode 100644 index 614610335..000000000 --- a/clippy_lints/src/methods/expect_used.rs +++ /dev/null @@ -1,44 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{is_in_cfg_test, is_in_test_function}; -use rustc_hir as hir; -use rustc_lint::LateContext; -use rustc_span::sym; - -use super::EXPECT_USED; - -/// lint use of `expect()` or `expect_err` for `Result` and `expect()` for `Option`. -pub(super) fn check( - cx: &LateContext<'_>, - expr: &hir::Expr<'_>, - recv: &hir::Expr<'_>, - is_err: bool, - allow_expect_in_tests: bool, -) { - let obj_ty = cx.typeck_results().expr_ty(recv).peel_refs(); - - let mess = if is_type_diagnostic_item(cx, obj_ty, sym::Option) && !is_err { - Some((EXPECT_USED, "an `Option`", "None", "")) - } else if is_type_diagnostic_item(cx, obj_ty, sym::Result) { - Some((EXPECT_USED, "a `Result`", if is_err { "Ok" } else { "Err" }, "an ")) - } else { - None - }; - - let method = if is_err { "expect_err" } else { "expect" }; - - if allow_expect_in_tests && (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id)) { - return; - } - - if let Some((lint, kind, none_value, none_prefix)) = mess { - span_lint_and_help( - cx, - lint, - expr.span, - &format!("used `{method}()` on {kind} value"), - None, - &format!("if this value is {none_prefix}`{none_value}`, it will panic"), - ); - } -} diff --git a/clippy_lints/src/methods/filter_map_bool_then.rs b/clippy_lints/src/methods/filter_map_bool_then.rs new file mode 100644 index 000000000..fafc97097 --- /dev/null +++ b/clippy_lints/src/methods/filter_map_bool_then.rs @@ -0,0 +1,53 @@ +use super::FILTER_MAP_BOOL_THEN; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::paths::BOOL_THEN; +use clippy_utils::source::snippet_opt; +use clippy_utils::ty::is_copy; +use clippy_utils::{is_from_proc_macro, is_trait_method, match_def_path, peel_blocks}; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_middle::ty::Binder; +use rustc_span::{sym, Span}; + +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, arg: &Expr<'_>, call_span: Span) { + if !in_external_macro(cx.sess(), expr.span) + && is_trait_method(cx, expr, sym::Iterator) + && let ExprKind::Closure(closure) = arg.kind + && let body = cx.tcx.hir().body(closure.body) + && let value = peel_blocks(body.value) + // Indexing should be fine as `filter_map` always has 1 input, we unfortunately need both + // `inputs` and `params` here as we need both the type and the span + && let param_ty = closure.fn_decl.inputs[0] + && let param = body.params[0] + // Issue #11309 + && let param_ty = cx.tcx.liberate_late_bound_regions( + closure.def_id.to_def_id(), + Binder::bind_with_vars( + cx.typeck_results().node_type(param_ty.hir_id), + cx.tcx.late_bound_vars(cx.tcx.hir().local_def_id_to_hir_id(closure.def_id)), + ), + ) + && is_copy(cx, param_ty) + && let ExprKind::MethodCall(_, recv, [then_arg], _) = value.kind + && let ExprKind::Closure(then_closure) = then_arg.kind + && let then_body = peel_blocks(cx.tcx.hir().body(then_closure.body).value) + && let Some(def_id) = cx.typeck_results().type_dependent_def_id(value.hir_id) + && match_def_path(cx, def_id, &BOOL_THEN) + && !is_from_proc_macro(cx, expr) + && let Some(param_snippet) = snippet_opt(cx, param.span) + && let Some(filter) = snippet_opt(cx, recv.span) + && let Some(map) = snippet_opt(cx, then_body.span) + { + span_lint_and_sugg( + cx, + FILTER_MAP_BOOL_THEN, + call_span, + "usage of `bool::then` in `filter_map`", + "use `filter` then `map` instead", + format!("filter(|&{param_snippet}| {filter}).map(|{param_snippet}| {map})"), + Applicability::MachineApplicable, + ); + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 28a897897..42756b27d 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -17,10 +17,10 @@ mod collapsible_str_replace; mod drain_collect; mod err_expect; mod expect_fun_call; -mod expect_used; mod extend_with_drain; mod filetype_is_file; mod filter_map; +mod filter_map_bool_then; mod filter_map_identity; mod filter_map_next; mod filter_next; @@ -104,7 +104,7 @@ mod unnecessary_lazy_eval; mod unnecessary_literal_unwrap; mod unnecessary_sort_by; mod unnecessary_to_owned; -mod unwrap_used; +mod unwrap_expect_used; mod useless_asref; mod utils; mod vec_resize_to_zero; @@ -3476,6 +3476,37 @@ declare_clippy_lint! { "disallows `.skip(0)`" } +declare_clippy_lint! { + /// ### What it does + /// Checks for usage of `bool::then` in `Iterator::filter_map`. + /// + /// ### Why is this bad? + /// This can be written with `filter` then `map` instead, which would reduce nesting and + /// separates the filtering from the transformation phase. This comes with no cost to + /// performance and is just cleaner. + /// + /// ### Limitations + /// Does not lint `bool::then_some`, as it eagerly evaluates its arguments rather than lazily. + /// This can create differing behavior, so better safe than sorry. + /// + /// ### Example + /// ```rust + /// # fn really_expensive_fn(i: i32) -> i32 { i } + /// # let v = vec![]; + /// _ = v.into_iter().filter_map(|i| (i % 2 == 0).then(|| really_expensive_fn(i))); + /// ``` + /// Use instead: + /// ```rust + /// # fn really_expensive_fn(i: i32) -> i32 { i } + /// # let v = vec![]; + /// _ = v.into_iter().filter(|i| i % 2 == 0).map(|i| really_expensive_fn(i)); + /// ``` + #[clippy::version = "1.72.0"] + pub FILTER_MAP_BOOL_THEN, + style, + "checks for usage of `bool::then` in `Iterator::filter_map`" +} + declare_clippy_lint! { /// ### What it does /// Looks for calls to `RwLock::write` where the lock is only used for reading. @@ -3644,6 +3675,7 @@ impl_lint_pass!(Methods => [ FORMAT_COLLECT, STRING_LIT_CHARS_ANY, ITER_SKIP_ZERO, + FILTER_MAP_BOOL_THEN, READONLY_WRITE_LOCK ]); @@ -3848,6 +3880,13 @@ impl Methods { unnecessary_lazy_eval::check(cx, expr, recv, arg, "and"); } }, + ("any", [arg]) if let ExprKind::Closure(arg) = arg.kind + && let body = cx.tcx.hir().body(arg.body) + && let [param] = body.params + && let Some(("chars", recv, _, _, _)) = method_call(recv) => + { + string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv); + } ("arg", [arg]) => { suspicious_command_arg_space::check(cx, recv, arg, span); } @@ -3908,13 +3947,27 @@ impl Methods { match method_call(recv) { Some(("ok", recv, [], _, _)) => ok_expect::check(cx, expr, recv), Some(("err", recv, [], err_span, _)) => err_expect::check(cx, expr, recv, span, err_span, &self.msrv), - _ => expect_used::check(cx, expr, recv, false, self.allow_expect_in_tests), + _ => unwrap_expect_used::check( + cx, + expr, + recv, + false, + self.allow_expect_in_tests, + unwrap_expect_used::Variant::Expect, + ), } unnecessary_literal_unwrap::check(cx, expr, recv, name, args); }, ("expect_err", [_]) => { unnecessary_literal_unwrap::check(cx, expr, recv, name, args); - expect_used::check(cx, expr, recv, true, self.allow_expect_in_tests); + unwrap_expect_used::check( + cx, + expr, + recv, + true, + self.allow_expect_in_tests, + unwrap_expect_used::Variant::Expect, + ); }, ("extend", [arg]) => { string_extend_chars::check(cx, expr, recv, arg); @@ -3922,6 +3975,7 @@ impl Methods { }, ("filter_map", [arg]) => { unnecessary_filter_map::check(cx, expr, arg, name); + filter_map_bool_then::check(cx, expr, arg, call_span); filter_map_identity::check(cx, expr, arg, span); }, ("find_map", [arg]) => { @@ -3965,20 +4019,9 @@ impl Methods { unnecessary_join::check(cx, expr, recv, join_arg, span); } }, - ("skip", [arg]) => { - iter_skip_zero::check(cx, expr, arg); - - if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) { - if let ("cloned", []) = (name2, args2) { - iter_overeager_cloned::check(cx, expr, recv, recv2, false, false); - } - } - } ("last", []) => { - if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) { - if let ("cloned", []) = (name2, args2) { - iter_overeager_cloned::check(cx, expr, recv, recv2, false, false); - } + if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) { + iter_overeager_cloned::check(cx, expr, recv, recv2, false, false); } }, ("lock", []) => { @@ -4026,13 +4069,6 @@ impl Methods { } } }, - ("any", [arg]) if let ExprKind::Closure(arg) = arg.kind - && let body = cx.tcx.hir().body(arg.body) - && let [param] = body.params - && let Some(("chars", recv, _, _, _)) = method_call(recv) => - { - string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv); - } ("nth", [n_arg]) => match method_call(recv) { Some(("bytes", recv2, [], _, _)) => bytes_nth::check(cx, expr, recv2, n_arg), Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false), @@ -4086,6 +4122,13 @@ impl Methods { seek_to_start_instead_of_rewind::check(cx, expr, recv, arg, span); } }, + ("skip", [arg]) => { + iter_skip_zero::check(cx, expr, arg); + + if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) { + iter_overeager_cloned::check(cx, expr, recv, recv2, false, false); + } + } ("sort", []) => { stable_sort_primitive::check(cx, expr, recv); }, @@ -4108,10 +4151,8 @@ impl Methods { }, ("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg), ("take", [_arg]) => { - if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) { - if let ("cloned", []) = (name2, args2) { - iter_overeager_cloned::check(cx, expr, recv, recv2, false, false); - } + if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) { + iter_overeager_cloned::check(cx, expr, recv, recv2, false, false); } }, ("take", []) => needless_option_take::check(cx, expr, recv), @@ -4146,11 +4187,25 @@ impl Methods { _ => {}, } unnecessary_literal_unwrap::check(cx, expr, recv, name, args); - unwrap_used::check(cx, expr, recv, false, self.allow_unwrap_in_tests); + unwrap_expect_used::check( + cx, + expr, + recv, + false, + self.allow_unwrap_in_tests, + unwrap_expect_used::Variant::Unwrap, + ); }, ("unwrap_err", []) => { unnecessary_literal_unwrap::check(cx, expr, recv, name, args); - unwrap_used::check(cx, expr, recv, true, self.allow_unwrap_in_tests); + unwrap_expect_used::check( + cx, + expr, + recv, + true, + self.allow_unwrap_in_tests, + unwrap_expect_used::Variant::Unwrap, + ); }, ("unwrap_or", [u_arg]) => { match method_call(recv) { @@ -4180,6 +4235,9 @@ impl Methods { } unnecessary_literal_unwrap::check(cx, expr, recv, name, args); }, + ("write", []) => { + readonly_write_lock::check(cx, expr, recv); + } ("zip", [arg]) => { if let ExprKind::MethodCall(name, iter_recv, [], _) = recv.kind && name.ident.name == sym::iter @@ -4187,9 +4245,6 @@ impl Methods { range_zip_with_len::check(cx, expr, iter_recv, arg); } }, - ("write", []) => { - readonly_write_lock::check(cx, expr, recv); - } _ => {}, } } diff --git a/clippy_lints/src/methods/unwrap_expect_used.rs b/clippy_lints/src/methods/unwrap_expect_used.rs new file mode 100644 index 000000000..7bd16b473 --- /dev/null +++ b/clippy_lints/src/methods/unwrap_expect_used.rs @@ -0,0 +1,83 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::ty::{is_never_like, is_type_diagnostic_item}; +use clippy_utils::{is_in_cfg_test, is_in_test_function, is_lint_allowed}; +use rustc_hir::Expr; +use rustc_lint::{LateContext, Lint}; +use rustc_middle::ty; +use rustc_span::sym; + +use super::{EXPECT_USED, UNWRAP_USED}; + +#[derive(Clone, Copy, Eq, PartialEq)] +pub(super) enum Variant { + Unwrap, + Expect, +} + +impl Variant { + fn method_name(self, is_err: bool) -> &'static str { + match (self, is_err) { + (Variant::Unwrap, true) => "unwrap_err", + (Variant::Unwrap, false) => "unwrap", + (Variant::Expect, true) => "expect_err", + (Variant::Expect, false) => "expect", + } + } + + fn lint(self) -> &'static Lint { + match self { + Variant::Unwrap => UNWRAP_USED, + Variant::Expect => EXPECT_USED, + } + } +} + +/// Lint usage of `unwrap` or `unwrap_err` for `Result` and `unwrap()` for `Option` (and their +/// `expect` counterparts). +pub(super) fn check( + cx: &LateContext<'_>, + expr: &Expr<'_>, + recv: &Expr<'_>, + is_err: bool, + allow_unwrap_in_tests: bool, + variant: Variant, +) { + let ty = cx.typeck_results().expr_ty(recv).peel_refs(); + + let (kind, none_value, none_prefix) = if is_type_diagnostic_item(cx, ty, sym::Option) && !is_err { + ("an `Option`", "None", "") + } else if is_type_diagnostic_item(cx, ty, sym::Result) + && let ty::Adt(_, substs) = ty.kind() + && let Some(t_or_e_ty) = substs[usize::from(!is_err)].as_type() + { + if is_never_like(t_or_e_ty) { + return; + } + + ("a `Result`", if is_err { "Ok" } else { "Err" }, "an ") + } else { + return; + }; + + let method_suffix = if is_err { "_err" } else { "" }; + + if allow_unwrap_in_tests && (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id)) { + return; + } + + span_lint_and_then( + cx, + variant.lint(), + expr.span, + &format!("used `{}()` on {kind} value", variant.method_name(is_err)), + |diag| { + diag.note(format!("if this value is {none_prefix}`{none_value}`, it will panic")); + + if variant == Variant::Unwrap && is_lint_allowed(cx, EXPECT_USED, expr.hir_id) { + diag.help(format!( + "consider using `expect{method_suffix}()` to provide a better panic message" + )); + } + }, + ); +} diff --git a/clippy_lints/src/methods/unwrap_used.rs b/clippy_lints/src/methods/unwrap_used.rs deleted file mode 100644 index 5e4c3daee..000000000 --- a/clippy_lints/src/methods/unwrap_used.rs +++ /dev/null @@ -1,53 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{is_in_cfg_test, is_in_test_function, is_lint_allowed}; -use rustc_hir as hir; -use rustc_lint::LateContext; -use rustc_span::sym; - -use super::{EXPECT_USED, UNWRAP_USED}; - -/// lint use of `unwrap()` or `unwrap_err` for `Result` and `unwrap()` for `Option`. -pub(super) fn check( - cx: &LateContext<'_>, - expr: &hir::Expr<'_>, - recv: &hir::Expr<'_>, - is_err: bool, - allow_unwrap_in_tests: bool, -) { - let obj_ty = cx.typeck_results().expr_ty(recv).peel_refs(); - - let mess = if is_type_diagnostic_item(cx, obj_ty, sym::Option) && !is_err { - Some((UNWRAP_USED, "an `Option`", "None", "")) - } else if is_type_diagnostic_item(cx, obj_ty, sym::Result) { - Some((UNWRAP_USED, "a `Result`", if is_err { "Ok" } else { "Err" }, "an ")) - } else { - None - }; - - let method_suffix = if is_err { "_err" } else { "" }; - - if allow_unwrap_in_tests && (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id)) { - return; - } - - if let Some((lint, kind, none_value, none_prefix)) = mess { - let help = if is_lint_allowed(cx, EXPECT_USED, expr.hir_id) { - format!( - "if you don't want to handle the `{none_value}` case gracefully, consider \ - using `expect{method_suffix}()` to provide a better panic message" - ) - } else { - format!("if this value is {none_prefix}`{none_value}`, it will panic") - }; - - span_lint_and_help( - cx, - lint, - expr.span, - &format!("used `unwrap{method_suffix}()` on {kind} value"), - None, - &help, - ); - } -} diff --git a/clippy_lints/src/mut_reference.rs b/clippy_lints/src/mut_reference.rs index 4b20aecad..e53e146ec 100644 --- a/clippy_lints/src/mut_reference.rs +++ b/clippy_lints/src/mut_reference.rs @@ -37,6 +37,11 @@ declare_lint_pass!(UnnecessaryMutPassed => [UNNECESSARY_MUT_PASSED]); impl<'tcx> LateLintPass<'tcx> for UnnecessaryMutPassed { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { + if e.span.from_expansion() { + // Issue #11268 + return; + } + match e.kind { ExprKind::Call(fn_expr, arguments) => { if let ExprKind::Path(ref path) = fn_expr.kind { diff --git a/clippy_lints/src/operators/const_comparisons.rs b/clippy_lints/src/operators/const_comparisons.rs new file mode 100644 index 000000000..abe8df195 --- /dev/null +++ b/clippy_lints/src/operators/const_comparisons.rs @@ -0,0 +1,207 @@ +#![allow(clippy::match_same_arms)] + +use std::cmp::Ordering; + +use clippy_utils::consts::{constant, Constant}; +use if_chain::if_chain; +use rustc_hir::{BinOpKind, Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_middle::ty::layout::HasTyCtxt; +use rustc_middle::ty::{Ty, TypeckResults}; +use rustc_span::source_map::{Span, Spanned}; + +use clippy_utils::diagnostics::span_lint_and_note; +use clippy_utils::source::snippet; +use clippy_utils::SpanlessEq; + +use super::{IMPOSSIBLE_COMPARISONS, REDUNDANT_COMPARISONS}; + +// Extract a comparison between a const and non-const +// Flip yoda conditionals, turnings expressions like `42 < x` into `x > 42` +fn comparison_to_const<'tcx>( + cx: &LateContext<'tcx>, + typeck: &TypeckResults<'tcx>, + expr: &'tcx Expr<'tcx>, +) -> Option<(CmpOp, &'tcx Expr<'tcx>, &'tcx Expr<'tcx>, Constant<'tcx>, Ty<'tcx>)> { + if_chain! { + if let ExprKind::Binary(operator, left, right) = expr.kind; + if let Ok(cmp_op) = CmpOp::try_from(operator.node); + then { + match (constant(cx, typeck, left), constant(cx, typeck, right)) { + (Some(_), Some(_)) => None, + (_, Some(con)) => Some((cmp_op, left, right, con, typeck.expr_ty(right))), + (Some(con), _) => Some((cmp_op.reverse(), right, left, con, typeck.expr_ty(left))), + _ => None, + } + } else { + None + } + } +} + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + and_op: Spanned, + left_cond: &'tcx Expr<'tcx>, + right_cond: &'tcx Expr<'tcx>, + span: Span, +) { + if_chain! { + // Ensure that the binary operator is && + if and_op.node == BinOpKind::And; + + // Check that both operands to '&&' are themselves a binary operation + // The `comparison_to_const` step also checks this, so this step is just an optimization + if let ExprKind::Binary(_, _, _) = left_cond.kind; + if let ExprKind::Binary(_, _, _) = right_cond.kind; + + let typeck = cx.typeck_results(); + + // Check that both operands to '&&' compare a non-literal to a literal + if let Some((left_cmp_op, left_expr, left_const_expr, left_const, left_type)) = + comparison_to_const(cx, typeck, left_cond); + if let Some((right_cmp_op, right_expr, right_const_expr, right_const, right_type)) = + comparison_to_const(cx, typeck, right_cond); + + if left_type == right_type; + + // Check that the same expression is compared in both comparisons + if SpanlessEq::new(cx).eq_expr(left_expr, right_expr); + + if !left_expr.can_have_side_effects(); + + // Compare the two constant expressions + if let Some(ordering) = Constant::partial_cmp(cx.tcx(), left_type, &left_const, &right_const); + + // Rule out the `x >= 42 && x <= 42` corner case immediately + // Mostly to simplify the implementation, but it is also covered by `clippy::double_comparisons` + if !matches!( + (&left_cmp_op, &right_cmp_op, ordering), + (CmpOp::Le | CmpOp::Ge, CmpOp::Le | CmpOp::Ge, Ordering::Equal) + ); + + then { + if left_cmp_op.direction() == right_cmp_op.direction() { + let lhs_str = snippet(cx, left_cond.span, ""); + let rhs_str = snippet(cx, right_cond.span, ""); + // We already know that either side of `&&` has no effect, + // but emit a different error message depending on which side it is + if left_side_is_useless(left_cmp_op, ordering) { + span_lint_and_note( + cx, + REDUNDANT_COMPARISONS, + span, + "left-hand side of `&&` operator has no effect", + Some(left_cond.span.until(right_cond.span)), + &format!("`if `{rhs_str}` evaluates to true, {lhs_str}` will always evaluate to true as well"), + ); + } else { + span_lint_and_note( + cx, + REDUNDANT_COMPARISONS, + span, + "right-hand side of `&&` operator has no effect", + Some(and_op.span.to(right_cond.span)), + &format!("`if `{lhs_str}` evaluates to true, {rhs_str}` will always evaluate to true as well"), + ); + } + // We could autofix this error but choose not to, + // because code triggering this lint probably not behaving correctly in the first place + } + else if !comparison_is_possible(left_cmp_op.direction(), ordering) { + let expr_str = snippet(cx, left_expr.span, ".."); + let lhs_str = snippet(cx, left_const_expr.span, ""); + let rhs_str = snippet(cx, right_const_expr.span, ""); + let note = match ordering { + Ordering::Less => format!("since `{lhs_str}` < `{rhs_str}`, the expression evaluates to false for any value of `{expr_str}`"), + Ordering::Equal => format!("`{expr_str}` cannot simultaneously be greater than and less than `{lhs_str}`"), + Ordering::Greater => format!("since `{lhs_str}` > `{rhs_str}`, the expression evaluates to false for any value of `{expr_str}`"), + }; + span_lint_and_note( + cx, + IMPOSSIBLE_COMPARISONS, + span, + "boolean expression will never evaluate to 'true'", + None, + ¬e, + ); + }; + } + } +} + +fn left_side_is_useless(left_cmp_op: CmpOp, ordering: Ordering) -> bool { + // Special-case for equal constants with an inclusive comparison + if ordering == Ordering::Equal { + match left_cmp_op { + CmpOp::Lt | CmpOp::Gt => false, + CmpOp::Le | CmpOp::Ge => true, + } + } else { + match (left_cmp_op.direction(), ordering) { + (CmpOpDirection::Lesser, Ordering::Less) => false, + (CmpOpDirection::Lesser, Ordering::Equal) => false, + (CmpOpDirection::Lesser, Ordering::Greater) => true, + (CmpOpDirection::Greater, Ordering::Less) => true, + (CmpOpDirection::Greater, Ordering::Equal) => false, + (CmpOpDirection::Greater, Ordering::Greater) => false, + } + } +} + +fn comparison_is_possible(left_cmp_direction: CmpOpDirection, ordering: Ordering) -> bool { + match (left_cmp_direction, ordering) { + (CmpOpDirection::Lesser, Ordering::Less | Ordering::Equal) => false, + (CmpOpDirection::Lesser, Ordering::Greater) => true, + (CmpOpDirection::Greater, Ordering::Greater | Ordering::Equal) => false, + (CmpOpDirection::Greater, Ordering::Less) => true, + } +} + +#[derive(PartialEq, Eq, Clone, Copy)] +enum CmpOpDirection { + Lesser, + Greater, +} + +#[derive(Clone, Copy)] +enum CmpOp { + Lt, + Le, + Ge, + Gt, +} + +impl CmpOp { + fn reverse(self) -> Self { + match self { + CmpOp::Lt => CmpOp::Gt, + CmpOp::Le => CmpOp::Ge, + CmpOp::Ge => CmpOp::Le, + CmpOp::Gt => CmpOp::Lt, + } + } + + fn direction(self) -> CmpOpDirection { + match self { + CmpOp::Lt => CmpOpDirection::Lesser, + CmpOp::Le => CmpOpDirection::Lesser, + CmpOp::Ge => CmpOpDirection::Greater, + CmpOp::Gt => CmpOpDirection::Greater, + } + } +} + +impl TryFrom for CmpOp { + type Error = (); + + fn try_from(bin_op: BinOpKind) -> Result { + match bin_op { + BinOpKind::Lt => Ok(CmpOp::Lt), + BinOpKind::Le => Ok(CmpOp::Le), + BinOpKind::Ge => Ok(CmpOp::Ge), + BinOpKind::Gt => Ok(CmpOp::Gt), + _ => Err(()), + } + } +} diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index 2cf15adda..4635e1164 100644 --- a/clippy_lints/src/operators/mod.rs +++ b/clippy_lints/src/operators/mod.rs @@ -2,6 +2,7 @@ mod absurd_extreme_comparisons; mod assign_op_pattern; mod bit_mask; mod cmp_owned; +mod const_comparisons; mod double_comparison; mod duration_subsec; mod eq_op; @@ -298,6 +299,45 @@ declare_clippy_lint! { "unnecessary double comparisons that can be simplified" } +declare_clippy_lint! { + /// ### What it does + /// Checks for double comparisons that can never succeed + /// + /// ### Why is this bad? + /// The whole expression can be replaced by `false`, + /// which is probably not the programmer's intention + /// + /// ### Example + /// ```rust + /// # let status_code = 200; + /// if status_code <= 400 && status_code > 500 {} + /// ``` + #[clippy::version = "1.71.0"] + pub IMPOSSIBLE_COMPARISONS, + correctness, + "double comparisons that will never evaluate to `true`" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for ineffective double comparisons against constants. + /// + /// ### Why is this bad? + /// Only one of the comparisons has any effect on the result, the programmer + /// probably intended to flip one of the comparison operators, or compare a + /// different value entirely. + /// + /// ### Example + /// ```rust + /// # let status_code = 200; + /// if status_code <= 400 && status_code < 500 {} + /// ``` + #[clippy::version = "1.71.0"] + pub REDUNDANT_COMPARISONS, + correctness, + "double comparisons where one of them can be removed" +} + declare_clippy_lint! { /// ### What it does /// Checks for calculation of subsecond microseconds or milliseconds @@ -742,6 +782,8 @@ impl_lint_pass!(Operators => [ INEFFECTIVE_BIT_MASK, VERBOSE_BIT_MASK, DOUBLE_COMPARISONS, + IMPOSSIBLE_COMPARISONS, + REDUNDANT_COMPARISONS, DURATION_SUBSEC, EQ_OP, OP_REF, @@ -786,6 +828,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators { bit_mask::check(cx, e, op.node, lhs, rhs); verbose_bit_mask::check(cx, e, op.node, lhs, rhs, self.verbose_bit_mask_threshold); double_comparison::check(cx, op.node, lhs, rhs, e.span); + const_comparisons::check(cx, op, lhs, rhs, e.span); duration_subsec::check(cx, e, op.node, lhs, rhs); float_equality_without_abs::check(cx, e, op.node, lhs, rhs); integer_division::check(cx, e, op.node, lhs, rhs); diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs index 40da002f4..a7a7f4fd8 100644 --- a/clippy_lints/src/option_if_let_else.rs +++ b/clippy_lints/src/option_if_let_else.rs @@ -155,7 +155,7 @@ fn try_get_option_occurrence<'tcx>( }); if let ExprKind::Path(QPath::Resolved(None, Path { res: Res::Local(local_id), .. })) = e.kind { match some_captures.get(local_id) - .or_else(|| (method_sugg == "map_or_else").then_some(()).and_then(|_| none_captures.get(local_id))) + .or_else(|| (method_sugg == "map_or_else").then_some(()).and_then(|()| none_captures.get(local_id))) { Some(CaptureKind::Value | CaptureKind::Ref(Mutability::Mut)) => return None, Some(CaptureKind::Ref(Mutability::Not)) if as_mut => return None, diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index f5502cffb..734ca2914 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -1,11 +1,13 @@ use crate::manual_let_else::{MatchLintBehaviour, MANUAL_LET_ELSE}; +use crate::question_mark_used::QUESTION_MARK_USED; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::msrvs::Msrv; use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::{ - eq_expr_value, get_parent_node, higher, in_constant, is_else_clause, is_path_lang_item, is_res_lang_ctor, - pat_and_expr_can_be_question_mark, path_to_local, path_to_local_id, peel_blocks, peel_blocks_with_stmt, + eq_expr_value, get_parent_node, higher, in_constant, is_else_clause, is_lint_allowed, is_path_lang_item, + is_res_lang_ctor, pat_and_expr_can_be_question_mark, path_to_local, path_to_local_id, peel_blocks, + peel_blocks_with_stmt, }; use if_chain::if_chain; use rustc_errors::Applicability; @@ -299,13 +301,17 @@ fn is_try_block(cx: &LateContext<'_>, bl: &rustc_hir::Block<'_>) -> bool { impl<'tcx> LateLintPass<'tcx> for QuestionMark { fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { + if !is_lint_allowed(cx, QUESTION_MARK_USED, stmt.hir_id) { + return; + } + if !in_constant(cx, stmt.hir_id) { check_let_some_else_return_none(cx, stmt); } self.check_manual_let_else(cx, stmt); } fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if !in_constant(cx, expr.hir_id) { + if !in_constant(cx, expr.hir_id) && is_lint_allowed(cx, QUESTION_MARK_USED, expr.hir_id) { self.check_is_none_or_err_and_early_return(cx, expr); self.check_if_let_some_or_err_and_early_return(cx, expr); } diff --git a/clippy_lints/src/redundant_locals.rs b/clippy_lints/src/redundant_locals.rs index 896bd79b2..0c89c7ee4 100644 --- a/clippy_lints/src/redundant_locals.rs +++ b/clippy_lints/src/redundant_locals.rs @@ -1,6 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::is_from_proc_macro; use clippy_utils::ty::needs_ordered_drop; +use rustc_ast::Mutability; use rustc_hir::def::Res; use rustc_hir::{ BindingAnnotation, ByRef, Expr, ExprKind, HirId, Local, Node, Pat, PatKind, QPath, @@ -9,6 +10,7 @@ use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::{in_external_macro, is_from_async_await}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::symbol::Ident; +use rustc_span::DesugaringKind; declare_clippy_lint! { /// ### What it does @@ -47,6 +49,7 @@ declare_lint_pass!(RedundantLocals => [REDUNDANT_LOCALS]); impl<'tcx> LateLintPass<'tcx> for RedundantLocals { fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) { if_chain! { + if !local.span.is_desugaring(DesugaringKind::Async); // the pattern is a single by-value binding if let PatKind::Binding(BindingAnnotation(ByRef::No, mutability), _, ident, None) = local.pat.kind; // the binding is not type-ascribed @@ -62,6 +65,8 @@ impl<'tcx> LateLintPass<'tcx> for RedundantLocals { if let Node::Pat(binding_pat) = cx.tcx.hir().get(binding_id); // the previous binding has the same mutability if find_binding(binding_pat, ident).unwrap().1 == mutability; + // the local does not change the effect of assignments to the binding. see #11290 + if !affects_assignments(cx, mutability, binding_id, local.hir_id); // the local does not affect the code's drop behavior if !affects_drop_behavior(cx, binding_id, local.hir_id, expr); // the local is user-controlled @@ -97,6 +102,14 @@ fn find_binding(pat: &Pat<'_>, name: Ident) -> Option { ret } +/// Check if a rebinding of a local changes the effect of assignments to the binding. +fn affects_assignments(cx: &LateContext<'_>, mutability: Mutability, bind: HirId, rebind: HirId) -> bool { + let hir = cx.tcx.hir(); + + // the binding is mutable and the rebinding is in a different scope than the original binding + mutability == Mutability::Mut && hir.get_enclosing_scope(bind) != hir.get_enclosing_scope(rebind) +} + /// Check if a rebinding of a local affects the code's drop behavior. fn affects_drop_behavior<'tcx>( cx: &LateContext<'tcx>, diff --git a/clippy_lints/src/redundant_type_annotations.rs b/clippy_lints/src/redundant_type_annotations.rs index 8e9234bba..3e963d798 100644 --- a/clippy_lints/src/redundant_type_annotations.rs +++ b/clippy_lints/src/redundant_type_annotations.rs @@ -1,6 +1,8 @@ use clippy_utils::diagnostics::span_lint; +use clippy_utils::is_lint_allowed; use rustc_ast::LitKind; use rustc_hir as hir; +use rustc_hir::def::DefKind; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::Ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -45,8 +47,8 @@ fn is_same_type<'tcx>(cx: &LateContext<'tcx>, ty_resolved_path: hir::def::Res, f return primty.name() == func_return_type_sym; } - // type annotation is any other non generic type - if let hir::def::Res::Def(_, defid) = ty_resolved_path + // type annotation is a non generic type + if let hir::def::Res::Def(DefKind::Struct | DefKind::Union | DefKind::Enum, defid) = ty_resolved_path && let Some(annotation_ty) = cx.tcx.type_of(defid).no_bound_vars() { return annotation_ty == func_return_type; @@ -130,8 +132,9 @@ fn extract_primty(ty_kind: &hir::TyKind<'_>) -> Option { impl LateLintPass<'_> for RedundantTypeAnnotations { fn check_local<'tcx>(&mut self, cx: &LateContext<'tcx>, local: &'tcx rustc_hir::Local<'tcx>) { - // type annotation part - if !local.span.from_expansion() + if !is_lint_allowed(cx, REDUNDANT_TYPE_ANNOTATIONS, local.hir_id) + // type annotation part + && !local.span.from_expansion() && let Some(ty) = &local.ty // initialization part diff --git a/clippy_lints/src/slow_vector_initialization.rs b/clippy_lints/src/slow_vector_initialization.rs index 54a33eb29..c9ab622ad 100644 --- a/clippy_lints/src/slow_vector_initialization.rs +++ b/clippy_lints/src/slow_vector_initialization.rs @@ -20,18 +20,27 @@ declare_clippy_lint! { /// These structures are non-idiomatic and less efficient than simply using /// `vec![0; len]`. /// + /// Specifically, for `vec![0; len]`, the compiler can use a specialized type of allocation + /// that also zero-initializes the allocated memory in the same call + /// (see: [alloc_zeroed](https://doc.rust-lang.org/stable/std/alloc/trait.GlobalAlloc.html#method.alloc_zeroed)). + /// + /// Writing `Vec::new()` followed by `vec.resize(len, 0)` is suboptimal because, + /// while it does do the same number of allocations, + /// it involves two operations for allocating and initializing. + /// The `resize` call first allocates memory (since `Vec::new()` did not), and only *then* zero-initializes it. + /// /// ### Example /// ```rust /// # use core::iter::repeat; /// # let len = 4; - /// let mut vec1 = Vec::with_capacity(len); + /// let mut vec1 = Vec::new(); /// vec1.resize(len, 0); /// - /// let mut vec1 = Vec::with_capacity(len); - /// vec1.resize(vec1.capacity(), 0); - /// /// let mut vec2 = Vec::with_capacity(len); - /// vec2.extend(repeat(0).take(len)); + /// vec2.resize(len, 0); + /// + /// let mut vec3 = Vec::with_capacity(len); + /// vec3.extend(repeat(0).take(len)); /// ``` /// /// Use instead: @@ -39,6 +48,7 @@ declare_clippy_lint! { /// # let len = 4; /// let mut vec1 = vec![0; len]; /// let mut vec2 = vec![0; len]; + /// let mut vec3 = vec![0; len]; /// ``` #[clippy::version = "1.32.0"] pub SLOW_VECTOR_INITIALIZATION, diff --git a/clippy_lints/src/suspicious_xor_used_as_pow.rs b/clippy_lints/src/suspicious_xor_used_as_pow.rs index 6fa49afe0..8e156b882 100644 --- a/clippy_lints/src/suspicious_xor_used_as_pow.rs +++ b/clippy_lints/src/suspicious_xor_used_as_pow.rs @@ -1,5 +1,7 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::numeric_literal::NumericLiteral; -use clippy_utils::source::snippet_with_context; +use clippy_utils::source::snippet; +use rustc_ast::LitKind; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -28,27 +30,29 @@ declare_lint_pass!(ConfusingXorAndPow => [SUSPICIOUS_XOR_USED_AS_POW]); impl LateLintPass<'_> for ConfusingXorAndPow { fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { - if !in_external_macro(cx.sess(), expr.span) && - let ExprKind::Binary(op, left, right) = &expr.kind && - op.node == BinOpKind::BitXor && - left.span.ctxt() == right.span.ctxt() && - let ExprKind::Lit(lit_left) = &left.kind && - let ExprKind::Lit(lit_right) = &right.kind && - let snip_left = snippet_with_context(cx, lit_left.span, lit_left.span.ctxt(), "..", &mut Applicability::MaybeIncorrect) && - let snip_right = snippet_with_context(cx, lit_right.span, lit_right.span.ctxt(), "..", &mut Applicability::MaybeIncorrect) && - let Some(left_val) = NumericLiteral::from_lit_kind(&snip_left.0, &lit_left.node) && - let Some(right_val) = NumericLiteral::from_lit_kind(&snip_right.0, &lit_right.node) && - left_val.is_decimal() && - right_val.is_decimal() { - clippy_utils::diagnostics::span_lint_and_sugg( - cx, - SUSPICIOUS_XOR_USED_AS_POW, - expr.span, - "`^` is not the exponentiation operator", - "did you mean to write", - format!("{}.pow({})", left_val.format(), right_val.format()), - Applicability::MaybeIncorrect, - ); + if !in_external_macro(cx.sess(), expr.span) + && let ExprKind::Binary(op, left, right) = &expr.kind + && op.node == BinOpKind::BitXor + && left.span.ctxt() == right.span.ctxt() + && let ExprKind::Lit(lit_left) = &left.kind + && let ExprKind::Lit(lit_right) = &right.kind + && matches!(lit_right.node, LitKind::Int(..) | LitKind::Float(..)) + && matches!(lit_left.node, LitKind::Int(..) | LitKind::Float(..)) + && NumericLiteral::from_lit_kind(&snippet(cx, lit_right.span, ".."), &lit_right.node).is_some_and(|x| x.is_decimal()) + { + span_lint_and_sugg( + cx, + SUSPICIOUS_XOR_USED_AS_POW, + expr.span, + "`^` is not the exponentiation operator", + "did you mean to write", + format!( + "{}.pow({})", + lit_left.node, + lit_right.node + ), + Applicability::MaybeIncorrect, + ); } } } diff --git a/clippy_lints/src/utils/internal_lints/invalid_paths.rs b/clippy_lints/src/utils/internal_lints/invalid_paths.rs index e4906944c..4ed985f54 100644 --- a/clippy_lints/src/utils/internal_lints/invalid_paths.rs +++ b/clippy_lints/src/utils/internal_lints/invalid_paths.rs @@ -34,7 +34,7 @@ impl<'tcx> LateLintPass<'tcx> for InvalidPaths { let mod_name = &cx.tcx.item_name(local_def_id.to_def_id()); if_chain! { if mod_name.as_str() == "paths"; - if let hir::ItemKind::Const(ty, body_id) = item.kind; + if let hir::ItemKind::Const(ty, _, body_id) = item.kind; let ty = hir_ty_to_ty(cx.tcx, ty); if let ty::Array(el_ty, _) = &ty.kind(); if let ty::Ref(_, el_ty, _) = &el_ty.kind(); diff --git a/clippy_utils/src/ast_utils.rs b/clippy_utils/src/ast_utils.rs index 2d0d6f559..140cfa219 100644 --- a/clippy_utils/src/ast_utils.rs +++ b/clippy_utils/src/ast_utils.rs @@ -178,7 +178,9 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool { (Yield(l), Yield(r)) | (Ret(l), Ret(r)) => eq_expr_opt(l, r), (Break(ll, le), Break(rl, re)) => eq_label(ll, rl) && eq_expr_opt(le, re), (Continue(ll), Continue(rl)) => eq_label(ll, rl), - (Assign(l1, l2, _), Assign(r1, r2, _)) | (Index(l1, l2, _), Index(r1, r2, _)) => eq_expr(l1, r1) && eq_expr(l2, r2), + (Assign(l1, l2, _), Assign(r1, r2, _)) | (Index(l1, l2, _), Index(r1, r2, _)) => { + eq_expr(l1, r1) && eq_expr(l2, r2) + }, (AssignOp(lo, lp, lv), AssignOp(ro, rp, rv)) => lo.node == ro.node && eq_expr(lp, rp) && eq_expr(lv, rv), (Field(lp, lf), Field(rp, rf)) => eq_id(*lf, *rf) && eq_expr(lp, rp), (Match(ls, la), Match(rs, ra)) => eq_expr(ls, rs) && over(la, ra, eq_arm), diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index d38e3f1ae..adeb673b6 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -329,7 +329,7 @@ pub struct ConstEvalLateContext<'a, 'tcx> { } impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { - fn new(lcx: &'a LateContext<'tcx>, typeck_results: &'a ty::TypeckResults<'tcx>) -> Self { + pub fn new(lcx: &'a LateContext<'tcx>, typeck_results: &'a ty::TypeckResults<'tcx>) -> Self { Self { lcx, typeck_results, diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 8d96d3cfe..914ea85ac 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -163,3 +163,5 @@ pub const OPTION_EXPECT: [&str; 4] = ["core", "option", "Option", "expect"]; pub const FORMATTER: [&str; 3] = ["core", "fmt", "Formatter"]; pub const DEBUG_STRUCT: [&str; 4] = ["core", "fmt", "builders", "DebugStruct"]; pub const ORD_CMP: [&str; 4] = ["core", "cmp", "Ord", "cmp"]; +#[expect(clippy::invalid_paths)] // not sure why it thinks this, it works so +pub const BOOL_THEN: [&str; 4] = ["core", "bool", "", "then"]; diff --git a/clippy_utils/src/qualify_min_const_fn.rs b/clippy_utils/src/qualify_min_const_fn.rs index f0a777c5b..4c695cb9b 100644 --- a/clippy_utils/src/qualify_min_const_fn.rs +++ b/clippy_utils/src/qualify_min_const_fn.rs @@ -14,7 +14,7 @@ use rustc_middle::mir::{ Body, CastKind, NonDivergingIntrinsic, NullOp, Operand, Place, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, }; -use rustc_middle::traits::{ImplSource, ObligationCause, BuiltinImplSource}; +use rustc_middle::traits::{BuiltinImplSource, ImplSource, ObligationCause}; use rustc_middle::ty::adjustment::PointerCoercion; use rustc_middle::ty::{self, GenericArgKind, TraitRef, Ty, TyCtxt}; use rustc_semver::RustcVersion; @@ -425,5 +425,5 @@ fn is_ty_const_destruct<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, body: &Body<'tcx> ocx.select_all_or_error().is_empty() } - !ty.needs_drop(tcx, ConstCx::new(tcx, body).param_env) + !ty.needs_drop(tcx, ConstCx::new(tcx, body).param_env) } diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs index a43a81bc6..ee5a49a20 100644 --- a/clippy_utils/src/sugg.rs +++ b/clippy_utils/src/sugg.rs @@ -1010,7 +1010,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> { projections_handled = true; }, // note: unable to trigger `Subslice` kind in tests - ProjectionKind::Subslice => (), + ProjectionKind::Subslice | // Doesn't have surface syntax. Only occurs in patterns. ProjectionKind::OpaqueCast => (), ProjectionKind::Deref => { diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index 717664805..a05f682aa 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -1093,6 +1093,11 @@ fn assert_generic_args_match<'tcx>(tcx: TyCtxt<'tcx>, did: DefId, args: &[Generi } } +/// Returns whether `ty` is never-like; i.e., `!` (never) or an enum with zero variants. +pub fn is_never_like(ty: Ty<'_>) -> bool { + ty.is_never() || (ty.is_enum() && ty.ty_adt_def().is_some_and(|def| def.variants().is_empty())) +} + /// Makes the projection type for the named associated type in the given impl or trait impl. /// /// This function is for associated types which are "known" to exist, and as such, will only return diff --git a/rust-toolchain b/rust-toolchain index 833608faf..8b3f819f0 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2023-07-28" +channel = "nightly-2023-08-10" components = ["cargo", "llvm-tools", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"] diff --git a/tests/ui-internal/custom_ice_message.stderr b/tests/ui-internal/custom_ice_message.stderr index 31df0ebd9..d8b158816 100644 --- a/tests/ui-internal/custom_ice_message.stderr +++ b/tests/ui-internal/custom_ice_message.stderr @@ -1,4 +1,5 @@ -thread '' panicked at 'Would you like some help with that?', clippy_lints/src/utils/internal_lints/produce_ice.rs +thread '' panicked at clippy_lints/src/utils/internal_lints/produce_ice.rs: +Would you like some help with that? note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace error: the compiler unexpectedly panicked. this is a bug. diff --git a/tests/ui-toml/expect_used/expect_used.stderr b/tests/ui-toml/expect_used/expect_used.stderr index 9eef0e1bf..815d00935 100644 --- a/tests/ui-toml/expect_used/expect_used.stderr +++ b/tests/ui-toml/expect_used/expect_used.stderr @@ -4,7 +4,7 @@ error: used `expect()` on an `Option` value LL | let _ = opt.expect(""); | ^^^^^^^^^^^^^^ | - = help: if this value is `None`, it will panic + = note: if this value is `None`, it will panic = note: `-D clippy::expect-used` implied by `-D warnings` error: used `expect()` on a `Result` value @@ -13,7 +13,7 @@ error: used `expect()` on a `Result` value LL | let _ = res.expect(""); | ^^^^^^^^^^^^^^ | - = help: if this value is an `Err`, it will panic + = note: if this value is an `Err`, it will panic error: aborting due to 2 previous errors diff --git a/tests/ui-toml/unwrap_used/unwrap_used.stderr b/tests/ui-toml/unwrap_used/unwrap_used.stderr index 4c9bdfa9d..10219beaf 100644 --- a/tests/ui-toml/unwrap_used/unwrap_used.stderr +++ b/tests/ui-toml/unwrap_used/unwrap_used.stderr @@ -12,7 +12,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = boxed_slice.get(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message = note: `-D clippy::unwrap-used` implied by `-D warnings` error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise @@ -27,7 +28,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = some_slice.get(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a Vec. Using `[]` is more clear and more concise --> $DIR/unwrap_used.rs:40:17 @@ -41,7 +43,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = some_vec.get(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a VecDeque. Using `[]` is more clear and more concise --> $DIR/unwrap_used.rs:41:17 @@ -55,7 +58,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = some_vecdeque.get(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a HashMap. Using `[]` is more clear and more concise --> $DIR/unwrap_used.rs:42:17 @@ -69,7 +73,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = some_hashmap.get(&1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a BTreeMap. Using `[]` is more clear and more concise --> $DIR/unwrap_used.rs:43:17 @@ -83,7 +88,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = some_btreemap.get(&1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise --> $DIR/unwrap_used.rs:47:21 @@ -97,7 +103,8 @@ error: used `unwrap()` on an `Option` value LL | let _: u8 = *boxed_slice.get(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise --> $DIR/unwrap_used.rs:52:9 @@ -111,7 +118,8 @@ error: used `unwrap()` on an `Option` value LL | *boxed_slice.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise --> $DIR/unwrap_used.rs:53:9 @@ -125,7 +133,8 @@ error: used `unwrap()` on an `Option` value LL | *some_slice.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get_mut().unwrap()` on a Vec. Using `[]` is more clear and more concise --> $DIR/unwrap_used.rs:54:9 @@ -139,7 +148,8 @@ error: used `unwrap()` on an `Option` value LL | *some_vec.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get_mut().unwrap()` on a VecDeque. Using `[]` is more clear and more concise --> $DIR/unwrap_used.rs:55:9 @@ -153,7 +163,8 @@ error: used `unwrap()` on an `Option` value LL | *some_vecdeque.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a Vec. Using `[]` is more clear and more concise --> $DIR/unwrap_used.rs:67:17 @@ -167,7 +178,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = some_vec.get(0..1).unwrap().to_vec(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get_mut().unwrap()` on a Vec. Using `[]` is more clear and more concise --> $DIR/unwrap_used.rs:68:17 @@ -181,7 +193,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = some_vec.get_mut(0..1).unwrap().to_vec(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise --> $DIR/unwrap_used.rs:75:13 diff --git a/tests/ui/const_comparisons.rs b/tests/ui/const_comparisons.rs new file mode 100644 index 000000000..8e265c914 --- /dev/null +++ b/tests/ui/const_comparisons.rs @@ -0,0 +1,93 @@ +#![allow(unused)] +#![warn(clippy::impossible_comparisons)] +#![warn(clippy::redundant_comparisons)] +#![allow(clippy::no_effect)] +#![allow(clippy::short_circuit_statement)] +#![allow(clippy::manual_range_contains)] + +const STATUS_BAD_REQUEST: u16 = 400; +const STATUS_SERVER_ERROR: u16 = 500; + +struct Status { + code: u16, +} + +impl PartialEq for Status { + fn eq(&self, other: &u16) -> bool { + self.code == *other + } +} + +impl PartialOrd for Status { + fn partial_cmp(&self, other: &u16) -> Option { + self.code.partial_cmp(other) + } +} + +impl PartialEq for u16 { + fn eq(&self, other: &Status) -> bool { + *self == other.code + } +} + +impl PartialOrd for u16 { + fn partial_cmp(&self, other: &Status) -> Option { + self.partial_cmp(&other.code) + } +} + +fn main() { + let status_code = 500; // Value doesn't matter for the lint + let status = Status { code: status_code }; + + status_code >= 400 && status_code < 500; // Correct + status_code <= 400 && status_code > 500; + status_code > 500 && status_code < 400; + status_code < 500 && status_code > 500; + + // More complex expressions + status_code < { 400 } && status_code > { 500 }; + status_code < STATUS_BAD_REQUEST && status_code > STATUS_SERVER_ERROR; + status_code <= u16::MIN + 1 && status_code > STATUS_SERVER_ERROR; + status_code < STATUS_SERVER_ERROR && status_code > STATUS_SERVER_ERROR; + + // Comparing two different types, via the `impl PartialOrd for Status` + status < { 400 } && status > { 500 }; + status < STATUS_BAD_REQUEST && status > STATUS_SERVER_ERROR; + status <= u16::MIN + 1 && status > STATUS_SERVER_ERROR; + status < STATUS_SERVER_ERROR && status > STATUS_SERVER_ERROR; + + // Yoda conditions + 500 <= status_code && 600 > status_code; // Correct + 500 <= status_code && status_code <= 600; // Correct + 500 >= status_code && 600 < status_code; // Incorrect + 500 >= status_code && status_code > 600; // Incorrect + + // Yoda conditions, comparing two different types + 500 <= status && 600 > status; // Correct + 500 <= status && status <= 600; // Correct + 500 >= status && 600 < status; // Incorrect + 500 >= status && status > 600; // Incorrect + + // Expressions where one of the sides has no effect + status_code < 200 && status_code <= 299; + status_code > 200 && status_code >= 299; + + status_code >= 500 && status_code > 500; // Useless left + status_code > 500 && status_code >= 500; // Useless right + status_code <= 500 && status_code < 500; // Useless left + status_code < 500 && status_code <= 500; // Useless right + + // Other types + let name = "Steve"; + name < "Jennifer" && name > "Shannon"; + + let numbers = [1, 2]; + numbers < [3, 4] && numbers > [5, 6]; + + let letter = 'a'; + letter < 'b' && letter > 'c'; + + let area = 42.0; + area < std::f32::consts::E && area > std::f32::consts::PI; +} diff --git a/tests/ui/const_comparisons.stderr b/tests/ui/const_comparisons.stderr new file mode 100644 index 000000000..90e6db647 --- /dev/null +++ b/tests/ui/const_comparisons.stderr @@ -0,0 +1,228 @@ +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:44:5 + | +LL | status_code <= 400 && status_code > 500; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `400` < `500`, the expression evaluates to false for any value of `status_code` + = note: `-D clippy::impossible-comparisons` implied by `-D warnings` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:45:5 + | +LL | status_code > 500 && status_code < 400; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `500` > `400`, the expression evaluates to false for any value of `status_code` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:46:5 + | +LL | status_code < 500 && status_code > 500; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `status_code` cannot simultaneously be greater than and less than `500` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:49:5 + | +LL | status_code < { 400 } && status_code > { 500 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `{ 400 }` < `{ 500 }`, the expression evaluates to false for any value of `status_code` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:50:5 + | +LL | status_code < STATUS_BAD_REQUEST && status_code > STATUS_SERVER_ERROR; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `STATUS_BAD_REQUEST` < `STATUS_SERVER_ERROR`, the expression evaluates to false for any value of `status_code` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:51:5 + | +LL | status_code <= u16::MIN + 1 && status_code > STATUS_SERVER_ERROR; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `u16::MIN + 1` < `STATUS_SERVER_ERROR`, the expression evaluates to false for any value of `status_code` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:52:5 + | +LL | status_code < STATUS_SERVER_ERROR && status_code > STATUS_SERVER_ERROR; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `status_code` cannot simultaneously be greater than and less than `STATUS_SERVER_ERROR` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:55:5 + | +LL | status < { 400 } && status > { 500 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `{ 400 }` < `{ 500 }`, the expression evaluates to false for any value of `status` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:56:5 + | +LL | status < STATUS_BAD_REQUEST && status > STATUS_SERVER_ERROR; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `STATUS_BAD_REQUEST` < `STATUS_SERVER_ERROR`, the expression evaluates to false for any value of `status` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:57:5 + | +LL | status <= u16::MIN + 1 && status > STATUS_SERVER_ERROR; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `u16::MIN + 1` < `STATUS_SERVER_ERROR`, the expression evaluates to false for any value of `status` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:58:5 + | +LL | status < STATUS_SERVER_ERROR && status > STATUS_SERVER_ERROR; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `status` cannot simultaneously be greater than and less than `STATUS_SERVER_ERROR` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:63:5 + | +LL | 500 >= status_code && 600 < status_code; // Incorrect + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `500` < `600`, the expression evaluates to false for any value of `status_code` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:64:5 + | +LL | 500 >= status_code && status_code > 600; // Incorrect + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `500` < `600`, the expression evaluates to false for any value of `status_code` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:69:5 + | +LL | 500 >= status && 600 < status; // Incorrect + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `500` < `600`, the expression evaluates to false for any value of `status` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:70:5 + | +LL | 500 >= status && status > 600; // Incorrect + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `500` < `600`, the expression evaluates to false for any value of `status` + +error: right-hand side of `&&` operator has no effect + --> $DIR/const_comparisons.rs:73:5 + | +LL | status_code < 200 && status_code <= 299; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: `if `status_code < 200` evaluates to true, status_code <= 299` will always evaluate to true as well + --> $DIR/const_comparisons.rs:73:23 + | +LL | status_code < 200 && status_code <= 299; + | ^^^^^^^^^^^^^^^^^^^^^ + = note: `-D clippy::redundant-comparisons` implied by `-D warnings` + +error: left-hand side of `&&` operator has no effect + --> $DIR/const_comparisons.rs:74:5 + | +LL | status_code > 200 && status_code >= 299; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: `if `status_code >= 299` evaluates to true, status_code > 200` will always evaluate to true as well + --> $DIR/const_comparisons.rs:74:5 + | +LL | status_code > 200 && status_code >= 299; + | ^^^^^^^^^^^^^^^^^^^^^ + +error: left-hand side of `&&` operator has no effect + --> $DIR/const_comparisons.rs:76:5 + | +LL | status_code >= 500 && status_code > 500; // Useless left + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: `if `status_code > 500` evaluates to true, status_code >= 500` will always evaluate to true as well + --> $DIR/const_comparisons.rs:76:5 + | +LL | status_code >= 500 && status_code > 500; // Useless left + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: right-hand side of `&&` operator has no effect + --> $DIR/const_comparisons.rs:77:5 + | +LL | status_code > 500 && status_code >= 500; // Useless right + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: `if `status_code > 500` evaluates to true, status_code >= 500` will always evaluate to true as well + --> $DIR/const_comparisons.rs:77:23 + | +LL | status_code > 500 && status_code >= 500; // Useless right + | ^^^^^^^^^^^^^^^^^^^^^ + +error: left-hand side of `&&` operator has no effect + --> $DIR/const_comparisons.rs:78:5 + | +LL | status_code <= 500 && status_code < 500; // Useless left + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: `if `status_code < 500` evaluates to true, status_code <= 500` will always evaluate to true as well + --> $DIR/const_comparisons.rs:78:5 + | +LL | status_code <= 500 && status_code < 500; // Useless left + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: right-hand side of `&&` operator has no effect + --> $DIR/const_comparisons.rs:79:5 + | +LL | status_code < 500 && status_code <= 500; // Useless right + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: `if `status_code < 500` evaluates to true, status_code <= 500` will always evaluate to true as well + --> $DIR/const_comparisons.rs:79:23 + | +LL | status_code < 500 && status_code <= 500; // Useless right + | ^^^^^^^^^^^^^^^^^^^^^ + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:83:5 + | +LL | name < "Jennifer" && name > "Shannon"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `"Jennifer"` < `"Shannon"`, the expression evaluates to false for any value of `name` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:86:5 + | +LL | numbers < [3, 4] && numbers > [5, 6]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `[3, 4]` < `[5, 6]`, the expression evaluates to false for any value of `numbers` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:89:5 + | +LL | letter < 'b' && letter > 'c'; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `'b'` < `'c'`, the expression evaluates to false for any value of `letter` + +error: boolean expression will never evaluate to 'true' + --> $DIR/const_comparisons.rs:92:5 + | +LL | area < std::f32::consts::E && area > std::f32::consts::PI; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: since `std::f32::consts::E` < `std::f32::consts::PI`, the expression evaluates to false for any value of `area` + +error: aborting due to 25 previous errors + diff --git a/tests/ui/expect.stderr b/tests/ui/expect.stderr index be340340d..f787fa973 100644 --- a/tests/ui/expect.stderr +++ b/tests/ui/expect.stderr @@ -4,7 +4,7 @@ error: used `expect()` on an `Option` value LL | let _ = opt.expect(""); | ^^^^^^^^^^^^^^ | - = help: if this value is `None`, it will panic + = note: if this value is `None`, it will panic = note: `-D clippy::expect-used` implied by `-D warnings` error: used `expect()` on a `Result` value @@ -13,7 +13,7 @@ error: used `expect()` on a `Result` value LL | let _ = res.expect(""); | ^^^^^^^^^^^^^^ | - = help: if this value is an `Err`, it will panic + = note: if this value is an `Err`, it will panic error: used `expect_err()` on a `Result` value --> $DIR/expect.rs:12:13 @@ -21,7 +21,7 @@ error: used `expect_err()` on a `Result` value LL | let _ = res.expect_err(""); | ^^^^^^^^^^^^^^^^^^ | - = help: if this value is an `Ok`, it will panic + = note: if this value is an `Ok`, it will panic error: aborting due to 3 previous errors diff --git a/tests/ui/filter_map_bool_then.fixed b/tests/ui/filter_map_bool_then.fixed new file mode 100644 index 000000000..e5c9f783f --- /dev/null +++ b/tests/ui/filter_map_bool_then.fixed @@ -0,0 +1,58 @@ +//@run-rustfix +//@aux-build:proc_macros.rs:proc-macro +#![allow( + clippy::clone_on_copy, + clippy::map_identity, + clippy::unnecessary_lazy_evaluations, + clippy::unnecessary_filter_map, + unused +)] +#![warn(clippy::filter_map_bool_then)] + +#[macro_use] +extern crate proc_macros; + +#[derive(Clone, PartialEq)] +struct NonCopy; + +fn main() { + let v = vec![1, 2, 3, 4, 5, 6]; + v.clone().iter().filter(|&i| (i % 2 == 0)).map(|i| i + 1); + v.clone().into_iter().filter(|&i| (i % 2 == 0)).map(|i| i + 1); + v.clone() + .into_iter() + .filter(|&i| (i % 2 == 0)).map(|i| i + 1); + v.clone() + .into_iter() + .filter(|&i| i != 1000) + .filter(|&i| (i % 2 == 0)).map(|i| i + 1); + v.iter() + .copied() + .filter(|&i| i != 1000) + .filter(|&i| (i.clone() % 2 == 0)).map(|i| i + 1); + // Despite this is non-copy, `is_copy` still returns true (at least now) because it's `&NonCopy`, + // and any `&` is `Copy`. So since we can dereference it in `filter` (since it's then `&&NonCopy`), + // we can lint this and still get the same input type. + // See: + let v = vec![NonCopy, NonCopy]; + v.clone().iter().filter(|&i| (i == &NonCopy)).map(|i| i); + // Do not lint + let v = vec![NonCopy, NonCopy]; + v.clone().into_iter().filter_map(|i| (i == NonCopy).then(|| i)); + // `&mut` is `!Copy`. + let v = vec![NonCopy, NonCopy]; + v.clone().iter_mut().filter_map(|i| (i == &mut NonCopy).then(|| i)); + external! { + let v = vec![1, 2, 3, 4, 5, 6]; + v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1)); + } + with_span! { + span + let v = vec![1, 2, 3, 4, 5, 6]; + v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1)); + } +} + +fn issue11309<'a>(iter: impl Iterator) -> Vec<&'a str> { + iter.filter_map(|(_, s): (&str, _)| Some(s)).collect() +} diff --git a/tests/ui/filter_map_bool_then.rs b/tests/ui/filter_map_bool_then.rs new file mode 100644 index 000000000..7c9b99df7 --- /dev/null +++ b/tests/ui/filter_map_bool_then.rs @@ -0,0 +1,58 @@ +//@run-rustfix +//@aux-build:proc_macros.rs:proc-macro +#![allow( + clippy::clone_on_copy, + clippy::map_identity, + clippy::unnecessary_lazy_evaluations, + clippy::unnecessary_filter_map, + unused +)] +#![warn(clippy::filter_map_bool_then)] + +#[macro_use] +extern crate proc_macros; + +#[derive(Clone, PartialEq)] +struct NonCopy; + +fn main() { + let v = vec![1, 2, 3, 4, 5, 6]; + v.clone().iter().filter_map(|i| (i % 2 == 0).then(|| i + 1)); + v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1)); + v.clone() + .into_iter() + .filter_map(|i| -> Option<_> { (i % 2 == 0).then(|| i + 1) }); + v.clone() + .into_iter() + .filter(|&i| i != 1000) + .filter_map(|i| (i % 2 == 0).then(|| i + 1)); + v.iter() + .copied() + .filter(|&i| i != 1000) + .filter_map(|i| (i.clone() % 2 == 0).then(|| i + 1)); + // Despite this is non-copy, `is_copy` still returns true (at least now) because it's `&NonCopy`, + // and any `&` is `Copy`. So since we can dereference it in `filter` (since it's then `&&NonCopy`), + // we can lint this and still get the same input type. + // See: + let v = vec![NonCopy, NonCopy]; + v.clone().iter().filter_map(|i| (i == &NonCopy).then(|| i)); + // Do not lint + let v = vec![NonCopy, NonCopy]; + v.clone().into_iter().filter_map(|i| (i == NonCopy).then(|| i)); + // `&mut` is `!Copy`. + let v = vec![NonCopy, NonCopy]; + v.clone().iter_mut().filter_map(|i| (i == &mut NonCopy).then(|| i)); + external! { + let v = vec![1, 2, 3, 4, 5, 6]; + v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1)); + } + with_span! { + span + let v = vec![1, 2, 3, 4, 5, 6]; + v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1)); + } +} + +fn issue11309<'a>(iter: impl Iterator) -> Vec<&'a str> { + iter.filter_map(|(_, s): (&str, _)| Some(s)).collect() +} diff --git a/tests/ui/filter_map_bool_then.stderr b/tests/ui/filter_map_bool_then.stderr new file mode 100644 index 000000000..fffa5252e --- /dev/null +++ b/tests/ui/filter_map_bool_then.stderr @@ -0,0 +1,40 @@ +error: usage of `bool::then` in `filter_map` + --> $DIR/filter_map_bool_then.rs:20:22 + | +LL | v.clone().iter().filter_map(|i| (i % 2 == 0).then(|| i + 1)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i % 2 == 0)).map(|i| i + 1)` + | + = note: `-D clippy::filter-map-bool-then` implied by `-D warnings` + +error: usage of `bool::then` in `filter_map` + --> $DIR/filter_map_bool_then.rs:21:27 + | +LL | v.clone().into_iter().filter_map(|i| (i % 2 == 0).then(|| i + 1)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i % 2 == 0)).map(|i| i + 1)` + +error: usage of `bool::then` in `filter_map` + --> $DIR/filter_map_bool_then.rs:24:10 + | +LL | .filter_map(|i| -> Option<_> { (i % 2 == 0).then(|| i + 1) }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i % 2 == 0)).map(|i| i + 1)` + +error: usage of `bool::then` in `filter_map` + --> $DIR/filter_map_bool_then.rs:28:10 + | +LL | .filter_map(|i| (i % 2 == 0).then(|| i + 1)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i % 2 == 0)).map(|i| i + 1)` + +error: usage of `bool::then` in `filter_map` + --> $DIR/filter_map_bool_then.rs:32:10 + | +LL | .filter_map(|i| (i.clone() % 2 == 0).then(|| i + 1)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i.clone() % 2 == 0)).map(|i| i + 1)` + +error: usage of `bool::then` in `filter_map` + --> $DIR/filter_map_bool_then.rs:38:22 + | +LL | v.clone().iter().filter_map(|i| (i == &NonCopy).then(|| i)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&i| (i == &NonCopy)).map(|i| i)` + +error: aborting due to 6 previous errors + diff --git a/tests/ui/get_unwrap.stderr b/tests/ui/get_unwrap.stderr index c567ed319..19dc9071f 100644 --- a/tests/ui/get_unwrap.stderr +++ b/tests/ui/get_unwrap.stderr @@ -16,7 +16,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = boxed_slice.get(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message = note: `-D clippy::unwrap-used` implied by `-D warnings` error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise @@ -31,7 +32,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = some_slice.get(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a Vec. Using `[]` is more clear and more concise --> $DIR/get_unwrap.rs:40:17 @@ -45,7 +47,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = some_vec.get(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a VecDeque. Using `[]` is more clear and more concise --> $DIR/get_unwrap.rs:41:17 @@ -59,7 +62,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = some_vecdeque.get(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a HashMap. Using `[]` is more clear and more concise --> $DIR/get_unwrap.rs:42:17 @@ -73,7 +77,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = some_hashmap.get(&1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a BTreeMap. Using `[]` is more clear and more concise --> $DIR/get_unwrap.rs:43:17 @@ -87,7 +92,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = some_btreemap.get(&1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise --> $DIR/get_unwrap.rs:47:21 @@ -101,7 +107,8 @@ error: used `unwrap()` on an `Option` value LL | let _: u8 = *boxed_slice.get(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise --> $DIR/get_unwrap.rs:52:9 @@ -115,7 +122,8 @@ error: used `unwrap()` on an `Option` value LL | *boxed_slice.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise --> $DIR/get_unwrap.rs:53:9 @@ -129,7 +137,8 @@ error: used `unwrap()` on an `Option` value LL | *some_slice.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get_mut().unwrap()` on a Vec. Using `[]` is more clear and more concise --> $DIR/get_unwrap.rs:54:9 @@ -143,7 +152,8 @@ error: used `unwrap()` on an `Option` value LL | *some_vec.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get_mut().unwrap()` on a VecDeque. Using `[]` is more clear and more concise --> $DIR/get_unwrap.rs:55:9 @@ -157,7 +167,8 @@ error: used `unwrap()` on an `Option` value LL | *some_vecdeque.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a Vec. Using `[]` is more clear and more concise --> $DIR/get_unwrap.rs:67:17 @@ -171,7 +182,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = some_vec.get(0..1).unwrap().to_vec(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get_mut().unwrap()` on a Vec. Using `[]` is more clear and more concise --> $DIR/get_unwrap.rs:68:17 @@ -185,7 +197,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = some_vec.get_mut(0..1).unwrap().to_vec(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise --> $DIR/get_unwrap.rs:78:24 diff --git a/tests/ui/ignored_unit_patterns.fixed b/tests/ui/ignored_unit_patterns.fixed new file mode 100644 index 000000000..492219fe4 --- /dev/null +++ b/tests/ui/ignored_unit_patterns.fixed @@ -0,0 +1,17 @@ +//@run-rustfix + +#![warn(clippy::ignored_unit_patterns)] +#![allow(clippy::redundant_pattern_matching, clippy::single_match)] + +fn foo() -> Result<(), ()> { + unimplemented!() +} + +fn main() { + match foo() { + Ok(()) => {}, + Err(()) => {}, + } + if let Ok(()) = foo() {} + let _ = foo().map_err(|()| todo!()); +} diff --git a/tests/ui/ignored_unit_patterns.rs b/tests/ui/ignored_unit_patterns.rs new file mode 100644 index 000000000..90af36f8e --- /dev/null +++ b/tests/ui/ignored_unit_patterns.rs @@ -0,0 +1,17 @@ +//@run-rustfix + +#![warn(clippy::ignored_unit_patterns)] +#![allow(clippy::redundant_pattern_matching, clippy::single_match)] + +fn foo() -> Result<(), ()> { + unimplemented!() +} + +fn main() { + match foo() { + Ok(_) => {}, + Err(_) => {}, + } + if let Ok(_) = foo() {} + let _ = foo().map_err(|_| todo!()); +} diff --git a/tests/ui/ignored_unit_patterns.stderr b/tests/ui/ignored_unit_patterns.stderr new file mode 100644 index 000000000..8feea3cc2 --- /dev/null +++ b/tests/ui/ignored_unit_patterns.stderr @@ -0,0 +1,28 @@ +error: matching over `()` is more explicit + --> $DIR/ignored_unit_patterns.rs:12:12 + | +LL | Ok(_) => {}, + | ^ help: use `()` instead of `_`: `()` + | + = note: `-D clippy::ignored-unit-patterns` implied by `-D warnings` + +error: matching over `()` is more explicit + --> $DIR/ignored_unit_patterns.rs:13:13 + | +LL | Err(_) => {}, + | ^ help: use `()` instead of `_`: `()` + +error: matching over `()` is more explicit + --> $DIR/ignored_unit_patterns.rs:15:15 + | +LL | if let Ok(_) = foo() {} + | ^ help: use `()` instead of `_`: `()` + +error: matching over `()` is more explicit + --> $DIR/ignored_unit_patterns.rs:16:28 + | +LL | let _ = foo().map_err(|_| todo!()); + | ^ help: use `()` instead of `_`: `()` + +error: aborting due to 4 previous errors + diff --git a/tests/ui/mut_reference.rs b/tests/ui/mut_reference.rs index 73906121c..00661c51a 100644 --- a/tests/ui/mut_reference.rs +++ b/tests/ui/mut_reference.rs @@ -1,8 +1,21 @@ -#![allow(unused_variables)] +#![allow(unused_variables, dead_code)] fn takes_an_immutable_reference(a: &i32) {} fn takes_a_mutable_reference(a: &mut i32) {} +mod issue11268 { + macro_rules! x { + ($f:expr) => { + $f(&mut 1); + }; + } + + fn f() { + x!(super::takes_an_immutable_reference); + x!(super::takes_a_mutable_reference); + } +} + struct MyStruct; impl MyStruct { diff --git a/tests/ui/mut_reference.stderr b/tests/ui/mut_reference.stderr index 1fdfbf922..d8a71d264 100644 --- a/tests/ui/mut_reference.stderr +++ b/tests/ui/mut_reference.stderr @@ -1,5 +1,5 @@ error: the function `takes_an_immutable_reference` doesn't need a mutable reference - --> $DIR/mut_reference.rs:17:34 + --> $DIR/mut_reference.rs:30:34 | LL | takes_an_immutable_reference(&mut 42); | ^^^^^^^ @@ -7,19 +7,19 @@ LL | takes_an_immutable_reference(&mut 42); = note: `-D clippy::unnecessary-mut-passed` implied by `-D warnings` error: the function `as_ptr` doesn't need a mutable reference - --> $DIR/mut_reference.rs:19:12 + --> $DIR/mut_reference.rs:32:12 | LL | as_ptr(&mut 42); | ^^^^^^^ error: the method `takes_an_immutable_reference` doesn't need a mutable reference - --> $DIR/mut_reference.rs:23:44 + --> $DIR/mut_reference.rs:36:44 | LL | my_struct.takes_an_immutable_reference(&mut 42); | ^^^^^^^ error: this argument is a mutable reference, but not used mutably - --> $DIR/mut_reference.rs:11:44 + --> $DIR/mut_reference.rs:24:44 | LL | fn takes_a_mutable_reference(&self, a: &mut i32) {} | ^^^^^^^^ help: consider changing to: `&i32` diff --git a/tests/ui/ptr_as_ptr.fixed b/tests/ui/ptr_as_ptr.fixed index 26a64c861..84babb974 100644 --- a/tests/ui/ptr_as_ptr.fixed +++ b/tests/ui/ptr_as_ptr.fixed @@ -3,8 +3,22 @@ #![warn(clippy::ptr_as_ptr)] +#[macro_use] extern crate proc_macros; -use proc_macros::{external, inline_macros}; + +mod issue_11278_a { + #[derive(Debug)] + pub struct T { + pub p: D, + } +} + +mod issue_11278_b { + pub fn f(o: &mut super::issue_11278_a::T) -> super::issue_11278_a::T { + // Retain `super` + *unsafe { Box::from_raw(Box::into_raw(Box::new(o)).cast::>()) } + } +} #[inline_macros] fn main() { diff --git a/tests/ui/ptr_as_ptr.rs b/tests/ui/ptr_as_ptr.rs index ea40d4947..34fd76428 100644 --- a/tests/ui/ptr_as_ptr.rs +++ b/tests/ui/ptr_as_ptr.rs @@ -3,8 +3,22 @@ #![warn(clippy::ptr_as_ptr)] +#[macro_use] extern crate proc_macros; -use proc_macros::{external, inline_macros}; + +mod issue_11278_a { + #[derive(Debug)] + pub struct T { + pub p: D, + } +} + +mod issue_11278_b { + pub fn f(o: &mut super::issue_11278_a::T) -> super::issue_11278_a::T { + // Retain `super` + *unsafe { Box::from_raw(Box::into_raw(Box::new(o)) as *mut super::issue_11278_a::T) } + } +} #[inline_macros] fn main() { diff --git a/tests/ui/ptr_as_ptr.stderr b/tests/ui/ptr_as_ptr.stderr index 78d733994..e64f33515 100644 --- a/tests/ui/ptr_as_ptr.stderr +++ b/tests/ui/ptr_as_ptr.stderr @@ -1,37 +1,43 @@ error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:14:13 + --> $DIR/ptr_as_ptr.rs:19:33 | -LL | let _ = ptr as *const i32; - | ^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `ptr.cast::()` +LL | *unsafe { Box::from_raw(Box::into_raw(Box::new(o)) as *mut super::issue_11278_a::T) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `Box::into_raw(Box::new(o)).cast::>()` | = note: `-D clippy::ptr-as-ptr` implied by `-D warnings` error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:15:13 + --> $DIR/ptr_as_ptr.rs:28:13 + | +LL | let _ = ptr as *const i32; + | ^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `ptr.cast::()` + +error: `as` casting between raw pointers without changing its mutability + --> $DIR/ptr_as_ptr.rs:29:13 | LL | let _ = mut_ptr as *mut i32; | ^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `mut_ptr.cast::()` error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:20:17 + --> $DIR/ptr_as_ptr.rs:34:17 | LL | let _ = *ptr_ptr as *const i32; | ^^^^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `(*ptr_ptr).cast::()` error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:33:25 + --> $DIR/ptr_as_ptr.rs:47:25 | LL | let _: *const i32 = ptr as *const _; | ^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `ptr.cast()` error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:34:23 + --> $DIR/ptr_as_ptr.rs:48:23 | LL | let _: *mut i32 = mut_ptr as _; | ^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `mut_ptr.cast()` error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:37:21 + --> $DIR/ptr_as_ptr.rs:51:21 | LL | let _ = inline!($ptr as *const i32); | ^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `$ptr.cast::()` @@ -39,16 +45,16 @@ LL | let _ = inline!($ptr as *const i32); = note: this error originates in the macro `__inline_mac_fn_main` (in Nightly builds, run with -Z macro-backtrace for more info) error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:58:13 + --> $DIR/ptr_as_ptr.rs:72:13 | LL | let _ = ptr as *const i32; | ^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `ptr.cast::()` error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:59:13 + --> $DIR/ptr_as_ptr.rs:73:13 | LL | let _ = mut_ptr as *mut i32; | ^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `mut_ptr.cast::()` -error: aborting due to 8 previous errors +error: aborting due to 9 previous errors diff --git a/tests/ui/question_mark.fixed b/tests/ui/question_mark.fixed index 2d8920ccc..20b9e42a7 100644 --- a/tests/ui/question_mark.fixed +++ b/tests/ui/question_mark.fixed @@ -138,6 +138,23 @@ fn result_func(x: Result) -> Result { // no warning let _ = if let Err(e) = x { Err(e) } else { Ok(0) }; + // issue #11283 + // no warning + #[warn(clippy::question_mark_used)] + { + if let Err(err) = Ok(()) { + return Err(err); + } + + if Err::(0).is_err() { + return Err(0); + } else { + return Ok(0); + } + + unreachable!() + } + Ok(y) } diff --git a/tests/ui/question_mark.rs b/tests/ui/question_mark.rs index 69451c17e..8bdafd46e 100644 --- a/tests/ui/question_mark.rs +++ b/tests/ui/question_mark.rs @@ -170,6 +170,23 @@ fn result_func(x: Result) -> Result { // no warning let _ = if let Err(e) = x { Err(e) } else { Ok(0) }; + // issue #11283 + // no warning + #[warn(clippy::question_mark_used)] + { + if let Err(err) = Ok(()) { + return Err(err); + } + + if Err::(0).is_err() { + return Err(0); + } else { + return Ok(0); + } + + unreachable!() + } + Ok(y) } diff --git a/tests/ui/question_mark.stderr b/tests/ui/question_mark.stderr index 2cfd75863..62489c8c8 100644 --- a/tests/ui/question_mark.stderr +++ b/tests/ui/question_mark.stderr @@ -115,7 +115,7 @@ LL | | } | |_____^ help: replace it with: `x?;` error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:197:5 + --> $DIR/question_mark.rs:214:5 | LL | / if let Err(err) = func_returning_result() { LL | | return Err(err); @@ -123,7 +123,7 @@ LL | | } | |_____^ help: replace it with: `func_returning_result()?;` error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:204:5 + --> $DIR/question_mark.rs:221:5 | LL | / if let Err(err) = func_returning_result() { LL | | return Err(err); @@ -131,7 +131,7 @@ LL | | } | |_____^ help: replace it with: `func_returning_result()?;` error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:281:13 + --> $DIR/question_mark.rs:298:13 | LL | / if a.is_none() { LL | | return None; diff --git a/tests/ui/range_contains.fixed b/tests/ui/range_contains.fixed index 0a92ee7c8..47c524811 100644 --- a/tests/ui/range_contains.fixed +++ b/tests/ui/range_contains.fixed @@ -5,6 +5,8 @@ #![allow(clippy::no_effect)] #![allow(clippy::short_circuit_statement)] #![allow(clippy::unnecessary_operation)] +#![allow(clippy::impossible_comparisons)] +#![allow(clippy::redundant_comparisons)] fn main() { let x = 9_i32; diff --git a/tests/ui/range_contains.rs b/tests/ui/range_contains.rs index 7a83be609..a35315a64 100644 --- a/tests/ui/range_contains.rs +++ b/tests/ui/range_contains.rs @@ -5,6 +5,8 @@ #![allow(clippy::no_effect)] #![allow(clippy::short_circuit_statement)] #![allow(clippy::unnecessary_operation)] +#![allow(clippy::impossible_comparisons)] +#![allow(clippy::redundant_comparisons)] fn main() { let x = 9_i32; diff --git a/tests/ui/range_contains.stderr b/tests/ui/range_contains.stderr index ea34023a4..1265db695 100644 --- a/tests/ui/range_contains.stderr +++ b/tests/ui/range_contains.stderr @@ -1,5 +1,5 @@ error: manual `Range::contains` implementation - --> $DIR/range_contains.rs:13:5 + --> $DIR/range_contains.rs:15:5 | LL | x >= 8 && x < 12; | ^^^^^^^^^^^^^^^^ help: use: `(8..12).contains(&x)` @@ -7,121 +7,121 @@ LL | x >= 8 && x < 12; = note: `-D clippy::manual-range-contains` implied by `-D warnings` error: manual `Range::contains` implementation - --> $DIR/range_contains.rs:14:5 + --> $DIR/range_contains.rs:16:5 | LL | x < 42 && x >= 21; | ^^^^^^^^^^^^^^^^^ help: use: `(21..42).contains(&x)` error: manual `Range::contains` implementation - --> $DIR/range_contains.rs:15:5 + --> $DIR/range_contains.rs:17:5 | LL | 100 > x && 1 <= x; | ^^^^^^^^^^^^^^^^^ help: use: `(1..100).contains(&x)` error: manual `RangeInclusive::contains` implementation - --> $DIR/range_contains.rs:18:5 + --> $DIR/range_contains.rs:20:5 | LL | x >= 9 && x <= 99; | ^^^^^^^^^^^^^^^^^ help: use: `(9..=99).contains(&x)` error: manual `RangeInclusive::contains` implementation - --> $DIR/range_contains.rs:19:5 + --> $DIR/range_contains.rs:21:5 | LL | x <= 33 && x >= 1; | ^^^^^^^^^^^^^^^^^ help: use: `(1..=33).contains(&x)` error: manual `RangeInclusive::contains` implementation - --> $DIR/range_contains.rs:20:5 + --> $DIR/range_contains.rs:22:5 | LL | 999 >= x && 1 <= x; | ^^^^^^^^^^^^^^^^^^ help: use: `(1..=999).contains(&x)` error: manual `!Range::contains` implementation - --> $DIR/range_contains.rs:23:5 + --> $DIR/range_contains.rs:25:5 | LL | x < 8 || x >= 12; | ^^^^^^^^^^^^^^^^ help: use: `!(8..12).contains(&x)` error: manual `!Range::contains` implementation - --> $DIR/range_contains.rs:24:5 + --> $DIR/range_contains.rs:26:5 | LL | x >= 42 || x < 21; | ^^^^^^^^^^^^^^^^^ help: use: `!(21..42).contains(&x)` error: manual `!Range::contains` implementation - --> $DIR/range_contains.rs:25:5 + --> $DIR/range_contains.rs:27:5 | LL | 100 <= x || 1 > x; | ^^^^^^^^^^^^^^^^^ help: use: `!(1..100).contains(&x)` error: manual `!RangeInclusive::contains` implementation - --> $DIR/range_contains.rs:28:5 + --> $DIR/range_contains.rs:30:5 | LL | x < 9 || x > 99; | ^^^^^^^^^^^^^^^ help: use: `!(9..=99).contains(&x)` error: manual `!RangeInclusive::contains` implementation - --> $DIR/range_contains.rs:29:5 + --> $DIR/range_contains.rs:31:5 | LL | x > 33 || x < 1; | ^^^^^^^^^^^^^^^ help: use: `!(1..=33).contains(&x)` error: manual `!RangeInclusive::contains` implementation - --> $DIR/range_contains.rs:30:5 + --> $DIR/range_contains.rs:32:5 | LL | 999 < x || 1 > x; | ^^^^^^^^^^^^^^^^ help: use: `!(1..=999).contains(&x)` error: manual `Range::contains` implementation - --> $DIR/range_contains.rs:45:5 + --> $DIR/range_contains.rs:47:5 | LL | y >= 0. && y < 1.; | ^^^^^^^^^^^^^^^^^ help: use: `(0. ..1.).contains(&y)` error: manual `!RangeInclusive::contains` implementation - --> $DIR/range_contains.rs:46:5 + --> $DIR/range_contains.rs:48:5 | LL | y < 0. || y > 1.; | ^^^^^^^^^^^^^^^^ help: use: `!(0. ..=1.).contains(&y)` error: manual `RangeInclusive::contains` implementation - --> $DIR/range_contains.rs:49:5 + --> $DIR/range_contains.rs:51:5 | LL | x >= -10 && x <= 10; | ^^^^^^^^^^^^^^^^^^^ help: use: `(-10..=10).contains(&x)` error: manual `RangeInclusive::contains` implementation - --> $DIR/range_contains.rs:51:5 + --> $DIR/range_contains.rs:53:5 | LL | y >= -3. && y <= 3.; | ^^^^^^^^^^^^^^^^^^^ help: use: `(-3. ..=3.).contains(&y)` error: manual `RangeInclusive::contains` implementation - --> $DIR/range_contains.rs:56:30 + --> $DIR/range_contains.rs:58:30 | LL | (x >= 0) && (x <= 10) && (z >= 0) && (z <= 10); | ^^^^^^^^^^^^^^^^^^^^^ help: use: `(0..=10).contains(&z)` error: manual `RangeInclusive::contains` implementation - --> $DIR/range_contains.rs:56:5 + --> $DIR/range_contains.rs:58:5 | LL | (x >= 0) && (x <= 10) && (z >= 0) && (z <= 10); | ^^^^^^^^^^^^^^^^^^^^^ help: use: `(0..=10).contains(&x)` error: manual `!Range::contains` implementation - --> $DIR/range_contains.rs:57:29 + --> $DIR/range_contains.rs:59:29 | LL | (x < 0) || (x >= 10) || (z < 0) || (z >= 10); | ^^^^^^^^^^^^^^^^^^^^ help: use: `!(0..10).contains(&z)` error: manual `!Range::contains` implementation - --> $DIR/range_contains.rs:57:5 + --> $DIR/range_contains.rs:59:5 | LL | (x < 0) || (x >= 10) || (z < 0) || (z >= 10); | ^^^^^^^^^^^^^^^^^^^^ help: use: `!(0..10).contains(&x)` error: manual `Range::contains` implementation - --> $DIR/range_contains.rs:76:5 + --> $DIR/range_contains.rs:78:5 | LL | x >= 8 && x < 35; | ^^^^^^^^^^^^^^^^ help: use: `(8..35).contains(&x)` diff --git a/tests/ui/redundant_guards.fixed b/tests/ui/redundant_guards.fixed index 77ac76668..49d7336ee 100644 --- a/tests/ui/redundant_guards.fixed +++ b/tests/ui/redundant_guards.fixed @@ -15,6 +15,19 @@ struct B { struct C(u32, u32); +#[derive(PartialEq)] +struct FloatWrapper(f32); +fn issue11304() { + match 0.1 { + x if x == 0.0 => todo!(), + _ => todo!(), + } + match FloatWrapper(0.1) { + x if x == FloatWrapper(0.0) => todo!(), + _ => todo!(), + } +} + fn main() { let c = C(1, 2); match c { diff --git a/tests/ui/redundant_guards.rs b/tests/ui/redundant_guards.rs index b072e4ea1..87761010d 100644 --- a/tests/ui/redundant_guards.rs +++ b/tests/ui/redundant_guards.rs @@ -15,6 +15,19 @@ struct B { struct C(u32, u32); +#[derive(PartialEq)] +struct FloatWrapper(f32); +fn issue11304() { + match 0.1 { + x if x == 0.0 => todo!(), + _ => todo!(), + } + match FloatWrapper(0.1) { + x if x == FloatWrapper(0.0) => todo!(), + _ => todo!(), + } +} + fn main() { let c = C(1, 2); match c { diff --git a/tests/ui/redundant_guards.stderr b/tests/ui/redundant_guards.stderr index c2a92071d..5bdf43d23 100644 --- a/tests/ui/redundant_guards.stderr +++ b/tests/ui/redundant_guards.stderr @@ -1,5 +1,5 @@ error: redundant guard - --> $DIR/redundant_guards.rs:21:20 + --> $DIR/redundant_guards.rs:34:20 | LL | C(x, y) if let 1 = y => .., | ^^^^^^^^^ @@ -12,7 +12,7 @@ LL + C(x, 1) => .., | error: redundant guard - --> $DIR/redundant_guards.rs:27:20 + --> $DIR/redundant_guards.rs:40:20 | LL | Some(x) if matches!(x, Some(1) if true) => .., | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -23,7 +23,7 @@ LL | Some(Some(1)) if true => .., | ~~~~~~~ ~~~~~~~ error: redundant guard - --> $DIR/redundant_guards.rs:28:20 + --> $DIR/redundant_guards.rs:41:20 | LL | Some(x) if matches!(x, Some(1)) => { | ^^^^^^^^^^^^^^^^^^^^ @@ -35,7 +35,7 @@ LL + Some(Some(1)) => { | error: redundant guard - --> $DIR/redundant_guards.rs:32:20 + --> $DIR/redundant_guards.rs:45:20 | LL | Some(x) if let Some(1) = x => .., | ^^^^^^^^^^^^^^^ @@ -47,7 +47,7 @@ LL + Some(Some(1)) => .., | error: redundant guard - --> $DIR/redundant_guards.rs:33:20 + --> $DIR/redundant_guards.rs:46:20 | LL | Some(x) if x == Some(2) => .., | ^^^^^^^^^^^^ @@ -59,7 +59,7 @@ LL + Some(Some(2)) => .., | error: redundant guard - --> $DIR/redundant_guards.rs:56:20 + --> $DIR/redundant_guards.rs:69:20 | LL | B { e } if matches!(e, Some(A(2))) => .., | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -71,7 +71,7 @@ LL + B { e: Some(A(2)) } => .., | error: redundant guard - --> $DIR/redundant_guards.rs:93:20 + --> $DIR/redundant_guards.rs:106:20 | LL | E::A(y) if y == "not from an or pattern" => {}, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -83,7 +83,7 @@ LL + E::A("not from an or pattern") => {}, | error: redundant guard - --> $DIR/redundant_guards.rs:100:14 + --> $DIR/redundant_guards.rs:113:14 | LL | x if matches!(x, Some(0)) => .., | ^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/redundant_locals.rs b/tests/ui/redundant_locals.rs index e74c78a50..80af38f47 100644 --- a/tests/ui/redundant_locals.rs +++ b/tests/ui/redundant_locals.rs @@ -27,6 +27,15 @@ fn downgraded_mutability() { let x = x; } +// see #11290 +fn shadow_mutation() { + let mut x = 1; + { + let mut x = x; + x = 2; + } +} + fn coercion(par: &mut i32) { let par: &i32 = par; diff --git a/tests/ui/redundant_locals.stderr b/tests/ui/redundant_locals.stderr index df07dd2f4..587de0575 100644 --- a/tests/ui/redundant_locals.stderr +++ b/tests/ui/redundant_locals.stderr @@ -20,7 +20,7 @@ LL | let mut x = x; = help: remove the redefinition of `x` error: redundant redefinition of a binding - --> $DIR/redundant_locals.rs:37:14 + --> $DIR/redundant_locals.rs:46:14 | LL | fn parameter(x: i32) { | ^ @@ -30,7 +30,7 @@ LL | let x = x; = help: remove the redefinition of `x` error: redundant redefinition of a binding - --> $DIR/redundant_locals.rs:42:9 + --> $DIR/redundant_locals.rs:51:9 | LL | let x = 1; | ^ @@ -40,7 +40,7 @@ LL | let x = x; = help: remove the redefinition of `x` error: redundant redefinition of a binding - --> $DIR/redundant_locals.rs:43:9 + --> $DIR/redundant_locals.rs:52:9 | LL | let x = x; | ^ @@ -50,7 +50,7 @@ LL | let x = x; = help: remove the redefinition of `x` error: redundant redefinition of a binding - --> $DIR/redundant_locals.rs:44:9 + --> $DIR/redundant_locals.rs:53:9 | LL | let x = x; | ^ @@ -60,7 +60,7 @@ LL | let x = x; = help: remove the redefinition of `x` error: redundant redefinition of a binding - --> $DIR/redundant_locals.rs:45:9 + --> $DIR/redundant_locals.rs:54:9 | LL | let x = x; | ^ @@ -70,7 +70,7 @@ LL | let x = x; = help: remove the redefinition of `x` error: redundant redefinition of a binding - --> $DIR/redundant_locals.rs:50:9 + --> $DIR/redundant_locals.rs:59:9 | LL | let a = 1; | ^ @@ -81,7 +81,7 @@ LL | let a = a; = help: remove the redefinition of `a` error: redundant redefinition of a binding - --> $DIR/redundant_locals.rs:51:9 + --> $DIR/redundant_locals.rs:60:9 | LL | let b = 2; | ^ @@ -92,7 +92,7 @@ LL | let b = b; = help: remove the redefinition of `b` error: redundant redefinition of a binding - --> $DIR/redundant_locals.rs:58:13 + --> $DIR/redundant_locals.rs:67:13 | LL | let x = 1; | ^ @@ -102,7 +102,7 @@ LL | let x = x; = help: remove the redefinition of `x` error: redundant redefinition of a binding - --> $DIR/redundant_locals.rs:65:13 + --> $DIR/redundant_locals.rs:74:13 | LL | let x = 1; | ^ @@ -112,7 +112,7 @@ LL | let x = x; = help: remove the redefinition of `x` error: redundant redefinition of a binding - --> $DIR/redundant_locals.rs:68:6 + --> $DIR/redundant_locals.rs:77:6 | LL | |x: i32| { | ^ @@ -122,7 +122,7 @@ LL | let x = x; = help: remove the redefinition of `x` error: redundant redefinition of a binding - --> $DIR/redundant_locals.rs:85:9 + --> $DIR/redundant_locals.rs:94:9 | LL | let x = 1; | ^ diff --git a/tests/ui/redundant_type_annotations.rs b/tests/ui/redundant_type_annotations.rs index cc507b8d6..09dbd3c9b 100644 --- a/tests/ui/redundant_type_annotations.rs +++ b/tests/ui/redundant_type_annotations.rs @@ -6,8 +6,8 @@ struct Cake { _data: T, } -fn make_something() -> T { - T::default() +fn make_something() -> T { + unimplemented!() } fn make_cake() -> Cake { @@ -117,7 +117,15 @@ fn test_non_locals() { let _closure_arg = |x: u32| x; } -fn test_complex_types() { +trait Trait { + type AssocTy; +} + +impl Trait for () { + type AssocTy = String; +} + +fn test_complex_types() { // Shouldn't be lint, since the literal will be i32 otherwise let _u8: u8 = 128; @@ -135,6 +143,10 @@ fn test_complex_types() { // Shouldn't be lint let _array: [u32; 2] = [8, 9]; + + let ty_param: T = make_something(); + + let assoc_ty: <() as Trait>::AssocTy = String::new(); } fn test_functions() { @@ -173,4 +185,6 @@ fn test_simple_types() { let _var: bool = false; } +fn issue11190() {} + fn main() {} diff --git a/tests/ui/redundant_type_annotations.stderr b/tests/ui/redundant_type_annotations.stderr index e8b2fe5c3..988ebe637 100644 --- a/tests/ui/redundant_type_annotations.stderr +++ b/tests/ui/redundant_type_annotations.stderr @@ -19,85 +19,85 @@ LL | let v: &Slice = self.return_a_ref_to_struct(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: redundant type annotation - --> $DIR/redundant_type_annotations.rs:143:5 + --> $DIR/redundant_type_annotations.rs:155:5 | LL | let _return: String = return_a_string(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: redundant type annotation - --> $DIR/redundant_type_annotations.rs:145:5 + --> $DIR/redundant_type_annotations.rs:157:5 | LL | let _return: Pie = return_a_struct(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: redundant type annotation - --> $DIR/redundant_type_annotations.rs:147:5 + --> $DIR/redundant_type_annotations.rs:159:5 | LL | let _return: Pizza = return_an_enum(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: redundant type annotation - --> $DIR/redundant_type_annotations.rs:149:5 + --> $DIR/redundant_type_annotations.rs:161:5 | LL | let _return: u32 = return_an_int(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: redundant type annotation - --> $DIR/redundant_type_annotations.rs:151:5 + --> $DIR/redundant_type_annotations.rs:163:5 | LL | let _return: String = String::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: redundant type annotation - --> $DIR/redundant_type_annotations.rs:153:5 + --> $DIR/redundant_type_annotations.rs:165:5 | LL | let new_pie: Pie = Pie::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: redundant type annotation - --> $DIR/redundant_type_annotations.rs:155:5 + --> $DIR/redundant_type_annotations.rs:167:5 | LL | let _return: u32 = new_pie.return_an_int(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: redundant type annotation - --> $DIR/redundant_type_annotations.rs:157:5 + --> $DIR/redundant_type_annotations.rs:169:5 | LL | let _return: u32 = Pie::associated_return_an_int(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: redundant type annotation - --> $DIR/redundant_type_annotations.rs:159:5 + --> $DIR/redundant_type_annotations.rs:171:5 | LL | let _return: String = Pie::associated_return_a_string(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: redundant type annotation - --> $DIR/redundant_type_annotations.rs:165:5 + --> $DIR/redundant_type_annotations.rs:177:5 | LL | let _var: u32 = u32::MAX; | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: redundant type annotation - --> $DIR/redundant_type_annotations.rs:167:5 + --> $DIR/redundant_type_annotations.rs:179:5 | LL | let _var: u32 = 5_u32; | ^^^^^^^^^^^^^^^^^^^^^^ error: redundant type annotation - --> $DIR/redundant_type_annotations.rs:169:5 + --> $DIR/redundant_type_annotations.rs:181:5 | LL | let _var: &str = "test"; | ^^^^^^^^^^^^^^^^^^^^^^^^ error: redundant type annotation - --> $DIR/redundant_type_annotations.rs:171:5 + --> $DIR/redundant_type_annotations.rs:183:5 | LL | let _var: &[u8] = b"test"; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: redundant type annotation - --> $DIR/redundant_type_annotations.rs:173:5 + --> $DIR/redundant_type_annotations.rs:185:5 | LL | let _var: bool = false; | ^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/rename.fixed b/tests/ui/rename.fixed index 8257bf294..e78b9e5c9 100644 --- a/tests/ui/rename.fixed +++ b/tests/ui/rename.fixed @@ -29,9 +29,9 @@ #![allow(clippy::recursive_format_impl)] #![allow(clippy::unwrap_or_default)] #![allow(clippy::invisible_characters)] +#![allow(invalid_reference_casting)] #![allow(suspicious_double_ref_op)] #![allow(invalid_nan_comparisons)] -#![allow(invalid_reference_casting)] #![allow(drop_bounds)] #![allow(dropping_copy_types)] #![allow(dropping_references)] diff --git a/tests/ui/rename.rs b/tests/ui/rename.rs index 6569dad18..2e6ef60cb 100644 --- a/tests/ui/rename.rs +++ b/tests/ui/rename.rs @@ -29,9 +29,9 @@ #![allow(clippy::recursive_format_impl)] #![allow(clippy::unwrap_or_default)] #![allow(clippy::invisible_characters)] +#![allow(invalid_reference_casting)] #![allow(suspicious_double_ref_op)] #![allow(invalid_nan_comparisons)] -#![allow(invalid_reference_casting)] #![allow(drop_bounds)] #![allow(dropping_copy_types)] #![allow(dropping_references)] diff --git a/tests/ui/suspicious_xor_used_as_pow.stderr b/tests/ui/suspicious_xor_used_as_pow.stderr index 8bb3c8fbe..d93a55ba9 100644 --- a/tests/ui/suspicious_xor_used_as_pow.stderr +++ b/tests/ui/suspicious_xor_used_as_pow.stderr @@ -10,31 +10,31 @@ error: `^` is not the exponentiation operator --> $DIR/suspicious_xor_used_as_pow.rs:20:13 | LL | let _ = 2i32 ^ 9i32; - | ^^^^^^^^^^^ help: did you mean to write: `2_i32.pow(9_i32)` + | ^^^^^^^^^^^ help: did you mean to write: `2i32.pow(9i32)` error: `^` is not the exponentiation operator --> $DIR/suspicious_xor_used_as_pow.rs:21:13 | LL | let _ = 2i32 ^ 2i32; - | ^^^^^^^^^^^ help: did you mean to write: `2_i32.pow(2_i32)` + | ^^^^^^^^^^^ help: did you mean to write: `2i32.pow(2i32)` error: `^` is not the exponentiation operator --> $DIR/suspicious_xor_used_as_pow.rs:22:13 | LL | let _ = 50i32 ^ 3i32; - | ^^^^^^^^^^^^ help: did you mean to write: `50_i32.pow(3_i32)` + | ^^^^^^^^^^^^ help: did you mean to write: `50i32.pow(3i32)` error: `^` is not the exponentiation operator --> $DIR/suspicious_xor_used_as_pow.rs:23:13 | LL | let _ = 5i32 ^ 8i32; - | ^^^^^^^^^^^ help: did you mean to write: `5_i32.pow(8_i32)` + | ^^^^^^^^^^^ help: did you mean to write: `5i32.pow(8i32)` error: `^` is not the exponentiation operator --> $DIR/suspicious_xor_used_as_pow.rs:24:13 | LL | let _ = 2i32 ^ 32i32; - | ^^^^^^^^^^^^ help: did you mean to write: `2_i32.pow(32_i32)` + | ^^^^^^^^^^^^ help: did you mean to write: `2i32.pow(32i32)` error: `^` is not the exponentiation operator --> $DIR/suspicious_xor_used_as_pow.rs:13:9 diff --git a/tests/ui/unwrap.stderr b/tests/ui/unwrap.stderr index 3796d942f..41db819f6 100644 --- a/tests/ui/unwrap.stderr +++ b/tests/ui/unwrap.stderr @@ -4,7 +4,8 @@ error: used `unwrap()` on an `Option` value LL | let _ = opt.unwrap(); | ^^^^^^^^^^^^ | - = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is `None`, it will panic + = help: consider using `expect()` to provide a better panic message = note: `-D clippy::unwrap-used` implied by `-D warnings` error: used `unwrap()` on a `Result` value @@ -13,7 +14,8 @@ error: used `unwrap()` on a `Result` value LL | let _ = res.unwrap(); | ^^^^^^^^^^^^ | - = help: if you don't want to handle the `Err` case gracefully, consider using `expect()` to provide a better panic message + = note: if this value is an `Err`, it will panic + = help: consider using `expect()` to provide a better panic message error: used `unwrap_err()` on a `Result` value --> $DIR/unwrap.rs:12:13 @@ -21,7 +23,8 @@ error: used `unwrap_err()` on a `Result` value LL | let _ = res.unwrap_err(); | ^^^^^^^^^^^^^^^^ | - = help: if you don't want to handle the `Ok` case gracefully, consider using `expect_err()` to provide a better panic message + = note: if this value is an `Ok`, it will panic + = help: consider using `expect_err()` to provide a better panic message error: aborting due to 3 previous errors diff --git a/tests/ui/unwrap_expect_used.rs b/tests/ui/unwrap_expect_used.rs index 7f57efc53..26f92ccde 100644 --- a/tests/ui/unwrap_expect_used.rs +++ b/tests/ui/unwrap_expect_used.rs @@ -1,5 +1,8 @@ #![warn(clippy::unwrap_used, clippy::expect_used)] #![allow(clippy::unnecessary_literal_unwrap)] +#![feature(never_type)] + +use std::convert::Infallible; trait OptionExt { type Item; @@ -28,6 +31,14 @@ fn main() { Some(3).unwrap_err(); Some(3).expect_err("Hellow none!"); + // Issue #11245: The `Err` variant can never be constructed so do not lint this. + let x: Result<(), !> = Ok(()); + x.unwrap(); + x.expect("is `!` (never)"); + let x: Result<(), Infallible> = Ok(()); + x.unwrap(); + x.expect("is never-like (0 variants)"); + let a: Result = Ok(3); a.unwrap(); a.expect("Hello world!"); diff --git a/tests/ui/unwrap_expect_used.stderr b/tests/ui/unwrap_expect_used.stderr index 1a551ab5a..f66e47612 100644 --- a/tests/ui/unwrap_expect_used.stderr +++ b/tests/ui/unwrap_expect_used.stderr @@ -1,52 +1,52 @@ error: used `unwrap()` on an `Option` value - --> $DIR/unwrap_expect_used.rs:24:5 + --> $DIR/unwrap_expect_used.rs:27:5 | LL | Some(3).unwrap(); | ^^^^^^^^^^^^^^^^ | - = help: if this value is `None`, it will panic + = note: if this value is `None`, it will panic = note: `-D clippy::unwrap-used` implied by `-D warnings` error: used `expect()` on an `Option` value - --> $DIR/unwrap_expect_used.rs:25:5 + --> $DIR/unwrap_expect_used.rs:28:5 | LL | Some(3).expect("Hello world!"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if this value is `None`, it will panic + = note: if this value is `None`, it will panic = note: `-D clippy::expect-used` implied by `-D warnings` error: used `unwrap()` on a `Result` value - --> $DIR/unwrap_expect_used.rs:32:5 + --> $DIR/unwrap_expect_used.rs:43:5 | LL | a.unwrap(); | ^^^^^^^^^^ | - = help: if this value is an `Err`, it will panic + = note: if this value is an `Err`, it will panic error: used `expect()` on a `Result` value - --> $DIR/unwrap_expect_used.rs:33:5 + --> $DIR/unwrap_expect_used.rs:44:5 | LL | a.expect("Hello world!"); | ^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if this value is an `Err`, it will panic + = note: if this value is an `Err`, it will panic error: used `unwrap_err()` on a `Result` value - --> $DIR/unwrap_expect_used.rs:34:5 + --> $DIR/unwrap_expect_used.rs:45:5 | LL | a.unwrap_err(); | ^^^^^^^^^^^^^^ | - = help: if this value is an `Ok`, it will panic + = note: if this value is an `Ok`, it will panic error: used `expect_err()` on a `Result` value - --> $DIR/unwrap_expect_used.rs:35:5 + --> $DIR/unwrap_expect_used.rs:46:5 | LL | a.expect_err("Hello error!"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: if this value is an `Ok`, it will panic + = note: if this value is an `Ok`, it will panic error: aborting due to 6 previous errors