mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #10489 - samueltardieu:issue-10476, r=giraffate

Browse Source 
New lint: detect unnecessary struct building

Fixes #10476.

Running this lint on the top 500 crates produced one hit (in `rust-lang/rust-bindgen`) and [a PR has been submitted there](https://github.com/rust-lang/rust-bindgen/pull/2440).

changelog: [`unnecessary_struct_initialization`]: new lint
This commit is contained in:
bors 2023-03-24 00:12:02 +00:00
commit c72c914d21
13 changed files with 355 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -4987,6 +4987,7 @@ Released 2018-09-13
[`unnecessary_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_doc
[`unnecessary_self_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_self_imports
[`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by
[`unnecessary_struct_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_struct_initialization
[`unnecessary_to_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned
[`unnecessary_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap
[`unnecessary_wraps`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_wraps

1
clippy_lints/src/declared_lints.rs
View File

@ -618,6 +618,7 @@
crate::unnamed_address::VTABLE_ADDRESS_COMPARISONS_INFO,
crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO,
crate::unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS_INFO,
crate::unnecessary_struct_initialization::UNNECESSARY_STRUCT_INITIALIZATION_INFO,
crate::unnecessary_wraps::UNNECESSARY_WRAPS_INFO,
crate::unnested_or_patterns::UNNESTED_OR_PATTERNS_INFO,
crate::unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME_INFO,

2
clippy_lints/src/lib.rs
View File

@ -302,6 +302,7 @@
mod unnamed_address;
mod unnecessary_owned_empty_strings;
mod unnecessary_self_imports;
mod unnecessary_struct_initialization;
mod unnecessary_wraps;
mod unnested_or_patterns;
mod unsafe_removed_from_name;
@ -938,6 +939,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|_| Box::new(let_with_type_underscore::UnderscoreTyped));
store.register_late_pass(|_| Box::new(allow_attributes::AllowAttribute));
store.register_late_pass(move |_| Box::new(manual_main_separator_str::ManualMainSeparatorStr::new(msrv())));
store.register_late_pass(|_| Box::new(unnecessary_struct_initialization::UnnecessaryStruct));
// add lints here, do not remove this comment, it's used in `new_lint`
}

84
clippy_lints/src/unnecessary_struct_initialization.rs Normal file
View File

@ -0,0 +1,84 @@
use clippy_utils::{diagnostics::span_lint_and_sugg, get_parent_expr, path_to_local, source::snippet, ty::is_copy};
use rustc_hir::{BindingAnnotation, Expr, ExprKind, Node, PatKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
declare_clippy_lint! {
/// ### What it does
/// Checks for initialization of a `struct` by copying a base without setting
/// any field.
///
/// ### Why is this bad?
/// Readibility suffers from unnecessary struct building.
///
/// ### Example
/// ```rust
/// struct S { s: String }
///
/// let a = S { s: String::from("Hello, world!") };
/// let b = S { ..a };
/// ```
/// Use instead:
/// ```rust
/// struct S { s: String }
///
/// let a = S { s: String::from("Hello, world!") };
/// let b = a;
/// ```
#[clippy::version = "1.70.0"]
pub UNNECESSARY_STRUCT_INITIALIZATION,
complexity,
"struct built from a base that can be written mode concisely"
}
declare_lint_pass!(UnnecessaryStruct => [UNNECESSARY_STRUCT_INITIALIZATION]);
impl LateLintPass<'_> for UnnecessaryStruct {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
if let ExprKind::Struct(_, &[], Some(base)) = expr.kind {
if let Some(parent) = get_parent_expr(cx, expr) &&
let parent_ty = cx.typeck_results().expr_ty_adjusted(parent) &&
parent_ty.is_any_ptr()
{
if is_copy(cx, cx.typeck_results().expr_ty(expr)) && path_to_local(base).is_some() {
// When the type implements `Copy`, a reference to the new struct works on the
// copy. Using the original would borrow it.
return;
}
if parent_ty.is_mutable_ptr() && !is_mutable(cx, base) {
// The original can be used in a mutable reference context only if it is mutable.
return;
}
}
// TODO: do not propose to replace *XX if XX is not Copy
if let ExprKind::Unary(UnOp::Deref, target) = base.kind &&
matches!(target.kind, ExprKind::Path(..)) &&
!is_copy(cx, cx.typeck_results().expr_ty(expr))
{
// `*base` cannot be used instead of the struct in the general case if it is not Copy.
return;
}
span_lint_and_sugg(
cx,
UNNECESSARY_STRUCT_INITIALIZATION,
expr.span,
"unnecessary struct building",
"replace with",
snippet(cx, base.span, "..").into_owned(),
rustc_errors::Applicability::MachineApplicable,
);
}
}
}
fn is_mutable(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
if let Some(hir_id) = path_to_local(expr) &&
let Node::Pat(pat) = cx.tcx.hir().get(hir_id)
{
matches!(pat.kind, PatKind::Binding(BindingAnnotation::MUT, ..))
} else {
true
}
}

2
tests/ui/needless_update.rs
View File

@ -1,5 +1,5 @@
#![warn(clippy::needless_update)]
#![allow(clippy::no_effect)]
#![allow(clippy::no_effect, clippy::unnecessary_struct_initialization)]
struct S {
pub a: i32,

7
tests/ui/no_effect.rs
View File

@ -1,7 +1,12 @@
#![feature(box_syntax, fn_traits, unboxed_closures)]
#![warn(clippy::no_effect_underscore_binding)]
#![allow(dead_code, path_statements)]
#![allow(clippy::deref_addrof, clippy::redundant_field_names, clippy::uninlined_format_args)]
#![allow(
clippy::deref_addrof,
clippy::redundant_field_names,
clippy::uninlined_format_args,
clippy::unnecessary_struct_initialization
)]
struct Unit;
struct Tuple(i32);

60
tests/ui/no_effect.stderr
View File

@ -1,5 +1,5 @@
error: statement with no effect
--> $DIR/no_effect.rs:92:5
--> $DIR/no_effect.rs:97:5
|
LL | 0;
| ^^
@ -7,157 +7,157 @@ LL | 0;
= note: `-D clippy::no-effect` implied by `-D warnings`
error: statement with no effect
--> $DIR/no_effect.rs:93:5
--> $DIR/no_effect.rs:98:5
|
LL | s2;
| ^^^
error: statement with no effect
--> $DIR/no_effect.rs:94:5
--> $DIR/no_effect.rs:99:5
|
LL | Unit;
| ^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:95:5
--> $DIR/no_effect.rs:100:5
|
LL | Tuple(0);
| ^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:96:5
--> $DIR/no_effect.rs:101:5
|
LL | Struct { field: 0 };
| ^^^^^^^^^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:97:5
--> $DIR/no_effect.rs:102:5
|
LL | Struct { ..s };
| ^^^^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:98:5
--> $DIR/no_effect.rs:103:5
|
LL | Union { a: 0 };
| ^^^^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:99:5
--> $DIR/no_effect.rs:104:5
|
LL | Enum::Tuple(0);
| ^^^^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:100:5
--> $DIR/no_effect.rs:105:5
|
LL | Enum::Struct { field: 0 };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:101:5
--> $DIR/no_effect.rs:106:5
|
LL | 5 + 6;
| ^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:102:5
--> $DIR/no_effect.rs:107:5
|
LL | *&42;
| ^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:103:5
--> $DIR/no_effect.rs:108:5
|
LL | &6;
| ^^^
error: statement with no effect
--> $DIR/no_effect.rs:104:5
--> $DIR/no_effect.rs:109:5
|
LL | (5, 6, 7);
| ^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:105:5
--> $DIR/no_effect.rs:110:5
|
LL | box 42;
| ^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:106:5
--> $DIR/no_effect.rs:111:5
|
LL | ..;
| ^^^
error: statement with no effect
--> $DIR/no_effect.rs:107:5
--> $DIR/no_effect.rs:112:5
|
LL | 5..;
| ^^^^
error: statement with no effect
--> $DIR/no_effect.rs:108:5
--> $DIR/no_effect.rs:113:5
|
LL | ..5;
| ^^^^
error: statement with no effect
--> $DIR/no_effect.rs:109:5
--> $DIR/no_effect.rs:114:5
|
LL | 5..6;
| ^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:110:5
--> $DIR/no_effect.rs:115:5
|
LL | 5..=6;
| ^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:111:5
--> $DIR/no_effect.rs:116:5
|
LL | [42, 55];
| ^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:112:5
--> $DIR/no_effect.rs:117:5
|
LL | [42, 55][1];
| ^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:113:5
--> $DIR/no_effect.rs:118:5
|
LL | (42, 55).1;
| ^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:114:5
--> $DIR/no_effect.rs:119:5
|
LL | [42; 55];
| ^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:115:5
--> $DIR/no_effect.rs:120:5
|
LL | [42; 55][13];
| ^^^^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:117:5
--> $DIR/no_effect.rs:122:5
|
LL | || x += 5;
| ^^^^^^^^^^
error: statement with no effect
--> $DIR/no_effect.rs:119:5
--> $DIR/no_effect.rs:124:5
|
LL | FooString { s: s };
| ^^^^^^^^^^^^^^^^^^^
error: binding to `_` prefixed variable with no side-effect
--> $DIR/no_effect.rs:120:5
--> $DIR/no_effect.rs:125:5
|
LL | let _unused = 1;
| ^^^^^^^^^^^^^^^^
@ -165,19 +165,19 @@ LL | let _unused = 1;
= note: `-D clippy::no-effect-underscore-binding` implied by `-D warnings`
error: binding to `_` prefixed variable with no side-effect
--> $DIR/no_effect.rs:121:5
--> $DIR/no_effect.rs:126:5
|
LL | let _penguin = || println!("Some helpful closure");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: binding to `_` prefixed variable with no side-effect
--> $DIR/no_effect.rs:122:5
--> $DIR/no_effect.rs:127:5
|
LL | let _duck = Struct { field: 0 };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: binding to `_` prefixed variable with no side-effect
--> $DIR/no_effect.rs:123:5
--> $DIR/no_effect.rs:128:5
|
LL | let _cat = [2, 4, 6, 8][2];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^

8
tests/ui/unnecessary_operation.fixed
View File

@ -1,7 +1,13 @@
// run-rustfix
#![feature(box_syntax)]
#![allow(clippy::deref_addrof, dead_code, unused, clippy::no_effect)]
#![allow(
clippy::deref_addrof,
dead_code,
unused,
clippy::no_effect,
clippy::unnecessary_struct_initialization
)]
#![warn(clippy::unnecessary_operation)]
struct Tuple(i32);

8
tests/ui/unnecessary_operation.rs
View File

@ -1,7 +1,13 @@
// run-rustfix
#![feature(box_syntax)]
#![allow(clippy::deref_addrof, dead_code, unused, clippy::no_effect)]
#![allow(
clippy::deref_addrof,
dead_code,
unused,
clippy::no_effect,
clippy::unnecessary_struct_initialization
)]
#![warn(clippy::unnecessary_operation)]
struct Tuple(i32);

40
tests/ui/unnecessary_operation.stderr
View File

@ -1,5 +1,5 @@
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:51:5
--> $DIR/unnecessary_operation.rs:57:5
|
LL | Tuple(get_number());
| ^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
@ -7,109 +7,109 @@ LL | Tuple(get_number());
= note: `-D clippy::unnecessary-operation` implied by `-D warnings`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:52:5
--> $DIR/unnecessary_operation.rs:58:5
|
LL | Struct { field: get_number() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:53:5
--> $DIR/unnecessary_operation.rs:59:5
|
LL | Struct { ..get_struct() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_struct();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:54:5
--> $DIR/unnecessary_operation.rs:60:5
|
LL | Enum::Tuple(get_number());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:55:5
--> $DIR/unnecessary_operation.rs:61:5
|
LL | Enum::Struct { field: get_number() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:56:5
--> $DIR/unnecessary_operation.rs:62:5
|
LL | 5 + get_number();
| ^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `5;get_number();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:57:5
--> $DIR/unnecessary_operation.rs:63:5
|
LL | *&get_number();
| ^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:58:5
--> $DIR/unnecessary_operation.rs:64:5
|
LL | &get_number();
| ^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:59:5
--> $DIR/unnecessary_operation.rs:65:5
|
LL | (5, 6, get_number());
| ^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `5;6;get_number();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:60:5
--> $DIR/unnecessary_operation.rs:66:5
|
LL | box get_number();
| ^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:61:5
--> $DIR/unnecessary_operation.rs:67:5
|
LL | get_number()..;
| ^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:62:5
--> $DIR/unnecessary_operation.rs:68:5
|
LL | ..get_number();
| ^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:63:5
--> $DIR/unnecessary_operation.rs:69:5
|
LL | 5..get_number();
| ^^^^^^^^^^^^^^^^ help: statement can be reduced to: `5;get_number();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:64:5
--> $DIR/unnecessary_operation.rs:70:5
|
LL | [42, get_number()];
| ^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `42;get_number();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:65:5
--> $DIR/unnecessary_operation.rs:71:5
|
LL | [42, 55][get_usize()];
| ^^^^^^^^^^^^^^^^^^^^^^ help: statement can be written as: `assert!([42, 55].len() > get_usize());`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:66:5
--> $DIR/unnecessary_operation.rs:72:5
|
LL | (42, get_number()).1;
| ^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `42;get_number();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:67:5
--> $DIR/unnecessary_operation.rs:73:5
|
LL | [get_number(); 55];
| ^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:68:5
--> $DIR/unnecessary_operation.rs:74:5
|
LL | [42; 55][get_usize()];
| ^^^^^^^^^^^^^^^^^^^^^^ help: statement can be written as: `assert!([42; 55].len() > get_usize());`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:69:5
--> $DIR/unnecessary_operation.rs:75:5
|
LL | / {
LL | | get_number()
@ -117,7 +117,7 @@ LL | | };
| |______^ help: statement can be reduced to: `get_number();`
error: unnecessary operation
--> $DIR/unnecessary_operation.rs:72:5
--> $DIR/unnecessary_operation.rs:78:5
|
LL | / FooString {
LL | | s: String::from("blah"),

73
tests/ui/unnecessary_struct_initialization.fixed Normal file
View File

@ -0,0 +1,73 @@
// run-rustfix
#![allow(unused)]
#![warn(clippy::unnecessary_struct_initialization)]
struct S {
f: String,
}
#[derive(Clone, Copy)]
struct T {
f: u32,
}
struct U {
f: u32,
}
impl Clone for U {
fn clone(&self) -> Self {
// Do not lint: `Self` does not implement `Copy`
Self { ..*self }
}
}
#[derive(Copy)]
struct V {
f: u32,
}
impl Clone for V {
fn clone(&self) -> Self {
// Lint: `Self` implements `Copy`
*self
}
}
fn main() {
// Should lint: `a` would be consumed anyway
let a = S { f: String::from("foo") };
let mut b = a;
// Should lint: `b` would be consumed, and is mutable
let c = &mut b;
// Should not lint as `d` is not mutable
let d = S { f: String::from("foo") };
let e = &mut S { ..d };
// Should lint as `f` would be consumed anyway
let f = S { f: String::from("foo") };
let g = &f;
// Should lint: the result of an expression is mutable
let h = &mut *Box::new(S { f: String::from("foo") });
// Should not lint: `m` would be both alive and borrowed
let m = T { f: 17 };
let n = &T { ..m };
// Should not lint: `m` should not be modified
let o = &mut T { ..m };
o.f = 32;
assert_eq!(m.f, 17);
// Should not lint: `m` should not be modified
let o = &mut T { ..m } as *mut T;
unsafe { &mut *o }.f = 32;
assert_eq!(m.f, 17);
// Should lint: the result of an expression is mutable and temporary
let p = &mut *Box::new(T { f: 5 });
}

77
tests/ui/unnecessary_struct_initialization.rs Normal file
View File

@ -0,0 +1,77 @@
// run-rustfix
#![allow(unused)]
#![warn(clippy::unnecessary_struct_initialization)]
struct S {
f: String,
}
#[derive(Clone, Copy)]
struct T {
f: u32,
}
struct U {
f: u32,
}
impl Clone for U {
fn clone(&self) -> Self {
// Do not lint: `Self` does not implement `Copy`
Self { ..*self }
}
}
#[derive(Copy)]
struct V {
f: u32,
}
impl Clone for V {
fn clone(&self) -> Self {
// Lint: `Self` implements `Copy`
Self { ..*self }
}
}
fn main() {
// Should lint: `a` would be consumed anyway
let a = S { f: String::from("foo") };
let mut b = S { ..a };
// Should lint: `b` would be consumed, and is mutable
let c = &mut S { ..b };
// Should not lint as `d` is not mutable
let d = S { f: String::from("foo") };
let e = &mut S { ..d };
// Should lint as `f` would be consumed anyway
let f = S { f: String::from("foo") };
let g = &S { ..f };
// Should lint: the result of an expression is mutable
let h = &mut S {
..*Box::new(S { f: String::from("foo") })
};
// Should not lint: `m` would be both alive and borrowed
let m = T { f: 17 };
let n = &T { ..m };
// Should not lint: `m` should not be modified
let o = &mut T { ..m };
o.f = 32;
assert_eq!(m.f, 17);
// Should not lint: `m` should not be modified
let o = &mut T { ..m } as *mut T;
unsafe { &mut *o }.f = 32;
assert_eq!(m.f, 17);
// Should lint: the result of an expression is mutable and temporary
let p = &mut T {
..*Box::new(T { f: 5 })
};
}

46
tests/ui/unnecessary_struct_initialization.stderr Normal file
View File

@ -0,0 +1,46 @@
error: unnecessary struct building
--> $DIR/unnecessary_struct_initialization.rs:34:9
|
LL | Self { ..*self }
| ^^^^^^^^^^^^^^^^ help: replace with: `*self`
|
= note: `-D clippy::unnecessary-struct-initialization` implied by `-D warnings`
error: unnecessary struct building
--> $DIR/unnecessary_struct_initialization.rs:41:17
|
LL | let mut b = S { ..a };
| ^^^^^^^^^ help: replace with: `a`
error: unnecessary struct building
--> $DIR/unnecessary_struct_initialization.rs:44:18
|
LL | let c = &mut S { ..b };
| ^^^^^^^^^ help: replace with: `b`
error: unnecessary struct building
--> $DIR/unnecessary_struct_initialization.rs:52:14
|
LL | let g = &S { ..f };
| ^^^^^^^^^ help: replace with: `f`
error: unnecessary struct building
--> $DIR/unnecessary_struct_initialization.rs:55:18
|
LL | let h = &mut S {
| __________________^
LL | | ..*Box::new(S { f: String::from("foo") })
LL | | };
| |_____^ help: replace with: `*Box::new(S { f: String::from("foo") })`
error: unnecessary struct building
--> $DIR/unnecessary_struct_initialization.rs:74:18
|
LL | let p = &mut T {
| __________________^
LL | | ..*Box::new(T { f: 5 })
LL | | };
| |_____^ help: replace with: `*Box::new(T { f: 5 })`
error: aborting due to 6 previous errors