Rollup of 5 pull requests

CHANGELOG.md

@@ -1256,6 +1256,7 @@ Released 2018-09-13
 [`expect_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_fun_call
 [`expl_impl_clone_on_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#expl_impl_clone_on_copy
 [`explicit_counter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_counter_loop
+[`explicit_deref_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_methods
 [`explicit_into_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_into_iter_loop
 [`explicit_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_iter_loop
 [`explicit_write`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_write
@@ -1319,6 +1320,7 @@ Released 2018-09-13
 [`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next
 [`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero
 [`just_underscores_and_digits`]: https://rust-lang.github.io/rust-clippy/master/index.html#just_underscores_and_digits
+[`large_const_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_const_arrays
 [`large_digit_groups`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_digit_groups
 [`large_enum_variant`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant
 [`large_stack_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_arrays

clippy_lints/src/arithmetic.rs

@@ -6,11 +6,17 @@ use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::source_map::Span;
 
 declare_clippy_lint! {
- /// **What it does:** Checks for plain integer arithmetic.
+ /// **What it does:** Checks for integer arithmetic operations which could overflow or panic.
  ///
- /// **Why is this bad?** This is only checked against overflow in debug builds.
- /// In some applications one wants explicitly checked, wrapping or saturating
- /// arithmetic.
+ /// Specifically, checks for any operators (`+`, `-`, `*`, `<<`, etc) which are capable
+ /// of overflowing according to the [Rust
+ /// Reference](https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow),
+ /// or which can panic (`/`, `%`). No bounds analysis or sophisticated reasoning is
+ /// attempted.
+ ///
+ /// **Why is this bad?** Integer overflow will trigger a panic in debug builds or will wrap in
+ /// release mode. Division by zero will cause a panic in either mode. In some applications one
+ /// wants explicitly checked, wrapping or saturating arithmetic.
  ///
  /// **Known problems:** None.
  ///
@@ -21,7 +27,7 @@ declare_clippy_lint! {
  /// ```
  pub INTEGER_ARITHMETIC,
  restriction,
- "any integer arithmetic statement"
+ "any integer arithmetic expression which could overflow or panic"
 }
 
 declare_clippy_lint! {
@@ -71,8 +77,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Arithmetic {
  | hir::BinOpKind::BitAnd
  | hir::BinOpKind::BitOr
  | hir::BinOpKind::BitXor
- | hir::BinOpKind::Shl
- | hir::BinOpKind::Shr
  | hir::BinOpKind::Eq
  | hir::BinOpKind::Lt
  | hir::BinOpKind::Le

clippy_lints/src/dereference.rs

@@ -0,0 +1,113 @@
+use crate::utils::{get_parent_expr, implements_trait, snippet, span_lint_and_sugg};
+use if_chain::if_chain;
+use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX, PREC_PREFIX};
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::source_map::Span;
+
+declare_clippy_lint! {
+ /// **What it does:** Checks for explicit `deref()` or `deref_mut()` method calls.
+ ///
+ /// **Why is this bad?** Derefencing by `&*x` or `&mut *x` is clearer and more concise,
+ /// when not part of a method chain.
+ ///
+ /// **Example:**
+ /// ```rust
+ /// use std::ops::Deref;
+ /// let a: &mut String = &mut String::from("foo");
+ /// let b: &str = a.deref();
+ /// ```
+ /// Could be written as:
+ /// ```rust
+ /// let a: &mut String = &mut String::from("foo");
+ /// let b = &*a;
+ /// ```
+ ///
+ /// This lint excludes
+ /// ```rust,ignore
+ /// let _ = d.unwrap().deref();
+ /// ```
+ pub EXPLICIT_DEREF_METHODS,
+ pedantic,
+ "Explicit use of deref or deref_mut method while not in a method chain."
+}
+
+declare_lint_pass!(Dereferencing => [
+ EXPLICIT_DEREF_METHODS
+]);
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
+ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
+ if_chain! {
+ if !expr.span.from_expansion();
+ if let ExprKind::MethodCall(ref method_name, _, ref args) = &expr.kind;
+ if args.len() == 1;
+
+ then {
+ if let Some(parent_expr) = get_parent_expr(cx, expr) {
+ // Check if we have the whole call chain here
+ if let ExprKind::MethodCall(..) = parent_expr.kind {
+ return;
+ }
+ // Check for Expr that we don't want to be linted
+ let precedence = parent_expr.precedence();
+ match precedence {
+ // Lint a Call is ok though
+ ExprPrecedence::Call | ExprPrecedence::AddrOf => (),
+ _ => {
+ if precedence.order() >= PREC_PREFIX && precedence.order() <= PREC_POSTFIX {
+ return;
+ }
+ }
+ }
+ }
+ let name = method_name.ident.as_str();
+ lint_deref(cx, &*name, &args[0], args[0].span, expr.span);
+ }
+ }
+ }
+}
+
+fn lint_deref(cx: &LateContext<'_, '_>, method_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) {
+ match method_name {
+ "deref" => {
+ if cx
+ .tcx
+ .lang_items()
+ .deref_trait()
+ .map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[]))
+ {
+ span_lint_and_sugg(
+ cx,
+ EXPLICIT_DEREF_METHODS,
+ expr_span,
+ "explicit deref method call",
+ "try this",
+ format!("&*{}", &snippet(cx, var_span, "..")),
+ Applicability::MachineApplicable,
+ );
+ }
+ },
+ "deref_mut" => {
+ if cx
+ .tcx
+ .lang_items()
+ .deref_mut_trait()
+ .map_or(false, |id| implements_trait(cx, cx.tables.expr_ty(&call_expr), id, &[]))
+ {
+ span_lint_and_sugg(
+ cx,
+ EXPLICIT_DEREF_METHODS,
+ expr_span,
+ "explicit deref_mut method call",
+ "try this",
+ format!("&mut *{}", &snippet(cx, var_span, "..")),
+ Applicability::MachineApplicable,
+ );
+ }
+ },
+ _ => (),
+ }
+}

clippy_lints/src/large_const_arrays.rs

@@ -0,0 +1,85 @@
+use crate::rustc_target::abi::LayoutOf;
+use crate::utils::span_lint_and_then;
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::{Item, ItemKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::mir::interpret::ConstValue;
+use rustc_middle::ty::{self, ConstKind};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::{BytePos, Pos, Span};
+use rustc_typeck::hir_ty_to_ty;
+
+declare_clippy_lint! {
+ /// **What it does:** Checks for large `const` arrays that should
+ /// be defined as `static` instead.
+ ///
+ /// **Why is this bad?** Performance: const variables are inlined upon use.
+ /// Static items result in only one instance and has a fixed location in memory.
+ ///
+ /// **Known problems:** None.
+ ///
+ /// **Example:**
+ /// ```rust,ignore
+ /// // Bad
+ /// pub const a = [0u32; 1_000_000];
+ ///
+ /// // Good
+ /// pub static a = [0u32; 1_000_000];
+ /// ```
+ pub LARGE_CONST_ARRAYS,
+ perf,
+ "large non-scalar const array may cause performance overhead"
+}
+
+pub struct LargeConstArrays {
+ maximum_allowed_size: u64,
+}
+
+impl LargeConstArrays {
+ #[must_use]
+ pub fn new(maximum_allowed_size: u64) -> Self {
+ Self { maximum_allowed_size }
+ }
+}
+
+impl_lint_pass!(LargeConstArrays => [LARGE_CONST_ARRAYS]);
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeConstArrays {
+ fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) {
+ if_chain! {
+ if !item.span.from_expansion();
+ if let ItemKind::Const(hir_ty, _) = &item.kind;
+ let ty = hir_ty_to_ty(cx.tcx, hir_ty);
+ if let ty::Array(element_type, cst) = ty.kind;
+ if let ConstKind::Value(val) = cst.val;
+ if let ConstValue::Scalar(element_count) = val;
+ if let Ok(element_count) = element_count.to_machine_usize(&cx.tcx);
+ if let Ok(element_size) = cx.layout_of(element_type).map(|l| l.size.bytes());
+ if self.maximum_allowed_size < element_count * element_size;
+
+ then {
+ let hi_pos = item.ident.span.lo() - BytePos::from_usize(1);
+ let sugg_span = Span::new(
+ hi_pos - BytePos::from_usize("const".len()),
+ hi_pos,
+ item.span.ctxt(),
+ );
+ span_lint_and_then(
+ cx,
+ LARGE_CONST_ARRAYS,
+ item.span,
+ "large array defined as const",
+ |db| {
+ db.span_suggestion(
+ sugg_span,
+ "make this a static item",
+ "static".to_string(),
+ Applicability::MachineApplicable,
+ );
+ }
+ );
+ }
+ }
+ }
+}

clippy_lints/src/large_enum_variant.rs

@@ -21,11 +21,19 @@ declare_clippy_lint! {
  /// measure the change this lint suggests.
  ///
  /// **Example:**
+ ///
  /// ```rust
+ /// // Bad
  /// enum Test {
  /// A(i32),
  /// B([i32; 8000]),
  /// }
+ ///
+ /// // Possibly better
+ /// enum Test2 {
+ /// A(i32),
+ /// B(Box<[i32; 8000]>),
+ /// }
  /// ```
  pub LARGE_ENUM_VARIANT,
  perf,
@@ -84,12 +92,21 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeEnumVariant {
  if difference > self.maximum_size_difference_allowed {
  let (i, variant) = largest.1;
 
+ let help_text = "consider boxing the large fields to reduce the total size of the enum";
  span_lint_and_then(
  cx,
  LARGE_ENUM_VARIANT,
  def.variants[i].span,
  "large size difference between variants",
  |db| {
+ db.span_label(
+ def.variants[(largest.1).0].span,
+ &format!("this variant is {} bytes", largest.0),
+ );
+ db.span_note(
+ def.variants[(second.1).0].span,
+ &format!("and the second-largest variant is {} bytes:", second.0),
+ );
  if variant.fields.len() == 1 {
  let span = match def.variants[i].data {
  VariantData::Struct(ref fields, ..) | VariantData::Tuple(ref fields, ..) => {
@@ -100,18 +117,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeEnumVariant {
  if let Some(snip) = snippet_opt(cx, span) {
  db.span_suggestion(
  span,
- "consider boxing the large fields to reduce the total size of the \
- enum",
+ help_text,
  format!("Box<{}>", snip),
  Applicability::MaybeIncorrect,
  );
  return;
  }
  }
- db.span_help(
- def.variants[i].span,
- "consider boxing the large fields to reduce the total size of the enum",
- );
+ db.span_help(def.variants[i].span, help_text);
  },
  );
  }

clippy_lints/src/lib.rs

@@ -191,6 +191,7 @@ mod copies;
 mod copy_iterator;
 mod dbg_macro;
 mod default_trait_access;
+mod dereference;
 mod derive;
 mod doc;
 mod double_comparison;
@@ -231,6 +232,7 @@ mod inline_fn_without_body;
 mod int_plus_one;
 mod integer_division;
 mod items_after_statements;
+mod large_const_arrays;
 mod large_enum_variant;
 mod large_stack_arrays;
 mod len_zero;
@@ -513,6 +515,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  &copy_iterator::COPY_ITERATOR,
  &dbg_macro::DBG_MACRO,
  &default_trait_access::DEFAULT_TRAIT_ACCESS,
+ &dereference::EXPLICIT_DEREF_METHODS,
  &derive::DERIVE_HASH_XOR_EQ,
  &derive::EXPL_IMPL_CLONE_ON_COPY,
  &doc::DOC_MARKDOWN,
@@ -580,6 +583,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  &int_plus_one::INT_PLUS_ONE,
  &integer_division::INTEGER_DIVISION,
  &items_after_statements::ITEMS_AFTER_STATEMENTS,
+ &large_const_arrays::LARGE_CONST_ARRAYS,
  &large_enum_variant::LARGE_ENUM_VARIANT,
  &large_stack_arrays::LARGE_STACK_ARRAYS,
  &len_zero::LEN_WITHOUT_IS_EMPTY,
@@ -1024,6 +1028,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  store.register_late_pass(|| box to_digit_is_some::ToDigitIsSome);
  let array_size_threshold = conf.array_size_threshold;
  store.register_late_pass(move || box large_stack_arrays::LargeStackArrays::new(array_size_threshold));
+ store.register_late_pass(move || box large_const_arrays::LargeConstArrays::new(array_size_threshold));
  store.register_late_pass(move || box floating_point_arithmetic::FloatingPointArithmetic);
  store.register_early_pass(|| box as_conversions::AsConversions);
  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:
  store.register_late_pass(|| box verbose_file_reads::VerboseFileReads);
  store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default());
  store.register_late_pass(|| box unnamed_address::UnnamedAddress);
+ store.register_late_pass(|| box dereference::Dereferencing);
 
  store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
  LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1089,6 +1095,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION),
  LintId::of(&copy_iterator::COPY_ITERATOR),
  LintId::of(&default_trait_access::DEFAULT_TRAIT_ACCESS),
+ LintId::of(&dereference::EXPLICIT_DEREF_METHODS),
  LintId::of(&derive::EXPL_IMPL_CLONE_ON_COPY),
  LintId::of(&doc::DOC_MARKDOWN),
  LintId::of(&doc::MISSING_ERRORS_DOC),
@@ -1221,6 +1228,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  LintId::of(&inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY),
  LintId::of(&inline_fn_without_body::INLINE_FN_WITHOUT_BODY),
  LintId::of(&int_plus_one::INT_PLUS_ONE),
+ LintId::of(&large_const_arrays::LARGE_CONST_ARRAYS),
  LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT),
  LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
  LintId::of(&len_zero::LEN_ZERO),
@@ -1652,6 +1660,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  LintId::of(&bytecount::NAIVE_BYTECOUNT),
  LintId::of(&entry::MAP_ENTRY),
  LintId::of(&escape::BOXED_LOCAL),
+ LintId::of(&large_const_arrays::LARGE_CONST_ARRAYS),
  LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT),
  LintId::of(&loops::MANUAL_MEMCPY),
  LintId::of(&loops::NEEDLESS_COLLECT),

clippy_lints/src/non_expressive_names.rs

@@ -93,7 +93,7 @@ impl<'a, 'tcx> SimilarNamesLocalVisitor<'a, 'tcx> {
  fn check_single_char_names(&self) {
  let num_single_char_names = self.single_char_names.iter().flatten().count();
  let threshold = self.lint.single_char_binding_names_threshold;
- if num_single_char_names as u64 >= threshold {
+ if num_single_char_names as u64 > threshold {
  let span = self
  .single_char_names
  .iter()