From 6d40b105ed69598205e4877db188a62d0817aa94 Mon Sep 17 00:00:00 2001 From: cocodery Date: Sun, 3 Dec 2023 00:58:47 +0800 Subject: Add a function to check whether binary oprands are nontrivial It's hard to check whether oprator is overrided through context of lint So assume nontrivial has overrided binary operator --- clippy_lints/src/no_effect.rs | 59 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index 6e65dd628a4..66c013b4f35 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -87,6 +87,10 @@ impl<'tcx> LateLintPass<'tcx> for NoEffect { fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { if let StmtKind::Semi(expr) = stmt.kind { + // assume nontrivial oprand of `Binary` Expr can skip `check_unnecessary_operation` + if has_nontrivial_oprand(expr) { + return true; + } if has_no_effect(cx, expr) { span_lint_hir_and_then( cx, @@ -153,6 +157,61 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { false } +fn has_nontrivial_oprand(expr: &Expr<'_>) -> bool { + if expr.span.from_expansion() { + return false; + } + return match peel_blocks(expr).kind { + ExprKind::Binary(_, lhs, rhs) => !check_nontrivial_operand(lhs, rhs), + _ => false, + }; +} + +fn check_nontrivial_operand(lhs: &Expr<'_>, rhs: &Expr<'_>) -> bool { + // It's seem that impossable to check whether operator is overrided through context of this lint, + // so, this function assume user-defined binary operator is overrided with an side-effect. + // The definition of user-defined structure here is `tuple`, `array`, `struct`, + // it looks like a little bit simple, but useful. + // Althrough this will weaken the ability of this lint, + // less miss lint-fix happen. + + // a closure to check whether expr belongs to user-defined structure + let closure = |expr: &Expr<'_>| -> bool { + match &expr.kind { + // check whether expr is a user-defined sturcture + ExprKind::Tup(..) | ExprKind::Array(..) | ExprKind::Struct(..) => true, + // resolve expr's path + ExprKind::Path(rustc_hir::QPath::Resolved( + _, + rustc_hir::Path { + span: _, + res, + segments: _, + }, + )) => { + match res { + Res::Def(defkind, _) => match defkind { + // user-defined + DefKind::Struct | DefKind::Ctor(_, _) => true, + _ => false, + }, + _ => false, + }; + false + }, + _ => false, + } + }; + + let lhs_ud = closure(lhs); + let rhs_ud = closure(rhs); + // one of lhs or rhs is user-defined structure + if lhs_ud || rhs_ud { + return false; + } + true +} + fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { if expr.span.from_expansion() { return false; -- cgit 1.4.1-3-g733a5 From 89774234be1ce368ea2b1c6c127fe254331ee47e Mon Sep 17 00:00:00 2001 From: cocodery Date: Mon, 4 Dec 2023 15:57:27 +0800 Subject: Rewrite logic of `has_nontrivial_oprand`. Check whether operator is overrided with a `struct` operand. The struct here refers to `struct`, `enum`, `union`. Add and fix test for `no_effect` lint. --- clippy_lints/src/no_effect.rs | 90 ++++++++++++++++--------------------------- tests/ui/no_effect.rs | 23 +++++++++++ tests/ui/no_effect.stderr | 58 ++++++++++++++-------------- 3 files changed, 86 insertions(+), 85 deletions(-) diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index 66c013b4f35..e3930b0568d 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -10,6 +10,7 @@ use rustc_hir::{ use rustc_infer::infer::TyCtxtInferExt as _; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; +use rustc_middle::ty; use rustc_session::declare_lint_pass; use std::ops::Deref; @@ -87,8 +88,13 @@ impl<'tcx> LateLintPass<'tcx> for NoEffect { fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { if let StmtKind::Semi(expr) = stmt.kind { + // move `expr.span.from_expansion()` ahead + if expr.span.from_expansion() { + return false; + } + let expr = peel_blocks(expr); // assume nontrivial oprand of `Binary` Expr can skip `check_unnecessary_operation` - if has_nontrivial_oprand(expr) { + if has_nontrivial_oprand(cx, expr) { return true; } if has_no_effect(cx, expr) { @@ -157,66 +163,38 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { false } -fn has_nontrivial_oprand(expr: &Expr<'_>) -> bool { - if expr.span.from_expansion() { - return false; - } - return match peel_blocks(expr).kind { - ExprKind::Binary(_, lhs, rhs) => !check_nontrivial_operand(lhs, rhs), - _ => false, - }; -} - -fn check_nontrivial_operand(lhs: &Expr<'_>, rhs: &Expr<'_>) -> bool { - // It's seem that impossable to check whether operator is overrided through context of this lint, - // so, this function assume user-defined binary operator is overrided with an side-effect. - // The definition of user-defined structure here is `tuple`, `array`, `struct`, - // it looks like a little bit simple, but useful. - // Althrough this will weaken the ability of this lint, - // less miss lint-fix happen. - - // a closure to check whether expr belongs to user-defined structure - let closure = |expr: &Expr<'_>| -> bool { - match &expr.kind { - // check whether expr is a user-defined sturcture - ExprKind::Tup(..) | ExprKind::Array(..) | ExprKind::Struct(..) => true, - // resolve expr's path - ExprKind::Path(rustc_hir::QPath::Resolved( - _, - rustc_hir::Path { - span: _, - res, - segments: _, - }, - )) => { - match res { - Res::Def(defkind, _) => match defkind { - // user-defined - DefKind::Struct | DefKind::Ctor(_, _) => true, - _ => false, - }, - _ => false, - }; - false - }, - _ => false, - } - }; +fn has_nontrivial_oprand(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + // It's very hard or impossable to check whether overrided operator have side-effect this lint. + // So, this function assume user-defined binary operator is overrided with an side-effect. + // The definition of user-defined structure here is `struct`, `enum`, `uniom`, + // Althrough this will weaken the ability of this lint, less error lint-fix happen. + match expr.kind { + ExprKind::Binary(_, lhs, rhs) => { + // get type of lhs and rhs + let tyck_result = cx.typeck_results(); + let ty_lhs = tyck_result.expr_ty(lhs).kind(); + let ty_rhs = tyck_result.expr_ty(rhs).kind(); + // check whether lhs is a user-defined structure + // only need to check lhs in fact + let ud_lhs = match ty_lhs { + ty::Adt(adt_def, _) => adt_def.is_struct() || adt_def.is_enum() || adt_def.is_union(), + _ => false, + }; + let ud_rhs = match ty_rhs { + ty::Adt(adt_def, _) => adt_def.is_struct() || adt_def.is_enum() || adt_def.is_union(), + _ => false, + }; - let lhs_ud = closure(lhs); - let rhs_ud = closure(rhs); - // one of lhs or rhs is user-defined structure - if lhs_ud || rhs_ud { - return false; + // reference: rust/compiler/rustc_middle/src/ty/typeck_results.rs: `is_method_call`. + // use this function to check whether operator is overrided in `ExprKind::Binary`. + (ud_lhs || ud_rhs) && tyck_result.is_method_call(expr) + }, + _ => false, } - true } fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - if expr.span.from_expansion() { - return false; - } - match peel_blocks(expr).kind { + match expr.kind { ExprKind::Lit(..) | ExprKind::Closure { .. } => true, ExprKind::Path(..) => !has_drop(cx, cx.typeck_results().expr_ty(expr)), ExprKind::Index(a, b, _) | ExprKind::Binary(_, a, b) => has_no_effect(cx, a) && has_no_effect(cx, b), diff --git a/tests/ui/no_effect.rs b/tests/ui/no_effect.rs index c52f4389192..7ffdeef6582 100644 --- a/tests/ui/no_effect.rs +++ b/tests/ui/no_effect.rs @@ -9,6 +9,22 @@ clippy::useless_vec )] +use std::fmt::Display; +use std::ops::Shl; + +struct Cout; + +impl Shl for Cout +where + T: Display, +{ + type Output = Self; + fn shl(self, rhs: T) -> Self::Output { + println!("{}", rhs); + self + } +} + struct Unit; struct Tuple(i32); struct Struct { @@ -174,4 +190,11 @@ fn main() { GreetStruct1("world"); GreetStruct2()("world"); GreetStruct3 {}("world"); + + fn n() -> i32 { + 42 + } + + Cout << 142; + Cout << n(); } diff --git a/tests/ui/no_effect.stderr b/tests/ui/no_effect.stderr index feba35697f5..437e556a7a8 100644 --- a/tests/ui/no_effect.stderr +++ b/tests/ui/no_effect.stderr @@ -1,5 +1,5 @@ error: statement with no effect - --> $DIR/no_effect.rs:98:5 + --> $DIR/no_effect.rs:114:5 | LL | 0; | ^^ @@ -8,151 +8,151 @@ LL | 0; = help: to override `-D warnings` add `#[allow(clippy::no_effect)]` error: statement with no effect - --> $DIR/no_effect.rs:101:5 + --> $DIR/no_effect.rs:117:5 | LL | s2; | ^^^ error: statement with no effect - --> $DIR/no_effect.rs:103:5 + --> $DIR/no_effect.rs:119:5 | LL | Unit; | ^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:105:5 + --> $DIR/no_effect.rs:121:5 | LL | Tuple(0); | ^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:107:5 + --> $DIR/no_effect.rs:123:5 | LL | Struct { field: 0 }; | ^^^^^^^^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:109:5 + --> $DIR/no_effect.rs:125:5 | LL | Struct { ..s }; | ^^^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:111:5 + --> $DIR/no_effect.rs:127:5 | LL | Union { a: 0 }; | ^^^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:113:5 + --> $DIR/no_effect.rs:129:5 | LL | Enum::Tuple(0); | ^^^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:115:5 + --> $DIR/no_effect.rs:131:5 | LL | Enum::Struct { field: 0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:117:5 + --> $DIR/no_effect.rs:133:5 | LL | 5 + 6; | ^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:119:5 + --> $DIR/no_effect.rs:135:5 | LL | *&42; | ^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:121:5 + --> $DIR/no_effect.rs:137:5 | LL | &6; | ^^^ error: statement with no effect - --> $DIR/no_effect.rs:123:5 + --> $DIR/no_effect.rs:139:5 | LL | (5, 6, 7); | ^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:125:5 + --> $DIR/no_effect.rs:141:5 | LL | ..; | ^^^ error: statement with no effect - --> $DIR/no_effect.rs:127:5 + --> $DIR/no_effect.rs:143:5 | LL | 5..; | ^^^^ error: statement with no effect - --> $DIR/no_effect.rs:129:5 + --> $DIR/no_effect.rs:145:5 | LL | ..5; | ^^^^ error: statement with no effect - --> $DIR/no_effect.rs:131:5 + --> $DIR/no_effect.rs:147:5 | LL | 5..6; | ^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:133:5 + --> $DIR/no_effect.rs:149:5 | LL | 5..=6; | ^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:135:5 + --> $DIR/no_effect.rs:151:5 | LL | [42, 55]; | ^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:137:5 + --> $DIR/no_effect.rs:153:5 | LL | [42, 55][1]; | ^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:139:5 + --> $DIR/no_effect.rs:155:5 | LL | (42, 55).1; | ^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:141:5 + --> $DIR/no_effect.rs:157:5 | LL | [42; 55]; | ^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:143:5 + --> $DIR/no_effect.rs:159:5 | LL | [42; 55][13]; | ^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:146:5 + --> $DIR/no_effect.rs:162:5 | LL | || x += 5; | ^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:149:5 + --> $DIR/no_effect.rs:165:5 | LL | FooString { s: s }; | ^^^^^^^^^^^^^^^^^^^ error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:151:5 + --> $DIR/no_effect.rs:167:5 | LL | let _unused = 1; | ^^^^^^^^^^^^^^^^ @@ -161,19 +161,19 @@ LL | let _unused = 1; = help: to override `-D warnings` add `#[allow(clippy::no_effect_underscore_binding)]` error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:154:5 + --> $DIR/no_effect.rs:170:5 | LL | let _penguin = || println!("Some helpful closure"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:156:5 + --> $DIR/no_effect.rs:172:5 | LL | let _duck = Struct { field: 0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:158:5 + --> $DIR/no_effect.rs:174:5 | LL | let _cat = [2, 4, 6, 8][2]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ -- cgit 1.4.1-3-g733a5 From 2e3c031528c26e01937e9e9434947d1289561051 Mon Sep 17 00:00:00 2001 From: cocodery Date: Tue, 5 Dec 2023 09:29:20 +0800 Subject: Rename `has_nontrivial_oprand` to `is_operator_overrided` Simpfy code of `is_operator_overrided`, directly use `is_method_call` to check if operator is overrided, at least one oprand of binary-expr must be ADT-type So no need to check type of lhs and rhs --- clippy_lints/src/no_effect.rs | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index e3930b0568d..1d92b928880 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -10,7 +10,6 @@ use rustc_hir::{ use rustc_infer::infer::TyCtxtInferExt as _; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; -use rustc_middle::ty; use rustc_session::declare_lint_pass; use std::ops::Deref; @@ -94,7 +93,7 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { } let expr = peel_blocks(expr); // assume nontrivial oprand of `Binary` Expr can skip `check_unnecessary_operation` - if has_nontrivial_oprand(cx, expr) { + if is_operator_overrided(cx, expr) { return true; } if has_no_effect(cx, expr) { @@ -163,31 +162,19 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { false } -fn has_nontrivial_oprand(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { +fn is_operator_overrided(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { // It's very hard or impossable to check whether overrided operator have side-effect this lint. // So, this function assume user-defined binary operator is overrided with an side-effect. - // The definition of user-defined structure here is `struct`, `enum`, `uniom`, + // The definition of user-defined structure here is ADT-type, // Althrough this will weaken the ability of this lint, less error lint-fix happen. match expr.kind { - ExprKind::Binary(_, lhs, rhs) => { - // get type of lhs and rhs - let tyck_result = cx.typeck_results(); - let ty_lhs = tyck_result.expr_ty(lhs).kind(); - let ty_rhs = tyck_result.expr_ty(rhs).kind(); - // check whether lhs is a user-defined structure - // only need to check lhs in fact - let ud_lhs = match ty_lhs { - ty::Adt(adt_def, _) => adt_def.is_struct() || adt_def.is_enum() || adt_def.is_union(), - _ => false, - }; - let ud_rhs = match ty_rhs { - ty::Adt(adt_def, _) => adt_def.is_struct() || adt_def.is_enum() || adt_def.is_union(), - _ => false, - }; + ExprKind::Binary(..) => { + // No need to check type of `lhs` and `rhs` + // because if the operator is overrided, at least one operand is ADT type // reference: rust/compiler/rustc_middle/src/ty/typeck_results.rs: `is_method_call`. // use this function to check whether operator is overrided in `ExprKind::Binary`. - (ud_lhs || ud_rhs) && tyck_result.is_method_call(expr) + cx.typeck_results().is_method_call(expr) }, _ => false, } -- cgit 1.4.1-3-g733a5 From ee2354badf390afc95b5fa180cefd0100c9cedbd Mon Sep 17 00:00:00 2001 From: cocodery Date: Wed, 6 Dec 2023 12:17:48 +0800 Subject: Add check for unary-operator Fix typo and add test for unary-opeator --- clippy_lints/src/no_effect.rs | 10 ++++---- tests/ui/no_effect.rs | 12 +++++++-- tests/ui/no_effect.stderr | 58 +++++++++++++++++++++---------------------- 3 files changed, 44 insertions(+), 36 deletions(-) diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index 1d92b928880..a0e7dbf9ec6 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -93,7 +93,7 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { } let expr = peel_blocks(expr); // assume nontrivial oprand of `Binary` Expr can skip `check_unnecessary_operation` - if is_operator_overrided(cx, expr) { + if is_operator_overriden(cx, expr) { return true; } if has_no_effect(cx, expr) { @@ -162,18 +162,18 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { false } -fn is_operator_overrided(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { +fn is_operator_overriden(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { // It's very hard or impossable to check whether overrided operator have side-effect this lint. - // So, this function assume user-defined binary operator is overrided with an side-effect. + // So, this function assume user-defined operator is overrided with an side-effect. // The definition of user-defined structure here is ADT-type, // Althrough this will weaken the ability of this lint, less error lint-fix happen. match expr.kind { - ExprKind::Binary(..) => { + ExprKind::Binary(..) | ExprKind::Unary(..) => { // No need to check type of `lhs` and `rhs` // because if the operator is overrided, at least one operand is ADT type // reference: rust/compiler/rustc_middle/src/ty/typeck_results.rs: `is_method_call`. - // use this function to check whether operator is overrided in `ExprKind::Binary`. + // use this function to check whether operator is overrided in `ExprKind::{Binary, Unary}`. cx.typeck_results().is_method_call(expr) }, _ => false, diff --git a/tests/ui/no_effect.rs b/tests/ui/no_effect.rs index 7ffdeef6582..777b1e52c2d 100644 --- a/tests/ui/no_effect.rs +++ b/tests/ui/no_effect.rs @@ -10,7 +10,7 @@ )] use std::fmt::Display; -use std::ops::Shl; +use std::ops::{Neg, Shl}; struct Cout; @@ -25,6 +25,14 @@ where } } +impl Neg for Cout { + type Output = Self; + fn neg(self) -> Self::Output { + println!("hello world"); + self + } +} + struct Unit; struct Tuple(i32); struct Struct { @@ -196,5 +204,5 @@ fn main() { } Cout << 142; - Cout << n(); + -Cout; } diff --git a/tests/ui/no_effect.stderr b/tests/ui/no_effect.stderr index 437e556a7a8..f5ba234b4cb 100644 --- a/tests/ui/no_effect.stderr +++ b/tests/ui/no_effect.stderr @@ -1,5 +1,5 @@ error: statement with no effect - --> $DIR/no_effect.rs:114:5 + --> $DIR/no_effect.rs:122:5 | LL | 0; | ^^ @@ -8,151 +8,151 @@ LL | 0; = help: to override `-D warnings` add `#[allow(clippy::no_effect)]` error: statement with no effect - --> $DIR/no_effect.rs:117:5 + --> $DIR/no_effect.rs:125:5 | LL | s2; | ^^^ error: statement with no effect - --> $DIR/no_effect.rs:119:5 + --> $DIR/no_effect.rs:127:5 | LL | Unit; | ^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:121:5 + --> $DIR/no_effect.rs:129:5 | LL | Tuple(0); | ^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:123:5 + --> $DIR/no_effect.rs:131:5 | LL | Struct { field: 0 }; | ^^^^^^^^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:125:5 + --> $DIR/no_effect.rs:133:5 | LL | Struct { ..s }; | ^^^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:127:5 + --> $DIR/no_effect.rs:135:5 | LL | Union { a: 0 }; | ^^^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:129:5 + --> $DIR/no_effect.rs:137:5 | LL | Enum::Tuple(0); | ^^^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:131:5 + --> $DIR/no_effect.rs:139:5 | LL | Enum::Struct { field: 0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:133:5 + --> $DIR/no_effect.rs:141:5 | LL | 5 + 6; | ^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:135:5 + --> $DIR/no_effect.rs:143:5 | LL | *&42; | ^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:137:5 + --> $DIR/no_effect.rs:145:5 | LL | &6; | ^^^ error: statement with no effect - --> $DIR/no_effect.rs:139:5 + --> $DIR/no_effect.rs:147:5 | LL | (5, 6, 7); | ^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:141:5 + --> $DIR/no_effect.rs:149:5 | LL | ..; | ^^^ error: statement with no effect - --> $DIR/no_effect.rs:143:5 + --> $DIR/no_effect.rs:151:5 | LL | 5..; | ^^^^ error: statement with no effect - --> $DIR/no_effect.rs:145:5 + --> $DIR/no_effect.rs:153:5 | LL | ..5; | ^^^^ error: statement with no effect - --> $DIR/no_effect.rs:147:5 + --> $DIR/no_effect.rs:155:5 | LL | 5..6; | ^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:149:5 + --> $DIR/no_effect.rs:157:5 | LL | 5..=6; | ^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:151:5 + --> $DIR/no_effect.rs:159:5 | LL | [42, 55]; | ^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:153:5 + --> $DIR/no_effect.rs:161:5 | LL | [42, 55][1]; | ^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:155:5 + --> $DIR/no_effect.rs:163:5 | LL | (42, 55).1; | ^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:157:5 + --> $DIR/no_effect.rs:165:5 | LL | [42; 55]; | ^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:159:5 + --> $DIR/no_effect.rs:167:5 | LL | [42; 55][13]; | ^^^^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:162:5 + --> $DIR/no_effect.rs:170:5 | LL | || x += 5; | ^^^^^^^^^^ error: statement with no effect - --> $DIR/no_effect.rs:165:5 + --> $DIR/no_effect.rs:173:5 | LL | FooString { s: s }; | ^^^^^^^^^^^^^^^^^^^ error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:167:5 + --> $DIR/no_effect.rs:175:5 | LL | let _unused = 1; | ^^^^^^^^^^^^^^^^ @@ -161,19 +161,19 @@ LL | let _unused = 1; = help: to override `-D warnings` add `#[allow(clippy::no_effect_underscore_binding)]` error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:170:5 + --> $DIR/no_effect.rs:178:5 | LL | let _penguin = || println!("Some helpful closure"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:172:5 + --> $DIR/no_effect.rs:180:5 | LL | let _duck = Struct { field: 0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:174:5 + --> $DIR/no_effect.rs:182:5 | LL | let _cat = [2, 4, 6, 8][2]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ -- cgit 1.4.1-3-g733a5 From 56d20c2b53db2c2d39ee3e8368c5cd556c35f5b0 Mon Sep 17 00:00:00 2001 From: cocodery Date: Fri, 8 Dec 2023 21:33:28 +0800 Subject: Fix nits and add test for `unnecessary_operation` --- clippy_lints/src/no_effect.rs | 16 ++++++++------- tests/ui/unnecessary_operation.fixed | 19 ++++++++++++++++++ tests/ui/unnecessary_operation.rs | 19 ++++++++++++++++++ tests/ui/unnecessary_operation.stderr | 38 +++++++++++++++++------------------ 4 files changed, 66 insertions(+), 26 deletions(-) diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index a0e7dbf9ec6..5978da83199 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -92,8 +92,10 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { return false; } let expr = peel_blocks(expr); - // assume nontrivial oprand of `Binary` Expr can skip `check_unnecessary_operation` - if is_operator_overriden(cx, expr) { + + if is_operator_overridden(cx, expr) { + // Return `true`, to prevent `check_unnecessary_operation` from + // linting on this statement as well. return true; } if has_no_effect(cx, expr) { @@ -162,18 +164,18 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { false } -fn is_operator_overriden(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - // It's very hard or impossable to check whether overrided operator have side-effect this lint. - // So, this function assume user-defined operator is overrided with an side-effect. +fn is_operator_overridden(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + // It's very hard or impossable to check whether overridden operator have side-effect this lint. + // So, this function assume user-defined operator is overridden with an side-effect. // The definition of user-defined structure here is ADT-type, // Althrough this will weaken the ability of this lint, less error lint-fix happen. match expr.kind { ExprKind::Binary(..) | ExprKind::Unary(..) => { // No need to check type of `lhs` and `rhs` - // because if the operator is overrided, at least one operand is ADT type + // because if the operator is overridden, at least one operand is ADT type // reference: rust/compiler/rustc_middle/src/ty/typeck_results.rs: `is_method_call`. - // use this function to check whether operator is overrided in `ExprKind::{Binary, Unary}`. + // use this function to check whether operator is overridden in `ExprKind::{Binary, Unary}`. cx.typeck_results().is_method_call(expr) }, _ => false, diff --git a/tests/ui/unnecessary_operation.fixed b/tests/ui/unnecessary_operation.fixed index d0c0298ef4c..463412daec0 100644 --- a/tests/ui/unnecessary_operation.fixed +++ b/tests/ui/unnecessary_operation.fixed @@ -7,6 +7,9 @@ )] #![warn(clippy::unnecessary_operation)] +use std::fmt::Display; +use std::ops::Shl; + struct Tuple(i32); struct Struct { field: i32, @@ -50,6 +53,19 @@ fn get_drop_struct() -> DropStruct { DropStruct { field: 0 } } +struct Cout; + +impl Shl for Cout +where + T: Display, +{ + type Output = Self; + fn shl(self, rhs: T) -> Self::Output { + println!("{}", rhs); + self + } +} + fn main() { get_number(); get_number(); @@ -87,4 +103,7 @@ fn main() { ($($e:expr),*) => {{ $($e;)* }} } use_expr!(isize::MIN / -(one() as isize), i8::MIN / -one()); + + // Issue #11885 + Cout << 16; } diff --git a/tests/ui/unnecessary_operation.rs b/tests/ui/unnecessary_operation.rs index e8e3a2d5657..f0d28e28902 100644 --- a/tests/ui/unnecessary_operation.rs +++ b/tests/ui/unnecessary_operation.rs @@ -7,6 +7,9 @@ )] #![warn(clippy::unnecessary_operation)] +use std::fmt::Display; +use std::ops::Shl; + struct Tuple(i32); struct Struct { field: i32, @@ -50,6 +53,19 @@ fn get_drop_struct() -> DropStruct { DropStruct { field: 0 } } +struct Cout; + +impl Shl for Cout +where + T: Display, +{ + type Output = Self; + fn shl(self, rhs: T) -> Self::Output { + println!("{}", rhs); + self + } +} + fn main() { Tuple(get_number()); Struct { field: get_number() }; @@ -91,4 +107,7 @@ fn main() { ($($e:expr),*) => {{ $($e;)* }} } use_expr!(isize::MIN / -(one() as isize), i8::MIN / -one()); + + // Issue #11885 + Cout << 16; } diff --git a/tests/ui/unnecessary_operation.stderr b/tests/ui/unnecessary_operation.stderr index fbe495f518f..eeee9ad6006 100644 --- a/tests/ui/unnecessary_operation.stderr +++ b/tests/ui/unnecessary_operation.stderr @@ -1,5 +1,5 @@ error: unnecessary operation - --> $DIR/unnecessary_operation.rs:54:5 + --> $DIR/unnecessary_operation.rs:70:5 | LL | Tuple(get_number()); | ^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` @@ -8,103 +8,103 @@ LL | Tuple(get_number()); = help: to override `-D warnings` add `#[allow(clippy::unnecessary_operation)]` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:55:5 + --> $DIR/unnecessary_operation.rs:71:5 | LL | 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:72:5 | LL | Struct { ..get_struct() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_struct();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:57:5 + --> $DIR/unnecessary_operation.rs:73:5 | LL | Enum::Tuple(get_number()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:58:5 + --> $DIR/unnecessary_operation.rs:74:5 | LL | Enum::Struct { field: get_number() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:59:5 + --> $DIR/unnecessary_operation.rs:75:5 | LL | 5 + get_number(); | ^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `5;get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:60:5 + --> $DIR/unnecessary_operation.rs:76:5 | LL | *&get_number(); | ^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:61:5 + --> $DIR/unnecessary_operation.rs:77: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:78:5 | LL | (5, 6, get_number()); | ^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `5;6;get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:63:5 + --> $DIR/unnecessary_operation.rs:79:5 | LL | get_number()..; | ^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:64:5 + --> $DIR/unnecessary_operation.rs:80:5 | LL | ..get_number(); | ^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:65:5 + --> $DIR/unnecessary_operation.rs:81:5 | LL | 5..get_number(); | ^^^^^^^^^^^^^^^^ help: statement can be reduced to: `5;get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:66:5 + --> $DIR/unnecessary_operation.rs:82:5 | LL | [42, get_number()]; | ^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `42;get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:67:5 + --> $DIR/unnecessary_operation.rs:83: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:68:5 + --> $DIR/unnecessary_operation.rs:84:5 | LL | (42, get_number()).1; | ^^^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `42;get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:69:5 + --> $DIR/unnecessary_operation.rs:85:5 | LL | [get_number(); 55]; | ^^^^^^^^^^^^^^^^^^^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:70:5 + --> $DIR/unnecessary_operation.rs:86: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:71:5 + --> $DIR/unnecessary_operation.rs:87:5 | LL | / { LL | | get_number() @@ -112,7 +112,7 @@ LL | | }; | |______^ help: statement can be reduced to: `get_number();` error: unnecessary operation - --> $DIR/unnecessary_operation.rs:74:5 + --> $DIR/unnecessary_operation.rs:90:5 | LL | / FooString { LL | | s: String::from("blah"), -- cgit 1.4.1-3-g733a5