njames93 created this revision.
njames93 added reviewers: aaron.ballman, gribozavr2, alexfh.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
njames93 requested review of this revision.
Herald added a subscriber: aheejin.

When checking for the style of a decl that isn't in the main file, the check 
will now search for the configuration that the included files uses to gather 
the style for its decls.

This can be useful to silence warnings in header files that follow a different 
naming convention without using header-filter to silence all warnings(even from 
other checks) in the header file.


Repository:
  rG LLVM Github Monorepo

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/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-case-violation.cpp
  
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,34 @@
+// Setup header directory
+
+// RUN: rm -rf %theaders
+// RUN: mkdir %theaders
+// RUN: cp -R %S/Inputs/readability-identifier-naming/. %theaders
+
+// C++11 isnt 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.
+
+// RUN: %check_clang_tidy -std=c++11 %s readability-identifier-naming %t -- \
+// RUN:   -config='{ InheritParentConfig: true, CheckOptions: [ \
+// RUN:     {key: readability-identifier-naming.FunctionCase, value: camelBack} \
+// RUN:  ]}' -header-filter='.*' -- -I%theaders
+
+#include "global-style1/header.h"
+#include "global-style2/header.h"
+// CHECK-MESSAGES-DAG: global-style1/header.h:5:6: warning: invalid case style for global function 'style1Bad'
+// CHECK-MESSAGES-DAG: global-style2/header.h:5:6: warning: invalid case style for global function 'style2Bad'
+
+void goodStyle() {
+  style1_good();
+  STYLE2_GOOD();
+}
+// CHECK-MESSAGES-DAG: :[[@LINE+1]]:6: warning: invalid case style for function 'bad_style'
+void bad_style() {
+  style1Bad();
+  style2Bad();
+}
+
+//      CHECK-FIXES: void badStyle() {
+// CHECK-FIXES-NEXT:   style1_bad();
+// CHECK-FIXES-NEXT:   STYLE2_BAD();
+// CHECK-FIXES-NEXT: }
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
@@ -5,11 +5,11 @@
 // RUN:   {key: readability-identifier-naming.ClassCase, value: UUPER_CASE}, \
 // RUN:   {key: readability-identifier-naming.StructCase, value: CAMEL}, \
 // RUN:   {key: readability-identifier-naming.EnumCase, value: AnY_cASe}, \
-// RUN:   ]}" 2>&1 | FileCheck %s --implicit-check-not warning
+// RUN:   ]}" 2>&1 | FileCheck %s --implicit-check-not error
 
-// CHECK-DAG: warning: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'?{{$}}
-// CHECK-DAG: warning: invalid configuration value 'UUPER_CASE' for option 'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'?{{$}}
+// CHECK-DAG: error: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'?{{$}}
+// CHECK-DAG: error: invalid configuration value 'UUPER_CASE' for option 'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'?{{$}}
 // Don't try to suggest an alternative for 'CAMEL'
-// CHECK-DAG: warning: invalid configuration value 'CAMEL' for option 'readability-identifier-naming.StructCase'{{$}}
+// CHECK-DAG: error: invalid configuration value 'CAMEL' for option 'readability-identifier-naming.StructCase'{{$}}
 // This fails on the EditDistance, but as it matches ignoring case suggest the correct value
-// CHECK-DAG: warning: invalid configuration value 'AnY_cASe' for option 'readability-identifier-naming.EnumCase'; did you mean 'aNy_CasE'?{{$}}
+// CHECK-DAG: error: invalid configuration value 'AnY_cASe' for option 'readability-identifier-naming.EnumCase'; did you mean 'aNy_CasE'?{{$}}
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 STYLE2_GOOD();
+
+void style2Bad();
\ No newline at end of file
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,4 @@
+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 style1_good();
+
+void style1Bad();
\ No newline at end of file
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,4 @@
+CheckOptions:
+  - key:             readability-identifier-naming.GlobalFunctionCase
+    value:           lower_case
+
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
@@ -59,6 +59,8 @@
     std::string Suffix;
   };
 
+  using OptionalStyle = llvm::Optional<NamingStyle>;
+
 private:
   llvm::Optional<FailureInfo>
   GetDeclFailureInfo(const NamedDecl *Decl,
@@ -69,7 +71,12 @@
   DiagInfo GetDiagInfo(const NamingCheckId &ID,
                        const NamingCheckFailure &Failure) const override;
 
-  std::vector<llvm::Optional<NamingStyle>> NamingStyles;
+  ArrayRef<OptionalStyle> getStyleForFile(StringRef FileName) const;
+
+  mutable llvm::StringMap<std::vector<OptionalStyle>> NamingStylesCache;
+
+  ClangTidyContext *const Context;
+  const std::string CheckName;
   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
@@ -15,7 +15,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 +120,41 @@
 #undef NAMING_KEYS
 // clang-format on
 
+static void
+populateNaimgStyle(std::vector<IdentifierNamingCheck::OptionalStyle> &Styles,
+                   const ClangTidyCheck::OptionsView &Options) {
+  assert(Styles.empty() && "Styles should be empty here");
+  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);
+  }
+}
+
 IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
                                              ClangTidyContext *Context)
-    : RenamerClangTidyCheck(Name, Context),
+    : RenamerClangTidyCheck(Name, Context), Context(Context), CheckName(Name),
       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);
-    }
-  }
+  populateNaimgStyle(NamingStylesCache[llvm::sys::path::parent_path(
+                         Context->getCurrentFile())],
+                     Options);
 }
 
 IdentifierNamingCheck::~IdentifierNamingCheck() = default;
 
 void IdentifierNamingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   RenamerClangTidyCheck::storeOptions(Opts);
+  ArrayRef<OptionalStyle> NamingStyles =
+      getStyleForFile(Context->getCurrentFile());
   for (size_t i = 0; i < SK_Count; ++i) {
     if (NamingStyles[i]) {
       if (NamingStyles[i]->Case) {
@@ -372,11 +373,10 @@
   return (Style.Prefix + Mid + Style.Suffix).str();
 }
 
-static StyleKind findStyleKind(
-    const NamedDecl *D,
-    const std::vector<llvm::Optional<IdentifierNamingCheck::NamingStyle>>
-        &NamingStyles,
-    bool IgnoreMainLikeFunctions) {
+static StyleKind
+findStyleKind(const NamedDecl *D,
+              ArrayRef<IdentifierNamingCheck::OptionalStyle> NamingStyles,
+              bool IgnoreMainLikeFunctions) {
   assert(D && D->getIdentifier() && !D->getName().empty() && !D->isImplicit() &&
          "Decl must be an explicit identifier with a name.");
 
@@ -652,63 +652,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)
-    return None;
-
-  if (!NamingStyles[SK])
+static llvm::Optional<RenamerClangTidyCheck::FailureInfo>
+getFailureInfo(StringRef Name, SourceLocation Location,
+               ArrayRef<IdentifierNamingCheck::OptionalStyle> NamingStyles,
+               StyleKind SK, const SourceManager &SM, bool IgnoreFailedSplit) {
+  if (SK == SK_Invalid || !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::GetMacroFailureInfo(const Token &MacroNameTok,
-                                           const SourceManager &SM) const {
-  if (!NamingStyles[SK_MacroDefinition])
-    return None;
+IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl,
+                                          const SourceManager &SM) const {
+  SourceLocation Loc = Decl->getLocation();
+  ArrayRef<OptionalStyle> NamingStyles = getStyleForFile(SM.getFilename(Loc));
 
-  StringRef Name = MacroNameTok.getIdentifierInfo()->getName();
-  const NamingStyle &Style = *NamingStyles[SK_MacroDefinition];
-  if (matchesStyle(Name, Style))
-    return None;
+  return getFailureInfo(
+      Decl->getName(), Loc, NamingStyles,
+      findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions), SM,
+      IgnoreFailedSplit);
+}
 
-  std::string KindName =
-      fixupWithCase(StyleNames[SK_MacroDefinition], CT_LowerCase);
-  std::replace(KindName.begin(), KindName.end(), '_', ' ');
+llvm::Optional<RenamerClangTidyCheck::FailureInfo>
+IdentifierNamingCheck::GetMacroFailureInfo(const Token &MacroNameTok,
+                                           const SourceManager &SM) const {
+  SourceLocation Loc = MacroNameTok.getLocation();
+  ArrayRef<OptionalStyle> NamingStyles = getStyleForFile(SM.getFilename(Loc));
 
-  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,
+                        NamingStyles, SK_MacroDefinition, SM,
+                        IgnoreFailedSplit);
 }
 
 RenamerClangTidyCheck::DiagInfo
@@ -720,6 +713,15 @@
                   }};
 }
 
+ArrayRef<IdentifierNamingCheck::OptionalStyle>
+IdentifierNamingCheck::getStyleForFile(StringRef FileName) const {
+  auto &Styles = NamingStylesCache[llvm::sys::path::parent_path(FileName)];
+  if (Styles.empty())
+    populateNaimgStyle(
+        Styles, {CheckName, Context->getOptionsForFile(FileName).CheckOptions});
+  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