diff --git a/src/tools/clippy/CHANGELOG.md b/src/tools/clippy/CHANGELOG.md index 7c798a3c2e5a..45bfb65b2e67 100644 --- a/src/tools/clippy/CHANGELOG.md +++ b/src/tools/clippy/CHANGELOG.md @@ -6,7 +6,103 @@ document. ## Unreleased / Beta / In Rust Nightly -[92b4b68...master](https://github.com/rust-lang/rust-clippy/compare/92b4b68...master) +[500e0ff...master](https://github.com/rust-lang/rust-clippy/compare/500e0ff...master) + +## Rust 1.94 + +Current stable, released 2026-03-05 + +[View all 94 merged pull requests](https://github.com/rust-lang/rust-clippy/pulls?q=merged%3A2025-11-29T21%3A01%3A29Z..2026-01-08T20%3A33%3A22Z+base%3Amaster) + +### New Lints + +* Added [`same_length_and_capacity`] to `pedantic` + [#15656](https://github.com/rust-lang/rust-clippy/pull/15656) +* Added [`manual_ilog2`] to `pedantic` + [#15865](https://github.com/rust-lang/rust-clippy/pull/15865) +* Added [`needless_type_cast`] to `nursery` + [#16139](https://github.com/rust-lang/rust-clippy/pull/16139) +* Added [`ptr_offset_by_literal`] to `pedantic` + [#15606](https://github.com/rust-lang/rust-clippy/pull/15606) +* Added [`decimal_bitwise_operands`] to `pedantic` + [#15215](https://github.com/rust-lang/rust-clippy/pull/15215) + +### Moves and Deprecations + +* Moved [`multiple_bound_locations`] from `suspicious` to `style` + [#16302](https://github.com/rust-lang/rust-clippy/pull/16302) +* Moved [`collapsible_else_if`] from `style` to `pedantic` + [#16211](https://github.com/rust-lang/rust-clippy/pull/16211) +* Moved [`needless_type_cast`] from `pedantic` to `nursery` + [#16246](https://github.com/rust-lang/rust-clippy/pull/16246) + +### Enhancements + +* [`never_loop`] do not consider `return` as preventing the iterator from looping; lint diverging + iterator reduction closures like `for_each` and `fold` + [#16364](https://github.com/rust-lang/rust-clippy/pull/16364) +* [`single_range_in_vec_init`] don't apply the suggestion automatically + [#16365](https://github.com/rust-lang/rust-clippy/pull/16365) +* [`useless_conversion`] refine `.into_iter()` suggestions to stop at final target type + [#16238](https://github.com/rust-lang/rust-clippy/pull/16238) +* Multiple lints fix wrongly unmangled macros + [#16337](https://github.com/rust-lang/rust-clippy/pull/16337) +* [`large_stack_arrays`] do not warn for libtest harness + [#16347](https://github.com/rust-lang/rust-clippy/pull/16347) +* [`derive_ord_xor_partial_ord`] allow `expect` on `impl` block + [#16303](https://github.com/rust-lang/rust-clippy/pull/16303) +* [`match_bool`] restrict to 2 arms + [#16333](https://github.com/rust-lang/rust-clippy/pull/16333) +* [`multiple_inherent_impl`] fix false negatives for generic impl blocks + [#16284](https://github.com/rust-lang/rust-clippy/pull/16284) +* [`unnecessary_fold`] warn about semantics change and lint `Add::add`/`Mul::mul` folds + [#16324](https://github.com/rust-lang/rust-clippy/pull/16324) +* [`transmuting_null`] check const blocks and const integer casts + [#16260](https://github.com/rust-lang/rust-clippy/pull/16260) +* [`needless_pass_by_ref_mut`] preserve user-provided lifetime information + [#16273](https://github.com/rust-lang/rust-clippy/pull/16273) +* [`while_let_on_iterator`] use reborrow for non-`Sized` trait references + [#16100](https://github.com/rust-lang/rust-clippy/pull/16100) +* [`collapsible_else_if`] prevent emitting when arms only `if {..} else {..}` + [#16286](https://github.com/rust-lang/rust-clippy/pull/16286) +* [`multiple_unsafe_ops_per_block`] count only towards innermost unsafe block + [#16117](https://github.com/rust-lang/rust-clippy/pull/16117) +* [`manual_saturating_arithmetic`] lint `x.checked_sub(y).unwrap_or_default()` + [#15845](https://github.com/rust-lang/rust-clippy/pull/15845) +* [`transmute_ptr_to_ref`] handle pointer in struct + [#15948](https://github.com/rust-lang/rust-clippy/pull/15948) +* [`disallowed_methods`] skip compiler-generated code + [#16186](https://github.com/rust-lang/rust-clippy/pull/16186) +* [`missing_enforced_import_renames`] do not enforce for "as _" + [#16352](https://github.com/rust-lang/rust-clippy/pull/16352) + +### False Positive Fixes + +* [`double_parens`] fix FP on macro repetition patterns + [#16301](https://github.com/rust-lang/rust-clippy/pull/16301) +* [`assertions_on_constants`] fix false positive when there is non-constant value in condition expr + [#16297](https://github.com/rust-lang/rust-clippy/pull/16297) +* [`use_self`] fix FP on type in const generics + [#16172](https://github.com/rust-lang/rust-clippy/pull/16172) +* [`set_contains_or_insert`] fix FP when set is mutated before `insert` + [#16009](https://github.com/rust-lang/rust-clippy/pull/16009) +* [`if_then_some_else_none`] fix FP when then block contains `await` + [#16178](https://github.com/rust-lang/rust-clippy/pull/16178) +* [`match_like_matches_macro`] fix FP with guards containing `if let` + [#15876](https://github.com/rust-lang/rust-clippy/pull/15876) +* [`tuple_array_conversions`] fix FP when binded vars are used before conversion + [#16197](https://github.com/rust-lang/rust-clippy/pull/16197) +* [`map_entry`] fix FP when it would cause `MutexGuard` to be held across await + [#16199](https://github.com/rust-lang/rust-clippy/pull/16199) +* [`panicking_unwrap`] fix FP on field access with implicit deref + [#16196](https://github.com/rust-lang/rust-clippy/pull/16196) +* [`large_stack_frames`] fix FP on compiler generated targets + [#15101](https://github.com/rust-lang/rust-clippy/pull/15101) + +### ICE Fixes + +* [`needless_type_cast`] do not ICE on struct constructor + [#16245](https://github.com/rust-lang/rust-clippy/pull/16245) ### New Lints @@ -6705,6 +6801,7 @@ Released 2018-09-13 [`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or [`manual_option_as_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_option_as_slice [`manual_pattern_char_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_pattern_char_comparison +[`manual_pop_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_pop_if [`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains [`manual_range_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_patterns [`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid diff --git a/src/tools/clippy/clippy.toml b/src/tools/clippy/clippy.toml index 4aa0a426e512..6f9db8da6c71 100644 --- a/src/tools/clippy/clippy.toml +++ b/src/tools/clippy/clippy.toml @@ -5,11 +5,11 @@ check-inconsistent-struct-field-initializers = true lint-commented-code = true [[disallowed-methods]] -path = "rustc_lint::context::LintContext::lint" +path = "rustc_lint::context::LintContext::opt_span_lint" reason = "this function does not add a link to our documentation; please use the `clippy_utils::diagnostics::span_lint*` functions instead" [[disallowed-methods]] -path = "rustc_lint::context::LintContext::span_lint" +path = "rustc_lint::context::LintContext::emit_span_lint" reason = "this function does not add a link to our documentation; please use the `clippy_utils::diagnostics::span_lint*` functions instead" [[disallowed-methods]] diff --git a/src/tools/clippy/clippy_dev/src/lib.rs b/src/tools/clippy/clippy_dev/src/lib.rs index 359bf17c68c5..99709ed2e4f2 100644 --- a/src/tools/clippy/clippy_dev/src/lib.rs +++ b/src/tools/clippy/clippy_dev/src/lib.rs @@ -5,8 +5,7 @@ os_str_slice, os_string_truncate, pattern, - rustc_private, - slice_split_once + rustc_private )] #![warn( trivial_casts, diff --git a/src/tools/clippy/clippy_lints/src/bool_to_int_with_if.rs b/src/tools/clippy/clippy_lints/src/bool_to_int_with_if.rs index b98a20a90ccb..8b45473687e6 100644 --- a/src/tools/clippy/clippy_lints/src/bool_to_int_with_if.rs +++ b/src/tools/clippy/clippy_lints/src/bool_to_int_with_if.rs @@ -1,5 +1,4 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::source::HasSession; use clippy_utils::sugg::Sugg; use clippy_utils::{higher, is_else_clause, is_in_const_context, span_contains_comment}; use rustc_ast::LitKind; @@ -60,7 +59,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { && !is_in_const_context(cx) { let ty = cx.typeck_results().expr_ty(then); - let mut applicability = if span_contains_comment(cx.sess().source_map(), expr.span) { + let mut applicability = if span_contains_comment(cx, expr.span) { Applicability::MaybeIncorrect } else { Applicability::MachineApplicable diff --git a/src/tools/clippy/clippy_lints/src/casts/mod.rs b/src/tools/clippy/clippy_lints/src/casts/mod.rs index 75761de4ae73..151f9c956c6d 100644 --- a/src/tools/clippy/clippy_lints/src/casts/mod.rs +++ b/src/tools/clippy/clippy_lints/src/casts/mod.rs @@ -691,7 +691,7 @@ /// const SIZE: usize = 15; /// let arr: [u8; SIZE] = [0; SIZE]; /// ``` - #[clippy::version = "1.93.0"] + #[clippy::version = "1.94.0"] pub NEEDLESS_TYPE_CAST, nursery, "binding defined with one type but always cast to another" diff --git a/src/tools/clippy/clippy_lints/src/collapsible_if.rs b/src/tools/clippy/clippy_lints/src/collapsible_if.rs index a76027caebc8..3850c55c49f8 100644 --- a/src/tools/clippy/clippy_lints/src/collapsible_if.rs +++ b/src/tools/clippy/clippy_lints/src/collapsible_if.rs @@ -1,7 +1,7 @@ use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_hir_and_then; use clippy_utils::msrvs::Msrv; -use clippy_utils::source::{IntoSpan as _, SpanRangeExt, snippet, snippet_block_with_applicability}; +use clippy_utils::source::{HasSession, IntoSpan as _, SpanRangeExt, snippet, snippet_block_with_applicability}; use clippy_utils::{can_use_if_let_chains, span_contains_non_whitespace, sym, tokenize_with_text}; use rustc_ast::{BinOpKind, MetaItemInner}; use rustc_errors::Applicability; @@ -9,7 +9,6 @@ use rustc_lexer::TokenKind; use rustc_lint::{LateContext, LateLintPass, Level}; use rustc_session::impl_lint_pass; -use rustc_span::source_map::SourceMap; use rustc_span::{BytePos, Span, Symbol}; declare_clippy_lint! { @@ -111,10 +110,8 @@ fn check_collapsible_else_if(&self, cx: &LateContext<'_>, then_span: Span, else_ let up_to_else = then_span.between(else_block.span); let else_before_if = else_.span.shrink_to_lo().with_hi(else_if_cond.span.lo() - BytePos(1)); if self.lint_commented_code - && let Some(else_keyword_span) = - span_extract_keyword(cx.tcx.sess.source_map(), up_to_else, "else") - && let Some(else_if_keyword_span) = - span_extract_keyword(cx.tcx.sess.source_map(), else_before_if, "if") + && let Some(else_keyword_span) = span_extract_keyword(cx, up_to_else, "else") + && let Some(else_if_keyword_span) = span_extract_keyword(cx, else_before_if, "if") { let else_keyword_span = else_keyword_span.with_leading_whitespace(cx).into_span(); let else_open_bracket = else_block.span.split_at(1).0.with_leading_whitespace(cx).into_span(); @@ -139,7 +136,7 @@ fn check_collapsible_else_if(&self, cx: &LateContext<'_>, then_span: Span, else_ } // Peel off any parentheses. - let (_, else_block_span, _) = peel_parens(cx.tcx.sess.source_map(), else_.span); + let (_, else_block_span, _) = peel_parens(cx, else_.span); // Prevent "elseif" // Check that the "else" is followed by whitespace @@ -187,7 +184,7 @@ fn check_collapsible_if_if(&self, cx: &LateContext<'_>, expr: &Expr<'_>, check: .with_leading_whitespace(cx) .into_span() }; - let (paren_start, inner_if_span, paren_end) = peel_parens(cx.tcx.sess.source_map(), inner.span); + let (paren_start, inner_if_span, paren_end) = peel_parens(cx, inner.span); let inner_if = inner_if_span.split_at(2).0; let mut sugg = vec![ // Remove the outer then block `{` @@ -320,33 +317,36 @@ pub(super) fn parens_around(expr: &Expr<'_>) -> Vec<(Span, String)> { } } -fn span_extract_keyword(sm: &SourceMap, span: Span, keyword: &str) -> Option { - let snippet = sm.span_to_snippet(span).ok()?; - tokenize_with_text(&snippet) - .filter(|(t, s, _)| matches!(t, TokenKind::Ident if *s == keyword)) - .map(|(_, _, inner)| { - span.split_at(u32::try_from(inner.start).unwrap()) - .1 - .split_at(u32::try_from(inner.end - inner.start).unwrap()) - .0 - }) - .next() +fn span_extract_keyword(cx: &impl HasSession, span: Span, keyword: &str) -> Option { + span.with_source_text(cx, |snippet| { + tokenize_with_text(snippet) + .filter(|(t, s, _)| matches!(t, TokenKind::Ident if *s == keyword)) + .map(|(_, _, inner)| { + span.split_at(u32::try_from(inner.start).unwrap()) + .1 + .split_at(u32::try_from(inner.end - inner.start).unwrap()) + .0 + }) + .next() + }) + .flatten() } /// Peel the parentheses from an `if` expression, e.g. `((if true {} else {}))`. -pub(super) fn peel_parens(sm: &SourceMap, mut span: Span) -> (Span, Span, Span) { +pub(super) fn peel_parens(cx: &impl HasSession, mut span: Span) -> (Span, Span, Span) { use crate::rustc_span::Pos; let start = span.shrink_to_lo(); let end = span.shrink_to_hi(); - let snippet = sm.span_to_snippet(span).unwrap(); - if let Some((trim_start, _, trim_end)) = peel_parens_str(&snippet) { - let mut data = span.data(); - data.lo = data.lo + BytePos::from_usize(trim_start); - data.hi = data.hi - BytePos::from_usize(trim_end); - span = data.span(); - } + span.with_source_text(cx, |snippet| { + if let Some((trim_start, _, trim_end)) = peel_parens_str(snippet) { + let mut data = span.data(); + data.lo = data.lo + BytePos::from_usize(trim_start); + data.hi = data.hi - BytePos::from_usize(trim_end); + span = data.span(); + } + }); (start.with_hi(span.lo()), span, end.with_lo(span.hi())) } diff --git a/src/tools/clippy/clippy_lints/src/declared_lints.rs b/src/tools/clippy/clippy_lints/src/declared_lints.rs index e16b194c0cad..441b907eaf2f 100644 --- a/src/tools/clippy/clippy_lints/src/declared_lints.rs +++ b/src/tools/clippy/clippy_lints/src/declared_lints.rs @@ -312,6 +312,7 @@ crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO, crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO, crate::manual_option_as_slice::MANUAL_OPTION_AS_SLICE_INFO, + crate::manual_pop_if::MANUAL_POP_IF_INFO, crate::manual_range_patterns::MANUAL_RANGE_PATTERNS_INFO, crate::manual_rem_euclid::MANUAL_REM_EUCLID_INFO, crate::manual_retain::MANUAL_RETAIN_INFO, diff --git a/src/tools/clippy/clippy_lints/src/floating_point_arithmetic/custom_abs.rs b/src/tools/clippy/clippy_lints/src/floating_point_arithmetic/custom_abs.rs index 6578cf20e637..f476abae708d 100644 --- a/src/tools/clippy/clippy_lints/src/floating_point_arithmetic/custom_abs.rs +++ b/src/tools/clippy/clippy_lints/src/floating_point_arithmetic/custom_abs.rs @@ -6,8 +6,7 @@ use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp}; use rustc_lint::LateContext; -use rustc_span::SyntaxContext; -use rustc_span::Spanned; +use rustc_span::{Spanned, SyntaxContext}; use super::SUBOPTIMAL_FLOPS; diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index 4d67c158c043..719484b30de8 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -210,6 +210,7 @@ mod manual_main_separator_str; mod manual_non_exhaustive; mod manual_option_as_slice; +mod manual_pop_if; mod manual_range_patterns; mod manual_rem_euclid; mod manual_retain; @@ -863,6 +864,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co Box::new(move |tcx| Box::new(duration_suboptimal_units::DurationSuboptimalUnits::new(tcx, conf))), Box::new(move |_| Box::new(manual_take::ManualTake::new(conf))), Box::new(|_| Box::new(manual_checked_ops::ManualCheckedOps)), + Box::new(move |_| Box::new(manual_pop_if::ManualPopIf::new(conf))), // add late passes here, used by `cargo dev new_lint` ]; store.late_passes.extend(late_lints); diff --git a/src/tools/clippy/clippy_lints/src/loops/manual_flatten.rs b/src/tools/clippy/clippy_lints/src/loops/manual_flatten.rs index 96de118b5233..2c89afc73974 100644 --- a/src/tools/clippy/clippy_lints/src/loops/manual_flatten.rs +++ b/src/tools/clippy/clippy_lints/src/loops/manual_flatten.rs @@ -3,7 +3,7 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::res::MaybeResPath; -use clippy_utils::source::{HasSession, indent_of, reindent_multiline, snippet_with_applicability}; +use clippy_utils::source::{indent_of, reindent_multiline, snippet_with_applicability}; use clippy_utils::visitors::is_local_used; use clippy_utils::{higher, is_refutable, peel_blocks_with_stmt, span_contains_comment}; use rustc_errors::Applicability; @@ -50,7 +50,7 @@ pub(super) fn check<'tcx>( format!("unnecessary `if let` since only the `{if_let_type}` variant of the iterator element is used"); // Prepare the help message - let mut applicability = if span_contains_comment(cx.sess().source_map(), body.span) { + let mut applicability = if span_contains_comment(cx, body.span) { Applicability::MaybeIncorrect } else { Applicability::MachineApplicable diff --git a/src/tools/clippy/clippy_lints/src/loops/manual_slice_fill.rs b/src/tools/clippy/clippy_lints/src/loops/manual_slice_fill.rs index 5d09f755bcd1..ffc6f7186922 100644 --- a/src/tools/clippy/clippy_lints/src/loops/manual_slice_fill.rs +++ b/src/tools/clippy/clippy_lints/src/loops/manual_slice_fill.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::eager_or_lazy::switch_to_eager_eval; use clippy_utils::msrvs::{self, Msrv}; -use clippy_utils::source::{HasSession, snippet_with_applicability}; +use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::{implements_trait, is_slice_like}; use clippy_utils::visitors::is_local_used; use clippy_utils::{higher, peel_blocks_with_stmt, span_contains_comment}; @@ -93,7 +93,7 @@ fn sugg<'tcx>( slice_span: rustc_span::Span, assignval_span: rustc_span::Span, ) { - let mut app = if span_contains_comment(cx.sess().source_map(), body.span) { + let mut app = if span_contains_comment(cx, body.span) { Applicability::MaybeIncorrect // Comments may be informational. } else { Applicability::MachineApplicable diff --git a/src/tools/clippy/clippy_lints/src/manual_abs_diff.rs b/src/tools/clippy/clippy_lints/src/manual_abs_diff.rs index e3886517fdd3..400cfcd18f2c 100644 --- a/src/tools/clippy/clippy_lints/src/manual_abs_diff.rs +++ b/src/tools/clippy/clippy_lints/src/manual_abs_diff.rs @@ -3,7 +3,6 @@ use clippy_utils::higher::If; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::res::MaybeDef; -use clippy_utils::source::HasSession as _; use clippy_utils::sugg::Sugg; use clippy_utils::ty::peel_and_count_ty_refs; use clippy_utils::{eq_expr_value, peel_blocks, span_contains_comment, sym}; @@ -76,10 +75,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { (a, b) = (b, a); } let applicability = { - let source_map = cx.sess().source_map(); - if span_contains_comment(source_map, if_expr.then.span) - || span_contains_comment(source_map, r#else.span) - { + if span_contains_comment(cx, if_expr.then.span) || span_contains_comment(cx, r#else.span) { Applicability::MaybeIncorrect } else { Applicability::MachineApplicable diff --git a/src/tools/clippy/clippy_lints/src/manual_assert.rs b/src/tools/clippy/clippy_lints/src/manual_assert.rs index c34e0d33e713..c77133130ba4 100644 --- a/src/tools/clippy/clippy_lints/src/manual_assert.rs +++ b/src/tools/clippy/clippy_lints/src/manual_assert.rs @@ -58,7 +58,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { "only a `panic!` in `if`-then statement", |diag| { let mut applicability = Applicability::MachineApplicable; - let mut comments = span_extract_comment(cx.sess().source_map(), expr.span); + let mut comments = span_extract_comment(cx, expr.span); if !comments.is_empty() { comments += "\n"; } diff --git a/src/tools/clippy/clippy_lints/src/manual_ilog2.rs b/src/tools/clippy/clippy_lints/src/manual_ilog2.rs index 7c397bd3f5e8..2c368f15d670 100644 --- a/src/tools/clippy/clippy_lints/src/manual_ilog2.rs +++ b/src/tools/clippy/clippy_lints/src/manual_ilog2.rs @@ -31,7 +31,7 @@ /// let log = x.ilog2(); /// let log = x.ilog2(); /// ``` - #[clippy::version = "1.93.0"] + #[clippy::version = "1.94.0"] pub MANUAL_ILOG2, pedantic, "manually reimplementing `ilog2`" diff --git a/src/tools/clippy/clippy_lints/src/manual_pop_if.rs b/src/tools/clippy/clippy_lints/src/manual_pop_if.rs new file mode 100644 index 000000000000..6662a34bc332 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/manual_pop_if.rs @@ -0,0 +1,434 @@ +use clippy_config::Conf; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::msrvs::{self, Msrv}; +use clippy_utils::res::MaybeDef; +use clippy_utils::source::snippet_with_context; +use clippy_utils::visitors::is_local_used; +use clippy_utils::{eq_expr_value, is_else_clause, is_lang_item_or_ctor, peel_blocks_with_stmt, sym}; +use rustc_ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, LangItem, PatKind, RustcVersion, StmtKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::impl_lint_pass; +use rustc_span::{Span, Symbol}; +use std::fmt; + +declare_clippy_lint! { + /// ### What it does + /// Checks for code to be replaced by `pop_if` methods. + /// + /// ### Why is this bad? + /// Using `pop_if` is more concise and idiomatic. + /// + /// ### Known issues + /// Currently, the lint does not handle the case where the + /// `if` condition is part of an `else if` branch. + /// + /// The lint also does not handle the case where + /// the popped value is assigned and used. + /// + /// ### Examples + /// ```no_run + /// # use std::collections::VecDeque; + /// # let mut vec = vec![1, 2, 3, 4, 5]; + /// # let mut deque: VecDeque = VecDeque::from([1, 2, 3, 4, 5]); + /// if vec.last().is_some_and(|x| *x > 5) { + /// vec.pop().unwrap(); + /// } + /// if deque.back().is_some_and(|x| *x > 5) { + /// deque.pop_back().unwrap(); + /// } + /// if deque.front().is_some_and(|x| *x > 5) { + /// deque.pop_front().unwrap(); + /// } + /// ``` + /// Use instead: + /// ```no_run + /// # use std::collections::VecDeque; + /// # let mut vec = vec![1, 2, 3, 4, 5]; + /// # let mut deque: VecDeque = VecDeque::from([1, 2, 3, 4, 5]); + /// vec.pop_if(|x| *x > 5); + /// deque.pop_back_if(|x| *x > 5); + /// deque.pop_front_if(|x| *x > 5); + /// ``` + #[clippy::version = "1.95.0"] + pub MANUAL_POP_IF, + complexity, + "manual implementation of `pop_if` methods" +} + +impl_lint_pass!(ManualPopIf => [MANUAL_POP_IF]); + +pub struct ManualPopIf { + msrv: Msrv, +} + +impl ManualPopIf { + pub fn new(conf: &'static Conf) -> Self { + Self { msrv: conf.msrv } + } +} + +#[allow(clippy::enum_variant_names)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ManualPopIfKind { + Vec, + VecDequeBack, + VecDequeFront, +} + +impl ManualPopIfKind { + fn check_method(self) -> Symbol { + match self { + ManualPopIfKind::Vec => sym::last, + ManualPopIfKind::VecDequeBack => sym::back, + ManualPopIfKind::VecDequeFront => sym::front, + } + } + + fn pop_method(self) -> Symbol { + match self { + ManualPopIfKind::Vec => sym::pop, + ManualPopIfKind::VecDequeBack => sym::pop_back, + ManualPopIfKind::VecDequeFront => sym::pop_front, + } + } + + fn pop_if_method(self) -> Symbol { + match self { + ManualPopIfKind::Vec => sym::pop_if, + ManualPopIfKind::VecDequeBack => sym::pop_back_if, + ManualPopIfKind::VecDequeFront => sym::pop_front_if, + } + } + + fn is_diag_item(self, cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + let ty = cx.typeck_results().expr_ty(expr).peel_refs(); + match self { + ManualPopIfKind::Vec => ty.is_diag_item(cx, sym::Vec), + ManualPopIfKind::VecDequeBack | ManualPopIfKind::VecDequeFront => ty.is_diag_item(cx, sym::VecDeque), + } + } + + fn msrv(self) -> RustcVersion { + match self { + ManualPopIfKind::Vec => msrvs::VEC_POP_IF, + ManualPopIfKind::VecDequeBack => msrvs::VEC_DEQUE_POP_BACK_IF, + ManualPopIfKind::VecDequeFront => msrvs::VEC_DEQUE_POP_FRONT_IF, + } + } +} + +impl fmt::Display for ManualPopIfKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ManualPopIfKind::Vec => write!(f, "`Vec::pop_if`"), + ManualPopIfKind::VecDequeBack => write!(f, "`VecDeque::pop_back_if`"), + ManualPopIfKind::VecDequeFront => write!(f, "`VecDeque::pop_front_if`"), + } + } +} + +struct ManualPopIfPattern<'tcx> { + kind: ManualPopIfKind, + + /// The collection (`vec` in `vec.last()`) + collection_expr: &'tcx Expr<'tcx>, + + /// The closure (`*x > 5` in `|x| *x > 5`) + predicate: &'tcx Expr<'tcx>, + + /// Parameter name for the closure (`x` in `|x| *x > 5`) + param_name: Symbol, + + /// Span of the if expression (including the `if` keyword) + if_span: Span, +} + +impl ManualPopIfPattern<'_> { + fn emit_lint(&self, cx: &LateContext<'_>) { + let mut app = Applicability::MachineApplicable; + let ctxt = self.if_span.ctxt(); + let collection_snippet = snippet_with_context(cx, self.collection_expr.span, ctxt, "..", &mut app).0; + let predicate_snippet = snippet_with_context(cx, self.predicate.span, ctxt, "..", &mut app).0; + let param_name = self.param_name; + let pop_if_method = self.kind.pop_if_method(); + + let suggestion = format!("{collection_snippet}.{pop_if_method}(|{param_name}| {predicate_snippet});"); + + span_lint_and_sugg( + cx, + MANUAL_POP_IF, + self.if_span, + format!("manual implementation of {}", self.kind), + "try", + suggestion, + app, + ); + } +} + +fn pop_value_is_used(then_block: &Expr<'_>) -> bool { + let ExprKind::Block(block, _) = then_block.kind else { + return true; + }; + + if block.expr.is_some() { + return true; + } + + match block.stmts { + [stmt] => !matches!(stmt.kind, StmtKind::Semi(_) | StmtKind::Item(_)), + [.., last] => matches!(last.kind, StmtKind::Expr(_)), + [] => false, + } +} + +/// Checks for the pattern: +/// ```ignore +/// if vec.last().is_some_and(|x| *x > 5) { +/// vec.pop().unwrap(); +/// } +/// ``` +fn check_is_some_and_pattern<'tcx>( + cx: &LateContext<'tcx>, + cond: &'tcx Expr<'_>, + then_block: &'tcx Expr<'_>, + if_expr_span: Span, + kind: ManualPopIfKind, +) -> Option> { + if pop_value_is_used(then_block) { + return None; + } + + let check_method = kind.check_method(); + let pop_method = kind.pop_method(); + + if let ExprKind::MethodCall(path, receiver, [closure_arg], _) = cond.kind + && path.ident.name == sym::is_some_and + && let ExprKind::MethodCall(check_path, collection_expr, [], _) = receiver.kind + && check_path.ident.name == check_method + && kind.is_diag_item(cx, collection_expr) + && let ExprKind::Closure(closure) = closure_arg.kind + && let body = cx.tcx.hir_body(closure.body) + && let Some((pop_collection, _pop_span)) = check_pop_unwrap(then_block, pop_method) + && eq_expr_value(cx, collection_expr, pop_collection) + && let Some(param) = body.params.first() + && let Some(ident) = param.pat.simple_ident() + { + return Some(ManualPopIfPattern { + kind, + collection_expr, + predicate: body.value, + param_name: ident.name, + if_span: if_expr_span, + }); + } + + None +} + +/// Checks for the pattern: +/// ```ignore +/// if let Some(x) = vec.last() { +/// if *x > 5 { +/// vec.pop().unwrap(); +/// } +/// } +/// ``` +fn check_if_let_pattern<'tcx>( + cx: &LateContext<'tcx>, + cond: &'tcx Expr<'_>, + then_block: &'tcx Expr<'_>, + if_expr_span: Span, + kind: ManualPopIfKind, +) -> Option> { + let check_method = kind.check_method(); + let pop_method = kind.pop_method(); + + if let ExprKind::Let(let_expr) = cond.kind + && let PatKind::TupleStruct(qpath, [binding_pat], _) = let_expr.pat.kind + { + let res = cx.qpath_res(&qpath, let_expr.pat.hir_id); + + if let Some(def_id) = res.opt_def_id() + && is_lang_item_or_ctor(cx, def_id, LangItem::OptionSome) + && let PatKind::Binding(_, binding_id, binding_name, _) = binding_pat.kind + && let ExprKind::MethodCall(path, collection_expr, [], _) = let_expr.init.kind + && path.ident.name == check_method + && kind.is_diag_item(cx, collection_expr) + && let ExprKind::Block(block, _) = then_block.kind + { + // The inner if can be either a statement or a block expression + let inner_if = match (block.stmts, block.expr) { + ([stmt], _) => match stmt.kind { + StmtKind::Expr(expr) | StmtKind::Semi(expr) => expr, + _ => return None, + }, + ([], Some(expr)) => expr, + _ => return None, + }; + + if let ExprKind::If(inner_cond, inner_then, None) = inner_if.kind + && is_local_used(cx, inner_cond, binding_id) + && !pop_value_is_used(inner_then) + && let Some((pop_collection, _pop_span)) = check_pop_unwrap(inner_then, pop_method) + && eq_expr_value(cx, collection_expr, pop_collection) + { + return Some(ManualPopIfPattern { + kind, + collection_expr, + predicate: inner_cond, + param_name: binding_name.name, + if_span: if_expr_span, + }); + } + } + } + + None +} + +/// Checks for the pattern: +/// ```ignore +/// if let Some(x) = vec.last() && *x > 5 { +/// vec.pop().unwrap(); +/// } +/// ``` +fn check_let_chain_pattern<'tcx>( + cx: &LateContext<'tcx>, + cond: &'tcx Expr<'_>, + then_block: &'tcx Expr<'_>, + if_expr_span: Span, + kind: ManualPopIfKind, +) -> Option> { + if pop_value_is_used(then_block) { + return None; + } + + let check_method = kind.check_method(); + let pop_method = kind.pop_method(); + + if let ExprKind::Binary(op, left, right) = cond.kind + && op.node == rustc_ast::BinOpKind::And + && let ExprKind::Let(let_expr) = left.kind + && let PatKind::TupleStruct(qpath, [binding_pat], _) = let_expr.pat.kind + { + let res = cx.qpath_res(&qpath, let_expr.pat.hir_id); + + if let Some(def_id) = res.opt_def_id() + && is_lang_item_or_ctor(cx, def_id, LangItem::OptionSome) + && let PatKind::Binding(_, binding_id, binding_name, _) = binding_pat.kind + && let ExprKind::MethodCall(path, collection_expr, [], _) = let_expr.init.kind + && path.ident.name == check_method + && kind.is_diag_item(cx, collection_expr) + && is_local_used(cx, right, binding_id) + && !pop_value_is_used(then_block) + && let Some((pop_collection, _pop_span)) = check_pop_unwrap(then_block, pop_method) + && eq_expr_value(cx, collection_expr, pop_collection) + { + return Some(ManualPopIfPattern { + kind, + collection_expr, + predicate: right, + param_name: binding_name.name, + if_span: if_expr_span, + }); + } + } + + None +} + +/// Checks for the pattern: +/// ```ignore +/// if vec.last().map(|x| *x > 5).unwrap_or(false) { +/// vec.pop().unwrap(); +/// } +/// ``` +fn check_map_unwrap_or_pattern<'tcx>( + cx: &LateContext<'tcx>, + cond: &'tcx Expr<'_>, + then_block: &'tcx Expr<'_>, + if_expr_span: Span, + kind: ManualPopIfKind, +) -> Option> { + if pop_value_is_used(then_block) { + return None; + } + + let check_method = kind.check_method(); + let pop_method = kind.pop_method(); + + if let ExprKind::MethodCall(unwrap_path, receiver, [default_arg], _) = cond.kind + && unwrap_path.ident.name == sym::unwrap_or + && matches!(default_arg.kind, ExprKind::Lit(lit) if matches!(lit.node, LitKind::Bool(false))) + && let ExprKind::MethodCall(map_path, map_receiver, [closure_arg], _) = receiver.kind + && map_path.ident.name == sym::map + && let ExprKind::MethodCall(check_path, collection_expr, [], _) = map_receiver.kind + && check_path.ident.name == check_method + && kind.is_diag_item(cx, collection_expr) + && let ExprKind::Closure(closure) = closure_arg.kind + && let body = cx.tcx.hir_body(closure.body) + && cx.typeck_results().expr_ty(body.value).is_bool() + && let Some((pop_collection, _pop_span)) = check_pop_unwrap(then_block, pop_method) + && eq_expr_value(cx, collection_expr, pop_collection) + && let Some(param) = body.params.first() + && let Some(ident) = param.pat.simple_ident() + { + return Some(ManualPopIfPattern { + kind, + collection_expr, + predicate: body.value, + param_name: ident.name, + if_span: if_expr_span, + }); + } + + None +} + +/// Checks for `collection.().unwrap()` or `collection.().expect(..)` +/// and returns the collection and the span of the peeled expr +fn check_pop_unwrap<'tcx>(expr: &'tcx Expr<'_>, pop_method: Symbol) -> Option<(&'tcx Expr<'tcx>, Span)> { + let inner_expr = peel_blocks_with_stmt(expr); + + if let ExprKind::MethodCall(unwrap_path, receiver, _, _) = inner_expr.kind + && matches!(unwrap_path.ident.name, sym::unwrap | sym::expect) + && let ExprKind::MethodCall(pop_path, collection_expr, [], _) = receiver.kind + && pop_path.ident.name == pop_method + { + return Some((collection_expr, inner_expr.span)); + } + + None +} + +impl<'tcx> LateLintPass<'tcx> for ManualPopIf { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + let ExprKind::If(cond, then_block, None) = expr.kind else { + return; + }; + + // Do not lint if we are in an else-if branch. + if is_else_clause(cx.tcx, expr) { + return; + } + + for kind in [ + ManualPopIfKind::Vec, + ManualPopIfKind::VecDequeBack, + ManualPopIfKind::VecDequeFront, + ] { + if let Some(pattern) = check_is_some_and_pattern(cx, cond, then_block, expr.span, kind) + .or_else(|| check_if_let_pattern(cx, cond, then_block, expr.span, kind)) + .or_else(|| check_let_chain_pattern(cx, cond, then_block, expr.span, kind)) + .or_else(|| check_map_unwrap_or_pattern(cx, cond, then_block, expr.span, kind)) + && self.msrv.meets(cx, kind.msrv()) + { + pattern.emit_lint(cx); + return; + } + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/matches/collapsible_match.rs b/src/tools/clippy/clippy_lints/src/matches/collapsible_match.rs index c95a72da6e29..de5f83b4745f 100644 --- a/src/tools/clippy/clippy_lints/src/matches/collapsible_match.rs +++ b/src/tools/clippy/clippy_lints/src/matches/collapsible_match.rs @@ -154,7 +154,7 @@ fn check_arm<'tcx>( } else { outer_pat.span.shrink_to_hi() }; - let (paren_start, inner_if_span, paren_end) = peel_parens(cx.tcx.sess.source_map(), inner_expr.span); + let (paren_start, inner_if_span, paren_end) = peel_parens(cx, inner_expr.span); let inner_if = inner_if_span.split_at(2).0; let mut sugg = vec![ (inner.then.span.shrink_to_lo(), "=> ".to_string()), diff --git a/src/tools/clippy/clippy_lints/src/matches/manual_ok_err.rs b/src/tools/clippy/clippy_lints/src/matches/manual_ok_err.rs index 1fc8bb9acce2..abbf60019c5c 100644 --- a/src/tools/clippy/clippy_lints/src/matches/manual_ok_err.rs +++ b/src/tools/clippy/clippy_lints/src/matches/manual_ok_err.rs @@ -9,7 +9,7 @@ use rustc_hir::LangItem::ResultErr; use rustc_hir::def::{DefKind, Res}; use rustc_hir::{Arm, Expr, ExprKind, Pat, PatExpr, PatExprKind, PatKind, Path, QPath}; -use rustc_lint::{LateContext, LintContext}; +use rustc_lint::LateContext; use rustc_middle::ty::Ty; use rustc_span::symbol::Ident; @@ -130,7 +130,7 @@ fn is_none(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { /// `err`, depending on `is_ok`. fn apply_lint(cx: &LateContext<'_>, expr: &Expr<'_>, scrutinee: &Expr<'_>, is_ok: bool) { let method = if is_ok { "ok" } else { "err" }; - let mut app = if span_contains_comment(cx.sess().source_map(), expr.span) { + let mut app = if span_contains_comment(cx, expr.span) { Applicability::MaybeIncorrect } else { Applicability::MachineApplicable diff --git a/src/tools/clippy/clippy_lints/src/matches/manual_unwrap_or.rs b/src/tools/clippy/clippy_lints/src/matches/manual_unwrap_or.rs index 421c6064284d..e3a112d1f780 100644 --- a/src/tools/clippy/clippy_lints/src/matches/manual_unwrap_or.rs +++ b/src/tools/clippy/clippy_lints/src/matches/manual_unwrap_or.rs @@ -5,7 +5,7 @@ use rustc_errors::Applicability; use rustc_hir::def::Res; use rustc_hir::{Arm, Expr, ExprKind, HirId, LangItem, Pat, PatExpr, PatExprKind, PatKind, QPath}; -use rustc_lint::{LateContext, LintContext}; +use rustc_lint::LateContext; use rustc_middle::ty::{GenericArgKind, Ty}; use rustc_span::sym; @@ -101,7 +101,7 @@ fn handle( && local_id == binding_id { // Machine applicable only if there are no comments present - let mut applicability = if span_contains_comment(cx.sess().source_map(), expr.span) { + let mut applicability = if span_contains_comment(cx, expr.span) { Applicability::MaybeIncorrect } else { Applicability::MachineApplicable diff --git a/src/tools/clippy/clippy_lints/src/matches/match_like_matches.rs b/src/tools/clippy/clippy_lints/src/matches/match_like_matches.rs index 3a16c2ed8790..7b3ad2596b9e 100644 --- a/src/tools/clippy/clippy_lints/src/matches/match_like_matches.rs +++ b/src/tools/clippy/clippy_lints/src/matches/match_like_matches.rs @@ -8,7 +8,7 @@ use rustc_ast::LitKind; use rustc_errors::Applicability; use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Pat, PatKind, QPath}; -use rustc_lint::{LateContext, LintContext}; +use rustc_lint::LateContext; use rustc_middle::ty; use rustc_span::Spanned; @@ -22,7 +22,7 @@ pub(crate) fn check_if_let<'tcx>( then_expr: &'tcx Expr<'_>, else_expr: &'tcx Expr<'_>, ) { - if !span_contains_comment(cx.sess().source_map(), expr.span) + if !span_contains_comment(cx, expr.span) && cx.typeck_results().expr_ty(expr).is_bool() && let Some(b0) = find_bool_lit(then_expr) && let Some(b1) = find_bool_lit(else_expr) @@ -71,7 +71,7 @@ pub(super) fn check_match<'tcx>( ) -> bool { if let Some((last_arm, arms_without_last)) = arms.split_last() && let Some((first_arm, middle_arms)) = arms_without_last.split_first() - && !span_contains_comment(cx.sess().source_map(), e.span) + && !span_contains_comment(cx, e.span) && cx.typeck_results().expr_ty(e).is_bool() && let Some(b0) = find_bool_lit(first_arm.body) && let Some(b1) = find_bool_lit(last_arm.body) diff --git a/src/tools/clippy/clippy_lints/src/matches/match_same_arms.rs b/src/tools/clippy/clippy_lints/src/matches/match_same_arms.rs index 5b0de80e67fd..a8312a04f36f 100644 --- a/src/tools/clippy/clippy_lints/src/matches/match_same_arms.rs +++ b/src/tools/clippy/clippy_lints/src/matches/match_same_arms.rs @@ -12,7 +12,7 @@ use rustc_hir::{Arm, Expr, HirId, HirIdMap, HirIdMapEntry, HirIdSet, Pat, PatExpr, PatExprKind, PatKind, RangeEnd}; use rustc_lint::builtin::NON_EXHAUSTIVE_OMITTED_PATTERNS; use rustc_lint::{LateContext, LintContext}; -use rustc_middle::ty; +use rustc_middle::ty::{self, TypeckResults}; use rustc_span::{ByteSymbol, ErrorGuaranteed, Span, Symbol}; use super::MATCH_SAME_ARMS; @@ -61,7 +61,10 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) { let check_eq_with_pat = |expr_a: &Expr<'_>, expr_b: &Expr<'_>| { let mut local_map: HirIdMap = HirIdMap::default(); - let eq_fallback = |a: &Expr<'_>, b: &Expr<'_>| { + let eq_fallback = |a_typeck_results: &TypeckResults<'tcx>, + a: &Expr<'_>, + b_typeck_results: &TypeckResults<'tcx>, + b: &Expr<'_>| { if let Some(a_id) = a.res_local_id() && let Some(b_id) = b.res_local_id() && let entry = match local_map.entry(a_id) { @@ -71,7 +74,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) { } // the names technically don't have to match; this makes the lint more conservative && cx.tcx.hir_name(a_id) == cx.tcx.hir_name(b_id) - && cx.typeck_results().expr_ty(a) == cx.typeck_results().expr_ty(b) + && a_typeck_results.expr_ty(a) == b_typeck_results.expr_ty(b) && pat_contains_local(lhs.pat, a_id) && pat_contains_local(rhs.pat, b_id) { diff --git a/src/tools/clippy/clippy_lints/src/matches/mod.rs b/src/tools/clippy/clippy_lints/src/matches/mod.rs index dc8c06ffd79e..717c47b8aed3 100644 --- a/src/tools/clippy/clippy_lints/src/matches/mod.rs +++ b/src/tools/clippy/clippy_lints/src/matches/mod.rs @@ -1093,12 +1093,11 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { } redundant_pattern_match::check_match(cx, expr, ex, arms); - let source_map = cx.tcx.sess.source_map(); - let mut match_comments = span_extract_comments(source_map, expr.span); + let mut match_comments = span_extract_comments(cx, expr.span); // We remove comments from inside arms block. if !match_comments.is_empty() { for arm in arms { - for comment in span_extract_comments(source_map, arm.body.span) { + for comment in span_extract_comments(cx, arm.body.span) { if let Some(index) = match_comments .iter() .enumerate() diff --git a/src/tools/clippy/clippy_lints/src/methods/case_sensitive_file_extension_comparisons.rs b/src/tools/clippy/clippy_lints/src/methods/case_sensitive_file_extension_comparisons.rs index 23a0046dec76..229670486db3 100644 --- a/src/tools/clippy/clippy_lints/src/methods/case_sensitive_file_extension_comparisons.rs +++ b/src/tools/clippy/clippy_lints/src/methods/case_sensitive_file_extension_comparisons.rs @@ -7,8 +7,7 @@ use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, LangItem}; use rustc_lint::LateContext; -use rustc_span::Span; -use rustc_span::Spanned; +use rustc_span::{Span, Spanned}; use super::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS; diff --git a/src/tools/clippy/clippy_lints/src/methods/filter_map.rs b/src/tools/clippy/clippy_lints/src/methods/filter_map.rs index 7b10c37de42d..d2e593fc17df 100644 --- a/src/tools/clippy/clippy_lints/src/methods/filter_map.rs +++ b/src/tools/clippy/clippy_lints/src/methods/filter_map.rs @@ -9,6 +9,7 @@ use rustc_hir::def::Res; use rustc_hir::{Closure, Expr, ExprKind, PatKind, PathSegment, QPath, UnOp}; use rustc_lint::LateContext; +use rustc_middle::ty::TypeckResults; use rustc_middle::ty::adjustment::Adjust; use rustc_span::Span; use rustc_span::symbol::{Ident, Symbol}; @@ -136,7 +137,9 @@ pub fn check_map_call( // .map(|y| y[.acceptable_method()].unwrap()) && let simple_equal = (receiver.res_local_id() == Some(filter_param_id) && map_arg_peeled.res_local_id() == Some(map_param_id)) - && let eq_fallback = (|a: &Expr<'_>, b: &Expr<'_>| { + && let eq_fallback = + (|a_typeck_results: &TypeckResults<'tcx>, a: &Expr<'_>, + b_typeck_results: &TypeckResults<'tcx>, b: &Expr<'_>| { // in `filter(|x| ..)`, replace `*x` with `x` let a_path = if !is_filter_param_ref && let ExprKind::Unary(UnOp::Deref, expr_path) = a.kind @@ -144,7 +147,7 @@ pub fn check_map_call( // let the filter closure arg and the map closure arg be equal a_path.res_local_id() == Some(filter_param_id) && b.res_local_id() == Some(map_param_id) - && cx.typeck_results().expr_ty_adjusted(a) == cx.typeck_results().expr_ty_adjusted(b) + && a_typeck_results.expr_ty_adjusted(a) == b_typeck_results.expr_ty_adjusted(b) }) && (simple_equal || SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(receiver, map_arg_peeled)) diff --git a/src/tools/clippy/clippy_lints/src/methods/iter_filter.rs b/src/tools/clippy/clippy_lints/src/methods/iter_filter.rs index aaface3aa971..3aa666145b86 100644 --- a/src/tools/clippy/clippy_lints/src/methods/iter_filter.rs +++ b/src/tools/clippy/clippy_lints/src/methods/iter_filter.rs @@ -1,7 +1,7 @@ use clippy_utils::res::{MaybeDef, MaybeTypeckRes}; use clippy_utils::ty::get_iterator_item_ty; use hir::ExprKind; -use rustc_lint::{LateContext, LintContext}; +use rustc_lint::LateContext; use super::{ITER_FILTER_IS_OK, ITER_FILTER_IS_SOME}; @@ -144,7 +144,7 @@ fn expression_type( ) -> Option { if !cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::Iterator) || parent_is_map(cx, expr) - || span_contains_comment(cx.sess().source_map(), filter_span.with_hi(expr.span.hi())) + || span_contains_comment(cx, filter_span.with_hi(expr.span.hi())) { return None; } diff --git a/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs b/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs index e4ae14b6cf59..09733953255e 100644 --- a/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs +++ b/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs @@ -19,7 +19,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, map_ let closure_snippet = snippet_with_applicability(cx, map_arg.span, "..", &mut applicability); let span = expr.span.with_lo(map_span.lo()); // If the methods are separated with comments, we don't apply suggestion automatically. - if span_contains_comment(cx.tcx.sess.source_map(), span) { + if span_contains_comment(cx, span) { applicability = Applicability::Unspecified; } span_lint_and_sugg( diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs index 0ed166f8dd5b..b647dbdc8468 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mod.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs @@ -134,9 +134,8 @@ mod unnecessary_lazy_eval; mod unnecessary_literal_unwrap; mod unnecessary_map_or; +mod unnecessary_map_or_else; mod unnecessary_min_or_max; -mod unnecessary_option_map_or_else; -mod unnecessary_result_map_or_else; mod unnecessary_sort_by; mod unnecessary_to_owned; mod unwrap_expect_used; @@ -3030,7 +3029,7 @@ /// ptr.sub(8); /// } /// ``` - #[clippy::version = "1.92.0"] + #[clippy::version = "1.94.0"] pub PTR_OFFSET_BY_LITERAL, pedantic, "unneeded pointer offset" @@ -4347,6 +4346,7 @@ } declare_clippy_lint! { + /// ### What it does /// Checks for usage of `.map_or_else()` "map closure" for `Option` type. /// /// ### Why is this bad? @@ -5449,8 +5449,7 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { }, (sym::map_or_else, [def, map]) => { result_map_or_else_none::check(cx, expr, recv, def, map); - unnecessary_option_map_or_else::check(cx, expr, recv, def, map); - unnecessary_result_map_or_else::check(cx, expr, recv, def, map); + unnecessary_map_or_else::check(cx, expr, recv, def, map, call_span); }, (sym::next, []) => { if let Some((name2, recv2, args2, _, _)) = method_call(recv) { diff --git a/src/tools/clippy/clippy_lints/src/methods/open_options.rs b/src/tools/clippy/clippy_lints/src/methods/open_options.rs index 042558d645e6..36dd4e952a2d 100644 --- a/src/tools/clippy/clippy_lints/src/methods/open_options.rs +++ b/src/tools/clippy/clippy_lints/src/methods/open_options.rs @@ -7,8 +7,7 @@ use rustc_hir::{Expr, ExprKind}; use rustc_lint::LateContext; use rustc_middle::ty::Ty; -use rustc_span::Span; -use rustc_span::Spanned; +use rustc_span::{Span, Spanned}; use super::{NONSENSICAL_OPEN_OPTIONS, SUSPICIOUS_OPEN_OPTIONS}; diff --git a/src/tools/clippy/clippy_lints/src/methods/suspicious_splitn.rs b/src/tools/clippy/clippy_lints/src/methods/suspicious_splitn.rs index 98b7834178f2..cbb7da327506 100644 --- a/src/tools/clippy/clippy_lints/src/methods/suspicious_splitn.rs +++ b/src/tools/clippy/clippy_lints/src/methods/suspicious_splitn.rs @@ -2,8 +2,7 @@ use rustc_ast::LitKind; use rustc_hir::{Expr, ExprKind}; use rustc_lint::LateContext; -use rustc_span::Symbol; -use rustc_span::Spanned; +use rustc_span::{Spanned, Symbol}; use super::SUSPICIOUS_SPLITN; diff --git a/src/tools/clippy/clippy_lints/src/methods/unnecessary_map_or_else.rs b/src/tools/clippy/clippy_lints/src/methods/unnecessary_map_or_else.rs new file mode 100644 index 000000000000..7c1c31d2bd38 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/methods/unnecessary_map_or_else.rs @@ -0,0 +1,39 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::is_expr_identity_function; +use clippy_utils::res::MaybeDef; +use clippy_utils::source::snippet_with_applicability; +use rustc_errors::Applicability; +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_span::Span; +use rustc_span::symbol::sym; + +use super::{UNNECESSARY_OPTION_MAP_OR_ELSE, UNNECESSARY_RESULT_MAP_OR_ELSE}; + +/// lint use of `_.map_or_else(|err| err, |n| n)` for `Result`s and `Option`s. +pub(super) fn check( + cx: &LateContext<'_>, + expr: &Expr<'_>, + recv: &Expr<'_>, + def_arg: &Expr<'_>, + map_arg: &Expr<'_>, + call_span: Span, +) { + let (symbol, lint) = match cx.typeck_results().expr_ty(recv).opt_diag_name(cx) { + Some(x @ sym::Result) => (x, UNNECESSARY_RESULT_MAP_OR_ELSE), + Some(x @ sym::Option) => (x, UNNECESSARY_OPTION_MAP_OR_ELSE), + _ => return, + }; + + if is_expr_identity_function(cx, map_arg) { + let msg = format!("unused \"map closure\" when calling `{symbol}::map_or_else` value"); + + span_lint_and_then(cx, lint, expr.span, msg, |diag| { + let mut applicability = Applicability::MachineApplicable; + let err_snippet = snippet_with_applicability(cx, def_arg.span, "..", &mut applicability); + let sugg = format!("unwrap_or_else({err_snippet})"); + + diag.span_suggestion_verbose(call_span, "consider using `unwrap_or_else`", sugg, applicability); + }); + } +} diff --git a/src/tools/clippy/clippy_lints/src/methods/unnecessary_option_map_or_else.rs b/src/tools/clippy/clippy_lints/src/methods/unnecessary_option_map_or_else.rs deleted file mode 100644 index 265619e26376..000000000000 --- a/src/tools/clippy/clippy_lints/src/methods/unnecessary_option_map_or_else.rs +++ /dev/null @@ -1,111 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::res::MaybeDef; -use clippy_utils::source::snippet_with_applicability; -use clippy_utils::{expr_or_init, find_binding_init, peel_blocks}; -use rustc_errors::Applicability; -use rustc_hir::def::{DefKind, Res}; -use rustc_hir::{Body, BodyId, Closure, Expr, ExprKind, HirId, QPath}; -use rustc_lint::LateContext; -use rustc_span::symbol::sym; - -use super::UNNECESSARY_OPTION_MAP_OR_ELSE; -use super::utils::get_last_chain_binding_hir_id; - -fn emit_lint(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &Expr<'_>) { - let msg = "unused \"map closure\" when calling `Option::map_or_else` value"; - let mut applicability = Applicability::MachineApplicable; - let self_snippet = snippet_with_applicability(cx, recv.span, "_", &mut applicability); - let err_snippet = snippet_with_applicability(cx, def_arg.span, "..", &mut applicability); - span_lint_and_sugg( - cx, - UNNECESSARY_OPTION_MAP_OR_ELSE, - expr.span, - msg, - "consider using `unwrap_or_else`", - format!("{self_snippet}.unwrap_or_else({err_snippet})"), - Applicability::MachineApplicable, - ); -} - -fn handle_qpath( - cx: &LateContext<'_>, - expr: &Expr<'_>, - recv: &Expr<'_>, - def_arg: &Expr<'_>, - expected_hir_id: HirId, - qpath: QPath<'_>, -) { - if let QPath::Resolved(_, path) = qpath - && let Res::Local(hir_id) = path.res - && expected_hir_id == hir_id - { - emit_lint(cx, expr, recv, def_arg); - } -} - -fn handle_closure(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &Expr<'_>, body_id: BodyId) { - let body = cx.tcx.hir_body(body_id); - handle_fn_body(cx, expr, recv, def_arg, body); -} - -fn handle_fn_body(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &Expr<'_>, body: &Body<'_>) { - if let Some(first_param) = body.params.first() { - let body_expr = peel_blocks(body.value); - match body_expr.kind { - ExprKind::Path(qpath) => { - handle_qpath(cx, expr, recv, def_arg, first_param.pat.hir_id, qpath); - }, - // If this is a block (that wasn't peeled off), then it means there are statements. - ExprKind::Block(block, _) => { - if let Some(block_expr) = block.expr - // First we ensure that this is a "binding chain" (each statement is a binding - // of the previous one) and that it is a binding of the closure argument. - && let Some(last_chain_binding_id) = - get_last_chain_binding_hir_id(first_param.pat.hir_id, block.stmts) - && let ExprKind::Path(qpath) = block_expr.kind - { - handle_qpath(cx, expr, recv, def_arg, last_chain_binding_id, qpath); - } - }, - _ => {}, - } - } -} - -/// lint use of `_.map_or_else(|err| err, |n| n)` for `Option`s. -pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &Expr<'_>, map_arg: &Expr<'_>) { - // lint if the caller of `map_or_else()` is an `Option` - if !cx.typeck_results().expr_ty(recv).is_diag_item(cx, sym::Option) { - return; - } - match map_arg.kind { - // If the second argument is a closure, we can check its body. - ExprKind::Closure(&Closure { body, .. }) => { - handle_closure(cx, expr, recv, def_arg, body); - }, - ExprKind::Path(qpath) => { - let res = cx.qpath_res(&qpath, map_arg.hir_id); - match res { - // Case 1: Local variable (could be a closure) - Res::Local(hir_id) => { - if let Some(init_expr) = find_binding_init(cx, hir_id) { - let origin = expr_or_init(cx, init_expr); - if let ExprKind::Closure(&Closure { body, .. }) = origin.kind { - handle_closure(cx, expr, recv, def_arg, body); - } - } - }, - // Case 2: Function definition - Res::Def(DefKind::Fn, def_id) => { - if let Some(local_def_id) = def_id.as_local() - && let Some(body) = cx.tcx.hir_maybe_body_owned_by(local_def_id) - { - handle_fn_body(cx, expr, recv, def_arg, body); - } - }, - _ => (), - } - }, - _ => (), - } -} diff --git a/src/tools/clippy/clippy_lints/src/methods/unnecessary_result_map_or_else.rs b/src/tools/clippy/clippy_lints/src/methods/unnecessary_result_map_or_else.rs deleted file mode 100644 index 1f6bb60414ae..000000000000 --- a/src/tools/clippy/clippy_lints/src/methods/unnecessary_result_map_or_else.rs +++ /dev/null @@ -1,80 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::peel_blocks; -use clippy_utils::res::MaybeDef; -use clippy_utils::source::snippet; -use rustc_errors::Applicability; -use rustc_hir as hir; -use rustc_hir::{Closure, Expr, ExprKind, HirId, QPath}; -use rustc_lint::LateContext; -use rustc_span::symbol::sym; - -use super::UNNECESSARY_RESULT_MAP_OR_ELSE; -use super::utils::get_last_chain_binding_hir_id; - -fn emit_lint(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, def_arg: &Expr<'_>) { - let msg = "unused \"map closure\" when calling `Result::map_or_else` value"; - let self_snippet = snippet(cx, recv.span, ".."); - let err_snippet = snippet(cx, def_arg.span, ".."); - span_lint_and_sugg( - cx, - UNNECESSARY_RESULT_MAP_OR_ELSE, - expr.span, - msg, - "consider using `unwrap_or_else`", - format!("{self_snippet}.unwrap_or_else({err_snippet})"), - Applicability::MachineApplicable, - ); -} - -fn handle_qpath( - cx: &LateContext<'_>, - expr: &Expr<'_>, - recv: &Expr<'_>, - def_arg: &Expr<'_>, - expected_hir_id: HirId, - qpath: QPath<'_>, -) { - if let QPath::Resolved(_, path) = qpath - && let hir::def::Res::Local(hir_id) = path.res - && expected_hir_id == hir_id - { - emit_lint(cx, expr, recv, def_arg); - } -} - -/// lint use of `_.map_or_else(|err| err, |n| n)` for `Result`s. -pub(super) fn check<'tcx>( - cx: &LateContext<'tcx>, - expr: &'tcx Expr<'_>, - recv: &'tcx Expr<'_>, - def_arg: &'tcx Expr<'_>, - map_arg: &'tcx Expr<'_>, -) { - // lint if the caller of `map_or_else()` is a `Result` - if cx.typeck_results().expr_ty(recv).is_diag_item(cx, sym::Result) - && let ExprKind::Closure(&Closure { body, .. }) = map_arg.kind - && let body = cx.tcx.hir_body(body) - && let Some(first_param) = body.params.first() - { - let body_expr = peel_blocks(body.value); - - match body_expr.kind { - ExprKind::Path(qpath) => { - handle_qpath(cx, expr, recv, def_arg, first_param.pat.hir_id, qpath); - }, - // If this is a block (that wasn't peeled off), then it means there are statements. - ExprKind::Block(block, _) => { - if let Some(block_expr) = block.expr - // First we ensure that this is a "binding chain" (each statement is a binding - // of the previous one) and that it is a binding of the closure argument. - && let Some(last_chain_binding_id) = - get_last_chain_binding_hir_id(first_param.pat.hir_id, block.stmts) - && let ExprKind::Path(qpath) = block_expr.kind - { - handle_qpath(cx, expr, recv, def_arg, last_chain_binding_id, qpath); - } - }, - _ => {}, - } - } -} diff --git a/src/tools/clippy/clippy_lints/src/needless_bool.rs b/src/tools/clippy/clippy_lints/src/needless_bool.rs index 387b6f8bf223..a0ad1fe00c62 100644 --- a/src/tools/clippy/clippy_lints/src/needless_bool.rs +++ b/src/tools/clippy/clippy_lints/src/needless_bool.rs @@ -108,7 +108,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { then, r#else: Some(else_expr), }) = higher::If::hir(e) - && !span_contains_comment(cx.tcx.sess.source_map(), e.span) + && !span_contains_comment(cx, e.span) { let reduce = |ret, not| { let mut applicability = Applicability::MachineApplicable; diff --git a/src/tools/clippy/clippy_lints/src/operators/mod.rs b/src/tools/clippy/clippy_lints/src/operators/mod.rs index 567443b8e86d..b4519d654722 100644 --- a/src/tools/clippy/clippy_lints/src/operators/mod.rs +++ b/src/tools/clippy/clippy_lints/src/operators/mod.rs @@ -221,7 +221,7 @@ /// ```rust,no_run /// let a = 0b1110 & 0b0110; /// ``` - #[clippy::version = "1.93.0"] + #[clippy::version = "1.94.0"] pub DECIMAL_BITWISE_OPERANDS, pedantic, "use binary, hex, or octal literals for bitwise operations" diff --git a/src/tools/clippy/clippy_lints/src/question_mark.rs b/src/tools/clippy/clippy_lints/src/question_mark.rs index e79ce91d8b32..dfd7834a149b 100644 --- a/src/tools/clippy/clippy_lints/src/question_mark.rs +++ b/src/tools/clippy/clippy_lints/src/question_mark.rs @@ -143,7 +143,7 @@ fn init_expr_can_use_question_mark(cx: &LateContext<'_>, init_expr: &Expr<'_>) - && init_expr_can_use_question_mark(cx, init_expr) && let Some(ret) = find_let_else_ret_expression(els) && let Some(inner_pat) = pat_and_expr_can_be_question_mark(cx, pat, ret) - && !span_contains_comment(cx.tcx.sess.source_map(), els.span) + && !span_contains_comment(cx, els.span) && !span_contains_cfg(cx, els.span) { let mut applicability = Applicability::MaybeIncorrect; @@ -501,7 +501,8 @@ fn check_if_let_some_or_err_and_early_return<'tcx>(cx: &LateContext<'tcx>, expr: let mut applicability = Applicability::MachineApplicable; let receiver_str = snippet_with_applicability(cx, let_expr.span, "..", &mut applicability); - let requires_semi = matches!(cx.tcx.parent_hir_node(expr.hir_id), Node::Stmt(_)); + let parent = cx.tcx.parent_hir_node(expr.hir_id); + let requires_semi = matches!(parent, Node::Stmt(_)) || cx.typeck_results().expr_ty(expr).is_unit(); let method_call_str = match by_ref { ByRef::Yes(_, Mutability::Mut) => ".as_mut()", ByRef::Yes(_, Mutability::Not) => ".as_ref()", @@ -512,7 +513,7 @@ fn check_if_let_some_or_err_and_early_return<'tcx>(cx: &LateContext<'tcx>, expr: "{receiver_str}{method_call_str}?{}", if requires_semi { ";" } else { "" } ); - if is_else_clause(cx.tcx, expr) { + if is_else_clause(cx.tcx, expr) || (requires_semi && !matches!(parent, Node::Stmt(_) | Node::Block(_))) { sugg = format!("{{ {sugg} }}"); } diff --git a/src/tools/clippy/clippy_lints/src/same_length_and_capacity.rs b/src/tools/clippy/clippy_lints/src/same_length_and_capacity.rs index b200fd1fe25f..ebf649c24307 100644 --- a/src/tools/clippy/clippy_lints/src/same_length_and_capacity.rs +++ b/src/tools/clippy/clippy_lints/src/same_length_and_capacity.rs @@ -65,7 +65,7 @@ /// // This time, leverage the previously saved capacity: /// let reconstructed = unsafe { Vec::from_raw_parts(ptr, len, cap) }; /// ``` - #[clippy::version = "1.93.0"] + #[clippy::version = "1.94.0"] pub SAME_LENGTH_AND_CAPACITY, pedantic, "`from_raw_parts` with same length and capacity" diff --git a/src/tools/clippy/clippy_lints/src/semicolon_block.rs b/src/tools/clippy/clippy_lints/src/semicolon_block.rs index 7c68762b22c0..e806123596b8 100644 --- a/src/tools/clippy/clippy_lints/src/semicolon_block.rs +++ b/src/tools/clippy/clippy_lints/src/semicolon_block.rs @@ -170,7 +170,7 @@ fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) { StmtKind::Semi(Expr { kind: ExprKind::Block(block, _), .. - }) if !block.span.from_expansion() => { + }) if !block.span.from_expansion() && !block.targeted_by_break => { let attrs = cx.tcx.hir_attrs(stmt.hir_id); if !attrs.is_empty() && !cx.tcx.features().stmt_expr_attributes() { return; diff --git a/src/tools/clippy/clippy_lints/src/slow_vector_initialization.rs b/src/tools/clippy/clippy_lints/src/slow_vector_initialization.rs index 8524497c387c..c99f2e2fb942 100644 --- a/src/tools/clippy/clippy_lints/src/slow_vector_initialization.rs +++ b/src/tools/clippy/clippy_lints/src/slow_vector_initialization.rs @@ -208,7 +208,7 @@ fn emit_lint(cx: &LateContext<'_>, slow_fill: &Expr<'_>, vec_alloc: &VecAllocati .with_lo(vec_alloc.allocation_expr.span.source_callsite().lo()); // If there is no comment in `span_to_replace`, Clippy can automatically fix the code. - let app = if span_contains_comment(cx.tcx.sess.source_map(), span_to_replace) { + let app = if span_contains_comment(cx, span_to_replace) { Applicability::Unspecified } else { Applicability::MachineApplicable diff --git a/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs b/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs index 44e35b8dc71b..ac2d3741338a 100644 --- a/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs +++ b/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs @@ -7,9 +7,8 @@ use rustc_errors::Applicability; use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_session::declare_lint_pass; -use rustc_span::Span; -use rustc_span::Spanned; use rustc_span::symbol::Ident; +use rustc_span::{Span, Spanned}; declare_clippy_lint! { /// ### What it does diff --git a/src/tools/clippy/clippy_lints/src/undocumented_unsafe_blocks.rs b/src/tools/clippy/clippy_lints/src/undocumented_unsafe_blocks.rs index 60775d1856eb..2bf1d8be4653 100644 --- a/src/tools/clippy/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/src/tools/clippy/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -826,7 +826,9 @@ fn text_has_safety_comment( // Don't lint if the safety comment is part of a codeblock in a doc comment. // It may or may not be required, and we can't very easily check it (and we shouldn't, since // the safety comment isn't referring to the node we're currently checking) - if line.trim_start_matches("///").trim_start().starts_with("```") { + if let Some(doc) = line.strip_prefix("///").or_else(|| line.strip_prefix("//!")) + && doc.trim_start().starts_with("```") + { in_codeblock = !in_codeblock; } diff --git a/src/tools/clippy/clippy_lints/src/useless_vec.rs b/src/tools/clippy/clippy_lints/src/useless_vec.rs index 28c339ce2b7d..7b6122213d60 100644 --- a/src/tools/clippy/clippy_lints/src/useless_vec.rs +++ b/src/tools/clippy/clippy_lints/src/useless_vec.rs @@ -251,7 +251,7 @@ fn check_crate_post(&mut self, cx: &LateContext<'tcx>) { let help_msg = format!("you can use {} directly", suggest_ty.desc()); // If the `vec!` macro contains comment, better not make the suggestion machine applicable as it // would remove them. - let applicability = if span_contains_comment(cx.tcx.sess.source_map(), span) { + let applicability = if span_contains_comment(cx, span) { Applicability::Unspecified } else { Applicability::MachineApplicable diff --git a/src/tools/clippy/clippy_lints/src/write/empty_string.rs b/src/tools/clippy/clippy_lints/src/write/empty_string.rs index 1291f2489a21..fa2ad4ba94a2 100644 --- a/src/tools/clippy/clippy_lints/src/write/empty_string.rs +++ b/src/tools/clippy/clippy_lints/src/write/empty_string.rs @@ -22,7 +22,7 @@ pub(super) fn check(cx: &LateContext<'_>, format_args: &FormatArgs, macro_call: macro_call.span, format!("empty string literal in `{name}!`"), |diag| { - if span_extract_comments(cx.sess().source_map(), macro_call.span).is_empty() { + if span_extract_comments(cx, macro_call.span).is_empty() { let closing_paren = cx.sess().source_map().span_extend_to_prev_char_before( macro_call.span.shrink_to_hi(), ')', diff --git a/src/tools/clippy/clippy_test_deps/Cargo.lock b/src/tools/clippy/clippy_test_deps/Cargo.lock index b22cf9d107d7..63e41e8abd10 100644 --- a/src/tools/clippy/clippy_test_deps/Cargo.lock +++ b/src/tools/clippy/clippy_test_deps/Cargo.lock @@ -55,9 +55,9 @@ checksum = "1b8e56985ec62d17e9c1001dc89c88ecd7dc08e47eba5ec7c29c7b5eeecde967" [[package]] name = "bytes" -version = "1.10.1" +version = "1.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d71b6127be86fdcfddb610f7182ac57211d4b18a3e9c82eb2d17662f2227ad6a" +checksum = "1e748733b7cbc798e1434b6ac524f0c1ff2ab456fe201501e6497c8417a4fc33" [[package]] name = "cfg-if" diff --git a/src/tools/clippy/clippy_utils/README.md b/src/tools/clippy/clippy_utils/README.md index d7e40130380d..b5e2e0fbf6c5 100644 --- a/src/tools/clippy/clippy_utils/README.md +++ b/src/tools/clippy/clippy_utils/README.md @@ -8,7 +8,7 @@ This crate is only guaranteed to build with this `nightly` toolchain: ``` -nightly-2026-03-05 +nightly-2026-03-21 ``` diff --git a/src/tools/clippy/clippy_utils/src/consts.rs b/src/tools/clippy/clippy_utils/src/consts.rs index 1a75bdff8124..51db6b6b64c7 100644 --- a/src/tools/clippy/clippy_utils/src/consts.rs +++ b/src/tools/clippy/clippy_utils/src/consts.rs @@ -841,12 +841,18 @@ fn fetch_path(&self, qpath: &QPath<'_>, id: HirId) -> Option { && ty.span.ctxt() == self.ctxt.get() && ty_name.ident.span.ctxt() == self.ctxt.get() && matches!(ty_path.res, Res::PrimTy(_)) - && let Some((DefKind::AssocConst { .. }, did)) = self.typeck.type_dependent_def(id) => + && let Some((DefKind::AssocConst { .. }, did)) = self.typeck.type_dependent_def(id) + && self.tcx.inherent_impl_of_assoc(did).is_some() => { did }, - _ if let Res::Def(DefKind::Const { .. } | DefKind::AssocConst { .. }, did) = - self.typeck.qpath_res(qpath, id) => + // TODO: revisit when feature `min_generic_const_args` is stabilized. In the meantime, + // `TyCtxt::const_eval_resolve()` will trigger an ICE when evaluating the body of the + // `type const` definition. + _ if let Res::Def( + DefKind::Const { is_type_const: false } | DefKind::AssocConst { is_type_const: false }, + did, + ) = self.typeck.qpath_res(qpath, id) => { self.source.set(ConstantSource::NonLocal); did diff --git a/src/tools/clippy/clippy_utils/src/diagnostics.rs b/src/tools/clippy/clippy_utils/src/diagnostics.rs index 88dd3d96b266..f3c4ae3500af 100644 --- a/src/tools/clippy/clippy_utils/src/diagnostics.rs +++ b/src/tools/clippy/clippy_utils/src/diagnostics.rs @@ -8,7 +8,7 @@ //! Thank you! //! ~The `INTERNAL_METADATA_COLLECTOR` lint -use rustc_errors::{Applicability, Diag, Diagnostic, DiagCtxtHandle, DiagMessage, Level, MultiSpan}; +use rustc_errors::{Applicability, Diag, DiagCtxtHandle, DiagMessage, Diagnostic, Level, MultiSpan}; #[cfg(debug_assertions)] use rustc_errors::{EmissionGuarantee, SubstitutionPart, Suggestions}; use rustc_hir::HirId; @@ -252,15 +252,19 @@ fn into_diag(self, dcx: DiagCtxtHandle<'a>, level: Level) -> Diag<'a, ()> { let sp = sp.into(); #[expect(clippy::disallowed_methods)] - cx.emit_span_lint(lint, sp.clone(), ClippyDiag(|diag: &mut Diag<'_, ()>| { - diag.primary_message(msg); - diag.span(sp); - f(diag); - docs_link(diag, lint); + cx.emit_span_lint( + lint, + sp.clone(), + ClippyDiag(|diag: &mut Diag<'_, ()>| { + diag.primary_message(msg); + diag.span(sp); + f(diag); + docs_link(diag, lint); - #[cfg(debug_assertions)] - validate_diag(diag); - })); + #[cfg(debug_assertions)] + validate_diag(diag); + }), + ); } /// Like [`span_lint`], but emits the lint at the node identified by the given `HirId`. @@ -326,14 +330,19 @@ pub fn span_lint_hir_and_then( f: impl FnOnce(&mut Diag<'_, ()>), ) { #[expect(clippy::disallowed_methods)] - cx.tcx.emit_node_span_lint(lint, hir_id, sp, rustc_errors::DiagDecorator(|diag| { - diag.primary_message(msg); - f(diag); - docs_link(diag, lint); + cx.tcx.emit_node_span_lint( + lint, + hir_id, + sp, + rustc_errors::DiagDecorator(|diag| { + diag.primary_message(msg); + f(diag); + docs_link(diag, lint); - #[cfg(debug_assertions)] - validate_diag(diag); - })); + #[cfg(debug_assertions)] + validate_diag(diag); + }), + ); } /// Add a span lint with a suggestion on how to fix it. diff --git a/src/tools/clippy/clippy_utils/src/hir_utils.rs b/src/tools/clippy/clippy_utils/src/hir_utils.rs index 4e9c64dd63c7..a4d8fd20e4d3 100644 --- a/src/tools/clippy/clippy_utils/src/hir_utils.rs +++ b/src/tools/clippy/clippy_utils/src/hir_utils.rs @@ -26,7 +26,8 @@ /// Callback that is called when two expressions are not equal in the sense of `SpanlessEq`, but /// other conditions would make them equal. -type SpanlessEqCallback<'a> = dyn FnMut(&Expr<'_>, &Expr<'_>) -> bool + 'a; +type SpanlessEqCallback<'a, 'tcx> = + dyn FnMut(&TypeckResults<'tcx>, &Expr<'_>, &TypeckResults<'tcx>, &Expr<'_>) -> bool + 'a; /// Determines how paths are hashed and compared for equality. #[derive(Copy, Clone, Debug, Default)] @@ -59,7 +60,7 @@ pub struct SpanlessEq<'a, 'tcx> { cx: &'a LateContext<'tcx>, maybe_typeck_results: Option<(&'tcx TypeckResults<'tcx>, &'tcx TypeckResults<'tcx>)>, allow_side_effects: bool, - expr_fallback: Option>>, + expr_fallback: Option>>, path_check: PathCheck, } @@ -94,7 +95,10 @@ pub fn paths_by_resolution(self) -> Self { } #[must_use] - pub fn expr_fallback(self, expr_fallback: impl FnMut(&Expr<'_>, &Expr<'_>) -> bool + 'a) -> Self { + pub fn expr_fallback( + self, + expr_fallback: impl FnMut(&TypeckResults<'tcx>, &Expr<'_>, &TypeckResults<'tcx>, &Expr<'_>) -> bool + 'a, + ) -> Self { Self { expr_fallback: Some(Box::new(expr_fallback)), ..self @@ -639,7 +643,15 @@ pub fn eq_expr(&mut self, left: &Expr<'_>, right: &Expr<'_>) -> bool { ) => false, }; (is_eq && (!self.should_ignore(left) || !self.should_ignore(right))) - || self.inner.expr_fallback.as_mut().is_some_and(|f| f(left, right)) + || self + .inner + .maybe_typeck_results + .is_some_and(|(left_typeck_results, right_typeck_results)| { + self.inner + .expr_fallback + .as_mut() + .is_some_and(|f| f(left_typeck_results, left, right_typeck_results, right)) + }) } fn eq_exprs(&mut self, left: &[Expr<'_>], right: &[Expr<'_>]) -> bool { diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs index 1e73e43e0a71..b37558c1a7fc 100644 --- a/src/tools/clippy/clippy_utils/src/lib.rs +++ b/src/tools/clippy/clippy_utils/src/lib.rs @@ -1838,13 +1838,38 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { /// * `|[x, y]| [x, y]` /// * `|Foo(bar, baz)| Foo(bar, baz)` /// * `|Foo { bar, baz }| Foo { bar, baz }` +/// * `|x| { let y = x; ...; let z = y; z }` +/// * `|x| { let y = x; ...; let z = y; return z }` /// /// Consider calling [`is_expr_untyped_identity_function`] or [`is_expr_identity_function`] instead. -fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool { +fn is_body_identity_function<'hir>(cx: &LateContext<'_>, func: &Body<'hir>) -> bool { let [param] = func.params else { return false; }; + let mut param_pat = param.pat; + + // Given a sequence of `Stmt`s of the form `let p = e` where `e` is an expr identical to the + // current `param_pat`, advance the current `param_pat` to `p`. + // + // Note: This is similar to `clippy_utils::get_last_chain_binding_hir_id`, but it works + // directly over a `Pattern` rather than a `HirId`. And it checks for compatibility via + // `is_expr_identity_of_pat` rather than `HirId` equality + let mut advance_param_pat_over_stmts = |stmts: &[Stmt<'hir>]| { + for stmt in stmts { + if let StmtKind::Let(local) = stmt.kind + && let Some(init) = local.init + && is_expr_identity_of_pat(cx, param_pat, init, true) + { + param_pat = local.pat; + } else { + return false; + } + } + + true + }; + let mut expr = func.value; loop { match expr.kind { @@ -1873,7 +1898,30 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool { return false; } }, - _ => return is_expr_identity_of_pat(cx, param.pat, expr, true), + ExprKind::Block( + &Block { + stmts, expr: Some(e), .. + }, + _, + ) => { + if !advance_param_pat_over_stmts(stmts) { + return false; + } + + expr = e; + }, + ExprKind::Block(&Block { stmts, expr: None, .. }, _) => { + if let Some((last_stmt, stmts)) = stmts.split_last() + && advance_param_pat_over_stmts(stmts) + && let StmtKind::Semi(e) | StmtKind::Expr(e) = last_stmt.kind + && let ExprKind::Ret(Some(ret_val)) = e.kind + { + expr = ret_val; + } else { + return false; + } + }, + _ => return is_expr_identity_of_pat(cx, param_pat, expr, true), } } } @@ -2762,16 +2810,15 @@ pub fn tokenize_with_text(s: &str) -> impl Iterator bool { - let Ok(snippet) = sm.span_to_snippet(span) else { - return false; - }; - return tokenize(&snippet, FrontmatterAllowed::No).any(|token| { - matches!( - token.kind, - TokenKind::BlockComment { .. } | TokenKind::LineComment { .. } - ) - }); +pub fn span_contains_comment(cx: &impl source::HasSession, span: Span) -> bool { + span.check_source_text(cx, |snippet| { + tokenize(snippet, FrontmatterAllowed::No).any(|token| { + matches!( + token.kind, + TokenKind::BlockComment { .. } | TokenKind::LineComment { .. } + ) + }) + }) } /// Checks whether a given span has any significant token. A significant token is a non-whitespace @@ -2779,30 +2826,32 @@ pub fn span_contains_comment(sm: &SourceMap, span: Span) -> bool { /// This is useful to determine if there are any actual code tokens in the span that are omitted in /// the late pass, such as platform-specific code. pub fn span_contains_non_whitespace(cx: &impl source::HasSession, span: Span, skip_comments: bool) -> bool { - matches!(span.get_source_text(cx), Some(snippet) if tokenize_with_text(&snippet).any(|(token, _, _)| - match token { + span.check_source_text(cx, |snippet| { + tokenize_with_text(snippet).any(|(token, _, _)| match token { TokenKind::Whitespace => false, TokenKind::BlockComment { .. } | TokenKind::LineComment { .. } => !skip_comments, _ => true, - } - )) + }) + }) } /// Returns all the comments a given span contains /// /// Comments are returned wrapped with their relevant delimiters -pub fn span_extract_comment(sm: &SourceMap, span: Span) -> String { - span_extract_comments(sm, span).join("\n") +pub fn span_extract_comment(cx: &impl source::HasSession, span: Span) -> String { + span_extract_comments(cx, span).join("\n") } /// Returns all the comments a given span contains. /// /// Comments are returned wrapped with their relevant delimiters. -pub fn span_extract_comments(sm: &SourceMap, span: Span) -> Vec { - let snippet = sm.span_to_snippet(span).unwrap_or_default(); - tokenize_with_text(&snippet) - .filter(|(t, ..)| matches!(t, TokenKind::BlockComment { .. } | TokenKind::LineComment { .. })) - .map(|(_, s, _)| s.to_string()) - .collect::>() +pub fn span_extract_comments(cx: &impl source::HasSession, span: Span) -> Vec { + span.with_source_text(cx, |snippet| { + tokenize_with_text(snippet) + .filter(|(t, ..)| matches!(t, TokenKind::BlockComment { .. } | TokenKind::LineComment { .. })) + .map(|(_, s, _)| s.to_string()) + .collect::>() + }) + .unwrap_or_default() } pub fn span_find_starting_semi(sm: &SourceMap, span: Span) -> Span { diff --git a/src/tools/clippy/clippy_utils/src/msrvs.rs b/src/tools/clippy/clippy_utils/src/msrvs.rs index 5aa457b9d19c..ecadc3322e14 100644 --- a/src/tools/clippy/clippy_utils/src/msrvs.rs +++ b/src/tools/clippy/clippy_utils/src/msrvs.rs @@ -23,9 +23,11 @@ macro_rules! msrv_aliases { // names may refer to stabilized feature flags or library items msrv_aliases! { + 1,93,0 { VEC_DEQUE_POP_BACK_IF, VEC_DEQUE_POP_FRONT_IF } 1,91,0 { DURATION_FROM_MINUTES_HOURS } 1,88,0 { LET_CHAINS } 1,87,0 { OS_STR_DISPLAY, INT_MIDPOINT, CONST_CHAR_IS_DIGIT, UNSIGNED_IS_MULTIPLE_OF, INTEGER_SIGN_CAST } + 1,86,0 { VEC_POP_IF } 1,85,0 { UINT_FLOAT_MIDPOINT, CONST_SIZE_OF_VAL } 1,84,0 { CONST_OPTION_AS_SLICE, MANUAL_DANGLING_PTR } 1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY, CONST_MUT_REFS, CONST_UNWRAP } diff --git a/src/tools/clippy/clippy_utils/src/sym.rs b/src/tools/clippy/clippy_utils/src/sym.rs index d6ba678434bc..bb67e4350d2b 100644 --- a/src/tools/clippy/clippy_utils/src/sym.rs +++ b/src/tools/clippy/clippy_utils/src/sym.rs @@ -140,6 +140,7 @@ macro_rules! generate { as_str, assert_failed, author, + back, binaryheap_iter, bool_then, borrow, @@ -295,6 +296,7 @@ macro_rules! generate { from_str_method, from_str_radix, from_weeks, + front, fs, fs_create_dir, fuse, @@ -464,6 +466,12 @@ macro_rules! generate { peekable, permissions_from_mode, pin_macro, + pop, + pop_back, + pop_back_if, + pop_front, + pop_front_if, + pop_if, position, pow, powf, diff --git a/src/tools/clippy/clippy_utils/src/ty/mod.rs b/src/tools/clippy/clippy_utils/src/ty/mod.rs index af514778f968..1ac417a8d692 100644 --- a/src/tools/clippy/clippy_utils/src/ty/mod.rs +++ b/src/tools/clippy/clippy_utils/src/ty/mod.rs @@ -29,8 +29,7 @@ use rustc_trait_selection::traits::query::normalize::QueryNormalizeExt; use rustc_trait_selection::traits::{Obligation, ObligationCause}; use std::collections::hash_map::Entry; -use std::debug_assert_matches; -use std::{iter, mem}; +use std::{debug_assert_matches, iter, mem}; use crate::paths::{PathNS, lookup_path_str}; use crate::res::{MaybeDef, MaybeQPath}; diff --git a/src/tools/clippy/rust-toolchain.toml b/src/tools/clippy/rust-toolchain.toml index c9bb94cededb..1f8bcdea8251 100644 --- a/src/tools/clippy/rust-toolchain.toml +++ b/src/tools/clippy/rust-toolchain.toml @@ -1,6 +1,6 @@ [toolchain] # begin autogenerated nightly -channel = "nightly-2026-03-05" +channel = "nightly-2026-03-21" # end autogenerated nightly components = ["cargo", "llvm-tools", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"] profile = "minimal" diff --git a/src/tools/clippy/tests/ui-internal/disallow_span_lint.rs b/src/tools/clippy/tests/ui-internal/disallow_span_lint.rs index cd69a12f89d4..efb57e6a1b00 100644 --- a/src/tools/clippy/tests/ui-internal/disallow_span_lint.rs +++ b/src/tools/clippy/tests/ui-internal/disallow_span_lint.rs @@ -12,17 +12,26 @@ use rustc_middle::ty::TyCtxt; pub fn a(cx: impl LintContext, lint: &'static Lint, span: impl Into, msg: impl Into) { - cx.span_lint(lint, span, |lint| { + cx.emit_span_lint( //~^ disallowed_methods - lint.primary_message(msg); - }); + lint, + span, + DiagDecorator(|lint| { + lint.primary_message(msg); + }), + ); } pub fn b(tcx: TyCtxt<'_>, lint: &'static Lint, hir_id: HirId, span: impl Into, msg: impl Into) { - tcx.emit_node_span_lint(lint, hir_id, span, DiagDecorator(|lint| { + tcx.emit_node_span_lint( //~^ disallowed_methods - lint.primary_message(msg); - })); + lint, + hir_id, + span, + DiagDecorator(|lint| { + lint.primary_message(msg); + }), + ); } fn main() {} diff --git a/src/tools/clippy/tests/ui-internal/disallow_span_lint.stderr b/src/tools/clippy/tests/ui-internal/disallow_span_lint.stderr index e9d53c64dd9b..8631ddbb2cf4 100644 --- a/src/tools/clippy/tests/ui-internal/disallow_span_lint.stderr +++ b/src/tools/clippy/tests/ui-internal/disallow_span_lint.stderr @@ -1,8 +1,8 @@ -error: use of a disallowed method `rustc_lint::context::LintContext::span_lint` +error: use of a disallowed method `rustc_lint::context::LintContext::emit_span_lint` --> tests/ui-internal/disallow_span_lint.rs:15:8 | -LL | cx.span_lint(lint, span, |lint| { - | ^^^^^^^^^ +LL | cx.emit_span_lint( + | ^^^^^^^^^^^^^^ | = note: this function does not add a link to our documentation; please use the `clippy_utils::diagnostics::span_lint*` functions instead note: the lint level is defined here @@ -11,11 +11,11 @@ note: the lint level is defined here LL | #![deny(clippy::disallowed_methods)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: use of a disallowed method `rustc_middle::ty::context::TyCtxt::node_span_lint` - --> tests/ui-internal/disallow_span_lint.rs:22:9 +error: use of a disallowed method `rustc_middle::ty::context::TyCtxt::emit_node_span_lint` + --> tests/ui-internal/disallow_span_lint.rs:26:9 | -LL | tcx.node_span_lint(lint, hir_id, span, |lint| { - | ^^^^^^^^^^^^^^ +LL | tcx.emit_node_span_lint( + | ^^^^^^^^^^^^^^^^^^^ | = note: this function does not add a link to our documentation; please use the `clippy_utils::diagnostics::span_lint_hir*` functions instead diff --git a/src/tools/clippy/tests/ui/crashes/mgca-16691.rs b/src/tools/clippy/tests/ui/crashes/mgca-16691.rs new file mode 100644 index 000000000000..b7039240770b --- /dev/null +++ b/src/tools/clippy/tests/ui/crashes/mgca-16691.rs @@ -0,0 +1,16 @@ +//@ check-pass +#![allow(incomplete_features)] +#![feature(min_generic_const_args)] + +trait Trait { + type const N: usize; + fn process(); +} + +impl Trait for () { + type const N: usize = 3; + fn process() { + const N: usize = <()>::N; + _ = 0..Self::N; + } +} diff --git a/src/tools/clippy/tests/ui/eta.fixed b/src/tools/clippy/tests/ui/eta.fixed index 41e5bbe3cbed..36300aa56eb3 100644 --- a/src/tools/clippy/tests/ui/eta.fixed +++ b/src/tools/clippy/tests/ui/eta.fixed @@ -13,7 +13,8 @@ clippy::uninlined_format_args, clippy::useless_vec, clippy::unnecessary_map_on_constructor, - clippy::needless_lifetimes + clippy::needless_lifetimes, + clippy::unnecessary_option_map_or_else )] use std::path::{Path, PathBuf}; diff --git a/src/tools/clippy/tests/ui/eta.rs b/src/tools/clippy/tests/ui/eta.rs index de0d3fb63c51..63b441851ec1 100644 --- a/src/tools/clippy/tests/ui/eta.rs +++ b/src/tools/clippy/tests/ui/eta.rs @@ -13,7 +13,8 @@ clippy::uninlined_format_args, clippy::useless_vec, clippy::unnecessary_map_on_constructor, - clippy::needless_lifetimes + clippy::needless_lifetimes, + clippy::unnecessary_option_map_or_else )] use std::path::{Path, PathBuf}; diff --git a/src/tools/clippy/tests/ui/eta.stderr b/src/tools/clippy/tests/ui/eta.stderr index c19947cddafa..b57038963b7e 100644 --- a/src/tools/clippy/tests/ui/eta.stderr +++ b/src/tools/clippy/tests/ui/eta.stderr @@ -1,5 +1,5 @@ error: redundant closure - --> tests/ui/eta.rs:34:27 + --> tests/ui/eta.rs:35:27 | LL | let a = Some(1u8).map(|a| foo(a)); | ^^^^^^^^^^ help: replace the closure with the function itself: `foo` @@ -8,31 +8,31 @@ LL | let a = Some(1u8).map(|a| foo(a)); = help: to override `-D warnings` add `#[allow(clippy::redundant_closure)]` error: redundant closure - --> tests/ui/eta.rs:39:40 + --> tests/ui/eta.rs:40:40 | LL | let _: Option> = true.then(|| vec![]); // special case vec! | ^^^^^^^^^ help: replace the closure with `Vec::new`: `std::vec::Vec::new` error: redundant closure - --> tests/ui/eta.rs:42:35 + --> tests/ui/eta.rs:43:35 | LL | let d = Some(1u8).map(|a| foo((|b| foo2(b))(a))); //is adjusted? | ^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo2` error: redundant closure - --> tests/ui/eta.rs:45:26 + --> tests/ui/eta.rs:46:26 | LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted | ^^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `below` error: redundant closure - --> tests/ui/eta.rs:54:27 + --> tests/ui/eta.rs:55:27 | LL | let e = Some(1u8).map(|a| generic(a)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `generic` error: redundant closure - --> tests/ui/eta.rs:107:51 + --> tests/ui/eta.rs:108:51 | LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo()); | ^^^^^^^^^^^ help: replace the closure with the method itself: `TestStruct::foo` @@ -41,229 +41,229 @@ LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo()); = help: to override `-D warnings` add `#[allow(clippy::redundant_closure_for_method_calls)]` error: redundant closure - --> tests/ui/eta.rs:109:51 + --> tests/ui/eta.rs:110:51 | LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.trait_foo()); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `TestTrait::trait_foo` error: redundant closure - --> tests/ui/eta.rs:112:42 + --> tests/ui/eta.rs:113:42 | LL | let e = Some(&mut vec![1, 2, 3]).map(|v| v.clear()); | ^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::vec::Vec::clear` error: redundant closure - --> tests/ui/eta.rs:117:29 + --> tests/ui/eta.rs:118:29 | LL | let e = Some("str").map(|s| s.to_string()); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::string::ToString::to_string` error: redundant closure - --> tests/ui/eta.rs:119:27 + --> tests/ui/eta.rs:120:27 | LL | let e = Some('a').map(|s| s.to_uppercase()); | ^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_uppercase` error: redundant closure - --> tests/ui/eta.rs:122:65 + --> tests/ui/eta.rs:123:65 | LL | let e: std::vec::Vec = vec!['a', 'b', 'c'].iter().map(|c| c.to_ascii_uppercase()).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `char::to_ascii_uppercase` error: redundant closure - --> tests/ui/eta.rs:139:23 + --> tests/ui/eta.rs:140:23 | LL | let _ = x.map(|x| x.parse::()); | ^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `str::parse::` error: redundant closure - --> tests/ui/eta.rs:192:22 + --> tests/ui/eta.rs:193:22 | LL | requires_fn_once(|| x()); | ^^^^^^ help: replace the closure with the function itself: `x` error: redundant closure - --> tests/ui/eta.rs:200:27 + --> tests/ui/eta.rs:201:27 | LL | let a = Some(1u8).map(|a| foo_ptr(a)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `foo_ptr` error: redundant closure - --> tests/ui/eta.rs:206:27 + --> tests/ui/eta.rs:207:27 | LL | let a = Some(1u8).map(|a| closure(a)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `closure` error: redundant closure - --> tests/ui/eta.rs:239:28 + --> tests/ui/eta.rs:240:28 | LL | x.into_iter().for_each(|x| add_to_res(x)); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res` error: redundant closure - --> tests/ui/eta.rs:241:28 + --> tests/ui/eta.rs:242:28 | LL | y.into_iter().for_each(|x| add_to_res(x)); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res` error: redundant closure - --> tests/ui/eta.rs:243:28 + --> tests/ui/eta.rs:244:28 | LL | z.into_iter().for_each(|x| add_to_res(x)); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `add_to_res` error: redundant closure - --> tests/ui/eta.rs:251:21 + --> tests/ui/eta.rs:252:21 | LL | Some(1).map(|n| closure(n)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut closure` error: redundant closure - --> tests/ui/eta.rs:256:21 + --> tests/ui/eta.rs:257:21 | LL | Some(1).map(|n| in_loop(n)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `in_loop` error: redundant closure - --> tests/ui/eta.rs:350:18 + --> tests/ui/eta.rs:351:18 | LL | takes_fn_mut(|| f()); | ^^^^^^ help: replace the closure with the function itself: `&mut f` error: redundant closure - --> tests/ui/eta.rs:354:19 + --> tests/ui/eta.rs:355:19 | LL | takes_fn_once(|| f()); | ^^^^^^ help: replace the closure with the function itself: `&mut f` error: redundant closure - --> tests/ui/eta.rs:359:26 + --> tests/ui/eta.rs:360:26 | LL | move || takes_fn_mut(|| f_used_once()) | ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut f_used_once` error: redundant closure - --> tests/ui/eta.rs:372:19 + --> tests/ui/eta.rs:373:19 | LL | array_opt.map(|a| a.as_slice()); | ^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8; 3]>::as_slice` error: redundant closure - --> tests/ui/eta.rs:376:19 + --> tests/ui/eta.rs:377:19 | LL | slice_opt.map(|s| s.len()); | ^^^^^^^^^^^ help: replace the closure with the method itself: `<[u8]>::len` error: redundant closure - --> tests/ui/eta.rs:380:17 + --> tests/ui/eta.rs:381:17 | LL | ptr_opt.map(|p| p.is_null()); | ^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `<*const usize>::is_null` error: redundant closure - --> tests/ui/eta.rs:385:17 + --> tests/ui/eta.rs:386:17 | LL | dyn_opt.map(|d| d.method_on_dyn()); | ^^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `::method_on_dyn` error: redundant closure - --> tests/ui/eta.rs:446:19 + --> tests/ui/eta.rs:447:19 | LL | let _ = f(&0, |x, y| f2(x, y)); | ^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `f2` error: redundant closure - --> tests/ui/eta.rs:475:22 + --> tests/ui/eta.rs:476:22 | LL | test.map(|t| t.method()) | ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `Test::method` error: redundant closure - --> tests/ui/eta.rs:480:22 + --> tests/ui/eta.rs:481:22 | LL | test.map(|t| t.method()) | ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `super::Outer::method` error: redundant closure - --> tests/ui/eta.rs:494:18 + --> tests/ui/eta.rs:495:18 | LL | test.map(|t| t.method()) | ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `test_mod::Test::method` error: redundant closure - --> tests/ui/eta.rs:502:30 + --> tests/ui/eta.rs:503:30 | LL | test.map(|t| t.method()) | ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `crate::issue_10854::d::Test::method` error: redundant closure - --> tests/ui/eta.rs:522:38 + --> tests/ui/eta.rs:523:38 | LL | let x = Box::new(|| None.map(|x| f(x))); | ^^^^^^^^ help: replace the closure with the function itself: `&f` error: redundant closure - --> tests/ui/eta.rs:527:38 + --> tests/ui/eta.rs:528:38 | LL | let x = Box::new(|| None.map(|x| f(x))); | ^^^^^^^^ help: replace the closure with the function itself: `f` error: redundant closure - --> tests/ui/eta.rs:545:35 + --> tests/ui/eta.rs:546:35 | LL | let _field = bind.or_else(|| get_default()).unwrap(); | ^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `get_default` error: redundant closure - --> tests/ui/eta.rs:592:16 + --> tests/ui/eta.rs:593:16 | LL | accepts_fn(|| f()); | ^^^^^^ help: replace the closure with the function itself: `**f` error: redundant closure - --> tests/ui/eta.rs:596:16 + --> tests/ui/eta.rs:597:16 | LL | accepts_fn(|| g()); | ^^^^^^ help: replace the closure with the function itself: `g` error: redundant closure - --> tests/ui/eta.rs:609:16 + --> tests/ui/eta.rs:610:16 | LL | accepts_fn(|| b()); | ^^^^^^ help: replace the closure with the function itself: `***b` error: redundant closure - --> tests/ui/eta.rs:633:14 + --> tests/ui/eta.rs:634:14 | LL | .map(|n| MyError::A(n)) | ^^^^^^^^^^^^^^^^^ help: replace the closure with the tuple variant itself: `MyError::A` error: redundant closure - --> tests/ui/eta.rs:630:14 + --> tests/ui/eta.rs:631:14 | LL | .map(|n| S(n)) | ^^^^^^^^ help: replace the closure with the tuple struct itself: `S` error: redundant closure - --> tests/ui/eta.rs:627:14 + --> tests/ui/eta.rs:628:14 | LL | .map(|n| g(n)) | ^^^^^^^^ help: replace the closure with the function itself: `g` error: redundant closure - --> tests/ui/eta.rs:624:14 + --> tests/ui/eta.rs:625:14 | LL | .map(|n| f(n)) | ^^^^^^^^ help: replace the closure with the function itself: `f` error: redundant closure - --> tests/ui/eta.rs:651:31 + --> tests/ui/eta.rs:652:31 | LL | array.iter().for_each(|item| item.method()); | ^^^^^^^^^^^^^^^^^^^^ help: replace the closure with the method itself: `Self::method` error: redundant closure - --> tests/ui/eta.rs:661:38 + --> tests/ui/eta.rs:662:38 | LL | (0..10).flat_map(|x| (0..10).map(|y| closure(y))).count(); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&*closure` diff --git a/src/tools/clippy/tests/ui/manual_pop_if.fixed b/src/tools/clippy/tests/ui/manual_pop_if.fixed new file mode 100644 index 000000000000..6e30786c8097 --- /dev/null +++ b/src/tools/clippy/tests/ui/manual_pop_if.fixed @@ -0,0 +1,247 @@ +#![warn(clippy::manual_pop_if)] +#![allow(clippy::collapsible_if, clippy::redundant_closure)] + +use std::collections::VecDeque; +use std::marker::PhantomData; + +// FakeVec has the same methods as Vec but isn't actually a Vec +struct FakeVec(PhantomData); + +impl FakeVec { + fn last(&self) -> Option<&T> { + None + } + + fn pop(&mut self) -> Option { + None + } +} + +fn is_some_and_pattern_positive(mut vec: Vec, mut deque: VecDeque) { + vec.pop_if(|x| *x > 2); + + vec.pop_if(|x| *x > 2); + + deque.pop_back_if(|x| *x > 2); + + deque.pop_front_if(|x| *x > 2); +} + +fn is_some_and_pattern_negative(mut vec: Vec, mut deque: VecDeque) { + // Do not lint, different vectors + let mut vec2 = vec![0]; + if vec.last().is_some_and(|x| *x > 2) { + vec2.pop().unwrap(); + } + + // Do not lint, non-Vec type + let mut fake_vec: FakeVec = FakeVec(PhantomData); + if fake_vec.last().is_some_and(|x| *x > 2) { + fake_vec.pop().unwrap(); + } + + // Do not lint, else-if branch + if false { + // something + } else if vec.last().is_some_and(|x| *x > 2) { + vec.pop().unwrap(); + } + + // Do not lint, value used in let binding + if vec.last().is_some_and(|x| *x > 2) { + let _value = vec.pop().unwrap(); + println!("Popped: {}", _value); + } + + // Do not lint, value used in expression + if vec.last().is_some_and(|x| *x > 2) { + println!("Popped: {}", vec.pop().unwrap()); + } + + // Do not lint, else block + let _result = if vec.last().is_some_and(|x| *x > 2) { + vec.pop().unwrap() + } else { + 0 + }; +} + +fn if_let_pattern_positive(mut vec: Vec, mut deque: VecDeque) { + vec.pop_if(|x| *x > 2); + + vec.pop_if(|x| *x > 2); + + deque.pop_back_if(|x| *x > 2); + + deque.pop_front_if(|x| *x > 2); +} + +fn if_let_pattern_negative(mut vec: Vec) { + // Do not lint, different vectors + let mut vec2 = vec![0]; + if let Some(x) = vec.last() { + if *x > 2 { + vec2.pop().unwrap(); + } + } + + // Do not lint, intervening statements + if let Some(x) = vec.last() { + println!("Checking {}", x); + if *x > 2 { + vec.pop().unwrap(); + } + } + + // Do not lint, bound variable not used in condition + if let Some(_x) = vec.last() { + if vec.len() > 2 { + vec.pop().unwrap(); + } + } + + // Do not lint, value used in let binding + if let Some(x) = vec.last() { + if *x > 2 { + let _val = vec.pop().unwrap(); + } + } + + // Do not lint, else block + let _result = if let Some(x) = vec.last() { + if *x > 2 { vec.pop().unwrap() } else { 0 } + } else { + 0 + }; +} + +fn let_chain_pattern_positive(mut vec: Vec, mut deque: VecDeque) { + vec.pop_if(|x| *x > 2); + + vec.pop_if(|x| *x > 2); + + deque.pop_back_if(|x| *x > 2); + + deque.pop_front_if(|x| *x > 2); +} + +fn let_chain_pattern_negative(mut vec: Vec) { + // Do not lint, different vectors + let mut vec2 = vec![0]; + if let Some(x) = vec.last() + && *x > 2 + { + vec2.pop().unwrap(); + } + + // Do not lint, bound variable not used in condition + if let Some(_x) = vec.last() + && vec.len() > 2 + { + vec.pop().unwrap(); + } + + // Do not lint, value used in let binding + if let Some(x) = vec.last() + && *x > 2 + { + let _val = vec.pop().unwrap(); + } + + // Do not lint, value used in expression + if let Some(x) = vec.last() + && *x > 2 + { + println!("Popped: {}", vec.pop().unwrap()); + } + + // Do not lint, else block + let _result = if let Some(x) = vec.last() + && *x > 2 + { + vec.pop().unwrap() + } else { + 0 + }; +} + +fn map_unwrap_or_pattern_positive(mut vec: Vec, mut deque: VecDeque) { + vec.pop_if(|x| *x > 2); + + vec.pop_if(|x| *x > 2); + + deque.pop_back_if(|x| *x > 2); + + deque.pop_front_if(|x| *x > 2); +} + +fn map_unwrap_or_pattern_negative(mut vec: Vec) { + // Do not lint, unwrap_or(true) instead of false + if vec.last().map(|x| *x > 2).unwrap_or(true) { + vec.pop().unwrap(); + } + + // Do not lint, different vectors + let mut vec2 = vec![0]; + if vec.last().map(|x| *x > 2).unwrap_or(false) { + vec2.pop().unwrap(); + } + + // Do not lint, non-Vec type + let mut fake_vec: FakeVec = FakeVec(PhantomData); + if fake_vec.last().map(|x| *x > 2).unwrap_or(false) { + fake_vec.pop().unwrap(); + } + + // Do not lint, map returns non-boolean + if vec.last().map(|x| x + 1).unwrap_or(0) > 2 { + vec.pop().unwrap(); + } + + // Do not lint, value used in let binding + if vec.last().map(|x| *x > 2).unwrap_or(false) { + let _val = vec.pop().unwrap(); + } + + // Do not lint, else block + let _result = if vec.last().map(|x| *x > 2).unwrap_or(false) { + vec.pop().unwrap() + } else { + 0 + }; +} + +// this makes sure we do not expand vec![] in the suggestion +fn handle_macro_in_closure(mut vec: Vec>) { + vec.pop_if(|e| *e == vec![1]); +} + +#[clippy::msrv = "1.85.0"] +fn msrv_too_low_vec(mut vec: Vec) { + if vec.last().is_some_and(|x| *x > 2) { + vec.pop().unwrap(); + } +} + +#[clippy::msrv = "1.92.0"] +fn msrv_too_low_vecdeque(mut deque: VecDeque) { + if deque.back().is_some_and(|x| *x > 2) { + deque.pop_back().unwrap(); + } + + if deque.front().is_some_and(|x| *x > 2) { + deque.pop_front().unwrap(); + } +} + +#[clippy::msrv = "1.86.0"] +fn msrv_high_enough_vec(mut vec: Vec) { + vec.pop_if(|x| *x > 2); +} + +#[clippy::msrv = "1.93.0"] +fn msrv_high_enough_vecdeque(mut deque: VecDeque) { + deque.pop_back_if(|x| *x > 2); + + deque.pop_front_if(|x| *x > 2); +} diff --git a/src/tools/clippy/tests/ui/manual_pop_if.rs b/src/tools/clippy/tests/ui/manual_pop_if.rs new file mode 100644 index 000000000000..a2f679dfc6b3 --- /dev/null +++ b/src/tools/clippy/tests/ui/manual_pop_if.rs @@ -0,0 +1,319 @@ +#![warn(clippy::manual_pop_if)] +#![allow(clippy::collapsible_if, clippy::redundant_closure)] + +use std::collections::VecDeque; +use std::marker::PhantomData; + +// FakeVec has the same methods as Vec but isn't actually a Vec +struct FakeVec(PhantomData); + +impl FakeVec { + fn last(&self) -> Option<&T> { + None + } + + fn pop(&mut self) -> Option { + None + } +} + +fn is_some_and_pattern_positive(mut vec: Vec, mut deque: VecDeque) { + if vec.last().is_some_and(|x| *x > 2) { + //~^ manual_pop_if + vec.pop().unwrap(); + } + + if vec.last().is_some_and(|x| *x > 2) { + //~^ manual_pop_if + vec.pop().expect("element"); + } + + if deque.back().is_some_and(|x| *x > 2) { + //~^ manual_pop_if + deque.pop_back().unwrap(); + } + + if deque.front().is_some_and(|x| *x > 2) { + //~^ manual_pop_if + deque.pop_front().unwrap(); + } +} + +fn is_some_and_pattern_negative(mut vec: Vec, mut deque: VecDeque) { + // Do not lint, different vectors + let mut vec2 = vec![0]; + if vec.last().is_some_and(|x| *x > 2) { + vec2.pop().unwrap(); + } + + // Do not lint, non-Vec type + let mut fake_vec: FakeVec = FakeVec(PhantomData); + if fake_vec.last().is_some_and(|x| *x > 2) { + fake_vec.pop().unwrap(); + } + + // Do not lint, else-if branch + if false { + // something + } else if vec.last().is_some_and(|x| *x > 2) { + vec.pop().unwrap(); + } + + // Do not lint, value used in let binding + if vec.last().is_some_and(|x| *x > 2) { + let _value = vec.pop().unwrap(); + println!("Popped: {}", _value); + } + + // Do not lint, value used in expression + if vec.last().is_some_and(|x| *x > 2) { + println!("Popped: {}", vec.pop().unwrap()); + } + + // Do not lint, else block + let _result = if vec.last().is_some_and(|x| *x > 2) { + vec.pop().unwrap() + } else { + 0 + }; +} + +fn if_let_pattern_positive(mut vec: Vec, mut deque: VecDeque) { + if let Some(x) = vec.last() { + //~^ manual_pop_if + if *x > 2 { + vec.pop().unwrap(); + } + } + + if let Some(x) = vec.last() { + //~^ manual_pop_if + if *x > 2 { + vec.pop().expect("element"); + } + } + + if let Some(x) = deque.back() { + //~^ manual_pop_if + if *x > 2 { + deque.pop_back().unwrap(); + } + } + + if let Some(x) = deque.front() { + //~^ manual_pop_if + if *x > 2 { + deque.pop_front().unwrap(); + } + } +} + +fn if_let_pattern_negative(mut vec: Vec) { + // Do not lint, different vectors + let mut vec2 = vec![0]; + if let Some(x) = vec.last() { + if *x > 2 { + vec2.pop().unwrap(); + } + } + + // Do not lint, intervening statements + if let Some(x) = vec.last() { + println!("Checking {}", x); + if *x > 2 { + vec.pop().unwrap(); + } + } + + // Do not lint, bound variable not used in condition + if let Some(_x) = vec.last() { + if vec.len() > 2 { + vec.pop().unwrap(); + } + } + + // Do not lint, value used in let binding + if let Some(x) = vec.last() { + if *x > 2 { + let _val = vec.pop().unwrap(); + } + } + + // Do not lint, else block + let _result = if let Some(x) = vec.last() { + if *x > 2 { vec.pop().unwrap() } else { 0 } + } else { + 0 + }; +} + +fn let_chain_pattern_positive(mut vec: Vec, mut deque: VecDeque) { + if let Some(x) = vec.last() //~ manual_pop_if + && *x > 2 + { + vec.pop().unwrap(); + } + + if let Some(x) = vec.last() //~ manual_pop_if + && *x > 2 + { + vec.pop().expect("element"); + } + + if let Some(x) = deque.back() //~ manual_pop_if + && *x > 2 + { + deque.pop_back().unwrap(); + } + + if let Some(x) = deque.front() //~ manual_pop_if + && *x > 2 + { + deque.pop_front().unwrap(); + } +} + +fn let_chain_pattern_negative(mut vec: Vec) { + // Do not lint, different vectors + let mut vec2 = vec![0]; + if let Some(x) = vec.last() + && *x > 2 + { + vec2.pop().unwrap(); + } + + // Do not lint, bound variable not used in condition + if let Some(_x) = vec.last() + && vec.len() > 2 + { + vec.pop().unwrap(); + } + + // Do not lint, value used in let binding + if let Some(x) = vec.last() + && *x > 2 + { + let _val = vec.pop().unwrap(); + } + + // Do not lint, value used in expression + if let Some(x) = vec.last() + && *x > 2 + { + println!("Popped: {}", vec.pop().unwrap()); + } + + // Do not lint, else block + let _result = if let Some(x) = vec.last() + && *x > 2 + { + vec.pop().unwrap() + } else { + 0 + }; +} + +fn map_unwrap_or_pattern_positive(mut vec: Vec, mut deque: VecDeque) { + if vec.last().map(|x| *x > 2).unwrap_or(false) { + //~^ manual_pop_if + vec.pop().unwrap(); + } + + if vec.last().map(|x| *x > 2).unwrap_or(false) { + //~^ manual_pop_if + vec.pop().expect("element"); + } + + if deque.back().map(|x| *x > 2).unwrap_or(false) { + //~^ manual_pop_if + deque.pop_back().unwrap(); + } + + if deque.front().map(|x| *x > 2).unwrap_or(false) { + //~^ manual_pop_if + deque.pop_front().unwrap(); + } +} + +fn map_unwrap_or_pattern_negative(mut vec: Vec) { + // Do not lint, unwrap_or(true) instead of false + if vec.last().map(|x| *x > 2).unwrap_or(true) { + vec.pop().unwrap(); + } + + // Do not lint, different vectors + let mut vec2 = vec![0]; + if vec.last().map(|x| *x > 2).unwrap_or(false) { + vec2.pop().unwrap(); + } + + // Do not lint, non-Vec type + let mut fake_vec: FakeVec = FakeVec(PhantomData); + if fake_vec.last().map(|x| *x > 2).unwrap_or(false) { + fake_vec.pop().unwrap(); + } + + // Do not lint, map returns non-boolean + if vec.last().map(|x| x + 1).unwrap_or(0) > 2 { + vec.pop().unwrap(); + } + + // Do not lint, value used in let binding + if vec.last().map(|x| *x > 2).unwrap_or(false) { + let _val = vec.pop().unwrap(); + } + + // Do not lint, else block + let _result = if vec.last().map(|x| *x > 2).unwrap_or(false) { + vec.pop().unwrap() + } else { + 0 + }; +} + +// this makes sure we do not expand vec![] in the suggestion +fn handle_macro_in_closure(mut vec: Vec>) { + if vec.last().is_some_and(|e| *e == vec![1]) { + //~^ manual_pop_if + vec.pop().unwrap(); + } +} + +#[clippy::msrv = "1.85.0"] +fn msrv_too_low_vec(mut vec: Vec) { + if vec.last().is_some_and(|x| *x > 2) { + vec.pop().unwrap(); + } +} + +#[clippy::msrv = "1.92.0"] +fn msrv_too_low_vecdeque(mut deque: VecDeque) { + if deque.back().is_some_and(|x| *x > 2) { + deque.pop_back().unwrap(); + } + + if deque.front().is_some_and(|x| *x > 2) { + deque.pop_front().unwrap(); + } +} + +#[clippy::msrv = "1.86.0"] +fn msrv_high_enough_vec(mut vec: Vec) { + if vec.last().is_some_and(|x| *x > 2) { + //~^ manual_pop_if + vec.pop().unwrap(); + } +} + +#[clippy::msrv = "1.93.0"] +fn msrv_high_enough_vecdeque(mut deque: VecDeque) { + if deque.back().is_some_and(|x| *x > 2) { + //~^ manual_pop_if + deque.pop_back().unwrap(); + } + + if deque.front().is_some_and(|x| *x > 2) { + //~^ manual_pop_if + deque.pop_front().unwrap(); + } +} diff --git a/src/tools/clippy/tests/ui/manual_pop_if.stderr b/src/tools/clippy/tests/ui/manual_pop_if.stderr new file mode 100644 index 000000000000..d57e20dc6205 --- /dev/null +++ b/src/tools/clippy/tests/ui/manual_pop_if.stderr @@ -0,0 +1,197 @@ +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if.rs:21:5 + | +LL | / if vec.last().is_some_and(|x| *x > 2) { +LL | | +LL | | vec.pop().unwrap(); +LL | | } + | |_____^ help: try: `vec.pop_if(|x| *x > 2);` + | + = note: `-D clippy::manual-pop-if` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::manual_pop_if)]` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if.rs:26:5 + | +LL | / if vec.last().is_some_and(|x| *x > 2) { +LL | | +LL | | vec.pop().expect("element"); +LL | | } + | |_____^ help: try: `vec.pop_if(|x| *x > 2);` + +error: manual implementation of `VecDeque::pop_back_if` + --> tests/ui/manual_pop_if.rs:31:5 + | +LL | / if deque.back().is_some_and(|x| *x > 2) { +LL | | +LL | | deque.pop_back().unwrap(); +LL | | } + | |_____^ help: try: `deque.pop_back_if(|x| *x > 2);` + +error: manual implementation of `VecDeque::pop_front_if` + --> tests/ui/manual_pop_if.rs:36:5 + | +LL | / if deque.front().is_some_and(|x| *x > 2) { +LL | | +LL | | deque.pop_front().unwrap(); +LL | | } + | |_____^ help: try: `deque.pop_front_if(|x| *x > 2);` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if.rs:82:5 + | +LL | / if let Some(x) = vec.last() { +LL | | +LL | | if *x > 2 { +LL | | vec.pop().unwrap(); +LL | | } +LL | | } + | |_____^ help: try: `vec.pop_if(|x| *x > 2);` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if.rs:89:5 + | +LL | / if let Some(x) = vec.last() { +LL | | +LL | | if *x > 2 { +LL | | vec.pop().expect("element"); +LL | | } +LL | | } + | |_____^ help: try: `vec.pop_if(|x| *x > 2);` + +error: manual implementation of `VecDeque::pop_back_if` + --> tests/ui/manual_pop_if.rs:96:5 + | +LL | / if let Some(x) = deque.back() { +LL | | +LL | | if *x > 2 { +LL | | deque.pop_back().unwrap(); +LL | | } +LL | | } + | |_____^ help: try: `deque.pop_back_if(|x| *x > 2);` + +error: manual implementation of `VecDeque::pop_front_if` + --> tests/ui/manual_pop_if.rs:103:5 + | +LL | / if let Some(x) = deque.front() { +LL | | +LL | | if *x > 2 { +LL | | deque.pop_front().unwrap(); +LL | | } +LL | | } + | |_____^ help: try: `deque.pop_front_if(|x| *x > 2);` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if.rs:151:5 + | +LL | / if let Some(x) = vec.last() +LL | | && *x > 2 +LL | | { +LL | | vec.pop().unwrap(); +LL | | } + | |_____^ help: try: `vec.pop_if(|x| *x > 2);` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if.rs:157:5 + | +LL | / if let Some(x) = vec.last() +LL | | && *x > 2 +LL | | { +LL | | vec.pop().expect("element"); +LL | | } + | |_____^ help: try: `vec.pop_if(|x| *x > 2);` + +error: manual implementation of `VecDeque::pop_back_if` + --> tests/ui/manual_pop_if.rs:163:5 + | +LL | / if let Some(x) = deque.back() +LL | | && *x > 2 +LL | | { +LL | | deque.pop_back().unwrap(); +LL | | } + | |_____^ help: try: `deque.pop_back_if(|x| *x > 2);` + +error: manual implementation of `VecDeque::pop_front_if` + --> tests/ui/manual_pop_if.rs:169:5 + | +LL | / if let Some(x) = deque.front() +LL | | && *x > 2 +LL | | { +LL | | deque.pop_front().unwrap(); +LL | | } + | |_____^ help: try: `deque.pop_front_if(|x| *x > 2);` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if.rs:217:5 + | +LL | / if vec.last().map(|x| *x > 2).unwrap_or(false) { +LL | | +LL | | vec.pop().unwrap(); +LL | | } + | |_____^ help: try: `vec.pop_if(|x| *x > 2);` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if.rs:222:5 + | +LL | / if vec.last().map(|x| *x > 2).unwrap_or(false) { +LL | | +LL | | vec.pop().expect("element"); +LL | | } + | |_____^ help: try: `vec.pop_if(|x| *x > 2);` + +error: manual implementation of `VecDeque::pop_back_if` + --> tests/ui/manual_pop_if.rs:227:5 + | +LL | / if deque.back().map(|x| *x > 2).unwrap_or(false) { +LL | | +LL | | deque.pop_back().unwrap(); +LL | | } + | |_____^ help: try: `deque.pop_back_if(|x| *x > 2);` + +error: manual implementation of `VecDeque::pop_front_if` + --> tests/ui/manual_pop_if.rs:232:5 + | +LL | / if deque.front().map(|x| *x > 2).unwrap_or(false) { +LL | | +LL | | deque.pop_front().unwrap(); +LL | | } + | |_____^ help: try: `deque.pop_front_if(|x| *x > 2);` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if.rs:276:5 + | +LL | / if vec.last().is_some_and(|e| *e == vec![1]) { +LL | | +LL | | vec.pop().unwrap(); +LL | | } + | |_____^ help: try: `vec.pop_if(|e| *e == vec![1]);` + +error: manual implementation of `Vec::pop_if` + --> tests/ui/manual_pop_if.rs:302:5 + | +LL | / if vec.last().is_some_and(|x| *x > 2) { +LL | | +LL | | vec.pop().unwrap(); +LL | | } + | |_____^ help: try: `vec.pop_if(|x| *x > 2);` + +error: manual implementation of `VecDeque::pop_back_if` + --> tests/ui/manual_pop_if.rs:310:5 + | +LL | / if deque.back().is_some_and(|x| *x > 2) { +LL | | +LL | | deque.pop_back().unwrap(); +LL | | } + | |_____^ help: try: `deque.pop_back_if(|x| *x > 2);` + +error: manual implementation of `VecDeque::pop_front_if` + --> tests/ui/manual_pop_if.rs:315:5 + | +LL | / if deque.front().is_some_and(|x| *x > 2) { +LL | | +LL | | deque.pop_front().unwrap(); +LL | | } + | |_____^ help: try: `deque.pop_front_if(|x| *x > 2);` + +error: aborting due to 20 previous errors + diff --git a/src/tools/clippy/tests/ui/match_same_arms.fixed b/src/tools/clippy/tests/ui/match_same_arms.fixed index 31684a5759fe..b0ab833c7835 100644 --- a/src/tools/clippy/tests/ui/match_same_arms.fixed +++ b/src/tools/clippy/tests/ui/match_same_arms.fixed @@ -140,3 +140,35 @@ fn main() { _ => false, }; } + +fn issue16678() { + // ICE in Rust 1.94.0 + match true { + true => { + fn wrapper(_arg: ()) { + _arg; + } + }, + false => { + fn wrapper(_arg: ()) { + _arg; + } + }, + } +} + +fn issue16698() { + trait Foo { + const X: u8; + } + impl Foo for u8 { + const X: u8 = 2; + } + impl Foo for i8 { + const X: u8 = 2; + } + match true { + false => u8::X, + true => i8::X, + }; +} diff --git a/src/tools/clippy/tests/ui/match_same_arms.rs b/src/tools/clippy/tests/ui/match_same_arms.rs index 39bee01bac22..9c7899122afb 100644 --- a/src/tools/clippy/tests/ui/match_same_arms.rs +++ b/src/tools/clippy/tests/ui/match_same_arms.rs @@ -149,3 +149,35 @@ fn main() { _ => false, }; } + +fn issue16678() { + // ICE in Rust 1.94.0 + match true { + true => { + fn wrapper(_arg: ()) { + _arg; + } + }, + false => { + fn wrapper(_arg: ()) { + _arg; + } + }, + } +} + +fn issue16698() { + trait Foo { + const X: u8; + } + impl Foo for u8 { + const X: u8 = 2; + } + impl Foo for i8 { + const X: u8 = 2; + } + match true { + false => u8::X, + true => i8::X, + }; +} diff --git a/src/tools/clippy/tests/ui/question_mark.fixed b/src/tools/clippy/tests/ui/question_mark.fixed index 102517d34c61..e209da5c8258 100644 --- a/src/tools/clippy/tests/ui/question_mark.fixed +++ b/src/tools/clippy/tests/ui/question_mark.fixed @@ -524,3 +524,16 @@ fn issue16429(b: i32) -> Option { Some(0) } + +fn issue16654() -> Result<(), i32> { + let result = func_returning_result(); + + #[allow(clippy::collapsible_if)] + if true { + result?; + } + + _ = [{ result?; }]; + + Ok(()) +} diff --git a/src/tools/clippy/tests/ui/question_mark.rs b/src/tools/clippy/tests/ui/question_mark.rs index cfea1277fe76..579b51461d13 100644 --- a/src/tools/clippy/tests/ui/question_mark.rs +++ b/src/tools/clippy/tests/ui/question_mark.rs @@ -649,3 +649,22 @@ fn issue16429(b: i32) -> Option { Some(0) } + +fn issue16654() -> Result<(), i32> { + let result = func_returning_result(); + + #[allow(clippy::collapsible_if)] + if true { + if let Err(err) = result { + //~^ question_mark + return Err(err); + } + } + + _ = [if let Err(err) = result { + //~^ question_mark + return Err(err); + }]; + + Ok(()) +} diff --git a/src/tools/clippy/tests/ui/question_mark.stderr b/src/tools/clippy/tests/ui/question_mark.stderr index c243f12de040..1d7f665a2662 100644 --- a/src/tools/clippy/tests/ui/question_mark.stderr +++ b/src/tools/clippy/tests/ui/question_mark.stderr @@ -362,5 +362,24 @@ LL | | return None; LL | | }; | |_____^ help: replace it with: `{ a? }` -error: aborting due to 38 previous errors +error: this block may be rewritten with the `?` operator + --> tests/ui/question_mark.rs:658:9 + | +LL | / if let Err(err) = result { +LL | | +LL | | return Err(err); +LL | | } + | |_________^ help: replace it with: `result?;` + +error: this block may be rewritten with the `?` operator + --> tests/ui/question_mark.rs:664:10 + | +LL | _ = [if let Err(err) = result { + | __________^ +LL | | +LL | | return Err(err); +LL | | }]; + | |_____^ help: replace it with: `{ result?; }` + +error: aborting due to 40 previous errors diff --git a/src/tools/clippy/tests/ui/semicolon_inside_block.fixed b/src/tools/clippy/tests/ui/semicolon_inside_block.fixed index 468f0a5b1e47..c7174881a4fa 100644 --- a/src/tools/clippy/tests/ui/semicolon_inside_block.fixed +++ b/src/tools/clippy/tests/ui/semicolon_inside_block.fixed @@ -7,6 +7,7 @@ clippy::double_parens )] #![warn(clippy::semicolon_inside_block)] +#![feature(try_blocks)] macro_rules! m { (()) => { @@ -106,3 +107,11 @@ pub fn issue15388() { #[rustfmt::skip] {0; 0}; } + +fn issue_try_blocks() { + // try blocks should NOT trigger semicolon_inside_block: + // moving `;` inside changes the return type (e.g. `Option` -> `Option<()>`) + // which in turn changes the type constraints on `?` operators inside the block, + // causing type errors. + try { Some(1)? }; +} diff --git a/src/tools/clippy/tests/ui/semicolon_inside_block.rs b/src/tools/clippy/tests/ui/semicolon_inside_block.rs index 101374af2647..25be9bee4f53 100644 --- a/src/tools/clippy/tests/ui/semicolon_inside_block.rs +++ b/src/tools/clippy/tests/ui/semicolon_inside_block.rs @@ -7,6 +7,7 @@ clippy::double_parens )] #![warn(clippy::semicolon_inside_block)] +#![feature(try_blocks)] macro_rules! m { (()) => { @@ -106,3 +107,11 @@ pub fn issue15388() { #[rustfmt::skip] {0; 0}; } + +fn issue_try_blocks() { + // try blocks should NOT trigger semicolon_inside_block: + // moving `;` inside changes the return type (e.g. `Option` -> `Option<()>`) + // which in turn changes the type constraints on `?` operators inside the block, + // causing type errors. + try { Some(1)? }; +} diff --git a/src/tools/clippy/tests/ui/semicolon_inside_block.stderr b/src/tools/clippy/tests/ui/semicolon_inside_block.stderr index 2046dd1c36be..e16590374295 100644 --- a/src/tools/clippy/tests/ui/semicolon_inside_block.stderr +++ b/src/tools/clippy/tests/ui/semicolon_inside_block.stderr @@ -1,5 +1,5 @@ error: consider moving the `;` inside the block for consistent formatting - --> tests/ui/semicolon_inside_block.rs:39:5 + --> tests/ui/semicolon_inside_block.rs:40:5 | LL | { unit_fn_block() }; | ^^^^^^^^^^^^^^^^^^^^ @@ -13,7 +13,7 @@ LL + { unit_fn_block(); } | error: consider moving the `;` inside the block for consistent formatting - --> tests/ui/semicolon_inside_block.rs:41:5 + --> tests/ui/semicolon_inside_block.rs:42:5 | LL | unsafe { unit_fn_block() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -25,7 +25,7 @@ LL + unsafe { unit_fn_block(); } | error: consider moving the `;` inside the block for consistent formatting - --> tests/ui/semicolon_inside_block.rs:50:5 + --> tests/ui/semicolon_inside_block.rs:51:5 | LL | / { LL | | @@ -41,7 +41,7 @@ LL ~ } | error: consider moving the `;` inside the block for consistent formatting - --> tests/ui/semicolon_inside_block.rs:64:5 + --> tests/ui/semicolon_inside_block.rs:65:5 | LL | { m!(()) }; | ^^^^^^^^^^^ diff --git a/src/tools/clippy/tests/ui/unnecessary_option_map_or_else.fixed b/src/tools/clippy/tests/ui/unnecessary_option_map_or_else.fixed index 9974ee2d08eb..8d25efbeea00 100644 --- a/src/tools/clippy/tests/ui/unnecessary_option_map_or_else.fixed +++ b/src/tools/clippy/tests/ui/unnecessary_option_map_or_else.fixed @@ -3,13 +3,10 @@ clippy::let_and_return, clippy::let_unit_value, clippy::unnecessary_lazy_evaluations, - clippy::unnecessary_literal_unwrap + clippy::unnecessary_literal_unwrap, + clippy::needless_return )] -const fn identity(x: T) -> T { - x -} - const fn double_it(x: i32) -> i32 { x * 2 } @@ -18,31 +15,42 @@ fn main() { // Expected errors // Basic scenario let option = Some(()); - option.unwrap_or_else(|| ()); //~ ERROR: unused "map closure" when calling + option.unwrap_or_else(|| ()); //~ unnecessary_option_map_or_else // Type ascription let option = Some(()); - option.unwrap_or_else(|| ()); //~ ERROR: unused "map closure" when calling + option.unwrap_or_else(|| ()); //~ unnecessary_option_map_or_else // Auto-deref let string = String::new(); let option = Some(&string); - let _: &str = option.unwrap_or_else(|| &string); //~ ERROR: unused "map closure" when calling + let _: &str = option.map_or_else(|| &string, |x| x); + // This should in theory lint with a smarter check // Temporary variable let option = Some(()); option.unwrap_or_else(|| ()); - // Identity + // Temporary variable with pattern + let option = Some(((), ())); + option.unwrap_or_else(|| ((), ())); + + // std::convert::identity let string = String::new(); let option = Some(&string); - let _: &str = option.unwrap_or_else(|| &string); //~ ERROR: unused "map closure" when calling + let _: &str = option.unwrap_or_else(|| &string); //~ unnecessary_option_map_or_else - // Closure bound to a variable - let do_nothing = |x: String| x; - let string = String::new(); - let option = Some(string.clone()); - let _: String = option.unwrap_or_else(|| string); //~ ERROR: unused "map closure" when calling + let x: Option<((), ())> = Some(((), ())); + x.unwrap_or_else(|| ((), ())); + //~^ unnecessary_option_map_or_else + + // Returned temporary variable. + let x: Option<()> = Some(()); + x.unwrap_or_else(|| ()); + + // Returned temporary variable with pattern + let x: Option<((), ())> = Some(((), ())); + x.unwrap_or_else(|| ((), ())); // Correct usages let option = Some(()); diff --git a/src/tools/clippy/tests/ui/unnecessary_option_map_or_else.rs b/src/tools/clippy/tests/ui/unnecessary_option_map_or_else.rs index 9b53f3fcd521..42d4ad6ac1f8 100644 --- a/src/tools/clippy/tests/ui/unnecessary_option_map_or_else.rs +++ b/src/tools/clippy/tests/ui/unnecessary_option_map_or_else.rs @@ -3,13 +3,10 @@ clippy::let_and_return, clippy::let_unit_value, clippy::unnecessary_lazy_evaluations, - clippy::unnecessary_literal_unwrap + clippy::unnecessary_literal_unwrap, + clippy::needless_return )] -const fn identity(x: T) -> T { - x -} - const fn double_it(x: i32) -> i32 { x * 2 } @@ -18,21 +15,22 @@ fn main() { // Expected errors // Basic scenario let option = Some(()); - option.map_or_else(|| (), |x| x); //~ ERROR: unused "map closure" when calling + option.map_or_else(|| (), |x| x); //~ unnecessary_option_map_or_else // Type ascription let option = Some(()); - option.map_or_else(|| (), |x: ()| x); //~ ERROR: unused "map closure" when calling + option.map_or_else(|| (), |x: ()| x); //~ unnecessary_option_map_or_else // Auto-deref let string = String::new(); let option = Some(&string); - let _: &str = option.map_or_else(|| &string, |x| x); //~ ERROR: unused "map closure" when calling + let _: &str = option.map_or_else(|| &string, |x| x); + // This should in theory lint with a smarter check // Temporary variable let option = Some(()); option.map_or_else( - //~^ ERROR: unused "map closure" when calling + //~^ unnecessary_option_map_or_else || (), |x| { let tmp = x; @@ -40,16 +38,50 @@ fn main() { }, ); - // Identity + // Temporary variable with pattern + let option = Some(((), ())); + option.map_or_else( + //~^ unnecessary_option_map_or_else + || ((), ()), + |x| { + let tmp = x; + let (a, b) = tmp; + (a, b) + }, + ); + + // std::convert::identity let string = String::new(); let option = Some(&string); - let _: &str = option.map_or_else(|| &string, identity); //~ ERROR: unused "map closure" when calling + let _: &str = option.map_or_else(|| &string, std::convert::identity); //~ unnecessary_option_map_or_else - // Closure bound to a variable - let do_nothing = |x: String| x; - let string = String::new(); - let option = Some(string.clone()); - let _: String = option.map_or_else(|| string, do_nothing); //~ ERROR: unused "map closure" when calling + let x: Option<((), ())> = Some(((), ())); + x.map_or_else(|| ((), ()), |(a, b)| (a, b)); + //~^ unnecessary_option_map_or_else + + // Returned temporary variable. + let x: Option<()> = Some(()); + x.map_or_else( + //~^ unnecessary_option_map_or_else + || (), + |n| { + let tmp = n; + let tmp2 = tmp; + return tmp2; + }, + ); + + // Returned temporary variable with pattern + let x: Option<((), ())> = Some(((), ())); + x.map_or_else( + //~^ unnecessary_option_map_or_else + || ((), ()), + |n| { + let tmp = n; + let (a, b) = tmp; + return (a, b); + }, + ); // Correct usages let option = Some(()); diff --git a/src/tools/clippy/tests/ui/unnecessary_option_map_or_else.stderr b/src/tools/clippy/tests/ui/unnecessary_option_map_or_else.stderr index d90875e4efc7..3b579ebdb289 100644 --- a/src/tools/clippy/tests/ui/unnecessary_option_map_or_else.stderr +++ b/src/tools/clippy/tests/ui/unnecessary_option_map_or_else.stderr @@ -1,26 +1,31 @@ error: unused "map closure" when calling `Option::map_or_else` value - --> tests/ui/unnecessary_option_map_or_else.rs:21:5 + --> tests/ui/unnecessary_option_map_or_else.rs:18:5 | LL | option.map_or_else(|| (), |x| x); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `option.unwrap_or_else(|| ())` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::unnecessary-option-map-or-else` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::unnecessary_option_map_or_else)]` +help: consider using `unwrap_or_else` + | +LL - option.map_or_else(|| (), |x| x); +LL + option.unwrap_or_else(|| ()); + | error: unused "map closure" when calling `Option::map_or_else` value - --> tests/ui/unnecessary_option_map_or_else.rs:25:5 + --> tests/ui/unnecessary_option_map_or_else.rs:22:5 | LL | option.map_or_else(|| (), |x: ()| x); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `option.unwrap_or_else(|| ())` - -error: unused "map closure" when calling `Option::map_or_else` value - --> tests/ui/unnecessary_option_map_or_else.rs:30:19 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `unwrap_or_else` + | +LL - option.map_or_else(|| (), |x: ()| x); +LL + option.unwrap_or_else(|| ()); | -LL | let _: &str = option.map_or_else(|| &string, |x| x); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `option.unwrap_or_else(|| &string)` error: unused "map closure" when calling `Option::map_or_else` value - --> tests/ui/unnecessary_option_map_or_else.rs:34:5 + --> tests/ui/unnecessary_option_map_or_else.rs:32:5 | LL | / option.map_or_else( LL | | @@ -29,19 +34,122 @@ LL | | |x| { ... | LL | | }, LL | | ); - | |_____^ help: consider using `unwrap_or_else`: `option.unwrap_or_else(|| ())` + | |_____^ + | +help: consider using `unwrap_or_else` + | +LL - option.map_or_else( +LL - +LL - || (), +LL - |x| { +LL - let tmp = x; +LL - tmp +LL - }, +LL - ); +LL + option.unwrap_or_else(|| ()); + | error: unused "map closure" when calling `Option::map_or_else` value - --> tests/ui/unnecessary_option_map_or_else.rs:46:19 + --> tests/ui/unnecessary_option_map_or_else.rs:43:5 + | +LL | / option.map_or_else( +LL | | +LL | | || ((), ()), +LL | | |x| { +... | +LL | | }, +LL | | ); + | |_____^ + | +help: consider using `unwrap_or_else` + | +LL - option.map_or_else( +LL - +LL - || ((), ()), +LL - |x| { +LL - let tmp = x; +LL - let (a, b) = tmp; +LL - (a, b) +LL - }, +LL - ); +LL + option.unwrap_or_else(|| ((), ())); | -LL | let _: &str = option.map_or_else(|| &string, identity); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `option.unwrap_or_else(|| &string)` error: unused "map closure" when calling `Option::map_or_else` value - --> tests/ui/unnecessary_option_map_or_else.rs:52:21 + --> tests/ui/unnecessary_option_map_or_else.rs:56:19 + | +LL | let _: &str = option.map_or_else(|| &string, std::convert::identity); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `unwrap_or_else` + | +LL - let _: &str = option.map_or_else(|| &string, std::convert::identity); +LL + let _: &str = option.unwrap_or_else(|| &string); | -LL | let _: String = option.map_or_else(|| string, do_nothing); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `option.unwrap_or_else(|| string)` -error: aborting due to 6 previous errors +error: unused "map closure" when calling `Option::map_or_else` value + --> tests/ui/unnecessary_option_map_or_else.rs:59:5 + | +LL | x.map_or_else(|| ((), ()), |(a, b)| (a, b)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `unwrap_or_else` + | +LL - x.map_or_else(|| ((), ()), |(a, b)| (a, b)); +LL + x.unwrap_or_else(|| ((), ())); + | + +error: unused "map closure" when calling `Option::map_or_else` value + --> tests/ui/unnecessary_option_map_or_else.rs:64:5 + | +LL | / x.map_or_else( +LL | | +LL | | || (), +LL | | |n| { +... | +LL | | }, +LL | | ); + | |_____^ + | +help: consider using `unwrap_or_else` + | +LL - x.map_or_else( +LL - +LL - || (), +LL - |n| { +LL - let tmp = n; +LL - let tmp2 = tmp; +LL - return tmp2; +LL - }, +LL - ); +LL + x.unwrap_or_else(|| ()); + | + +error: unused "map closure" when calling `Option::map_or_else` value + --> tests/ui/unnecessary_option_map_or_else.rs:76:5 + | +LL | / x.map_or_else( +LL | | +LL | | || ((), ()), +LL | | |n| { +... | +LL | | }, +LL | | ); + | |_____^ + | +help: consider using `unwrap_or_else` + | +LL - x.map_or_else( +LL - +LL - || ((), ()), +LL - |n| { +LL - let tmp = n; +LL - let (a, b) = tmp; +LL - return (a, b); +LL - }, +LL - ); +LL + x.unwrap_or_else(|| ((), ())); + | + +error: aborting due to 8 previous errors diff --git a/src/tools/clippy/tests/ui/unnecessary_result_map_or_else.fixed b/src/tools/clippy/tests/ui/unnecessary_result_map_or_else.fixed index 5d7e3fa355fd..55542519bdb7 100644 --- a/src/tools/clippy/tests/ui/unnecessary_result_map_or_else.fixed +++ b/src/tools/clippy/tests/ui/unnecessary_result_map_or_else.fixed @@ -1,5 +1,10 @@ #![warn(clippy::unnecessary_result_map_or_else)] -#![allow(clippy::unnecessary_literal_unwrap, clippy::let_and_return, clippy::let_unit_value)] +#![allow( + clippy::unnecessary_literal_unwrap, + clippy::let_and_return, + clippy::let_unit_value, + clippy::needless_return +)] fn main() { let x: Result<(), ()> = Ok(()); @@ -14,13 +19,17 @@ fn main() { // Auto-deref. let y = String::new(); let x: Result<&String, &String> = Ok(&y); - let y: &str = x.unwrap_or_else(|err| err); - //~^ unnecessary_result_map_or_else + let y: &str = x.map_or_else(|err| err, |n| n); + // This should lint with a smarter check // Temporary variable. let x: Result<(), ()> = Ok(()); x.unwrap_or_else(|err| err); + // Temporary variable with pattern + let x: Result<((), ()), ((), ())> = Ok(((), ())); + x.unwrap_or_else(|err| err); + // Should not warn. let x: Result = Ok(0); x.map_or_else(|err| err, |n| n + 1); @@ -61,4 +70,16 @@ fn main() { y }, ); + + let x: Result<((), ()), ((), ())> = Ok(((), ())); + x.unwrap_or_else(|err| err); + //~^ unnecessary_result_map_or_else + + // Returned temporary variable. + let x: Result<(), ()> = Ok(()); + x.unwrap_or_else(|err| err); + + // Returned temporary variable with pattern + let x: Result<((), ()), ((), ())> = Ok(((), ())); + x.unwrap_or_else(|err| err); } diff --git a/src/tools/clippy/tests/ui/unnecessary_result_map_or_else.rs b/src/tools/clippy/tests/ui/unnecessary_result_map_or_else.rs index d2bab0f9d9c1..21a4826911e5 100644 --- a/src/tools/clippy/tests/ui/unnecessary_result_map_or_else.rs +++ b/src/tools/clippy/tests/ui/unnecessary_result_map_or_else.rs @@ -1,5 +1,10 @@ #![warn(clippy::unnecessary_result_map_or_else)] -#![allow(clippy::unnecessary_literal_unwrap, clippy::let_and_return, clippy::let_unit_value)] +#![allow( + clippy::unnecessary_literal_unwrap, + clippy::let_and_return, + clippy::let_unit_value, + clippy::needless_return +)] fn main() { let x: Result<(), ()> = Ok(()); @@ -15,7 +20,7 @@ fn main() { let y = String::new(); let x: Result<&String, &String> = Ok(&y); let y: &str = x.map_or_else(|err| err, |n| n); - //~^ unnecessary_result_map_or_else + // This should lint with a smarter check // Temporary variable. let x: Result<(), ()> = Ok(()); @@ -29,6 +34,18 @@ fn main() { }, ); + // Temporary variable with pattern + let x: Result<((), ()), ((), ())> = Ok(((), ())); + x.map_or_else( + //~^ unnecessary_result_map_or_else + |err| err, + |n| { + let tmp = n; + let (a, b) = tmp; + (a, b) + }, + ); + // Should not warn. let x: Result = Ok(0); x.map_or_else(|err| err, |n| n + 1); @@ -69,4 +86,32 @@ fn main() { y }, ); + + let x: Result<((), ()), ((), ())> = Ok(((), ())); + x.map_or_else(|err| err, |(a, b)| (a, b)); + //~^ unnecessary_result_map_or_else + + // Returned temporary variable. + let x: Result<(), ()> = Ok(()); + x.map_or_else( + //~^ unnecessary_result_map_or_else + |err| err, + |n| { + let tmp = n; + let tmp2 = tmp; + return tmp2; + }, + ); + + // Returned temporary variable with pattern + let x: Result<((), ()), ((), ())> = Ok(((), ())); + x.map_or_else( + //~^ unnecessary_result_map_or_else + |err| err, + |n| { + let tmp = n; + let (a, b) = tmp; + return (a, b); + }, + ); } diff --git a/src/tools/clippy/tests/ui/unnecessary_result_map_or_else.stderr b/src/tools/clippy/tests/ui/unnecessary_result_map_or_else.stderr index e6afa50217cc..f76999127c64 100644 --- a/src/tools/clippy/tests/ui/unnecessary_result_map_or_else.stderr +++ b/src/tools/clippy/tests/ui/unnecessary_result_map_or_else.stderr @@ -1,26 +1,31 @@ -error: unused "map closure" when calling `Result::map_or_else` value - --> tests/ui/unnecessary_result_map_or_else.rs:6:5 - | -LL | x.map_or_else(|err| err, |n| n); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `x.unwrap_or_else(|err| err)` - | - = note: `-D clippy::unnecessary-result-map-or-else` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::unnecessary_result_map_or_else)]` - error: unused "map closure" when calling `Result::map_or_else` value --> tests/ui/unnecessary_result_map_or_else.rs:11:5 | -LL | x.map_or_else(|err: ()| err, |n: ()| n); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `x.unwrap_or_else(|err: ()| err)` - -error: unused "map closure" when calling `Result::map_or_else` value - --> tests/ui/unnecessary_result_map_or_else.rs:17:19 +LL | x.map_or_else(|err| err, |n| n); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::unnecessary-result-map-or-else` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unnecessary_result_map_or_else)]` +help: consider using `unwrap_or_else` + | +LL - x.map_or_else(|err| err, |n| n); +LL + x.unwrap_or_else(|err| err); | -LL | let y: &str = x.map_or_else(|err| err, |n| n); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `unwrap_or_else`: `x.unwrap_or_else(|err| err)` error: unused "map closure" when calling `Result::map_or_else` value - --> tests/ui/unnecessary_result_map_or_else.rs:22:5 + --> tests/ui/unnecessary_result_map_or_else.rs:16:5 + | +LL | x.map_or_else(|err: ()| err, |n: ()| n); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `unwrap_or_else` + | +LL - x.map_or_else(|err: ()| err, |n: ()| n); +LL + x.unwrap_or_else(|err: ()| err); + | + +error: unused "map closure" when calling `Result::map_or_else` value + --> tests/ui/unnecessary_result_map_or_else.rs:27:5 | LL | / x.map_or_else( LL | | @@ -29,7 +34,111 @@ LL | | |n| { ... | LL | | }, LL | | ); - | |_____^ help: consider using `unwrap_or_else`: `x.unwrap_or_else(|err| err)` + | |_____^ + | +help: consider using `unwrap_or_else` + | +LL - x.map_or_else( +LL - +LL - |err| err, +LL - |n| { +LL - let tmp = n; +LL - let tmp2 = tmp; +LL - tmp2 +LL - }, +LL - ); +LL + x.unwrap_or_else(|err| err); + | -error: aborting due to 4 previous errors +error: unused "map closure" when calling `Result::map_or_else` value + --> tests/ui/unnecessary_result_map_or_else.rs:39:5 + | +LL | / x.map_or_else( +LL | | +LL | | |err| err, +LL | | |n| { +... | +LL | | }, +LL | | ); + | |_____^ + | +help: consider using `unwrap_or_else` + | +LL - x.map_or_else( +LL - +LL - |err| err, +LL - |n| { +LL - let tmp = n; +LL - let (a, b) = tmp; +LL - (a, b) +LL - }, +LL - ); +LL + x.unwrap_or_else(|err| err); + | + +error: unused "map closure" when calling `Result::map_or_else` value + --> tests/ui/unnecessary_result_map_or_else.rs:91:5 + | +LL | x.map_or_else(|err| err, |(a, b)| (a, b)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `unwrap_or_else` + | +LL - x.map_or_else(|err| err, |(a, b)| (a, b)); +LL + x.unwrap_or_else(|err| err); + | + +error: unused "map closure" when calling `Result::map_or_else` value + --> tests/ui/unnecessary_result_map_or_else.rs:96:5 + | +LL | / x.map_or_else( +LL | | +LL | | |err| err, +LL | | |n| { +... | +LL | | }, +LL | | ); + | |_____^ + | +help: consider using `unwrap_or_else` + | +LL - x.map_or_else( +LL - +LL - |err| err, +LL - |n| { +LL - let tmp = n; +LL - let tmp2 = tmp; +LL - return tmp2; +LL - }, +LL - ); +LL + x.unwrap_or_else(|err| err); + | + +error: unused "map closure" when calling `Result::map_or_else` value + --> tests/ui/unnecessary_result_map_or_else.rs:108:5 + | +LL | / x.map_or_else( +LL | | +LL | | |err| err, +LL | | |n| { +... | +LL | | }, +LL | | ); + | |_____^ + | +help: consider using `unwrap_or_else` + | +LL - x.map_or_else( +LL - +LL - |err| err, +LL - |n| { +LL - let tmp = n; +LL - let (a, b) = tmp; +LL - return (a, b); +LL - }, +LL - ); +LL + x.unwrap_or_else(|err| err); + | + +error: aborting due to 7 previous errors diff --git a/src/tools/clippy/tests/ui/unnecessary_safety_comment.rs b/src/tools/clippy/tests/ui/unnecessary_safety_comment.rs index d82a7b969080..75e8315343d3 100644 --- a/src/tools/clippy/tests/ui/unnecessary_safety_comment.rs +++ b/src/tools/clippy/tests/ui/unnecessary_safety_comment.rs @@ -100,3 +100,13 @@ pub fn point_to_five() -> *const u8 { } fn main() {} + +mod issue16553 { + //! ``` + //! // SAFETY: All is well. + //! unsafe { + //! foo() + //! } + //! ``` + mod blah {} +} diff --git a/src/tools/clippy/util/gh-pages/index_template.html b/src/tools/clippy/util/gh-pages/index_template.html index 91a5c1263195..26ca901fcd22 100644 --- a/src/tools/clippy/util/gh-pages/index_template.html +++ b/src/tools/clippy/util/gh-pages/index_template.html @@ -174,13 +174,13 @@ Otherwise, have a great day =^.^=

{# #}
{# #} {{lint.id ~}} - {#+ #} - {# #} + {#+ #} + {# #} 📋 {# #} {# #}
{# #} - {{lint.group}} {#+ #} + {{lint.group}} {#+ #} {{lint.level}} {#+ #} @@ -194,24 +194,20 @@ Otherwise, have a great day =^.^= {# Applicability #}
{# #} Applicability: {#+ #} - {{ lint.applicability_str() }} {# #} + {{ lint.applicability_str() }} {# #} (?) {# #}
{# Clippy version #}
{# #} {% if lint.group == "deprecated" %}Deprecated{% else %} Added{% endif +%} in: {#+ #} - {{lint.version}} {# #} + {{lint.version}} {# #}
{# Open related issues #} -
{# #} - Related Issues {# #} -
+ Related Issues {# Jump to source #} {% if let Some(id_location) = lint.id_location %} -
{# #} - View Source {# #} -
+ View Source {% endif %} {# #} {# #} diff --git a/src/tools/clippy/util/gh-pages/style.css b/src/tools/clippy/util/gh-pages/style.css index ce478a3e18d0..4f1935ba972f 100644 --- a/src/tools/clippy/util/gh-pages/style.css +++ b/src/tools/clippy/util/gh-pages/style.css @@ -1,5 +1,6 @@ body { --icon-filter: initial; + --label-background: #777; } body.ayu { @@ -230,7 +231,10 @@ button .caret { .panel-title-name { flex: 1; min-width: 400px;} -.panel-title-name .anchor { display: none; } +.panel-title-name .anchor { + display: none; + background-color: var(--label-background); +} article:hover .panel-title-name .anchor { display: inline;} .search-control { @@ -397,10 +401,6 @@ article:hover .panel-title-name .anchor { display: inline;} text-decoration: none; } -.label-default { - background-color: #777; -} - .lint-level { min-width: 4em; } @@ -420,6 +420,7 @@ article:hover .panel-title-name .anchor { display: inline;} .lint-group { min-width: 8em; + background-color: var(--label-background); } .group-deprecated { opacity: 0.5; @@ -459,7 +460,7 @@ article:hover .panel-title-name .anchor { display: inline;} display: flex; flex-flow: column; } - .lint-additional-info > div + div { + .lint-additional-info > * + * { border-top: 1px solid var(--theme-popup-border); } } @@ -468,12 +469,12 @@ article:hover .panel-title-name .anchor { display: inline;} display: flex; flex-flow: row; } - .lint-additional-info > div + div { + .lint-additional-info > * + * { border-left: 1px solid var(--theme-popup-border); } } -.lint-additional-info > div { +.lint-additional-info > * { display: inline-flex; min-width: 200px; flex-grow: 1;