Skip to content

Commit

Permalink
Show lines for cognitive complexity
Browse files Browse the repository at this point in the history
  • Loading branch information
@iddm
iddm committed Sep 1, 2019
1 parent a3fcaee commit 8b8b2be
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 34 deletions.
97 changes: 64 additions & 33 deletions clippy_lints/src/cognitive_complexity.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc::{declare_tool_lint, impl_lint_pass};
use syntax::ast::Attribute;
use syntax::source_map::Span;

use crate::utils::{is_allowed, match_type, paths, span_help_and_lint, LimitStack};
use crate::utils::{is_allowed, match_type, paths, span_help_and_lint, span_notes_and_lint, LimitStack};

declare_clippy_lint! {
/// **What it does:** Checks for methods with high cognitive complexity.
Expand All @@ -28,12 +28,14 @@ declare_clippy_lint! {

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

impl CognitiveComplexity {
pub fn new(limit: u64) -> Self {
pub fn new(limit: u64, verbosity: u64) -> Self {
Self {
limit: LimitStack::new(limit),
verbosity: LimitStack::new(verbosity),
}
}
}
Expand All @@ -56,20 +58,14 @@ impl CognitiveComplexity {
}
let cc = e + 2 - n;
let mut helper = CCHelper {
match_arms: 0,
divergence: 0,
short_circuits: 0,
returns: 0,
spans: Vec::new(),
cx,
};
helper.visit_expr(expr);
let CCHelper {
match_arms,
divergence,
short_circuits,
returns,
..
} = helper;
let match_arms = helper.spans.iter().filter(|(_, t)| *t == CCType::MatchArm).count() as u64;
let divergence = helper.spans.iter().filter(|(_, t)| *t == CCType::Divergence).count() as u64;
let short_circuits = helper.spans.iter().filter(|(_, t)| *t == CCType::ShortCircuit).count() as u64;
let returns = helper.spans.iter().filter(|(_, t)| *t == CCType::Return).count() as u64;
let ret_ty = cx.tables.node_type(expr.hir_id);
let ret_adjust = if match_type(cx, ret_ty, &paths::RESULT) {
returns
Expand All @@ -78,35 +74,57 @@ impl CognitiveComplexity {
(returns / 2)
};


if cc + divergence < match_arms + short_circuits {
report_cc_bug(
cx,
cc,
match_arms,
divergence,
short_circuits,
ret_adjust,
// ret_adjust,
returns,
span,
body.id().hir_id,
);
} else {
let mut rust_cc = cc + divergence - match_arms - short_circuits;
// let rust_cc = cc + divergence - match_arms - short_circuits;
// prevent degenerate cases where unreachable code contains `return` statements
if rust_cc >= ret_adjust {
rust_cc -= ret_adjust;
}
if rust_cc > self.limit.limit() {
span_help_and_lint(
cx,
COGNITIVE_COMPLEXITY,
span,
&format!(
"the function has a cognitive complexity of ({}/{})",
rust_cc,
self.limit.limit()
),
"you could split it up into multiple smaller functions",
);
// if rust_cc > self.verbosity.limit() {
// let mut notes: Vec<(Span, String)> = Vec::new();
// let mut counter = 1;
// for span in helper.spans {
// notes.push((span.0, format!("Increases cognitive complexity to {}", counter)));
// counter += 1;
// }
// span_notes_and_lint(
// cx,
// COGNITIVE_COMPLEXITY,
// span,
// &format!(
// "the function has a cognitive complexity of ({}/{})",
// rust_cc,
// self.limit.limit()
// ),
// notes);
// } else {
span_help_and_lint(
cx,
COGNITIVE_COMPLEXITY,
span,
&format!(
"the function has a cognitive complexity of ({}/{})",
rust_cc,
self.limit.limit()
),
"you could split it up into multiple smaller functions",
);
// }
}
}
}
Expand All @@ -130,17 +148,24 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CognitiveComplexity {

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

#[derive(Copy, Clone, Eq, PartialEq)]
enum CCType {
MatchArm,
Divergence,
Return,
ShortCircuit, // && and ||
}

struct CCHelper<'a, 'tcx> {
match_arms: u64,
divergence: u64,
returns: u64,
short_circuits: u64, // && and ||
spans: Vec<(Span, CCType)>,
cx: &'a LateContext<'a, 'tcx>,
}

Expand All @@ -151,7 +176,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CCHelper<'a, 'tcx> {
walk_expr(self, e);
let arms_n: u64 = arms.iter().map(|arm| arm.pats.len() as u64).sum();
if arms_n > 1 {
self.match_arms += arms_n - 2;
self.spans.push((e.span, CCType::MatchArm));
}
},
ExprKind::Call(ref callee, _) => {
Expand All @@ -161,7 +186,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CCHelper<'a, 'tcx> {
ty::FnDef(..) | ty::FnPtr(_) => {
let sig = ty.fn_sig(self.cx.tcx);
if sig.skip_binder().output().sty == ty::Never {
self.divergence += 1;
self.spans.push((e.span, CCType::Divergence));
}
},
_ => (),
Expand All @@ -171,11 +196,17 @@ impl<'a, 'tcx> Visitor<'tcx> for CCHelper<'a, 'tcx> {
ExprKind::Binary(op, _, _) => {
walk_expr(self, e);
match op.node {
BinOpKind::And | BinOpKind::Or => self.short_circuits += 1,
BinOpKind::And | BinOpKind::Or => self.spans.push((e.span, CCType::ShortCircuit)),
_ => (),
}
},
ExprKind::Ret(_) => self.returns += 1,
// ExprKind::Ret(_) => self.returns.push(e.span),
ExprKind::Ret(_) => self.spans.push((e.span, CCType::Return)),
// ExprKind::Ret(_) => {
// if self.spans.iter().find(|(s, t)| s.source_equal(&e.span) && *t == CCType::Return).is_none() {
// self.spans.push((e.span, CCType::Return))
// }
// },
_ => walk_expr(self, e),
}
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
reg.register_late_lint_pass(box temporary_assignment::TemporaryAssignment);
reg.register_late_lint_pass(box transmute::Transmute);
reg.register_late_lint_pass(
box cognitive_complexity::CognitiveComplexity::new(conf.cognitive_complexity_threshold)
box cognitive_complexity::CognitiveComplexity::new(conf.cognitive_complexity_threshold, conf.cognitive_complexity_verbosity)
);
reg.register_late_lint_pass(box escape::BoxedLocal{too_large_for_stack: conf.too_large_for_stack});
reg.register_early_lint_pass(box misc_early::MiscEarlyLints);
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/utils/conf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ define_Conf! {
(blacklisted_names, "blacklisted_names", ["foo", "bar", "baz", "quux"] => Vec<String>),
/// Lint: COGNITIVE_COMPLEXITY. The maximum cognitive complexity a function can have
(cognitive_complexity_threshold, "cognitive_complexity_threshold", 25 => u64),
/// Lint: COGNITIVE_COMPLEXITY. The verbosity of the cognitive complexity lint
(cognitive_complexity_verbosity, "cognitive_complexity_verbosity", 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
Expand Down
23 changes: 23 additions & 0 deletions clippy_lints/src/utils/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,29 @@ pub fn span_note_and_lint<'a, T: LintContext>(
db.docs_link(lint);
}

/// Like `span_note_and_lint` but with multiple note sections.
///
/// The note messages are provided for bounded to spans.
///
pub fn span_notes_and_lint<'a, T: LintContext>(
cx: &'a T,
lint: &'static Lint,
span: Span,
msg: &str,
notes: Vec<(Span, String)>,
) {
let mut db = DiagnosticWrapper(cx.struct_span_lint(lint, span, msg));
for note in &notes {
if note.0 == span {
db.0.note(&note.1);
} else {
db.0.span_note(note.0, &note.1);
}
}
db.docs_link(lint);
}


pub fn span_lint_and_then<'a, T: LintContext, F>(cx: &'a T, lint: &'static Lint, sp: Span, msg: &str, f: F)
where
F: for<'b> FnOnce(&mut DiagnosticBuilder<'b>),
Expand Down

0 comments on commit 8b8b2be

Please sign in to comment.