Skip to content

Commit 6651c1b

Browse files
borsbors
committed
Auto merge of #5470 - flip1995:rollup-cvkuiza, r=flip1995
Rollup of 5 pull requests Successful merges: - #5226 (Add lint for explicit deref and deref_mut method calls) - #5248 (Add lint on large non scalar const) - #5430 (Disallow bit-shifting in integer_arithmetic) - #5466 (large_enum_variant: Report sizes of variants) - #5468 (Zero single char names) Failed merges: r? @ghost changelog: rollup
2 parents a9a4c8e + 19183a6 commit 6651c1b

21 files changed

+731
-39
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1256,6 +1256,7 @@ Released 2018-09-13
12561256
[`expect_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_fun_call
12571257
[`expl_impl_clone_on_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#expl_impl_clone_on_copy
12581258
[`explicit_counter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_counter_loop
1259+
[`explicit_deref_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_methods
12591260
[`explicit_into_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_into_iter_loop
12601261
[`explicit_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_iter_loop
12611262
[`explicit_write`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_write
@@ -1319,6 +1320,7 @@ Released 2018-09-13
13191320
[`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next
13201321
[`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero
13211322
[`just_underscores_and_digits`]: https://rust-lang.github.io/rust-clippy/master/index.html#just_underscores_and_digits
1323+
[`large_const_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_const_arrays
13221324
[`large_digit_groups`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_digit_groups
13231325
[`large_enum_variant`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant
13241326
[`large_stack_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_arrays

‎clippy_lints/src/arithmetic.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,17 @@ use rustc_session::{declare_tool_lint, impl_lint_pass};
66
use rustc_span::source_map::Span;
77

88
declare_clippy_lint! {
9-
/// **What it does:** Checks for plain integer arithmetic.
9+
/// **What it does:** Checks for integer arithmetic operations which could overflow or panic.
1010
///
11-
/// **Why is this bad?** This is only checked against overflow in debug builds.
12-
/// In some applications one wants explicitly checked, wrapping or saturating
13-
/// arithmetic.
11+
/// Specifically, checks for any operators (`+`, `-`, `*`, `<<`, etc) which are capable
12+
/// of overflowing according to the [Rust
13+
/// Reference](https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow),
14+
/// or which can panic (`/`, `%`). No bounds analysis or sophisticated reasoning is
15+
/// attempted.
16+
///
17+
/// **Why is this bad?** Integer overflow will trigger a panic in debug builds or will wrap in
18+
/// release mode. Division by zero will cause a panic in either mode. In some applications one
19+
/// wants explicitly checked, wrapping or saturating arithmetic.
1420
///
1521
/// **Known problems:** None.
1622
///
@@ -21,7 +27,7 @@ declare_clippy_lint! {
2127
/// ```
2228
pub INTEGER_ARITHMETIC,
2329
restriction,
24-
"any integer arithmetic statement"
30+
"any integer arithmetic expression which could overflow or panic"
2531
}
2632

2733
declare_clippy_lint! {
@@ -71,8 +77,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Arithmetic {
7177
| hir::BinOpKind::BitAnd
7278
| hir::BinOpKind::BitOr
7379
| hir::BinOpKind::BitXor
74-
| hir::BinOpKind::Shl
75-
| hir::BinOpKind::Shr
7680
| hir::BinOpKind::Eq
7781
| hir::BinOpKind::Lt
7882
| hir::BinOpKind::Le

‎clippy_lints/src/dereference.rs

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
use crate::utils::{get_parent_expr, implements_trait, snippet, span_lint_and_sugg};
2+
use if_chain::if_chain;
3+
use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX, PREC_PREFIX};
4+
use rustc_errors::Applicability;
5+
use rustc_hir::{Expr, ExprKind};
6+
use rustc_lint::{LateContext, LateLintPass};
7+
use rustc_session::{declare_lint_pass, declare_tool_lint};
8+
use rustc_span::source_map::Span;
9+
10+
declare_clippy_lint! {
11+
/// **What it does:** Checks for explicit `deref()` or `deref_mut()` method calls.
12+
///
13+
/// **Why is this bad?** Derefencing by `&*x` or `&mut *x` is clearer and more concise,
14+
/// when not part of a method chain.
15+
///
16+
/// **Example:**
17+
/// ```rust
18+
/// use std::ops::Deref;
19+
/// let a: &mut String = &mut String::from("foo");
20+
/// let b: &str = a.deref();
21+
/// ```
22+
/// Could be written as:
23+
/// ```rust
24+
/// let a: &mut String = &mut String::from("foo");
25+
/// let b = &*a;
26+
/// ```
27+
///
28+
/// This lint excludes
29+
/// ```rust,ignore
30+
/// let _ = d.unwrap().deref();
31+
/// ```
32+
pub EXPLICIT_DEREF_METHODS,
33+
pedantic,
34+
"Explicit use of deref or deref_mut method while not in a method chain."
35+
}
36+
37+
declare_lint_pass!(Dereferencing => [
38+
EXPLICIT_DEREF_METHODS
39+
]);
40+
41+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
42+
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
43+
if_chain! {
44+
if !expr.span.from_expansion();
45+
if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.kind;
46+
if args.len() == 1;
47+
48+
then {
49+
if let Some(parent_expr) = get_parent_expr(cx, expr) {
50+
// Check if we have the whole call chain here
51+
if let ExprKind::MethodCall(..) = parent_expr.kind {
52+
return;
53+
}
54+
// Check for Expr that we don't want to be linted
55+
let precedence = parent_expr.precedence();
56+
match precedence {
57+
// Lint a Call is ok though
58+
ExprPrecedence::Call | ExprPrecedence::AddrOf => (),
59+
_ => {
60+
if precedence.order() >= PREC_PREFIX && precedence.order() <= PREC_POSTFIX {
61+
return;
62+
}
63+
}
64+
}
65+
}
66+
let name = method_name.ident.as_str();
67+
lint_deref(cx, &*name, &args[0], args[0].span, expr.span);
68+
}
69+
}
70+
}
71+
}
72+
73+
fn lint_deref(cx: &LateContext<'_, '_>, method_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) {
74+
match method_name {
75+
"deref" => {
76+
if cx
77+
.tcx
78+
.lang_items()
79+
.deref_trait()
80+
.map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[]))
81+
{
82+
span_lint_and_sugg(
83+
cx,
84+
EXPLICIT_DEREF_METHODS,
85+
expr_span,
86+
"explicit deref method call",
87+
"try this",
88+
format!("&*{}", &snippet(cx, var_span, "..")),
89+
Applicability::MachineApplicable,
90+
);
91+
}
92+
},
93+
"deref_mut" => {
94+
if cx
95+
.tcx
96+
.lang_items()
97+
.deref_mut_trait()
98+
.map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[]))
99+
{
100+
span_lint_and_sugg(
101+
cx,
102+
EXPLICIT_DEREF_METHODS,
103+
expr_span,
104+
"explicit deref_mut method call",
105+
"try this",
106+
format!("&mut *{}", &snippet(cx, var_span, "..")),
107+
Applicability::MachineApplicable,
108+
);
109+
}
110+
},
111+
_ => (),
112+
}
113+
}

‎clippy_lints/src/large_const_arrays.rs

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
use crate::rustc_target::abi::LayoutOf;
2+
use crate::utils::span_lint_and_then;
3+
use if_chain::if_chain;
4+
use rustc_errors::Applicability;
5+
use rustc_hir::{Item, ItemKind};
6+
use rustc_lint::{LateContext, LateLintPass};
7+
use rustc_middle::mir::interpret::ConstValue;
8+
use rustc_middle::ty::{self, ConstKind};
9+
use rustc_session::{declare_tool_lint, impl_lint_pass};
10+
use rustc_span::{BytePos, Pos, Span};
11+
use rustc_typeck::hir_ty_to_ty;
12+
13+
declare_clippy_lint! {
14+
/// **What it does:** Checks for large `const` arrays that should
15+
/// be defined as `static` instead.
16+
///
17+
/// **Why is this bad?** Performance: const variables are inlined upon use.
18+
/// Static items result in only one instance and has a fixed location in memory.
19+
///
20+
/// **Known problems:** None.
21+
///
22+
/// **Example:**
23+
/// ```rust,ignore
24+
/// // Bad
25+
/// pub const a = [0u32; 1_000_000];
26+
///
27+
/// // Good
28+
/// pub static a = [0u32; 1_000_000];
29+
/// ```
30+
pub LARGE_CONST_ARRAYS,
31+
perf,
32+
"large non-scalar const array may cause performance overhead"
33+
}
34+
35+
pub struct LargeConstArrays {
36+
maximum_allowed_size: u64,
37+
}
38+
39+
impl LargeConstArrays {
40+
#[must_use]
41+
pub fn new(maximum_allowed_size: u64) -> Self {
42+
Self { maximum_allowed_size }
43+
}
44+
}
45+
46+
impl_lint_pass!(LargeConstArrays => [LARGE_CONST_ARRAYS]);
47+
48+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeConstArrays {
49+
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) {
50+
if_chain! {
51+
if !item.span.from_expansion();
52+
if let ItemKind::Const(hir_ty, _) = &item.kind;
53+
let ty = hir_ty_to_ty(cx.tcx, hir_ty);
54+
if let ty::Array(element_type, cst) = ty.kind;
55+
if let ConstKind::Value(val) = cst.val;
56+
if let ConstValue::Scalar(element_count) = val;
57+
if let Ok(element_count) = element_count.to_machine_usize(&cx.tcx);
58+
if let Ok(element_size) = cx.layout_of(element_type).map(|l| l.size.bytes());
59+
if self.maximum_allowed_size < element_count * element_size;
60+
61+
then {
62+
let hi_pos = item.ident.span.lo() - BytePos::from_usize(1);
63+
let sugg_span = Span::new(
64+
hi_pos - BytePos::from_usize("const".len()),
65+
hi_pos,
66+
item.span.ctxt(),
67+
);
68+
span_lint_and_then(
69+
cx,
70+
LARGE_CONST_ARRAYS,
71+
item.span,
72+
"large array defined as const",
73+
|db| {
74+
db.span_suggestion(
75+
sugg_span,
76+
"make this a static item",
77+
"static".to_string(),
78+
Applicability::MachineApplicable,
79+
);
80+
}
81+
);
82+
}
83+
}
84+
}
85+
}

‎clippy_lints/src/large_enum_variant.rs

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,19 @@ declare_clippy_lint! {
2121
/// measure the change this lint suggests.
2222
///
2323
/// **Example:**
24+
///
2425
/// ```rust
26+
/// // Bad
2527
/// enum Test {
2628
/// A(i32),
2729
/// B([i32; 8000]),
2830
/// }
31+
///
32+
/// // Possibly better
33+
/// enum Test2 {
34+
/// A(i32),
35+
/// B(Box<[i32; 8000]>),
36+
/// }
2937
/// ```
3038
pub LARGE_ENUM_VARIANT,
3139
perf,
@@ -84,12 +92,21 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeEnumVariant {
8492
if difference > self.maximum_size_difference_allowed {
8593
let (i, variant) = largest.1;
8694

95+
let help_text = "consider boxing the large fields to reduce the total size of the enum";
8796
span_lint_and_then(
8897
cx,
8998
LARGE_ENUM_VARIANT,
9099
def.variants[i].span,
91100
"large size difference between variants",
92101
|db| {
102+
db.span_label(
103+
def.variants[(largest.1).0].span,
104+
&format!("this variant is {} bytes", largest.0),
105+
);
106+
db.span_note(
107+
def.variants[(second.1).0].span,
108+
&format!("and the second-largest variant is {} bytes:", second.0),
109+
);
93110
if variant.fields.len() == 1 {
94111
let span = match def.variants[i].data {
95112
VariantData::Struct(ref fields, ..) | VariantData::Tuple(ref fields, ..) => {
@@ -100,18 +117,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeEnumVariant {
100117
if let Some(snip) = snippet_opt(cx, span) {
101118
db.span_suggestion(
102119
span,
103-
"consider boxing the large fields to reduce the total size of the \
104-
enum",
120+
help_text,
105121
format!("Box<{}>", snip),
106122
Applicability::MaybeIncorrect,
107123
);
108124
return;
109125
}
110126
}
111-
db.span_help(
112-
def.variants[i].span,
113-
"consider boxing the large fields to reduce the total size of the enum",
114-
);
127+
db.span_help(def.variants[i].span, help_text);
115128
},
116129
);
117130
}

‎clippy_lints/src/lib.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ mod copies;
191191
mod copy_iterator;
192192
mod dbg_macro;
193193
mod default_trait_access;
194+
mod dereference;
194195
mod derive;
195196
mod doc;
196197
mod double_comparison;
@@ -231,6 +232,7 @@ mod inline_fn_without_body;
231232
mod int_plus_one;
232233
mod integer_division;
233234
mod items_after_statements;
235+
mod large_const_arrays;
234236
mod large_enum_variant;
235237
mod large_stack_arrays;
236238
mod len_zero;
@@ -513,6 +515,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
513515
&copy_iterator::COPY_ITERATOR,
514516
&dbg_macro::DBG_MACRO,
515517
&default_trait_access::DEFAULT_TRAIT_ACCESS,
518+
&dereference::EXPLICIT_DEREF_METHODS,
516519
&derive::DERIVE_HASH_XOR_EQ,
517520
&derive::EXPL_IMPL_CLONE_ON_COPY,
518521
&doc::DOC_MARKDOWN,
@@ -580,6 +583,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
580583
&int_plus_one::INT_PLUS_ONE,
581584
&integer_division::INTEGER_DIVISION,
582585
&items_after_statements::ITEMS_AFTER_STATEMENTS,
586+
&large_const_arrays::LARGE_CONST_ARRAYS,
583587
&large_enum_variant::LARGE_ENUM_VARIANT,
584588
&large_stack_arrays::LARGE_STACK_ARRAYS,
585589
&len_zero::LEN_WITHOUT_IS_EMPTY,
@@ -1024,6 +1028,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10241028
store.register_late_pass(|| box to_digit_is_some::ToDigitIsSome);
10251029
let array_size_threshold = conf.array_size_threshold;
10261030
store.register_late_pass(move || box large_stack_arrays::LargeStackArrays::new(array_size_threshold));
1031+
store.register_late_pass(move || box large_const_arrays::LargeConstArrays::new(array_size_threshold));
10271032
store.register_late_pass(move || box floating_point_arithmetic::FloatingPointArithmetic);
10281033
store.register_early_pass(|| box as_conversions::AsConversions);
10291034
store.register_early_pass(|| box utils::internal_lints::ProduceIce);
@@ -1039,6 +1044,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10391044
store.register_late_pass(|| box verbose_file_reads::VerboseFileReads);
10401045
store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default());
10411046
store.register_late_pass(|| box unnamed_address::UnnamedAddress);
1047+
store.register_late_pass(|| box dereference::Dereferencing);
10421048

10431049
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
10441050
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1089,6 +1095,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10891095
LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION),
10901096
LintId::of(&copy_iterator::COPY_ITERATOR),
10911097
LintId::of(&default_trait_access::DEFAULT_TRAIT_ACCESS),
1098+
LintId::of(&dereference::EXPLICIT_DEREF_METHODS),
10921099
LintId::of(&derive::EXPL_IMPL_CLONE_ON_COPY),
10931100
LintId::of(&doc::DOC_MARKDOWN),
10941101
LintId::of(&doc::MISSING_ERRORS_DOC),
@@ -1221,6 +1228,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12211228
LintId::of(&inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY),
12221229
LintId::of(&inline_fn_without_body::INLINE_FN_WITHOUT_BODY),
12231230
LintId::of(&int_plus_one::INT_PLUS_ONE),
1231+
LintId::of(&large_const_arrays::LARGE_CONST_ARRAYS),
12241232
LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT),
12251233
LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
12261234
LintId::of(&len_zero::LEN_ZERO),
@@ -1652,6 +1660,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16521660
LintId::of(&bytecount::NAIVE_BYTECOUNT),
16531661
LintId::of(&entry::MAP_ENTRY),
16541662
LintId::of(&escape::BOXED_LOCAL),
1663+
LintId::of(&large_const_arrays::LARGE_CONST_ARRAYS),
16551664
LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT),
16561665
LintId::of(&loops::MANUAL_MEMCPY),
16571666
LintId::of(&loops::NEEDLESS_COLLECT),

‎clippy_lints/src/non_expressive_names.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ impl<'a, 'tcx> SimilarNamesLocalVisitor<'a, 'tcx> {
9393
fn check_single_char_names(&self) {
9494
let num_single_char_names = self.single_char_names.iter().flatten().count();
9595
let threshold = self.lint.single_char_binding_names_threshold;
96-
if num_single_char_names as u64 >= threshold {
96+
if num_single_char_names as u64 > threshold {
9797
let span = self
9898
.single_char_names
9999
.iter()

0 commit comments

Comments
 (0)