diff --git a/.cursor/rules/data-flow-patterns.mdc b/.cursor/rules/data-flow-patterns.mdc new file mode 100644 index 000000000000..74e9bff517e7 --- /dev/null +++ b/.cursor/rules/data-flow-patterns.mdc @@ -0,0 +1,50 @@ +--- +description: Documents data flow patterns for static analysis and linting rules in Rust code. +globs: **/src/**/*.rs,**/clippy_lints/**/*.rs,**/clippy_config/**/*.rs +alwaysApply: false +--- + + +# data-flow-patterns + +The data flow in this linting system follows these core patterns: + +1. Lint Registration Flow +- Lints are registered through `RegistrationGroups` in lib.rs +- Groups are organized by category (Correctness, Style, Complexity, etc.) +- Each lint registration contains metadata like name, description, level +- Registration maps to specific analysis implementations + +2. Lint Analysis Flow +- Source code is parsed into AST/MIR representations +- Lint implementations analyze specific patterns in the code +- Results are collected into a centralized reporting system +- Early vs Late pass analysis controls when lints are applied + +3. Configuration Processing Flow +- Configuration is loaded from clippy.toml and environment +- Rules are validated and normalized through conf.rs +- Configuration flows to individual lint implementations +- Default configurations are merged with user overrides + +4. Data Model Validation Flow +- Type-based validation rules check compatibility +- Memory safety requirements are enforced through ownership analysis +- Reference validity is tracked across code paths +- Custom type constraints are applied based on lint requirements + +5. Suggestion Generation Flow +- Issues detected by lints generate suggestion spans +- Suggestions include code replacements and rationale +- Multiple suggestions may be generated per issue +- Suggestions preserve original code formatting + +Key Files: +- clippy_lints/src/lib.rs - Core registration system +- clippy_config/src/conf.rs - Configuration processing +- clippy_lints/src/declared_lints.rs - Lint definitions +- clippy_lints/src/non_copy_const.rs - Data validation + +The system implements an extensible pipeline for static code analysis with strict separation between registration, analysis, and reporting phases. + +$END$ \ No newline at end of file diff --git a/.cursor/rules/lint-analysis-algorithms.mdc b/.cursor/rules/lint-analysis-algorithms.mdc new file mode 100644 index 000000000000..6cfcc7393407 --- /dev/null +++ b/.cursor/rules/lint-analysis-algorithms.mdc @@ -0,0 +1,50 @@ +--- +description: Specification for analyzing and documenting lint algorithms, rules and validations in Rust static analysis tools +globs: src/clippy_lints/**/*.rs,src/**/lint*.rs,src/**/rules/**/*.rs,src/**/check*.rs +alwaysApply: false +--- + + +# lint-analysis-algorithms + +The core business logic encompasses specialized static analysis algorithms for detecting code patterns and enforcing domain rules in Rust. Key components include: + +1. Pattern Matching Analysis +- AST traversal for identifying specific code structures requiring linting +- Type analysis and validation during pattern matching +- Context tracking for multi-block pattern validation +- Template matching against known anti-patterns + +2. Reference Safety Rules +- Borrow checker augmentation for reference analysis +- Lifetime validation for borrowed values +- Mutable reference access pattern checking +- Reference scoping and drop timing validation + +3. Type System Integration +- Layout compatibility verification for transmutes +- Generic parameter substitution tracking +- Trait bound satisfaction checking +- Zero-sized type special case handling + +4. Domain Rule Enforcement +- Documentation style and completeness validation +- API visibility and access pattern checks +- Naming convention verification +- Resource safety guarantees + +5. Suggestion Generation +- Context-aware code transformation suggestions +- Macro expansion preservation in suggestions +- Comment and attribute retention during fixes +- Edition-specific code recommendations + +The algorithms focus on static verification of Rust-specific safety requirements and coding conventions, with extensive customization points for project-specific rules. + +Core file paths: +- clippy_lints/src/methods/*.rs +- clippy_lints/src/types/*.rs +- clippy_lints/src/matches/*.rs +- clippy_lints/src/attrs/*.rs + +$END$ \ No newline at end of file diff --git a/.cursor/rules/lint-data-model.mdc b/.cursor/rules/lint-data-model.mdc new file mode 100644 index 000000000000..599f5fcb3797 --- /dev/null +++ b/.cursor/rules/lint-data-model.mdc @@ -0,0 +1,43 @@ +--- +description: Core lint metadata models and data structures used for evaluating and tracking lint rules and their relationships with Rust's type system +globs: clippy_lints/src/**.rs,clippy_config/src/**.rs +alwaysApply: false +--- + + +# lint-data-model + +## Core Lint State Model +- Tracks lint metadata including name, description, level (deny/warn/allow) +- Maintains contextual relationships between lints and their diagnostics +- Special handling for edition-specific lint configurations +- Enforces lint deprecation policies through staged transitions + +## Lint Category Hierarchies +- Implements layered categorization for lints: + - Style (code formatting/convention violations) + - Correctness (potential runtime errors) + - Complexity (code structure issues) + - Performance (inefficient patterns) + - Restriction (banned patterns/code constructs) + - Pedantic (best practice violations) + - Nursery (new/unstable lints) + +## Configuration Management +- Handles configuration hierarchies and inheritance +- Implements context-aware configuration application based on file paths +- Manages per-crate and per-module lint configurations +- Supports nested configuration overrides with precedence rules + +## Diagnostic Relationship Model +- Maps lints to compiler diagnostic codes +- Tracks diagnostic severities and suggested fixes +- Manages related diagnostic notes and explanations +- Handles diagnostic suppression chains + +Key Files: +- clippy_config/src/conf.rs +- clippy_lints/src/lib.rs +- clippy_lints/src/declared_lints.rs + +$END$ \ No newline at end of file diff --git a/.cursor/rules/type-validation-system.mdc b/.cursor/rules/type-validation-system.mdc new file mode 100644 index 000000000000..634bfceab833 --- /dev/null +++ b/.cursor/rules/type-validation-system.mdc @@ -0,0 +1,53 @@ +--- +description: Specification for the type validation and safety system used in Clippy linting rules and checks. +globs: clippy_lints/**/*.rs,clippy_config/**/*.rs +alwaysApply: false +--- + + +# type-validation-system + +The type validation system comprises several key components implementing specialized safety and validation rules: + +1. Cast Safety Validation: +- Validates numeric type conversions for potential data loss +- Enforces pointer safety rules during casting operations +- Contains domain rules for casting between collection types +- Handles specialized validation for floating point casts + +2. Reference Type Safety: +- Validates proper handling of references vs raw pointers +- Implements borrowing pattern detection and safety rules +- Contains custom validation for self-referential structs +- Handles complex lifetime relationship validation + +3. Collection Type Validation: +- Enforces collection type safety policies +- Validates element type compatibility in collections +- Implements specialized slice operation safety +- Contains custom validation for generic type parameters + +4. Specialized Domain Rules: +- Custom validation for mutable reference patterns +- Zero-sized type handling and validation +- Orphan implementation checking +- Associated type relationship validation + +Key implementations: +- clippy_lints/src/casts/*.rs: Core casting validation +- clippy_lints/src/types/*.rs: Type relationship checking +- clippy_lints/src/transmute/*.rs: Memory transmutation safety + +The system enforces domain-specific type safety policies through static analysis, focusing on: +- Reference and pointer safety guarantees +- Collection type compatibility +- Memory layout validation +- Generic type parameter bounds + +Notable validations include: +- Transmute operation safety +- Zero-cost abstraction validation +- Reference lifetime analysis +- Collection element type compatibility + +$END$ \ No newline at end of file diff --git a/.cursorrules b/.cursorrules new file mode 100644 index 000000000000..9ae574a0f153 --- /dev/null +++ b/.cursorrules @@ -0,0 +1,64 @@ + +START SPECIFICATION: +--- +description: This overview should be applied when analyzing Clippy's linting system components, particularly focusing on custom business rules, lint implementations, and domain-specific code analysis patterns. +globs: clippy_lints/src/**,clippy_config/src/**,clippy_dev/src/** +alwaysApply: false +--- + + +# main-overview + +## Development Guidelines + +- Only modify code directly relevant to the specific request. Avoid changing unrelated functionality. +- Never replace code with placeholders like `# ... rest of the processing ...`. Always include complete code. +- Break problems into smaller steps. Think through each step separately before implementing. +- Always provide a complete PLAN with REASONING based on evidence from code and logs before making changes. +- Explain your OBSERVATIONS clearly, then provide REASONING to identify the exact issue. Add console logs when needed to gather more information. + + +Clippy implements a sophisticated static analysis system for Rust code through several key components: + +1. Configuration Management (clippy_config/src/conf.rs) +- Enforces validation rules for linting configuration +- Implements custom documentation processing with code block preservation +- Controls field visibility and access patterns +- Validates naming conventions with configurable exceptions + +2. Core Linting System (clippy_lints/src/lib.rs) +- Organizes lints into functional categories (Correctness, Style, Complexity, etc.) +- Implements two-phase lint checking: + - Early pass: Syntax-level validation + - Late pass: Semantic analysis +- Manages centralized lint explanations and configuration metadata + +3. Code Pattern Analysis (clippy_lints/src/matches/mod.rs) +- Detects redundant match patterns and suggests simplifications +- Validates mutex safety in pattern matching +- Enforces pattern matching exhaustiveness +- Handles reference types and binding modes + +4. Memory Safety Validation (clippy_lints/src/transmute/) +- Validates transmute operations between types +- Enforces pointer safety and alignment rules +- Detects invalid collection transmutes +- Prevents unsafe pointer casts + +5. Documentation Enforcement (clippy_lints/src/doc/mod.rs) +- Enforces structured documentation requirements: + - Safety sections for unsafe code + - Error documentation for Result types + - Panic documentation where relevant +- Validates documentation formatting and structure + +The system is organized around a plugin architecture where individual lints implement specific business rules for code analysis. Each lint can provide custom suggestions for code improvement while maintaining semantic correctness. + +Key policy enforcement happens through: +- Configuration validation (clippy_config) +- Lint registration and management (clippy_lints/src/lib.rs) +- Custom pattern analysis (clippy_lints/src/matches) +- Safety rule enforcement (clippy_lints/src/transmute) + +$END$ +END SPECIFICATION \ No newline at end of file diff --git a/.giga/specifications.json b/.giga/specifications.json new file mode 100644 index 000000000000..6dfe1eabab7f --- /dev/null +++ b/.giga/specifications.json @@ -0,0 +1,22 @@ +[ + { + "fileName": "main-overview.mdc", + "description": "Complete overview of Clippy's architecture, including the lint pass system, core components, and high-level workflow of lint processing" + }, + { + "fileName": "lint-data-model.mdc", + "description": "Documentation of core lint data structures, including lint attributes, levels, categories, and their relationships with the Rust compiler's internal representations" + }, + { + "fileName": "lint-analysis-algorithms.mdc", + "description": "Detailed documentation of key algorithms used in lint analysis, including pattern matching, AST traversal, and type checking implementations" + }, + { + "fileName": "data-flow-patterns.mdc", + "description": "Documentation of how data flows through the lint system, including lint registration, application, and suggestion generation processes" + }, + { + "fileName": "type-validation-system.mdc", + "description": "Detailed documentation of the type validation system, focusing on transmute checks, casting validations, and type safety enforcement algorithms" + } +] \ No newline at end of file diff --git a/clippy_lints/src/matches/manual_unwrap_or.rs b/clippy_lints/src/matches/manual_unwrap_or.rs index 3ac2c9fc2b36..21e0d43ea79a 100644 --- a/clippy_lints/src/matches/manual_unwrap_or.rs +++ b/clippy_lints/src/matches/manual_unwrap_or.rs @@ -1,4 +1,5 @@ use clippy_utils::consts::ConstEvalCtxt; +use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::source::{SpanRangeExt as _, indent_of, reindent_multiline}; use rustc_ast::{BindingMode, ByRef}; use rustc_errors::Applicability; @@ -82,6 +83,7 @@ fn handle( body_some: &Expr<'_>, body_none: &Expr<'_>, binding_id: HirId, + msrv: Msrv, ) { // Only deal with situations where both alternatives return the same non-adjusted type. if cx.typeck_results().expr_ty(body_some) != cx.typeck_results().expr_ty(body_none) { @@ -112,6 +114,8 @@ fn handle( && let GenericArgKind::Type(condition_ty) = condition_ty.unpack() && implements_trait(cx, condition_ty, default_trait_id, &[]) && is_default_equivalent(cx, peel_blocks(body_none)) + // Check if MSRV meets the requirement for unwrap_or_default (stabilized in 1.16) + && msrv.meets(cx, msrvs::UNWRAP_OR_DEFAULT) { // We now check if the condition is a None variant, in which case we need to specify the type if path_res(cx, condition) @@ -186,6 +190,7 @@ pub fn check_match<'tcx>( expr: &'tcx Expr<'tcx>, scrutinee: &'tcx Expr<'tcx>, arms: &'tcx [Arm<'tcx>], + msrv: Msrv, ) { if let [arm1, arm2] = arms // Make sure there are no guards to keep things simple @@ -194,7 +199,7 @@ pub fn check_match<'tcx>( // Get the some and none bodies and the binding id of the some arm && let Some(((body_some, binding_id), body_none)) = get_some_and_none_bodies(cx, arm1, arm2) { - handle(cx, expr, "match", scrutinee, body_some, body_none, binding_id); + handle(cx, expr, "match", scrutinee, body_some, body_none, binding_id, msrv); } } @@ -205,6 +210,7 @@ pub fn check_if_let<'tcx>( scrutinee: &'tcx Expr<'tcx>, then_expr: &'tcx Expr<'tcx>, else_expr: &'tcx Expr<'tcx>, + msrv: Msrv, ) { if let Some(binding_id) = get_some(cx, pat) { handle( @@ -215,6 +221,7 @@ pub fn check_if_let<'tcx>( peel_blocks(then_expr), peel_blocks(else_expr), binding_id, + msrv, ); } } diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index c128fc40b733..06a35d0e9746 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -1062,7 +1062,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches { if is_direct_expn_of(expr.span, sym::matches).is_some() && let [arm, _] = arms { - redundant_pattern_match::check_match(cx, expr, ex, arms); + redundant_pattern_match::check_match(cx, expr, ex, &arms); redundant_pattern_match::check_matches_true(cx, expr, arm, ex); } @@ -1070,29 +1070,29 @@ impl<'tcx> LateLintPass<'tcx> for Matches { return; } if matches!(source, MatchSource::Normal | MatchSource::ForLoopDesugar) { - significant_drop_in_scrutinee::check_match(cx, expr, ex, arms, source); + significant_drop_in_scrutinee::check_match(cx, expr, ex, &arms, source); } - collapsible_match::check_match(cx, arms, self.msrv); + collapsible_match::check_match(cx, &arms, self.msrv); if !from_expansion { // These don't depend on a relationship between multiple arms - match_wild_err_arm::check(cx, ex, arms); - wild_in_or_pats::check(cx, ex, arms); + match_wild_err_arm::check(cx, ex, &arms); + wild_in_or_pats::check(cx, ex, &arms); } if let MatchSource::TryDesugar(_) = source { try_err::check(cx, expr, ex); } - if !from_expansion && !contains_cfg_arm(cx, expr, ex, arms) { + if !from_expansion && !contains_cfg_arm(cx, expr, ex, &arms) { if source == MatchSource::Normal { if !(self.msrv.meets(cx, msrvs::MATCHES_MACRO) - && match_like_matches::check_match(cx, expr, ex, arms)) + && match_like_matches::check_match(cx, expr, ex, &arms)) { - match_same_arms::check(cx, arms); + match_same_arms::check(cx, &arms); } - redundant_pattern_match::check_match(cx, expr, ex, arms); + 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); // We remove comments from inside arms block. @@ -1112,26 +1112,26 @@ impl<'tcx> LateLintPass<'tcx> for Matches { } // If there are still comments, it means they are outside of the arms. Tell the lint // code about it. - single_match::check(cx, ex, arms, expr, !match_comments.is_empty()); - match_bool::check(cx, ex, arms, expr); - overlapping_arms::check(cx, ex, arms); - match_wild_enum::check(cx, ex, arms); - match_as_ref::check(cx, ex, arms, expr); - needless_match::check_match(cx, ex, arms, expr); - match_str_case_mismatch::check(cx, ex, arms); - redundant_guards::check(cx, arms, self.msrv); + single_match::check(cx, ex, &arms, expr, !match_comments.is_empty()); + match_bool::check(cx, ex, &arms, expr); + overlapping_arms::check(cx, ex, &arms); + match_wild_enum::check(cx, ex, &arms); + match_as_ref::check(cx, ex, &arms, expr); + needless_match::check_match(cx, ex, &arms, expr); + match_str_case_mismatch::check(cx, ex, &arms); + redundant_guards::check(cx, &arms, self.msrv); if !is_in_const_context(cx) { - manual_unwrap_or::check_match(cx, expr, ex, arms); - manual_map::check_match(cx, expr, ex, arms); - manual_filter::check_match(cx, ex, arms, expr); - manual_ok_err::check_match(cx, expr, ex, arms); + manual_unwrap_or::check_match(cx, expr, ex, &arms, self.msrv); + manual_map::check_match(cx, expr, ex, &arms); + manual_filter::check_match(cx, ex, &arms, expr); + manual_ok_err::check_match(cx, expr, ex, &arms); } if self.infallible_destructuring_match_linted { self.infallible_destructuring_match_linted = false; } else { - match_single_binding::check(cx, ex, arms, expr); + match_single_binding::check(cx, ex, &arms, expr); } } match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr); @@ -1159,6 +1159,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches { if_let.let_expr, if_let.if_then, else_expr, + self.msrv, ); manual_map::check_if_let(cx, expr, if_let.let_pat, if_let.let_expr, if_let.if_then, else_expr); manual_filter::check_if_let( diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index d2d59f0013c0..4585754e9621 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -4412,7 +4412,7 @@ declare_clippy_lint! { /// Checks for calls to `Read::bytes` on types which don't implement `BufRead`. /// /// ### Why is this bad? - /// The default implementation calls `read` for each byte, which can be very inefficient for data that’s not in memory, such as `File`. + /// The default implementation calls `read` for each byte, which can be very inefficient for data that's not in memory, such as `File`. /// /// ### Example /// ```no_run @@ -4724,6 +4724,7 @@ pub fn method_call<'tcx>(recv: &'tcx Expr<'tcx>) -> Option<(Symbol, &'tcx Expr<' impl<'tcx> LateLintPass<'tcx> for Methods { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + extract_msrv_attr!(); if expr.span.from_expansion() { return; } @@ -4741,7 +4742,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods { }, ExprKind::MethodCall(method_call, receiver, args, _) => { let method_span = method_call.ident.span; - or_fun_call::check(cx, expr, method_span, method_call.ident.name, receiver, args); + or_fun_call::check(cx, expr, method_span, method_call.ident.name, receiver, args, self.msrv); expect_fun_call::check( cx, &self.format_args, diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index c74c42e9e5bd..a995c21ae3e4 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -2,6 +2,7 @@ use std::ops::ControlFlow; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::eager_or_lazy::switch_to_lazy_eval; +use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::source::snippet_with_context; use clippy_utils::ty::{expr_type_is_certain, implements_trait, is_type_diagnostic_item}; use clippy_utils::visitors::for_each_expr; @@ -26,6 +27,7 @@ pub(super) fn check<'tcx>( name: Symbol, receiver: &'tcx hir::Expr<'_>, args: &'tcx [hir::Expr<'_>], + msrv: Msrv, ) { /// Checks for `unwrap_or(T::new())`, `unwrap_or(T::default())`, /// `or_insert(T::new())` or `or_insert(T::default())`. @@ -39,6 +41,7 @@ pub(super) fn check<'tcx>( call_expr: Option<&hir::Expr<'_>>, span: Span, method_span: Span, + msrv: Msrv, ) -> bool { if !expr_type_is_certain(cx, receiver) { return false; @@ -98,6 +101,11 @@ pub(super) fn check<'tcx>( return false; } + // Check if MSRV meets the requirement for unwrap_or_default (stabilized in 1.16) + if !msrv.meets(cx, msrvs::UNWRAP_OR_DEFAULT) { + return false; + } + // needs to target Default::default in particular or be *::new and have a Default impl // available if (is_new(fun) && output_type_implements_default(fun)) @@ -124,72 +132,97 @@ pub(super) fn check<'tcx>( /// Checks for `*or(foo())`. #[expect(clippy::too_many_arguments)] - fn check_or_fn_call<'tcx>( - cx: &LateContext<'tcx>, + fn check_or_fn_call( + cx: &LateContext<'_>, name: Symbol, method_span: Span, - self_expr: &hir::Expr<'_>, - arg: &'tcx hir::Expr<'_>, - // `Some` if fn has second argument - second_arg: Option<&hir::Expr<'_>>, - span: Span, - // None if lambda is required + receiver: &hir::Expr<'_>, + arg: &hir::Expr<'_>, fun_span: Option, + span: Span, + msrv: Msrv, ) -> bool { - // (path, fn_has_argument, methods, suffix) - const KNOW_TYPES: [(Symbol, bool, &[Symbol], &str); 4] = [ - (sym::BTreeEntry, false, &[sym::or_insert], "with"), - (sym::HashMapEntry, false, &[sym::or_insert], "with"), - ( - sym::Option, - false, - &[sym::map_or, sym::ok_or, sym::or, sym::unwrap_or], - "else", - ), - (sym::Result, true, &[sym::or, sym::unwrap_or], "else"), - ]; + if !expr_type_is_certain(cx, receiver) { + return false; + } - if KNOW_TYPES.iter().any(|k| k.2.contains(&name)) - && switch_to_lazy_eval(cx, arg) - && !contains_return(arg) - && let self_ty = cx.typeck_results().expr_ty(self_expr) - && let Some(&(_, fn_has_arguments, poss, suffix)) = - KNOW_TYPES.iter().find(|&&i| is_type_diagnostic_item(cx, self_ty, i.0)) - && poss.contains(&name) - { - let ctxt = span.ctxt(); - let mut app = Applicability::HasPlaceholders; - let sugg = { - let (snippet_span, use_lambda) = match (fn_has_arguments, fun_span) { - (false, Some(fun_span)) => (fun_span, false), - _ => (arg.span, true), - }; + let is_new = |fun: &hir::Expr<'_>| { + if let hir::ExprKind::Path(ref qpath) = fun.kind { + let path = last_path_segment(qpath).ident.name; + matches!(path, sym::new) + } else { + false + } + }; - let snip = snippet_with_context(cx, snippet_span, ctxt, "..", &mut app).0; - let snip = if use_lambda { - let l_arg = if fn_has_arguments { "_" } else { "" }; - format!("|{l_arg}| {snip}") - } else { - snip.into_owned() - }; + let output_type_implements_default = |fun| { + let fun_ty = cx.typeck_results().expr_ty(fun); + if let ty::FnDef(def_id, args) = fun_ty.kind() { + let output_ty = cx.tcx.fn_sig(def_id).instantiate(cx.tcx, args).skip_binder().output(); + cx.tcx + .get_diagnostic_item(sym::Default) + .is_some_and(|default_trait_id| implements_trait(cx, output_ty, default_trait_id, &[])) + } else { + false + } + }; - if let Some(f) = second_arg { - let f = snippet_with_context(cx, f.span, ctxt, "..", &mut app).0; - format!("{snip}, {f}") - } else { - snip - } - }; - let span_replace_word = method_span.with_hi(span.hi()); + let sugg = match (name, fun_span.is_some()) { + (sym::unwrap_or, true) | (sym::unwrap_or_else, false) => sym::unwrap_or_default, + (sym::or_insert, true) | (sym::or_insert_with, false) => sym::or_default, + _ => return false, + }; + + let receiver_ty = cx.typeck_results().expr_ty_adjusted(receiver).peel_refs(); + let Some(suggested_method_def_id) = receiver_ty.ty_adt_def().and_then(|adt_def| { + cx.tcx + .inherent_impls(adt_def.did()) + .iter() + .flat_map(|impl_id| cx.tcx.associated_items(impl_id).filter_by_name_unhygienic(sugg)) + .find_map(|assoc| { + if assoc.is_method() && cx.tcx.fn_sig(assoc.def_id).skip_binder().inputs().skip_binder().len() == 1 + { + Some(assoc.def_id) + } else { + None + } + }) + }) else { + return false; + }; + let in_sugg_method_implementation = { + matches!( + suggested_method_def_id.as_local(), + Some(local_def_id) if local_def_id == cx.tcx.hir_get_parent_item(receiver.hir_id).def_id + ) + }; + if in_sugg_method_implementation { + return false; + } + + // Check if MSRV meets the requirement for unwrap_or_default (stabilized in 1.16) + if !msrv.meets(cx, msrvs::UNWRAP_OR_DEFAULT) { + return false; + } + + // needs to target Default::default in particular or be *::new and have a Default impl + // available + if (is_new(arg) && output_type_implements_default(arg)) + || match fun_span { + Some(_) => is_default_equivalent(cx, arg), + None => is_default_equivalent_call(cx, arg, None) || closure_body_returns_empty_to_string(cx, arg), + } + { span_lint_and_sugg( cx, OR_FUN_CALL, - span_replace_word, - format!("function call inside of `{name}`"), + method_span.with_hi(span.hi()), + format!("use of `{name}` to construct default value"), "try", - format!("{name}_{suffix}({sugg})"), - app, + format!("{sugg}()"), + Applicability::MachineApplicable, ); + true } else { false @@ -215,14 +248,14 @@ pub(super) fn check<'tcx>( }; (!inner_fun_has_args && !is_nested_expr - && check_unwrap_or_default(cx, name, receiver, fun, Some(ex), expr.span, method_span)) - || check_or_fn_call(cx, name, method_span, receiver, arg, None, expr.span, fun_span) + && check_unwrap_or_default(cx, name, receiver, fun, Some(ex), expr.span, method_span, msrv)) + || check_or_fn_call(cx, name, method_span, receiver, arg, None, expr.span, msrv) }, hir::ExprKind::Path(..) | hir::ExprKind::Closure(..) if !is_nested_expr => { - check_unwrap_or_default(cx, name, receiver, ex, None, expr.span, method_span) + check_unwrap_or_default(cx, name, receiver, ex, None, expr.span, method_span, msrv) }, hir::ExprKind::Index(..) | hir::ExprKind::MethodCall(..) => { - check_or_fn_call(cx, name, method_span, receiver, arg, None, expr.span, None) + check_or_fn_call(cx, name, method_span, receiver, arg, None, expr.span, msrv) }, _ => false, }; @@ -234,25 +267,6 @@ pub(super) fn check<'tcx>( } }); } - - // `map_or` takes two arguments - if let [arg, lambda] = args { - let inner_arg = peel_blocks(arg); - for_each_expr(cx, inner_arg, |ex| { - let is_top_most_expr = ex.hir_id == inner_arg.hir_id; - if let hir::ExprKind::Call(fun, fun_args) = ex.kind { - let fun_span = if fun_args.is_empty() && is_top_most_expr { - Some(fun.span) - } else { - None - }; - if check_or_fn_call(cx, name, method_span, receiver, arg, Some(lambda), expr.span, fun_span) { - return ControlFlow::Break(()); - } - } - ControlFlow::Continue(()) - }); - } } fn closure_body_returns_empty_to_string(cx: &LateContext<'_>, e: &hir::Expr<'_>) -> bool { diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index a5e66ad463bb..7bdb0308418c 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -76,7 +76,7 @@ msrv_aliases! { 1,24,0 { IS_ASCII_DIGIT } 1,18,0 { HASH_MAP_RETAIN, HASH_SET_RETAIN } 1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST, EXPECT_ERR } - 1,16,0 { STR_REPEAT } + 1,16,0 { STR_REPEAT, UNWRAP_OR_DEFAULT } 1,15,0 { MAYBE_BOUND_IN_WHERE } 1,13,0 { QUESTION_MARK_OPERATOR } } diff --git a/tests/ui/manual_unwrap_or_msrv.rs b/tests/ui/manual_unwrap_or_msrv.rs new file mode 100644 index 000000000000..8bb797102a3d --- /dev/null +++ b/tests/ui/manual_unwrap_or_msrv.rs @@ -0,0 +1,38 @@ +#![feature(custom_inner_attributes)] +#![clippy::msrv = "1.15"] + +fn main() { + // Should not suggest unwrap_or_default() when MSRV is 1.15 + let x: Option> = Some(vec![1, 2, 3]); + #[allow(clippy::manual_unwrap_or)] + let _ = match x { + Some(v) => v, + None => vec![], + }; + + // Should not suggest unwrap_or_default() for Result when MSRV is 1.15 + let y: Result, &str> = Ok(vec![1, 2, 3]); + #[allow(clippy::manual_unwrap_or)] + let _ = match y { + Ok(v) => v, + Err(_) => vec![], + }; +} + +// Test with MSRV 1.16 +#[clippy::msrv = "1.16"] +fn msrv_1_16() { + // Should suggest unwrap_or_default() when MSRV is 1.16 + let x: Option> = Some(vec![1, 2, 3]); + let _ = match x { + Some(v) => v, + None => vec![], + }; + + // Should suggest unwrap_or_default() for Result when MSRV is 1.16 + let y: Result, &str> = Ok(vec![1, 2, 3]); + let _ = match y { + Ok(v) => v, + Err(_) => vec![], + }; +} diff --git a/tests/ui/manual_unwrap_or_msrv.stderr b/tests/ui/manual_unwrap_or_msrv.stderr new file mode 100644 index 000000000000..0580cb2818fb --- /dev/null +++ b/tests/ui/manual_unwrap_or_msrv.stderr @@ -0,0 +1,25 @@ +error: match can be simplified with `.unwrap_or_default()` + --> tests/ui/manual_unwrap_or_msrv.rs:27:13 + | +LL | let _ = match x { + | _____________^ +LL | | Some(v) => v, +LL | | None => vec![], +LL | | }; + | |_____^ help: replace it with: `x.unwrap_or_default()` + | + = note: `-D clippy::manual-unwrap-or-default` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::manual_unwrap_or_default)]` + +error: match can be simplified with `.unwrap_or_default()` + --> tests/ui/manual_unwrap_or_msrv.rs:34:13 + | +LL | let _ = match y { + | _____________^ +LL | | Ok(v) => v, +LL | | Err(_) => vec![], +LL | | }; + | |_____^ help: replace it with: `y.unwrap_or_default()` + +error: aborting due to 2 previous errors + diff --git a/tests/ui/manual_unwrap_or_msrv.toml b/tests/ui/manual_unwrap_or_msrv.toml new file mode 100644 index 000000000000..43a146753b08 --- /dev/null +++ b/tests/ui/manual_unwrap_or_msrv.toml @@ -0,0 +1 @@ +msrv = "1.15" diff --git a/tests/ui/msrv_unwrap_or_default.rs b/tests/ui/msrv_unwrap_or_default.rs new file mode 100644 index 000000000000..56da765fbec6 --- /dev/null +++ b/tests/ui/msrv_unwrap_or_default.rs @@ -0,0 +1,8 @@ +#![warn(clippy::unwrap_or_default)] + +#[clippy::msrv = "1.15"] +fn f(foo: Result, &'static str>) -> Vec { + foo.unwrap_or(vec![]) +} + +fn main() {} diff --git a/tests/ui/msrv_unwrap_or_default.stderr b/tests/ui/msrv_unwrap_or_default.stderr new file mode 100644 index 000000000000..534a516d786a --- /dev/null +++ b/tests/ui/msrv_unwrap_or_default.stderr @@ -0,0 +1,7 @@ +warning: unused variable: `foo` + --> $DIR/msrv_unwrap_or_default.rs:4:5 + | +4 | fn f(foo: Result, &'static str>) -> Vec { + | ^^^ + | + = note: `#[warn(unused_variables)]` on by default