[PATCH] D25024: [clang-tidy] Add check for detecting declarations with multiple names

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


> CppCoreGuidelinesTidyModule.cpp:40
> +    CheckFactories.registerCheck<OneNamePerDeclarationCheck>(
> +        "cppcoreguidelines-one-name-per-declaration");
>      CheckFactories.registerCheck<ProBoundsArrayToPointerDecayCheck>(

Can you also register this check under the name `cert-dcl04-c` as well? It fits the behavior of that recommendation too (aside from the exceptions in the recommendation, but those can be handled later). This will also require adding a .rst file for that check name and having it redirect appropriately.

https://www.securecoding.cert.org/confluence/display/c/DCL04-C.+Do+not+declare+more+than+one+variable+per+declaration

> OneNamePerDeclarationCheck.cpp:27
> +void OneNamePerDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
> +  // FIXME: Also provide diagnostics splitting the declarations. E.g.
> +  //

s/diagnostics/fixit ?

> OneNamePerDeclarationCheck.cpp:38
> +  diag(MultipleNameDeclaration->getStartLoc(),
> +       "Do not declare multiple names per declaration");
> +}

Diagnostics do not start with a capital letter. Also, this diagnostic is not really accurate. Consider `void f(int x, int y);` -- that's a single declaration, but it declares multiple names. Perhaps: `do not declare more than one variable per declaration` since this should really be focusing on variable declarations rather than parameters, template parameter lists, etc

> cppcoreguidelines-one-name-per-declaration.cpp:16
> +  return 0;
> +}

Please add tests that show the exceptions in the C++ Core Guidelines are properly handled. Also, I'd like to see tests with other named declarations, such as typedefs, template parameter lists, for loop initializers, etc.

https://reviews.llvm.org/D25024