This revision was automatically updated to reflect the committed changes.
Closed by commit rG4888c9ce97d8: [clang-tidy] readability-identifier-naming 
checks configs for included files (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84814

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/.clang-tidy
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/header.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/.clang-tidy
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/header.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/.clang-tidy
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/header.h
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp
@@ -0,0 +1,64 @@
+// Setup header directory
+
+// RUN: rm -rf %theaders
+// RUN: mkdir %theaders
+// RUN: cp -R %S/Inputs/readability-identifier-naming/. %theaders
+
+// C++11 isn't explicitly required, but failing to specify a standard means the
+// check will run multiple times for different standards. This will cause the
+// second test to fail as the header file will be changed during the first run.
+// InheritParentConfig is needed to look for the clang-tidy configuration files.
+
+// RUN: %check_clang_tidy -check-suffixes=ENABLED,SHARED -std=c++11 %s \
+// RUN: readability-identifier-naming %t -- \
+// RUN:  -config='{ InheritParentConfig: true, CheckOptions: [ \
+// RUN:   {key: readability-identifier-naming.FunctionCase, value: camelBack}, \
+// RUN:   {key: readability-identifier-naming.GetConfigPerFile, value: true} \
+// RUN:  ]}' -header-filter='.*' -- -I%theaders
+
+// On DISABLED run, everything should be made 'camelBack'.
+
+// RUN: cp -R %S/Inputs/readability-identifier-naming/. %theaders
+// RUN: %check_clang_tidy -check-suffixes=DISABLED,SHARED -std=c++11 %s \
+// RUN: readability-identifier-naming %t -- \
+// RUN:  -config='{ InheritParentConfig: true, CheckOptions: [ \
+// RUN:   {key: readability-identifier-naming.FunctionCase, value: camelBack}, \
+// RUN:   {key: readability-identifier-naming.GetConfigPerFile, value: false} \
+// RUN:  ]}' -header-filter='.*' -- -I%theaders
+
+#include "global-style-disabled/header.h"
+#include "global-style1/header.h"
+#include "global-style2/header.h"
+// CHECK-MESSAGES-ENABLED-DAG: global-style1/header.h:5:6: warning: invalid case style for global function 'styleFirstBad'
+// CHECK-MESSAGES-ENABLED-DAG: global-style2/header.h:5:6: warning: invalid case style for global function 'styleSecondBad'
+// CHECK-MESSAGES-DISABLED-DAG: global-style1/header.h:3:6: warning: invalid case style for function 'style_first_good'
+// CHECK-MESSAGES-DISABLED-DAG: global-style2/header.h:3:6: warning: invalid case style for function 'STYLE_SECOND_GOOD'
+// CHECK-MESSAGES-DISABLED-DAG: global-style-disabled/header.h:1:6: warning: invalid case style for function 'disabled_style_1'
+// CHECK-MESSAGES-DISABLED-DAG: global-style-disabled/header.h:3:6: warning: invalid case style for function 'DISABLED_STYLE_3'
+
+void goodStyle() {
+  style_first_good();
+  STYLE_SECOND_GOOD();
+  //      CHECK-FIXES-DISABLED: styleFirstGood();
+  // CHECK-FIXES-DISABLED-NEXT: styleSecondGood();
+}
+// CHECK-MESSAGES-SHARED-DAG: :[[@LINE+1]]:6: warning: invalid case style for function 'bad_style'
+void bad_style() {
+  styleFirstBad();
+  styleSecondBad();
+}
+//        CHECK-FIXES-SHARED: void badStyle() {
+// CHECK-FIXES-DISABLED-NEXT:   styleFirstBad();
+//  CHECK-FIXES-ENABLED-NEXT:   style_first_bad();
+// CHECK-FIXES-DISABLED-NEXT:   styleSecondBad();
+//  CHECK-FIXES-ENABLED-NEXT:   STYLE_SECOND_BAD();
+//   CHECK-FIXES-SHARED-NEXT: }
+
+void expectNoStyle() {
+  disabled_style_1();
+  disabledStyle2();
+  DISABLED_STYLE_3();
+  //      CHECK-FIXES-DISABLED: disabledStyle1();
+  // CHECK-FIXES-DISABLED-NEXT: disabledStyle2();
+  // CHECK-FIXES-DISABLED-NEXT: disabledStyle3();
+}
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/header.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/header.h
@@ -0,0 +1,5 @@
+
+
+void STYLE_SECOND_GOOD();
+
+void styleSecondBad();
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/.clang-tidy
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/.clang-tidy
@@ -0,0 +1,5 @@
+Checks: readability-identifier-naming
+CheckOptions:
+  - key:             readability-identifier-naming.GlobalFunctionCase
+    value:           UPPER_CASE
+
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/header.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/header.h
@@ -0,0 +1,5 @@
+
+
+void style_first_good();
+
+void styleFirstBad();
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/.clang-tidy
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/.clang-tidy
@@ -0,0 +1,5 @@
+Checks: readability-identifier-naming
+CheckOptions:
+  - key:             readability-identifier-naming.GlobalFunctionCase
+    value:           lower_case
+
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/header.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/header.h
@@ -0,0 +1,3 @@
+void disabled_style_1();
+void disabledStyle2();
+void DISABLED_STYLE_3();
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/.clang-tidy
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/.clang-tidy
@@ -0,0 +1,5 @@
+Checks: -readability-identifier-naming
+CheckOptions:
+  - key:             readability-identifier-naming.GlobalFunctionCase
+    value:           lower_case
+
Index: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
@@ -51,6 +51,7 @@
  - :option:`EnumCase`, :option:`EnumPrefix`, :option:`EnumSuffix`
  - :option:`EnumConstantCase`, :option:`EnumConstantPrefix`, :option:`EnumConstantSuffix`
  - :option:`FunctionCase`, :option:`FunctionPrefix`, :option:`FunctionSuffix`
+ - :option:`GetConfigPerFile`
  - :option:`GlobalConstantCase`, :option:`GlobalConstantPrefix`, :option:`GlobalConstantSuffix`
  - :option:`GlobalConstantPointerCase`, :option:`GlobalConstantPointerPrefix`, :option:`GlobalConstantPointerSuffix`
  - :option:`GlobalFunctionCase`, :option:`GlobalFunctionPrefix`, :option:`GlobalFunctionSuffix`
@@ -713,6 +714,13 @@
 
     char pre_my_function_string_post();
 
+.. option:: GetConfigPerFile
+
+    When `true` the check will look for the configuration for where an
+    identifier is declared. Useful for when included header files use a 
+    different style. 
+    Default value is `true`.
+
 .. option:: GlobalConstantCase
 
     When defined, the check will ensure global constant names conform to the
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,7 +67,14 @@
 Improvements to clang-tidy
 --------------------------
 
-The improvements are...
+Changes in existing checks
+^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+- Improved :doc:`readability-identifier-naming
+  <clang-tidy/checks/readability-identifier-naming>` check.
+
+  Added an option `GetConfigPerFile` to support including files which use
+  different naming styles.
 
 Improvements to include-fixer
 -----------------------------
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IDENTIFIERNAMINGCHECK_H
 
 #include "../utils/RenamerClangTidyCheck.h"
+#include "llvm/ADT/Optional.h"
 namespace clang {
 
 class MacroInfo;
@@ -69,7 +70,17 @@
   DiagInfo GetDiagInfo(const NamingCheckId &ID,
                        const NamingCheckFailure &Failure) const override;
 
-  std::vector<llvm::Optional<NamingStyle>> NamingStyles;
+  ArrayRef<llvm::Optional<NamingStyle>>
+  getStyleForFile(StringRef FileName) const;
+
+  /// Stores the style options as a vector, indexed by the specified \ref
+  /// StyleKind, for a given directory.
+  mutable llvm::StringMap<std::vector<llvm::Optional<NamingStyle>>>
+      NamingStylesCache;
+  ArrayRef<llvm::Optional<NamingStyle>> MainFileStyle;
+  ClangTidyContext *const Context;
+  const std::string CheckName;
+  const bool GetConfigPerFile;
   const bool IgnoreFailedSplit;
   const bool IgnoreMainLikeFunctions;
 };
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "IdentifierNamingCheck.h"
 
+#include "../GlobList.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
@@ -15,7 +16,8 @@
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Error.h"
-#include "llvm/Support/Format.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 
 #define DEBUG_TYPE "clang-tidy"
@@ -119,41 +121,47 @@
 #undef NAMING_KEYS
 // clang-format on
 
+static std::vector<llvm::Optional<IdentifierNamingCheck::NamingStyle>>
+getNamingStyles(const ClangTidyCheck::OptionsView &Options) {
+  std::vector<llvm::Optional<IdentifierNamingCheck::NamingStyle>> Styles;
+  Styles.reserve(StyleNames->size());
+  for (auto const &StyleName : StyleNames) {
+    auto CaseOptional = Options.getOptional<IdentifierNamingCheck::CaseType>(
+        (StyleName + "Case").str());
+    auto Prefix = Options.get((StyleName + "Prefix").str(), "");
+    auto Postfix = Options.get((StyleName + "Suffix").str(), "");
+
+    if (CaseOptional || !Prefix.empty() || !Postfix.empty())
+      Styles.emplace_back(IdentifierNamingCheck::NamingStyle{
+          std::move(CaseOptional), std::move(Prefix), std::move(Postfix)});
+    else
+      Styles.emplace_back(llvm::None);
+  }
+  return Styles;
+}
+
 IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
                                              ClangTidyContext *Context)
-    : RenamerClangTidyCheck(Name, Context),
+    : RenamerClangTidyCheck(Name, Context), Context(Context), CheckName(Name),
+      GetConfigPerFile(Options.get("GetConfigPerFile", true)),
       IgnoreFailedSplit(Options.get("IgnoreFailedSplit", false)),
       IgnoreMainLikeFunctions(Options.get("IgnoreMainLikeFunctions", false)) {
 
-  for (auto const &Name : StyleNames) {
-    auto CaseOptional = [&]() -> llvm::Optional<CaseType> {
-      auto ValueOr = Options.get<CaseType>((Name + "Case").str());
-      if (ValueOr)
-        return *ValueOr;
-      llvm::logAllUnhandledErrors(
-          llvm::handleErrors(ValueOr.takeError(),
-                             [](const MissingOptionError &) -> llvm::Error {
-                               return llvm::Error::success();
-                             }),
-          llvm::errs(), "warning: ");
-      return llvm::None;
-    }();
-
-    auto prefix = Options.get((Name + "Prefix").str(), "");
-    auto postfix = Options.get((Name + "Suffix").str(), "");
-
-    if (CaseOptional || !prefix.empty() || !postfix.empty()) {
-      NamingStyles.push_back(NamingStyle(CaseOptional, prefix, postfix));
-    } else {
-      NamingStyles.push_back(llvm::None);
-    }
-  }
+  auto IterAndInserted = NamingStylesCache.try_emplace(
+      llvm::sys::path::parent_path(Context->getCurrentFile()),
+      getNamingStyles(Options));
+  assert(IterAndInserted.second && "Couldn't insert Style");
+  // Holding a reference to the data in the vector is safe as it should never
+  // move.
+  MainFileStyle = IterAndInserted.first->getValue();
 }
 
 IdentifierNamingCheck::~IdentifierNamingCheck() = default;
 
 void IdentifierNamingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   RenamerClangTidyCheck::storeOptions(Opts);
+  ArrayRef<llvm::Optional<NamingStyle>> NamingStyles =
+      getStyleForFile(Context->getCurrentFile());
   for (size_t i = 0; i < SK_Count; ++i) {
     if (NamingStyles[i]) {
       if (NamingStyles[i]->Case) {
@@ -166,7 +174,7 @@
                     NamingStyles[i]->Suffix);
     }
   }
-
+  Options.store(Opts, "GetConfigPerFile", GetConfigPerFile);
   Options.store(Opts, "IgnoreFailedSplit", IgnoreFailedSplit);
   Options.store(Opts, "IgnoreMainLikeFunctions", IgnoreMainLikeFunctions);
 }
@@ -374,8 +382,7 @@
 
 static StyleKind findStyleKind(
     const NamedDecl *D,
-    const std::vector<llvm::Optional<IdentifierNamingCheck::NamingStyle>>
-        &NamingStyles,
+    ArrayRef<llvm::Optional<IdentifierNamingCheck::NamingStyle>> NamingStyles,
     bool IgnoreMainLikeFunctions) {
   assert(D && D->getIdentifier() && !D->getName().empty() && !D->isImplicit() &&
          "Decl must be an explicit identifier with a name.");
@@ -652,63 +659,56 @@
   return SK_Invalid;
 }
 
-llvm::Optional<RenamerClangTidyCheck::FailureInfo>
-IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl,
-                                          const SourceManager &SM) const {
-  StyleKind SK = findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions);
-  if (SK == SK_Invalid)
+static llvm::Optional<RenamerClangTidyCheck::FailureInfo> getFailureInfo(
+    StringRef Name, SourceLocation Location,
+    ArrayRef<llvm::Optional<IdentifierNamingCheck::NamingStyle>> NamingStyles,
+    StyleKind SK, const SourceManager &SM, bool IgnoreFailedSplit) {
+  if (SK == SK_Invalid || !NamingStyles[SK])
     return None;
 
-  if (!NamingStyles[SK])
-    return None;
-
-  const NamingStyle &Style = *NamingStyles[SK];
-  StringRef Name = Decl->getName();
+  const IdentifierNamingCheck::NamingStyle &Style = *NamingStyles[SK];
   if (matchesStyle(Name, Style))
     return None;
 
-  std::string KindName = fixupWithCase(StyleNames[SK], CT_LowerCase);
+  std::string KindName =
+      fixupWithCase(StyleNames[SK], IdentifierNamingCheck::CT_LowerCase);
   std::replace(KindName.begin(), KindName.end(), '_', ' ');
 
   std::string Fixup = fixupWithStyle(Name, Style);
   if (StringRef(Fixup).equals(Name)) {
     if (!IgnoreFailedSplit) {
-      LLVM_DEBUG(llvm::dbgs()
-                 << Decl->getBeginLoc().printToString(SM)
-                 << llvm::format(": unable to split words for %s '%s'\n",
-                                 KindName.c_str(), Name.str().c_str()));
+      LLVM_DEBUG(Location.print(llvm::dbgs(), SM);
+                 llvm::dbgs()
+                 << llvm::formatv(": unable to split words for {0} '{1}'\n",
+                                  KindName, Name));
     }
     return None;
   }
-  return FailureInfo{std::move(KindName), std::move(Fixup)};
+  return RenamerClangTidyCheck::FailureInfo{std::move(KindName),
+                                            std::move(Fixup)};
+}
+
+llvm::Optional<RenamerClangTidyCheck::FailureInfo>
+IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl,
+                                          const SourceManager &SM) const {
+  SourceLocation Loc = Decl->getLocation();
+  ArrayRef<llvm::Optional<NamingStyle>> NamingStyles =
+      getStyleForFile(SM.getFilename(Loc));
+
+  return getFailureInfo(
+      Decl->getName(), Loc, NamingStyles,
+      findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions), SM,
+      IgnoreFailedSplit);
 }
 
 llvm::Optional<RenamerClangTidyCheck::FailureInfo>
 IdentifierNamingCheck::GetMacroFailureInfo(const Token &MacroNameTok,
                                            const SourceManager &SM) const {
-  if (!NamingStyles[SK_MacroDefinition])
-    return None;
-
-  StringRef Name = MacroNameTok.getIdentifierInfo()->getName();
-  const NamingStyle &Style = *NamingStyles[SK_MacroDefinition];
-  if (matchesStyle(Name, Style))
-    return None;
+  SourceLocation Loc = MacroNameTok.getLocation();
 
-  std::string KindName =
-      fixupWithCase(StyleNames[SK_MacroDefinition], CT_LowerCase);
-  std::replace(KindName.begin(), KindName.end(), '_', ' ');
-
-  std::string Fixup = fixupWithStyle(Name, Style);
-  if (StringRef(Fixup).equals(Name)) {
-    if (!IgnoreFailedSplit) {
-      LLVM_DEBUG(llvm::dbgs()
-                 << MacroNameTok.getLocation().printToString(SM)
-                 << llvm::format(": unable to split words for %s '%s'\n",
-                                 KindName.c_str(), Name.str().c_str()));
-    }
-    return None;
-  }
-  return FailureInfo{std::move(KindName), std::move(Fixup)};
+  return getFailureInfo(MacroNameTok.getIdentifierInfo()->getName(), Loc,
+                        getStyleForFile(SM.getFilename(Loc)),
+                        SK_MacroDefinition, SM, IgnoreFailedSplit);
 }
 
 RenamerClangTidyCheck::DiagInfo
@@ -720,6 +720,21 @@
                   }};
 }
 
+ArrayRef<llvm::Optional<IdentifierNamingCheck::NamingStyle>>
+IdentifierNamingCheck::getStyleForFile(StringRef FileName) const {
+  if (!GetConfigPerFile)
+    return MainFileStyle;
+  auto &Styles = NamingStylesCache[llvm::sys::path::parent_path(FileName)];
+  if (Styles.empty()) {
+    ClangTidyOptions Options = Context->getOptionsForFile(FileName);
+    if (Options.Checks && GlobList(*Options.Checks).contains(CheckName))
+      Styles = getNamingStyles({CheckName, Options.CheckOptions});
+    else
+      Styles.resize(SK_Count, None);
+  }
+  return Styles;
+}
+
 } // namespace readability
 } // namespace tidy
 } // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to