[clang-tidy] readability-identifier-naming disregards parameters restrictions on...

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

diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index 6a39ece..119502e 100644 (file)
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -8,15 +8,13 @@
 
 #include "IdentifierNamingCheck.h"
 
-#include "../utils/ASTUtils.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/AST/CXXInheritance.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/Regex.h"
 
 #define DEBUG_TYPE "clang-tidy"
 
@@ -126,7 +124,9 @@ private:
 
 IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
                                              ClangTidyContext *Context)
-    : RenamerClangTidyCheck(Name, Context) {
+    : RenamerClangTidyCheck(Name, Context),
+      IgnoreFailedSplit(Options.get("IgnoreFailedSplit", 0)),
+      IgnoreMainLikeFunctions(Options.get("IgnoreMainLikeFunctions", 0)) {
   auto const fromString = [](StringRef Str) {
     return llvm::StringSwitch<llvm::Optional<CaseType>>(Str)
         .Case("aNy_CasE", CT_AnyCase)
@@ -151,8 +151,6 @@ IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
       NamingStyles.push_back(llvm::None);
     }
   }
-
-  IgnoreFailedSplit = Options.get("IgnoreFailedSplit", 0);
 }
 
 IdentifierNamingCheck::~IdentifierNamingCheck() = default;
@@ -193,6 +191,7 @@ void IdentifierNamingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   }
 
   Options.store(Opts, "IgnoreFailedSplit", IgnoreFailedSplit);
+  Options.store(Opts, "IgnoreMainLikeFunctions", IgnoreMainLikeFunctions);
 }
 
 static bool matchesStyle(StringRef Name,
@@ -324,6 +323,67 @@ static std::string fixupWithCase(StringRef Name,
   return Fixup;
 }
 
+static bool isParamInMainLikeFunction(const ParmVarDecl &ParmDecl,
+                                      bool IncludeMainLike) {
+  const auto *FDecl =
+      dyn_cast_or_null<FunctionDecl>(ParmDecl.getParentFunctionOrMethod());
+  if (!FDecl)
+    return false;
+  if (FDecl->isMain())
+    return true;
+  if (!IncludeMainLike)
+    return false;
+  if (FDecl->getAccess() != AS_public && FDecl->getAccess() != AS_none)
+    return false;
+  enum MainType { None, Main, WMain };
+  auto IsCharPtrPtr = [](QualType QType) -> MainType {
+    if (QType.isNull())
+      return None;
+    if (QType = QType->getPointeeType(), QType.isNull())
+      return None;
+    if (QType = QType->getPointeeType(), QType.isNull())
+      return None;
+    if (QType->isCharType())
+      return Main;
+    if (QType->isWideCharType())
+      return WMain;
+    return None;
+  };
+  auto IsIntType = [](QualType QType) {
+    if (QType.isNull())
+      return false;
+    if (const auto *Builtin =
+            dyn_cast<BuiltinType>(QType->getUnqualifiedDesugaredType())) {
+      return Builtin->getKind() == BuiltinType::Int;
+    }
+    return false;
+  };
+  if (!IsIntType(FDecl->getReturnType()))
+    return false;
+  if (FDecl->getNumParams() < 2 || FDecl->getNumParams() > 3)
+    return false;
+  if (!IsIntType(FDecl->parameters()[0]->getType()))
+    return false;
+  MainType Type = IsCharPtrPtr(FDecl->parameters()[1]->getType());
+  if (Type == None)
+    return false;
+  if (FDecl->getNumParams() == 3 &&
+      IsCharPtrPtr(FDecl->parameters()[2]->getType()) != Type)
+    return false;
+
+  if (Type == Main) {
+    static llvm::Regex Matcher(
+        "(^[Mm]ain([_A-Z]|$))|([a-z0-9_]Main([_A-Z]|$))|(_main(_|$))");
+    assert(Matcher.isValid() && "Invalid Matcher for main like functions.");
+    return Matcher.match(FDecl->getName());
+  } else {
+    static llvm::Regex Matcher("(^((W[Mm])|(wm))ain([_A-Z]|$))|([a-z0-9_]W[Mm]"
+                               "ain([_A-Z]|$))|(_wmain(_|$))");
+    assert(Matcher.isValid() && "Invalid Matcher for wmain like functions.");
+    return Matcher.match(FDecl->getName());
+  }
+}
+
 static std::string
 fixupWithStyle(StringRef Name,
                const IdentifierNamingCheck::NamingStyle &Style) {
@@ -338,7 +398,8 @@ fixupWithStyle(StringRef Name,
 static StyleKind findStyleKind(
     const NamedDecl *D,
     const std::vector<llvm::Optional<IdentifierNamingCheck::NamingStyle>>
-        &NamingStyles) {
+        &NamingStyles,
+    bool IgnoreMainLikeFunctions) {
   assert(D && D->getIdentifier() && !D->getName().empty() && !D->isImplicit() &&
          "Decl must be an explicit identifier with a name.");
 
@@ -434,6 +495,8 @@ static StyleKind findStyleKind(
   }
 
   if (const auto *Decl = dyn_cast<ParmVarDecl>(D)) {
+    if (isParamInMainLikeFunction(*Decl, IgnoreMainLikeFunctions))
+      return SK_Invalid;
     QualType Type = Decl->getType();
 
     if (Decl->isConstexpr() && NamingStyles[SK_ConstexprVariable])
@@ -615,7 +678,7 @@ static StyleKind findStyleKind(
 llvm::Optional<RenamerClangTidyCheck::FailureInfo>
 IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl,
                                           const SourceManager &SM) const {
-  StyleKind SK = findStyleKind(Decl, NamingStyles);
+  StyleKind SK = findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions);
   if (SK == SK_Invalid)
     return None;
 

diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
index 86cd52f..04bf53f 100644 (file)
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@@ -70,7 +70,8 @@ private:
                        const NamingCheckFailure &Failure) const override;
 
   std::vector<llvm::Optional<NamingStyle>> NamingStyles;
-  bool IgnoreFailedSplit;
+  const bool IgnoreFailedSplit;
+  const bool IgnoreMainLikeFunctions;
 };
 
 } // namespace readability

diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
index c419b03..0998bf2 100644 (file)
--- a/clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
@@ -55,6 +55,7 @@ The following options are describe below:
  - :option:`GlobalFunctionCase`, :option:`GlobalFunctionPrefix`, :option:`GlobalFunctionSuffix`
  - :option:`GlobalPointerCase`, :option:`GlobalPointerPrefix`, :option:`GlobalPointerSuffix`
  - :option:`GlobalVariableCase`, :option:`GlobalVariablePrefix`, :option:`GlobalVariableSuffix`
+ - :option:`IgnoreMainLikeFunctions`
  - :option:`InlineNamespaceCase`, :option:`InlineNamespacePrefix`, :option:`InlineNamespaceSuffix`
  - :option:`LocalConstantCase`, :option:`LocalConstantPrefix`, :option:`LocalConstantSuffix`
  - :option:`LocalConstantPointerCase`, :option:`LocalConstantPointerPrefix`, :option:`LocalConstantPointerSuffix`
@@ -827,6 +828,12 @@ After:
 
     int pre_global3_post;
 
+.. option:: IgnoreMainLikeFunctions
+
+    When set to `1` functions that have a similar signature to ``main`` or 
+    ``wmain`` won't enforce checks on the names of their parameters.
+    Default value is `0`.
+
 .. option:: InlineNamespaceCase
 
     When defined, the check will ensure inline namespaces names conform to the

diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-main-like.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-main-like.cpp
new file mode 100644 (file)
index 0000000..6336ffb
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-main-like.cpp
@@ -0,0 +1,88 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: [ \
+// RUN:     {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
+// RUN:     {key: readability-identifier-naming.IgnoreMainLikeFunctions, value: 1} \
+// RUN:  ]}'
+
+int mainLike(int argc, char **argv);
+int mainLike(int argc, char **argv, const char **env);
+int mainLike(int argc, const char **argv);
+int mainLike(int argc, const char **argv, const char **env);
+int mainLike(int argc, char *argv[]);
+int mainLike(int argc, const char *argv[]);
+int mainLike(int argc, char *argv[], char *env[]);
+int mainLike(int argc, const char *argv[], const char *env[]);
+void notMain(int argc, char **argv);
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:31: warning: invalid case style for parameter 'argv'
+void notMain(int argc, char **argv, char **env);
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:31: warning: invalid case style for parameter 'argv'
+// CHECK-MESSAGES: :[[@LINE-3]]:44: warning: invalid case style for parameter 'env'
+int notMain(int argc, char **argv, char **env, int Extra);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'argv'
+// CHECK-MESSAGES: :[[@LINE-3]]:43: warning: invalid case style for parameter 'env'
+int notMain(int argc, char **argv, int Extra);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'argv'
+int notMain(int argc, char *argv);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: invalid case style for parameter 'argv'
+int notMain(unsigned argc, char **argv);
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:35: warning: invalid case style for parameter 'argv'
+int notMain(long argc, char *argv);
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'argv'
+int notMain(int argc, char16_t **argv);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:34: warning: invalid case style for parameter 'argv'
+int notMain(int argc, char argv[]);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:28: warning: invalid case style for parameter 'argv'
+typedef char myFunChar;
+typedef int myFunInt;
+typedef char **myFunCharPtr;
+typedef long myFunLong;
+myFunInt mainLikeTypedef(myFunInt argc, myFunChar **argv);
+int mainLikeTypedef(int argc, myFunCharPtr argv);
+int notMainTypedef(myFunLong argc, char **argv);
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:43: warning: invalid case style for parameter 'argv'
+
+// Don't flag as name contains the word main
+int myMainFunction(int argc, char *argv[]);
+
+// This is fine, named with wmain and has wchar ptr.
+int wmainLike(int argc, wchar_t *argv[]);
+
+// Flag this as has signature of main, but named as wmain.
+int wmainLike(int argc, char *argv[]);
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:31: warning: invalid case style for parameter 'argv'
+
+struct Foo {
+  Foo(int argc, char *argv[]) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: invalid case style for parameter 'argc'
+  // CHECK-MESSAGES: :[[@LINE-2]]:23: warning: invalid case style for parameter 'argv'
+
+  int mainPub(int argc, char *argv[]);
+  static int mainPubStatic(int argc, char *argv[]);
+
+protected:
+  int mainProt(int argc, char *argv[]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for parameter 'argc'
+  // CHECK-MESSAGES: :[[@LINE-2]]:32: warning: invalid case style for parameter 'argv'
+  static int mainProtStatic(int argc, char *argv[]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: invalid case style for parameter 'argc'
+  // CHECK-MESSAGES: :[[@LINE-2]]:45: warning: invalid case style for parameter 'argv'
+
+private:
+  int mainPriv(int argc, char *argv[]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for parameter 'argc'
+  // CHECK-MESSAGES: :[[@LINE-2]]:32: warning: invalid case style for parameter 'argv'
+  static int mainPrivStatic(int argc, char *argv[]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: invalid case style for parameter 'argc'
+  // CHECK-MESSAGES: :[[@LINE-2]]:45: warning: invalid case style for parameter 'argv'
+};