Implement manual_and and manual_or lints

Suggests replacing if-else expressions where one branch is a boolean literal with && / || operators. Co-authored-by: Francisco Salgueiro <[email protected]>
rust-lang · Jun 17, 2024 · 7b68c50 · 7b68c50
2 parents 32b35e7 + a7fc2ff
commit 7b68c50
Show file tree

Hide file tree

Showing 38 changed files with 1,604 additions and 271 deletions.
diff --git a/.github/workflows/lintcheck.yml b/.github/workflows/lintcheck.yml
@@ -0,0 +1,115 @@
+name: Lintcheck
+
+on: pull_request
+
+env:
+  RUST_BACKTRACE: 1
+  CARGO_INCREMENTAL: 0
+
+concurrency:
+  # For a given workflow, if we push to the same PR, cancel all previous builds on that PR.
+  group: "${{ github.workflow }}-${{ github.event.pull_request.number}}"
+  cancel-in-progress: true
+
+jobs:
+  # Generates `lintcheck-logs/base.json` and stores it in a cache
+  base:
+    runs-on: ubuntu-latest
+
+    outputs:
+      key: ${{ steps.key.outputs.key }}
+
+    steps:
+    - name: Checkout
+      uses: actions/checkout@v4
+      with:
+        fetch-depth: 2
+
+    # HEAD is the generated merge commit `refs/pull/N/merge` between the PR and `master`, `HEAD^`
+    # being the commit from `master` that is the base of the merge
+    - name: Checkout base
+      run: git checkout HEAD^
+
+    # Use the lintcheck from the PR to generate the JSON in case the PR modifies lintcheck in some
+    # way
+    - name: Checkout current lintcheck
+      run: |
+        rm -rf lintcheck
+        git checkout ${{ github.sha }} -- lintcheck
+
+    - name: Create cache key
+      id: key
+      run: echo "key=lintcheck-base-${{ hashfiles('lintcheck/**') }}-$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT"
+
+    - name: Cache results
+      id: cache
+      uses: actions/cache@v4
+      with:
+        path: lintcheck-logs/base.json
+        key: ${{ steps.key.outputs.key }}
+
+    - name: Run lintcheck
+      if: steps.cache.outputs.cache-hit != 'true'
+      run: cargo lintcheck --format json
+
+    - name: Rename JSON file
+      if: steps.cache.outputs.cache-hit != 'true'
+      run: mv lintcheck-logs/lintcheck_crates_logs.json lintcheck-logs/base.json
+
+  # Generates `lintcheck-logs/head.json` and stores it in a cache
+  head:
+    runs-on: ubuntu-latest
+
+    outputs:
+      key: ${{ steps.key.outputs.key }}
+
+    steps:
+    - name: Checkout
+      uses: actions/checkout@v4
+
+    - name: Create cache key
+      id: key
+      run: echo "key=lintcheck-head-${{ github.sha }}" >> "$GITHUB_OUTPUT"
+
+    - name: Cache results
+      id: cache
+      uses: actions/cache@v4
+      with:
+        path: lintcheck-logs/head.json
+        key: ${{ steps.key.outputs.key }}
+
+    - name: Run lintcheck
+      if: steps.cache.outputs.cache-hit != 'true'
+      run: cargo lintcheck --format json
+
+    - name: Rename JSON file
+      if: steps.cache.outputs.cache-hit != 'true'
+      run: mv lintcheck-logs/lintcheck_crates_logs.json lintcheck-logs/head.json
+
+  # Retrieves `lintcheck-logs/base.json` and `lintcheck-logs/head.json` from the cache and prints
+  # the diff to the GH actions step summary
+  diff:
+    runs-on: ubuntu-latest
+
+    needs: [base, head]
+
+    steps:
+    - name: Checkout
+      uses: actions/checkout@v4
+
+    - name: Restore base JSON
+      uses: actions/cache/restore@v4
+      with:
+        key: ${{ needs.base.outputs.key }}
+        path: lintcheck-logs/base.json
+        fail-on-cache-miss: true
+
+    - name: Restore head JSON
+      uses: actions/cache/restore@v4
+      with:
+        key: ${{ needs.head.outputs.key }}
+        path: lintcheck-logs/head.json
+        fail-on-cache-miss: true
+
+    - name: Diff results
+      run: cargo lintcheck diff lintcheck-logs/base.json lintcheck-logs/head.json >> $GITHUB_STEP_SUMMARY
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5362,6 +5362,7 @@ Released 2018-09-13
 [`extra_unused_type_parameters`]: https://rust-lang.github.io/rust-clippy/master/index.html#extra_unused_type_parameters
 [`fallible_impl_from`]: https://rust-lang.github.io/rust-clippy/master/index.html#fallible_impl_from
 [`field_reassign_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#field_reassign_with_default
+[`field_scoped_visibility_modifiers`]: https://rust-lang.github.io/rust-clippy/master/index.html#field_scoped_visibility_modifiers
 [`filetype_is_file`]: https://rust-lang.github.io/rust-clippy/master/index.html#filetype_is_file
 [`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map
 [`filter_map_bool_then`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_bool_then
@@ -5521,6 +5522,7 @@ Released 2018-09-13
 [`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
 [`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten
 [`manual_hash_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one
+[`manual_inspect`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_inspect
 [`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed
 [`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

diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md
@@ -693,6 +693,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
 * [`manual_is_ascii_check`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check)
 * [`manual_let_else`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else)
 * [`manual_non_exhaustive`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive)
+* [`manual_pattern_char_comparison`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_pattern_char_comparison)
 * [`manual_range_contains`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains)
 * [`manual_rem_euclid`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid)
 * [`manual_retain`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain)

diff --git a/book/src/usage.md b/book/src/usage.md
@@ -36,7 +36,7 @@ You can configure lint levels on the command line by adding
 cargo clippy -- -Aclippy::style -Wclippy::double_neg -Dclippy::perf
 ```
 
-For [CI] all warnings can be elevated to errors which will inturn fail
+For [CI] all warnings can be elevated to errors which will in turn fail
 the build and cause Clippy to exit with a code other than `0`.
 
 ```

diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs
@@ -265,7 +265,7 @@ define_Conf! {
     ///
     /// Suppress lints whenever the suggested change would cause breakage for other crates.
     (avoid_breaking_exported_api: bool = true),
-    /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE, ITER_KV_MAP, MANUAL_C_STR_LITERALS, ASSIGNING_CLONES, LEGACY_NUMERIC_CONSTANTS.
+    /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE, ITER_KV_MAP, MANUAL_C_STR_LITERALS, ASSIGNING_CLONES, LEGACY_NUMERIC_CONSTANTS, MANUAL_PATTERN_CHAR_COMPARISON.
     ///
     /// The minimum rust version that the project supports. Defaults to the `rust-version` field in `Cargo.toml`
     #[default_text = ""]

diff --git a/clippy_config/src/msrvs.rs b/clippy_config/src/msrvs.rs
@@ -18,7 +18,7 @@ macro_rules! msrv_aliases {
 // names may refer to stabilized feature flags or library items
 msrv_aliases! {
     1,77,0 { C_STR_LITERALS }
-    1,76,0 { PTR_FROM_REF }
+    1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
     1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }
     1,70,0 { OPTION_RESULT_IS_VARIANT_AND, BINARY_HEAP_RETAIN }
     1,68,0 { PATH_MAIN_SEPARATOR_STR }

diff --git a/clippy_lints/src/casts/ref_as_ptr.rs b/clippy_lints/src/casts/ref_as_ptr.rs
@@ -22,9 +22,9 @@ pub(super) fn check<'tcx>(
 
     if matches!(cast_from.kind(), ty::Ref(..))
         && let ty::RawPtr(_, to_mutbl) = cast_to.kind()
-        && let Some(use_cx) = expr_use_ctxt(cx, expr)
+        && let use_cx = expr_use_ctxt(cx, expr)
         // TODO: only block the lint if `cast_expr` is a temporary
-        && !matches!(use_cx.node, ExprUseNode::LetStmt(_) | ExprUseNode::ConstStatic(_))
+        && !matches!(use_cx.use_node(cx), ExprUseNode::LetStmt(_) | ExprUseNode::ConstStatic(_))
     {
         let core_or_std = if is_no_std_crate(cx) { "core" } else { "std" };
         let fn_name = match to_mutbl {

diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
@@ -178,6 +178,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::explicit_write::EXPLICIT_WRITE_INFO,
     crate::extra_unused_type_parameters::EXTRA_UNUSED_TYPE_PARAMETERS_INFO,
     crate::fallible_impl_from::FALLIBLE_IMPL_FROM_INFO,
+    crate::field_scoped_visibility_modifiers::FIELD_SCOPED_VISIBILITY_MODIFIERS_INFO,
     crate::float_literal::EXCESSIVE_PRECISION_INFO,
     crate::float_literal::LOSSY_FLOAT_LITERAL_INFO,
     crate::floating_point_arithmetic::IMPRECISE_FLOPS_INFO,
@@ -404,6 +405,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::methods::MANUAL_C_STR_LITERALS_INFO,
     crate::methods::MANUAL_FILTER_MAP_INFO,
     crate::methods::MANUAL_FIND_MAP_INFO,
+    crate::methods::MANUAL_INSPECT_INFO,
     crate::methods::MANUAL_IS_VARIANT_AND_INFO,
     crate::methods::MANUAL_NEXT_BACK_INFO,
     crate::methods::MANUAL_OK_OR_INFO,

diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs
@@ -260,18 +260,13 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
             (None, kind) => {
                 let expr_ty = typeck.expr_ty(expr);
                 let use_cx = expr_use_ctxt(cx, expr);
-                let adjusted_ty = match &use_cx {
-                    Some(use_cx) => match use_cx.adjustments {
-                        [.., a] => a.target,
-                        _ => expr_ty,
-                    },
-                    _ => typeck.expr_ty_adjusted(expr),
-                };
+                let adjusted_ty = use_cx.adjustments.last().map_or(expr_ty, |a| a.target);
 
-                match (use_cx, kind) {
-                    (Some(use_cx), RefOp::Deref) => {
+                match kind {
+                    RefOp::Deref if use_cx.same_ctxt => {
+                        let use_node = use_cx.use_node(cx);
                         let sub_ty = typeck.expr_ty(sub_expr);
-                        if let ExprUseNode::FieldAccess(name) = use_cx.node
+                        if let ExprUseNode::FieldAccess(name) = use_node
                             && !use_cx.moved_before_use
                             && !ty_contains_field(sub_ty, name.name)
                         {
@@ -288,9 +283,9 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
                         } else if sub_ty.is_ref()
                             // Linting method receivers would require verifying that name lookup
                             // would resolve the same way. This is complicated by trait methods.
-                            && !use_cx.node.is_recv()
-                            && let Some(ty) = use_cx.node.defined_ty(cx)
-                            && TyCoercionStability::for_defined_ty(cx, ty, use_cx.node.is_return()).is_deref_stable()
+                            && !use_node.is_recv()
+                            && let Some(ty) = use_node.defined_ty(cx)
+                            && TyCoercionStability::for_defined_ty(cx, ty, use_node.is_return()).is_deref_stable()
                         {
                             self.state = Some((
                                 State::ExplicitDeref { mutability: None },
@@ -301,7 +296,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
                             ));
                         }
                     },
-                    (_, RefOp::Method { mutbl, is_ufcs })
+                    RefOp::Method { mutbl, is_ufcs }
                         if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id)
                             // Allow explicit deref in method chains. e.g. `foo.deref().bar()`
                             && (is_ufcs || !in_postfix_position(cx, expr)) =>
@@ -319,7 +314,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
                             },
                         ));
                     },
-                    (Some(use_cx), RefOp::AddrOf(mutability)) => {
+                    RefOp::AddrOf(mutability) if use_cx.same_ctxt => {
                         // Find the number of times the borrow is auto-derefed.
                         let mut iter = use_cx.adjustments.iter();
                         let mut deref_count = 0usize;
@@ -338,10 +333,11 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
                             };
                         };
 
-                        let stability = use_cx.node.defined_ty(cx).map_or(TyCoercionStability::None, |ty| {
-                            TyCoercionStability::for_defined_ty(cx, ty, use_cx.node.is_return())
+                        let use_node = use_cx.use_node(cx);
+                        let stability = use_node.defined_ty(cx).map_or(TyCoercionStability::None, |ty| {
+                            TyCoercionStability::for_defined_ty(cx, ty, use_node.is_return())
                         });
-                        let can_auto_borrow = match use_cx.node {
+                        let can_auto_borrow = match use_node {
                             ExprUseNode::FieldAccess(_)
                                 if !use_cx.moved_before_use && matches!(sub_expr.kind, ExprKind::Field(..)) =>
                             {
@@ -353,7 +349,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
                                 // deref through `ManuallyDrop<_>` will not compile.
                                 !adjust_derefs_manually_drop(use_cx.adjustments, expr_ty)
                             },
-                            ExprUseNode::Callee | ExprUseNode::FieldAccess(_) => true,
+                            ExprUseNode::Callee | ExprUseNode::FieldAccess(_) if !use_cx.moved_before_use => true,
                             ExprUseNode::MethodArg(hir_id, _, 0) if !use_cx.moved_before_use => {
                                 // Check for calls to trait methods where the trait is implemented
                                 // on a reference.
@@ -363,9 +359,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
                                 //   priority.
                                 if let Some(fn_id) = typeck.type_dependent_def_id(hir_id)
                                     && let Some(trait_id) = cx.tcx.trait_of_item(fn_id)
-                                    && let arg_ty = cx
-                                        .tcx
-                                        .erase_regions(use_cx.adjustments.last().map_or(expr_ty, |a| a.target))
+                                    && let arg_ty = cx.tcx.erase_regions(adjusted_ty)
                                     && let ty::Ref(_, sub_ty, _) = *arg_ty.kind()
                                     && let args =
                                         typeck.node_args_opt(hir_id).map(|args| &args[1..]).unwrap_or_default()
@@ -443,7 +437,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
                                     count: deref_count - required_refs,
                                     msg,
                                     stability,
-                                    for_field_access: if let ExprUseNode::FieldAccess(name) = use_cx.node
+                                    for_field_access: if let ExprUseNode::FieldAccess(name) = use_node
                                         && !use_cx.moved_before_use
                                     {
                                         Some(name.name)
@@ -453,7 +447,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
                                 }),
                                 StateData {
                                     first_expr: expr,
-                                    adjusted_ty: use_cx.adjustments.last().map_or(expr_ty, |a| a.target),
+                                    adjusted_ty,
                                 },
                             ));
                         } else if stability.is_deref_stable()
@@ -465,12 +459,12 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
                                 State::Borrow { mutability },
                                 StateData {
                                     first_expr: expr,
-                                    adjusted_ty: use_cx.adjustments.last().map_or(expr_ty, |a| a.target),
+                                    adjusted_ty,
                                 },
                             ));
                         }
                     },
-                    (None, _) | (_, RefOp::Method { .. }) => (),
+                    _ => {},
                 }
             },
             (