From e7352877c5cad5c06ce011643ef9e3995f097575 Mon Sep 17 00:00:00 2001 From: Theemathas Chirananthavat Date: Wed, 28 Dec 2016 10:53:33 -0800 Subject: [PATCH 1/5] Add test for double_parens lint. --- tests/compile-fail/double_parens.rs | 51 +++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 tests/compile-fail/double_parens.rs diff --git a/tests/compile-fail/double_parens.rs b/tests/compile-fail/double_parens.rs new file mode 100644 index 00000000000..2e415d7232d --- /dev/null +++ b/tests/compile-fail/double_parens.rs @@ -0,0 +1,51 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#![deny(double_parens)] +#![allow(dead_code)] + +fn dummy_fn(_: T) {} + +struct DummyStruct; + +impl DummyStruct { + fn dummy_method(self, _: T) {} +} + +fn simple_double_parens() -> i32 { + ((0)) //~ERROR Consider removing unnecessary double parentheses +} + +fn fn_double_parens() { + dummy_fn((0)); //~ERROR Consider removing unnecessary double parentheses +} + +fn method_double_parens(x: DummyStruct) { + x.dummy_method((0)); //~ERROR Consider removing unnecessary double parentheses +} + +fn tuple_double_parens() -> (i32, i32) { + ((1, 2)) //~ERROR Consider removing unnecessary double parentheses +} + +fn unit_double_parens() { + (()) //~ERROR Consider removing unnecessary double parentheses +} + +fn fn_tuple_ok() { + dummy_fn((1, 2)); +} + +fn method_tuple_ok(x: DummyStruct) { + x.dummy_method((1, 2)); +} + +fn fn_unit_ok() { + dummy_fn(()); +} + +fn method_unit_ok(x: DummyStruct) { + x.dummy_method(()); +} + +fn main() {} From d76fa3dfd949bfbf456514da02efbc9a51cf061b Mon Sep 17 00:00:00 2001 From: Theemathas Chirananthavat Date: Wed, 28 Dec 2016 10:54:23 -0800 Subject: [PATCH 2/5] Add skeleton for double_parens lint. --- clippy_lints/src/double_parens.rs | 35 +++++++++++++++++++++++++++++++ clippy_lints/src/lib.rs | 3 +++ 2 files changed, 38 insertions(+) create mode 100644 clippy_lints/src/double_parens.rs diff --git a/clippy_lints/src/double_parens.rs b/clippy_lints/src/double_parens.rs new file mode 100644 index 00000000000..85611cd6c85 --- /dev/null +++ b/clippy_lints/src/double_parens.rs @@ -0,0 +1,35 @@ +use syntax::ast::*; +use rustc::lint::{EarlyContext, LintArray, LintPass, EarlyLintPass}; + +/// **What it does:** Checks for unnecessary double parentheses. +/// +/// **Why is this bad?** This makes code harder to read and might indicate a +/// mistake. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// ((0)) +/// foo((0)) +/// ((1, 2)) +/// ``` +declare_lint! { + pub DOUBLE_PARENS, Warn, + "Warn on unnecessary double parentheses" +} + +#[derive(Copy, Clone)] +pub struct DoubleParens; + +impl LintPass for DoubleParens { + fn get_lints(&self) -> LintArray { + lint_array!(DOUBLE_PARENS) + } +} + +impl EarlyLintPass for DoubleParens { + fn check_expr(&mut self, cx: &EarlyContext, expr: &Expr) { + // insert check here. + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 06aab286b2f..ac92d5ec97b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -69,6 +69,7 @@ pub mod copies; pub mod cyclomatic_complexity; pub mod derive; pub mod doc; +pub mod double_parens; pub mod drop_ref; pub mod entry; pub mod enum_clike; @@ -283,6 +284,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box if_let_redundant_pattern_matching::Pass); reg.register_late_lint_pass(box partialeq_ne_impl::Pass); reg.register_early_lint_pass(box reference::Pass); + reg.register_early_lint_pass(box double_parens::DoubleParens); reg.register_lint_group("clippy_restrictions", vec![ arithmetic::FLOAT_ARITHMETIC, @@ -355,6 +357,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { derive::DERIVE_HASH_XOR_EQ, derive::EXPL_IMPL_CLONE_ON_COPY, doc::DOC_MARKDOWN, + double_parens::DOUBLE_PARENS, drop_ref::DROP_REF, entry::MAP_ENTRY, enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT, From e1c540bfd1a843b102a5d7cdc5818525c28a5f15 Mon Sep 17 00:00:00 2001 From: Theemathas Chirananthavat Date: Wed, 28 Dec 2016 12:03:49 -0800 Subject: [PATCH 3/5] Implement the double_parens lint. --- clippy_lints/src/double_parens.rs | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/double_parens.rs b/clippy_lints/src/double_parens.rs index 85611cd6c85..a5b469d9c6c 100644 --- a/clippy_lints/src/double_parens.rs +++ b/clippy_lints/src/double_parens.rs @@ -1,5 +1,5 @@ use syntax::ast::*; -use rustc::lint::{EarlyContext, LintArray, LintPass, EarlyLintPass}; +use rustc::lint::{EarlyContext, LintContext, LintArray, LintPass, EarlyLintPass}; /// **What it does:** Checks for unnecessary double parentheses. /// @@ -30,6 +30,33 @@ impl LintPass for DoubleParens { impl EarlyLintPass for DoubleParens { fn check_expr(&mut self, cx: &EarlyContext, expr: &Expr) { - // insert check here. + match expr.node { + ExprKind::Paren(ref in_paren) => { + match in_paren.node { + ExprKind::Paren(_) | + ExprKind::Tup(_) => { + cx.span_lint(DOUBLE_PARENS, expr.span, "Consider removing unnecessary double parentheses"); + }, + _ => {}, + } + }, + ExprKind::Call(_, ref params) => { + if params.len() == 1 { + let param = ¶ms[0]; + if let ExprKind::Paren(_) = param.node { + cx.span_lint(DOUBLE_PARENS, param.span, "Consider removing unnecessary double parentheses"); + } + } + }, + ExprKind::MethodCall(_, _, ref params) => { + if params.len() == 2 { + let param = ¶ms[1]; + if let ExprKind::Paren(_) = param.node { + cx.span_lint(DOUBLE_PARENS, param.span, "Consider removing unnecessary double parentheses"); + } + } + }, + _ => {}, + } } } From c0c20145d187b100c6137f41f1f2564a3943651e Mon Sep 17 00:00:00 2001 From: Theemathas Chirananthavat Date: Wed, 28 Dec 2016 12:04:46 -0800 Subject: [PATCH 4/5] Fix compile-fail tests by allowing double_parens. --- tests/compile-fail/eq_op.rs | 2 +- tests/compile-fail/identity_op.rs | 2 +- tests/compile-fail/reference.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/compile-fail/eq_op.rs b/tests/compile-fail/eq_op.rs index f0502a71796..0ec86157d80 100644 --- a/tests/compile-fail/eq_op.rs +++ b/tests/compile-fail/eq_op.rs @@ -2,7 +2,7 @@ #![plugin(clippy)] #[deny(eq_op)] -#[allow(identity_op)] +#[allow(identity_op, double_parens)] #[allow(no_effect, unused_variables, unnecessary_operation)] #[deny(nonminimal_bool)] fn main() { diff --git a/tests/compile-fail/identity_op.rs b/tests/compile-fail/identity_op.rs index 329c4a6bbf4..6a0b5e927cf 100644 --- a/tests/compile-fail/identity_op.rs +++ b/tests/compile-fail/identity_op.rs @@ -5,7 +5,7 @@ const ONE : i64 = 1; const NEG_ONE : i64 = -1; const ZERO : i64 = 0; -#[allow(eq_op, no_effect, unnecessary_operation)] +#[allow(eq_op, no_effect, unnecessary_operation, double_parens)] #[deny(identity_op)] fn main() { let x = 0; diff --git a/tests/compile-fail/reference.rs b/tests/compile-fail/reference.rs index b77afbc1270..789425e71fe 100644 --- a/tests/compile-fail/reference.rs +++ b/tests/compile-fail/reference.rs @@ -9,7 +9,7 @@ fn get_reference(n : &usize) -> &usize { n } -#[allow(many_single_char_names)] +#[allow(many_single_char_names, double_parens)] #[allow(unused_variables)] #[deny(deref_addrof)] fn main() { From 7034d169e5e04f8cd13c3bbc5a83127add0f85f8 Mon Sep 17 00:00:00 2001 From: Theemathas Chirananthavat Date: Wed, 28 Dec 2016 12:06:43 -0800 Subject: [PATCH 5/5] Update lint documentation using util/update_lints.py --- CHANGELOG.md | 1 + README.md | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51381a6eb02..7eb5df200f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -268,6 +268,7 @@ All notable changes to this project will be documented in this file. [`diverging_sub_expression`]: https://github.com/Manishearth/rust-clippy/wiki#diverging_sub_expression [`doc_markdown`]: https://github.com/Manishearth/rust-clippy/wiki#doc_markdown [`double_neg`]: https://github.com/Manishearth/rust-clippy/wiki#double_neg +[`double_parens`]: https://github.com/Manishearth/rust-clippy/wiki#double_parens [`drop_ref`]: https://github.com/Manishearth/rust-clippy/wiki#drop_ref [`duplicate_underscore_argument`]: https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument [`empty_loop`]: https://github.com/Manishearth/rust-clippy/wiki#empty_loop diff --git a/README.md b/README.md index 15751a0d3d0..94017cd53c6 100644 --- a/README.md +++ b/README.md @@ -179,7 +179,7 @@ transparently: ## Lints -There are 180 lints included in this crate: +There are 181 lints included in this crate: name | default | triggers on -----------------------------------------------------------------------------------------------------------------------|---------|---------------------------------------------------------------------------------------------------------------------------------- @@ -216,6 +216,7 @@ name [diverging_sub_expression](https://github.com/Manishearth/rust-clippy/wiki#diverging_sub_expression) | warn | whether an expression contains a diverging sub expression [doc_markdown](https://github.com/Manishearth/rust-clippy/wiki#doc_markdown) | warn | presence of `_`, `::` or camel-case outside backticks in documentation [double_neg](https://github.com/Manishearth/rust-clippy/wiki#double_neg) | warn | `--x`, which is a double negation of `x` and not a pre-decrement as in C/C++ +[double_parens](https://github.com/Manishearth/rust-clippy/wiki#double_parens) | warn | Warn on unnecessary double parentheses [drop_ref](https://github.com/Manishearth/rust-clippy/wiki#drop_ref) | warn | calls to `std::mem::drop` with a reference instead of an owned value [duplicate_underscore_argument](https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument) | warn | function arguments having names which only differ by an underscore [empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop) | warn | empty `loop {}`, which should block or sleep