power-grid-model
power-grid-model copied to clipboard
Resolve code smells from `clang-tidy` on C++ side
The clang-tidy
action checks for static analyzer warnings (similarly to SonarCube). Currently, there are quite some checks that are disabled in the .clang-tidy file. Disabled warnings are prefixed -
in the .clang-tidy
file, e.g. -llvm-else-after-return
disables the llvm-else-after-return
check.
Note that some checks are more easily fixable than others! If you get stuck on one check, just move on to the next one! Check the list of easily fixable checks below.
Desired solution
- Create a branch
- Remove one disabled check from the list of easily fixable checks below
- Check the checkbox on the error you fixed below
- Compile using one of the CMake Presets with
clang-tidy
in the name, as explained in the Documentation, or wait for CI to fail on the given check. - Fix the
clang-tidy
errors- Reasonably good explanations for each error exist in the LLVM docs (just do a Google search on the name of the error).
- Sometimes, the
clang-tidy
error message and following notes provide suggestions on how to fix the issue. Most of the times, these proposed changes are good, but beware about applying them blindly, as sometimes they fail to consider the greater picture. Always double-check
- Create a pull request
It is good practice to fix only one check per pull request to keep the amount of changes manageable.
Compilation of checks
As mentioned above, some clang-tidy checks may be hard to fix, especially for people that are relatively new to C++. Here is a list of good starting points. Again, if you get stuck, don't focus too long on one check but just move on to the next one. There are plenty low hanging fruits left.
Suggestions for easily fixable checks
-
cppcoreguidelines-*
checks are important but sometimes hard to resolve. Here are some of the few exceptions that may be easy regardless:`- [x]
-cppcoreguidelines-narrowing-conversions
- [x]
-cppcoreguidelines-prefer-member-initializer
- [x]
-cppcoreguidelines-special-member-functions
- [x]
-
performance-*
checks are almost always useful and easy to fix!- [x]
-performance-no-automatic-move
- [x]
-performance-unnecessary-copy-initialization
- [x]
-
bugprone-*
checks help write code better, faster- [x]
-bugprone-macro-parentheses
- [x]
-bugprone-narrowing-conversions
- [x]
-
misc-*
are checks that do not really fit into any of the other checks topics. Most are easy to solve.- [x]
-misc-const-correctness
- [x]
- most
readability-*
checks are fairly easy to solve- [x]
-readability-braces-around-statements
- [x]
-readability-container-size-empty
- [x]
-readability-convert-member-functions-to-static
- [x]
-readability-else-after-return
- [x]
-readability-implicit-bool-conversion
- [x]
-readability-isolate-declaration
- [x]
-readability-make-member-function-const
- [x]
-readability-named-parameter
- [x]
-readability-redundant-access-specifiers
- [x]
-readability-redundant-member-init
- [x]
-readability-suspicious-call-argument
- [x]
- some
modernize-*
checks are easy to resolve- [x]
-modernize-concat-nested-namespaces
- [x]
-modernize-deprecated-headers
- [ ]
-modernize-loop-convert
- [x]
-modernize-pass-by-value
- [x]
-modernize-use-auto
- [x]
-modernize-use-default-member-init
- [x]
-modernize-use-emplace
- [x]
-modernize-use-transparent-functors
- [x]
-modernize-use-using
- [x]
Known hard-to-fix checks
Below are a couple of issues that are hard to fix, may take some time, or require more extensive C++ knowledge.
-
cppcoreguidelines-*
- [ ]
-cppcoreguidelines-avoid-c-arrays
- [ ]
-cppcoreguidelines-avoid-magic-numbers
- [ ]
-cppcoreguidelines-init-variables
- [ ]
-cppcoreguidelines-macro-usage
- [ ]
-cppcoreguidelines-no-malloc
- [ ]
-cppcoreguidelines-narrowing-conversions
- [ ]
-cppcoreguidelines-non-private-member-variables-in-classes
- [ ]
-cppcoreguidelines-owning-memory
- [ ]
-cppcoreguidelines-pro-bounds-constant-array-index
- [ ]
-cppcoreguidelines-pro-type-cstyle-cast
- [ ]
-cppcoreguidelines-pro-type-member-init
- [ ]
-cppcoreguidelines-pro-bounds-pointer-arithmetic
- [ ]
-cppcoreguidelines-pro-type-reinterpret-cast
- [ ]
-
bugprone-*
- [ ]
-bugprone-easily-swappable-parameters
- [ ]
-bugprone-exception-escape
- [ ]
-
misc-*
- [ ]
-misc-no-recursion
- [ ]
-misc-non-private-member-variables-in-classes
- [ ]
-misc-include-header-cleaner
- [ ]
-
readability-*
- [ ]
-readability-function-cognitive-complexity
- [ ]
-readability-identifier-length
- [ ]
-readability-magic-numbers
- [ ]
-
modernize-*
- [ ]
-modernize-avoid-bind
- [ ]
-modernize-avoid-c-arrays
- ~[ ]
-modernize-use-nodiscard
~ (we decided not to introduce this into the code due to its verbosity) - [ ]
-modernize-use-trailing-return-type
- [ ]
I removed the good-first-issue
label because all easy-to-fix things were fixed and merged. the remaining issues require more extensive knowledge and/or a discussion about our own opinion on the smells