Revert "[clang-tidy] readability-identifier-naming disregards parameters restrictions on main like functions"

This reverts commit 27e3671ff4.

This was an accidental push, and the author requested a revert on IRC as
their local branch is in a bad state.

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

@@ -8,13 +8,15 @@
 
 #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"
 
@@ -124,9 +126,7 @@ private:
 
 IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
                                              ClangTidyContext *Context)
-    : RenamerClangTidyCheck(Name, Context),
-      IgnoreFailedSplit(Options.get("IgnoreFailedSplit", 0)),
-      IgnoreMainLikeFunctions(Options.get("IgnoreMainLikeFunctions", 0)) {
+    : RenamerClangTidyCheck(Name, Context) {
   auto const fromString = [](StringRef Str) {
     return llvm::StringSwitch<llvm::Optional<CaseType>>(Str)
         .Case("aNy_CasE", CT_AnyCase)
@@ -151,6 +151,8 @@ IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
       NamingStyles.push_back(llvm::None);
     }
   }
+
+  IgnoreFailedSplit = Options.get("IgnoreFailedSplit", 0);
 }
 
 IdentifierNamingCheck::~IdentifierNamingCheck() = default;
@@ -191,7 +193,6 @@ void IdentifierNamingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   }
 
   Options.store(Opts, "IgnoreFailedSplit", IgnoreFailedSplit);
-  Options.store(Opts, "IgnoreMainLikeFunctions", IgnoreMainLikeFunctions);
 }
 
 static bool matchesStyle(StringRef Name,
@@ -323,67 +324,6 @@ 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) {
@@ -398,8 +338,7 @@ fixupWithStyle(StringRef Name,
 static StyleKind findStyleKind(
     const NamedDecl *D,
     const std::vector<llvm::Optional<IdentifierNamingCheck::NamingStyle>>
-        &NamingStyles,
-    bool IgnoreMainLikeFunctions) {
+        &NamingStyles) {
   assert(D && D->getIdentifier() && !D->getName().empty() && !D->isImplicit() &&
          "Decl must be an explicit identifier with a name.");
 
@@ -495,8 +434,6 @@ 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])
@@ -678,7 +615,7 @@ static StyleKind findStyleKind(
 llvm::Optional<RenamerClangTidyCheck::FailureInfo>
 IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl,
                                           const SourceManager &SM) const {
-  StyleKind SK = findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions);
+  StyleKind SK = findStyleKind(Decl, NamingStyles);
   if (SK == SK_Invalid)
     return None;
 

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h

@@ -70,8 +70,7 @@ private:
                        const NamingCheckFailure &Failure) const override;
 
   std::vector<llvm::Optional<NamingStyle>> NamingStyles;
-  const bool IgnoreFailedSplit;
-  const bool IgnoreMainLikeFunctions;
+  bool IgnoreFailedSplit;
 };
 
 } // namespace readability

clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp

@@ -14,6 +14,8 @@
 #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"
 
 #define DEBUG_TYPE "clang-tidy"
 
@@ -104,22 +106,20 @@ void RenamerClangTidyCheck::registerMatchers(MatchFinder *Finder) {
                      this);
   Finder->addMatcher(typeLoc().bind("typeLoc"), this);
   Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this);
-  auto MemberOrDependent =
-      expr(eachOf(memberExpr().bind("memberExpr"),
-                  cxxDependentScopeMemberExpr().bind("depMemberExpr")));
   Finder->addMatcher(
       functionDecl(unless(cxxMethodDecl(isImplicit())),
-                   hasBody(forEachDescendant(MemberOrDependent))),
+                   hasBody(forEachDescendant(memberExpr().bind("memberExpr")))),
       this);
   Finder->addMatcher(
-      cxxConstructorDecl(unless(isImplicit()),
-                         forEachConstructorInitializer(allOf(
-                             isWritten(), withInitializer(forEachDescendant(
-                                              MemberOrDependent))))),
-      this);
-  Finder->addMatcher(
-      fieldDecl(hasInClassInitializer(forEachDescendant(MemberOrDependent))),
+      cxxConstructorDecl(
+          unless(isImplicit()),
+          forEachConstructorInitializer(
+              allOf(isWritten(), withInitializer(forEachDescendant(
+                                     memberExpr().bind("memberExpr")))))),
       this);
+  Finder->addMatcher(fieldDecl(hasInClassInitializer(
+                         forEachDescendant(memberExpr().bind("memberExpr")))),
+                     this);
 }
 
 void RenamerClangTidyCheck::registerPPCallbacks(
@@ -271,39 +271,6 @@ void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) {
     return;
   }
 
-  if (const auto *DepMemberRef =
-          Result.Nodes.getNodeAs<CXXDependentScopeMemberExpr>(
-              "depMemberExpr")) {
-    QualType BaseType = DepMemberRef->isArrow()
-                            ? DepMemberRef->getBaseType()->getPointeeType()
-                            : DepMemberRef->getBaseType();
-    if (BaseType.isNull())
-      return;
-    const CXXRecordDecl *Base = BaseType.getTypePtr()->getAsCXXRecordDecl();
-    if (!Base)
-      return;
-    DeclarationName DeclName = DepMemberRef->getMemberNameInfo().getName();
-    if (!DeclName.isIdentifier())
-      return;
-    StringRef DependentName = DeclName.getAsIdentifierInfo()->getName();
-    for (const FieldDecl *Field : Base->fields()) {
-      if (Field->getParent() == Base && Field->getDeclName().isIdentifier() &&
-          Field->getName().equals(DependentName)) {
-        SourceRange Range = DepMemberRef->getMemberNameInfo().getSourceRange();
-        addUsage(NamingCheckFailures, Field, Range, Result.SourceManager);
-        return;
-      }
-    }
-    for (const CXXMethodDecl *Method : Base->methods()) {
-      if (Method->getParent() == Base && Method->getDeclName().isIdentifier() &&
-          Method->getName().equals(DependentName)) {
-        SourceRange Range = DepMemberRef->getMemberNameInfo().getSourceRange();
-        addUsage(NamingCheckFailures, Method, Range, Result.SourceManager);
-        return;
-      }
-    }
-  }
-
   if (const auto *Decl = Result.Nodes.getNodeAs<NamedDecl>("decl")) {
     if (!Decl->getIdentifier() || Decl->getName().empty() || Decl->isImplicit())
       return;

clang-tools-extra/docs/ReleaseNotes.rst

@@ -104,11 +104,6 @@ New aliases
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-- Improved :doc:'readability-identifier-naming
-  <clang-tidy/checks/readability-identifier-naming>` check.
-
-  Now able to rename member references in class template definitions with 
-  explicit access.
 
 Renamed checks
 ^^^^^^^^^^^^^^

clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst

@@ -55,7 +55,6 @@ 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`
@@ -828,12 +827,6 @@ 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

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-main-like.cpp

@@ -1,88 +0,0 @@
-// 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'
-};

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp

@@ -1,8 +1,7 @@
 // RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
 // RUN:   -config='{CheckOptions: [ \
 // RUN:     {key: readability-identifier-naming.MemberCase, value: CamelCase}, \
-// RUN:     {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
-// RUN:     {key: readability-identifier-naming.MethodCase, value: camelBack} \
+// RUN:     {key: readability-identifier-naming.ParameterCase, value: CamelCase} \
 // RUN:  ]}' -- -fno-delayed-template-parsing
 
 int set_up(int);
@@ -84,12 +83,12 @@ public:
 }; //NOLINT
 }; // namespace std
 
-class Foo {
+class Foo { 
   std::vector<bool> &stack;
   // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: invalid case style for member 'stack' [readability-identifier-naming]
 public:
   Foo(std::vector<bool> &stack)
-      // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for parameter 'stack' [readability-identifier-naming]
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for parameter 'stack' [readability-identifier-naming]
       // CHECK-FIXES: {{^}}  Foo(std::vector<bool> &Stack)
       : stack(stack) {
     // CHECK-FIXES: {{^}}      : Stack(Stack) {
@@ -135,94 +134,4 @@ public:
 void foo() {
   Container<int, 5> container;
 }
-} // namespace CtorInits
-
-namespace resolved_dependance {
-template <typename T>
-struct A0 {
-  int value;
-  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for member 'value'
-  A0 &operator=(const A0 &Other) {
-    value = Other.value;       // A0
-    this->value = Other.value; // A0
-    // CHECK-FIXES:      {{^}}    Value = Other.Value;       // A0
-    // CHECK-FIXES-NEXT: {{^}}    this->Value = Other.Value; // A0
-    return *this;
-  }
-  void outOfLineReset();
-};
-
-template <typename T>
-void A0<T>::outOfLineReset() {
-  this->value -= value; // A0
-  // CHECK-FIXES: {{^}}  this->Value -= Value; // A0
-}
-
-template <typename T>
-struct A1 {
-  int value; // A1
-  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for member 'value'
-  // CHECK-FIXES: {{^}}  int Value; // A1
-  int GetValue() const { return value; } // A1
-  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for method 'GetValue'
-  // CHECK-FIXES {{^}}  int getValue() const { return Value; } // A1
-  void SetValue(int Value) { this->value = Value; } // A1
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for method 'SetValue'
-  // CHECK-FIXES {{^}}  void setValue(int Value) { this->Value = Value; } // A1
-  A1 &operator=(const A1 &Other) {
-    this->SetValue(Other.GetValue()); // A1
-    this->value = Other.value;        // A1
-    // CHECK-FIXES:      {{^}}    this->setValue(Other.getValue()); // A1
-    // CHECK-FIXES-NEXT: {{^}}    this->Value = Other.Value;        // A1
-    return *this;
-  }
-  void outOfLineReset();
-};
-
-template <typename T>
-void A1<T>::outOfLineReset() {
-  this->value -= value; // A1
-  // CHECK-FIXES: {{^}}  this->Value -= Value; // A1
-}
-
-template <unsigned T>
-struct A2 {
-  int value; // A2
-  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for member 'value'
-  // CHECK-FIXES: {{^}}  int Value; // A2
-  A2 &operator=(const A2 &Other) {
-    value = Other.value;       // A2
-    this->value = Other.value; // A2
-    // CHECK-FIXES:      {{^}}    Value = Other.Value;       // A2
-    // CHECK-FIXES-NEXT: {{^}}    this->Value = Other.Value; // A2
-    return *this;
-  }
-};
-
-// create some instances to check it works when instantiated.
-A1<int> AInt{};
-A1<int> BInt = (AInt.outOfLineReset(), AInt);
-A1<unsigned> AUnsigned{};
-A1<unsigned> BUnsigned = AUnsigned;
-} // namespace resolved_dependance
-
-namespace unresolved_dependance {
-template <typename T>
-struct DependentBase {
-  int depValue;
-  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for member 'depValue'
-};
-
-template <typename T>
-struct Derived : DependentBase<T> {
-  Derived &operator=(const Derived &Other) {
-    this->depValue = Other.depValue;
-    // CHECK-FIXES-NOT: {{^}}    this->DepValue = Other.DepValue;
-    return *this;
-  }
-};
-
-Derived<int> AInt{};
-Derived<int> BInt = AInt;
-
-} // namespace unresolved_dependance
+}; // namespace CtorInits