diff options
| author | bors <bors@rust-lang.org> | 2021-01-06 08:22:47 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2021-01-06 08:22:47 +0000 |
| commit | efccfe80e4e5514d6f0da0f22eebd0f88f95c1a5 (patch) | |
| tree | 1a1f607eb5c703b0fc284b475a469bebf69b7d6c | |
| parent | 42b9e92af7241d8ca1ec5207ffd8bfda32558730 (diff) | |
| parent | 92f2bbbe06b92ad9fc984804307958d8566cd4ed (diff) | |
| download | rust-efccfe80e4e5514d6f0da0f22eebd0f88f95c1a5.tar.gz rust-efccfe80e4e5514d6f0da0f22eebd0f88f95c1a5.zip | |
Auto merge of #6553 - phansch:field-reassign-with-default-macros, r=flip1995
Fix derive and macro related false positives in `field_reassign_with_default` Closes #6545 changelog: Fix derive and macro related false positives in [`field_reassign_with_default`]
| -rw-r--r-- | clippy_lints/src/default.rs | 3 | ||||
| -rw-r--r-- | clippy_lints/src/needless_borrow.rs | 5 | ||||
| -rw-r--r-- | tests/ui/auxiliary/macro_rules.rs | 16 | ||||
| -rw-r--r-- | tests/ui/auxiliary/proc_macro_derive.rs | 18 | ||||
| -rw-r--r-- | tests/ui/field_reassign_with_default.rs | 16 | ||||
| -rw-r--r-- | tests/ui/field_reassign_with_default.stderr | 40 |
6 files changed, 75 insertions, 23 deletions
diff --git a/clippy_lints/src/default.rs b/clippy_lints/src/default.rs index 9fa06d7cde9..f7224811e6e 100644 --- a/clippy_lints/src/default.rs +++ b/clippy_lints/src/default.rs @@ -8,6 +8,7 @@ use rustc_errors::Applicability; use rustc_hir::def::Res; use rustc_hir::{Block, Expr, ExprKind, PatKind, QPath, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::lint::in_external_macro; use rustc_middle::ty; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::symbol::{Ident, Symbol}; @@ -120,6 +121,8 @@ impl LateLintPass<'_> for Default { // only take `let ...` statements if let StmtKind::Local(local) = stmt.kind; if let Some(expr) = local.init; + if !any_parent_is_automatically_derived(cx.tcx, expr.hir_id); + if !in_external_macro(cx.tcx.sess, expr.span); // only take bindings to identifiers if let PatKind::Binding(_, binding_id, ident, _) = local.pat.kind; // only when assigning `... = Default::default()` diff --git a/clippy_lints/src/needless_borrow.rs b/clippy_lints/src/needless_borrow.rs index bff53eb8cca..f1c06692e30 100644 --- a/clippy_lints/src/needless_borrow.rs +++ b/clippy_lints/src/needless_borrow.rs @@ -2,7 +2,7 @@ //! //! This lint is **warn** by default -use crate::utils::{snippet_opt, span_lint_and_then}; +use crate::utils::{is_automatically_derived, snippet_opt, span_lint_and_then}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{BindingAnnotation, BorrowKind, Expr, ExprKind, HirId, Item, Mutability, Pat, PatKind}; @@ -10,7 +10,6 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_middle::ty::adjustment::{Adjust, Adjustment}; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::sym; declare_clippy_lint! { /// **What it does:** Checks for address of operations (`&`) that are going to @@ -116,7 +115,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow { } fn check_item(&mut self, _: &LateContext<'tcx>, item: &'tcx Item<'_>) { - if item.attrs.iter().any(|a| a.has_name(sym::automatically_derived)) { + if is_automatically_derived(item.attrs) { debug_assert!(self.derived_item.is_none()); self.derived_item = Some(item.hir_id); } diff --git a/tests/ui/auxiliary/macro_rules.rs b/tests/ui/auxiliary/macro_rules.rs index 18324823468..d6ecd8568ce 100644 --- a/tests/ui/auxiliary/macro_rules.rs +++ b/tests/ui/auxiliary/macro_rules.rs @@ -94,3 +94,19 @@ macro_rules! large_enum_variant { } }; } + +#[macro_export] +macro_rules! field_reassign_with_default { + () => { + #[derive(Default)] + struct A { + pub i: i32, + pub j: i64, + } + fn lint() { + let mut a: A = Default::default(); + a.i = 42; + a; + } + }; +} diff --git a/tests/ui/auxiliary/proc_macro_derive.rs b/tests/ui/auxiliary/proc_macro_derive.rs index 7c4e4a14551..24891682d36 100644 --- a/tests/ui/auxiliary/proc_macro_derive.rs +++ b/tests/ui/auxiliary/proc_macro_derive.rs @@ -4,6 +4,7 @@ #![crate_type = "proc-macro"] #![feature(repr128, proc_macro_quote)] #![allow(incomplete_features)] +#![allow(clippy::field_reassign_with_default)] #![allow(clippy::eq_op)] extern crate proc_macro; @@ -23,3 +24,20 @@ pub fn derive(_: TokenStream) -> TokenStream { }; output } + +#[proc_macro_derive(FieldReassignWithDefault)] +pub fn derive_foo(_input: TokenStream) -> TokenStream { + quote! { + #[derive(Default)] + struct A { + pub i: i32, + pub j: i64, + } + #[automatically_derived] + fn lint() { + let mut a: A = Default::default(); + a.i = 42; + a; + } + } +} diff --git a/tests/ui/field_reassign_with_default.rs b/tests/ui/field_reassign_with_default.rs index 2990397c03e..9fc208f5332 100644 --- a/tests/ui/field_reassign_with_default.rs +++ b/tests/ui/field_reassign_with_default.rs @@ -1,5 +1,18 @@ +// aux-build:proc_macro_derive.rs +// aux-build:macro_rules.rs + #![warn(clippy::field_reassign_with_default)] +#[macro_use] +extern crate proc_macro_derive; +#[macro_use] +extern crate macro_rules; + +// Don't lint on derives that derive `Default` +// See https://github.com/rust-lang/rust-clippy/issues/6545 +#[derive(FieldReassignWithDefault)] +struct DerivedStruct; + #[derive(Default)] struct A { i: i32, @@ -120,6 +133,9 @@ fn main() { // don't expand macros in the suggestion (#6522) let mut a: C = C::default(); a.i = vec![1]; + + // Don't lint in external macros + field_reassign_with_default!(); } mod m { diff --git a/tests/ui/field_reassign_with_default.stderr b/tests/ui/field_reassign_with_default.stderr index 59d2ac8ed69..2f0f28f7bb7 100644 --- a/tests/ui/field_reassign_with_default.stderr +++ b/tests/ui/field_reassign_with_default.stderr @@ -1,84 +1,84 @@ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:35:5 + --> $DIR/field_reassign_with_default.rs:48:5 | LL | a.i = 42; | ^^^^^^^^^ | = note: `-D clippy::field-reassign-with-default` implied by `-D warnings` -note: consider initializing the variable with `A { i: 42, ..Default::default() }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:34:5 +note: consider initializing the variable with `main::A { i: 42, ..Default::default() }` and removing relevant reassignments + --> $DIR/field_reassign_with_default.rs:47:5 | LL | let mut a: A = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:75:5 + --> $DIR/field_reassign_with_default.rs:88:5 | LL | a.j = 43; | ^^^^^^^^^ | -note: consider initializing the variable with `A { j: 43, i: 42 }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:74:5 +note: consider initializing the variable with `main::A { j: 43, i: 42 }` and removing relevant reassignments + --> $DIR/field_reassign_with_default.rs:87:5 | LL | let mut a: A = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:80:5 + --> $DIR/field_reassign_with_default.rs:93:5 | LL | a.i = 42; | ^^^^^^^^^ | -note: consider initializing the variable with `A { i: 42, j: 44 }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:79:5 +note: consider initializing the variable with `main::A { i: 42, j: 44 }` and removing relevant reassignments + --> $DIR/field_reassign_with_default.rs:92:5 | LL | let mut a: A = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:86:5 + --> $DIR/field_reassign_with_default.rs:99:5 | LL | a.i = 42; | ^^^^^^^^^ | -note: consider initializing the variable with `A { i: 42, ..Default::default() }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:85:5 +note: consider initializing the variable with `main::A { i: 42, ..Default::default() }` and removing relevant reassignments + --> $DIR/field_reassign_with_default.rs:98:5 | LL | let mut a = A::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:96:5 + --> $DIR/field_reassign_with_default.rs:109:5 | LL | a.i = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ | -note: consider initializing the variable with `A { i: Default::default(), ..Default::default() }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:95:5 +note: consider initializing the variable with `main::A { i: Default::default(), ..Default::default() }` and removing relevant reassignments + --> $DIR/field_reassign_with_default.rs:108:5 | LL | let mut a: A = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:100:5 + --> $DIR/field_reassign_with_default.rs:113:5 | LL | a.i = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ | -note: consider initializing the variable with `A { i: Default::default(), j: 45 }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:99:5 +note: consider initializing the variable with `main::A { i: Default::default(), j: 45 }` and removing relevant reassignments + --> $DIR/field_reassign_with_default.rs:112:5 | LL | let mut a: A = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:122:5 + --> $DIR/field_reassign_with_default.rs:135:5 | LL | a.i = vec![1]; | ^^^^^^^^^^^^^^ | note: consider initializing the variable with `C { i: vec![1], ..Default::default() }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:121:5 + --> $DIR/field_reassign_with_default.rs:134:5 | LL | let mut a: C = C::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ |
