Skip to content

Commit 7b68c50

Browse files
mira-eandafranciscoBSalgueiro
mira-eanda
and
franciscoBSalgueiro
committed
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]>
2 parents 32b35e7 + a7fc2ff commit 7b68c50

38 files changed

+1604
-271
lines changed

‎.github/workflows/lintcheck.yml

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
name: Lintcheck
2+
3+
on: pull_request
4+
5+
env:
6+
RUST_BACKTRACE: 1
7+
CARGO_INCREMENTAL: 0
8+
9+
concurrency:
10+
# For a given workflow, if we push to the same PR, cancel all previous builds on that PR.
11+
group: "${{ github.workflow }}-${{ github.event.pull_request.number}}"
12+
cancel-in-progress: true
13+
14+
jobs:
15+
# Generates `lintcheck-logs/base.json` and stores it in a cache
16+
base:
17+
runs-on: ubuntu-latest
18+
19+
outputs:
20+
key: ${{ steps.key.outputs.key }}
21+
22+
steps:
23+
- name: Checkout
24+
uses: actions/checkout@v4
25+
with:
26+
fetch-depth: 2
27+
28+
# HEAD is the generated merge commit `refs/pull/N/merge` between the PR and `master`, `HEAD^`
29+
# being the commit from `master` that is the base of the merge
30+
- name: Checkout base
31+
run: git checkout HEAD^
32+
33+
# Use the lintcheck from the PR to generate the JSON in case the PR modifies lintcheck in some
34+
# way
35+
- name: Checkout current lintcheck
36+
run: |
37+
rm -rf lintcheck
38+
git checkout ${{ github.sha }} -- lintcheck
39+
40+
- name: Create cache key
41+
id: key
42+
run: echo "key=lintcheck-base-${{ hashfiles('lintcheck/**') }}-$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT"
43+
44+
- name: Cache results
45+
id: cache
46+
uses: actions/cache@v4
47+
with:
48+
path: lintcheck-logs/base.json
49+
key: ${{ steps.key.outputs.key }}
50+
51+
- name: Run lintcheck
52+
if: steps.cache.outputs.cache-hit != 'true'
53+
run: cargo lintcheck --format json
54+
55+
- name: Rename JSON file
56+
if: steps.cache.outputs.cache-hit != 'true'
57+
run: mv lintcheck-logs/lintcheck_crates_logs.json lintcheck-logs/base.json
58+
59+
# Generates `lintcheck-logs/head.json` and stores it in a cache
60+
head:
61+
runs-on: ubuntu-latest
62+
63+
outputs:
64+
key: ${{ steps.key.outputs.key }}
65+
66+
steps:
67+
- name: Checkout
68+
uses: actions/checkout@v4
69+
70+
- name: Create cache key
71+
id: key
72+
run: echo "key=lintcheck-head-${{ github.sha }}" >> "$GITHUB_OUTPUT"
73+
74+
- name: Cache results
75+
id: cache
76+
uses: actions/cache@v4
77+
with:
78+
path: lintcheck-logs/head.json
79+
key: ${{ steps.key.outputs.key }}
80+
81+
- name: Run lintcheck
82+
if: steps.cache.outputs.cache-hit != 'true'
83+
run: cargo lintcheck --format json
84+
85+
- name: Rename JSON file
86+
if: steps.cache.outputs.cache-hit != 'true'
87+
run: mv lintcheck-logs/lintcheck_crates_logs.json lintcheck-logs/head.json
88+
89+
# Retrieves `lintcheck-logs/base.json` and `lintcheck-logs/head.json` from the cache and prints
90+
# the diff to the GH actions step summary
91+
diff:
92+
runs-on: ubuntu-latest
93+
94+
needs: [base, head]
95+
96+
steps:
97+
- name: Checkout
98+
uses: actions/checkout@v4
99+
100+
- name: Restore base JSON
101+
uses: actions/cache/restore@v4
102+
with:
103+
key: ${{ needs.base.outputs.key }}
104+
path: lintcheck-logs/base.json
105+
fail-on-cache-miss: true
106+
107+
- name: Restore head JSON
108+
uses: actions/cache/restore@v4
109+
with:
110+
key: ${{ needs.head.outputs.key }}
111+
path: lintcheck-logs/head.json
112+
fail-on-cache-miss: true
113+
114+
- name: Diff results
115+
run: cargo lintcheck diff lintcheck-logs/base.json lintcheck-logs/head.json >> $GITHUB_STEP_SUMMARY

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5362,6 +5362,7 @@ Released 2018-09-13
53625362
[`extra_unused_type_parameters`]: https://rust-lang.github.io/rust-clippy/master/index.html#extra_unused_type_parameters
53635363
[`fallible_impl_from`]: https://rust-lang.github.io/rust-clippy/master/index.html#fallible_impl_from
53645364
[`field_reassign_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#field_reassign_with_default
5365+
[`field_scoped_visibility_modifiers`]: https://rust-lang.github.io/rust-clippy/master/index.html#field_scoped_visibility_modifiers
53655366
[`filetype_is_file`]: https://rust-lang.github.io/rust-clippy/master/index.html#filetype_is_file
53665367
[`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map
53675368
[`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
55215522
[`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
55225523
[`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten
55235524
[`manual_hash_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one
5525+
[`manual_inspect`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_inspect
55245526
[`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed
55255527
[`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check
55265528
[`manual_is_finite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_finite

‎book/src/lint_configuration.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,6 +693,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
693693
* [`manual_is_ascii_check`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check)
694694
* [`manual_let_else`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else)
695695
* [`manual_non_exhaustive`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive)
696+
* [`manual_pattern_char_comparison`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_pattern_char_comparison)
696697
* [`manual_range_contains`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains)
697698
* [`manual_rem_euclid`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid)
698699
* [`manual_retain`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain)

‎book/src/usage.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ You can configure lint levels on the command line by adding
3636
cargo clippy -- -Aclippy::style -Wclippy::double_neg -Dclippy::perf
3737
```
3838

39-
For [CI] all warnings can be elevated to errors which will inturn fail
39+
For [CI] all warnings can be elevated to errors which will in turn fail
4040
the build and cause Clippy to exit with a code other than `0`.
4141

4242
```

‎clippy_config/src/conf.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ define_Conf! {
265265
///
266266
/// Suppress lints whenever the suggested change would cause breakage for other crates.
267267
(avoid_breaking_exported_api: bool = true),
268-
/// 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.
268+
/// 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.
269269
///
270270
/// The minimum rust version that the project supports. Defaults to the `rust-version` field in `Cargo.toml`
271271
#[default_text = ""]

‎clippy_config/src/msrvs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ macro_rules! msrv_aliases {
1818
// names may refer to stabilized feature flags or library items
1919
msrv_aliases! {
2020
1,77,0 { C_STR_LITERALS }
21-
1,76,0 { PTR_FROM_REF }
21+
1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
2222
1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }
2323
1,70,0 { OPTION_RESULT_IS_VARIANT_AND, BINARY_HEAP_RETAIN }
2424
1,68,0 { PATH_MAIN_SEPARATOR_STR }

‎clippy_lints/src/casts/ref_as_ptr.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ pub(super) fn check<'tcx>(
2222

2323
if matches!(cast_from.kind(), ty::Ref(..))
2424
&& let ty::RawPtr(_, to_mutbl) = cast_to.kind()
25-
&& let Some(use_cx) = expr_use_ctxt(cx, expr)
25+
&& let use_cx = expr_use_ctxt(cx, expr)
2626
// TODO: only block the lint if `cast_expr` is a temporary
27-
&& !matches!(use_cx.node, ExprUseNode::LetStmt(_) | ExprUseNode::ConstStatic(_))
27+
&& !matches!(use_cx.use_node(cx), ExprUseNode::LetStmt(_) | ExprUseNode::ConstStatic(_))
2828
{
2929
let core_or_std = if is_no_std_crate(cx) { "core" } else { "std" };
3030
let fn_name = match to_mutbl {

‎clippy_lints/src/declared_lints.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
178178
crate::explicit_write::EXPLICIT_WRITE_INFO,
179179
crate::extra_unused_type_parameters::EXTRA_UNUSED_TYPE_PARAMETERS_INFO,
180180
crate::fallible_impl_from::FALLIBLE_IMPL_FROM_INFO,
181+
crate::field_scoped_visibility_modifiers::FIELD_SCOPED_VISIBILITY_MODIFIERS_INFO,
181182
crate::float_literal::EXCESSIVE_PRECISION_INFO,
182183
crate::float_literal::LOSSY_FLOAT_LITERAL_INFO,
183184
crate::floating_point_arithmetic::IMPRECISE_FLOPS_INFO,
@@ -404,6 +405,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
404405
crate::methods::MANUAL_C_STR_LITERALS_INFO,
405406
crate::methods::MANUAL_FILTER_MAP_INFO,
406407
crate::methods::MANUAL_FIND_MAP_INFO,
408+
crate::methods::MANUAL_INSPECT_INFO,
407409
crate::methods::MANUAL_IS_VARIANT_AND_INFO,
408410
crate::methods::MANUAL_NEXT_BACK_INFO,
409411
crate::methods::MANUAL_OK_OR_INFO,

‎clippy_lints/src/dereference.rs

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -260,18 +260,13 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
260260
(None, kind) => {
261261
let expr_ty = typeck.expr_ty(expr);
262262
let use_cx = expr_use_ctxt(cx, expr);
263-
let adjusted_ty = match &use_cx {
264-
Some(use_cx) => match use_cx.adjustments {
265-
[.., a] => a.target,
266-
_ => expr_ty,
267-
},
268-
_ => typeck.expr_ty_adjusted(expr),
269-
};
263+
let adjusted_ty = use_cx.adjustments.last().map_or(expr_ty, |a| a.target);
270264

271-
match (use_cx, kind) {
272-
(Some(use_cx), RefOp::Deref) => {
265+
match kind {
266+
RefOp::Deref if use_cx.same_ctxt => {
267+
let use_node = use_cx.use_node(cx);
273268
let sub_ty = typeck.expr_ty(sub_expr);
274-
if let ExprUseNode::FieldAccess(name) = use_cx.node
269+
if let ExprUseNode::FieldAccess(name) = use_node
275270
&& !use_cx.moved_before_use
276271
&& !ty_contains_field(sub_ty, name.name)
277272
{
@@ -288,9 +283,9 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
288283
} else if sub_ty.is_ref()
289284
// Linting method receivers would require verifying that name lookup
290285
// would resolve the same way. This is complicated by trait methods.
291-
&& !use_cx.node.is_recv()
292-
&& let Some(ty) = use_cx.node.defined_ty(cx)
293-
&& TyCoercionStability::for_defined_ty(cx, ty, use_cx.node.is_return()).is_deref_stable()
286+
&& !use_node.is_recv()
287+
&& let Some(ty) = use_node.defined_ty(cx)
288+
&& TyCoercionStability::for_defined_ty(cx, ty, use_node.is_return()).is_deref_stable()
294289
{
295290
self.state = Some((
296291
State::ExplicitDeref { mutability: None },
@@ -301,7 +296,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
301296
));
302297
}
303298
},
304-
(_, RefOp::Method { mutbl, is_ufcs })
299+
RefOp::Method { mutbl, is_ufcs }
305300
if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id)
306301
// Allow explicit deref in method chains. e.g. `foo.deref().bar()`
307302
&& (is_ufcs || !in_postfix_position(cx, expr)) =>
@@ -319,7 +314,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
319314
},
320315
));
321316
},
322-
(Some(use_cx), RefOp::AddrOf(mutability)) => {
317+
RefOp::AddrOf(mutability) if use_cx.same_ctxt => {
323318
// Find the number of times the borrow is auto-derefed.
324319
let mut iter = use_cx.adjustments.iter();
325320
let mut deref_count = 0usize;
@@ -338,10 +333,11 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
338333
};
339334
};
340335

341-
let stability = use_cx.node.defined_ty(cx).map_or(TyCoercionStability::None, |ty| {
342-
TyCoercionStability::for_defined_ty(cx, ty, use_cx.node.is_return())
336+
let use_node = use_cx.use_node(cx);
337+
let stability = use_node.defined_ty(cx).map_or(TyCoercionStability::None, |ty| {
338+
TyCoercionStability::for_defined_ty(cx, ty, use_node.is_return())
343339
});
344-
let can_auto_borrow = match use_cx.node {
340+
let can_auto_borrow = match use_node {
345341
ExprUseNode::FieldAccess(_)
346342
if !use_cx.moved_before_use && matches!(sub_expr.kind, ExprKind::Field(..)) =>
347343
{
@@ -353,7 +349,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
353349
// deref through `ManuallyDrop<_>` will not compile.
354350
!adjust_derefs_manually_drop(use_cx.adjustments, expr_ty)
355351
},
356-
ExprUseNode::Callee | ExprUseNode::FieldAccess(_) => true,
352+
ExprUseNode::Callee | ExprUseNode::FieldAccess(_) if !use_cx.moved_before_use => true,
357353
ExprUseNode::MethodArg(hir_id, _, 0) if !use_cx.moved_before_use => {
358354
// Check for calls to trait methods where the trait is implemented
359355
// on a reference.
@@ -363,9 +359,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
363359
// priority.
364360
if let Some(fn_id) = typeck.type_dependent_def_id(hir_id)
365361
&& let Some(trait_id) = cx.tcx.trait_of_item(fn_id)
366-
&& let arg_ty = cx
367-
.tcx
368-
.erase_regions(use_cx.adjustments.last().map_or(expr_ty, |a| a.target))
362+
&& let arg_ty = cx.tcx.erase_regions(adjusted_ty)
369363
&& let ty::Ref(_, sub_ty, _) = *arg_ty.kind()
370364
&& let args =
371365
typeck.node_args_opt(hir_id).map(|args| &args[1..]).unwrap_or_default()
@@ -443,7 +437,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
443437
count: deref_count - required_refs,
444438
msg,
445439
stability,
446-
for_field_access: if let ExprUseNode::FieldAccess(name) = use_cx.node
440+
for_field_access: if let ExprUseNode::FieldAccess(name) = use_node
447441
&& !use_cx.moved_before_use
448442
{
449443
Some(name.name)
@@ -453,7 +447,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
453447
}),
454448
StateData {
455449
first_expr: expr,
456-
adjusted_ty: use_cx.adjustments.last().map_or(expr_ty, |a| a.target),
450+
adjusted_ty,
457451
},
458452
));
459453
} else if stability.is_deref_stable()
@@ -465,12 +459,12 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
465459
State::Borrow { mutability },
466460
StateData {
467461
first_expr: expr,
468-
adjusted_ty: use_cx.adjustments.last().map_or(expr_ty, |a| a.target),
462+
adjusted_ty,
469463
},
470464
));
471465
}
472466
},
473-
(None, _) | (_, RefOp::Method { .. }) => (),
467+
_ => {},
474468
}
475469
},
476470
(

0 commit comments

Comments
 (0)