diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index 99d80bec025..5ba960db66d 100644 --- a/.github/workflows/clippy.yml +++ b/.github/workflows/clippy.yml @@ -38,7 +38,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install toolchain run: rustup show active-toolchain diff --git a/.github/workflows/clippy_bors.yml b/.github/workflows/clippy_bors.yml index 73c25550742..012797e5ca7 100644 --- a/.github/workflows/clippy_bors.yml +++ b/.github/workflows/clippy_bors.yml @@ -26,7 +26,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: ref: ${{ github.ref }} @@ -72,7 +72,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install i686 dependencies if: matrix.host == 'i686-unknown-linux-gnu' @@ -151,7 +151,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install toolchain run: rustup show active-toolchain @@ -175,7 +175,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install toolchain run: rustup show active-toolchain @@ -231,7 +231,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install toolchain run: rustup show active-toolchain diff --git a/.github/workflows/clippy_dev.yml b/.github/workflows/clippy_dev.yml index 0f0e3f2db92..37f18a4c087 100644 --- a/.github/workflows/clippy_dev.yml +++ b/.github/workflows/clippy_dev.yml @@ -24,7 +24,7 @@ jobs: steps: # Setup - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 # Run - name: Build diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 999ee7acfe7..94f494b65c4 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -21,10 +21,10 @@ jobs: steps: # Setup - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: ref: ${{ env.TARGET_BRANCH }} path: 'out' diff --git a/.github/workflows/remark.yml b/.github/workflows/remark.yml index 30bd476332f..05e1b3b9202 100644 --- a/.github/workflows/remark.yml +++ b/.github/workflows/remark.yml @@ -16,7 +16,7 @@ jobs: steps: # Setup - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Setup Node.js uses: actions/setup-node@v3 diff --git a/CHANGELOG.md b/CHANGELOG.md index f82421a687b..4d32bbec914 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5105,6 +5105,7 @@ Released 2018-09-13 [`else_if_without_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#else_if_without_else [`empty_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_drop [`empty_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum +[`empty_enum_variants_with_brackets`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum_variants_with_brackets [`empty_line_after_doc_comments`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_doc_comments [`empty_line_after_outer_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_outer_attr [`empty_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_loop @@ -5294,6 +5295,7 @@ Released 2018-09-13 [`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check [`manual_is_finite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_finite [`manual_is_infinite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_infinite +[`manual_is_variant_and`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_variant_and [`manual_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else [`manual_main_separator_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_main_separator_str [`manual_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_map @@ -5440,6 +5442,7 @@ Released 2018-09-13 [`only_used_in_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#only_used_in_recursion [`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref [`option_and_then_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_and_then_some +[`option_as_ref_cloned`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_cloned [`option_as_ref_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref [`option_env_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_env_unwrap [`option_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_expect_used @@ -5483,6 +5486,7 @@ Released 2018-09-13 [`ptr_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq [`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast [`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names +[`pub_underscore_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_underscore_fields [`pub_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_use [`pub_with_shorthand`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_with_shorthand [`pub_without_shorthand`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_without_shorthand @@ -5580,6 +5584,7 @@ Released 2018-09-13 [`stable_sort_primitive`]: https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive [`std_instead_of_alloc`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_alloc [`std_instead_of_core`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_core +[`str_split_at_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_split_at_newline [`str_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_to_string [`string_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add [`string_add_assign`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add_assign @@ -5612,6 +5617,7 @@ Released 2018-09-13 [`temporary_cstring_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr [`test_attr_in_doctest`]: https://rust-lang.github.io/rust-clippy/master/index.html#test_attr_in_doctest [`tests_outside_test_module`]: https://rust-lang.github.io/rust-clippy/master/index.html#tests_outside_test_module +[`thread_local_initializer_can_be_made_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#thread_local_initializer_can_be_made_const [`to_digit_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_digit_is_some [`to_string_in_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_display [`to_string_in_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_format_args @@ -5810,4 +5816,5 @@ Released 2018-09-13 [`allowed-dotfiles`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allowed-dotfiles [`enforce-iter-loop-reborrow`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enforce-iter-loop-reborrow [`check-private-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-private-items +[`pub-underscore-fields-behavior`]: https://doc.rust-lang.org/clippy/lint_configuration.html#pub-underscore-fields-behavior diff --git a/COPYRIGHT b/COPYRIGHT index 82703b18fd7..219693d63d9 100644 --- a/COPYRIGHT +++ b/COPYRIGHT @@ -1,6 +1,6 @@ // REUSE-IgnoreStart -Copyright 2014-2022 The Rust Project Developers +Copyright 2014-2024 The Rust Project Developers Licensed under the Apache License, Version 2.0 or the MIT license diff --git a/LICENSE-APACHE b/LICENSE-APACHE index 0d62c37278e..506582c31d6 100644 --- a/LICENSE-APACHE +++ b/LICENSE-APACHE @@ -186,7 +186,7 @@ APPENDIX: How to apply the Apache License to your work. same "printed page" as the copyright notice for easier identification within third-party archives. -Copyright 2014-2022 The Rust Project Developers +Copyright 2014-2024 The Rust Project Developers Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/LICENSE-MIT b/LICENSE-MIT index b724b24aa83..6d8ee9afb61 100644 --- a/LICENSE-MIT +++ b/LICENSE-MIT @@ -1,6 +1,6 @@ MIT License -Copyright (c) 2014-2022 The Rust Project Developers +Copyright (c) 2014-2024 The Rust Project Developers Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated diff --git a/README.md b/README.md index 5d490645d89..fa18447090c 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are over 650 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category. @@ -278,7 +278,7 @@ If you want to contribute to Clippy, you can find more information in [CONTRIBUT -Copyright 2014-2023 The Rust Project Developers +Copyright 2014-2024 The Rust Project Developers Licensed under the Apache License, Version 2.0 or the MIT license diff --git a/book/src/README.md b/book/src/README.md index 486ea3df704..e7972b0db19 100644 --- a/book/src/README.md +++ b/book/src/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are over 650 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index b02457307d7..a048fbbd8ac 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -9,6 +9,7 @@ - [Clippy's Lints](lints.md) - [Continuous Integration](continuous_integration/README.md) - [GitHub Actions](continuous_integration/github_actions.md) + - [GitLab CI](continuous_integration/gitlab.md) - [Travis CI](continuous_integration/travis.md) - [Development](development/README.md) - [Basics](development/basics.md) diff --git a/book/src/continuous_integration/github_actions.md b/book/src/continuous_integration/github_actions.md index 339287a7dd9..b588c8f0f02 100644 --- a/book/src/continuous_integration/github_actions.md +++ b/book/src/continuous_integration/github_actions.md @@ -15,7 +15,7 @@ jobs: clippy_check: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Run Clippy run: cargo clippy --all-targets --all-features ``` diff --git a/book/src/continuous_integration/gitlab.md b/book/src/continuous_integration/gitlab.md new file mode 100644 index 00000000000..bb3ef246c2f --- /dev/null +++ b/book/src/continuous_integration/gitlab.md @@ -0,0 +1,16 @@ +# GitLab CI + +You can add Clippy to GitLab CI by using the latest stable [rust docker image](https://hub.docker.com/_/rust), +as it is shown in the `.gitlab-ci.yml` CI configuration file below, + +```yml +# Make sure CI fails on all warnings, including Clippy lints +variables: + RUSTFLAGS: "-Dwarnings" + +clippy_check: + image: rust:latest + script: + - rustup component add clippy + - cargo clippy --all-targets --all-features +``` diff --git a/book/src/development/macro_expansions.md b/book/src/development/macro_expansions.md index c5eb000272d..aecca9ef72e 100644 --- a/book/src/development/macro_expansions.md +++ b/book/src/development/macro_expansions.md @@ -102,7 +102,7 @@ let x: Option = Some(42); m!(x, x.unwrap()); ``` -If the `m!(x, x.unwrapp());` line is expanded, we would get two expanded +If the `m!(x, x.unwrap());` line is expanded, we would get two expanded expressions: - `x.is_some()` (from the `$a.is_some()` line in the `m` macro) diff --git a/book/src/development/type_checking.md b/book/src/development/type_checking.md index a8c9660da4c..dc29ab5d08d 100644 --- a/book/src/development/type_checking.md +++ b/book/src/development/type_checking.md @@ -133,7 +133,7 @@ in this chapter: - [Type checking](https://rustc-dev-guide.rust-lang.org/type-checking.html) - [Ty module](https://rustc-dev-guide.rust-lang.org/ty.html) -[Adt]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/sty/enum.TyKind.html#variant.Adt +[Adt]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/ty_kind/enum.TyKind.html#variant.Adt [AdtDef]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/adt/struct.AdtDef.html [expr_ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TypeckResults.html#method.expr_ty [node_type]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TypeckResults.html#method.node_type @@ -144,7 +144,7 @@ in this chapter: [LateLintPass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.LateLintPass.html [pat_ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/typeck_results/struct.TypeckResults.html#method.pat_ty [Ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.Ty.html -[TyKind]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/sty/enum.TyKind.html +[TyKind]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/ty_kind/enum.TyKind.html [TypeckResults]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TypeckResults.html [middle_ty]: https://doc.rust-lang.org/beta/nightly-rustc/rustc_middle/ty/struct.Ty.html [hir_ty]: https://doc.rust-lang.org/beta/nightly-rustc/rustc_hir/struct.Ty.html diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 7c9a8eb1bfb..3b62ae0524a 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -805,3 +805,13 @@ for _ in &mut *rmvec {} * [`missing_errors_doc`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_errors_doc) +## `pub-underscore-fields-behavior` + + +**Default Value:** `"PublicallyExported"` + +--- +**Affected lints:** +* [`pub_underscore_fields`](https://rust-lang.github.io/rust-clippy/master/index.html#pub_underscore_fields) + + diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index a4f368397ce..5477d9b83a7 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -1,5 +1,5 @@ use crate::msrvs::Msrv; -use crate::types::{DisallowedPath, MacroMatcher, MatchLintBehaviour, Rename}; +use crate::types::{DisallowedPath, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour, Rename}; use crate::ClippyConfiguration; use rustc_data_structures::fx::FxHashSet; use rustc_session::Session; @@ -547,6 +547,11 @@ pub fn get_configuration_metadata() -> Vec { /// /// Whether to also run the listed lints on private items. (check_private_items: bool = false), + /// Lint: PUB_UNDERSCORE_FIELDS + /// + /// Lint "public" fields in a struct that are prefixed with an underscore based on their + /// exported visibility, or whether they are marked as "pub". + (pub_underscore_fields_behavior: PubUnderscoreFieldsBehaviour = PubUnderscoreFieldsBehaviour::PublicallyExported), } /// Search for the configuration file. diff --git a/clippy_config/src/metadata.rs b/clippy_config/src/metadata.rs index 2451fbc91e8..3ba2796e18d 100644 --- a/clippy_config/src/metadata.rs +++ b/clippy_config/src/metadata.rs @@ -96,6 +96,9 @@ fn parse_config_field_doc(doc_comment: &str) -> Option<(Vec, String)> { doc_comment.make_ascii_lowercase(); let lints: Vec = doc_comment .split_off(DOC_START.len()) + .lines() + .next() + .unwrap() .split(", ") .map(str::to_string) .collect(); diff --git a/clippy_config/src/msrvs.rs b/clippy_config/src/msrvs.rs index dae9f09ec00..72d5b9aff28 100644 --- a/clippy_config/src/msrvs.rs +++ b/clippy_config/src/msrvs.rs @@ -17,7 +17,7 @@ macro_rules! msrv_aliases { // names may refer to stabilized feature flags or library items msrv_aliases! { 1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE } - 1,70,0 { OPTION_IS_SOME_AND, BINARY_HEAP_RETAIN } + 1,70,0 { OPTION_RESULT_IS_VARIANT_AND, BINARY_HEAP_RETAIN } 1,68,0 { PATH_MAIN_SEPARATOR_STR } 1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS } 1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE } diff --git a/clippy_config/src/types.rs b/clippy_config/src/types.rs index df48cc3f5e3..baee09629ac 100644 --- a/clippy_config/src/types.rs +++ b/clippy_config/src/types.rs @@ -126,3 +126,9 @@ fn serialize(&self, _serializer: S) -> Result Rename, MacroMatcher, } + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)] +pub enum PubUnderscoreFieldsBehaviour { + PublicallyExported, + AllPubFields, +} diff --git a/clippy_dev/src/new_lint.rs b/clippy_dev/src/new_lint.rs index 31a42734c13..5d9cde06cd8 100644 --- a/clippy_dev/src/new_lint.rs +++ b/clippy_dev/src/new_lint.rs @@ -67,7 +67,7 @@ pub fn create( if pass == "early" { println!( "\n\ - NOTE: Use a late pass unless you need something specific from\ + NOTE: Use a late pass unless you need something specific from\n\ an early pass, as they lack many features and utilities" ); } diff --git a/clippy_lints/src/casts/cast_sign_loss.rs b/clippy_lints/src/casts/cast_sign_loss.rs index bd12ee40628..1df5a25f674 100644 --- a/clippy_lints/src/casts/cast_sign_loss.rs +++ b/clippy_lints/src/casts/cast_sign_loss.rs @@ -1,12 +1,14 @@ use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::span_lint; -use clippy_utils::{method_chain_args, sext}; -use rustc_hir::{Expr, ExprKind}; +use clippy_utils::{clip, method_chain_args, sext}; +use rustc_hir::{BinOpKind, Expr, ExprKind}; use rustc_lint::LateContext; -use rustc_middle::ty::{self, Ty}; +use rustc_middle::ty::{self, Ty, UintTy}; use super::CAST_SIGN_LOSS; +const METHODS_RET_POSITIVE: &[&str] = &["abs", "checked_abs", "rem_euclid", "checked_rem_euclid"]; + pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { if should_lint(cx, cast_op, cast_from, cast_to) { span_lint( @@ -25,33 +27,28 @@ fn should_lint(cx: &LateContext<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast return false; } - // Don't lint for positive constants. - let const_val = constant(cx, cx.typeck_results(), cast_op); - if let Some(Constant::Int(n)) = const_val - && let ty::Int(ity) = *cast_from.kind() - && sext(cx.tcx, n, ity) >= 0 - { + // Don't lint if `cast_op` is known to be positive. + if let Sign::ZeroOrPositive = expr_sign(cx, cast_op, cast_from) { return false; } - // Don't lint for the result of methods that always return non-negative values. - if let ExprKind::MethodCall(path, ..) = cast_op.kind { - let mut method_name = path.ident.name.as_str(); - let allowed_methods = ["abs", "checked_abs", "rem_euclid", "checked_rem_euclid"]; - - if method_name == "unwrap" - && let Some(arglist) = method_chain_args(cast_op, &["unwrap"]) - && let ExprKind::MethodCall(inner_path, ..) = &arglist[0].0.kind - { - method_name = inner_path.ident.name.as_str(); - } - - if allowed_methods.iter().any(|&name| method_name == name) { - return false; - } + let (mut uncertain_count, mut negative_count) = (0, 0); + // Peel off possible binary expressions, e.g. x * x * y => [x, x, y] + let Some(exprs) = exprs_with_selected_binop_peeled(cast_op) else { + // Assume cast sign lose if we cannot determine the sign of `cast_op` + return true; + }; + for expr in exprs { + let ty = cx.typeck_results().expr_ty(expr); + match expr_sign(cx, expr, ty) { + Sign::Negative => negative_count += 1, + Sign::Uncertain => uncertain_count += 1, + Sign::ZeroOrPositive => (), + }; } - true + // Lint if there are odd number of uncertain or negative results + uncertain_count % 2 == 1 || negative_count % 2 == 1 }, (false, true) => !cast_to.is_signed(), @@ -59,3 +56,97 @@ fn should_lint(cx: &LateContext<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast (_, _) => false, } } + +fn get_const_int_eval(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Option { + if let Constant::Int(n) = constant(cx, cx.typeck_results(), expr)? + && let ty::Int(ity) = *ty.kind() + { + return Some(sext(cx.tcx, n, ity)); + } + None +} + +enum Sign { + ZeroOrPositive, + Negative, + Uncertain, +} + +fn expr_sign(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Sign { + // Try evaluate this expr first to see if it's positive + if let Some(val) = get_const_int_eval(cx, expr, ty) { + return if val >= 0 { Sign::ZeroOrPositive } else { Sign::Negative }; + } + // Calling on methods that always return non-negative values. + if let ExprKind::MethodCall(path, caller, args, ..) = expr.kind { + let mut method_name = path.ident.name.as_str(); + + if method_name == "unwrap" + && let Some(arglist) = method_chain_args(expr, &["unwrap"]) + && let ExprKind::MethodCall(inner_path, ..) = &arglist[0].0.kind + { + method_name = inner_path.ident.name.as_str(); + } + + if method_name == "pow" + && let [arg] = args + { + return pow_call_result_sign(cx, caller, arg); + } else if METHODS_RET_POSITIVE.iter().any(|&name| method_name == name) { + return Sign::ZeroOrPositive; + } + } + + Sign::Uncertain +} + +/// Return the sign of the `pow` call's result. +/// +/// If the caller is a positive number, the result is always positive, +/// If the `power_of` is a even number, the result is always positive as well, +/// Otherwise a [`Sign::Uncertain`] will be returned. +fn pow_call_result_sign(cx: &LateContext<'_>, caller: &Expr<'_>, power_of: &Expr<'_>) -> Sign { + let caller_ty = cx.typeck_results().expr_ty(caller); + if let Some(caller_val) = get_const_int_eval(cx, caller, caller_ty) + && caller_val >= 0 + { + return Sign::ZeroOrPositive; + } + + if let Some(Constant::Int(n)) = constant(cx, cx.typeck_results(), power_of) + && clip(cx.tcx, n, UintTy::U32) % 2 == 0 + { + return Sign::ZeroOrPositive; + } + + Sign::Uncertain +} + +/// Peels binary operators such as [`BinOpKind::Mul`], [`BinOpKind::Div`] or [`BinOpKind::Rem`], +/// which the result could always be positive under certain condition. +/// +/// Other operators such as `+`/`-` causing the result's sign hard to determine, which we will +/// return `None` +fn exprs_with_selected_binop_peeled<'a>(expr: &'a Expr<'_>) -> Option>> { + #[inline] + fn collect_operands<'a>(expr: &'a Expr<'a>, operands: &mut Vec<&'a Expr<'a>>) -> Option<()> { + match expr.kind { + ExprKind::Binary(op, lhs, rhs) => { + if matches!(op.node, BinOpKind::Mul | BinOpKind::Div | BinOpKind::Rem) { + collect_operands(lhs, operands); + operands.push(rhs); + } else { + // Things are complicated when there are other binary ops exist, + // abort checking by returning `None` for now. + return None; + } + }, + _ => operands.push(expr), + } + Some(()) + } + + let mut res = vec![]; + collect_operands(expr, &mut res)?; + Some(res) +} diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index eae9dfac064..20230106d53 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -150,7 +150,8 @@ crate::else_if_without_else::ELSE_IF_WITHOUT_ELSE_INFO, crate::empty_drop::EMPTY_DROP_INFO, crate::empty_enum::EMPTY_ENUM_INFO, - crate::empty_structs_with_brackets::EMPTY_STRUCTS_WITH_BRACKETS_INFO, + crate::empty_with_brackets::EMPTY_ENUM_VARIANTS_WITH_BRACKETS_INFO, + crate::empty_with_brackets::EMPTY_STRUCTS_WITH_BRACKETS_INFO, crate::endian_bytes::BIG_ENDIAN_BYTES_INFO, crate::endian_bytes::HOST_ENDIAN_BYTES_INFO, crate::endian_bytes::LITTLE_ENDIAN_BYTES_INFO, @@ -385,6 +386,7 @@ crate::methods::JOIN_ABSOLUTE_PATHS_INFO, crate::methods::MANUAL_FILTER_MAP_INFO, crate::methods::MANUAL_FIND_MAP_INFO, + crate::methods::MANUAL_IS_VARIANT_AND_INFO, crate::methods::MANUAL_NEXT_BACK_INFO, crate::methods::MANUAL_OK_OR_INFO, crate::methods::MANUAL_SATURATING_ARITHMETIC_INFO, @@ -408,6 +410,7 @@ crate::methods::NO_EFFECT_REPLACE_INFO, crate::methods::OBFUSCATED_IF_ELSE_INFO, crate::methods::OK_EXPECT_INFO, + crate::methods::OPTION_AS_REF_CLONED_INFO, crate::methods::OPTION_AS_REF_DEREF_INFO, crate::methods::OPTION_FILTER_MAP_INFO, crate::methods::OPTION_MAP_OR_ERR_OK_INFO, @@ -433,6 +436,7 @@ crate::methods::STABLE_SORT_PRIMITIVE_INFO, crate::methods::STRING_EXTEND_CHARS_INFO, crate::methods::STRING_LIT_CHARS_ANY_INFO, + crate::methods::STR_SPLIT_AT_NEWLINE_INFO, crate::methods::SUSPICIOUS_COMMAND_ARG_SPACE_INFO, crate::methods::SUSPICIOUS_MAP_INFO, crate::methods::SUSPICIOUS_SPLITN_INFO, @@ -576,6 +580,7 @@ crate::ptr::MUT_FROM_REF_INFO, crate::ptr::PTR_ARG_INFO, crate::ptr_offset_with_cast::PTR_OFFSET_WITH_CAST_INFO, + crate::pub_underscore_fields::PUB_UNDERSCORE_FIELDS_INFO, crate::pub_use::PUB_USE_INFO, crate::question_mark::QUESTION_MARK_INFO, crate::question_mark_used::QUESTION_MARK_USED_INFO, @@ -648,6 +653,7 @@ crate::tabs_in_doc_comments::TABS_IN_DOC_COMMENTS_INFO, crate::temporary_assignment::TEMPORARY_ASSIGNMENT_INFO, crate::tests_outside_test_module::TESTS_OUTSIDE_TEST_MODULE_INFO, + crate::thread_local_initializer_can_be_made_const::THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST_INFO, crate::to_digit_is_some::TO_DIGIT_IS_SOME_INFO, crate::trailing_empty_array::TRAILING_EMPTY_ARRAY_INFO, crate::trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS_INFO, diff --git a/clippy_lints/src/default_numeric_fallback.rs b/clippy_lints/src/default_numeric_fallback.rs index 64a924a776a..712bc075650 100644 --- a/clippy_lints/src/default_numeric_fallback.rs +++ b/clippy_lints/src/default_numeric_fallback.rs @@ -4,7 +4,7 @@ use rustc_ast::ast::{LitFloatType, LitIntType, LitKind}; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_expr, walk_stmt, Visitor}; -use rustc_hir::{Body, Expr, ExprKind, HirId, ItemKind, Lit, Node, Stmt, StmtKind}; +use rustc_hir::{Block, Body, Expr, ExprKind, FnRetTy, HirId, ItemKind, Lit, Node, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::{self, FloatTy, IntTy, PolyFnSig, Ty}; @@ -122,13 +122,42 @@ fn check_lit(&self, lit: &Lit, lit_ty: Ty<'tcx>, emit_hir_id: HirId) { impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> { fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { match &expr.kind { + ExprKind::Block( + Block { + stmts, expr: Some(_), .. + }, + _, + ) => { + if let Some(parent) = self.cx.tcx.hir().find_parent(expr.hir_id) + && let Some(fn_sig) = parent.fn_sig() + && let FnRetTy::Return(_ty) = fn_sig.decl.output + { + // We cannot check the exact type since it's a `hir::Ty`` which does not implement `is_numeric` + self.ty_bounds.push(ExplicitTyBound(true)); + for stmt in *stmts { + self.visit_stmt(stmt); + } + self.ty_bounds.pop(); + // Ignore return expr since we know its type was inferred from return ty + return; + } + }, + + // Ignore return expr since we know its type was inferred from return ty + ExprKind::Ret(_) => return, + ExprKind::Call(func, args) => { if let Some(fn_sig) = fn_sig_opt(self.cx, func.hir_id) { for (expr, bound) in iter::zip(*args, fn_sig.skip_binder().inputs()) { - // Push found arg type, then visit arg. - self.ty_bounds.push((*bound).into()); - self.visit_expr(expr); - self.ty_bounds.pop(); + // If is from macro, try to use last bound type (typically pushed when visiting stmt), + // otherwise push found arg type, then visit arg, + if expr.span.from_expansion() { + self.visit_expr(expr); + } else { + self.ty_bounds.push((*bound).into()); + self.visit_expr(expr); + self.ty_bounds.pop(); + } } return; } @@ -137,7 +166,7 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { ExprKind::MethodCall(_, receiver, args, _) => { if let Some(def_id) = self.cx.typeck_results().type_dependent_def_id(expr.hir_id) { let fn_sig = self.cx.tcx.fn_sig(def_id).instantiate_identity().skip_binder(); - for (expr, bound) in iter::zip(std::iter::once(*receiver).chain(args.iter()), fn_sig.inputs()) { + for (expr, bound) in iter::zip(iter::once(*receiver).chain(args.iter()), fn_sig.inputs()) { self.ty_bounds.push((*bound).into()); self.visit_expr(expr); self.ty_bounds.pop(); diff --git a/clippy_lints/src/empty_structs_with_brackets.rs b/clippy_lints/src/empty_with_brackets.rs similarity index 62% rename from clippy_lints/src/empty_structs_with_brackets.rs rename to clippy_lints/src/empty_with_brackets.rs index 3cf67b3ecbf..969df6d85b5 100644 --- a/clippy_lints/src/empty_structs_with_brackets.rs +++ b/clippy_lints/src/empty_with_brackets.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet_opt; -use rustc_ast::ast::{Item, ItemKind, VariantData}; +use rustc_ast::ast::{Item, ItemKind, Variant, VariantData}; use rustc_errors::Applicability; use rustc_lexer::TokenKind; use rustc_lint::{EarlyContext, EarlyLintPass}; @@ -27,9 +27,38 @@ restriction, "finds struct declarations with empty brackets" } -declare_lint_pass!(EmptyStructsWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS]); -impl EarlyLintPass for EmptyStructsWithBrackets { +declare_clippy_lint! { + /// ### What it does + /// Finds enum variants without fields that are declared with empty brackets. + /// + /// ### Why is this bad? + /// Empty brackets while defining enum variants are redundant and can be omitted. + /// + /// ### Example + /// ```no_run + /// enum MyEnum { + /// HasData(u8), + /// HasNoData(), // redundant parentheses + /// } + /// ``` + /// + /// Use instead: + /// ```no_run + /// enum MyEnum { + /// HasData(u8), + /// HasNoData, + /// } + /// ``` + #[clippy::version = "1.77.0"] + pub EMPTY_ENUM_VARIANTS_WITH_BRACKETS, + restriction, + "finds enum variants with empty brackets" +} + +declare_lint_pass!(EmptyWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS, EMPTY_ENUM_VARIANTS_WITH_BRACKETS]); + +impl EarlyLintPass for EmptyWithBrackets { fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { let span_after_ident = item.span.with_lo(item.ident.span.hi()); @@ -53,6 +82,27 @@ fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { ); } } + + fn check_variant(&mut self, cx: &EarlyContext<'_>, variant: &Variant) { + let span_after_ident = variant.span.with_lo(variant.ident.span.hi()); + + if has_brackets(&variant.data) && has_no_fields(cx, &variant.data, span_after_ident) { + span_lint_and_then( + cx, + EMPTY_ENUM_VARIANTS_WITH_BRACKETS, + span_after_ident, + "enum variant has empty brackets", + |diagnostic| { + diagnostic.span_suggestion_hidden( + span_after_ident, + "remove the brackets", + "", + Applicability::MaybeIncorrect, + ); + }, + ); + } + } } fn has_no_ident_token(braces_span_str: &str) -> bool { diff --git a/clippy_lints/src/if_then_some_else_none.rs b/clippy_lints/src/if_then_some_else_none.rs index cd6c46a71a8..8f48941c4a9 100644 --- a/clippy_lints/src/if_then_some_else_none.rs +++ b/clippy_lints/src/if_then_some_else_none.rs @@ -3,7 +3,7 @@ use clippy_utils::eager_or_lazy::switch_to_eager_eval; use clippy_utils::source::snippet_with_context; use clippy_utils::sugg::Sugg; -use clippy_utils::{contains_return, higher, is_else_clause, is_res_lang_ctor, path_res, peel_blocks}; +use clippy_utils::{contains_return, higher, in_constant, is_else_clause, is_res_lang_ctor, path_res, peel_blocks}; use rustc_errors::Applicability; use rustc_hir::LangItem::{OptionNone, OptionSome}; use rustc_hir::{Expr, ExprKind}; @@ -74,6 +74,11 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { return; } + // `bool::then()` and `bool::then_some()` are not const + if in_constant(cx, expr.hir_id) { + return; + } + let ctxt = expr.span.ctxt(); if let Some(higher::If { diff --git a/clippy_lints/src/instant_subtraction.rs b/clippy_lints/src/instant_subtraction.rs index 655f4b82aa4..17b6256f982 100644 --- a/clippy_lints/src/instant_subtraction.rs +++ b/clippy_lints/src/instant_subtraction.rs @@ -40,7 +40,7 @@ declare_clippy_lint! { /// ### What it does - /// Lints subtraction between an [`Instant`] and a [`Duration`]. + /// Lints subtraction between an `Instant` and a `Duration`. /// /// ### Why is this bad? /// Unchecked subtraction could cause underflow on certain platforms, leading to @@ -57,9 +57,6 @@ /// # use std::time::{Instant, Duration}; /// let time_passed = Instant::now().checked_sub(Duration::from_secs(5)); /// ``` - /// - /// [`Duration`]: std::time::Duration - /// [`Instant::now()`]: std::time::Instant::now; #[clippy::version = "1.67.0"] pub UNCHECKED_DURATION_SUBTRACTION, pedantic, diff --git a/clippy_lints/src/item_name_repetitions.rs b/clippy_lints/src/item_name_repetitions.rs index a9f1612ff05..276c1abb60c 100644 --- a/clippy_lints/src/item_name_repetitions.rs +++ b/clippy_lints/src/item_name_repetitions.rs @@ -1,6 +1,7 @@ //! lint on enum variants that are prefixed or suffixed by the same characters use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_hir}; +use clippy_utils::is_bool; use clippy_utils::macros::span_is_local; use clippy_utils::source::is_present_in_source; use clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start, to_camel_case, to_snake_case}; @@ -231,6 +232,10 @@ fn check_fields(cx: &LateContext<'_>, threshold: u64, item: &Item<'_>, fields: & (false, _) => ("pre", prefix), (true, false) => ("post", postfix), }; + if fields.iter().all(|field| is_bool(field.ty)) { + // If all fields are booleans, we don't want to emit this lint. + return; + } span_lint_and_help( cx, STRUCT_FIELD_NAMES, diff --git a/clippy_lints/src/iter_without_into_iter.rs b/clippy_lints/src/iter_without_into_iter.rs index c9dc48668f2..903d3a2ab89 100644 --- a/clippy_lints/src/iter_without_into_iter.rs +++ b/clippy_lints/src/iter_without_into_iter.rs @@ -5,7 +5,8 @@ use rustc_ast::Mutability; use rustc_errors::Applicability; use rustc_hir::{FnRetTy, ImplItemKind, ImplicitSelfKind, ItemKind, TyKind}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; use rustc_middle::ty::{self, Ty}; use rustc_session::declare_lint_pass; use rustc_span::{sym, Symbol}; @@ -152,7 +153,8 @@ fn adt_has_inherent_method(cx: &LateContext<'_>, ty: Ty<'_>, method_name: Symbol impl LateLintPass<'_> for IterWithoutIntoIter { fn check_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::Item<'_>) { - if let ItemKind::Impl(imp) = item.kind + if !in_external_macro(cx.sess(), item.span) + && let ItemKind::Impl(imp) = item.kind && let TyKind::Ref(_, self_ty_without_ref) = &imp.self_ty.kind && let Some(trait_ref) = imp.of_trait && trait_ref @@ -219,7 +221,8 @@ fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::ImplItem<' _ => return, }; - if let ImplItemKind::Fn(sig, _) = item.kind + if !in_external_macro(cx.sess(), item.span) + && let ImplItemKind::Fn(sig, _) = item.kind && let FnRetTy::Return(ret) = sig.decl.output && is_nameable_in_impl_trait(ret) && cx.tcx.generics_of(item_did).params.is_empty() diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 755a4ff525d..efdd3925949 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -115,7 +115,7 @@ mod else_if_without_else; mod empty_drop; mod empty_enum; -mod empty_structs_with_brackets; +mod empty_with_brackets; mod endian_bytes; mod entry; mod enum_clike; @@ -272,6 +272,7 @@ mod precedence; mod ptr; mod ptr_offset_with_cast; +mod pub_underscore_fields; mod pub_use; mod question_mark; mod question_mark_used; @@ -322,6 +323,7 @@ mod tabs_in_doc_comments; mod temporary_assignment; mod tests_outside_test_module; +mod thread_local_initializer_can_be_made_const; mod to_digit_is_some; mod trailing_empty_array; mod trait_bounds; @@ -571,6 +573,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { verbose_bit_mask_threshold, warn_on_all_wildcard_imports, check_private_items, + pub_underscore_fields_behavior, blacklisted_names: _, cyclomatic_complexity_threshold: _, @@ -947,7 +950,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { }) }); store.register_early_pass(|| Box::new(crate_in_macro_def::CrateInMacroDef)); - store.register_early_pass(|| Box::new(empty_structs_with_brackets::EmptyStructsWithBrackets)); + store.register_early_pass(|| Box::new(empty_with_brackets::EmptyWithBrackets)); store.register_late_pass(|_| Box::new(unnecessary_owned_empty_strings::UnnecessaryOwnedEmptyStrings)); store.register_early_pass(|| Box::new(pub_use::PubUse)); store.register_late_pass(|_| Box::new(format_push_string::FormatPushString)); @@ -1080,7 +1083,15 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(repeat_vec_with_capacity::RepeatVecWithCapacity)); store.register_late_pass(|_| Box::new(uninhabited_references::UninhabitedReferences)); store.register_late_pass(|_| Box::new(ineffective_open_options::IneffectiveOpenOptions)); - store.register_late_pass(|_| Box::new(unconditional_recursion::UnconditionalRecursion)); + store.register_late_pass(|_| Box::::default()); + store.register_late_pass(move |_| { + Box::new(pub_underscore_fields::PubUnderscoreFields { + behavior: pub_underscore_fields_behavior, + }) + }); + store.register_late_pass(move |_| { + Box::new(thread_local_initializer_can_be_made_const::ThreadLocalInitializerCanBeMadeConst::new(msrv())) + }); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/lines_filter_map_ok.rs b/clippy_lints/src/lines_filter_map_ok.rs index 8a0955147bb..29957e423b0 100644 --- a/clippy_lints/src/lines_filter_map_ok.rs +++ b/clippy_lints/src/lines_filter_map_ok.rs @@ -96,8 +96,7 @@ fn should_lint(cx: &LateContext<'_>, args: &[Expr<'_>], method_str: &str) -> boo ExprKind::Path(qpath) => cx .qpath_res(qpath, fm_arg.hir_id) .opt_def_id() - .map(|did| match_def_path(cx, did, &paths::CORE_RESULT_OK_METHOD)) - .unwrap_or_default(), + .is_some_and(|did| match_def_path(cx, did, &paths::CORE_RESULT_OK_METHOD)), // Detect `|x| x.ok()` ExprKind::Closure(Closure { body, .. }) => { if let Body { diff --git a/clippy_lints/src/methods/filter_map.rs b/clippy_lints/src/methods/filter_map.rs index 7cfd3d346b6..e489899c19e 100644 --- a/clippy_lints/src/methods/filter_map.rs +++ b/clippy_lints/src/methods/filter_map.rs @@ -26,13 +26,14 @@ fn is_method(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_name: Symbol) -> hir::ExprKind::Closure(&hir::Closure { body, .. }) => { let body = cx.tcx.hir().body(body); let closure_expr = peel_blocks(body.value); - let arg_id = body.params[0].pat.hir_id; match closure_expr.kind { hir::ExprKind::MethodCall(hir::PathSegment { ident, .. }, receiver, ..) => { if ident.name == method_name && let hir::ExprKind::Path(path) = &receiver.kind && let Res::Local(ref local) = cx.qpath_res(path, receiver.hir_id) + && !body.params.is_empty() { + let arg_id = body.params[0].pat.hir_id; return arg_id == *local; } false diff --git a/clippy_lints/src/methods/iter_filter.rs b/clippy_lints/src/methods/iter_filter.rs index ade8e3155fa..9f84321ced4 100644 --- a/clippy_lints/src/methods/iter_filter.rs +++ b/clippy_lints/src/methods/iter_filter.rs @@ -1,80 +1,181 @@ +use clippy_utils::ty::get_iterator_item_ty; +use hir::ExprKind; use rustc_lint::{LateContext, LintContext}; use super::{ITER_FILTER_IS_OK, ITER_FILTER_IS_SOME}; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::{indent_of, reindent_multiline}; -use clippy_utils::{is_trait_method, peel_blocks, span_contains_comment}; +use clippy_utils::{get_parent_expr, is_trait_method, peel_blocks, span_contains_comment}; use rustc_errors::Applicability; use rustc_hir as hir; -use rustc_hir::def::Res; use rustc_hir::QPath; -use rustc_span::symbol::{sym, Symbol}; +use rustc_span::symbol::{sym, Ident, Symbol}; use rustc_span::Span; use std::borrow::Cow; -fn is_method(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_name: Symbol) -> bool { - match &expr.kind { - hir::ExprKind::Path(QPath::TypeRelative(_, mname)) => mname.ident.name == method_name, - hir::ExprKind::Path(QPath::Resolved(_, segments)) => { - segments.segments.last().unwrap().ident.name == method_name +/// +/// Returns true if the expression is a method call to `method_name` +/// e.g. `a.method_name()` or `Option::method_name`. +/// +/// The type-checker verifies for us that the method accepts the right kind of items +/// (e.g. `Option::is_some` accepts `Option<_>`), so we don't need to check that. +/// +/// How to capture each case: +/// +/// `.filter(|a| { std::option::Option::is_some(a) })` +/// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ <- this is a closure, getting unwrapped and +/// recursively checked. +/// `std::option::Option::is_some(a)` +/// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ <- this is a call. It unwraps to a path with +/// `QPath::TypeRelative`. Since this is a type relative path, we need to check the method name, the +/// type, and that the parameter of the closure is passed in the call. This part is the dual of +/// `receiver.method_name()` below. +/// +/// `filter(std::option::Option::is_some);` +/// ^^^^^^^^^^^^^^^^^^^^^^^^^^^ <- this is a type relative path, like above, we check the +/// type and the method name. +/// +/// `filter(|a| a.is_some());` +/// ^^^^^^^^^^^^^^^ <- this is a method call inside a closure, +/// we check that the parameter of the closure is the receiver of the method call and don't allow +/// any other parameters. +fn is_method( + cx: &LateContext<'_>, + expr: &hir::Expr<'_>, + type_symbol: Symbol, + method_name: Symbol, + params: &[&hir::Pat<'_>], +) -> bool { + fn pat_is_recv(ident: Ident, param: &hir::Pat<'_>) -> bool { + match param.kind { + hir::PatKind::Binding(_, _, other, _) => ident == other, + hir::PatKind::Ref(pat, _) => pat_is_recv(ident, pat), + _ => false, + } + } + match expr.kind { + hir::ExprKind::MethodCall(hir::PathSegment { ident, .. }, recv, ..) => { + // compare the identifier of the receiver to the parameter + // we are in a filter => closure has a single parameter and a single, non-block + // expression, this means that the parameter shadows all outside variables with + // the same name => avoid FPs. If the parameter is not the receiver, then this hits + // outside variables => avoid FP + if ident.name == method_name + && let ExprKind::Path(QPath::Resolved(None, path)) = recv.kind + && let &[seg] = path.segments + && params.iter().any(|p| pat_is_recv(seg.ident, p)) + { + return true; + } + false + }, + // This is used to check for complete paths via `|a| std::option::Option::is_some(a)` + // this then unwraps to a path with `QPath::TypeRelative` + // we pass the params as they've been passed to the current call through the closure + hir::ExprKind::Call(expr, [param]) => { + // this will hit the `QPath::TypeRelative` case and check that the method name is correct + if is_method(cx, expr, type_symbol, method_name, params) + // we then check that this is indeed passing the parameter of the closure + && let ExprKind::Path(QPath::Resolved(None, path)) = param.kind + && let &[seg] = path.segments + && params.iter().any(|p| pat_is_recv(seg.ident, p)) + { + return true; + } + false + }, + hir::ExprKind::Path(QPath::TypeRelative(ty, mname)) => { + let ty = cx.typeck_results().node_type(ty.hir_id); + if let Some(did) = cx.tcx.get_diagnostic_item(type_symbol) + && ty.ty_adt_def() == cx.tcx.type_of(did).skip_binder().ty_adt_def() + { + return mname.ident.name == method_name; + } + false }, - hir::ExprKind::MethodCall(segment, _, _, _) => segment.ident.name == method_name, hir::ExprKind::Closure(&hir::Closure { body, .. }) => { let body = cx.tcx.hir().body(body); let closure_expr = peel_blocks(body.value); - let arg_id = body.params[0].pat.hir_id; - match closure_expr.kind { - hir::ExprKind::MethodCall(hir::PathSegment { ident, .. }, receiver, ..) => { - if ident.name == method_name - && let hir::ExprKind::Path(path) = &receiver.kind - && let Res::Local(ref local) = cx.qpath_res(path, receiver.hir_id) - { - return arg_id == *local; - } - false - }, - _ => false, - } + let params = body.params.iter().map(|param| param.pat).collect::>(); + is_method(cx, closure_expr, type_symbol, method_name, params.as_slice()) }, _ => false, } } fn parent_is_map(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { - if let hir::Node::Expr(parent_expr) = cx.tcx.hir().get_parent(expr.hir_id) { - is_method(cx, parent_expr, rustc_span::sym::map) - } else { - false + if let Some(expr) = get_parent_expr(cx, expr) + && is_trait_method(cx, expr, sym::Iterator) + && let hir::ExprKind::MethodCall(path, _, _, _) = expr.kind + && path.ident.name == rustc_span::sym::map + { + return true; } + false } -#[allow(clippy::too_many_arguments)] -pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_arg: &hir::Expr<'_>, filter_span: Span) { - let is_iterator = is_trait_method(cx, expr, sym::Iterator); - let parent_is_not_map = !parent_is_map(cx, expr); +enum FilterType { + IsSome, + IsOk, +} - if is_iterator - && parent_is_not_map - && is_method(cx, filter_arg, sym!(is_some)) - && !span_contains_comment(cx.sess().source_map(), filter_span.with_hi(expr.span.hi())) +/// Returns the `FilterType` of the expression if it is a filter over an Iter