Automerge: [analyzer] Prevent triplicate warnings for sarif-html (#158112)

When `-analyzer-output=sarif-html` is specified, the analyzer was
reporting each warning to the console three times. This is because the
code to create the diagnostic consumers for `sarif-html` was calling the
code for `sarif` and `html` separately, each of which also creates its
own console text consumer. Then the `sarif-html` code itself created a
third.

The fix is to factor out the creation of the SARIF and HTML consumers
from the text consumers, so `sarif-html` just calls the code to create
the SARIF and HTML consumers without the text consumers.

The same fix applies for `plist-html`.

I've updated one of the SARIF tests to specify `sarif-html`. This test
would fail in the regular `-verify` validation due to the triplicated
warnings, but now passes with my fix.

Fixes #158103
rdar://160383710

Author: dbartol

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

@@ -10,6 +10,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "PlistDiagnostics.h"
+#include "SarifDiagnostics.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/Stmt.h"
@@ -169,6 +171,21 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const ArrowMap &Indices) {
 
 } // namespace
 
+/// Creates and registers an HTML diagnostic consumer, without any additional
+/// text consumer.
+static void createHTMLDiagnosticConsumerImpl(
+    PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
+    const std::string &OutputDir, const Preprocessor &PP,
+    bool SupportMultipleFiles) {
+
+  // TODO: Emit an error here.
+  if (OutputDir.empty())
+    return;
+
+  C.emplace_back(std::make_unique<HTMLDiagnostics>(
+      std::move(DiagOpts), OutputDir, PP, SupportMultipleFiles));
+}
+
 void ento::createHTMLDiagnosticConsumer(
     PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
     const std::string &OutputDir, const Preprocessor &PP,
@@ -183,12 +200,8 @@ void ento::createHTMLDiagnosticConsumer(
   createTextMinimalPathDiagnosticConsumer(DiagOpts, C, OutputDir, PP, CTU,
                                           MacroExpansions);
 
-  // TODO: Emit an error here.
-  if (OutputDir.empty())
-    return;
-
-  C.emplace_back(std::make_unique<HTMLDiagnostics>(std::move(DiagOpts),
-                                                   OutputDir, PP, true));
+  createHTMLDiagnosticConsumerImpl(DiagOpts, C, OutputDir, PP,
+                                   /*SupportMultipleFiles=*/true);
 }
 
 void ento::createHTMLSingleFileDiagnosticConsumer(
@@ -199,24 +212,19 @@ void ento::createHTMLSingleFileDiagnosticConsumer(
   createTextMinimalPathDiagnosticConsumer(DiagOpts, C, OutputDir, PP, CTU,
                                           MacroExpansions);
 
-  // TODO: Emit an error here.
-  if (OutputDir.empty())
-    return;
-
-  C.emplace_back(std::make_unique<HTMLDiagnostics>(std::move(DiagOpts),
-                                                   OutputDir, PP, false));
+  createHTMLDiagnosticConsumerImpl(DiagOpts, C, OutputDir, PP,
+                                   /*SupportMultipleFiles=*/false);
 }
 
 void ento::createPlistHTMLDiagnosticConsumer(
     PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
     const std::string &prefix, const Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU,
     const MacroExpansionContext &MacroExpansions) {
-  createHTMLDiagnosticConsumer(
-      DiagOpts, C, std::string(llvm::sys::path::parent_path(prefix)), PP, CTU,
-      MacroExpansions);
-  createPlistMultiFileDiagnosticConsumer(DiagOpts, C, prefix, PP, CTU,
-                                         MacroExpansions);
+  createHTMLDiagnosticConsumerImpl(
+      DiagOpts, C, std::string(llvm::sys::path::parent_path(prefix)), PP, true);
+  createPlistDiagnosticConsumerImpl(DiagOpts, C, prefix, PP, CTU,
+                                    MacroExpansions, true);
   createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, prefix, PP,
                                           CTU, MacroExpansions);
 }
@@ -226,11 +234,11 @@ void ento::createSarifHTMLDiagnosticConsumer(
     const std::string &sarif_file, const Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU,
     const MacroExpansionContext &MacroExpansions) {
-  createHTMLDiagnosticConsumer(
+  createHTMLDiagnosticConsumerImpl(
       DiagOpts, C, std::string(llvm::sys::path::parent_path(sarif_file)), PP,
-      CTU, MacroExpansions);
-  createSarifDiagnosticConsumer(DiagOpts, C, sarif_file, PP, CTU,
-                                MacroExpansions);
+      true);
+  createSarifDiagnosticConsumerImpl(DiagOpts, C, sarif_file, PP);
+
   createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, sarif_file,
                                           PP, CTU, MacroExpansions);
 }

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "PlistDiagnostics.h"
 #include "clang/Analysis/IssueHash.h"
 #include "clang/Analysis/MacroExpansionContext.h"
 #include "clang/Analysis/PathDiagnostic.h"
@@ -528,19 +529,31 @@ PlistDiagnostics::PlistDiagnostics(
   (void)this->CTU;
 }
 
-void ento::createPlistDiagnosticConsumer(
+/// Creates and registers a Plist diagnostic consumer, without any additional
+/// text consumer.
+void ento::createPlistDiagnosticConsumerImpl(
     PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
     const std::string &OutputFile, const Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU,
-    const MacroExpansionContext &MacroExpansions) {
+    const MacroExpansionContext &MacroExpansions, bool SupportsMultipleFiles) {
 
   // TODO: Emit an error here.
   if (OutputFile.empty())
     return;
 
   C.push_back(std::make_unique<PlistDiagnostics>(
-      DiagOpts, OutputFile, PP, CTU, MacroExpansions,
-      /*supportsMultipleFiles=*/false));
+      DiagOpts, OutputFile, PP, CTU, MacroExpansions, SupportsMultipleFiles));
+}
+
+void ento::createPlistDiagnosticConsumer(
+    PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
+    const std::string &OutputFile, const Preprocessor &PP,
+    const cross_tu::CrossTranslationUnitContext &CTU,
+    const MacroExpansionContext &MacroExpansions) {
+
+  createPlistDiagnosticConsumerImpl(DiagOpts, C, OutputFile, PP, CTU,
+                                    MacroExpansions,
+                                    /*SupportsMultipleFiles=*/false);
   createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, OutputFile,
                                           PP, CTU, MacroExpansions);
 }
@@ -551,13 +564,10 @@ void ento::createPlistMultiFileDiagnosticConsumer(
     const cross_tu::CrossTranslationUnitContext &CTU,
     const MacroExpansionContext &MacroExpansions) {
 
-  // TODO: Emit an error here.
-  if (OutputFile.empty())
-    return;
+  createPlistDiagnosticConsumerImpl(DiagOpts, C, OutputFile, PP, CTU,
+                                    MacroExpansions,
+                                    /*SupportsMultipleFiles=*/true);
 
-  C.push_back(std::make_unique<PlistDiagnostics>(
-      DiagOpts, OutputFile, PP, CTU, MacroExpansions,
-      /*supportsMultipleFiles=*/true));
   createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, OutputFile,
                                           PP, CTU, MacroExpansions);
 }

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.h

@@ -0,0 +1,27 @@
+//==- PlistDiagnostics.h - Plist Diagnostics for Paths -------------*- 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_LIB_STATICANALYZER_CORE_PLISTDIAGNOSTICS_H
+#define LLVM_CLANG_LIB_STATICANALYZER_CORE_PLISTDIAGNOSTICS_H
+
+#include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
+#include <string>
+
+namespace clang::ento {
+
+void createPlistDiagnosticConsumerImpl(
+    PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
+    const std::string &Output, const Preprocessor &PP,
+    const cross_tu::CrossTranslationUnitContext &CTU,
+    const MacroExpansionContext &MacroExpansions, bool SupportsMultipleFiles);
+
+} // namespace clang::ento
+
+#endif

clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp

@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "SarifDiagnostics.h"
 #include "clang/Analysis/MacroExpansionContext.h"
 #include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/Sarif.h"
@@ -54,14 +55,24 @@ void ento::createSarifDiagnosticConsumer(
     const cross_tu::CrossTranslationUnitContext &CTU,
     const MacroExpansionContext &MacroExpansions) {
 
+  createSarifDiagnosticConsumerImpl(DiagOpts, C, Output, PP);
+
+  createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, Output, PP,
+                                          CTU, MacroExpansions);
+}
+
+/// Creates and registers a SARIF diagnostic consumer, without any additional
+/// text consumer.
+void ento::createSarifDiagnosticConsumerImpl(
+    PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
+    const std::string &Output, const Preprocessor &PP) {
+
   // TODO: Emit an error here.
   if (Output.empty())
     return;
 
   C.push_back(std::make_unique<SarifDiagnostics>(Output, PP.getLangOpts(),
                                                  PP.getSourceManager()));
-  createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, Output, PP,
-                                          CTU, MacroExpansions);
 }
 
 static StringRef getRuleDescription(StringRef CheckName) {

clang/lib/StaticAnalyzer/Core/SarifDiagnostics.h

@@ -0,0 +1,25 @@
+//==- SarifDiagnostics.h - SARIF Diagnostics for Paths -------------*- 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_LIB_STATICANALYZER_CORE_SARIFDIAGNOSTICS_H
+#define LLVM_CLANG_LIB_STATICANALYZER_CORE_SARIFDIAGNOSTICS_H
+
+#include "clang/Lex/Preprocessor.h"
+#include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
+#include <string>
+
+namespace clang::ento {
+
+void createSarifDiagnosticConsumerImpl(PathDiagnosticConsumerOptions DiagOpts,
+                                       PathDiagnosticConsumers &C,
+                                       const std::string &Output,
+                                       const Preprocessor &PP);
+
+} // namespace clang::ento
+
+#endif