Skip to content

Commit

Permalink
Renamed: Cyclomatic Complexity -> Cognitive Complexity
Browse files Browse the repository at this point in the history
* Ran automatic naming update

* Formalized rename of `cyclomatic_complexity` to `cognitive_complexity`
** Added the rename to `lib.rs`
** Added rename test

* Added warning for deprecated key `cyclomatic_complexity_threshold` and tests for it

* Added deprecation status for Clippy's builtin attribute

* Updated tests for new builtin attribute renaming
  • Loading branch information
felix91gr committed Mar 6, 2019
1 parent 15d1731 commit ddc7180
Show file tree
Hide file tree
Showing 29 changed files with 175 additions and 120 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Expand Up @@ -798,11 +798,11 @@ All notable changes to this project will be documented in this file.
[`cmp_nan`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_nan
[`cmp_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_null
[`cmp_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_owned
[`cognitive_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity
[`collapsible_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
[`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime
[`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator
[`crosspointer_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#crosspointer_transmute
[`cyclomatic_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#cyclomatic_complexity
[`dbg_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#dbg_macro
[`decimal_literal_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#decimal_literal_representation
[`declare_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const
Expand Down
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -131,7 +131,7 @@ Some lints can be configured in a TOML file named `clippy.toml` or `.clippy.toml

```toml
blacklisted-names = ["toto", "tata", "titi"]
cyclomatic-complexity-threshold = 30
cognitive-complexity-threshold = 30
```

See the [list of lints](https://rust-lang.github.io/rust-clippy/master/index.html) for more information about which lints can be configured and the
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/assign_ops.rs
Expand Up @@ -119,7 +119,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps {
},
hir::ExprKind::Assign(assignee, e) => {
if let hir::ExprKind::Binary(op, l, r) = &e.node {
#[allow(clippy::cyclomatic_complexity)]
#[allow(clippy::cognitive_complexity)]
let lint = |assignee: &hir::Expr, rhs: &hir::Expr| {
let ty = cx.tables.expr_ty(assignee);
let rty = cx.tables.expr_ty(rhs);
Expand Down
@@ -1,4 +1,4 @@
//! calculate cyclomatic complexity and warn about overly complex functions
//! calculate cognitive complexity and warn about overly complex functions

use rustc::cfg::CFG;
use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
Expand All @@ -12,43 +12,43 @@ use syntax::source_map::Span;
use crate::utils::{in_macro, is_allowed, match_type, paths, span_help_and_lint, LimitStack};

declare_clippy_lint! {
/// **What it does:** Checks for methods with high cyclomatic complexity.
/// **What it does:** Checks for methods with high cognitive complexity.
///
/// **Why is this bad?** Methods of high cyclomatic complexity tend to be badly
/// readable. Also LLVM will usually optimize small methods better.
/// **Why is this bad?** Methods of high cognitive complexity tend to be hard to
/// both read and maintain. Also LLVM will tend to optimize small methods better.
///
/// **Known problems:** Sometimes it's hard to find a way to reduce the
/// complexity.
///
/// **Example:** No. You'll see it when you get the warning.
pub CYCLOMATIC_COMPLEXITY,
pub COGNITIVE_COMPLEXITY,
complexity,
"functions that should be split up into multiple functions"
}

pub struct CyclomaticComplexity {
pub struct CognitiveComplexity {
limit: LimitStack,
}

impl CyclomaticComplexity {
impl CognitiveComplexity {
pub fn new(limit: u64) -> Self {
Self {
limit: LimitStack::new(limit),
}
}
}

impl LintPass for CyclomaticComplexity {
impl LintPass for CognitiveComplexity {
fn get_lints(&self) -> LintArray {
lint_array!(CYCLOMATIC_COMPLEXITY)
lint_array!(COGNITIVE_COMPLEXITY)
}

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

impl CyclomaticComplexity {
impl CognitiveComplexity {
fn check<'a, 'tcx: 'a>(&mut self, cx: &'a LateContext<'a, 'tcx>, body: &'tcx Body, span: Span) {
if in_macro(span) {
return;
Expand Down Expand Up @@ -105,17 +105,17 @@ impl CyclomaticComplexity {
if rust_cc > self.limit.limit() {
span_help_and_lint(
cx,
CYCLOMATIC_COMPLEXITY,
COGNITIVE_COMPLEXITY,
span,
&format!("the function has a cyclomatic complexity of {}", rust_cc),
&format!("the function has a cognitive complexity of {}", rust_cc),
"you could split it up into multiple smaller functions",
);
}
}
}
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CyclomaticComplexity {
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CognitiveComplexity {
fn check_fn(
&mut self,
cx: &LateContext<'a, 'tcx>,
Expand All @@ -132,10 +132,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CyclomaticComplexity {
}

fn enter_lint_attrs(&mut self, cx: &LateContext<'a, 'tcx>, attrs: &'tcx [Attribute]) {
self.limit.push_attrs(cx.sess(), attrs, "cyclomatic_complexity");
self.limit.push_attrs(cx.sess(), attrs, "cognitive_complexity");
}
fn exit_lint_attrs(&mut self, cx: &LateContext<'a, 'tcx>, attrs: &'tcx [Attribute]) {
self.limit.pop_attrs(cx.sess(), attrs, "cyclomatic_complexity");
self.limit.pop_attrs(cx.sess(), attrs, "cognitive_complexity");
}
}

Expand Down Expand Up @@ -201,7 +201,7 @@ fn report_cc_bug(
) {
span_bug!(
span,
"Clippy encountered a bug calculating cyclomatic complexity: cc = {}, arms = {}, \
"Clippy encountered a bug calculating cognitive complexity: cc = {}, arms = {}, \
div = {}, shorts = {}, returns = {}. Please file a bug report.",
cc,
narms,
Expand All @@ -222,12 +222,12 @@ fn report_cc_bug(
span: Span,
id: HirId,
) {
if !is_allowed(cx, CYCLOMATIC_COMPLEXITY, id) {
if !is_allowed(cx, COGNITIVE_COMPLEXITY, id) {
cx.sess().span_note_without_error(
span,
&format!(
"Clippy encountered a bug calculating cyclomatic complexity \
(hide this message with `#[allow(cyclomatic_complexity)]`): \
"Clippy encountered a bug calculating cognitive complexity \
(hide this message with `#[allow(cognitive_complexity)]`): \
cc = {}, arms = {}, div = {}, shorts = {}, returns = {}. \
Please file a bug report.",
cc, narms, div, shorts, returns
Expand Down
9 changes: 5 additions & 4 deletions clippy_lints/src/lib.rs
Expand Up @@ -152,11 +152,11 @@ pub mod block_in_if_condition;
pub mod booleans;
pub mod bytecount;
pub mod cargo_common_metadata;
pub mod cognitive_complexity;
pub mod collapsible_if;
pub mod const_static_lifetime;
pub mod copies;
pub mod copy_iterator;
pub mod cyclomatic_complexity;
pub mod dbg_macro;
pub mod default_trait_access;
pub mod derive;
Expand Down Expand Up @@ -478,7 +478,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
reg.register_late_lint_pass(box temporary_assignment::Pass);
reg.register_late_lint_pass(box transmute::Transmute);
reg.register_late_lint_pass(
box cyclomatic_complexity::CyclomaticComplexity::new(conf.cyclomatic_complexity_threshold)
box cognitive_complexity::CognitiveComplexity::new(conf.cognitive_complexity_threshold)
);
reg.register_late_lint_pass(box escape::Pass{too_large_for_stack: conf.too_large_for_stack});
reg.register_early_lint_pass(box misc_early::MiscEarly);
Expand Down Expand Up @@ -666,11 +666,11 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
booleans::LOGIC_BUG,
booleans::NONMINIMAL_BOOL,
bytecount::NAIVE_BYTECOUNT,
cognitive_complexity::COGNITIVE_COMPLEXITY,
collapsible_if::COLLAPSIBLE_IF,
const_static_lifetime::CONST_STATIC_LIFETIME,
copies::IFS_SAME_COND,
copies::IF_SAME_THEN_ELSE,
cyclomatic_complexity::CYCLOMATIC_COMPLEXITY,
derive::DERIVE_HASH_XOR_EQ,
double_comparison::DOUBLE_COMPARISONS,
double_parens::DOUBLE_PARENS,
Expand Down Expand Up @@ -962,7 +962,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
assign_ops::MISREFACTORED_ASSIGN_OP,
attrs::DEPRECATED_CFG_ATTR,
booleans::NONMINIMAL_BOOL,
cyclomatic_complexity::CYCLOMATIC_COMPLEXITY,
cognitive_complexity::COGNITIVE_COMPLEXITY,
double_comparison::DOUBLE_COMPARISONS,
double_parens::DOUBLE_PARENS,
duration_subsec::DURATION_SUBSEC,
Expand Down Expand Up @@ -1131,6 +1131,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
pub fn register_renamed(ls: &mut rustc::lint::LintStore) {
ls.register_renamed("clippy::stutter", "clippy::module_name_repetitions");
ls.register_renamed("clippy::new_without_default_derive", "clippy::new_without_default");
ls.register_renamed("clippy::cyclomatic_complexity", "clippy::cognitive_complexity");
}

// only exists to let the dogfood integration test works.
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/mod.rs
Expand Up @@ -821,7 +821,7 @@ impl LintPass for Pass {
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
#[allow(clippy::cyclomatic_complexity)]
#[allow(clippy::cognitive_complexity)]
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) {
if in_macro(expr.span) {
return;
Expand Down
6 changes: 5 additions & 1 deletion clippy_lints/src/utils/attrs.rs
Expand Up @@ -15,7 +15,11 @@ pub enum DeprecationStatus {

pub const BUILTIN_ATTRIBUTES: &[(&str, DeprecationStatus)] = &[
("author", DeprecationStatus::None),
("cyclomatic_complexity", DeprecationStatus::None),
("cognitive_complexity", DeprecationStatus::None),
(
"cyclomatic_complexity",
DeprecationStatus::Replaced("cognitive_complexity"),
),
("dump", DeprecationStatus::None),
];

Expand Down
25 changes: 19 additions & 6 deletions clippy_lints/src/utils/conf.rs
Expand Up @@ -110,8 +110,10 @@ macro_rules! define_Conf {
define_Conf! {
/// Lint: BLACKLISTED_NAME. The list of blacklisted names to lint about
(blacklisted_names, "blacklisted_names", ["foo", "bar", "baz", "quux"] => Vec<String>),
/// Lint: CYCLOMATIC_COMPLEXITY. The maximum cyclomatic complexity a function can have
(cyclomatic_complexity_threshold, "cyclomatic_complexity_threshold", 25 => u64),
/// Lint: COGNITIVE_COMPLEXITY. The maximum cognitive complexity a function can have
(cognitive_complexity_threshold, "cognitive_complexity_threshold", 25 => u64),
/// DEPRECATED LINT: CYCLOMATIC_COMPLEXITY. Use the Cognitive Complexity lint instead.
(cyclomatic_complexity_threshold, "cyclomatic_complexity_threshold", None => Option<u64>),
/// Lint: DOC_MARKDOWN. The list of words this lint should not consider as identifiers needing ticks
(doc_valid_idents, "doc_valid_idents", [
"KiB", "MiB", "GiB", "TiB", "PiB", "EiB",
Expand Down Expand Up @@ -227,13 +229,24 @@ pub fn read(path: Option<&path::Path>) -> (Conf, Vec<Error>) {

assert!(ERRORS.lock().expect("no threading -> mutex always safe").is_empty());
match toml::from_str(&file) {
Ok(toml) => (
toml,
ERRORS.lock().expect("no threading -> mutex always safe").split_off(0),
),
Ok(toml) => {
let mut errors = ERRORS.lock().expect("no threading -> mutex always safe").split_off(0);

let toml_ref: &Conf = &toml;

let cyc_field: Option<u64> = toml_ref.cyclomatic_complexity_threshold;

if cyc_field.is_some() {
let cyc_err = "found deprecated field `cyclomatic-complexity-threshold`. Please use `cognitive-complexity-threshold` instead.".to_string();
errors.push(Error::Toml(cyc_err));
}

(toml, errors)
},
Err(e) => {
let mut errors = ERRORS.lock().expect("no threading -> mutex always safe").split_off(0);
errors.push(Error::Toml(e.to_string()));

default(errors)
},
}
Expand Down
6 changes: 6 additions & 0 deletions tests/ui-toml/conf_deprecated_key/clippy.toml
@@ -0,0 +1,6 @@
# that one is an error
cyclomatic-complexity-threshold = 42

# that one is white-listed
[third-party]
clippy-feature = "nightly"
4 changes: 4 additions & 0 deletions tests/ui-toml/conf_deprecated_key/conf_deprecated_key.rs
@@ -0,0 +1,4 @@
// error-pattern: error reading Clippy's configuration file: found deprecated field
// `cyclomatic-complexity-threshold`. Please use `cognitive-complexity-threshold` instead.

fn main() {}
4 changes: 4 additions & 0 deletions tests/ui-toml/conf_deprecated_key/conf_deprecated_key.stderr
@@ -0,0 +1,4 @@
error: error reading Clippy's configuration file `$DIR/clippy.toml`: found deprecated field `cyclomatic-complexity-threshold`. Please use `cognitive-complexity-threshold` instead.

error: aborting due to previous error

3 changes: 3 additions & 0 deletions tests/ui-toml/good_toml_no_false_negatives/clippy.toml
@@ -0,0 +1,3 @@
# that one is white-listed
[third-party]
clippy-feature = "nightly"
@@ -0,0 +1,3 @@
// error-pattern: should give absolutely no error

fn main() {}
2 changes: 1 addition & 1 deletion tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
@@ -1,4 +1,4 @@
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `too-many-lines-threshold`, `third-party`
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `too-many-lines-threshold`, `third-party`

error: aborting due to previous error

0 comments on commit ddc7180

Please sign in to comment.