Since i initially forgot to include cfe-commits as a subscriber into
the review, the first mail was not sent to the list.
So maybe, for history and clarity, i should forward the mail to the list:
(i'm keeping the reviewers as CC since phabricator, unlike github,
does not add mail-based reviews as comments...)
On Thu, Aug 17, 2017 at 6:57 PM, Roman Lebedev via Phabricator
<[email protected]> wrote:
lebedev.ri created this revision.
lebedev.ri added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, JDevlieghere, mgorny.
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 Cyclomatic complexity
<https://en.wikipedia.org/wiki/Cyclomatic_complexity>, but certainly
has its downsides.
And there is a COGNITIVE COMPLEXITY by SonarSource
<https://www.sonarsource.com/docs/CognitiveComplexity.pdf>, which is
available for opensource on https://sonarcloud.io/.
I did ask them, and received an answer that it is it can be
implemented in clang-tidy.
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 COGNITIVE COMPLEXITY by SonarSource
<https://www.sonarsource.com/docs/CognitiveComplexity.pdf>
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.
Repository:
rL LLVM
https://reviews.llvm.org/D36836
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
clang-tidy/readability/FunctionCognitiveComplexityCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
test/clang-tidy/check_clang_tidy.py
test/clang-tidy/readability-function-cognitive-complexity.cpp
Index: test/clang-tidy/readability-function-cognitive-complexity.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/readability-function-cognitive-complexity.cpp
@@ -0,0 +1,549 @@
+// RUN: %check_clang_tidy %s readability-function-cognitive-complexity %t -- -config='{CheckOptions: [{key: readability-function-cognitive-complexity.Threshold, value: 0}]}' -- -std=c++11
+
+// any function should be checked.
+
+extern int ext_func(int x = 0);
+
+int some_func(int x = 0);
+
+static int some_other_func(int x = 0) {}
+
+template<typename T> void some_templ_func(T x = 0) {}
+
+class SomeClass {
+public:
+ int *begin(int x = 0);
+ int *end(int x = 0);
+ static int func(int x = 0);
+ template<typename T> void some_templ_func(T x = 0) {}
+};
+
+// nothing ever decreases cognitive complexity, so we can check all the things
+// in one go. none of the following should increase cognitive complexity:
+void unittest_false() {
+ {};
+ ext_func();
+ some_func();
+ some_other_func();
+ some_templ_func<int>();
+ some_templ_func<bool>();
+ SomeClass::func();
+ SomeClass C;
+ C.some_templ_func<int>();
+ C.some_templ_func<bool>();
+ C.func();
+ C.end();
+ int i = some_func();
+ i = i;
+ i++;
+ --i;
+ i < 0;
+ int j = 0 ?: 1;
+ auto k = new int;
+ delete k;
+ throw i;
+end:
+ return;
+}
+
+//----------------------------------------------------------------------------//
+//------------------------------ B1. Increments ------------------------------//
+//----------------------------------------------------------------------------//
+// Check that every thing listed in B1 of the specification does indeed //
+// recieve the base increment, and that not-body does not increase nesting //
+//----------------------------------------------------------------------------//
+
+// break does not increase cognitive complexity.
+// only break LABEL does, but it is unavaliable in C or C++
+
+// continue does not increase cognitive complexity.
+// only continue LABEL does, but it is unavaliable in C or C++
+
+void unittest_b1_00() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_00' has cognitive complexity of 5 (threshold 0) [readability-function-cognitive-complexity]
+ if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:7: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ } else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:10: note: +1, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:14: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ } else {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +1, nesting level increased to 1{{$}}
+ }
+}
+
+void unittest_b1_01() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_01' has cognitive complexity of 2 (threshold 0) [readability-function-cognitive-complexity]
+ int i = (1 ? 1 : 0) ? 1 : 0;
+// CHECK-NOTES: :[[@LINE-1]]:11: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:12: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// FIXME: would be nice to point at the '?' symbol. does not seem to be possible
+}
+
+void unittest_b1_02(int x) {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_02' has cognitive complexity of 5 (threshold 0) [readability-function-cognitive-complexity]
+ switch (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:11: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ case -1:
+ return;
+ case 1 ? 1 : 0:
+// CHECK-NOTES: :[[@LINE-1]]:8: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ return;
+ case (1 ? 2 : 0) ... (1 ? 3 : 0):
+// CHECK-NOTES: :[[@LINE-1]]:9: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:25: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ return;
+ default:
+ break;
+ }
+}
+
+void unittest_b1_03(int x) {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_03' has cognitive complexity of 4 (threshold 0) [readability-function-cognitive-complexity]
+ for (x = 1 ? 1 : 0; x < (1 ? 1 : 0); x += 1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:12: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-3]]:28: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-4]]:45: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ break;
+ continue;
+ }
+}
+
+void unittest_b1_04() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_04' has cognitive complexity of 2 (threshold 0) [readability-function-cognitive-complexity]
+ SomeClass C;
+ for (int i : (1 ? C : C)) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:17: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ break;
+ continue;
+ }
+}
+
+void unittest_b1_05() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_05' has cognitive complexity of 2 (threshold 0) [readability-function-cognitive-complexity]
+ while (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:10: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ break;
+ continue;
+ }
+}
+
+void unittest_b1_06() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_06' has cognitive complexity of 2 (threshold 0) [readability-function-cognitive-complexity]
+ do {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ break;
+ continue;
+ } while (1 ? 1 : 0);
+// CHECK-NOTES: :[[@LINE-1]]:12: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+}
+
+void unittest_b1_07() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_07' has cognitive complexity of 1 (threshold 0) [readability-function-cognitive-complexity]
+ try {
+ } catch (...) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ }
+}
+
+void unittest_b1_08() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_08' has cognitive complexity of 1 (threshold 0) [readability-function-cognitive-complexity]
+ goto end;
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1{{$}}
+end:
+ return;
+}
+
+void unittest_b1_09_00() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_09_00' has cognitive complexity of 34 (threshold 0) [readability-function-cognitive-complexity]
+ if(1 && 1) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:8: note: +1{{$}}
+ }
+ if(1 && 1 && 1) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:13: note: +1{{$}}
+ }
+ if((1 && 1) && 1) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:15: note: +1{{$}}
+ }
+ if(1 && (1 && 1)) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:8: note: +1{{$}}
+ }
+
+ if(1 && 1 || 1) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:13: note: +1{{$}}
+// CHECK-NOTES: :[[@LINE-3]]:8: note: +1{{$}}
+ }
+ if((1 && 1) || 1) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:15: note: +1{{$}}
+// CHECK-NOTES: :[[@LINE-3]]:9: note: +1{{$}}
+ }
+ if(1 && (1 || 1)) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:8: note: +1{{$}}
+// CHECK-NOTES: :[[@LINE-3]]:14: note: +1{{$}}
+ }
+
+ if(1 || 1) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:8: note: +1{{$}}
+ }
+ if(1 || 1 || 1) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:13: note: +1{{$}}
+ }
+ if((1 || 1) || 1) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:15: note: +1{{$}}
+ }
+ if(1 || (1 || 1)) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:8: note: +1{{$}}
+ }
+
+ if(1 || 1 && 1) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:8: note: +1{{$}}
+// CHECK-NOTES: :[[@LINE-3]]:13: note: +1{{$}}
+ }
+ if((1 || 1) && 1) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:15: note: +1{{$}}
+// CHECK-NOTES: :[[@LINE-3]]:9: note: +1{{$}}
+ }
+ if(1 || (1 && 1)) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:8: note: +1{{$}}
+// CHECK-NOTES: :[[@LINE-3]]:14: note: +1{{$}}
+ }
+}
+
+void unittest_b1_09_01() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_09_01' has cognitive complexity of 12 (threshold 0) [readability-function-cognitive-complexity]
+ if(1 && some_func(1 && 1)) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:8: note: +1{{$}}
+// CHECK-NOTES: :[[@LINE-3]]:23: note: +1{{$}}
+ }
+ if(1 && some_func(1 || 1)) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:8: note: +1{{$}}
+// CHECK-NOTES: :[[@LINE-3]]:23: note: +1{{$}}
+ }
+ if(1 || some_func(1 || 1)) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:8: note: +1{{$}}
+// CHECK-NOTES: :[[@LINE-3]]:23: note: +1{{$}}
+ }
+ if(1 || some_func(1 && 1)) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:8: note: +1{{$}}
+// CHECK-NOTES: :[[@LINE-3]]:23: note: +1{{$}}
+ }
+}
+
+// FIXME: each method in a recursion cycle
+
+//----------------------------------------------------------------------------//
+//---------------------------- B2. Nesting lebel -----------------------------//
+//----------------------------------------------------------------------------//
+// Check that every thing listed in B2 of the specification does indeed //
+// increase the nesting level //
+//----------------------------------------------------------------------------//
+
+void unittest_b2_00() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b2_00' has cognitive complexity of 9 (threshold 0) [readability-function-cognitive-complexity]
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ if(true) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ } else if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:10: note: +1, nesting level increased to 1{{$}}
+ if(true) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ } else {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +1, nesting level increased to 1{{$}}
+ if(true) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+}
+
+void unittest_b2_01() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b2_01' has cognitive complexity of 5 (threshold 0) [readability-function-cognitive-complexity]
+ int i = 1 ? (1 ? 1 : 0) : (1 ? 1 : 0);
+// CHECK-NOTES: :[[@LINE-1]]:11: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:16: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-3]]:30: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+// FIXME: would be nice to point at the '?' symbol. does not seem to be possible
+}
+
+void unittest_b2_02(int x) {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b2_02' has cognitive complexity of 3 (threshold 0) [readability-function-cognitive-complexity]
+ switch (x) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ case -1:
+ if(true) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ return;
+ default:
+ return;
+ }
+}
+
+void unittest_b2_03() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b2_03' has cognitive complexity of 3 (threshold 0) [readability-function-cognitive-complexity]
+ for (;;) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ if(true) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+}
+
+void unittest_b2_04() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b2_04' has cognitive complexity of 3 (threshold 0) [readability-function-cognitive-complexity]
+ SomeClass C;
+ for (int i : C) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ if(true) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+}
+
+void unittest_b2_05() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b2_05' has cognitive complexity of 3 (threshold 0) [readability-function-cognitive-complexity]
+ while (true) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ if(true) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+}
+
+void unittest_b2_06() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b2_06' has cognitive complexity of 3 (threshold 0) [readability-function-cognitive-complexity]
+ do {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ if(true) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ } while (true);
+}
+
+void unittest_b2_07() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b2_07' has cognitive complexity of 3 (threshold 0) [readability-function-cognitive-complexity]
+ try {
+ } catch (...) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ if(true) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+}
+
+void unittest_b2_08_00() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b2_08_00' has cognitive complexity of 8 (threshold 0) [readability-function-cognitive-complexity]
+ class X {
+ X() {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: nesting level increased to 1{{$}}
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+
+ ~X() {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: nesting level increased to 1{{$}}
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+
+ void Y() {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: nesting level increased to 1{{$}}
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+
+ static void Z() {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: nesting level increased to 1{{$}}
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+
+// CHECK-NOTES: :[[@LINE-28]]:5: warning: function 'X' has cognitive complexity of 1 (threshold 0) [readability-function-cognitive-complexity]
+// CHECK-NOTES: :[[@LINE-27]]:7: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+
+// CHECK-NOTES: :[[@LINE-24]]:5: warning: function '~X' has cognitive complexity of 1 (threshold 0) [readability-function-cognitive-complexity]
+// CHECK-NOTES: :[[@LINE-23]]:7: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+
+// CHECK-NOTES: :[[@LINE-20]]:10: warning: function 'Y' has cognitive complexity of 1 (threshold 0) [readability-function-cognitive-complexity]
+// CHECK-NOTES: :[[@LINE-19]]:7: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+
+// CHECK-NOTES: :[[@LINE-16]]:17: warning: function 'Z' has cognitive complexity of 1 (threshold 0) [readability-function-cognitive-complexity]
+// CHECK-NOTES: :[[@LINE-15]]:7: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ };
+}
+
+void unittest_b2_08_01() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b2_08_01' has cognitive complexity of 8 (threshold 0) [readability-function-cognitive-complexity]
+ struct X {
+ X() {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: nesting level increased to 1{{$}}
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+
+ ~X() {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: nesting level increased to 1{{$}}
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+
+ void Y() {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: nesting level increased to 1{{$}}
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+
+ static void Z() {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: nesting level increased to 1{{$}}
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+
+// CHECK-NOTES: :[[@LINE-28]]:5: warning: function 'X' has cognitive complexity of 1 (threshold 0) [readability-function-cognitive-complexity]
+// CHECK-NOTES: :[[@LINE-27]]:7: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+
+// CHECK-NOTES: :[[@LINE-24]]:5: warning: function '~X' has cognitive complexity of 1 (threshold 0) [readability-function-cognitive-complexity]
+// CHECK-NOTES: :[[@LINE-23]]:7: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+
+// CHECK-NOTES: :[[@LINE-20]]:10: warning: function 'Y' has cognitive complexity of 1 (threshold 0) [readability-function-cognitive-complexity]
+// CHECK-NOTES: :[[@LINE-19]]:7: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+
+// CHECK-NOTES: :[[@LINE-16]]:17: warning: function 'Z' has cognitive complexity of 1 (threshold 0) [readability-function-cognitive-complexity]
+// CHECK-NOTES: :[[@LINE-15]]:7: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ };
+}
+
+void unittest_b2_08_02() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b2_08_02' has cognitive complexity of 2 (threshold 0) [readability-function-cognitive-complexity]
+ auto fun = []() {
+// CHECK-NOTES: :[[@LINE-1]]:14: note: nesting level increased to 1{{$}}
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ };
+}
+
+//----------------------------------------------------------------------------//
+//-------------------------- B2. Nesting increments --------------------------//
+//----------------------------------------------------------------------------//
+// Check that every thing listed in B3 of the specification does indeed //
+// recieve the penalty of the current nesting level //
+//----------------------------------------------------------------------------//
+
+void unittest_b3_00() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b3_00' has cognitive complexity of 3 (threshold 0) [readability-function-cognitive-complexity]
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+}
+
+void unittest_b3_01() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b3_01' has cognitive complexity of 3 (threshold 0) [readability-function-cognitive-complexity]
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ int i = 1 ? 1 : 0;
+// CHECK-NOTES: :[[@LINE-1]]:13: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+// FIXME: would be nice to point at the '?' symbol. does not seem to be possible
+ }
+}
+
+void unittest_b3_02(int x) {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b3_02' has cognitive complexity of 3 (threshold 0) [readability-function-cognitive-complexity]
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ switch (x) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ case -1:
+ return;
+ default:
+ return;
+ }
+ }
+}
+
+void unittest_b3_03() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b3_03' has cognitive complexity of 3 (threshold 0) [readability-function-cognitive-complexity]
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ for (;;) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+}
+
+void unittest_b3_04() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b3_04' has cognitive complexity of 3 (threshold 0) [readability-function-cognitive-complexity]
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ SomeClass C;
+ for (int i : C) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+}
+
+void unittest_b3_05() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b3_05' has cognitive complexity of 3 (threshold 0) [readability-function-cognitive-complexity]
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ while (true) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+}
+
+void unittest_b3_06() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b3_06' has cognitive complexity of 3 (threshold 0) [readability-function-cognitive-complexity]
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ do {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ } while (true);
+ }
+}
+
+void unittest_b3_07() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b3_07' has cognitive complexity of 3 (threshold 0) [readability-function-cognitive-complexity]
+ if (true) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+ try {
+ } catch (...) {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+ }
+ }
+}
Index: test/clang-tidy/check_clang_tidy.py
===================================================================
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -73,9 +73,11 @@
has_check_fixes = input_text.find('CHECK-FIXES') >= 0
has_check_messages = input_text.find('CHECK-MESSAGES') >= 0
+ has_check_notes = input_text.find('CHECK-NOTES') >= 0
- if not has_check_fixes and not has_check_messages:
- sys.exit('Neither CHECK-FIXES nor CHECK-MESSAGES found in the input')
+ if not has_check_fixes and not has_check_messages and not has_check_notes:
+ sys.exit('Neither CHECK-FIXES nor CHECK-MESSAGES '
+ 'nor CHECK-NOTES found in the input')
# Remove the contents of the CHECK lines to avoid CHECKs matching on
# themselves. We need to keep the comments to preserve line numbers while
@@ -136,5 +138,18 @@
print('FileCheck failed:\n' + e.output.decode())
raise
+ if has_check_notes:
+ messages_file = temp_file_name + '.msg'
+ write_file(messages_file, clang_tidy_output)
+ try:
+ subprocess.check_output(
+ ['FileCheck', '-input-file=' + messages_file, input_file_name,
+ '-check-prefix=CHECK-NOTES',
+ '-implicit-check-not={{note|warning|error}}:'],
+ stderr=subprocess.STDOUT)
+ except subprocess.CalledProcessError as e:
+ print('FileCheck failed:\n' + e.output.decode())
+ raise
+
if __name__ == '__main__':
main()
Index: docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
@@ -0,0 +1,23 @@
+.. title:: clang-tidy - readability-function-cognitive-complexity
+
+readability-function-cognitive-complexity
+=========================================
+
+Checks function Cognitive Complexity metric.
+
+The metric is implemented as per `COGNITIVE COMPLEXITY by SonarSource
+<https://www.sonarsource.com/docs/CognitiveComplexity.pdf>`_ 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.
+ * `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.
+
+Options
+-------
+
+.. option:: Threshold
+
+ Flag functions with Cognitive Complexity exceeding this number.
+ The default is `25`.
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -174,6 +174,7 @@
readability-delete-null-pointer
readability-deleted-default
readability-else-after-return
+ readability-function-cognitive-complexity
readability-function-size
readability-identifier-naming
readability-implicit-bool-conversion
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -126,6 +126,12 @@
Checks if the required file flag ``IN_CLOEXEC`` is present in the argument of
``inotify_init1()``.
+- New `readability-function-cognitive-complexity
+ <http://clang.llvm.org/extra/clang-tidy/checks/readability-function-cognitive-complexity.html>`_ check
+
+ Checks function Cognitive Complexity metric, and flags the functions with
+ Cognitive Complexity exceeding the configured limit.
+
- New `readability-static-accessed-through-instance
<http://clang.llvm.org/extra/clang-tidy/checks/readability-static-accessed-through-instance.html>`_ check
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -16,6 +16,7 @@
#include "DeleteNullPointerCheck.h"
#include "DeletedDefaultCheck.h"
#include "ElseAfterReturnCheck.h"
+#include "FunctionCognitiveComplexityCheck.h"
#include "FunctionSizeCheck.h"
#include "IdentifierNamingCheck.h"
#include "ImplicitBoolConversionCheck.h"
@@ -55,6 +56,8 @@
"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>(
Index: clang-tidy/readability/FunctionCognitiveComplexityCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/readability/FunctionCognitiveComplexityCheck.h
@@ -0,0 +1,44 @@
+//===--- FunctionCognitiveComplexityCheck.h - clang-tidy---------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_FUNCTION_COGNITIVE_COMPLEXITY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_FUNCTION_COGNITIVE_COMPLEXITY_H
+
+#include "../ClangTidy.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_FUNCTION_COGNITIVE_COMPLEXITY_H
Index: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
@@ -0,0 +1,592 @@
+//===--- FunctionCognitiveComplexityCheck.cpp - clang-tidy-----------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "FunctionCognitiveComplexityCheck.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+namespace {
+
+struct CognitiveComplexity final {
+ // Limit of 25 is the "upstream"'s default.
+ static constexpr const unsigned DefaultLimit = 25U;
+
+ // Any increment is based on some combination of reasons.
+ // Here are all the possible reasons:
+ enum Criteria : unsigned char {
+ None = 0,
+
+ // B1, increases cognitive complexity (by 1)
+ Increment = 1 << 0,
+
+ // B2, increases current nesting level (by 1)
+ IncrementNesting = 1 << 1,
+
+ // B3, increases cognitive complexity by the current nesting level
+ // Applied before IncrementNesting
+ PenalizeNesting = 1 << 2,
+
+ All = Increment | PenalizeNesting | IncrementNesting,
+ };
+
+ // All the possible messages that can be outputed. The choice of the message
+ // to use is based of the combination of the Criterias
+ static const std::array<const char *const, 4> Msgs;
+
+ // The helper struct used to record one increment occurence, with all the
+ // details nessesary
+ struct Detail final {
+ const SourceLocation Loc; // what caused the increment?
+ const unsigned short Nesting; // how deeply nestedly is Loc located?
+ const Criteria C : 3; // the criteria of the increment
+
+ Detail(SourceLocation SLoc, unsigned short CurrentNesting, Criteria Crit)
+ : Loc(SLoc), Nesting(CurrentNesting), C(Crit) {}
+
+ // To minimize the 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
+ // occurence actually result in
+ std::pair<const char *, unsigned short> Process() const {
+ assert(C != Criteria::None && "invalid criteria");
+
+ const char *Msg;
+ unsigned short Increment;
+
+ if (C == Criteria::All) {
+ Increment = 1 + Nesting;
+ Msg = Msgs[0];
+ } else if (C == (Criteria::Increment | Criteria::IncrementNesting)) {
+ Increment = 1;
+ Msg = Msgs[1];
+ } else if (C == Criteria::Increment) {
+ Increment = 1;
+ Msg = Msgs[2];
+ } else if (C == Criteria::IncrementNesting) {
+ Msg = Msgs[3];
+ } else
+ llvm_unreachable("should not get to here.");
+
+ return std::make_pair(Msg, Increment);
+ }
+ };
+ // static_assert(sizeof(Detail) <= 8, "it's best to keep the size minimal");
+
+ // 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 allocations 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%
+ 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);
+};
+
+const std::array<const char *const, 4> CognitiveComplexity::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
+static 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));
+}
+static 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));
+}
+static CognitiveComplexity::Criteria &
+operator|=(CognitiveComplexity::Criteria &lhs,
+ CognitiveComplexity::Criteria rhs) {
+ lhs = operator|(lhs, rhs);
+ return lhs;
+}
+static 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());
+
+ const std::pair<const char *, unsigned short> Reasoning(d.Process());
+ Total += Reasoning.second;
+}
+
+class FunctionASTVisitor final
+ : public RecursiveASTVisitor<FunctionASTVisitor> {
+ using Base = RecursiveASTVisitor<FunctionASTVisitor>;
+
+ // Count the current nesting level (increased by Criteria::IncrementNesting)
+ unsigned short CurrentNestingLevel = 0;
+
+ // Used to efficiently know the last type of binary sequence operator that
+ // was encoutered. It would make sense for the function call to start the
+ // new sequence, thus it is a stack.
+ std::stack< Optional<BinaryOperator::Opcode>,
+ SmallVector<Optional<BinaryOperator::Opcode>, 4>>
+ BinaryOperatorsStack;
+
+public:
+
+ // Important piece of the information: nesting level is only increased for the
+ // body, but not the init/cond/inc !
+
+#define IncreasesNestingLevel(CLASS) \
+ bool Traverse##CLASS(CLASS *Node) { \
+ if (!Node) \
+ return Base::Traverse##CLASS(Node); \
+ \
+ ++CurrentNestingLevel; \
+ const bool ShouldContinue = Base::Traverse##CLASS(Node); \
+ --CurrentNestingLevel; \
+ \
+ return ShouldContinue; \
+ }
+
+ // B2. Nesting level
+ // The following structures increment the nesting level:
+ IncreasesNestingLevel(DefaultStmt);
+ IncreasesNestingLevel(CXXCatchStmt);
+ // Other instances are more complex, and need standalone functions
+
+#undef IncreasesNestingLevel
+
+ bool TraverseIfStmt(IfStmt *Node, bool InElseIf = false) {
+ if (!Node)
+ return Base::TraverseIfStmt(Node);
+
+ {
+ CognitiveComplexity::Criteria Reasons =
+ CognitiveComplexity::Criteria::None;
+
+ // if increases cognitive complexity
+ Reasons |= CognitiveComplexity::Criteria::Increment;
+ // if increases nesting level
+ Reasons |= CognitiveComplexity::Criteria::IncrementNesting;
+
+ // if receives a nesting increment commensurate with it's nested depth
+ // but only if it is not part of else if
+ if (!InElseIf)
+ Reasons |= CognitiveComplexity::Criteria::PenalizeNesting;
+
+ CC.Account(Node->getIfLoc(), CurrentNestingLevel, Reasons);
+ }
+
+ bool ShouldContinue = TraverseStmt(Node->getInit());
+ if (!ShouldContinue)
+ return ShouldContinue;
+
+ ShouldContinue = TraverseStmt(Node->getCond());
+ if (!ShouldContinue)
+ return ShouldContinue;
+
+ // if increases nesting level
+ ++CurrentNestingLevel;
+ ShouldContinue = TraverseStmt(Node->getThen());
+ --CurrentNestingLevel;
+
+ if (!ShouldContinue || !Node->getElse())
+ return ShouldContinue;
+
+ // else OR else if increases cognitive complexity and nesting level
+ // but else if does NOT result in double increase !
+ if (isa<IfStmt>(Node->getElse()))
+ return TraverseIfStmt(dyn_cast<IfStmt>(Node->getElse()), true);
+
+ {
+ CognitiveComplexity::Criteria 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);
+ }
+
+ ++CurrentNestingLevel;
+ ShouldContinue = TraverseStmt(Node->getElse());
+ --CurrentNestingLevel;
+
+ return ShouldContinue;
+ }
+
+ bool TraverseConditionalOperator(ConditionalOperator *Node) {
+ if (!Node)
+ return Base::TraverseConditionalOperator(Node);
+
+ bool ShouldContinue = TraverseStmt(Node->getCond());
+ if (!ShouldContinue)
+ return ShouldContinue;
+
+ ++CurrentNestingLevel;
+ ShouldContinue = TraverseStmt(Node->getLHS());
+ --CurrentNestingLevel;
+
+ if (!ShouldContinue)
+ return ShouldContinue;
+
+ ++CurrentNestingLevel;
+ ShouldContinue = TraverseStmt(Node->getRHS());
+ --CurrentNestingLevel;
+
+ return ShouldContinue;
+ }
+
+ bool TraverseCaseStmt(CaseStmt *Node) {
+ if (!Node)
+ return Base::TraverseCaseStmt(Node);
+
+ bool ShouldContinue = TraverseStmt(Node->getLHS());
+ if (!ShouldContinue)
+ return ShouldContinue;
+
+ ShouldContinue = TraverseStmt(Node->getRHS());
+ if (!ShouldContinue)
+ return ShouldContinue;
+
+ ++CurrentNestingLevel;
+ ShouldContinue = TraverseStmt(Node->getSubStmt());
+ --CurrentNestingLevel;
+
+ return ShouldContinue;
+ }
+
+ bool TraverseForStmt(ForStmt *Node) {
+ if (!Node)
+ return Base::TraverseForStmt(Node);
+
+ bool ShouldContinue = TraverseStmt(Node->getInit());
+ if (!ShouldContinue)
+ return ShouldContinue;
+
+ ShouldContinue = TraverseStmt(Node->getCond());
+ if (!ShouldContinue)
+ return ShouldContinue;
+
+ ShouldContinue = TraverseStmt(Node->getInc());
+ if (!ShouldContinue)
+ return ShouldContinue;
+
+ ++CurrentNestingLevel;
+ ShouldContinue = TraverseStmt(Node->getBody());
+ --CurrentNestingLevel;
+
+ return ShouldContinue;
+ }
+
+ bool TraverseCXXForRangeStmt(CXXForRangeStmt *Node) {
+ if (!Node)
+ return Base::TraverseCXXForRangeStmt(Node);
+
+ bool ShouldContinue = TraverseDecl(Node->getLoopVariable());
+ if (!ShouldContinue)
+ return ShouldContinue;
+
+ ShouldContinue = TraverseStmt(Node->getRangeInit());
+ if (!ShouldContinue)
+ return ShouldContinue;
+
+ ++CurrentNestingLevel;
+ ShouldContinue = TraverseStmt(Node->getBody());
+ --CurrentNestingLevel;
+
+ return ShouldContinue;
+ }
+
+ bool TraverseWhileStmt(WhileStmt *Node) {
+ if (!Node)
+ return Base::TraverseWhileStmt(Node);
+
+ bool ShouldContinue = TraverseStmt(Node->getCond());
+ if (!ShouldContinue)
+ return ShouldContinue;
+
+ ++CurrentNestingLevel;
+ ShouldContinue = TraverseStmt(Node->getBody());
+ --CurrentNestingLevel;
+
+ return ShouldContinue;
+ }
+
+ bool TraverseDoStmt(DoStmt *Node) {
+ if (!Node)
+ return Base::TraverseDoStmt(Node);
+
+ bool ShouldContinue = TraverseStmt(Node->getCond());
+ if (!ShouldContinue)
+ return ShouldContinue;
+
+ ++CurrentNestingLevel;
+ ShouldContinue = TraverseStmt(Node->getBody());
+ --CurrentNestingLevel;
+
+ return ShouldContinue;
+ }
+
+// The currently-being-processed stack entry, which is always the top
+#define CurrentBinaryOperator BinaryOperatorsStack.top()
+
+#define TraverseBinOp(CLASS) \
+ bool TraverseBin##CLASS(BinaryOperator *Op) { \
+ if (!Op) \
+ return Base::TraverseBin##CLASS(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 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(); \
+ const bool ShouldContinue = Base::TraverseBin##CLASS(Op); \
+ /* And restore the previous binary operator, which might be nonexistant */ \
+ CurrentBinaryOperator = BinOpCopy; \
+ \
+ return ShouldContinue; \
+ }
+
+ // In a sequence of binary logical operators, if new operator is different
+ // from the previous-one, then the cognitive complexity is increased.
+ TraverseBinOp(LAnd);
+ TraverseBinOp(LOr);
+
+#define TraverseCallExpr_(CLASS) \
+ bool Traverse##CLASS(CLASS *Call) { \
+ /* If we are not currently processing any binary operator sequence, then \
+ * no special-handling is needed */ \
+ if (!Call || BinaryOperatorsStack.empty() || !CurrentBinaryOperator) \
+ return Base::Traverse##CLASS(Call); \
+ \
+ /* Else, do add [uninitialized] frame to the stack, and traverse call */ \
+ BinaryOperatorsStack.emplace(); \
+ const bool ShouldContinue = Base::Traverse##CLASS(Call); \
+ /* And remove the top frame */ \
+ BinaryOperatorsStack.pop(); \
+ \
+ 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 new stack frame
+ TraverseCallExpr_(CallExpr);
+ TraverseCallExpr_(CXXMemberCallExpr);
+ TraverseCallExpr_(CXXOperatorCallExpr);
+
+ 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;
+
+ // 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
+ case Stmt::ConditionalOperatorClass:
+ case Stmt::SwitchStmtClass:
+ case Stmt::ForStmtClass:
+ case Stmt::CXXForRangeStmtClass:
+ case Stmt::WhileStmtClass:
+ case Stmt::DoStmtClass:
+ case Stmt::CXXCatchStmtClass:
+ case Stmt::GotoStmtClass:
+ 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:
+ 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 we have found any reasons, let's account it.
+ if (Reasons & CognitiveComplexity::Criteria::All)
+ CC.Account(Node->getLocStart(), CurrentNestingLevel, Reasons);
+
+ return Base::TraverseStmt(Node);
+ }
+
+ 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()) {
+ // lambda is handled in TraverseLambdaExpr().
+ // can't have nested functions, so not accounted for
+ case Decl::CXXConstructor:
+ case Decl::CXXDestructor:
+ case Decl::CXXMethod:
+ break;
+ default:
+ return Base::TraverseDecl(Node);
+ break;
+ }
+
+ CC.Account(Node->getLocStart(), CurrentNestingLevel,
+ CognitiveComplexity::Criteria::IncrementNesting);
+
+ ++CurrentNestingLevel;
+ const bool ShouldContinue = Base::TraverseDecl(Node);
+ --CurrentNestingLevel;
+
+ return ShouldContinue;
+ }
+
+ bool TraverseLambdaExpr(LambdaExpr *Node) {
+ if (!Node)
+ return Base::TraverseLambdaExpr(Node);
+
+ CC.Account(Node->getLocStart(), CurrentNestingLevel,
+ CognitiveComplexity::Criteria::IncrementNesting);
+
+ ++CurrentNestingLevel;
+ const bool ShouldContinue = Base::TraverseLambdaExpr(Node);
+ --CurrentNestingLevel;
+
+ return ShouldContinue;
+ }
+
+ 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(unless(anyOf(isInstantiated(), isImplicit()))).bind("func"),
+ this);
+}
+
+void FunctionCognitiveComplexityCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");
+
+ 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;
+
+ for (const auto &Detail : Visitor.CC.Details) {
+ const std::pair<const char *, unsigned short> Reasoning(Detail.Process());
+ diag(Detail.Loc, Reasoning.first, DiagnosticIDs::Note)
+ << Reasoning.second << Detail.Nesting << 1 + Detail.Nesting;
+ }
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tidy/readability/CMakeLists.txt
+++ clang-tidy/readability/CMakeLists.txt
@@ -7,6 +7,7 @@
DeleteNullPointerCheck.cpp
DeletedDefaultCheck.cpp
ElseAfterReturnCheck.cpp
+ FunctionCognitiveComplexityCheck.cpp
FunctionSizeCheck.cpp
IdentifierNamingCheck.cpp
ImplicitBoolConversionCheck.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
