Skip to content

Commit a6be4c5

Browse files
committed
reduce applicability when finding exprs with uncertain types
this makes the test case `unfixable`, so move it to a separate file
1 parent 81e72cc commit a6be4c5

File tree

6 files changed

+51
-36
lines changed

6 files changed

+51
-36
lines changed

clippy_lints/src/no_effect.rs

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -272,11 +272,12 @@ fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
272272
}
273273

274274
fn check_unnecessary_operation(cx: &LateContext<'_>, stmt: &Stmt<'_>) {
275+
let mut applicability = Applicability::MachineApplicable;
275276
if let StmtKind::Semi(expr) = stmt.kind
276277
&& !stmt.span.in_external_macro(cx.sess().source_map())
277278
&& let ctxt = stmt.span.ctxt()
278279
&& expr.span.ctxt() == ctxt
279-
&& let Some(reduced) = reduce_expression(cx, expr)
280+
&& let Some(reduced) = reduce_expression(cx, expr, &mut applicability)
280281
&& reduced.iter().all(|e| e.span.ctxt() == ctxt)
281282
{
282283
if let ExprKind::Index(..) = &expr.kind {
@@ -318,19 +319,18 @@ fn check_unnecessary_operation(cx: &LateContext<'_>, stmt: &Stmt<'_>) {
318319
stmt.span,
319320
"unnecessary operation",
320321
|diag| {
321-
diag.span_suggestion(
322-
stmt.span,
323-
"statement can be reduced to",
324-
snippet,
325-
Applicability::MachineApplicable,
326-
);
322+
diag.span_suggestion(stmt.span, "statement can be reduced to", snippet, applicability);
327323
},
328324
);
329325
}
330326
}
331327
}
332328

333-
fn reduce_expression<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<Vec<&'a Expr<'a>>> {
329+
fn reduce_expression<'a>(
330+
cx: &LateContext<'_>,
331+
expr: &'a Expr<'a>,
332+
applicability: &mut Applicability,
333+
) -> Option<Vec<&'a Expr<'a>>> {
334334
if expr.span.from_expansion() {
335335
return None;
336336
}
@@ -344,9 +344,9 @@ fn reduce_expression<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<Vec
344344
| ExprKind::Type(inner, _)
345345
| ExprKind::Unary(_, inner)
346346
| ExprKind::Field(inner, _)
347-
| ExprKind::AddrOf(_, _, inner) => reduce_expression(cx, inner).or_else(|| Some(vec![inner])),
347+
| ExprKind::AddrOf(_, _, inner) => reduce_expression(cx, inner, applicability).or_else(|| Some(vec![inner])),
348348
ExprKind::Cast(inner, _) if expr_type_is_certain(cx, inner) => {
349-
reduce_expression(cx, inner).or_else(|| Some(vec![inner]))
349+
reduce_expression(cx, inner, applicability).or_else(|| Some(vec![inner]))
350350
},
351351
// In the normal `Struct` case, we bail out if any of the fields has an uncertain type.
352352
// But for two-sided ranges, we know that if the type of one of the sides is certain, then so is the other
@@ -366,9 +366,12 @@ fn reduce_expression<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<Vec
366366
}
367367
},
368368
ExprKind::Struct(_, fields, ref base) => {
369-
if fields.iter().any(|f| !expr_type_is_certain(cx, &f.expr))
370-
|| has_drop(cx, cx.typeck_results().expr_ty(expr))
371-
{
369+
if fields.iter().any(|f| !expr_type_is_certain(cx, f.expr)) {
370+
// there's a risk that if we take the field exprs out of the context of the struct constructor,
371+
// their types might become ambiguous
372+
*applicability = Applicability::MaybeIncorrect;
373+
}
374+
if has_drop(cx, cx.typeck_results().expr_ty(expr)) {
372375
None
373376
} else {
374377
let base = match base {
@@ -404,7 +407,7 @@ fn reduce_expression<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<Vec
404407
BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) => None,
405408
BlockCheckMode::DefaultBlock => Some(vec![&**e]),
406409
// in case of compiler-inserted signaling blocks
407-
BlockCheckMode::UnsafeBlock(_) => reduce_expression(cx, e),
410+
BlockCheckMode::UnsafeBlock(_) => reduce_expression(cx, e, applicability),
408411
}
409412
})
410413
} else {

tests/ui/unnecessary_operation.fixed

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -157,15 +157,3 @@ fn issue15173_original<MsU>(handler: impl FnOnce() -> MsU + Clone + 'static) {
157157
None
158158
}) as Box<dyn Fn(i32) -> Option<i32>>;
159159
}
160-
161-
fn issue15381() {
162-
struct Resources;
163-
struct DescriptorSet {
164-
slots: Vec<Resources>,
165-
}
166-
167-
fn main() {
168-
Vec::new();
169-
//~^ unnecessary_operation
170-
}
171-
}

tests/ui/unnecessary_operation.rs

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -163,12 +163,3 @@ fn issue15173_original<MsU>(handler: impl FnOnce() -> MsU + Clone + 'static) {
163163
None
164164
}) as Box<dyn Fn(i32) -> Option<i32>>;
165165
}
166-
167-
// don't lint if any of the fields has an ambiguous type when used by themselves
168-
fn issue15381() {
169-
struct DescriptorSet {
170-
slots: Vec<u32>,
171-
}
172-
173-
DescriptorSet { slots: Vec::new() };
174-
}

tests/ui/unnecessary_operation.stderr

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,20 @@ LL | | get_number()
112112
LL | | };
113113
| |______^ help: statement can be reduced to: `get_number();`
114114

115+
error: unnecessary operation
116+
--> tests/ui/unnecessary_operation.rs:109:5
117+
|
118+
LL | / FooString {
119+
LL | |
120+
LL | | s: String::from("blah"),
121+
LL | | };
122+
| |______^ help: statement can be reduced to: `String::from("blah");`
123+
115124
error: unnecessary operation
116125
--> tests/ui/unnecessary_operation.rs:150:5
117126
|
118127
LL | [42, 55][get_usize()];
119128
| ^^^^^^^^^^^^^^^^^^^^^^ help: statement can be written as: `assert!([42, 55].len() > get_usize());`
120129

121-
error: aborting due to 19 previous errors
130+
error: aborting due to 20 previous errors
122131

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
//@no-rustfix
2+
#![expect(unused)]
3+
#![warn(clippy::unnecessary_operation)]
4+
5+
// don't lint if any of the fields has an ambiguous type when used by themselves
6+
fn issue15381() {
7+
struct DescriptorSet {
8+
slots: Vec<u32>,
9+
}
10+
11+
DescriptorSet { slots: Vec::new() };
12+
//~^ unnecessary_operation
13+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
error: unnecessary operation
2+
--> tests/ui/unnecessary_operation_unfixable.rs:11:5
3+
|
4+
LL | DescriptorSet { slots: Vec::new() };
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `Vec::new();`
6+
|
7+
= note: `-D clippy::unnecessary-operation` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_operation)]`
9+
10+
error: aborting due to 1 previous error
11+

0 commit comments

Comments
 (0)