carlosgalvezp created this revision.
Herald added a subscriber: xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
carlosgalvezp requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Re-introduce it, now with actual functionality in the checks
to use the global option if available. The first attempt at
solving this did not use the global option at all.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142655

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.h
  clang-tools-extra/clang-tidy/FileExtensionsSet.h
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -76,13 +76,20 @@
 
 TEST(ParseConfiguration, ValidConfiguration) {
   llvm::ErrorOr<ClangTidyOptions> Options =
-      parseConfiguration(llvm::MemoryBufferRef("Checks: \"-*,misc-*\"\n"
-                                               "HeaderFilterRegex: \".*\"\n"
-                                               "AnalyzeTemporaryDtors: true\n"
-                                               "User: some.user",
-                                               "Options"));
+      parseConfiguration(llvm::MemoryBufferRef(
+          "Checks: \"-*,misc-*\"\n"
+          "HeaderFileExtensions: [\"\",\"h\",\"hh\",\"hpp\",\"hxx\"]\n"
+          "ImplementationFileExtensions: [\"c\",\"cc\",\"cpp\",\"cxx\"]\n"
+          "HeaderFilterRegex: \".*\"\n"
+          "AnalyzeTemporaryDtors: true\n"
+          "User: some.user",
+          "Options"));
   EXPECT_TRUE(!!Options);
   EXPECT_EQ("-*,misc-*", *Options->Checks);
+  EXPECT_EQ(std::vector<std::string>({"", "h", "hh", "hpp", "hxx"}),
+            *Options->HeaderFileExtensions);
+  EXPECT_EQ(std::vector<std::string>({"c", "cc", "cpp", "cxx"}),
+            *Options->ImplementationFileExtensions);
   EXPECT_EQ(".*", *Options->HeaderFilterRegex);
   EXPECT_EQ("some.user", *Options->User);
 }
@@ -105,6 +112,8 @@
   llvm::ErrorOr<ClangTidyOptions> Options1 =
       parseConfiguration(llvm::MemoryBufferRef(R"(
       Checks: "check1,check2"
+      HeaderFileExtensions: ["h","hh"]
+      ImplementationFileExtensions: ["c","cc"]
       HeaderFilterRegex: "filter1"
       AnalyzeTemporaryDtors: true
       User: user1
@@ -117,6 +126,8 @@
   llvm::ErrorOr<ClangTidyOptions> Options2 =
       parseConfiguration(llvm::MemoryBufferRef(R"(
       Checks: "check3,check4"
+      HeaderFileExtensions: ["hpp","hxx"]
+      ImplementationFileExtensions: ["cpp","cxx"]
       HeaderFilterRegex: "filter2"
       AnalyzeTemporaryDtors: false
       User: user2
@@ -128,6 +139,10 @@
   ASSERT_TRUE(!!Options2);
   ClangTidyOptions Options = Options1->merge(*Options2, 0);
   EXPECT_EQ("check1,check2,check3,check4", *Options.Checks);
+  EXPECT_EQ(std::vector<std::string>({"hpp", "hxx"}),
+            *Options.HeaderFileExtensions);
+  EXPECT_EQ(std::vector<std::string>({"cpp", "cxx"}),
+            *Options.ImplementationFileExtensions);
   EXPECT_EQ("filter2", *Options.HeaderFilterRegex);
   EXPECT_EQ("user2", *Options.User);
   ASSERT_TRUE(Options.ExtraArgs.has_value());
Index: clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
@@ -3,6 +3,8 @@
 
 // RUN: not clang-tidy -verify-config \
 // RUN: --checks='-*,bad*glob,llvm*,llvm-includeorder,my-made-up-check' --config='{Checks: "readability-else-after-ret", \
+// RUN: HeaderFileExtensions: ["h", "hh", "hpp"], \
+// RUN: ImplementationFileExtensions: ["c", "cc", "hpp"], \
 // RUN: CheckOptions: [{key: "IgnoreMacros", value: "true"}, \
 // RUN:                {key: "StriceMode", value: "true"}, \
 // RUN:                {key: modernize-lop-convert.UseCxx20ReverseRanges, value: true} \
@@ -12,6 +14,7 @@
 // CHECK-VERIFY-DAG: command-line option '-config': warning: unknown check 'readability-else-after-ret'; did you mean 'readability-else-after-return' [-verify-config]
 // CHECK-VERIFY-DAG: command-line option '-config': warning: unknown check option 'modernize-lop-convert.UseCxx20ReverseRanges'; did you mean 'modernize-loop-convert.UseCxx20ReverseRanges' [-verify-config]
 // CHECK-VERIFY-DAG: command-line option '-config': warning: unknown check option 'StriceMode'; did you mean 'StrictMode' [-verify-config]
+// CHECK-VERIFY-DAG: command-line option '-config': warning: HeaderFileExtension 'hpp' is the same as ImplementationFileExtension 'hpp' [-verify-config]
 // CHECK-VERIFY: command-line option '-checks': warning: check glob 'bad*glob' doesn't match any known check [-verify-config]
 // CHECK-VERIFY: command-line option '-checks': warning: unknown check 'llvm-includeorder'; did you mean 'llvm-include-order' [-verify-config]
 // CHECK-VERIFY: command-line option '-checks': warning: unknown check 'my-made-up-check' [-verify-config]
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===================================================================
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_FILE_EXTENSIONS_UTILS_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_FILE_EXTENSIONS_UTILS_H
 
+#include "../FileExtensionsSet.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/SmallSet.h"
@@ -17,8 +18,6 @@
 
 namespace clang::tidy::utils {
 
-typedef llvm::SmallSet<llvm::StringRef, 5> FileExtensionsSet;
-
 /// Checks whether expansion location of \p Loc is in header file.
 bool isExpansionLocInHeaderFile(SourceLocation Loc, const SourceManager &SM,
                                 const FileExtensionsSet &HeaderFileExtensions);
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -46,12 +46,14 @@
 
     $ clang-tidy -dump-config
     ---
-    Checks:              '-*,some-check'
-    WarningsAsErrors:    ''
-    HeaderFilterRegex:   ''
-    FormatStyle:         none
-    InheritParentConfig: true
-    User:                user
+    Checks:                       '-*,some-check'
+    WarningsAsErrors:             ''
+    HeaderFileExtensions:         ['', 'h','hh','hpp','hxx']
+    ImplementationFileExtensions: ['c','cc','cpp','cxx']
+    HeaderFilterRegex:            ''
+    FormatStyle:                  none
+    InheritParentConfig:          true
+    User:                         user
     CheckOptions:
       some-check.SomeOption: 'some value'
     ...
@@ -130,10 +132,10 @@
                                cl::init(false), cl::cat(ClangTidyCategory));
 
 static cl::opt<bool> FixNotes("fix-notes", cl::desc(R"(
-If a warning has no fix, but a single fix can 
-be found through an associated diagnostic note, 
-apply the fix. 
-Specifying this flag will implicitly enable the 
+If a warning has no fix, but a single fix can
+be found through an associated diagnostic note,
+apply the fix.
+Specifying this flag will implicitly enable the
 '--fix' flag.
 )"),
                               cl::init(false), cl::cat(ClangTidyCategory));
@@ -458,6 +460,26 @@
   return AnyInvalid;
 }
 
+static bool verifyFileExtensions(
+    const std::vector<std::string> &HeaderFileExtensions,
+    const std::vector<std::string> &ImplementationFileExtensions,
+    StringRef Source) {
+  bool AnyInvalid = false;
+  for (const auto &HeaderExtension : HeaderFileExtensions) {
+    for (const auto &ImplementationExtension : ImplementationFileExtensions) {
+      if (HeaderExtension == ImplementationExtension) {
+        AnyInvalid = true;
+        auto &Output = llvm::WithColor::warning(llvm::errs(), Source)
+                       << "HeaderFileExtension '" << HeaderExtension << '\''
+                       << " is the same as ImplementationFileExtension '"
+                       << ImplementationExtension << '\'';
+        Output << VerifyConfigWarningEnd;
+      }
+    }
+  }
+  return AnyInvalid;
+}
+
 int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
 
@@ -561,6 +583,11 @@
       if (Opts.Checks)
         AnyInvalid |= verifyChecks(Valid.Names, *Opts.Checks, Source);
 
+      if (Opts.HeaderFileExtensions && Opts.ImplementationFileExtensions)
+        AnyInvalid |=
+            verifyFileExtensions(*Opts.HeaderFileExtensions,
+                                 *Opts.ImplementationFileExtensions, Source);
+
       for (auto Key : Opts.CheckOptions.keys()) {
         if (Valid.Options.contains(Key))
           continue;
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "SuspiciousIncludeCheck.h"
+#include "../utils/FileExtensionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Lex/Preprocessor.h"
 #include <optional>
Index: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
@@ -10,7 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_DYNAMIC_STATIC_INITIALIZERS_CHECK_H
 
 #include "../ClangTidyCheck.h"
-#include "../utils/FileExtensionsUtils.h"
+#include "../FileExtensionsSet.h"
 
 namespace clang::tidy::bugprone {
 
@@ -34,7 +34,7 @@
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
-  const StringRef RawStringHeaderFileExtensions;
+  StringRef RawStringHeaderFileExtensions;
   utils::FileExtensionsSet HeaderFileExtensions;
 };
 
Index: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "DynamicStaticInitializersCheck.h"
+#include "../utils/FileExtensionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
@@ -24,17 +25,22 @@
   return false;
 }
 
-DynamicStaticInitializersCheck::DynamicStaticInitializersCheck(StringRef Name,
-                                                               ClangTidyContext *Context)
-    : ClangTidyCheck(Name, Context),
-      RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
-        "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
-  if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
-                                  HeaderFileExtensions,
-                                  utils::defaultFileExtensionDelimiters())) {
-    this->configurationDiag("Invalid header file extension: '%0'")
-        << RawStringHeaderFileExtensions;
-  }
+DynamicStaticInitializersCheck::DynamicStaticInitializersCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context) {
+  std::optional<StringRef> HeaderFileExtensionsOption =
+      Options.get("HeaderFileExtensions");
+  RawStringHeaderFileExtensions =
+      HeaderFileExtensionsOption.value_or(utils::defaultHeaderFileExtensions());
+  if (HeaderFileExtensionsOption) {
+    if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
+                                    HeaderFileExtensions,
+                                    utils::defaultFileExtensionDelimiters())) {
+      this->configurationDiag("Invalid header file extension: '%0'")
+          << RawStringHeaderFileExtensions;
+    }
+  } else
+    HeaderFileExtensions = Context->getHeaderFileExtensions();
 }
 
 void DynamicStaticInitializersCheck::storeOptions(
Index: clang-tools-extra/clang-tidy/FileExtensionsSet.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/FileExtensionsSet.h
@@ -0,0 +1,19 @@
+//===--- FileExtensionsSet.h - clang-tidy -----------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FILE_EXTENSIONS_SET_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FILE_EXTENSIONS_SET_H
+
+#include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang::tidy::utils {
+typedef llvm::SmallSet<llvm::StringRef, 5> FileExtensionsSet;
+} // namespace clang::tidy::utils
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FILE_EXTENSIONS_SET_H
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.h
@@ -71,6 +71,14 @@
   /// WarningsAsErrors filter.
   std::optional<std::string> WarningsAsErrors;
 
+  /// File extensions to consider to determine if a given diagnostic is located
+  /// in a header file.
+  std::optional<std::vector<std::string>> HeaderFileExtensions;
+
+  /// File extensions to consider to determine if a given diagnostic is located
+  /// is located in an implementation file.
+  std::optional<std::vector<std::string>> ImplementationFileExtensions;
+
   /// Output warnings from headers matching this filter. Warnings from
   /// main files will always be displayed.
   std::optional<std::string> HeaderFilterRegex;
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -122,6 +122,9 @@
     bool Ignored = false;
     IO.mapOptional("Checks", Options.Checks);
     IO.mapOptional("WarningsAsErrors", Options.WarningsAsErrors);
+    IO.mapOptional("HeaderFileExtensions", Options.HeaderFileExtensions);
+    IO.mapOptional("ImplementationFileExtensions",
+                   Options.ImplementationFileExtensions);
     IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex);
     IO.mapOptional("AnalyzeTemporaryDtors", Ignored); // deprecated
     IO.mapOptional("FormatStyle", Options.FormatStyle);
@@ -142,6 +145,8 @@
   ClangTidyOptions Options;
   Options.Checks = "";
   Options.WarningsAsErrors = "";
+  Options.HeaderFileExtensions = {"", "h", "hh", "hpp", "hxx"};
+  Options.ImplementationFileExtensions = {"c", "cc", "cpp", "cxx"};
   Options.HeaderFilterRegex = "";
   Options.SystemHeaders = false;
   Options.FormatStyle = "none";
@@ -178,6 +183,9 @@
                                               unsigned Order) {
   mergeCommaSeparatedLists(Checks, Other.Checks);
   mergeCommaSeparatedLists(WarningsAsErrors, Other.WarningsAsErrors);
+  overrideValue(HeaderFileExtensions, Other.HeaderFileExtensions);
+  overrideValue(ImplementationFileExtensions,
+                Other.ImplementationFileExtensions);
   overrideValue(HeaderFilterRegex, Other.HeaderFilterRegex);
   overrideValue(SystemHeaders, Other.SystemHeaders);
   overrideValue(FormatStyle, Other.FormatStyle);
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -11,6 +11,7 @@
 
 #include "ClangTidyOptions.h"
 #include "ClangTidyProfiling.h"
+#include "FileExtensionsSet.h"
 #include "NoLintDirectiveHandler.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Tooling/Core/Diagnostic.h"
@@ -160,6 +161,14 @@
   /// \c CurrentFile.
   ClangTidyOptions getOptionsForFile(StringRef File) const;
 
+  const utils::FileExtensionsSet &getHeaderFileExtensions() const {
+    return HeaderFileExtensions;
+  }
+
+  const utils::FileExtensionsSet &getImplementationFileExtensions() const {
+    return ImplementationFileExtensions;
+  }
+
   /// Returns \c ClangTidyStats containing issued and ignored diagnostic
   /// counters.
   const ClangTidyStats &getStats() const { return Stats; }
@@ -221,6 +230,9 @@
   std::unique_ptr<CachedGlobList> CheckFilter;
   std::unique_ptr<CachedGlobList> WarningAsErrorFilter;
 
+  utils::FileExtensionsSet HeaderFileExtensions;
+  utils::FileExtensionsSet ImplementationFileExtensions;
+
   LangOptions LangOpts;
 
   ClangTidyStats Stats;
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDiagnostic.h"
 #include "clang/AST/Attr.h"
+#include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
@@ -222,12 +223,35 @@
   DiagEngine->setSourceManager(SourceMgr);
 }
 
+static bool
+parseFileExtensions(const std::vector<std::string> &AllFileExtensions,
+                    utils::FileExtensionsSet &FileExtensions) {
+  FileExtensions.clear();
+  for (StringRef Suffix : AllFileExtensions) {
+    StringRef Extension = Suffix.trim();
+    if (!llvm::all_of(Extension, isAlphanumeric))
+      return false;
+    FileExtensions.insert(Extension);
+  }
+  return true;
+}
+
 void ClangTidyContext::setCurrentFile(StringRef File) {
   CurrentFile = std::string(File);
   CurrentOptions = getOptionsForFile(CurrentFile);
   CheckFilter = std::make_unique<CachedGlobList>(*getOptions().Checks);
   WarningAsErrorFilter =
       std::make_unique<CachedGlobList>(*getOptions().WarningsAsErrors);
+
+  if (!parseFileExtensions(*getOptions().HeaderFileExtensions,
+                           HeaderFileExtensions)) {
+    this->configurationDiag("Invalid header file extensions");
+  }
+
+  if (!parseFileExtensions(*getOptions().ImplementationFileExtensions,
+                           ImplementationFileExtensions)) {
+    this->configurationDiag("Invalid implementation file extensions");
+  }
 }
 
 void ClangTidyContext::setASTContext(ASTContext *Context) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to