Skip to content

Commit c0b2411

Browse files
borsbors
committed
Auto merge of #4657 - Licenser:additional-restrictions, r=flip1995
Additional restrictions Add restriction lints for `panic!`, `unreachable!`, `todo!` and `.expect(...)` changelog: Add 5 new `restriction` lints: `panic`, `unreachable`, `todo`, `option_expect_used`, `result_expect_used`
2 parents ee6fc1b + 7f454d8 commit c0b2411

16 files changed

+342
-39
lines changed

‎CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,6 +1129,7 @@ Released 2018-09-13
11291129
[`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect
11301130
[`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref
11311131
[`option_and_then_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_and_then_some
1132+
[`option_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_expect_used
11321133
[`option_map_or_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_or_none
11331134
[`option_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unit_fn
11341135
[`option_map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unwrap_or
@@ -1138,6 +1139,7 @@ Released 2018-09-13
11381139
[`or_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#or_fun_call
11391140
[`out_of_bounds_indexing`]: https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing
11401141
[`overflow_check_conditional`]: https://rust-lang.github.io/rust-clippy/master/index.html#overflow_check_conditional
1142+
[`panic`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic
11411143
[`panic_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_params
11421144
[`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap
11431145
[`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl
@@ -1167,6 +1169,7 @@ Released 2018-09-13
11671169
[`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
11681170
[`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro
11691171
[`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
1172+
[`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used
11701173
[`result_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unit_fn
11711174
[`result_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or_else
11721175
[`result_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unwrap_used
@@ -1198,6 +1201,7 @@ Released 2018-09-13
11981201
[`suspicious_unary_op_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_unary_op_formatting
11991202
[`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment
12001203
[`temporary_cstring_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr
1204+
[`todo`]: https://rust-lang.github.io/rust-clippy/master/index.html#todo
12011205
[`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
12021206
[`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines
12031207
[`toplevel_ref_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#toplevel_ref_arg
@@ -1227,6 +1231,7 @@ Released 2018-09-13
12271231
[`unnecessary_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap
12281232
[`unneeded_field_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_field_pattern
12291233
[`unneeded_wildcard_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_wildcard_pattern
1234+
[`unreachable`]: https://rust-lang.github.io/rust-clippy/master/index.html#unreachable
12301235
[`unreadable_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#unreadable_literal
12311236
[`unsafe_removed_from_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_removed_from_name
12321237
[`unsafe_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_vector_initialization

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
88

9-
[There are 326 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
9+
[There are 331 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
1010

1111
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
1212

‎clippy_lints/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,13 +625,18 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
625625
mem_forget::MEM_FORGET,
626626
methods::CLONE_ON_REF_PTR,
627627
methods::GET_UNWRAP,
628+
methods::OPTION_EXPECT_USED,
628629
methods::OPTION_UNWRAP_USED,
630+
methods::RESULT_EXPECT_USED,
629631
methods::RESULT_UNWRAP_USED,
630632
methods::WRONG_PUB_SELF_CONVENTION,
631633
misc::FLOAT_CMP_CONST,
632634
missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS,
633635
missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS,
636+
panic_unimplemented::PANIC,
637+
panic_unimplemented::TODO,
634638
panic_unimplemented::UNIMPLEMENTED,
639+
panic_unimplemented::UNREACHABLE,
635640
shadow::SHADOW_REUSE,
636641
shadow::SHADOW_SAME,
637642
strings::STRING_ADD,

‎clippy_lints/src/methods/mod.rs

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ declare_clippy_lint! {
7373
/// **Known problems:** None.
7474
///
7575
/// **Example:**
76-
/// Using unwrap on an `Option`:
76+
/// Using unwrap on an `Result`:
7777
///
7878
/// ```rust
7979
/// let res: Result<usize, ()> = Ok(1);
@@ -91,6 +91,65 @@ declare_clippy_lint! {
9191
"using `Result.unwrap()`, which might be better handled"
9292
}
9393

94+
declare_clippy_lint! {
95+
/// **What it does:** Checks for `.expect()` calls on `Option`s.
96+
///
97+
/// **Why is this bad?** Usually it is better to handle the `None` case. Still,
98+
/// for a lot of quick-and-dirty code, `expect` is a good choice, which is why
99+
/// this lint is `Allow` by default.
100+
///
101+
/// **Known problems:** None.
102+
///
103+
/// **Example:**
104+
///
105+
/// Using expect on an `Option`:
106+
///
107+
/// ```rust
108+
/// let opt = Some(1);
109+
/// opt.expect("one");
110+
/// ```
111+
///
112+
/// Better:
113+
///
114+
/// ```ignore
115+
/// let opt = Some(1);
116+
/// opt?;
117+
/// # Some::<()>(())
118+
/// ```
119+
pub OPTION_EXPECT_USED,
120+
restriction,
121+
"using `Option.expect()`, which might be better handled"
122+
}
123+
124+
declare_clippy_lint! {
125+
/// **What it does:** Checks for `.expect()` calls on `Result`s.
126+
///
127+
/// **Why is this bad?** `result.expect()` will let the thread panic on `Err`
128+
/// values. Normally, you want to implement more sophisticated error handling,
129+
/// and propagate errors upwards with `try!`.
130+
///
131+
/// **Known problems:** None.
132+
///
133+
/// **Example:**
134+
/// Using expect on an `Result`:
135+
///
136+
/// ```rust
137+
/// let res: Result<usize, ()> = Ok(1);
138+
/// res.expect("one");
139+
/// ```
140+
///
141+
/// Better:
142+
///
143+
/// ```
144+
/// let res: Result<usize, ()> = Ok(1);
145+
/// res?;
146+
/// # Ok::<(), ()>(())
147+
/// ```
148+
pub RESULT_EXPECT_USED,
149+
restriction,
150+
"using `Result.expect()`, which might be better handled"
151+
}
152+
94153
declare_clippy_lint! {
95154
/// **What it does:** Checks for methods that should live in a trait
96155
/// implementation of a `std` trait (see [llogiq's blog
@@ -1037,6 +1096,8 @@ declare_clippy_lint! {
10371096
declare_lint_pass!(Methods => [
10381097
OPTION_UNWRAP_USED,
10391098
RESULT_UNWRAP_USED,
1099+
OPTION_EXPECT_USED,
1100+
RESULT_EXPECT_USED,
10401101
SHOULD_IMPLEMENT_TRAIT,
10411102
WRONG_SELF_CONVENTION,
10421103
WRONG_PUB_SELF_CONVENTION,
@@ -1095,6 +1156,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
10951156
["unwrap", "get_mut"] => lint_get_unwrap(cx, expr, arg_lists[1], true),
10961157
["unwrap", ..] => lint_unwrap(cx, expr, arg_lists[0]),
10971158
["expect", "ok"] => lint_ok_expect(cx, expr, arg_lists[1]),
1159+
["expect", ..] => lint_expect(cx, expr, arg_lists[0]),
10981160
["unwrap_or", "map"] => option_map_unwrap_or::lint(cx, expr, arg_lists[1], arg_lists[0]),
10991161
["unwrap_or_else", "map"] => lint_map_unwrap_or_else(cx, expr, arg_lists[1], arg_lists[0]),
11001162
["map_or", ..] => lint_map_or_none(cx, expr, arg_lists[0]),
@@ -2063,6 +2125,31 @@ fn lint_unwrap(cx: &LateContext<'_, '_>, expr: &hir::Expr, unwrap_args: &[hir::E
20632125
}
20642126
}
20652127

2128+
/// lint use of `expect()` for `Option`s and `Result`s
2129+
fn lint_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr, expect_args: &[hir::Expr]) {
2130+
let obj_ty = walk_ptrs_ty(cx.tables.expr_ty(&expect_args[0]));
2131+
2132+
let mess = if match_type(cx, obj_ty, &paths::OPTION) {
2133+
Some((OPTION_EXPECT_USED, "an Option", "None"))
2134+
} else if match_type(cx, obj_ty, &paths::RESULT) {
2135+
Some((RESULT_EXPECT_USED, "a Result", "Err"))
2136+
} else {
2137+
None
2138+
};
2139+
2140+
if let Some((lint, kind, none_value)) = mess {
2141+
span_lint(
2142+
cx,
2143+
lint,
2144+
expr.span,
2145+
&format!(
2146+
"used expect() on {} value. If this value is an {} it will panic",
2147+
kind, none_value
2148+
),
2149+
);
2150+
}
2151+
}
2152+
20662153
/// lint use of `ok().expect()` for `Result`s
20672154
fn lint_ok_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr, ok_args: &[hir::Expr]) {
20682155
if_chain! {

‎clippy_lints/src/panic_unimplemented.rs

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,22 @@ declare_clippy_lint! {
2525
"missing parameters in `panic!` calls"
2626
}
2727

28+
declare_clippy_lint! {
29+
/// **What it does:** Checks for usage of `panic!`.
30+
///
31+
/// **Why is this bad?** `panic!` will stop the execution of the executable
32+
///
33+
/// **Known problems:** None.
34+
///
35+
/// **Example:**
36+
/// ```no_run
37+
/// panic!("even with a good reason");
38+
/// ```
39+
pub PANIC,
40+
restriction,
41+
"usage of the `panic!` macro"
42+
}
43+
2844
declare_clippy_lint! {
2945
/// **What it does:** Checks for usage of `unimplemented!`.
3046
///
@@ -41,7 +57,39 @@ declare_clippy_lint! {
4157
"`unimplemented!` should not be present in production code"
4258
}
4359

44-
declare_lint_pass!(PanicUnimplemented => [PANIC_PARAMS, UNIMPLEMENTED]);
60+
declare_clippy_lint! {
61+
/// **What it does:** Checks for usage of `todo!`.
62+
///
63+
/// **Why is this bad?** This macro should not be present in production code
64+
///
65+
/// **Known problems:** None.
66+
///
67+
/// **Example:**
68+
/// ```no_run
69+
/// todo!();
70+
/// ```
71+
pub TODO,
72+
restriction,
73+
"`todo!` should not be present in production code"
74+
}
75+
76+
declare_clippy_lint! {
77+
/// **What it does:** Checks for usage of `unreachable!`.
78+
///
79+
/// **Why is this bad?** This macro can cause code to panic
80+
///
81+
/// **Known problems:** None.
82+
///
83+
/// **Example:**
84+
/// ```no_run
85+
/// unreachable!();
86+
/// ```
87+
pub UNREACHABLE,
88+
restriction,
89+
"`unreachable!` should not be present in production code"
90+
}
91+
92+
declare_lint_pass!(PanicUnimplemented => [PANIC_PARAMS, UNIMPLEMENTED, UNREACHABLE, TODO, PANIC]);
4593

4694
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PanicUnimplemented {
4795
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
@@ -55,7 +103,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PanicUnimplemented {
55103
let span = get_outer_span(expr);
56104
span_lint(cx, UNIMPLEMENTED, span,
57105
"`unimplemented` should not be present in production code");
58-
} else {
106+
} else if is_expn_of(expr.span, "todo").is_some() {
107+
let span = get_outer_span(expr);
108+
span_lint(cx, TODO, span,
109+
"`todo` should not be present in production code");
110+
} else if is_expn_of(expr.span, "unreachable").is_some() {
111+
let span = get_outer_span(expr);
112+
span_lint(cx, UNREACHABLE, span,
113+
"`unreachable` should not be present in production code");
114+
} else if is_expn_of(expr.span, "panic").is_some() {
115+
let span = get_outer_span(expr);
116+
span_lint(cx, PANIC, span,
117+
"`panic` should not be present in production code");
59118
match_panic(params, expr, cx);
60119
}
61120
}

‎src/lintlist/mod.rs

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pub use lint::Lint;
66
pub use lint::LINT_LEVELS;
77

88
// begin lint list, do not remove this comment, it’s used in `update_lints`
9-
pub const ALL_LINTS: [Lint; 326] = [
9+
pub const ALL_LINTS: [Lint; 331] = [
1010
Lint {
1111
name: "absurd_extreme_comparisons",
1212
group: "correctness",
@@ -1393,6 +1393,13 @@ pub const ALL_LINTS: [Lint; 326] = [
13931393
deprecation: None,
13941394
module: "methods",
13951395
},
1396+
Lint {
1397+
name: "option_expect_used",
1398+
group: "restriction",
1399+
desc: "using `Option.expect()`, which might be better handled",
1400+
deprecation: None,
1401+
module: "methods",
1402+
},
13961403
Lint {
13971404
name: "option_map_or_none",
13981405
group: "style",
@@ -1456,6 +1463,13 @@ pub const ALL_LINTS: [Lint; 326] = [
14561463
deprecation: None,
14571464
module: "overflow_check_conditional",
14581465
},
1466+
Lint {
1467+
name: "panic",
1468+
group: "restriction",
1469+
desc: "usage of the `panic!` macro",
1470+
deprecation: None,
1471+
module: "panic_unimplemented",
1472+
},
14591473
Lint {
14601474
name: "panic_params",
14611475
group: "style",
@@ -1652,6 +1666,13 @@ pub const ALL_LINTS: [Lint; 326] = [
16521666
deprecation: None,
16531667
module: "replace_consts",
16541668
},
1669+
Lint {
1670+
name: "result_expect_used",
1671+
group: "restriction",
1672+
desc: "using `Result.expect()`, which might be better handled",
1673+
deprecation: None,
1674+
module: "methods",
1675+
},
16551676
Lint {
16561677
name: "result_map_unit_fn",
16571678
group: "complexity",
@@ -1848,6 +1869,13 @@ pub const ALL_LINTS: [Lint; 326] = [
18481869
deprecation: None,
18491870
module: "methods",
18501871
},
1872+
Lint {
1873+
name: "todo",
1874+
group: "restriction",
1875+
desc: "`todo!` should not be present in production code",
1876+
deprecation: None,
1877+
module: "panic_unimplemented",
1878+
},
18511879
Lint {
18521880
name: "too_many_arguments",
18531881
group: "complexity",
@@ -2051,6 +2079,13 @@ pub const ALL_LINTS: [Lint; 326] = [
20512079
deprecation: None,
20522080
module: "misc_early",
20532081
},
2082+
Lint {
2083+
name: "unreachable",
2084+
group: "restriction",
2085+
desc: "`unreachable!` should not be present in production code",
2086+
deprecation: None,
2087+
module: "panic_unimplemented",
2088+
},
20542089
Lint {
20552090
name: "unreadable_literal",
20562091
group: "style",

‎tests/ui/expect.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#![warn(clippy::option_expect_used, clippy::result_expect_used)]
2+
3+
fn expect_option() {
4+
let opt = Some(0);
5+
let _ = opt.expect("");
6+
}
7+
8+
fn expect_result() {
9+
let res: Result<u8, ()> = Ok(0);
10+
let _ = res.expect("");
11+
}
12+
13+
fn main() {
14+
expect_option();
15+
expect_result();
16+
}

‎tests/ui/expect.stderr

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error: used expect() on an Option value. If this value is an None it will panic
2+
--> $DIR/expect.rs:5:13
3+
|
4+
LL | let _ = opt.expect("");
5+
| ^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::option-expect-used` implied by `-D warnings`
8+
9+
error: used expect() on a Result value. If this value is an Err it will panic
10+
--> $DIR/expect.rs:10:13
11+
|
12+
LL | let _ = res.expect("");
13+
| ^^^^^^^^^^^^^^
14+
|
15+
= note: `-D clippy::result-expect-used` implied by `-D warnings`
16+
17+
error: aborting due to 2 previous errors
18+

0 commit comments

Comments
 (0)