Skip to content

Cognitive Complexity (step 1 out of 3+): name changes #3803

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

Merged
merged 1 commit into from
Mar 6, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion clippy_lints/src/assign_ops.rs
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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};
@@ -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;
@@ -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>,
@@ -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");
}
}

@@ -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,
@@ -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
9 changes: 5 additions & 4 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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;
@@ -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);
@@ -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,
@@ -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,
@@ -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.
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
6 changes: 5 additions & 1 deletion clippy_lints/src/utils/attrs.rs
Original file line number Diff line number Diff line change
@@ -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"),
),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I understand this list correctly, you'll need to add an entry for cognitive_complexity with DeprecationStatus::None, otherwise you'll get complaints about an unknown attribute when you start using #[clippy::cognitive_complexity(..)]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh! I see. Thank you :D

("dump", DeprecationStatus::None),
];

25 changes: 19 additions & 6 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
@@ -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",
@@ -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)
},
}
6 changes: 6 additions & 0 deletions tests/ui-toml/conf_deprecated_key/clippy.toml
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# that one is white-listed
[third-party]
clippy-feature = "nightly"
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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

Loading