Skip to content

New lint: detect unnecessary struct building #10489

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
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,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ mod unit_types;
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;
Expand Down Expand Up @@ -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`
}

Expand Down
84 changes: 84 additions & 0 deletions clippy_lints/src/unnecessary_struct_initialization.rs
Original file line number Diff line number Diff line change
@@ -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 changes: 1 addition & 1 deletion tests/ui/needless_update.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
7 changes: 6 additions & 1 deletion tests/ui/no_effect.rs
Original file line number Diff line number Diff line change
@@ -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);
Expand Down
60 changes: 30 additions & 30 deletions tests/ui/no_effect.stderr
Original file line number Diff line number Diff line change
@@ -1,183 +1,183 @@
error: statement with no effect
--> $DIR/no_effect.rs:92:5
--> $DIR/no_effect.rs:97:5
|
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;
| ^^^^^^^^^^^^^^^^
|
= 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];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/unnecessary_operation.fixed
Original file line number Diff line number Diff line change
@@ -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);
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/unnecessary_operation.rs
Original file line number Diff line number Diff line change
@@ -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);
Expand Down
Loading