forked from OSchip/llvm-project
[clang-tidy] Implement readability-function-cognitive-complexity check
Currently, there is basically just one clang-tidy check to impose some sanity limits on functions - `clang-tidy-readability-function-size`. It is nice, allows to limit line count, total number of statements, number of branches, number of function parameters (not counting implicit `this`), nesting level. However, those are simple generic metrics. It is still trivially possible to write a function, which does not violate any of these metrics, yet is still rather unreadable. Thus, some additional, slightly more complicated metric is needed. There is a well-known [[ https://en.wikipedia.org/wiki/Cyclomatic_complexity | Cyclomatic complexity]], but certainly has its downsides. And there is a [[ https://www.sonarsource.com/docs/CognitiveComplexity.pdf | COGNITIVE COMPLEXITY by SonarSource ]], which is available for opensource on https://sonarcloud.io/. This check checks function Cognitive Complexity metric, and flags the functions with Cognitive Complexity exceeding the configured limit. The default limit is `25`, same as in 'upstream'. The metric is implemented as per [[ https://www.sonarsource.com/docs/CognitiveComplexity.pdf | COGNITIVE COMPLEXITY by SonarSource ]] specification version 1.2 (19 April 2017), with two notable exceptions: * `preprocessor conditionals` (`#ifdef`, `#if`, `#elif`, `#else`, `#endif`) are not accounted for. Could be done. Currently, upstream does not account for them either. * `each method in a recursion cycle` is not accounted for. It can't be fully implemented, because cross-translational-unit analysis would be needed, which is not possible in clang-tidy. Thus, at least right now, i completely avoided implementing it. There are some further possible improvements: * Are GNU statement expressions (`BinaryConditionalOperator`) really free? They should probably cause nesting level increase, and complexity level increase when they are nested within eachother. * Microsoft SEH support * ??? Reviewed By: aaron.ballman, JonasToth, lattner Differential Revision: https://reviews.llvm.org/D36836
This commit is contained in:
parent
a594fd28e3
commit
ace644030e
|
@ -12,6 +12,7 @@ add_clang_library(clangTidyReadabilityModule
|
|||
DeleteNullPointerCheck.cpp
|
||||
DeletedDefaultCheck.cpp
|
||||
ElseAfterReturnCheck.cpp
|
||||
FunctionCognitiveComplexityCheck.cpp
|
||||
FunctionSizeCheck.cpp
|
||||
IdentifierNamingCheck.cpp
|
||||
ImplicitBoolConversionCheck.cpp
|
||||
|
|
|
@ -0,0 +1,542 @@
|
|||
//===--- FunctionCognitiveComplexityCheck.cpp - clang-tidy ------*- C++ -*-===//
|
||||
//
|
||||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
|
||||
// See https://llvm.org/LICENSE.txt for license information.
|
||||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
|
||||
//
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
||||
#include "FunctionCognitiveComplexityCheck.h"
|
||||
#include "../ClangTidyDiagnosticConsumer.h"
|
||||
#include "clang/AST/Decl.h"
|
||||
#include "clang/AST/DeclBase.h"
|
||||
#include "clang/AST/Expr.h"
|
||||
#include "clang/AST/RecursiveASTVisitor.h"
|
||||
#include "clang/AST/Stmt.h"
|
||||
#include "clang/ASTMatchers/ASTMatchFinder.h"
|
||||
#include "clang/ASTMatchers/ASTMatchers.h"
|
||||
#include "clang/ASTMatchers/ASTMatchersInternal.h"
|
||||
#include "clang/Basic/Diagnostic.h"
|
||||
#include "clang/Basic/DiagnosticIDs.h"
|
||||
#include "clang/Basic/LLVM.h"
|
||||
#include "clang/Basic/SourceLocation.h"
|
||||
#include "llvm/ADT/Optional.h"
|
||||
#include "llvm/ADT/SmallVector.h"
|
||||
#include "llvm/Support/Casting.h"
|
||||
#include "llvm/Support/ErrorHandling.h"
|
||||
#include <array>
|
||||
#include <cassert>
|
||||
#include <stack>
|
||||
#include <tuple>
|
||||
#include <type_traits>
|
||||
#include <utility>
|
||||
|
||||
using namespace clang::ast_matchers;
|
||||
|
||||
namespace clang {
|
||||
namespace tidy {
|
||||
namespace readability {
|
||||
namespace {
|
||||
|
||||
struct CognitiveComplexity final {
|
||||
// Any increment is based on some combination of reasons.
|
||||
// For details you can look at the Specification at
|
||||
// https://www.sonarsource.com/docs/CognitiveComplexity.pdf
|
||||
// or user-facing docs at
|
||||
// http://clang.llvm.org/extra/clang-tidy/checks/readability-function-cognitive-complexity.html
|
||||
// Here are all the possible reasons:
|
||||
enum Criteria : uint8_t {
|
||||
None = 0U,
|
||||
|
||||
// B1, increases cognitive complexity (by 1)
|
||||
// What causes it:
|
||||
// * if, else if, else, ConditionalOperator (not BinaryConditionalOperator)
|
||||
// * SwitchStmt
|
||||
// * ForStmt, CXXForRangeStmt
|
||||
// * WhileStmt, DoStmt
|
||||
// * CXXCatchStmt
|
||||
// * GotoStmt, IndirectGotoStmt (but not BreakStmt, ContinueStmt)
|
||||
// * sequences of binary logical operators (BinOpLAnd, BinOpLOr)
|
||||
// * each method in a recursion cycle (not implemented)
|
||||
Increment = 1U << 0,
|
||||
|
||||
// B2, increases current nesting level (by 1)
|
||||
// What causes it:
|
||||
// * if, else if, else, ConditionalOperator (not BinaryConditionalOperator)
|
||||
// * SwitchStmt
|
||||
// * ForStmt, CXXForRangeStmt
|
||||
// * WhileStmt, DoStmt
|
||||
// * CXXCatchStmt
|
||||
// * nested CXXConstructor, CXXDestructor, CXXMethod (incl. C++11 Lambda)
|
||||
// * GNU Statement Expression
|
||||
// * Apple Block declaration
|
||||
IncrementNesting = 1U << 1,
|
||||
|
||||
// B3, increases cognitive complexity by the current nesting level
|
||||
// Applied before IncrementNesting
|
||||
// What causes it:
|
||||
// * IfStmt, ConditionalOperator (not BinaryConditionalOperator)
|
||||
// * SwitchStmt
|
||||
// * ForStmt, CXXForRangeStmt
|
||||
// * WhileStmt, DoStmt
|
||||
// * CXXCatchStmt
|
||||
PenalizeNesting = 1U << 2,
|
||||
|
||||
All = Increment | PenalizeNesting | IncrementNesting,
|
||||
};
|
||||
|
||||
// The helper struct used to record one increment occurrence, with all the
|
||||
// details nessesary.
|
||||
struct Detail {
|
||||
const SourceLocation Loc; // What caused the increment?
|
||||
const unsigned short Nesting; // How deeply nested is Loc located?
|
||||
const Criteria C; // The criteria of the increment
|
||||
|
||||
Detail(SourceLocation SLoc, unsigned short CurrentNesting, Criteria Crit)
|
||||
: Loc(SLoc), Nesting(CurrentNesting), C(Crit) {}
|
||||
|
||||
// To minimize the sizeof(Detail), we only store the minimal info there.
|
||||
// This function is used to convert from the stored info into the usable
|
||||
// information - what message to output, how much of an increment did this
|
||||
// occurrence actually result in.
|
||||
std::pair<unsigned, unsigned short> process() const {
|
||||
assert(C != Criteria::None && "invalid criteria");
|
||||
|
||||
unsigned MsgId; // The id of the message to output.
|
||||
unsigned short Increment; // How much of an increment?
|
||||
|
||||
if (C == Criteria::All) {
|
||||
Increment = 1 + Nesting;
|
||||
MsgId = 0;
|
||||
} else if (C == (Criteria::Increment | Criteria::IncrementNesting)) {
|
||||
Increment = 1;
|
||||
MsgId = 1;
|
||||
} else if (C == Criteria::Increment) {
|
||||
Increment = 1;
|
||||
MsgId = 2;
|
||||
} else if (C == Criteria::IncrementNesting) {
|
||||
Increment = 0; // Unused in this message.
|
||||
MsgId = 3;
|
||||
} else
|
||||
llvm_unreachable("should not get to here.");
|
||||
|
||||
return std::make_pair(MsgId, Increment);
|
||||
}
|
||||
};
|
||||
|
||||
// Limit of 25 is the "upstream"'s default.
|
||||
static constexpr unsigned DefaultLimit = 25U;
|
||||
|
||||
// Based on the publicly-avaliable numbers for some big open-source projects
|
||||
// https://sonarcloud.io/projects?languages=c%2Ccpp&size=5 we can estimate:
|
||||
// value ~20 would result in no allocs for 98% of functions, ~12 for 96%, ~10
|
||||
// for 91%, ~8 for 88%, ~6 for 84%, ~4 for 77%, ~2 for 64%, and ~1 for 37%.
|
||||
static_assert(sizeof(Detail) <= 8,
|
||||
"Since we use SmallVector to minimize the amount of "
|
||||
"allocations, we also need to consider the price we pay for "
|
||||
"that in terms of stack usage. "
|
||||
"Thus, it is good to minimize the size of the Detail struct.");
|
||||
SmallVector<Detail, DefaultLimit> Details; // 25 elements is 200 bytes.
|
||||
// Yes, 25 is a magic number. This is the seemingly-sane default for the
|
||||
// upper limit for function cognitive complexity. Thus it would make sense
|
||||
// to avoid allocations for any function that does not violate the limit.
|
||||
|
||||
// The grand total Cognitive Complexity of the function.
|
||||
unsigned Total = 0;
|
||||
|
||||
// The function used to store new increment, calculate the total complexity.
|
||||
void account(SourceLocation Loc, unsigned short Nesting, Criteria C);
|
||||
};
|
||||
|
||||
// All the possible messages that can be output. The choice of the message
|
||||
// to use is based of the combination of the CognitiveComplexity::Criteria.
|
||||
// It would be nice to have it in CognitiveComplexity struct, but then it is
|
||||
// not static.
|
||||
static const std::array<const StringRef, 4> Msgs = {{
|
||||
// B1 + B2 + B3
|
||||
"+%0, including nesting penalty of %1, nesting level increased to %2",
|
||||
|
||||
// B1 + B2
|
||||
"+%0, nesting level increased to %2",
|
||||
|
||||
// B1
|
||||
"+%0",
|
||||
|
||||
// B2
|
||||
"nesting level increased to %2",
|
||||
}};
|
||||
|
||||
// Criteria is a bitset, thus a few helpers are needed.
|
||||
CognitiveComplexity::Criteria operator|(CognitiveComplexity::Criteria LHS,
|
||||
CognitiveComplexity::Criteria RHS) {
|
||||
return static_cast<CognitiveComplexity::Criteria>(
|
||||
static_cast<std::underlying_type<CognitiveComplexity::Criteria>::type>(
|
||||
LHS) |
|
||||
static_cast<std::underlying_type<CognitiveComplexity::Criteria>::type>(
|
||||
RHS));
|
||||
}
|
||||
CognitiveComplexity::Criteria operator&(CognitiveComplexity::Criteria LHS,
|
||||
CognitiveComplexity::Criteria RHS) {
|
||||
return static_cast<CognitiveComplexity::Criteria>(
|
||||
static_cast<std::underlying_type<CognitiveComplexity::Criteria>::type>(
|
||||
LHS) &
|
||||
static_cast<std::underlying_type<CognitiveComplexity::Criteria>::type>(
|
||||
RHS));
|
||||
}
|
||||
CognitiveComplexity::Criteria &operator|=(CognitiveComplexity::Criteria &LHS,
|
||||
CognitiveComplexity::Criteria RHS) {
|
||||
LHS = operator|(LHS, RHS);
|
||||
return LHS;
|
||||
}
|
||||
CognitiveComplexity::Criteria &operator&=(CognitiveComplexity::Criteria &LHS,
|
||||
CognitiveComplexity::Criteria RHS) {
|
||||
LHS = operator&(LHS, RHS);
|
||||
return LHS;
|
||||
}
|
||||
|
||||
void CognitiveComplexity::account(SourceLocation Loc, unsigned short Nesting,
|
||||
Criteria C) {
|
||||
C &= Criteria::All;
|
||||
assert(C != Criteria::None && "invalid criteria");
|
||||
|
||||
Details.emplace_back(Loc, Nesting, C);
|
||||
const Detail &D = Details.back();
|
||||
|
||||
unsigned MsgId;
|
||||
unsigned short Increase;
|
||||
std::tie(MsgId, Increase) = D.process();
|
||||
|
||||
Total += Increase;
|
||||
}
|
||||
|
||||
class FunctionASTVisitor final
|
||||
: public RecursiveASTVisitor<FunctionASTVisitor> {
|
||||
using Base = RecursiveASTVisitor<FunctionASTVisitor>;
|
||||
|
||||
// The current nesting level (increased by Criteria::IncrementNesting).
|
||||
unsigned short CurrentNestingLevel = 0;
|
||||
|
||||
// Used to efficiently know the last type of the binary sequence operator
|
||||
// that was encountered. It would make sense for the function call to start
|
||||
// the new sequence, thus it is a stack.
|
||||
using OBO = Optional<BinaryOperator::Opcode>;
|
||||
std::stack<OBO, SmallVector<OBO, 4>> BinaryOperatorsStack;
|
||||
|
||||
public:
|
||||
bool TraverseStmtWithIncreasedNestingLevel(Stmt *Node) {
|
||||
++CurrentNestingLevel;
|
||||
bool ShouldContinue = Base::TraverseStmt(Node);
|
||||
--CurrentNestingLevel;
|
||||
return ShouldContinue;
|
||||
}
|
||||
|
||||
bool TraverseDeclWithIncreasedNestingLevel(Decl *Node) {
|
||||
++CurrentNestingLevel;
|
||||
bool ShouldContinue = Base::TraverseDecl(Node);
|
||||
--CurrentNestingLevel;
|
||||
return ShouldContinue;
|
||||
}
|
||||
|
||||
bool TraverseIfStmt(IfStmt *Node, bool InElseIf = false) {
|
||||
if (!Node)
|
||||
return Base::TraverseIfStmt(Node);
|
||||
|
||||
{
|
||||
CognitiveComplexity::Criteria Reasons;
|
||||
|
||||
Reasons = CognitiveComplexity::Criteria::None;
|
||||
|
||||
// "If" increases cognitive complexity.
|
||||
Reasons |= CognitiveComplexity::Criteria::Increment;
|
||||
// "If" increases nesting level.
|
||||
Reasons |= CognitiveComplexity::Criteria::IncrementNesting;
|
||||
|
||||
if (!InElseIf) {
|
||||
// "If" receives a nesting increment commensurate with it's nested
|
||||
// depth, if it is not part of "else if".
|
||||
Reasons |= CognitiveComplexity::Criteria::PenalizeNesting;
|
||||
}
|
||||
|
||||
CC.account(Node->getIfLoc(), CurrentNestingLevel, Reasons);
|
||||
}
|
||||
|
||||
// If this IfStmt is *NOT* "else if", then only the body (i.e. "Then" and
|
||||
// "Else") is traversed with increased Nesting level.
|
||||
// However if this IfStmt *IS* "else if", then Nesting level is increased
|
||||
// for the whole IfStmt (i.e. for "Init", "Cond", "Then" and "Else").
|
||||
|
||||
if (!InElseIf) {
|
||||
if (!TraverseStmt(Node->getInit()))
|
||||
return false;
|
||||
|
||||
if (!TraverseStmt(Node->getCond()))
|
||||
return false;
|
||||
} else {
|
||||
if (!TraverseStmtWithIncreasedNestingLevel(Node->getInit()))
|
||||
return false;
|
||||
|
||||
if (!TraverseStmtWithIncreasedNestingLevel(Node->getCond()))
|
||||
return false;
|
||||
}
|
||||
|
||||
// "Then" always increases nesting level.
|
||||
if (!TraverseStmtWithIncreasedNestingLevel(Node->getThen()))
|
||||
return false;
|
||||
|
||||
if (!Node->getElse())
|
||||
return true;
|
||||
|
||||
if (auto *E = dyn_cast<IfStmt>(Node->getElse()))
|
||||
return TraverseIfStmt(E, true);
|
||||
|
||||
{
|
||||
CognitiveComplexity::Criteria Reasons;
|
||||
|
||||
Reasons = CognitiveComplexity::Criteria::None;
|
||||
|
||||
// "Else" increases cognitive complexity.
|
||||
Reasons |= CognitiveComplexity::Criteria::Increment;
|
||||
// "Else" increases nesting level.
|
||||
Reasons |= CognitiveComplexity::Criteria::IncrementNesting;
|
||||
// "Else" DOES NOT receive a nesting increment commensurate with it's
|
||||
// nested depth.
|
||||
|
||||
CC.account(Node->getElseLoc(), CurrentNestingLevel, Reasons);
|
||||
}
|
||||
|
||||
// "Else" always increases nesting level.
|
||||
return TraverseStmtWithIncreasedNestingLevel(Node->getElse());
|
||||
}
|
||||
|
||||
// The currently-being-processed stack entry, which is always the top.
|
||||
#define CurrentBinaryOperator BinaryOperatorsStack.top()
|
||||
|
||||
// In a sequence of binary logical operators, if the new operator is different
|
||||
// from the previous one, then the cognitive complexity is increased.
|
||||
bool TraverseBinaryOperator(BinaryOperator *Op) {
|
||||
if (!Op || !Op->isLogicalOp())
|
||||
return Base::TraverseBinaryOperator(Op);
|
||||
|
||||
// Make sure that there is always at least one frame in the stack.
|
||||
if (BinaryOperatorsStack.empty())
|
||||
BinaryOperatorsStack.emplace();
|
||||
|
||||
// If this is the first binary operator that we are processing, or the
|
||||
// previous binary operator was different, there is an increment.
|
||||
if (!CurrentBinaryOperator || Op->getOpcode() != CurrentBinaryOperator)
|
||||
CC.account(Op->getOperatorLoc(), CurrentNestingLevel,
|
||||
CognitiveComplexity::Criteria::Increment);
|
||||
|
||||
// We might encounter a function call, which starts a new sequence, thus
|
||||
// we need to save the current previous binary operator.
|
||||
const Optional<BinaryOperator::Opcode> BinOpCopy(CurrentBinaryOperator);
|
||||
|
||||
// Record the operator that we are currently processing and traverse it.
|
||||
CurrentBinaryOperator = Op->getOpcode();
|
||||
bool ShouldContinue = Base::TraverseBinaryOperator(Op);
|
||||
|
||||
// And restore the previous binary operator, which might be nonexistent.
|
||||
CurrentBinaryOperator = BinOpCopy;
|
||||
|
||||
return ShouldContinue;
|
||||
}
|
||||
|
||||
// It would make sense for the function call to start the new binary
|
||||
// operator sequence, thus let's make sure that it creates a new stack frame.
|
||||
bool TraverseCallExpr(CallExpr *Node) {
|
||||
// If we are not currently processing any binary operator sequence, then
|
||||
// no Node-handling is needed.
|
||||
if (!Node || BinaryOperatorsStack.empty() || !CurrentBinaryOperator)
|
||||
return Base::TraverseCallExpr(Node);
|
||||
|
||||
// Else, do add [uninitialized] frame to the stack, and traverse call.
|
||||
BinaryOperatorsStack.emplace();
|
||||
bool ShouldContinue = Base::TraverseCallExpr(Node);
|
||||
// And remove the top frame.
|
||||
BinaryOperatorsStack.pop();
|
||||
|
||||
return ShouldContinue;
|
||||
}
|
||||
|
||||
#undef CurrentBinaryOperator
|
||||
|
||||
bool TraverseStmt(Stmt *Node) {
|
||||
if (!Node)
|
||||
return Base::TraverseStmt(Node);
|
||||
|
||||
// Three following switch()'es have huge duplication, but it is better to
|
||||
// keep them separate, to simplify comparing them with the Specification.
|
||||
|
||||
CognitiveComplexity::Criteria Reasons = CognitiveComplexity::Criteria::None;
|
||||
SourceLocation Location = Node->getBeginLoc();
|
||||
|
||||
// B1. Increments
|
||||
// There is an increment for each of the following:
|
||||
switch (Node->getStmtClass()) {
|
||||
// if, else if, else are handled in TraverseIfStmt(),
|
||||
// FIXME: "each method in a recursion cycle" Increment is not implemented.
|
||||
case Stmt::ConditionalOperatorClass:
|
||||
case Stmt::SwitchStmtClass:
|
||||
case Stmt::ForStmtClass:
|
||||
case Stmt::CXXForRangeStmtClass:
|
||||
case Stmt::WhileStmtClass:
|
||||
case Stmt::DoStmtClass:
|
||||
case Stmt::CXXCatchStmtClass:
|
||||
case Stmt::GotoStmtClass:
|
||||
case Stmt::IndirectGotoStmtClass:
|
||||
Reasons |= CognitiveComplexity::Criteria::Increment;
|
||||
break;
|
||||
default:
|
||||
// break LABEL, continue LABEL increase cognitive complexity,
|
||||
// but they are not supported in C++ or C.
|
||||
// Regular break/continue do not increase cognitive complexity.
|
||||
break;
|
||||
}
|
||||
|
||||
// B2. Nesting level
|
||||
// The following structures increment the nesting level:
|
||||
switch (Node->getStmtClass()) {
|
||||
// if, else if, else are handled in TraverseIfStmt(),
|
||||
// Nested methods and such are handled in TraverseDecl.
|
||||
case Stmt::ConditionalOperatorClass:
|
||||
case Stmt::SwitchStmtClass:
|
||||
case Stmt::ForStmtClass:
|
||||
case Stmt::CXXForRangeStmtClass:
|
||||
case Stmt::WhileStmtClass:
|
||||
case Stmt::DoStmtClass:
|
||||
case Stmt::CXXCatchStmtClass:
|
||||
case Stmt::LambdaExprClass:
|
||||
case Stmt::StmtExprClass:
|
||||
Reasons |= CognitiveComplexity::Criteria::IncrementNesting;
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
||||
// B3. Nesting increments
|
||||
// The following structures receive a nesting increment
|
||||
// commensurate with their nested depth inside B2 structures:
|
||||
switch (Node->getStmtClass()) {
|
||||
// if, else if, else are handled in TraverseIfStmt().
|
||||
case Stmt::ConditionalOperatorClass:
|
||||
case Stmt::SwitchStmtClass:
|
||||
case Stmt::ForStmtClass:
|
||||
case Stmt::CXXForRangeStmtClass:
|
||||
case Stmt::WhileStmtClass:
|
||||
case Stmt::DoStmtClass:
|
||||
case Stmt::CXXCatchStmtClass:
|
||||
Reasons |= CognitiveComplexity::Criteria::PenalizeNesting;
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
||||
if (Node->getStmtClass() == Stmt::ConditionalOperatorClass) {
|
||||
// A little beautification.
|
||||
// For conditional operator "cond ? true : false" point at the "?"
|
||||
// symbol.
|
||||
ConditionalOperator *COp = dyn_cast<ConditionalOperator>(Node);
|
||||
Location = COp->getQuestionLoc();
|
||||
}
|
||||
|
||||
// If we have found any reasons, let's account it.
|
||||
if (Reasons & CognitiveComplexity::Criteria::All)
|
||||
CC.account(Location, CurrentNestingLevel, Reasons);
|
||||
|
||||
// Did we decide that the nesting level should be increased?
|
||||
if (!(Reasons & CognitiveComplexity::Criteria::IncrementNesting))
|
||||
return Base::TraverseStmt(Node);
|
||||
|
||||
return TraverseStmtWithIncreasedNestingLevel(Node);
|
||||
}
|
||||
|
||||
// The parameter MainAnalyzedFunction is needed to differentiate between the
|
||||
// cases where TraverseDecl() is the entry point from
|
||||
// FunctionCognitiveComplexityCheck::check() and the cases where it was called
|
||||
// from the FunctionASTVisitor itself. Explanation: if we get a function
|
||||
// definition (e.g. constructor, destructor, method), the Cognitive Complexity
|
||||
// specification states that the Nesting level shall be increased. But if this
|
||||
// function is the entry point, then the Nesting level should not be
|
||||
// increased. Thus that parameter is there and is used to fall-through
|
||||
// directly to traversing if this is the main function that is being analyzed.
|
||||
bool TraverseDecl(Decl *Node, bool MainAnalyzedFunction = false) {
|
||||
if (!Node || MainAnalyzedFunction)
|
||||
return Base::TraverseDecl(Node);
|
||||
|
||||
// B2. Nesting level
|
||||
// The following structures increment the nesting level:
|
||||
switch (Node->getKind()) {
|
||||
case Decl::Function:
|
||||
case Decl::CXXMethod:
|
||||
case Decl::CXXConstructor:
|
||||
case Decl::CXXDestructor:
|
||||
case Decl::Block:
|
||||
break;
|
||||
default:
|
||||
// If this is something else, we use early return!
|
||||
return Base::TraverseDecl(Node);
|
||||
break;
|
||||
}
|
||||
|
||||
CC.account(Node->getBeginLoc(), CurrentNestingLevel,
|
||||
CognitiveComplexity::Criteria::IncrementNesting);
|
||||
|
||||
return TraverseDeclWithIncreasedNestingLevel(Node);
|
||||
}
|
||||
|
||||
CognitiveComplexity CC;
|
||||
};
|
||||
|
||||
} // namespace
|
||||
|
||||
FunctionCognitiveComplexityCheck::FunctionCognitiveComplexityCheck(
|
||||
StringRef Name, ClangTidyContext *Context)
|
||||
: ClangTidyCheck(Name, Context),
|
||||
Threshold(Options.get("Threshold", CognitiveComplexity::DefaultLimit)) {}
|
||||
|
||||
void FunctionCognitiveComplexityCheck::storeOptions(
|
||||
ClangTidyOptions::OptionMap &Opts) {
|
||||
Options.store(Opts, "Threshold", Threshold);
|
||||
}
|
||||
|
||||
void FunctionCognitiveComplexityCheck::registerMatchers(MatchFinder *Finder) {
|
||||
Finder->addMatcher(
|
||||
functionDecl(
|
||||
allOf(isDefinition(), unless(anyOf(isDefaulted(), isDeleted(),
|
||||
isImplicit(), isInstantiated()))))
|
||||
.bind("func"),
|
||||
this);
|
||||
}
|
||||
|
||||
void FunctionCognitiveComplexityCheck::check(
|
||||
const MatchFinder::MatchResult &Result) {
|
||||
const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");
|
||||
assert(Func->hasBody() && "The matchers should only match the functions that "
|
||||
"have user-provided body.");
|
||||
|
||||
FunctionASTVisitor Visitor;
|
||||
Visitor.TraverseDecl(const_cast<FunctionDecl *>(Func), true);
|
||||
|
||||
if (Visitor.CC.Total <= Threshold)
|
||||
return;
|
||||
|
||||
diag(Func->getLocation(),
|
||||
"function %0 has cognitive complexity of %1 (threshold %2)")
|
||||
<< Func << Visitor.CC.Total << Threshold;
|
||||
|
||||
// Output all the basic increments of complexity.
|
||||
for (const auto &Detail : Visitor.CC.Details) {
|
||||
unsigned MsgId; // The id of the message to output.
|
||||
unsigned short Increase; // How much of an increment?
|
||||
std::tie(MsgId, Increase) = Detail.process();
|
||||
assert(MsgId < Msgs.size() && "MsgId should always be valid");
|
||||
// Increase, on the other hand, can be 0.
|
||||
|
||||
diag(Detail.Loc, Msgs[MsgId], DiagnosticIDs::Note)
|
||||
<< Increase << Detail.Nesting << 1 + Detail.Nesting;
|
||||
}
|
||||
}
|
||||
|
||||
} // namespace readability
|
||||
} // namespace tidy
|
||||
} // namespace clang
|
|
@ -0,0 +1,43 @@
|
|||
//===--- FunctionCognitiveComplexityCheck.h - clang-tidy --------*- C++ -*-===//
|
||||
//
|
||||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
|
||||
// See https://llvm.org/LICENSE.txt for license information.
|
||||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
|
||||
//
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
||||
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_FUNCTIONCOGNITIVECOMPLEXITYCHECK_H
|
||||
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_FUNCTIONCOGNITIVECOMPLEXITYCHECK_H
|
||||
|
||||
#include "../ClangTidyCheck.h"
|
||||
|
||||
namespace clang {
|
||||
namespace tidy {
|
||||
namespace readability {
|
||||
|
||||
/// Checks function Cognitive Complexity metric.
|
||||
///
|
||||
/// There is only one configuration option:
|
||||
///
|
||||
/// * `Threshold` - flag functions with Cognitive Complexity exceeding
|
||||
/// this number. The default is `25`.
|
||||
///
|
||||
/// For the user-facing documentation see:
|
||||
/// http://clang.llvm.org/extra/clang-tidy/checks/readability-function-cognitive-complexity.html
|
||||
class FunctionCognitiveComplexityCheck : public ClangTidyCheck {
|
||||
public:
|
||||
FunctionCognitiveComplexityCheck(StringRef Name, ClangTidyContext *Context);
|
||||
|
||||
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
|
||||
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
|
||||
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
|
||||
|
||||
private:
|
||||
const unsigned Threshold;
|
||||
};
|
||||
|
||||
} // namespace readability
|
||||
} // namespace tidy
|
||||
} // namespace clang
|
||||
|
||||
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_FUNCTIONCOGNITIVECOMPLEXITYCHECK_H
|
|
@ -17,6 +17,7 @@
|
|||
#include "DeleteNullPointerCheck.h"
|
||||
#include "DeletedDefaultCheck.h"
|
||||
#include "ElseAfterReturnCheck.h"
|
||||
#include "FunctionCognitiveComplexityCheck.h"
|
||||
#include "FunctionSizeCheck.h"
|
||||
#include "IdentifierNamingCheck.h"
|
||||
#include "ImplicitBoolConversionCheck.h"
|
||||
|
@ -70,6 +71,8 @@ public:
|
|||
"readability-deleted-default");
|
||||
CheckFactories.registerCheck<ElseAfterReturnCheck>(
|
||||
"readability-else-after-return");
|
||||
CheckFactories.registerCheck<FunctionCognitiveComplexityCheck>(
|
||||
"readability-function-cognitive-complexity");
|
||||
CheckFactories.registerCheck<FunctionSizeCheck>(
|
||||
"readability-function-size");
|
||||
CheckFactories.registerCheck<IdentifierNamingCheck>(
|
||||
|
|
|
@ -106,6 +106,11 @@ New checks
|
|||
Finds condition variables in nested ``if`` statements that were also checked
|
||||
in the outer ``if`` statement and were not changed.
|
||||
|
||||
- New :doc:`readability-function-cognitive-complexity
|
||||
<clang-tidy/checks/readability-function-cognitive-complexity>` check.
|
||||
|
||||
Flags functions with Cognitive Complexity metric exceeding the configured limit.
|
||||
|
||||
Changes in existing checks
|
||||
^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
|
|
|
@ -276,6 +276,7 @@ Clang-Tidy Checks
|
|||
`readability-delete-null-pointer <readability-delete-null-pointer.html>`_, "Yes"
|
||||
`readability-deleted-default <readability-deleted-default.html>`_,
|
||||
`readability-else-after-return <readability-else-after-return.html>`_, "Yes"
|
||||
`readability-function-cognitive-complexity <readability-function-cognitive-complexity.html>`_,
|
||||
`readability-function-size <readability-function-size.html>`_,
|
||||
`readability-identifier-naming <readability-identifier-naming.html>`_, "Yes"
|
||||
`readability-implicit-bool-conversion <readability-implicit-bool-conversion.html>`_, "Yes"
|
||||
|
|
|
@ -0,0 +1,146 @@
|
|||
.. title:: clang-tidy - readability-function-cognitive-complexity
|
||||
|
||||
readability-function-cognitive-complexity
|
||||
=========================================
|
||||
|
||||
Checks function Cognitive Complexity metric.
|
||||
|
||||
The metric is implemented as per the `COGNITIVE COMPLEXITY by SonarSource
|
||||
<https://www.sonarsource.com/docs/CognitiveComplexity.pdf>`_ specification
|
||||
version 1.2 (19 April 2017).
|
||||
|
||||
Options
|
||||
-------
|
||||
|
||||
.. option:: Threshold
|
||||
|
||||
Flag functions with Cognitive Complexity exceeding this number.
|
||||
The default is `25`.
|
||||
|
||||
Building blocks
|
||||
---------------
|
||||
|
||||
There are three basic building blocks of a Cognitive Complexity metric:
|
||||
|
||||
Increment
|
||||
^^^^^^^^^
|
||||
|
||||
The following structures increase the function's Cognitive Complexity metric
|
||||
(by `1`):
|
||||
|
||||
* Conditional operators:
|
||||
|
||||
- ``if()``
|
||||
- ``else if()``
|
||||
- ``else``
|
||||
- ``cond ? true : false``
|
||||
|
||||
* ``switch()``
|
||||
* Loops:
|
||||
|
||||
- ``for()``
|
||||
- C++11 range-based ``for()``
|
||||
- ``while()``
|
||||
- ``do while()``
|
||||
|
||||
* ``catch ()``
|
||||
* ``goto LABEL``, ``goto *(&&LABEL))``,
|
||||
* sequences of binary logical operators:
|
||||
|
||||
- ``boolean1 || boolean2``
|
||||
- ``boolean1 && boolean2``
|
||||
|
||||
Nesting level
|
||||
^^^^^^^^^^^^^
|
||||
|
||||
While by itself the nesting level not change the function's Cognitive Complexity
|
||||
metric, it is tracked, and is used by the next, third building block.
|
||||
The following structures increase the nesting level (by `1`):
|
||||
|
||||
* Conditional operators:
|
||||
|
||||
- ``if()``
|
||||
- ``else if()``
|
||||
- ``else``
|
||||
- ``cond ? true : false``
|
||||
|
||||
* ``switch()``
|
||||
* Loops:
|
||||
|
||||
- ``for()``
|
||||
- C++11 range-based ``for()``
|
||||
- ``while()``
|
||||
- ``do while()``
|
||||
|
||||
* ``catch ()``
|
||||
* Nested functions:
|
||||
|
||||
- C++11 Lambda
|
||||
- Nested ``class``
|
||||
- Nested ``struct``
|
||||
* GNU statement expression
|
||||
* Apple Block Declaration
|
||||
|
||||
Nesting increment
|
||||
^^^^^^^^^^^^^^^^^
|
||||
|
||||
This is where the previous basic building block, `Nesting level`_, matters.
|
||||
The following structures increase the function's Cognitive Complexity metric by
|
||||
the current `Nesting level`_:
|
||||
|
||||
* Conditional operators:
|
||||
|
||||
- ``if()``
|
||||
- ``cond ? true : false``
|
||||
|
||||
* ``switch()``
|
||||
* Loops:
|
||||
|
||||
- ``for()``
|
||||
- C++11 range-based ``for()``
|
||||
- ``while()``
|
||||
- ``do while()``
|
||||
|
||||
* ``catch ()``
|
||||
|
||||
Examples
|
||||
--------
|
||||
|
||||
The simplest case. This function has Cognitive Complexity of `0`.
|
||||
|
||||
.. code-block:: c++
|
||||
|
||||
void function0() {}
|
||||
|
||||
Slightly better example. This function has Cognitive Complexity of `1`.
|
||||
|
||||
.. code-block:: c++
|
||||
|
||||
int function1(bool var) {
|
||||
if(var) // +1, nesting level +1
|
||||
return 42;
|
||||
return 0;
|
||||
}
|
||||
|
||||
Full example. This function has Cognitive Complexity of `3`.
|
||||
|
||||
.. code-block:: c++
|
||||
|
||||
int function3(bool var1, bool var2) {
|
||||
if(var1) { // +1, nesting level +1
|
||||
if(var2) // +2 (1 + current nesting level of 1), nesting level +1
|
||||
return 42;
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
Limitations
|
||||
-----------
|
||||
|
||||
The metric is implemented with two notable exceptions:
|
||||
* `preprocessor conditionals` (``#ifdef``, ``#if``, ``#elif``, ``#else``,
|
||||
``#endif``) are not accounted for.
|
||||
* `each method in a recursion cycle` is not accounted for. It can't be fully
|
||||
implemented, because cross-translational-unit analysis would be needed,
|
||||
which is currently not possible in clang-tidy.
|
File diff suppressed because it is too large
Load Diff
Loading…
Reference in New Issue