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 change groups
* Rename: `ignoreParenBaseCasts` -> `IgnoreParenBaseCasts` for uniformity
* Rename: `IgnoreConversionOperator` -> `IgnoreConversionOperatorSingleStep` for uniformity
* Inline `IgnoreNoopCastsSingleStep` into a lambda inside `IgnoreNoopCasts`
* Refactor `IgnoreUnlessSpelledInSource` to make adequate use of `IgnoreExprNodes`
Differential Revision: https://reviews.llvm.org/D86880
When checking for the style of a decl that isn't in the main file, the check will now search for the configuration that the included files uses to gather the style for its decls.
This can be useful to silence warnings in header files that follow a different naming convention without using header-filter to silence all warnings(even from other checks) in the header file.
Reviewed By: aaron.ballman, gribozavr2
Differential Revision: https://reviews.llvm.org/D84814
The previous fix for this, https://reviews.llvm.org/D76761, Passed test cases but failed in the real world as std::string has a non trivial destructor so creates a CXXBindTemporaryExpr.
This handles that shortfall and updates the test case std::basic_string implementation to use a non trivial destructor to reflect real world behaviour.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D84831
Reland b9306fd after fixing the issue causing mac builds to fail unittests.
Following on from D77085, I was never happy with the passing a mapping to the option get/store functions. This patch addresses this by using explicit specializations to handle the serializing and deserializing of enum options.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D82188
Added a 'RefactorConditionVariables' option to control how the check handles condition variables
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D82824
Added some sanity checks to figure out the cause of a (seemingly unrelated) test failure on mac.
These can be removed should no issues arise on that platform again.
Following on from D77085, I was never happy with the passing a mapping to the option get/store functions. This patch addresses this by using explicit specializations to handle the serializing and deserializing of enum options.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D82188
Just adds the storeOptions for Checks that weren't already storing their options.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D82223
Fix various tool libraries not to link to clang's .a libraries and dylib
simultaneously. This may cause breakage, in particular through
duplicate command-line option declarations.
Differential Revision: https://reviews.llvm.org/D81967
Fix a crash in clangd caused by an (admittidly incorrect) Remark diagnositic being emitted from readability-else-after-return.
This crash doesn't occur in clang-tidy so there are no tests there for this.
Reviewed By: hokein
Differential Revision: https://reviews.llvm.org/D81785
Summary:
Finds range-based for loops that can be replaced by a call to ``std::any_of`` or
``std::all_of``. In C++ 20 mode, suggests ``std::ranges::any_of`` or
``std::ranges::all_of``.
For now, no fixits are produced.
Reviewers: aaron.ballman, alexfh, hokein
Subscribers: mgorny, xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77572
Summary:
Sometimes in templated code Member references are reported as `DependentScopeMemberExpr` because that's what the standard dictates, however in many trivial cases it is easy to resolve the reference to its actual Member.
Take this code:
```
template<typename T>
class A{
int value;
A& operator=(const A& Other){
value = Other.value;
this->value = Other.value;
return *this;
}
};
```
When ran with `clang-tidy file.cpp -checks=readability-identifier-naming --config="{CheckOptions: [{key: readability-identifier-naming.MemberPrefix, value: m_}]}" -fix`
Current behaviour:
```
template<typename T>
class A{
int m_value;
A& operator=(const A& Other){
m_value = Other.value;
this->value = Other.value;
return *this;
}
};
```
As `this->value` and `Other.value` are Dependent they are ignored when creating the fix-its, however this can easily be resolved.
Proposed behaviour:
```
template<typename T>
class A{
int m_value;
A& operator=(const A& Other){
m_value = Other.m_value;
this->m_value = Other.m_value;
return *this;
}
};
```
Reviewers: aaron.ballman, JonasToth, alexfh, hokein, gribozavr2
Reviewed By: aaron.ballman
Subscribers: merge_guards_bot, xazax.hun, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D73052
Summary: This was done with a script that looks for calls to Options.get(GlobalOrLocal) that take an integer for the second argument and the result is either compared not equal to 0 or implicitly converted to bool. There may be other occurances
Reviewers: aaron.ballman, alexfh, gribozavr2
Reviewed By: aaron.ballman
Subscribers: wuzish, nemanjai, xazax.hun, kbarton, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77831
This reverts commit 97aa593a83 as it
causes problems (PR45453) https://reviews.llvm.org/D77574#1966321.
This additionally adds an explicit reference to FrontendOpenMP to
clang-tidy where ASTMatchers is used.
This is hopefully just a temporary solution. The dependence on
`FrontendOpenMP` from `ASTMatchers` should be handled by CMake
implicitly, not us explicitly.
Reviewed By: aheejin
Differential Revision: https://reviews.llvm.org/D77666
Summary: Change all checks that take enums as configuration to use enum specific methods in `ClangTidyCheck::OptionsView`.
Reviewers: aaron.ballman, alexfh
Reviewed By: aaron.ballman
Subscribers: wuzish, nemanjai, kbarton, arphaman, xazax.hun, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D76606
Added FixItHint comments to ReservedIdentifierCheck and IdentifierNamingCheck to trick the python scripts into detecting a fix it is provided as it can't see the FixItHints in RenamerClangTidyCheck.cpp
Summary: Adds an option called `AddConstToQualified` to readability-qualified-auto to toggle adding const to the auto typed pointers and references. By default its enabled but in the LLVM module its disabled.
Reviewers: aaron.ballman, alexfh, JonasToth, hokein, sammccall
Reviewed By: aaron.ballman
Subscribers: Quuxplusone, merge_guards_bot, lebedev.ri, xazax.hun, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D73548
This is how it should've been and brings it more in line with
std::string_view. There should be no functional change here.
This is mostly mechanical from a custom clang-tidy check, with a lot of
manual fixups. It uncovers a lot of minor inefficiencies.
This doesn't actually modify StringRef yet, I'll do that in a follow-up.
Summary:
The original behaviour of this check only looked at VarDecls with strings that had an empty string initializer. This has been improved to check for FieldDecls with an in class initializer as well as constructor initializers.
Addresses [[ https://bugs.llvm.org/show_bug.cgi?id=44474 | clang-tidy "modernize-use-default-member-init"/"readability-redundant-string-init" and redundant initializer of std::string ]]
Reviewers: aaron.ballman, alexfh, hokein
Reviewed By: aaron.ballman
Subscribers: merge_guards_bot, mgorny, Eugene.Zelenko, xazax.hun, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D72448
Summary:
Typically most main functions have the signature:
```
int main(int argc, char *argv[])
```
To stick with convention when renaming parameters we should ignore the `argc` and `argv` names even if the parameter style says they should be renamed. This patch addresses this by checking all ParmVarDecls if they form part of a function with a signature that matches main `int name(int argc, char * argv[], (optional char *env[]))`
Reviewers: aaron.ballman, JonasToth, alexfh, hokein
Reviewed By: aaron.ballman
Subscribers: Mordante, merge_guards_bot, xazax.hun, kristof.beyls, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D73098
Summary:
Typically most main functions have the signature:
```
int main(int argc, char *argv[])
```
To stick with convention when renaming parameters we should ignore the `argc` and `argv` names even if the parameter style says they should be renamed. This patch addresses this by checking all ParmVarDecls if they form part of a function with a signature that matches main `int name(int argc, char * argv[], (optional char *env[]))`
Reviewers: aaron.ballman, JonasToth, alexfh, hokein
Reviewed By: aaron.ballman
Subscribers: Mordante, merge_guards_bot, xazax.hun, kristof.beyls, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D73098
Zinovy Nis