Skip to content

Commit

Permalink
Changed Cognitive Complexity as per issue 3793
Browse files Browse the repository at this point in the history
* CoC is now a pre-expansion Early pass.
* The full draft of issue 3793 has been implemented.
* `compile-test.rs` now allows `clippy::cognitive_complexity` in ui tests by default.
  This is because most ui tests don't use functions in a standard manner, and tend to have rather large ones.
* Tests for CoC have been updated.
  • Loading branch information
felix91gr committed Apr 24, 2019
1 parent fbb3a47 commit 508115e
Show file tree
Hide file tree
Showing 213 changed files with 3,253 additions and 3,522 deletions.
3 changes: 3 additions & 0 deletions .cargo/config
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
[alias]
uitest = "test --test compile-test"

[build]
rustflags = ["-Zunstable-options"]
28 changes: 28 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<!--
Thank you for making Clippy better!

We're collecting our changelog from pull request descriptions.
If your PR only updates to the latest nightly, you can leave the
`changelog` entry as `none`. Otherwise, please write a short comment
explaining your change.

If your PR fixes an issue, you can add "fixes #issue_number" into this
PR description. This way the issue will be automatically closed when
your PR is merged.

If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.

- [ ] Followed [lint naming conventions][lint_naming]
- [ ] Added passing UI tests (including committed `.stderr` file)
- [ ] `cargo test` passes locally
- [ ] Executed `util/dev update_lints`
- [ ] Added lint documentation
- [ ] Run `cargo fmt`

Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.

Delete this line and everything above before opening your PR -->

changelog: none
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,7 @@ All notable changes to this project will be documented in this file.
[`panic_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_params
[`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap
[`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl
[`path_buf_push_overwrite`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_buf_push_overwrite
[`possible_missing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#possible_missing_comma
[`precedence`]: https://rust-lang.github.io/rust-clippy/master/index.html#precedence
[`print_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_literal
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ rustc_tools_util = { version = "0.1.1", path = "rustc_tools_util"}

[dev-dependencies]
cargo_metadata = "0.7.1"
compiletest_rs = { version = "0.3.21", features = ["tmp"] }
compiletest_rs = { version = "0.3.22", features = ["tmp"] }
lazy_static = "1.0"
serde_derive = "1.0"
clippy-mini-macro-test = { version = "0.2", path = "mini-macro" }
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 298 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 299 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
1 change: 0 additions & 1 deletion clippy_dev/rust-toolchain

This file was deleted.

2 changes: 0 additions & 2 deletions clippy_dev/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![allow(clippy::default_hash_types)]

use itertools::Itertools;
use lazy_static::lazy_static;
use regex::Regex;
Expand Down
17 changes: 3 additions & 14 deletions clippy_lints/src/approx_const.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::utils::span_lint;
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::{declare_tool_lint, lint_array};
use rustc::{declare_lint_pass, declare_tool_lint};
use std::f64::consts as f64;
use syntax::ast::{FloatTy, Lit, LitKind};
use syntax::symbol;
Expand Down Expand Up @@ -53,20 +53,9 @@ const KNOWN_CONSTS: &[(f64, &str, usize)] = &[
(f64::SQRT_2, "SQRT_2", 5),
];

#[derive(Copy, Clone)]
pub struct Pass;
declare_lint_pass!(ApproxConstant => [APPROX_CONSTANT]);

impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(APPROX_CONSTANT)
}

fn name(&self) -> &'static str {
"ApproxConstant"
}
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ApproxConstant {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
if let ExprKind::Lit(lit) = &e.node {
check_lit(cx, lit, e);
Expand Down
12 changes: 2 additions & 10 deletions clippy_lints/src/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::consts::constant_simple;
use crate::utils::span_lint;
use rustc::hir;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::{declare_tool_lint, lint_array};
use rustc::{declare_tool_lint, impl_lint_pass};
use syntax::source_map::Span;

declare_clippy_lint! {
Expand Down Expand Up @@ -48,15 +48,7 @@ pub struct Arithmetic {
const_span: Option<Span>,
}

impl LintPass for Arithmetic {
fn get_lints(&self) -> LintArray {
lint_array!(INTEGER_ARITHMETIC, FLOAT_ARITHMETIC)
}

fn name(&self) -> &'static str {
"Arithmetic"
}
}
impl_lint_pass!(Arithmetic => [INTEGER_ARITHMETIC, FLOAT_ARITHMETIC]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Arithmetic {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) {
Expand Down
75 changes: 32 additions & 43 deletions clippy_lints/src/assertions_on_constants.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use if_chain::if_chain;
use rustc::hir::{Expr, ExprKind};
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::{declare_tool_lint, lint_array};
use rustc::{declare_lint_pass, declare_tool_lint};
use syntax_pos::Span;

use crate::consts::{constant, Constant};
use crate::syntax::ast::LitKind;
use crate::utils::{in_macro, is_direct_expn_of, span_help_and_lint};

declare_clippy_lint! {
Expand All @@ -29,55 +29,44 @@ declare_clippy_lint! {
"`assert!(true)` / `assert!(false)` will be optimized out by the compiler, and should probably be replaced by a `panic!()` or `unreachable!()`"
}

pub struct AssertionsOnConstants;

impl LintPass for AssertionsOnConstants {
fn get_lints(&self) -> LintArray {
lint_array![ASSERTIONS_ON_CONSTANTS]
}

fn name(&self) -> &'static str {
"AssertionsOnConstants"
}
}
declare_lint_pass!(AssertionsOnConstants => [ASSERTIONS_ON_CONSTANTS]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
let mut is_debug_assert = false;
let debug_assert_not_in_macro = |span: Span| {
is_debug_assert = true;
// Check that `debug_assert!` itself is not inside a macro
!in_macro(span)
};
if_chain! {
if let Some(assert_span) = is_direct_expn_of(e.span, "assert");
if !in_macro(assert_span)
|| is_direct_expn_of(assert_span, "debug_assert").map_or(false, |span| !in_macro(span));
|| is_direct_expn_of(assert_span, "debug_assert")
.map_or(false, debug_assert_not_in_macro);
if let ExprKind::Unary(_, ref lit) = e.node;
if let Some(bool_const) = constant(cx, cx.tables, lit);
then {
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!()");
},
_ => (),
}
match bool_const.0 {
Constant::Bool(true) => {
span_help_and_lint(
cx,
ASSERTIONS_ON_CONSTANTS,
e.span,
"`assert!(true)` will be optimized out by the compiler",
"remove it"
);
},
Constant::Bool(false) if !is_debug_assert => {
span_help_and_lint(
cx,
ASSERTIONS_ON_CONSTANTS,
e.span,
"`assert!(false)` should probably be replaced",
"use `panic!()` or `unreachable!()`"
);
},
_ => (),
}
}
}
Expand Down
15 changes: 2 additions & 13 deletions clippy_lints/src/assign_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use if_chain::if_chain;
use rustc::hir;
use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::{declare_tool_lint, lint_array};
use rustc::{declare_lint_pass, declare_tool_lint};
use rustc_errors::Applicability;

use crate::utils::{
Expand Down Expand Up @@ -53,18 +53,7 @@ declare_clippy_lint! {
"having a variable on both sides of an assign op"
}

#[derive(Copy, Clone, Default)]
pub struct AssignOps;

impl LintPass for AssignOps {
fn get_lints(&self) -> LintArray {
lint_array!(ASSIGN_OP_PATTERN, MISREFACTORED_ASSIGN_OP)
}

fn name(&self) -> &'static str {
"AssignOps"
}
}
declare_lint_pass!(AssignOps => [ASSIGN_OP_PATTERN, MISREFACTORED_ASSIGN_OP]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps {
#[allow(clippy::too_many_lines)]
Expand Down
Loading

0 comments on commit 508115e

Please sign in to comment.