Skip to content

Commit

Permalink
Auto merge of #93001 - flip1995:clippyup, r=Manishearth
Browse files Browse the repository at this point in the history
Out of cycle Clippy update

I want to do an out-of-cycle sync for #8295, and possibly backport this to stable together with rust-lang/rust#92938. If this doesn't get backported to stable, then I at least want to backport it to beta.

r? `@Manishearth`
  • Loading branch information
bors committed Jan 18, 2022
2 parents 8f48116 + ddad101 commit 7a2cabb
Show file tree
Hide file tree
Showing 43 changed files with 754 additions and 100 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3049,6 +3049,7 @@ Released 2018-09-13
[`iter_not_returning_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_not_returning_iterator
[`iter_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth
[`iter_nth_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth_zero
[`iter_overeager_cloned`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_overeager_cloned
[`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next
[`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero
[`just_underscores_and_digits`]: https://rust-lang.github.io/rust-clippy/master/index.html#just_underscores_and_digits
Expand Down
2 changes: 1 addition & 1 deletion COPYRIGHT
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Copyright 2014-2021 The Rust Project Developers
Copyright 2014-2022 The Rust Project Developers

Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
Expand Down
2 changes: 1 addition & 1 deletion LICENSE-APACHE
Original file line number Diff line number Diff line change
Expand Up @@ -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-2021 The Rust Project Developers
Copyright 2014-2022 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.
Expand Down
2 changes: 1 addition & 1 deletion LICENSE-MIT
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
MIT License

Copyright (c) 2014-2021 The Rust Project Developers
Copyright (c) 2014-2022 The Rust Project Developers

Permission is hereby granted, free of charge, to any
person obtaining a copy of this software and associated
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ If you want to contribute to Clippy, you can find more information in [CONTRIBUT

## License

Copyright 2014-2021 The Rust Project Developers
Copyright 2014-2022 The Rust Project Developers

Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
[https://www.apache.org/licenses/LICENSE-2.0](https://www.apache.org/licenses/LICENSE-2.0)> or the MIT license
Expand Down
24 changes: 12 additions & 12 deletions clippy_lints/src/bit_mask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ declare_clippy_lint! {
/// {`!=`, `>=`, `>`, `!=`, `>=`, `>`}) can be determined from the following
/// table:
///
/// |Comparison |Bit Op|Example |is always|Formula |
/// |------------|------|------------|---------|----------------------|
/// |`==` or `!=`| `&` |`x & 2 == 3`|`false` |`c & m != c` |
/// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` |
/// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` |
/// |`==` or `!=`| `|` |`x | 1 == 0`|`false` |`c | m != c` |
/// |`<` or `>=`| `|` |`x | 1 < 1` |`false` |`m >= c` |
/// |`<=` or `>` | `|` |`x | 1 > 0` |`true` |`m > c` |
/// |Comparison |Bit Op|Example |is always|Formula |
/// |------------|------|-------------|---------|----------------------|
/// |`==` or `!=`| `&` |`x & 2 == 3` |`false` |`c & m != c` |
/// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` |
/// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` |
/// |`==` or `!=`| `\|` |`x \| 1 == 0`|`false` |`c \| m != c` |
/// |`<` or `>=`| `\|` |`x \| 1 < 1` |`false` |`m >= c` |
/// |`<=` or `>` | `\|` |`x \| 1 > 0` |`true` |`m > c` |
///
/// ### Why is this bad?
/// If the bits that the comparison cares about are always
Expand Down Expand Up @@ -53,10 +53,10 @@ declare_clippy_lint! {
/// without changing the outcome. The basic structure can be seen in the
/// following table:
///
/// |Comparison| Bit Op |Example |equals |
/// |----------|---------|-----------|-------|
/// |`>` / `<=`|`|` / `^`|`x | 2 > 3`|`x > 3`|
/// |`<` / `>=`|`|` / `^`|`x ^ 1 < 4`|`x < 4`|
/// |Comparison| Bit Op |Example |equals |
/// |----------|----------|------------|-------|
/// |`>` / `<=`|`\|` / `^`|`x \| 2 > 3`|`x > 3`|
/// |`<` / `>=`|`\|` / `^`|`x ^ 1 < 4` |`x < 4`|
///
/// ### Why is this bad?
/// Not equally evil as [`bad_bit_mask`](#bad_bit_mask),
Expand Down
29 changes: 17 additions & 12 deletions clippy_lints/src/copies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
lint_same_cond(cx, &conds);
lint_same_fns_in_if_cond(cx, &conds);
// Block duplication
lint_same_then_else(cx, &blocks, conds.len() == blocks.len(), expr);
lint_same_then_else(cx, &conds, &blocks, conds.len() == blocks.len(), expr);
}
}
}
Expand All @@ -192,6 +192,7 @@ impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
/// Implementation of `BRANCHES_SHARING_CODE` and `IF_SAME_THEN_ELSE` if the blocks are equal.
fn lint_same_then_else<'tcx>(
cx: &LateContext<'tcx>,
conds: &[&'tcx Expr<'_>],
blocks: &[&Block<'tcx>],
has_conditional_else: bool,
expr: &'tcx Expr<'_>,
Expand All @@ -204,7 +205,7 @@ fn lint_same_then_else<'tcx>(
// Check if each block has shared code
let has_expr = blocks[0].expr.is_some();

let (start_eq, mut end_eq, expr_eq) = if let Some(block_eq) = scan_block_for_eq(cx, blocks) {
let (start_eq, mut end_eq, expr_eq) = if let Some(block_eq) = scan_block_for_eq(cx, conds, blocks) {
(block_eq.start_eq, block_eq.end_eq, block_eq.expr_eq)
} else {
return;
Expand Down Expand Up @@ -316,14 +317,14 @@ struct BlockEqual {

/// This function can also trigger the `IF_SAME_THEN_ELSE` in which case it'll return `None` to
/// abort any further processing and avoid duplicate lint triggers.
fn scan_block_for_eq(cx: &LateContext<'_>, blocks: &[&Block<'_>]) -> Option<BlockEqual> {
fn scan_block_for_eq(cx: &LateContext<'_>, conds: &[&Expr<'_>], blocks: &[&Block<'_>]) -> Option<BlockEqual> {
let mut start_eq = usize::MAX;
let mut end_eq = usize::MAX;
let mut expr_eq = true;
let mut iter = blocks.windows(2);
while let Some(&[win0, win1]) = iter.next() {
let l_stmts = win0.stmts;
let r_stmts = win1.stmts;
let mut iter = blocks.windows(2).enumerate();
while let Some((i, &[block0, block1])) = iter.next() {
let l_stmts = block0.stmts;
let r_stmts = block1.stmts;

// `SpanlessEq` now keeps track of the locals and is therefore context sensitive clippy#6752.
// The comparison therefore needs to be done in a way that builds the correct context.
Expand All @@ -340,22 +341,26 @@ fn scan_block_for_eq(cx: &LateContext<'_>, blocks: &[&Block<'_>]) -> Option<Bloc
it1.zip(it2)
.fold(0, |acc, (l, r)| if evaluator.eq_stmt(l, r) { acc + 1 } else { 0 })
};
let block_expr_eq = both(&win0.expr, &win1.expr, |l, r| evaluator.eq_expr(l, r));
let block_expr_eq = both(&block0.expr, &block1.expr, |l, r| evaluator.eq_expr(l, r));

// IF_SAME_THEN_ELSE
if_chain! {
if block_expr_eq;
if l_stmts.len() == r_stmts.len();
if l_stmts.len() == current_start_eq;
if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win0.hir_id);
if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win1.hir_id);
// `conds` may have one last item than `blocks`.
// Any `i` from `blocks.windows(2)` will exist in `conds`, but `i+1` may not exist on the last iteration.
if !matches!(conds[i].kind, ExprKind::Let(..));
if !matches!(conds.get(i + 1).map(|e| &e.kind), Some(ExprKind::Let(..)));
if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, block0.hir_id);
if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, block1.hir_id);
then {
span_lint_and_note(
cx,
IF_SAME_THEN_ELSE,
win0.span,
block0.span,
"this `if` has identical blocks",
Some(win1.span),
Some(block1.span),
"same as this",
);

Expand Down
19 changes: 17 additions & 2 deletions clippy_lints/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::kw;
use rustc_span::{sym, Span};
use rustc_span::{sym, BytePos, Span};

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -84,7 +84,22 @@ impl<'tcx> LateLintPass<'tcx> for UselessFormat {
ExprKind::MethodCall(path, ..) => path.ident.name.as_str() == "to_string",
_ => false,
};
let sugg = if is_new_string {
let sugg = if format_args.format_string_span.contains(value.span) {
// Implicit argument. e.g. `format!("{x}")` span points to `{x}`
let spdata = value.span.data();
let span = Span::new(
spdata.lo + BytePos(1),
spdata.hi - BytePos(1),
spdata.ctxt,
spdata.parent
);
let snip = snippet_with_applicability(cx, span, "..", &mut applicability);
if is_new_string {
snip.into()
} else {
format!("{snip}.to_string()")
}
} else if is_new_string {
snippet_with_applicability(cx, value.span, "..", &mut applicability).into_owned()
} else {
let sugg = Sugg::hir_with_applicability(cx, value, "<arg>", &mut applicability);
Expand Down
11 changes: 7 additions & 4 deletions clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ fn check_raw_ptr<'tcx>(
let expr = &body.value;
if unsafety == hir::Unsafety::Normal && cx.access_levels.is_exported(def_id) {
let raw_ptrs = iter_input_pats(decl, body)
.zip(decl.inputs.iter())
.filter_map(|(arg, ty)| raw_ptr_arg(arg, ty))
.filter_map(|arg| raw_ptr_arg(cx, arg))
.collect::<HirIdSet>();

if !raw_ptrs.is_empty() {
Expand All @@ -59,8 +58,12 @@ fn check_raw_ptr<'tcx>(
}
}

fn raw_ptr_arg(arg: &hir::Param<'_>, ty: &hir::Ty<'_>) -> Option<hir::HirId> {
if let (&hir::PatKind::Binding(_, id, _, _), &hir::TyKind::Ptr(_)) = (&arg.pat.kind, &ty.kind) {
fn raw_ptr_arg(cx: &LateContext<'_>, arg: &hir::Param<'_>) -> Option<hir::HirId> {
if let (&hir::PatKind::Binding(_, id, _, _), Some(&ty::RawPtr(_))) = (
&arg.pat.kind,
cx.maybe_typeck_results()
.map(|typeck_results| typeck_results.pat_ty(arg.pat).kind()),
) {
Some(id)
} else {
None
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/iter_not_returning_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl<'tcx> LateLintPass<'tcx> for IterNotReturningIterator {

fn check_sig(cx: &LateContext<'_>, name: &str, sig: &FnSig<'_>, fn_id: LocalDefId) {
if sig.decl.implicit_self.has_implicit_self() {
let ret_ty = cx.tcx.fn_sig(fn_id).skip_binder().output();
let ret_ty = cx.tcx.erase_late_bound_regions(cx.tcx.fn_sig(fn_id).output());
let ret_ty = cx
.tcx
.try_normalize_erasing_regions(cx.param_env, ret_ty)
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.register_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(methods::ITER_NEXT_SLICE),
LintId::of(methods::ITER_NTH),
LintId::of(methods::ITER_NTH_ZERO),
LintId::of(methods::ITER_OVEREAGER_CLONED),
LintId::of(methods::ITER_SKIP_NEXT),
LintId::of(methods::MANUAL_FILTER_MAP),
LintId::of(methods::MANUAL_FIND_MAP),
Expand Down Expand Up @@ -249,7 +250,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(reference::REF_IN_DEREF),
LintId::of(regex::INVALID_REGEX),
LintId::of(repeat_once::REPEAT_ONCE),
LintId::of(return_self_not_must_use::RETURN_SELF_NOT_MUST_USE),
LintId::of(returns::LET_AND_RETURN),
LintId::of(returns::NEEDLESS_RETURN),
LintId::of(self_assignment::SELF_ASSIGNMENT),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ store.register_lints(&[
methods::ITER_NEXT_SLICE,
methods::ITER_NTH,
methods::ITER_NTH_ZERO,
methods::ITER_OVEREAGER_CLONED,
methods::ITER_SKIP_NEXT,
methods::MANUAL_FILTER_MAP,
methods::MANUAL_FIND_MAP,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_pedantic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
LintId::of(ranges::RANGE_PLUS_ONE),
LintId::of(redundant_else::REDUNDANT_ELSE),
LintId::of(ref_option_ref::REF_OPTION_REF),
LintId::of(return_self_not_must_use::RETURN_SELF_NOT_MUST_USE),
LintId::of(semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED),
LintId::of(strings::STRING_ADD_ASSIGN),
LintId::of(trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_perf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![
LintId::of(methods::EXPECT_FUN_CALL),
LintId::of(methods::EXTEND_WITH_DRAIN),
LintId::of(methods::ITER_NTH),
LintId::of(methods::ITER_OVEREAGER_CLONED),
LintId::of(methods::MANUAL_STR_REPEAT),
LintId::of(methods::OR_FUN_CALL),
LintId::of(methods::SINGLE_CHAR_PATTERN),
Expand Down
1 change: 0 additions & 1 deletion clippy_lints/src/lib.register_suspicious.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec!
LintId::of(methods::SUSPICIOUS_MAP),
LintId::of(mut_key::MUTABLE_KEY_TYPE),
LintId::of(octal_escapes::OCTAL_ESCAPES),
LintId::of(return_self_not_must_use::RETURN_SELF_NOT_MUST_USE),
LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),
])
21 changes: 15 additions & 6 deletions clippy_lints/src/methods/implicit_clone.rs
Original file line number Diff line number Diff line change
@@ -1,31 +1,40 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_context;
use clippy_utils::ty::peel_mid_ty_refs;
use clippy_utils::{is_diag_item_method, is_diag_trait_item};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty::TyS;
use rustc_span::{sym, Span};
use rustc_span::sym;

use super::IMPLICIT_CLONE;

pub fn check(cx: &LateContext<'_>, method_name: &str, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, span: Span) {
pub fn check(cx: &LateContext<'_>, method_name: &str, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>) {
if_chain! {
if let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
if is_clone_like(cx, method_name, method_def_id);
let return_type = cx.typeck_results().expr_ty(expr);
let input_type = cx.typeck_results().expr_ty(recv).peel_refs();
let input_type = cx.typeck_results().expr_ty(recv);
let (input_type, ref_count) = peel_mid_ty_refs(input_type);
if let Some(ty_name) = input_type.ty_adt_def().map(|adt_def| cx.tcx.item_name(adt_def.did));
if TyS::same_type(return_type, input_type);
then {
let mut app = Applicability::MachineApplicable;
let recv_snip = snippet_with_context(cx, recv.span, expr.span.ctxt(), "..", &mut app).0;
span_lint_and_sugg(
cx,
IMPLICIT_CLONE,
span,
expr.span,
&format!("implicitly cloning a `{}` by calling `{}` on its dereferenced type", ty_name, method_name),
"consider using",
"clone".to_string(),
Applicability::MachineApplicable
if ref_count > 1 {
format!("({}{}).clone()", "*".repeat(ref_count - 1), recv_snip)
} else {
format!("{}.clone()", recv_snip)
},
app,
);
}
}
Expand Down
62 changes: 62 additions & 0 deletions clippy_lints/src/methods/iter_overeager_cloned.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet;
use clippy_utils::ty::{get_iterator_item_ty, is_copy};
use itertools::Itertools;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty;
use std::ops::Not;

use super::ITER_OVEREAGER_CLONED;
use crate::redundant_clone::REDUNDANT_CLONE;

/// lint overeager use of `cloned()` for `Iterator`s
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
recv: &'tcx hir::Expr<'_>,
name: &str,
map_arg: &[hir::Expr<'_>],
) {
// Check if it's iterator and get type associated with `Item`.
let inner_ty = match get_iterator_item_ty(cx, cx.typeck_results().expr_ty_adjusted(recv)) {
Some(ty) => ty,
_ => return,
};

match inner_ty.kind() {
ty::Ref(_, ty, _) if !is_copy(cx, ty) => {},
_ => return,
};

let (lint, preserve_cloned) = match name {
"count" => (REDUNDANT_CLONE, false),
_ => (ITER_OVEREAGER_CLONED, true),
};
let wildcard_params = map_arg.is_empty().not().then(|| "...").unwrap_or_default();
let msg = format!(
"called `cloned().{}({})` on an `Iterator`. It may be more efficient to call `{}({}){}` instead",
name,
wildcard_params,
name,
wildcard_params,
preserve_cloned.then(|| ".cloned()").unwrap_or_default(),
);

span_lint_and_sugg(
cx,
lint,
expr.span,
&msg,
"try this",
format!(
"{}.{}({}){}",
snippet(cx, recv.span, ".."),
name,
map_arg.iter().map(|a| snippet(cx, a.span, "..")).join(", "),
preserve_cloned.then(|| ".cloned()").unwrap_or_default(),
),
Applicability::MachineApplicable,
);
}
Loading

0 comments on commit 7a2cabb

Please sign in to comment.