From 98c5f37ad2f0b40721805b3af21d26109297ce95 Mon Sep 17 00:00:00 2001 From: "A.A.Abroskin" Date: Wed, 26 Dec 2018 01:29:03 +0300 Subject: [PATCH 1/7] Add assert(true) and assert(false) lints --- clippy_lints/src/assert_checks.rs | 78 ++++++++++++++++++++ clippy_lints/src/lib.rs | 6 ++ tests/ui/assert_checks.rs | 13 ++++ tests/ui/assert_checks.stderr | 18 +++++ tests/ui/attrs.rs | 2 +- tests/ui/empty_line_after_outer_attribute.rs | 2 +- tests/ui/panic_unimplemented.rs | 2 +- 7 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 clippy_lints/src/assert_checks.rs create mode 100644 tests/ui/assert_checks.rs create mode 100644 tests/ui/assert_checks.stderr diff --git a/clippy_lints/src/assert_checks.rs b/clippy_lints/src/assert_checks.rs new file mode 100644 index 0000000000000..ecd71952b93ca --- /dev/null +++ b/clippy_lints/src/assert_checks.rs @@ -0,0 +1,78 @@ +// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use crate::rustc::hir::{Expr, ExprKind}; +use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; +use crate::rustc::{declare_tool_lint, lint_array}; +use crate::syntax::ast::LitKind; +use crate::utils::{is_direct_expn_of, span_lint}; +use if_chain::if_chain; + +/// **What it does:** Check explicit call assert!(true) +/// +/// **Why is this bad?** Will be optimized out by the compiler +/// +/// **Known problems:** None +/// +/// **Example:** +/// ```rust +/// assert!(true) +/// ``` +declare_clippy_lint! { + pub EXPLICIT_TRUE, + correctness, + "assert!(true) will be optimized out by the compiler" +} + +/// **What it does:** Check explicit call assert!(false) +/// +/// **Why is this bad?** Should probably be replaced by a panic!() +/// +/// **Known problems:** None +/// +/// **Example:** +/// ```rust +/// assert!(false) +/// ``` +declare_clippy_lint! { + pub EXPLICIT_FALSE, + correctness, + "assert!(false) should probably be replaced by a panic!()r" +} + +pub struct AssertChecks; + +impl LintPass for AssertChecks { + fn get_lints(&self) -> LintArray { + lint_array![EXPLICIT_TRUE, EXPLICIT_FALSE] + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertChecks { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { + if_chain! { + if is_direct_expn_of(e.span, "assert").is_some(); + if let ExprKind::Unary(_, ref lit) = e.node; + if let ExprKind::Lit(ref inner) = lit.node; + then { + match inner.node { + LitKind::Bool(true) => { + span_lint(cx, EXPLICIT_TRUE, e.span, + "assert!(true) will be optimized out by the compiler"); + }, + LitKind::Bool(false) => { + span_lint(cx, EXPLICIT_FALSE, e.span, + "assert!(false) should probably be replaced by a panic!()"); + }, + _ => (), + } + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 2e515cc8aea60..1f4cb27891b96 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -88,6 +88,7 @@ mod utils; // begin lints modules, do not remove this comment, it’s used in `update_lints` pub mod approx_const; pub mod arithmetic; +pub mod assert_checks; pub mod assign_ops; pub mod attrs; pub mod bit_mask; @@ -486,6 +487,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box ptr_offset_with_cast::Pass); reg.register_late_lint_pass(box redundant_clone::RedundantClone); reg.register_late_lint_pass(box slow_vector_initialization::Pass); + reg.register_late_lint_pass(box assert_checks::AssertChecks); reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![ arithmetic::FLOAT_ARITHMETIC, @@ -563,6 +565,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_lint_group("clippy::all", Some("clippy"), vec![ approx_const::APPROX_CONSTANT, + assert_checks::EXPLICIT_TRUE, + assert_checks::EXPLICIT_FALSE, assign_ops::ASSIGN_OP_PATTERN, assign_ops::MISREFACTORED_ASSIGN_OP, attrs::DEPRECATED_CFG_ATTR, @@ -940,6 +944,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_lint_group("clippy::correctness", Some("clippy_correctness"), vec![ approx_const::APPROX_CONSTANT, + assert_checks::EXPLICIT_TRUE, + assert_checks::EXPLICIT_FALSE, attrs::DEPRECATED_SEMVER, attrs::USELESS_ATTRIBUTE, bit_mask::BAD_BIT_MASK, diff --git a/tests/ui/assert_checks.rs b/tests/ui/assert_checks.rs new file mode 100644 index 0000000000000..811046d060a25 --- /dev/null +++ b/tests/ui/assert_checks.rs @@ -0,0 +1,13 @@ +// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +fn main() { + assert!(true); + assert!(false); +} diff --git a/tests/ui/assert_checks.stderr b/tests/ui/assert_checks.stderr new file mode 100644 index 0000000000000..fd7e4e014201d --- /dev/null +++ b/tests/ui/assert_checks.stderr @@ -0,0 +1,18 @@ +error: assert!(true) will be optimized out by the compiler + --> $DIR/assert_checks.rs:11:5 + | +11 | assert!(true); + | ^^^^^^^^^^^^^^ + | + = note: #[deny(clippy::explicit_true)] on by default + +error: assert!(false) should probably be replaced by a panic!() + --> $DIR/assert_checks.rs:12:5 + | +12 | assert!(false); + | ^^^^^^^^^^^^^^^ + | + = note: #[deny(clippy::explicit_false)] on by default + +error: aborting due to 2 previous errors + diff --git a/tests/ui/attrs.rs b/tests/ui/attrs.rs index 413c30a194532..c0ea732971810 100644 --- a/tests/ui/attrs.rs +++ b/tests/ui/attrs.rs @@ -8,7 +8,7 @@ // except according to those terms. #![warn(clippy::inline_always, clippy::deprecated_semver)] - +#![allow(clippy::assert_checks::explicit_true)] #[inline(always)] fn test_attr_lint() { assert!(true) diff --git a/tests/ui/empty_line_after_outer_attribute.rs b/tests/ui/empty_line_after_outer_attribute.rs index ede1244df7ed1..a6e6adcac5c1b 100644 --- a/tests/ui/empty_line_after_outer_attribute.rs +++ b/tests/ui/empty_line_after_outer_attribute.rs @@ -8,7 +8,7 @@ // except according to those terms. #![warn(clippy::empty_line_after_outer_attr)] - +#![allow(clippy::assert_checks::explicit_true)] // This should produce a warning #[crate_type = "lib"] diff --git a/tests/ui/panic_unimplemented.rs b/tests/ui/panic_unimplemented.rs index 93dec197ff5f8..3c568658a6cf4 100644 --- a/tests/ui/panic_unimplemented.rs +++ b/tests/ui/panic_unimplemented.rs @@ -8,7 +8,7 @@ // except according to those terms. #![warn(clippy::panic_params, clippy::unimplemented)] - +#![allow(clippy::assert_checks::explicit_true)] fn missing() { if true { panic!("{}"); From 3d9535a1067ad2913eab4bfd44b220bab0655b47 Mon Sep 17 00:00:00 2001 From: "A.A.Abroskin" Date: Thu, 27 Dec 2018 16:12:11 +0300 Subject: [PATCH 2/7] Add unreachable!() as option --- clippy_lints/src/assert_checks.rs | 6 +++--- tests/ui/assert_checks.stderr | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/assert_checks.rs b/clippy_lints/src/assert_checks.rs index ecd71952b93ca..dcc11951b267d 100644 --- a/clippy_lints/src/assert_checks.rs +++ b/clippy_lints/src/assert_checks.rs @@ -32,7 +32,7 @@ declare_clippy_lint! { /// **What it does:** Check explicit call assert!(false) /// -/// **Why is this bad?** Should probably be replaced by a panic!() +/// **Why is this bad?** Should probably be replaced by a panic!() or unreachable!() /// /// **Known problems:** None /// @@ -43,7 +43,7 @@ declare_clippy_lint! { declare_clippy_lint! { pub EXPLICIT_FALSE, correctness, - "assert!(false) should probably be replaced by a panic!()r" + "assert!(false) should probably be replaced by a panic!() or unreachable!()" } pub struct AssertChecks; @@ -68,7 +68,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertChecks { }, LitKind::Bool(false) => { span_lint(cx, EXPLICIT_FALSE, e.span, - "assert!(false) should probably be replaced by a panic!()"); + "assert!(false) should probably be replaced by a panic!() or unreachable!()"); }, _ => (), } diff --git a/tests/ui/assert_checks.stderr b/tests/ui/assert_checks.stderr index fd7e4e014201d..e203994165061 100644 --- a/tests/ui/assert_checks.stderr +++ b/tests/ui/assert_checks.stderr @@ -6,7 +6,7 @@ error: assert!(true) will be optimized out by the compiler | = note: #[deny(clippy::explicit_true)] on by default -error: assert!(false) should probably be replaced by a panic!() +error: assert!(false) should probably be replaced by a panic!() or unreachable!() --> $DIR/assert_checks.rs:12:5 | 12 | assert!(false); From 96058616e2a5d8af709da67cb03c35715437a990 Mon Sep 17 00:00:00 2001 From: "A.A.Abroskin" Date: Thu, 27 Dec 2018 16:48:11 +0300 Subject: [PATCH 3/7] run ./util/dev update_lints --- CHANGELOG.md | 2 ++ README.md | 6 ++++++ clippy_lints/src/lib.rs | 4 ++-- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index efa637b185cda..603fb0b5b7bd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -679,8 +679,10 @@ All notable changes to this project will be documented in this file. [`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_false`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_false [`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_true`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_true [`explicit_write`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_write [`extend_from_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#extend_from_slice [`extra_unused_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#extra_unused_lifetimes diff --git a/README.md b/README.md index be54424dc3811..658b2a70adc2b 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,13 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. +<<<<<<< HEAD [There are 290 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +||||||| merged common ancestors +[There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +======= +[There are 293 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +>>>>>>> run ./util/dev update_lints We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1f4cb27891b96..8af73a4f99b28 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -565,8 +565,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_lint_group("clippy::all", Some("clippy"), vec![ approx_const::APPROX_CONSTANT, - assert_checks::EXPLICIT_TRUE, assert_checks::EXPLICIT_FALSE, + assert_checks::EXPLICIT_TRUE, assign_ops::ASSIGN_OP_PATTERN, assign_ops::MISREFACTORED_ASSIGN_OP, attrs::DEPRECATED_CFG_ATTR, @@ -944,8 +944,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_lint_group("clippy::correctness", Some("clippy_correctness"), vec![ approx_const::APPROX_CONSTANT, - assert_checks::EXPLICIT_TRUE, assert_checks::EXPLICIT_FALSE, + assert_checks::EXPLICIT_TRUE, attrs::DEPRECATED_SEMVER, attrs::USELESS_ATTRIBUTE, bit_mask::BAD_BIT_MASK, From 906b51637ca4bfa0cf68b909160937546234a2e2 Mon Sep 17 00:00:00 2001 From: "A.A.Abroskin" Date: Wed, 9 Jan 2019 13:38:38 +0300 Subject: [PATCH 4/7] change assert_checks to assertions_on_constants --- CHANGELOG.md | 3 +- README.md | 4 +- ...t_checks.rs => assertions_on_constants.rs} | 54 +++++++++---------- clippy_lints/src/lib.rs | 10 ++-- tests/ui/assert_checks.stderr | 18 ------- ...t_checks.rs => assertions_on_constants.rs} | 0 tests/ui/assertions_on_constants.stderr | 16 ++++++ tests/ui/attrs.rs | 2 +- tests/ui/empty_line_after_outer_attribute.rs | 2 +- tests/ui/panic_unimplemented.rs | 2 +- 10 files changed, 51 insertions(+), 60 deletions(-) rename clippy_lints/src/{assert_checks.rs => assertions_on_constants.rs} (58%) delete mode 100644 tests/ui/assert_checks.stderr rename tests/ui/{assert_checks.rs => assertions_on_constants.rs} (100%) create mode 100644 tests/ui/assertions_on_constants.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 603fb0b5b7bd6..69d0350eb09f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -616,6 +616,7 @@ All notable changes to this project will be documented in this file. [`absurd_extreme_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#absurd_extreme_comparisons [`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped [`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant +[`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants [`assign_op_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern [`assign_ops`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_ops [`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask @@ -679,10 +680,8 @@ All notable changes to this project will be documented in this file. [`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_false`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_false [`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_true`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_true [`explicit_write`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_write [`extend_from_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#extend_from_slice [`extra_unused_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#extra_unused_lifetimes diff --git a/README.md b/README.md index 658b2a70adc2b..751f0add8eb50 100644 --- a/README.md +++ b/README.md @@ -8,11 +8,11 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. <<<<<<< HEAD -[There are 290 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) ||||||| merged common ancestors [There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) ======= -[There are 293 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) >>>>>>> run ./util/dev update_lints We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/assert_checks.rs b/clippy_lints/src/assertions_on_constants.rs similarity index 58% rename from clippy_lints/src/assert_checks.rs rename to clippy_lints/src/assertions_on_constants.rs index dcc11951b267d..0068d6fa15781 100644 --- a/clippy_lints/src/assert_checks.rs +++ b/clippy_lints/src/assertions_on_constants.rs @@ -11,50 +11,40 @@ use crate::rustc::hir::{Expr, ExprKind}; use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use crate::rustc::{declare_tool_lint, lint_array}; use crate::syntax::ast::LitKind; -use crate::utils::{is_direct_expn_of, span_lint}; +use crate::utils::{is_direct_expn_of, span_lint, span_lint_and_sugg}; +use rustc_errors::Applicability; use if_chain::if_chain; -/// **What it does:** Check explicit call assert!(true) +/// **What it does:** Check explicit call assert!(true/false) /// -/// **Why is this bad?** Will be optimized out by the compiler -/// -/// **Known problems:** None -/// -/// **Example:** -/// ```rust -/// assert!(true) -/// ``` -declare_clippy_lint! { - pub EXPLICIT_TRUE, - correctness, - "assert!(true) will be optimized out by the compiler" -} - -/// **What it does:** Check explicit call assert!(false) -/// -/// **Why is this bad?** Should probably be replaced by a panic!() or unreachable!() +/// **Why is this bad?** Will be optimized out by the compiler or should probably be replaced by a panic!() or unreachable!() /// /// **Known problems:** None /// /// **Example:** /// ```rust /// assert!(false) +/// // or +/// assert!(true) +/// // or +/// const B: bool = false; +/// assert!(B) /// ``` declare_clippy_lint! { - pub EXPLICIT_FALSE, - correctness, - "assert!(false) should probably be replaced by a panic!() or unreachable!()" + pub ASSERTIONS_ON_CONSTANTS, + style, + "assert!(true/false) will be optimized out by the compiler/should probably be replaced by a panic!() or unreachable!()" } -pub struct AssertChecks; +pub struct AssertionsOnConstants; -impl LintPass for AssertChecks { +impl LintPass for AssertionsOnConstants { fn get_lints(&self) -> LintArray { - lint_array![EXPLICIT_TRUE, EXPLICIT_FALSE] + lint_array![ASSERTIONS_ON_CONSTANTS] } } -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertChecks { +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { if_chain! { if is_direct_expn_of(e.span, "assert").is_some(); @@ -63,12 +53,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertChecks { then { match inner.node { LitKind::Bool(true) => { - span_lint(cx, EXPLICIT_TRUE, e.span, + span_lint(cx, ASSERTIONS_ON_CONSTANTS, e.span, "assert!(true) will be optimized out by the compiler"); }, LitKind::Bool(false) => { - span_lint(cx, EXPLICIT_FALSE, e.span, - "assert!(false) should probably be replaced by a panic!() or unreachable!()"); + span_lint_and_sugg( + cx, + ASSERTIONS_ON_CONSTANTS, + e.span, + "assert!(false) should probably be replaced", + "try", + "panic!()".to_string(), + Applicability::MachineApplicable); }, _ => (), } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 8af73a4f99b28..192c9226b699d 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -88,7 +88,7 @@ mod utils; // begin lints modules, do not remove this comment, it’s used in `update_lints` pub mod approx_const; pub mod arithmetic; -pub mod assert_checks; +pub mod assertions_on_constants; pub mod assign_ops; pub mod attrs; pub mod bit_mask; @@ -487,7 +487,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box ptr_offset_with_cast::Pass); reg.register_late_lint_pass(box redundant_clone::RedundantClone); reg.register_late_lint_pass(box slow_vector_initialization::Pass); - reg.register_late_lint_pass(box assert_checks::AssertChecks); + reg.register_late_lint_pass(box assertions_on_constants::AssertionsOnConstants); reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![ arithmetic::FLOAT_ARITHMETIC, @@ -565,8 +565,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_lint_group("clippy::all", Some("clippy"), vec![ approx_const::APPROX_CONSTANT, - assert_checks::EXPLICIT_FALSE, - assert_checks::EXPLICIT_TRUE, + assertions_on_constants::ASSERTIONS_ON_CONSTANTS, assign_ops::ASSIGN_OP_PATTERN, assign_ops::MISREFACTORED_ASSIGN_OP, attrs::DEPRECATED_CFG_ATTR, @@ -788,6 +787,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { ]); reg.register_lint_group("clippy::style", Some("clippy_style"), vec![ + assertions_on_constants::ASSERTIONS_ON_CONSTANTS, assign_ops::ASSIGN_OP_PATTERN, attrs::UNKNOWN_CLIPPY_LINTS, bit_mask::VERBOSE_BIT_MASK, @@ -944,8 +944,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_lint_group("clippy::correctness", Some("clippy_correctness"), vec![ approx_const::APPROX_CONSTANT, - assert_checks::EXPLICIT_FALSE, - assert_checks::EXPLICIT_TRUE, attrs::DEPRECATED_SEMVER, attrs::USELESS_ATTRIBUTE, bit_mask::BAD_BIT_MASK, diff --git a/tests/ui/assert_checks.stderr b/tests/ui/assert_checks.stderr deleted file mode 100644 index e203994165061..0000000000000 --- a/tests/ui/assert_checks.stderr +++ /dev/null @@ -1,18 +0,0 @@ -error: assert!(true) will be optimized out by the compiler - --> $DIR/assert_checks.rs:11:5 - | -11 | assert!(true); - | ^^^^^^^^^^^^^^ - | - = note: #[deny(clippy::explicit_true)] on by default - -error: assert!(false) should probably be replaced by a panic!() or unreachable!() - --> $DIR/assert_checks.rs:12:5 - | -12 | assert!(false); - | ^^^^^^^^^^^^^^^ - | - = note: #[deny(clippy::explicit_false)] on by default - -error: aborting due to 2 previous errors - diff --git a/tests/ui/assert_checks.rs b/tests/ui/assertions_on_constants.rs similarity index 100% rename from tests/ui/assert_checks.rs rename to tests/ui/assertions_on_constants.rs diff --git a/tests/ui/assertions_on_constants.stderr b/tests/ui/assertions_on_constants.stderr new file mode 100644 index 0000000000000..33104ed2066a7 --- /dev/null +++ b/tests/ui/assertions_on_constants.stderr @@ -0,0 +1,16 @@ +error: assert!(true) will be optimized out by the compiler + --> $DIR/assertions_on_constants.rs:11:5 + | +LL | assert!(true); + | ^^^^^^^^^^^^^^ + | + = note: `-D clippy::assertions-on-constants` implied by `-D warnings` + +error: assert!(false) should probably be replaced + --> $DIR/assertions_on_constants.rs:12:5 + | +LL | assert!(false); + | ^^^^^^^^^^^^^^^ help: try: `panic!()` + +error: aborting due to 2 previous errors + diff --git a/tests/ui/attrs.rs b/tests/ui/attrs.rs index c0ea732971810..2c7f67d4505a8 100644 --- a/tests/ui/attrs.rs +++ b/tests/ui/attrs.rs @@ -8,7 +8,7 @@ // except according to those terms. #![warn(clippy::inline_always, clippy::deprecated_semver)] -#![allow(clippy::assert_checks::explicit_true)] +#![allow(clippy::assertions_on_constants::assertions_on_constants)] #[inline(always)] fn test_attr_lint() { assert!(true) diff --git a/tests/ui/empty_line_after_outer_attribute.rs b/tests/ui/empty_line_after_outer_attribute.rs index a6e6adcac5c1b..2bb5037614db3 100644 --- a/tests/ui/empty_line_after_outer_attribute.rs +++ b/tests/ui/empty_line_after_outer_attribute.rs @@ -8,7 +8,7 @@ // except according to those terms. #![warn(clippy::empty_line_after_outer_attr)] -#![allow(clippy::assert_checks::explicit_true)] +#![allow(clippy::assertions_on_constants::assertions_on_constants)] // This should produce a warning #[crate_type = "lib"] diff --git a/tests/ui/panic_unimplemented.rs b/tests/ui/panic_unimplemented.rs index 3c568658a6cf4..309a22dc2151a 100644 --- a/tests/ui/panic_unimplemented.rs +++ b/tests/ui/panic_unimplemented.rs @@ -8,7 +8,7 @@ // except according to those terms. #![warn(clippy::panic_params, clippy::unimplemented)] -#![allow(clippy::assert_checks::explicit_true)] +#![allow(clippy::assertions_on_constants::assertions_on_constants)] fn missing() { if true { panic!("{}"); From a9f8d3c8fdc18080df800e65d73a737ccfd08933 Mon Sep 17 00:00:00 2001 From: "A.A.Abroskin" Date: Wed, 9 Jan 2019 21:30:47 +0300 Subject: [PATCH 5/7] add assert(true/false, some message) tests --- clippy_lints/src/assertions_on_constants.rs | 55 +++++++++++++-------- tests/ui/assertions_on_constants.rs | 8 +++ tests/ui/assertions_on_constants.stderr | 39 ++++++++++++++- 3 files changed, 79 insertions(+), 23 deletions(-) diff --git a/clippy_lints/src/assertions_on_constants.rs b/clippy_lints/src/assertions_on_constants.rs index 0068d6fa15781..f88ef8e83edb7 100644 --- a/clippy_lints/src/assertions_on_constants.rs +++ b/clippy_lints/src/assertions_on_constants.rs @@ -7,17 +7,18 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use crate::consts::{constant, Constant}; use crate::rustc::hir::{Expr, ExprKind}; use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use crate::rustc::{declare_tool_lint, lint_array}; use crate::syntax::ast::LitKind; -use crate::utils::{is_direct_expn_of, span_lint, span_lint_and_sugg}; -use rustc_errors::Applicability; +use crate::utils::{is_direct_expn_of, span_help_and_lint}; use if_chain::if_chain; -/// **What it does:** Check explicit call assert!(true/false) +/// **What it does:** Check to call assert!(true/false) /// -/// **Why is this bad?** Will be optimized out by the compiler or should probably be replaced by a panic!() or unreachable!() +/// **Why is this bad?** Will be optimized out by the compiler or should probably be replaced by a +/// panic!() or unreachable!() /// /// **Known problems:** None /// @@ -49,24 +50,36 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants { if_chain! { if is_direct_expn_of(e.span, "assert").is_some(); if let ExprKind::Unary(_, ref lit) = e.node; - if let ExprKind::Lit(ref inner) = lit.node; then { - match inner.node { - LitKind::Bool(true) => { - span_lint(cx, ASSERTIONS_ON_CONSTANTS, e.span, - "assert!(true) will be optimized out by the compiler"); - }, - LitKind::Bool(false) => { - span_lint_and_sugg( - cx, - ASSERTIONS_ON_CONSTANTS, - e.span, - "assert!(false) should probably be replaced", - "try", - "panic!()".to_string(), - Applicability::MachineApplicable); - }, - _ => (), + if let ExprKind::Lit(ref inner) = lit.node { + match inner.node { + LitKind::Bool(true) => { + span_help_and_lint(cx, ASSERTIONS_ON_CONSTANTS, e.span, + "assert!(true) will be optimized out by the compiler", + "remove it"); + }, + LitKind::Bool(false) => { + span_help_and_lint( + cx, ASSERTIONS_ON_CONSTANTS, e.span, + "assert!(false) should probably be replaced", + "use panic!() or unreachable!()"); + }, + _ => (), + } + } else if let Some(bool_const) = constant(cx, cx.tables, lit) { + match bool_const.0 { + Constant::Bool(true) => { + span_help_and_lint(cx, ASSERTIONS_ON_CONSTANTS, e.span, + "assert!(const: true) will be optimized out by the compiler", + "remove it"); + }, + Constant::Bool(false) => { + span_help_and_lint(cx, ASSERTIONS_ON_CONSTANTS, e.span, + "assert!(const: false) should probably be replaced", + "use panic!() or unreachable!()"); + }, + _ => (), + } } } } diff --git a/tests/ui/assertions_on_constants.rs b/tests/ui/assertions_on_constants.rs index 811046d060a25..dcefe83f8c2f9 100644 --- a/tests/ui/assertions_on_constants.rs +++ b/tests/ui/assertions_on_constants.rs @@ -10,4 +10,12 @@ fn main() { assert!(true); assert!(false); + assert!(true, "true message"); + assert!(false, "false message"); + + const B: bool = true; + assert!(B); + + const C: bool = false; + assert!(C); } diff --git a/tests/ui/assertions_on_constants.stderr b/tests/ui/assertions_on_constants.stderr index 33104ed2066a7..1f1a80e0e77b9 100644 --- a/tests/ui/assertions_on_constants.stderr +++ b/tests/ui/assertions_on_constants.stderr @@ -5,12 +5,47 @@ LL | assert!(true); | ^^^^^^^^^^^^^^ | = note: `-D clippy::assertions-on-constants` implied by `-D warnings` + = help: remove it error: assert!(false) should probably be replaced --> $DIR/assertions_on_constants.rs:12:5 | LL | assert!(false); - | ^^^^^^^^^^^^^^^ help: try: `panic!()` + | ^^^^^^^^^^^^^^^ + | + = help: use panic!() or unreachable!() + +error: assert!(true) will be optimized out by the compiler + --> $DIR/assertions_on_constants.rs:13:5 + | +LL | assert!(true, "true message"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: remove it + +error: assert!(false) should probably be replaced + --> $DIR/assertions_on_constants.rs:14:5 + | +LL | assert!(false, "false message"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: use panic!() or unreachable!() + +error: assert!(const: true) will be optimized out by the compiler + --> $DIR/assertions_on_constants.rs:17:5 + | +LL | assert!(B); + | ^^^^^^^^^^^ + | + = help: remove it + +error: assert!(const: false) should probably be replaced + --> $DIR/assertions_on_constants.rs:20:5 + | +LL | assert!(C); + | ^^^^^^^^^^^ + | + = help: use panic!() or unreachable!() -error: aborting due to 2 previous errors +error: aborting due to 6 previous errors From 58abdb591884bcde77d53a5933a78c89e26c6752 Mon Sep 17 00:00:00 2001 From: "A.A.Abroskin" Date: Wed, 9 Jan 2019 21:31:29 +0300 Subject: [PATCH 6/7] run ./util/dev update_lints --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8ca10da416ddb..3a7e9a165c167 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 292 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: From c771f339d73e2a02c5a61734ffa3106721407265 Mon Sep 17 00:00:00 2001 From: "A.A.Abroskin" Date: Wed, 23 Jan 2019 11:49:02 +0300 Subject: [PATCH 7/7] allow assertions_on_constants for collapsible_if and missing_test_files --- clippy_lints/src/assertions_on_constants.rs | 9 --------- tests/missing-test-files.rs | 2 ++ tests/ui/assertions_on_constants.rs | 9 --------- tests/ui/assertions_on_constants.stderr | 12 ++++++------ tests/ui/attrs.rs | 2 +- tests/ui/collapsible_if.fixed | 2 +- tests/ui/collapsible_if.rs | 2 +- tests/ui/empty_line_after_outer_attribute.rs | 2 +- tests/ui/panic_unimplemented.rs | 2 +- 9 files changed, 13 insertions(+), 29 deletions(-) diff --git a/clippy_lints/src/assertions_on_constants.rs b/clippy_lints/src/assertions_on_constants.rs index f88ef8e83edb7..a148cb1c3a6f2 100644 --- a/clippy_lints/src/assertions_on_constants.rs +++ b/clippy_lints/src/assertions_on_constants.rs @@ -1,12 +1,3 @@ -// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - use crate::consts::{constant, Constant}; use crate::rustc::hir::{Expr, ExprKind}; use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; diff --git a/tests/missing-test-files.rs b/tests/missing-test-files.rs index 558e001d3d10b..bd0cee7564442 100644 --- a/tests/missing-test-files.rs +++ b/tests/missing-test-files.rs @@ -1,3 +1,5 @@ +#![allow(clippy::assertions_on_constants)] + use std::fs::{self, DirEntry}; use std::path::Path; diff --git a/tests/ui/assertions_on_constants.rs b/tests/ui/assertions_on_constants.rs index dcefe83f8c2f9..daeceebd3a2c7 100644 --- a/tests/ui/assertions_on_constants.rs +++ b/tests/ui/assertions_on_constants.rs @@ -1,12 +1,3 @@ -// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - fn main() { assert!(true); assert!(false); diff --git a/tests/ui/assertions_on_constants.stderr b/tests/ui/assertions_on_constants.stderr index 1f1a80e0e77b9..e8001acceb1ec 100644 --- a/tests/ui/assertions_on_constants.stderr +++ b/tests/ui/assertions_on_constants.stderr @@ -1,5 +1,5 @@ error: assert!(true) will be optimized out by the compiler - --> $DIR/assertions_on_constants.rs:11:5 + --> $DIR/assertions_on_constants.rs:2:5 | LL | assert!(true); | ^^^^^^^^^^^^^^ @@ -8,7 +8,7 @@ LL | assert!(true); = help: remove it error: assert!(false) should probably be replaced - --> $DIR/assertions_on_constants.rs:12:5 + --> $DIR/assertions_on_constants.rs:3:5 | LL | assert!(false); | ^^^^^^^^^^^^^^^ @@ -16,7 +16,7 @@ LL | assert!(false); = help: use panic!() or unreachable!() error: assert!(true) will be optimized out by the compiler - --> $DIR/assertions_on_constants.rs:13:5 + --> $DIR/assertions_on_constants.rs:4:5 | LL | assert!(true, "true message"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -24,7 +24,7 @@ LL | assert!(true, "true message"); = help: remove it error: assert!(false) should probably be replaced - --> $DIR/assertions_on_constants.rs:14:5 + --> $DIR/assertions_on_constants.rs:5:5 | LL | assert!(false, "false message"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -32,7 +32,7 @@ LL | assert!(false, "false message"); = help: use panic!() or unreachable!() error: assert!(const: true) will be optimized out by the compiler - --> $DIR/assertions_on_constants.rs:17:5 + --> $DIR/assertions_on_constants.rs:8:5 | LL | assert!(B); | ^^^^^^^^^^^ @@ -40,7 +40,7 @@ LL | assert!(B); = help: remove it error: assert!(const: false) should probably be replaced - --> $DIR/assertions_on_constants.rs:20:5 + --> $DIR/assertions_on_constants.rs:11:5 | LL | assert!(C); | ^^^^^^^^^^^ diff --git a/tests/ui/attrs.rs b/tests/ui/attrs.rs index df7eafc65518b..91b65a43be77f 100644 --- a/tests/ui/attrs.rs +++ b/tests/ui/attrs.rs @@ -1,5 +1,5 @@ #![warn(clippy::inline_always, clippy::deprecated_semver)] -#![allow(clippy::assertions_on_constants::assertions_on_constants)] +#![allow(clippy::assertions_on_constants)] #[inline(always)] fn test_attr_lint() { assert!(true) diff --git a/tests/ui/collapsible_if.fixed b/tests/ui/collapsible_if.fixed index 2c6dd95a63799..3c7de56406eee 100644 --- a/tests/ui/collapsible_if.fixed +++ b/tests/ui/collapsible_if.fixed @@ -1,5 +1,5 @@ // run-rustfix -#![allow(clippy::cyclomatic_complexity)] +#![allow(clippy::cyclomatic_complexity, clippy::assertions_on_constants)] #[rustfmt::skip] #[warn(clippy::collapsible_if)] diff --git a/tests/ui/collapsible_if.rs b/tests/ui/collapsible_if.rs index f482d7704def9..e46d753757741 100644 --- a/tests/ui/collapsible_if.rs +++ b/tests/ui/collapsible_if.rs @@ -1,5 +1,5 @@ // run-rustfix -#![allow(clippy::cyclomatic_complexity)] +#![allow(clippy::cyclomatic_complexity, clippy::assertions_on_constants)] #[rustfmt::skip] #[warn(clippy::collapsible_if)] diff --git a/tests/ui/empty_line_after_outer_attribute.rs b/tests/ui/empty_line_after_outer_attribute.rs index 3af8e3eeac087..b43dc20082523 100644 --- a/tests/ui/empty_line_after_outer_attribute.rs +++ b/tests/ui/empty_line_after_outer_attribute.rs @@ -1,5 +1,5 @@ #![warn(clippy::empty_line_after_outer_attr)] -#![allow(clippy::assertions_on_constants::assertions_on_constants)] +#![allow(clippy::assertions_on_constants)] // This should produce a warning #[crate_type = "lib"] diff --git a/tests/ui/panic_unimplemented.rs b/tests/ui/panic_unimplemented.rs index a7c5b91fdb560..92290da8a6ac0 100644 --- a/tests/ui/panic_unimplemented.rs +++ b/tests/ui/panic_unimplemented.rs @@ -1,5 +1,5 @@ #![warn(clippy::panic_params, clippy::unimplemented)] -#![allow(clippy::assertions_on_constants::assertions_on_constants)] +#![allow(clippy::assertions_on_constants)] fn missing() { if true { panic!("{}");