[clang-tidy] Support comments in WarningsAsErrors

[zeyi2]

Closes #171792

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: mitchell (zeyi2)

Changes

Closes #171792

---

Full diff: https://github.com/llvm/llvm-project/pull/171816.diff

2 Files Affected:

• (modified) clang-tools-extra/clang-tidy/ClangTidyOptions.cpp (+14-12)
• (modified) clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp (+31)

diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
index 550f7809d75f9..46a30d25ec441 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -178,12 +178,13 @@ template <> struct MappingTraits<ClangTidyOptions::CustomCheckValue> {
   }
 };
 
-struct ChecksVariant {
+struct GlobListVariant {
   std::optional<std::string> AsString;
   std::optional<std::vector<std::string>> AsVector;
 };
 
-template <> void yamlize(IO &IO, ChecksVariant &Val, bool, EmptyContext &Ctx) {
+template <>
+void yamlize(IO &IO, GlobListVariant &Val, bool, EmptyContext &Ctx) {
   if (!IO.outputting()) {
     // Special case for reading from YAML
     // Must support reading from both a string or a list
@@ -200,25 +201,26 @@ template <> void yamlize(IO &IO, ChecksVariant &Val, bool, EmptyContext &Ctx) {
   }
 }
 
-static void mapChecks(IO &IO, std::optional<std::string> &Checks) {
+static void mapGlobList(IO &IO, std::optional<std::string> &GlobList,
+                        const char *Key) {
   if (IO.outputting()) {
     // Output always a string
-    IO.mapOptional("Checks", Checks);
+    IO.mapOptional(Key, GlobList);
   } else {
     // Input as either a string or a list
-    ChecksVariant ChecksAsVariant;
-    IO.mapOptional("Checks", ChecksAsVariant);
-    if (ChecksAsVariant.AsString)
-      Checks = ChecksAsVariant.AsString;
-    else if (ChecksAsVariant.AsVector)
-      Checks = llvm::join(*ChecksAsVariant.AsVector, ",");
+    GlobListVariant GlobListAsVariant;
+    IO.mapOptional(Key, GlobListAsVariant);
+    if (GlobListAsVariant.AsString)
+      GlobList = GlobListAsVariant.AsString;
+    else if (GlobListAsVariant.AsVector)
+      GlobList = llvm::join(*GlobListAsVariant.AsVector, ",");
   }
 }
 
 template <> struct MappingTraits<ClangTidyOptions> {
   static void mapping(IO &IO, ClangTidyOptions &Options) {
-    mapChecks(IO, Options.Checks);
-    IO.mapOptional("WarningsAsErrors", Options.WarningsAsErrors);
+    mapGlobList(IO, Options.Checks, "Checks");
+    mapGlobList(IO, Options.WarningsAsErrors, "WarningsAsErrors");
     IO.mapOptional("HeaderFileExtensions", Options.HeaderFileExtensions);
     IO.mapOptional("ImplementationFileExtensions",
                    Options.ImplementationFileExtensions);
diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
index 410cebf90913d..aee3313f2263b 100644
--- a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -107,6 +107,37 @@ TEST(ParseConfiguration, ChecksSeparatedByNewlines) {
   EXPECT_EQ("-*,misc-*\nllvm-*\n-clang-*,\ngoogle-*\n", *Options->Checks);
 }
 
+TEST(ParseConfiguration, WarningsAsErrorsSeparatedByNewlines) {
+  const auto MemoryBuffer = llvm::MemoryBufferRef("WarningsAsErrors: |\n"
+                                                  "  -*,misc-*\n"
+                                                  "  llvm-*\n"
+                                                  "  -clang-*,\n"
+                                                  "  google-*",
+                                                  "Options");
+
+  const auto Options = parseConfiguration(MemoryBuffer);
+
+  EXPECT_TRUE(!!Options);
+  EXPECT_EQ("-*,misc-*\nllvm-*\n-clang-*,\ngoogle-*\n",
+            *Options->WarningsAsErrors);
+}
+
+TEST(ParseConfiguration, WarningsAsErrorsAsList) {
+  const auto MemoryBuffer = llvm::MemoryBufferRef("WarningsAsErrors: [\n"
+                                                  "  '-*',\n"
+                                                  "  'misc-*',\n"
+                                                  "  'llvm-*',\n"
+                                                  "  '-clang-*',\n"
+                                                  "  'google-*'\n"
+                                                  "]",
+                                                  "Options");
+
+  const auto Options = parseConfiguration(MemoryBuffer);
+
+  EXPECT_TRUE(!!Options);
+  EXPECT_EQ("-*,misc-*,llvm-*,-clang-*,google-*", *Options->WarningsAsErrors);
+}
+
 TEST(ParseConfiguration, MergeConfigurations) {
   llvm::ErrorOr<ClangTidyOptions> Options1 =
       parseConfiguration(llvm::MemoryBufferRef(R"(

[localspook]

Just a small comment:

[zeyi2]

Just a small comment:

Thanks :)

[zwuis]

Is release note needed?

[vbvictor]

Is release note needed?

Yes endeed! Should go into general improvements of clang-tidy