Skip to content

Commit

Permalink
Auto merge of #106780 - flip1995:clippyup, r=Manishearth
Browse files Browse the repository at this point in the history
Update Clippy

r? `@Manishearth`
  • Loading branch information
bors committed Jan 12, 2023
2 parents 1bc3683 + 01c7584 commit 61a415b
Show file tree
Hide file tree
Showing 85 changed files with 1,879 additions and 850 deletions.
7 changes: 7 additions & 0 deletions src/tools/clippy/.github/driver.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ test "$sysroot" = $desired_sysroot
sysroot=$(SYSROOT=$desired_sysroot ./target/debug/clippy-driver --print sysroot)
test "$sysroot" = $desired_sysroot

# Check that the --sysroot argument is only passed once (SYSROOT is ignored)
(
cd rustc_tools_util
touch src/lib.rs
SYSROOT=/tmp RUSTFLAGS="--sysroot=$(rustc --print sysroot)" ../target/debug/cargo-clippy clippy --verbose
)

# Make sure this isn't set - clippy-driver should cope without it
unset CARGO_MANIFEST_DIR

Expand Down
1 change: 1 addition & 0 deletions src/tools/clippy/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4137,6 +4137,7 @@ Released 2018-09-13
[`derive_hash_xor_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq
[`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord
[`derive_partial_eq_without_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_partial_eq_without_eq
[`derived_hash_with_manual_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derived_hash_with_manual_eq
[`disallowed_macros`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_macros
[`disallowed_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method
[`disallowed_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/book/src/installation.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Installation

If you're using `rustup` to install and manage you're Rust toolchains, Clippy is
If you're using `rustup` to install and manage your Rust toolchains, Clippy is
usually **already installed**. In that case you can skip this chapter and go to
the [Usage] chapter.

Expand Down
3 changes: 3 additions & 0 deletions src/tools/clippy/clippy_dev/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
// warn on lints, that are included in `rust-lang/rust`s bootstrap
#![warn(rust_2018_idioms, unused_lifetimes)]

// The `rustc_driver` crate seems to be required in order to use the `rust_lexer` crate.
#[allow(unused_extern_crates)]
extern crate rustc_driver;
extern crate rustc_lexer;

use std::path::PathBuf;
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/box_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_hir::{
Block, Expr, ExprKind, Local, Node, QPath, TyKind,
};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_middle::{lint::in_external_macro, ty::print::with_forced_trimmed_paths};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;

Expand Down Expand Up @@ -59,7 +59,7 @@ impl LateLintPass<'_> for BoxDefault {
if is_plain_default(arg_path) || given_type(cx, expr) {
"Box::default()".into()
} else {
format!("Box::<{arg_ty}>::default()")
with_forced_trimmed_paths!(format!("Box::<{arg_ty}>::default()"))
},
Applicability::MachineApplicable
);
Expand Down
6 changes: 5 additions & 1 deletion src/tools/clippy/clippy_lints/src/copies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,11 @@ fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbo
.iter()
.filter(|&&(_, name)| !name.as_str().starts_with('_'))
.any(|&(_, name)| {
let mut walker = ContainsName { name, result: false };
let mut walker = ContainsName {
name,
result: false,
cx,
};

// Scan block
block
Expand Down
10 changes: 5 additions & 5 deletions src/tools/clippy/clippy_lints/src/dbg_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ use rustc_span::sym;

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of dbg!() macro.
/// Checks for usage of the [`dbg!`](https://doc.rust-lang.org/std/macro.dbg.html) macro.
///
/// ### Why is this bad?
/// `dbg!` macro is intended as a debugging tool. It
/// should not be in version control.
/// The `dbg!` macro is intended as a debugging tool. It should not be present in released
/// software or committed to a version control system.
///
/// ### Example
/// ```rust,ignore
Expand Down Expand Up @@ -91,8 +91,8 @@ impl LateLintPass<'_> for DbgMacro {
cx,
DBG_MACRO,
macro_call.span,
"`dbg!` macro is intended as a debugging tool",
"ensure to avoid having uses of it in version control",
"the `dbg!` macro is intended as a debugging tool",
"remove the invocation before committing it to a version control system",
suggestion,
applicability,
);
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::dereference::NEEDLESS_BORROW_INFO,
crate::dereference::REF_BINDING_TO_REFERENCE_INFO,
crate::derivable_impls::DERIVABLE_IMPLS_INFO,
crate::derive::DERIVE_HASH_XOR_EQ_INFO,
crate::derive::DERIVED_HASH_WITH_MANUAL_EQ_INFO,
crate::derive::DERIVE_ORD_XOR_PARTIAL_ORD_INFO,
crate::derive::DERIVE_PARTIAL_EQ_WITHOUT_EQ_INFO,
crate::derive::EXPL_IMPL_CLONE_ON_COPY_INFO,
Expand Down
7 changes: 3 additions & 4 deletions src/tools/clippy/clippy_lints/src/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use rustc_hir::def::Res;
use rustc_hir::{Block, Expr, ExprKind, PatKind, QPath, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_middle::ty::print::with_forced_trimmed_paths;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::{Ident, Symbol};
use rustc_span::Span;
Expand Down Expand Up @@ -98,9 +99,7 @@ impl<'tcx> LateLintPass<'tcx> for Default {
if let ty::Adt(def, ..) = expr_ty.kind();
if !is_from_proc_macro(cx, expr);
then {
// TODO: Work out a way to put "whatever the imported way of referencing
// this type in this file" rather than a fully-qualified type.
let replacement = format!("{}::default()", cx.tcx.def_path_str(def.did()));
let replacement = with_forced_trimmed_paths!(format!("{}::default()", cx.tcx.def_path_str(def.did())));
span_lint_and_sugg(
cx,
DEFAULT_TRAIT_ACCESS,
Expand Down Expand Up @@ -170,7 +169,7 @@ impl<'tcx> LateLintPass<'tcx> for Default {
// find out if and which field was set by this `consecutive_statement`
if let Some((field_ident, assign_rhs)) = field_reassigned_by_stmt(consecutive_statement, binding_name) {
// interrupt and cancel lint if assign_rhs references the original binding
if contains_name(binding_name, assign_rhs) {
if contains_name(binding_name, assign_rhs, cx) {
cancel_lint = true;
break;
}
Expand Down
8 changes: 4 additions & 4 deletions src/tools/clippy/clippy_lints/src/dereference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1282,10 +1282,10 @@ fn referent_used_exactly_once<'tcx>(
possible_borrowers.push((body_owner_local_def_id, PossibleBorrowerMap::new(cx, mir)));
}
let possible_borrower = &mut possible_borrowers.last_mut().unwrap().1;
// If `place.local` were not included here, the `copyable_iterator::warn` test would fail. The
// reason is that `PossibleBorrowerVisitor::visit_terminator` considers `place.local` a possible
// borrower of itself. See the comment in that method for an explanation as to why.
possible_borrower.at_most_borrowers(cx, &[local, place.local], place.local, location)
// If `only_borrowers` were used here, the `copyable_iterator::warn` test would fail. The reason is
// that `PossibleBorrowerVisitor::visit_terminator` considers `place.local` a possible borrower of
// itself. See the comment in that method for an explanation as to why.
possible_borrower.bounded_borrowers(&[local], &[local, place.local], place.local, location)
&& used_exactly_once(mir, place.local).unwrap_or(false)
} else {
false
Expand Down
161 changes: 117 additions & 44 deletions src/tools/clippy/clippy_lints/src/derivable_impls.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::source::indent_of;
use clippy_utils::{is_default_equivalent, peel_blocks};
use rustc_errors::Applicability;
use rustc_hir::{
def::{DefKind, Res},
Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, TyKind,
def::{CtorKind, CtorOf, DefKind, Res},
Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, Ty, TyKind,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_middle::ty::{AdtDef, DefIdTree};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::sym;

declare_clippy_lint! {
Expand Down Expand Up @@ -51,7 +54,18 @@ declare_clippy_lint! {
"manual implementation of the `Default` trait which is equal to a derive"
}

declare_lint_pass!(DerivableImpls => [DERIVABLE_IMPLS]);
pub struct DerivableImpls {
msrv: Msrv,
}

impl DerivableImpls {
#[must_use]
pub fn new(msrv: Msrv) -> Self {
DerivableImpls { msrv }
}
}

impl_lint_pass!(DerivableImpls => [DERIVABLE_IMPLS]);

fn is_path_self(e: &Expr<'_>) -> bool {
if let ExprKind::Path(QPath::Resolved(_, p)) = e.kind {
Expand All @@ -61,6 +75,98 @@ fn is_path_self(e: &Expr<'_>) -> bool {
}
}

fn check_struct<'tcx>(
cx: &LateContext<'tcx>,
item: &'tcx Item<'_>,
self_ty: &Ty<'_>,
func_expr: &Expr<'_>,
adt_def: AdtDef<'_>,
) {
if let TyKind::Path(QPath::Resolved(_, p)) = self_ty.kind {
if let Some(PathSegment { args: Some(a), .. }) = p.segments.last() {
for arg in a.args {
if !matches!(arg, GenericArg::Lifetime(_)) {
return;
}
}
}
}
let should_emit = match peel_blocks(func_expr).kind {
ExprKind::Tup(fields) => fields.iter().all(|e| is_default_equivalent(cx, e)),
ExprKind::Call(callee, args) if is_path_self(callee) => args.iter().all(|e| is_default_equivalent(cx, e)),
ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_equivalent(cx, ef.expr)),
_ => false,
};

if should_emit {
let struct_span = cx.tcx.def_span(adt_def.did());
span_lint_and_then(cx, DERIVABLE_IMPLS, item.span, "this `impl` can be derived", |diag| {
diag.span_suggestion_hidden(
item.span,
"remove the manual implementation...",
String::new(),
Applicability::MachineApplicable,
);
diag.span_suggestion(
struct_span.shrink_to_lo(),
"...and instead derive it",
"#[derive(Default)]\n".to_string(),
Applicability::MachineApplicable,
);
});
}
}

fn check_enum<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>, func_expr: &Expr<'_>, adt_def: AdtDef<'_>) {
if_chain! {
if let ExprKind::Path(QPath::Resolved(None, p)) = &peel_blocks(func_expr).kind;
if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Const), id) = p.res;
if let variant_id = cx.tcx.parent(id);
if let Some(variant_def) = adt_def.variants().iter().find(|v| v.def_id == variant_id);
if variant_def.fields.is_empty();
if !variant_def.is_field_list_non_exhaustive();

then {
let enum_span = cx.tcx.def_span(adt_def.did());
let indent_enum = indent_of(cx, enum_span).unwrap_or(0);
let variant_span = cx.tcx.def_span(variant_def.def_id);
let indent_variant = indent_of(cx, variant_span).unwrap_or(0);
span_lint_and_then(
cx,
DERIVABLE_IMPLS,
item.span,
"this `impl` can be derived",
|diag| {
diag.span_suggestion_hidden(
item.span,
"remove the manual implementation...",
String::new(),
Applicability::MachineApplicable
);
diag.span_suggestion(
enum_span.shrink_to_lo(),
"...and instead derive it...",
format!(
"#[derive(Default)]\n{indent}",
indent = " ".repeat(indent_enum),
),
Applicability::MachineApplicable
);
diag.span_suggestion(
variant_span.shrink_to_lo(),
"...and mark the default variant",
format!(
"#[default]\n{indent}",
indent = " ".repeat(indent_variant),
),
Applicability::MachineApplicable
);
}
);
}
}
}

impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
if_chain! {
Expand All @@ -83,49 +189,16 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
if !attrs.iter().any(|attr| attr.doc_str().is_some());
if let child_attrs = cx.tcx.hir().attrs(impl_item_hir);
if !child_attrs.iter().any(|attr| attr.doc_str().is_some());
if adt_def.is_struct();
then {
if let TyKind::Path(QPath::Resolved(_, p)) = self_ty.kind {
if let Some(PathSegment { args: Some(a), .. }) = p.segments.last() {
for arg in a.args {
if !matches!(arg, GenericArg::Lifetime(_)) {
return;
}
}
}
}
let should_emit = match peel_blocks(func_expr).kind {
ExprKind::Tup(fields) => fields.iter().all(|e| is_default_equivalent(cx, e)),
ExprKind::Call(callee, args)
if is_path_self(callee) => args.iter().all(|e| is_default_equivalent(cx, e)),
ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_equivalent(cx, ef.expr)),
_ => false,
};

if should_emit {
let struct_span = cx.tcx.def_span(adt_def.did());
span_lint_and_then(
cx,
DERIVABLE_IMPLS,
item.span,
"this `impl` can be derived",
|diag| {
diag.span_suggestion_hidden(
item.span,
"remove the manual implementation...",
String::new(),
Applicability::MachineApplicable
);
diag.span_suggestion(
struct_span.shrink_to_lo(),
"...and instead derive it",
"#[derive(Default)]\n".to_string(),
Applicability::MachineApplicable
);
}
);
then {
if adt_def.is_struct() {
check_struct(cx, item, self_ty, func_expr, adt_def);
} else if adt_def.is_enum() && self.msrv.meets(msrvs::DEFAULT_ENUM_ATTRIBUTE) {
check_enum(cx, item, func_expr, adt_def);
}
}
}
}

extract_msrv_attr!(LateContext);
}
Loading

0 comments on commit 61a415b

Please sign in to comment.