Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of 4 Pull requests with new lints #4839

Merged
merged 11 commits into from
Nov 23, 2019
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1059,6 +1059,7 @@ Released 2018-09-13
[`just_underscores_and_digits`]: https://rust-lang.github.io/rust-clippy/master/index.html#just_underscores_and_digits
[`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
[`len_without_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_without_is_empty
[`len_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_zero
[`let_and_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
Expand Down Expand Up @@ -1176,6 +1177,7 @@ Released 2018-09-13
[`result_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or_else
[`result_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unwrap_used
[`reverse_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#reverse_range_loop
[`same_functions_in_if_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_functions_in_if_condition
[`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some
[`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse
[`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse
Expand All @@ -1201,6 +1203,7 @@ Released 2018-09-13
[`suspicious_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_map
[`suspicious_op_assign_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_op_assign_impl
[`suspicious_unary_op_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_unary_op_formatting
[`tabs_in_doc_comments`]: https://rust-lang.github.io/rust-clippy/master/index.html#tabs_in_doc_comments
[`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment
[`temporary_cstring_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr
[`to_digit_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_digit_is_some
Expand Down Expand Up @@ -1273,4 +1276,5 @@ Released 2018-09-13
[`zero_prefixed_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_prefixed_literal
[`zero_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_ptr
[`zero_width_space`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_width_space
[`zst_offset`]: https://rust-lang.github.io/rust-clippy/master/index.html#zst_offset
<!-- end autogenerated links to lint list -->
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

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

[There are 333 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 337 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

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

Expand Down
78 changes: 77 additions & 1 deletion clippy_lints/src/copies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,53 @@ declare_clippy_lint! {
"consecutive `ifs` with the same condition"
}

declare_clippy_lint! {
/// **What it does:** Checks for consecutive `if`s with the same function call.
///
/// **Why is this bad?** This is probably a copy & paste error.
/// Despite the fact that function can have side effects and `if` works as
/// intended, such an approach is implicit and can be considered a "code smell".
///
/// **Known problems:** Hopefully none.
///
/// **Example:**
/// ```ignore
/// if foo() == bar {
/// …
/// } else if foo() == bar {
/// …
/// }
/// ```
///
/// This probably should be:
/// ```ignore
/// if foo() == bar {
/// …
/// } else if foo() == baz {
/// …
/// }
/// ```
///
/// or if the original code was not a typo and called function mutates a state,
/// consider move the mutation out of the `if` condition to avoid similarity to
/// a copy & paste error:
///
/// ```ignore
/// let first = foo();
/// if first == bar {
/// …
/// } else {
/// let second = foo();
/// if second == bar {
/// …
/// }
/// }
/// ```
pub SAME_FUNCTIONS_IN_IF_CONDITION,
pedantic,
"consecutive `ifs` with the same function call"
}

declare_clippy_lint! {
/// **What it does:** Checks for `if/else` with the same body as the *then* part
/// and the *else* part.
Expand Down Expand Up @@ -102,7 +149,7 @@ declare_clippy_lint! {
"`match` with identical arm bodies"
}

declare_lint_pass!(CopyAndPaste => [IFS_SAME_COND, IF_SAME_THEN_ELSE, MATCH_SAME_ARMS]);
declare_lint_pass!(CopyAndPaste => [IFS_SAME_COND, SAME_FUNCTIONS_IN_IF_CONDITION, IF_SAME_THEN_ELSE, MATCH_SAME_ARMS]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyAndPaste {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
Expand All @@ -119,6 +166,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyAndPaste {
let (conds, blocks) = if_sequence(expr);
lint_same_then_else(cx, &blocks);
lint_same_cond(cx, &conds);
lint_same_fns_in_if_cond(cx, &conds);
lint_match_arms(cx, expr);
}
}
Expand Down Expand Up @@ -163,6 +211,34 @@ fn lint_same_cond(cx: &LateContext<'_, '_>, conds: &[&Expr]) {
}
}

/// Implementation of `SAME_FUNCTIONS_IN_IF_CONDITION`.
fn lint_same_fns_in_if_cond(cx: &LateContext<'_, '_>, conds: &[&Expr]) {
let hash: &dyn Fn(&&Expr) -> u64 = &|expr| -> u64 {
let mut h = SpanlessHash::new(cx, cx.tables);
h.hash_expr(expr);
h.finish()
};

let eq: &dyn Fn(&&Expr, &&Expr) -> bool = &|&lhs, &rhs| -> bool {
// Do not spawn warning if `IFS_SAME_COND` already produced it.
if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, rhs) {
return false;
}
SpanlessEq::new(cx).eq_expr(lhs, rhs)
};

for (i, j) in search_same(conds, hash, eq) {
span_note_and_lint(
cx,
SAME_FUNCTIONS_IN_IF_CONDITION,
j.span,
"this `if` has the same function call as a previous if",
i.span,
"same as this",
);
}
}

/// Implementation of `MATCH_SAME_ARMS`.
fn lint_match_arms<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &Expr) {
fn same_bindings<'tcx>(
Expand Down
68 changes: 68 additions & 0 deletions clippy_lints/src/large_stack_arrays.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::mir::interpret::ConstValue;
use rustc::ty::{self, ConstKind};
use rustc::{declare_tool_lint, impl_lint_pass};

use if_chain::if_chain;

use crate::rustc_target::abi::LayoutOf;
use crate::utils::{snippet, span_help_and_lint};

declare_clippy_lint! {
/// **What it does:** Checks for local arrays that may be too large.
///
/// **Why is this bad?** Large local arrays may cause stack overflow.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust,ignore
/// let a = [0u32; 1_000_000];
/// ```
pub LARGE_STACK_ARRAYS,
pedantic,
"allocating large arrays on stack may cause stack overflow"
}

pub struct LargeStackArrays {
maximum_allowed_size: u64,
}

impl LargeStackArrays {
#[must_use]
pub fn new(maximum_allowed_size: u64) -> Self {
Self { maximum_allowed_size }
}
}

impl_lint_pass!(LargeStackArrays => [LARGE_STACK_ARRAYS]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeStackArrays {
fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &Expr) {
if_chain! {
if let ExprKind::Repeat(_, _) = expr.kind;
if let ty::Array(element_type, cst) = cx.tables.expr_ty(expr).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 {
span_help_and_lint(
cx,
LARGE_STACK_ARRAYS,
expr.span,
&format!(
"allocating a local array larger than {} bytes",
self.maximum_allowed_size
),
&format!(
"consider allocating on the heap with vec!{}.into_boxed_slice()",
snippet(cx, expr.span, "[...]")
),
);
}
}
}
}
15 changes: 15 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ pub mod int_plus_one;
pub mod integer_division;
pub mod items_after_statements;
pub mod large_enum_variant;
pub mod large_stack_arrays;
pub mod len_zero;
pub mod let_if_seq;
pub mod lifetimes;
Expand Down Expand Up @@ -274,6 +275,7 @@ pub mod slow_vector_initialization;
pub mod strings;
pub mod suspicious_trait_impl;
pub mod swap;
pub mod tabs_in_doc_comments;
pub mod temporary_assignment;
pub mod to_digit_is_some;
pub mod trait_bounds;
Expand Down Expand Up @@ -472,6 +474,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
&copies::IFS_SAME_COND,
&copies::IF_SAME_THEN_ELSE,
&copies::MATCH_SAME_ARMS,
&copies::SAME_FUNCTIONS_IN_IF_CONDITION,
&copy_iterator::COPY_ITERATOR,
&dbg_macro::DBG_MACRO,
&default_trait_access::DEFAULT_TRAIT_ACCESS,
Expand Down Expand Up @@ -538,6 +541,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
&integer_division::INTEGER_DIVISION,
&items_after_statements::ITEMS_AFTER_STATEMENTS,
&large_enum_variant::LARGE_ENUM_VARIANT,
&large_stack_arrays::LARGE_STACK_ARRAYS,
&len_zero::LEN_WITHOUT_IS_EMPTY,
&len_zero::LEN_ZERO,
&let_if_seq::USELESS_LET_IF_SEQ,
Expand Down Expand Up @@ -624,6 +628,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
&methods::USELESS_ASREF,
&methods::WRONG_PUB_SELF_CONVENTION,
&methods::WRONG_SELF_CONVENTION,
&methods::ZST_OFFSET,
&minmax::MIN_MAX,
&misc::CMP_NAN,
&misc::CMP_OWNED,
Expand Down Expand Up @@ -716,6 +721,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
&suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL,
&swap::ALMOST_SWAPPED,
&swap::MANUAL_SWAP,
&tabs_in_doc_comments::TABS_IN_DOC_COMMENTS,
&temporary_assignment::TEMPORARY_ASSIGNMENT,
&to_digit_is_some::TO_DIGIT_IS_SOME,
&trait_bounds::TYPE_REPETITION_IN_BOUNDS,
Expand Down Expand Up @@ -946,10 +952,13 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
store.register_early_pass(|| box utils::internal_lints::ClippyLintsInternal);
let enum_variant_name_threshold = conf.enum_variant_name_threshold;
store.register_early_pass(move || box enum_variants::EnumVariantNames::new(enum_variant_name_threshold));
store.register_early_pass(|| box tabs_in_doc_comments::TabsInDocComments);
store.register_late_pass(|| box unused_self::UnusedSelf);
store.register_late_pass(|| box mutable_debug_assertion::DebugAssertWithMutCall);
store.register_late_pass(|| box exit::Exit);
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_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
Expand Down Expand Up @@ -989,6 +998,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
LintId::of(&attrs::INLINE_ALWAYS),
LintId::of(&checked_conversions::CHECKED_CONVERSIONS),
LintId::of(&copies::MATCH_SAME_ARMS),
LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION),
LintId::of(&copy_iterator::COPY_ITERATOR),
LintId::of(&default_trait_access::DEFAULT_TRAIT_ACCESS),
LintId::of(&derive::EXPL_IMPL_CLONE_ON_COPY),
Expand All @@ -1003,6 +1013,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
LintId::of(&if_not_else::IF_NOT_ELSE),
LintId::of(&infinite_iter::MAYBE_INFINITE_ITER),
LintId::of(&items_after_statements::ITEMS_AFTER_STATEMENTS),
LintId::of(&large_stack_arrays::LARGE_STACK_ARRAYS),
LintId::of(&literal_representation::LARGE_DIGIT_GROUPS),
LintId::of(&loops::EXPLICIT_INTO_ITER_LOOP),
LintId::of(&loops::EXPLICIT_ITER_LOOP),
Expand Down Expand Up @@ -1176,6 +1187,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
LintId::of(&methods::UNNECESSARY_FOLD),
LintId::of(&methods::USELESS_ASREF),
LintId::of(&methods::WRONG_SELF_CONVENTION),
LintId::of(&methods::ZST_OFFSET),
LintId::of(&minmax::MIN_MAX),
LintId::of(&misc::CMP_NAN),
LintId::of(&misc::CMP_OWNED),
Expand Down Expand Up @@ -1243,6 +1255,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
LintId::of(&suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),
LintId::of(&swap::ALMOST_SWAPPED),
LintId::of(&swap::MANUAL_SWAP),
LintId::of(&tabs_in_doc_comments::TABS_IN_DOC_COMMENTS),
LintId::of(&temporary_assignment::TEMPORARY_ASSIGNMENT),
LintId::of(&to_digit_is_some::TO_DIGIT_IS_SOME),
LintId::of(&transmute::CROSSPOINTER_TRANSMUTE),
Expand Down Expand Up @@ -1370,6 +1383,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
LintId::of(&returns::NEEDLESS_RETURN),
LintId::of(&returns::UNUSED_UNIT),
LintId::of(&strings::STRING_LIT_AS_BYTES),
LintId::of(&tabs_in_doc_comments::TABS_IN_DOC_COMMENTS),
LintId::of(&to_digit_is_some::TO_DIGIT_IS_SOME),
LintId::of(&try_err::TRY_ERR),
LintId::of(&types::FN_TO_NUMERIC_CAST),
Expand Down Expand Up @@ -1497,6 +1511,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
LintId::of(&methods::CLONE_DOUBLE_REF),
LintId::of(&methods::TEMPORARY_CSTRING_AS_PTR),
LintId::of(&methods::UNINIT_ASSUMED_INIT),
LintId::of(&methods::ZST_OFFSET),
LintId::of(&minmax::MIN_MAX),
LintId::of(&misc::CMP_NAN),
LintId::of(&misc::FLOAT_CMP),
Expand Down
33 changes: 33 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1065,6 +1065,23 @@ declare_clippy_lint! {
"`.chcked_add/sub(x).unwrap_or(MAX/MIN)`"
}

declare_clippy_lint! {
/// **What it does:** Checks for `offset(_)`, `wrapping_`{`add`, `sub`}, etc. on raw pointers to
/// zero-sized types
///
/// **Why is this bad?** This is a no-op, and likely unintended
///
/// **Known problems:** None
///
/// **Example:**
/// ```ignore
/// unsafe { (&() as *const ()).offest(1) };
/// ```
pub ZST_OFFSET,
correctness,
"Check for offset calculations on raw pointers to zero-sized types"
}

declare_lint_pass!(Methods => [
OPTION_UNWRAP_USED,
RESULT_UNWRAP_USED,
Expand Down Expand Up @@ -1109,6 +1126,7 @@ declare_lint_pass!(Methods => [
SUSPICIOUS_MAP,
UNINIT_ASSUMED_INIT,
MANUAL_SATURATING_ARITHMETIC,
ZST_OFFSET,
]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
Expand Down Expand Up @@ -1167,6 +1185,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
| ["unwrap_or", arith @ "checked_mul"] => {
manual_saturating_arithmetic::lint(cx, expr, &arg_lists, &arith["checked_".len()..])
},
["add"] | ["offset"] | ["sub"] | ["wrapping_offset"] | ["wrapping_add"] | ["wrapping_sub"] => {
check_pointer_offset(cx, expr, arg_lists[0])
},
_ => {},
}

Expand Down Expand Up @@ -3063,3 +3084,15 @@ fn contains_return(expr: &hir::Expr) -> bool {
visitor.visit_expr(expr);
visitor.found
}

fn check_pointer_offset(cx: &LateContext<'_, '_>, expr: &hir::Expr, args: &[hir::Expr]) {
if_chain! {
if args.len() == 2;
if let ty::RawPtr(ty::TypeAndMut { ref ty, .. }) = cx.tables.expr_ty(&args[0]).kind;
if let Ok(layout) = cx.tcx.layout_of(cx.param_env.and(ty));
if layout.is_zst();
then {
span_lint(cx, ZST_OFFSET, expr.span, "offset calculation on zero-sized value");
}
}
}
Loading