[PATCH] D74463: [clang-tidy] Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Feb 16 05:27:11 PST 2020


njames93 added a comment.

I have a feeling this check is going to throw so many false positives that it'll be too noisy to run on any real codebase. 
There should be a way to silence this when it is undesired like in the example for `int max(int a, int b);`. 
A possible solution could be based on parameter name length, usually interchangeable parameters have short names. 
Maybe an option could be added `IgnoreShortParamNamesSize` which takes an int and disregards results if both params are shorter, set as `0` to disable the option



================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.cpp:18
+namespace cppcoreguidelines {
+// FIXME: Make sure this matcher only accepts functions with minimum 2
+// parameters
----------------
You'd need to create a matcher for that

```
AST_MATCHER(FunctionDecl, hasMoreThan2Args) {
  return Node.getNumParams() > 2;
}
```



================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.cpp:29
+      Result.Nodes.getNodeAs<FunctionDecl>("multi-parameter-function");
+  bool adjacentParametersOfTheSameTypeFound = false;
+  auto parameters = MatchedDecl->parameters();
----------------
Please follow the naming convention of CamelCase for all Variables, same goes for all other occurances below


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.cpp:30
+  bool adjacentParametersOfTheSameTypeFound = false;
+  auto parameters = MatchedDecl->parameters();
+  auto parameter = parameters.begin();
----------------
Don't use auto when the type isn't spelled out in the initialisation, the below cases are ok as they are iterators


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:72
 ^^^^^^^^^^
+- New :doc:`cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type
+  <clang-tidy/checks/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type>` check.
----------------
A new line is needed above here


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.rst:29
+int max(int a, int b);
\ No newline at end of file

----------------
New line


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.cpp:58
+// CHECK-MESSAGES: :[[@LINE-3]]:17: warning: function 'func_member3' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
\ No newline at end of file

----------------
New line


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74463/new/

https://reviews.llvm.org/D74463





More information about the cfe-commits mailing list