Skip to content

Commit 56d5657

Browse files
borsbors
committed
Auto merge of rust-lang#9962 - mdgaziur:master, r=dswij
Fix rust-lang#9958 This PR fixes rust-lang#9958. In order to fix the issue, the lint will now peel reference operators and enclose the expression with parentheses when necessary. changelog: [`comparison_to_empty`]: Peel deref operators in suggestions when necessary
2 parents b43c9f7 + 7a0f0c0 commit 56d5657

File tree

4 files changed

+153
-21
lines changed

4 files changed

+153
-21
lines changed

‎clippy_lints/src/len_zero.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then};
22
use clippy_utils::source::snippet_with_applicability;
3-
use clippy_utils::{get_item_name, get_parent_as_impl, is_lint_allowed};
3+
use clippy_utils::{get_item_name, get_parent_as_impl, is_lint_allowed, peel_ref_operators};
44
use if_chain::if_chain;
55
use rustc_ast::ast::LitKind;
66
use rustc_errors::Applicability;
77
use rustc_hir::def_id::DefIdSet;
88
use rustc_hir::{
99
def_id::DefId, AssocItemKind, BinOpKind, Expr, ExprKind, FnRetTy, ImplItem, ImplItemKind, ImplicitSelfKind, Item,
10-
ItemKind, Mutability, Node, TraitItemRef, TyKind,
10+
ItemKind, Mutability, Node, TraitItemRef, TyKind, UnOp,
1111
};
1212
use rustc_lint::{LateContext, LateLintPass};
1313
use rustc_middle::ty::{self, AssocKind, FnSig, Ty};
@@ -16,6 +16,7 @@ use rustc_span::{
1616
source_map::{Span, Spanned, Symbol},
1717
symbol::sym,
1818
};
19+
use std::borrow::Cow;
1920

2021
declare_clippy_lint! {
2122
/// ### What it does
@@ -428,16 +429,23 @@ fn check_len(
428429
fn check_empty_expr(cx: &LateContext<'_>, span: Span, lit1: &Expr<'_>, lit2: &Expr<'_>, op: &str) {
429430
if (is_empty_array(lit2) || is_empty_string(lit2)) && has_is_empty(cx, lit1) {
430431
let mut applicability = Applicability::MachineApplicable;
432+
433+
let lit1 = peel_ref_operators(cx, lit1);
434+
let mut lit_str = snippet_with_applicability(cx, lit1.span, "_", &mut applicability);
435+
436+
// Wrap the expression in parentheses if it's a deref expression. Otherwise operator precedence will
437+
// cause the code to dereference boolean(won't compile).
438+
if let ExprKind::Unary(UnOp::Deref, _) = lit1.kind {
439+
lit_str = Cow::from(format!("({lit_str})"));
440+
}
441+
431442
span_lint_and_sugg(
432443
cx,
433444
COMPARISON_TO_EMPTY,
434445
span,
435446
"comparison to empty slice",
436447
&format!("using `{op}is_empty` is clearer and more explicit"),
437-
format!(
438-
"{op}{}.is_empty()",
439-
snippet_with_applicability(cx, lit1.span, "_", &mut applicability)
440-
),
448+
format!("{op}{lit_str}.is_empty()"),
441449
applicability,
442450
);
443451
}

‎tests/ui/len_zero.fixed

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
#![warn(clippy::len_zero)]
44
#![allow(dead_code, unused, clippy::len_without_is_empty)]
55

6+
extern crate core;
7+
use core::ops::Deref;
8+
69
pub struct One;
710
struct Wither;
811

@@ -56,6 +59,26 @@ impl WithIsEmpty for Wither {
5659
}
5760
}
5861

62+
struct DerefToDerefToString;
63+
64+
impl Deref for DerefToDerefToString {
65+
type Target = DerefToString;
66+
67+
fn deref(&self) -> &Self::Target {
68+
&DerefToString {}
69+
}
70+
}
71+
72+
struct DerefToString;
73+
74+
impl Deref for DerefToString {
75+
type Target = str;
76+
77+
fn deref(&self) -> &Self::Target {
78+
"Hello, world!"
79+
}
80+
}
81+
5982
fn main() {
6083
let x = [1, 2];
6184
if x.is_empty() {
@@ -64,6 +87,23 @@ fn main() {
6487

6588
if "".is_empty() {}
6689

90+
let s = "Hello, world!";
91+
let s1 = &s;
92+
let s2 = &s1;
93+
let s3 = &s2;
94+
let s4 = &s3;
95+
let s5 = &s4;
96+
let s6 = &s5;
97+
println!("{}", s1.is_empty());
98+
println!("{}", s2.is_empty());
99+
println!("{}", s3.is_empty());
100+
println!("{}", s4.is_empty());
101+
println!("{}", s5.is_empty());
102+
println!("{}", (s6).is_empty());
103+
104+
let d2s = DerefToDerefToString {};
105+
println!("{}", (**d2s).is_empty());
106+
67107
let y = One;
68108
if y.len() == 0 {
69109
// No error; `One` does not have `.is_empty()`.

‎tests/ui/len_zero.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
#![warn(clippy::len_zero)]
44
#![allow(dead_code, unused, clippy::len_without_is_empty)]
55

6+
extern crate core;
7+
use core::ops::Deref;
8+
69
pub struct One;
710
struct Wither;
811

@@ -56,6 +59,26 @@ impl WithIsEmpty for Wither {
5659
}
5760
}
5861

62+
struct DerefToDerefToString;
63+
64+
impl Deref for DerefToDerefToString {
65+
type Target = DerefToString;
66+
67+
fn deref(&self) -> &Self::Target {
68+
&DerefToString {}
69+
}
70+
}
71+
72+
struct DerefToString;
73+
74+
impl Deref for DerefToString {
75+
type Target = str;
76+
77+
fn deref(&self) -> &Self::Target {
78+
"Hello, world!"
79+
}
80+
}
81+
5982
fn main() {
6083
let x = [1, 2];
6184
if x.len() == 0 {
@@ -64,6 +87,23 @@ fn main() {
6487

6588
if "".len() == 0 {}
6689

90+
let s = "Hello, world!";
91+
let s1 = &s;
92+
let s2 = &s1;
93+
let s3 = &s2;
94+
let s4 = &s3;
95+
let s5 = &s4;
96+
let s6 = &s5;
97+
println!("{}", *s1 == "");
98+
println!("{}", **s2 == "");
99+
println!("{}", ***s3 == "");
100+
println!("{}", ****s4 == "");
101+
println!("{}", *****s5 == "");
102+
println!("{}", ******(s6) == "");
103+
104+
let d2s = DerefToDerefToString {};
105+
println!("{}", &**d2s == "");
106+
67107
let y = One;
68108
if y.len() == 0 {
69109
// No error; `One` does not have `.is_empty()`.

‎tests/ui/len_zero.stderr

Lines changed: 59 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,88 +1,132 @@
11
error: length comparison to zero
2-
--> $DIR/len_zero.rs:61:8
2+
--> $DIR/len_zero.rs:84:8
33
|
44
LL | if x.len() == 0 {
55
| ^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `x.is_empty()`
66
|
77
= note: `-D clippy::len-zero` implied by `-D warnings`
88

99
error: length comparison to zero
10-
--> $DIR/len_zero.rs:65:8
10+
--> $DIR/len_zero.rs:88:8
1111
|
1212
LL | if "".len() == 0 {}
1313
| ^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `"".is_empty()`
1414

15+
error: comparison to empty slice
16+
--> $DIR/len_zero.rs:97:20
17+
|
18+
LL | println!("{}", *s1 == "");
19+
| ^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `s1.is_empty()`
20+
|
21+
= note: `-D clippy::comparison-to-empty` implied by `-D warnings`
22+
23+
error: comparison to empty slice
24+
--> $DIR/len_zero.rs:98:20
25+
|
26+
LL | println!("{}", **s2 == "");
27+
| ^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `s2.is_empty()`
28+
29+
error: comparison to empty slice
30+
--> $DIR/len_zero.rs:99:20
31+
|
32+
LL | println!("{}", ***s3 == "");
33+
| ^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `s3.is_empty()`
34+
35+
error: comparison to empty slice
36+
--> $DIR/len_zero.rs:100:20
37+
|
38+
LL | println!("{}", ****s4 == "");
39+
| ^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `s4.is_empty()`
40+
41+
error: comparison to empty slice
42+
--> $DIR/len_zero.rs:101:20
43+
|
44+
LL | println!("{}", *****s5 == "");
45+
| ^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `s5.is_empty()`
46+
47+
error: comparison to empty slice
48+
--> $DIR/len_zero.rs:102:20
49+
|
50+
LL | println!("{}", ******(s6) == "");
51+
| ^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `(s6).is_empty()`
52+
53+
error: comparison to empty slice
54+
--> $DIR/len_zero.rs:105:20
55+
|
56+
LL | println!("{}", &**d2s == "");
57+
| ^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `(**d2s).is_empty()`
58+
1559
error: length comparison to zero
16-
--> $DIR/len_zero.rs:80:8
60+
--> $DIR/len_zero.rs:120:8
1761
|
1862
LL | if has_is_empty.len() == 0 {
1963
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
2064

2165
error: length comparison to zero
22-
--> $DIR/len_zero.rs:83:8
66+
--> $DIR/len_zero.rs:123:8
2367
|
2468
LL | if has_is_empty.len() != 0 {
2569
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
2670

2771
error: length comparison to zero
28-
--> $DIR/len_zero.rs:86:8
72+
--> $DIR/len_zero.rs:126:8
2973
|
3074
LL | if has_is_empty.len() > 0 {
3175
| ^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
3276

3377
error: length comparison to one
34-
--> $DIR/len_zero.rs:89:8
78+
--> $DIR/len_zero.rs:129:8
3579
|
3680
LL | if has_is_empty.len() < 1 {
3781
| ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
3882

3983
error: length comparison to one
40-
--> $DIR/len_zero.rs:92:8
84+
--> $DIR/len_zero.rs:132:8
4185
|
4286
LL | if has_is_empty.len() >= 1 {
4387
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
4488

4589
error: length comparison to zero
46-
--> $DIR/len_zero.rs:103:8
90+
--> $DIR/len_zero.rs:143:8
4791
|
4892
LL | if 0 == has_is_empty.len() {
4993
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
5094

5195
error: length comparison to zero
52-
--> $DIR/len_zero.rs:106:8
96+
--> $DIR/len_zero.rs:146:8
5397
|
5498
LL | if 0 != has_is_empty.len() {
5599
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
56100

57101
error: length comparison to zero
58-
--> $DIR/len_zero.rs:109:8
102+
--> $DIR/len_zero.rs:149:8
59103
|
60104
LL | if 0 < has_is_empty.len() {
61105
| ^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
62106

63107
error: length comparison to one
64-
--> $DIR/len_zero.rs:112:8
108+
--> $DIR/len_zero.rs:152:8
65109
|
66110
LL | if 1 <= has_is_empty.len() {
67111
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
68112

69113
error: length comparison to one
70-
--> $DIR/len_zero.rs:115:8
114+
--> $DIR/len_zero.rs:155:8
71115
|
72116
LL | if 1 > has_is_empty.len() {
73117
| ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
74118

75119
error: length comparison to zero
76-
--> $DIR/len_zero.rs:129:8
120+
--> $DIR/len_zero.rs:169:8
77121
|
78122
LL | if with_is_empty.len() == 0 {
79123
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `with_is_empty.is_empty()`
80124

81125
error: length comparison to zero
82-
--> $DIR/len_zero.rs:142:8
126+
--> $DIR/len_zero.rs:182:8
83127
|
84128
LL | if b.len() != 0 {}
85129
| ^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!b.is_empty()`
86130

87-
error: aborting due to 14 previous errors
131+
error: aborting due to 21 previous errors
88132

0 commit comments

Comments
 (0)