Skip to content
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

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

Merged
merged 1 commit into from
Mar 6, 2019
Merged
Show file tree
Hide file tree
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
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
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};
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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"),
),
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),
];

Expand Down
25 changes: 19 additions & 6 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
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),
felix91gr marked this conversation as resolved.
Show resolved Hide resolved
/// 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
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