Skip to content

Commit 2110522

Browse files
HarrisonMc555HarrisonMc555
committed
Merge branch 'master' of github.com:rust-lang/rust-clippy
2 parents 7f868c9 + cafbe7f commit 2110522

10 files changed

+411
-206
lines changed

‎Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ rustc_tools_util = { version = "0.1.1", path = "rustc_tools_util"}
4646

4747
[dev-dependencies]
4848
cargo_metadata = "0.7.1"
49-
compiletest_rs = { version = "0.3.21", features = ["tmp"] }
49+
compiletest_rs = { version = "0.3.22", features = ["tmp"] }
5050
lazy_static = "1.0"
5151
serde_derive = "1.0"
5252
clippy-mini-macro-test = { version = "0.2", path = "mini-macro" }

‎clippy_lints/src/assertions_on_constants.rs

Lines changed: 30 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ use if_chain::if_chain;
22
use rustc::hir::{Expr, ExprKind};
33
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
44
use rustc::{declare_lint_pass, declare_tool_lint};
5+
use syntax_pos::Span;
56

67
use crate::consts::{constant, Constant};
7-
use crate::syntax::ast::LitKind;
88
use crate::utils::{in_macro, is_direct_expn_of, span_help_and_lint};
99

1010
declare_clippy_lint! {
@@ -33,41 +33,40 @@ declare_lint_pass!(AssertionsOnConstants => [ASSERTIONS_ON_CONSTANTS]);
3333

3434
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
3535
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
36+
let mut is_debug_assert = false;
37+
let debug_assert_not_in_macro = |span: Span| {
38+
is_debug_assert = true;
39+
// Check that `debug_assert!` itself is not inside a macro
40+
!in_macro(span)
41+
};
3642
if_chain! {
3743
if let Some(assert_span) = is_direct_expn_of(e.span, "assert");
3844
if !in_macro(assert_span)
39-
|| is_direct_expn_of(assert_span, "debug_assert").map_or(false, |span| !in_macro(span));
45+
|| is_direct_expn_of(assert_span, "debug_assert")
46+
.map_or(false, debug_assert_not_in_macro);
4047
if let ExprKind::Unary(_, ref lit) = e.node;
48+
if let Some(bool_const) = constant(cx, cx.tables, lit);
4149
then {
42-
if let ExprKind::Lit(ref inner) = lit.node {
43-
match inner.node {
44-
LitKind::Bool(true) => {
45-
span_help_and_lint(cx, ASSERTIONS_ON_CONSTANTS, e.span,
46-
"assert!(true) will be optimized out by the compiler",
47-
"remove it");
48-
},
49-
LitKind::Bool(false) => {
50-
span_help_and_lint(
51-
cx, ASSERTIONS_ON_CONSTANTS, e.span,
52-
"assert!(false) should probably be replaced",
53-
"use panic!() or unreachable!()");
54-
},
55-
_ => (),
56-
}
57-
} else if let Some(bool_const) = constant(cx, cx.tables, lit) {
58-
match bool_const.0 {
59-
Constant::Bool(true) => {
60-
span_help_and_lint(cx, ASSERTIONS_ON_CONSTANTS, e.span,
61-
"assert!(const: true) will be optimized out by the compiler",
62-
"remove it");
63-
},
64-
Constant::Bool(false) => {
65-
span_help_and_lint(cx, ASSERTIONS_ON_CONSTANTS, e.span,
66-
"assert!(const: false) should probably be replaced",
67-
"use panic!() or unreachable!()");
68-
},
69-
_ => (),
70-
}
50+
match bool_const.0 {
51+
Constant::Bool(true) => {
52+
span_help_and_lint(
53+
cx,
54+
ASSERTIONS_ON_CONSTANTS,
55+
e.span,
56+
"`assert!(true)` will be optimized out by the compiler",
57+
"remove it"
58+
);
59+
},
60+
Constant::Bool(false) if !is_debug_assert => {
61+
span_help_and_lint(
62+
cx,
63+
ASSERTIONS_ON_CONSTANTS,
64+
e.span,
65+
"`assert!(false)` should probably be replaced",
66+
"use `panic!()` or `unreachable!()`"
67+
);
68+
},
69+
_ => (),
7170
}
7271
}
7372
}

‎clippy_lints/src/methods/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ declare_clippy_lint! {
359359
/// **Why is this bad?** The function will always be called.
360360
///
361361
/// **Known problems:** If the function has side-effects, not calling it will
362-
/// change the semantic of the program, but you shouldn't rely on that anyway.
362+
/// change the semantics of the program, but you shouldn't rely on that anyway.
363363
///
364364
/// **Example:**
365365
/// ```rust

‎tests/ui/assertions_on_constants.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ fn main() {
1818
assert!(C);
1919

2020
debug_assert!(true);
21+
// Don't lint this, since there is no better way for expressing "Only panic in debug mode".
22+
debug_assert!(false); // #3948
2123
assert_const!(3);
2224
assert_const!(-1);
2325
}

‎tests/ui/assertions_on_constants.stderr

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error: assert!(true) will be optimized out by the compiler
1+
error: `assert!(true)` will be optimized out by the compiler
22
--> $DIR/assertions_on_constants.rs:9:5
33
|
44
LL | assert!(true);
@@ -7,47 +7,47 @@ LL | assert!(true);
77
= note: `-D clippy::assertions-on-constants` implied by `-D warnings`
88
= help: remove it
99

10-
error: assert!(false) should probably be replaced
10+
error: `assert!(false)` should probably be replaced
1111
--> $DIR/assertions_on_constants.rs:10:5
1212
|
1313
LL | assert!(false);
1414
| ^^^^^^^^^^^^^^^
1515
|
16-
= help: use panic!() or unreachable!()
16+
= help: use `panic!()` or `unreachable!()`
1717

18-
error: assert!(true) will be optimized out by the compiler
18+
error: `assert!(true)` will be optimized out by the compiler
1919
--> $DIR/assertions_on_constants.rs:11:5
2020
|
2121
LL | assert!(true, "true message");
2222
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2323
|
2424
= help: remove it
2525

26-
error: assert!(false) should probably be replaced
26+
error: `assert!(false)` should probably be replaced
2727
--> $DIR/assertions_on_constants.rs:12:5
2828
|
2929
LL | assert!(false, "false message");
3030
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3131
|
32-
= help: use panic!() or unreachable!()
32+
= help: use `panic!()` or `unreachable!()`
3333

34-
error: assert!(const: true) will be optimized out by the compiler
34+
error: `assert!(true)` will be optimized out by the compiler
3535
--> $DIR/assertions_on_constants.rs:15:5
3636
|
3737
LL | assert!(B);
3838
| ^^^^^^^^^^^
3939
|
4040
= help: remove it
4141

42-
error: assert!(const: false) should probably be replaced
42+
error: `assert!(false)` should probably be replaced
4343
--> $DIR/assertions_on_constants.rs:18:5
4444
|
4545
LL | assert!(C);
4646
| ^^^^^^^^^^^
4747
|
48-
= help: use panic!() or unreachable!()
48+
= help: use `panic!()` or `unreachable!()`
4949

50-
error: assert!(true) will be optimized out by the compiler
50+
error: `assert!(true)` will be optimized out by the compiler
5151
--> $DIR/assertions_on_constants.rs:20:5
5252
|
5353
LL | debug_assert!(true);

‎tests/ui/len_without_is_empty.rs

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
#![warn(clippy::len_without_is_empty)]
2+
#![allow(dead_code, unused)]
3+
4+
pub struct PubOne;
5+
6+
impl PubOne {
7+
pub fn len(self: &Self) -> isize {
8+
1
9+
}
10+
}
11+
12+
impl PubOne {
13+
// A second impl for this struct -- the error span shouldn't mention this.
14+
pub fn irrelevant(self: &Self) -> bool {
15+
false
16+
}
17+
}
18+
19+
// Identical to `PubOne`, but with an `allow` attribute on the impl complaining `len`.
20+
pub struct PubAllowed;
21+
22+
#[allow(clippy::len_without_is_empty)]
23+
impl PubAllowed {
24+
pub fn len(self: &Self) -> isize {
25+
1
26+
}
27+
}
28+
29+
// No `allow` attribute on this impl block, but that doesn't matter -- we only require one on the
30+
// impl containing `len`.
31+
impl PubAllowed {
32+
pub fn irrelevant(self: &Self) -> bool {
33+
false
34+
}
35+
}
36+
37+
pub trait PubTraitsToo {
38+
fn len(self: &Self) -> isize;
39+
}
40+
41+
impl PubTraitsToo for One {
42+
fn len(self: &Self) -> isize {
43+
0
44+
}
45+
}
46+
47+
pub struct HasIsEmpty;
48+
49+
impl HasIsEmpty {
50+
pub fn len(self: &Self) -> isize {
51+
1
52+
}
53+
54+
fn is_empty(self: &Self) -> bool {
55+
false
56+
}
57+
}
58+
59+
pub struct HasWrongIsEmpty;
60+
61+
impl HasWrongIsEmpty {
62+
pub fn len(self: &Self) -> isize {
63+
1
64+
}
65+
66+
pub fn is_empty(self: &Self, x: u32) -> bool {
67+
false
68+
}
69+
}
70+
71+
struct NotPubOne;
72+
73+
impl NotPubOne {
74+
pub fn len(self: &Self) -> isize {
75+
// No error; `len` is pub but `NotPubOne` is not exported anyway.
76+
1
77+
}
78+
}
79+
80+
struct One;
81+
82+
impl One {
83+
fn len(self: &Self) -> isize {
84+
// No error; `len` is private; see issue #1085.
85+
1
86+
}
87+
}
88+
89+
trait TraitsToo {
90+
fn len(self: &Self) -> isize;
91+
// No error; `len` is private; see issue #1085.
92+
}
93+
94+
impl TraitsToo for One {
95+
fn len(self: &Self) -> isize {
96+
0
97+
}
98+
}
99+
100+
struct HasPrivateIsEmpty;
101+
102+
impl HasPrivateIsEmpty {
103+
pub fn len(self: &Self) -> isize {
104+
1
105+
}
106+
107+
fn is_empty(self: &Self) -> bool {
108+
false
109+
}
110+
}
111+
112+
struct Wither;
113+
114+
pub trait WithIsEmpty {
115+
fn len(self: &Self) -> isize;
116+
fn is_empty(self: &Self) -> bool;
117+
}
118+
119+
impl WithIsEmpty for Wither {
120+
fn len(self: &Self) -> isize {
121+
1
122+
}
123+
124+
fn is_empty(self: &Self) -> bool {
125+
false
126+
}
127+
}
128+
129+
pub trait Empty {
130+
fn is_empty(&self) -> bool;
131+
}
132+
133+
pub trait InheritingEmpty: Empty {
134+
// Must not trigger `LEN_WITHOUT_IS_EMPTY`.
135+
fn len(&self) -> isize;
136+
}
137+
138+
// This used to ICE.
139+
pub trait Foo: Sized {}
140+
141+
pub trait DependsOnFoo: Foo {
142+
fn len(&mut self) -> usize;
143+
}
144+
145+
fn main() {}

‎tests/ui/len_without_is_empty.stderr

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
error: item `PubOne` has a public `len` method but no corresponding `is_empty` method
2+
--> $DIR/len_without_is_empty.rs:6:1
3+
|
4+
LL | / impl PubOne {
5+
LL | | pub fn len(self: &Self) -> isize {
6+
LL | | 1
7+
LL | | }
8+
LL | | }
9+
| |_^
10+
|
11+
= note: `-D clippy::len-without-is-empty` implied by `-D warnings`
12+
13+
error: trait `PubTraitsToo` has a `len` method but no (possibly inherited) `is_empty` method
14+
--> $DIR/len_without_is_empty.rs:37:1
15+
|
16+
LL | / pub trait PubTraitsToo {
17+
LL | | fn len(self: &Self) -> isize;
18+
LL | | }
19+
| |_^
20+
21+
error: item `HasIsEmpty` has a public `len` method but a private `is_empty` method
22+
--> $DIR/len_without_is_empty.rs:49:1
23+
|
24+
LL | / impl HasIsEmpty {
25+
LL | | pub fn len(self: &Self) -> isize {
26+
LL | | 1
27+
LL | | }
28+
... |
29+
LL | | }
30+
LL | | }
31+
| |_^
32+
33+
error: item `HasWrongIsEmpty` has a public `len` method but no corresponding `is_empty` method
34+
--> $DIR/len_without_is_empty.rs:61:1
35+
|
36+
LL | / impl HasWrongIsEmpty {
37+
LL | | pub fn len(self: &Self) -> isize {
38+
LL | | 1
39+
LL | | }
40+
... |
41+
LL | | }
42+
LL | | }
43+
| |_^
44+
45+
error: trait `DependsOnFoo` has a `len` method but no (possibly inherited) `is_empty` method
46+
--> $DIR/len_without_is_empty.rs:141:1
47+
|
48+
LL | / pub trait DependsOnFoo: Foo {
49+
LL | | fn len(&mut self) -> usize;
50+
LL | | }
51+
| |_^
52+
53+
error: aborting due to 5 previous errors
54+

0 commit comments

Comments
 (0)