From c4e67e29edeb60c195726812a8317707b45ab258 Mon Sep 17 00:00:00 2001
From: Owen Voorhees <owen@squibner.com>
Date: Sun, 19 Apr 2020 07:59:25 -0700
Subject: [PATCH] [Diagnostics] Add -diagnostic-style=(llvm|swift) to control
 printed output

This default formatting style remains the same "LLVM style". "Swift style"
is what was previously enabled via -enable-experimental-diagnostic-formatting
---
 include/swift/Basic/DiagnosticOptions.h        |  4 +++-
 .../Frontend/PrintingDiagnosticConsumer.h      | 10 +++++++---
 include/swift/Option/FrontendOptions.td        |  4 ----
 include/swift/Option/Options.td                |  7 +++++++
 lib/Driver/ToolChains.cpp                      |  1 +
 lib/Frontend/CompilerInvocation.cpp            | 18 ++++++++++++++++--
 lib/Frontend/PrintingDiagnosticConsumer.cpp    |  8 ++++++--
 lib/FrontendTool/FrontendTool.cpp              |  6 ++----
 test/Driver/color-diagnostics.swift            |  2 +-
 test/diagnostics/educational-notes.swift       |  4 ++--
 .../pretty-printed-diagnostics.swift           |  2 +-
 .../pretty-printed-diags-in-clang-buffer.swift |  2 +-
 ...ty-printed-source-loc-directive-diags.swift |  2 +-
 13 files changed, 48 insertions(+), 22 deletions(-)

diff --git a/include/swift/Basic/DiagnosticOptions.h b/include/swift/Basic/DiagnosticOptions.h
index 58531d5908641..31e6149412692 100644
--- a/include/swift/Basic/DiagnosticOptions.h
+++ b/include/swift/Basic/DiagnosticOptions.h
@@ -32,6 +32,8 @@ class DiagnosticOptions {
     VerifyAndApplyFixes
   } VerifyMode = NoVerify;
 
+  enum FormattingStyle { LLVM, Swift };
+
   /// Indicates whether to allow diagnostics for \c <unknown> locations if
   /// \c VerifyMode is not \c NoVerify.
   bool VerifyIgnoreUnknown = false;
@@ -61,7 +63,7 @@ class DiagnosticOptions {
 
   // If set to true, use the more descriptive experimental formatting style for
   // diagnostics.
-  bool EnableExperimentalFormatting = false;
+  FormattingStyle PrintedFormattingStyle = FormattingStyle::LLVM;
 
   std::string DiagnosticDocumentationPath = "";
 
diff --git a/include/swift/Frontend/PrintingDiagnosticConsumer.h b/include/swift/Frontend/PrintingDiagnosticConsumer.h
index a4720ce5c6f51..cbbca307f6297 100644
--- a/include/swift/Frontend/PrintingDiagnosticConsumer.h
+++ b/include/swift/Frontend/PrintingDiagnosticConsumer.h
@@ -18,8 +18,9 @@
 #ifndef SWIFT_PRINTINGDIAGNOSTICCONSUMER_H
 #define SWIFT_PRINTINGDIAGNOSTICCONSUMER_H
 
-#include "swift/Basic/LLVM.h"
 #include "swift/AST/DiagnosticConsumer.h"
+#include "swift/Basic/DiagnosticOptions.h"
+#include "swift/Basic/LLVM.h"
 
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/Process.h"
@@ -33,7 +34,8 @@ class PrintingDiagnosticConsumer : public DiagnosticConsumer {
   bool ForceColors = false;
   bool PrintEducationalNotes = false;
   bool DidErrorOccur = false;
-  bool ExperimentalFormattingEnabled = false;
+  DiagnosticOptions::FormattingStyle FormattingStyle =
+      DiagnosticOptions::FormattingStyle::LLVM;
   // The current snippet used to display an error/warning/remark and the notes
   // implicitly associated with it. Uses `std::unique_ptr` so that
   // `AnnotatedSourceSnippet` can be forward declared.
@@ -65,7 +67,9 @@ class PrintingDiagnosticConsumer : public DiagnosticConsumer {
     PrintEducationalNotes = ShouldPrint;
   }
 
-  void enableExperimentalFormatting() { ExperimentalFormattingEnabled = true; }
+  void setFormattingStyle(DiagnosticOptions::FormattingStyle style) {
+    FormattingStyle = style;
+  }
 
   bool didErrorOccur() {
     return DidErrorOccur;
diff --git a/include/swift/Option/FrontendOptions.td b/include/swift/Option/FrontendOptions.td
index c8168fde7d545..5d640b7588c11 100644
--- a/include/swift/Option/FrontendOptions.td
+++ b/include/swift/Option/FrontendOptions.td
@@ -131,10 +131,6 @@ def enable_cross_import_overlays : Flag<["-"], "enable-cross-import-overlays">,
 def disable_cross_import_overlays : Flag<["-"], "disable-cross-import-overlays">,
   HelpText<"Do not automatically import declared cross-import overlays.">;
   
-def enable_experimental_diagnostic_formatting :
-  Flag<["-"], "enable-experimental-diagnostic-formatting">,
-  HelpText<"Enable experimental diagnostic formatting features.">;
-  
 def diagnostic_documentation_path
   : Separate<["-"], "diagnostic-documentation-path">, MetaVarName<"<path>">,
   HelpText<"Path to diagnostic documentation resources">;
diff --git a/include/swift/Option/Options.td b/include/swift/Option/Options.td
index a430124cc7f27..b771b48b3895d 100644
--- a/include/swift/Option/Options.td
+++ b/include/swift/Option/Options.td
@@ -370,6 +370,13 @@ def debug_diagnostic_names : Flag<["-"], "debug-diagnostic-names">,
 def print_educational_notes : Flag<["-"], "print-educational-notes">,
   Flags<[FrontendOption, DoesNotAffectIncrementalBuild]>,
   HelpText<"Include educational notes in printed diagnostic output, if available">;
+def diagnostic_style : Separate<["-"], "diagnostic-style">,
+  Flags<[FrontendOption, DoesNotAffectIncrementalBuild]>,
+  MetaVarName<"<style>">,
+  HelpText<"The formatting style used when printing diagnostics ('swift' or 'llvm')">;
+def diagnostic_style_EQ : Joined<["-"], "diagnostic-style=">,
+  Flags<[FrontendOption, DoesNotAffectIncrementalBuild]>,
+  MetaVarName<"<style>">, Alias<diagnostic_style>;
 
 def module_cache_path : Separate<["-"], "module-cache-path">,
   Flags<[FrontendOption, DoesNotAffectIncrementalBuild, ArgumentIsPath]>,
diff --git a/lib/Driver/ToolChains.cpp b/lib/Driver/ToolChains.cpp
index 89d2efb1771d4..ab29e4de0b47c 100644
--- a/lib/Driver/ToolChains.cpp
+++ b/lib/Driver/ToolChains.cpp
@@ -243,6 +243,7 @@ void ToolChain::addCommonFrontendArgs(const OutputInfo &OI,
   inputArgs.AddLastArg(arguments, options::OPT_serialize_diagnostics_path);
   inputArgs.AddLastArg(arguments, options::OPT_debug_diagnostic_names);
   inputArgs.AddLastArg(arguments, options::OPT_print_educational_notes);
+  inputArgs.AddLastArg(arguments, options::OPT_diagnostic_style);
   inputArgs.AddLastArg(arguments, options::OPT_enable_astscope_lookup);
   inputArgs.AddLastArg(arguments, options::OPT_disable_astscope_lookup);
   inputArgs.AddLastArg(arguments, options::OPT_disable_parser_lookup);
diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
index b1e9916376c5c..d07baa9c2d101 100644
--- a/lib/Frontend/CompilerInvocation.cpp
+++ b/lib/Frontend/CompilerInvocation.cpp
@@ -884,10 +884,26 @@ static bool ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
   Opts.SkipDiagnosticPasses |= Args.hasArg(OPT_disable_diagnostic_passes);
   Opts.ShowDiagnosticsAfterFatalError |=
     Args.hasArg(OPT_show_diagnostics_after_fatal);
+
   Opts.UseColor |=
       Args.hasFlag(OPT_color_diagnostics,
                    OPT_no_color_diagnostics,
                    /*Default=*/llvm::sys::Process::StandardErrHasColors());
+  // If no style options are specified, default to LLVM style.
+  Opts.PrintedFormattingStyle = DiagnosticOptions::FormattingStyle::LLVM;
+  if (const Arg *arg = Args.getLastArg(OPT_diagnostic_style)) {
+    StringRef contents = arg->getValue();
+    if (contents == "llvm") {
+      Opts.PrintedFormattingStyle = DiagnosticOptions::FormattingStyle::LLVM;
+    } else if (contents == "swift") {
+      Opts.PrintedFormattingStyle = DiagnosticOptions::FormattingStyle::Swift;
+    } else {
+      Diags.diagnose(SourceLoc(), diag::error_unsupported_option_argument,
+                     arg->getOption().getPrefixedName(), arg->getValue());
+      return true;
+    }
+  }
+
   Opts.FixitCodeForAllDiagnostics |= Args.hasArg(OPT_fixit_all);
   Opts.SuppressWarnings |= Args.hasArg(OPT_suppress_warnings);
   Opts.WarningsAsErrors = Args.hasFlag(options::OPT_warnings_as_errors,
@@ -895,8 +911,6 @@ static bool ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
                                        false);
   Opts.PrintDiagnosticNames |= Args.hasArg(OPT_debug_diagnostic_names);
   Opts.PrintEducationalNotes |= Args.hasArg(OPT_print_educational_notes);
-  Opts.EnableExperimentalFormatting |=
-      Args.hasArg(OPT_enable_experimental_diagnostic_formatting);
   if (Arg *A = Args.getLastArg(OPT_diagnostic_documentation_path)) {
     Opts.DiagnosticDocumentationPath = A->getValue();
   }
diff --git a/lib/Frontend/PrintingDiagnosticConsumer.cpp b/lib/Frontend/PrintingDiagnosticConsumer.cpp
index ab4937d448b8b..9e8cd37c03e05 100644
--- a/lib/Frontend/PrintingDiagnosticConsumer.cpp
+++ b/lib/Frontend/PrintingDiagnosticConsumer.cpp
@@ -932,7 +932,8 @@ void PrintingDiagnosticConsumer::handleDiagnostic(SourceManager &SM,
   if (Info.IsChildNote)
     return;
 
-  if (ExperimentalFormattingEnabled) {
+  switch (FormattingStyle) {
+  case DiagnosticOptions::FormattingStyle::Swift:
     if (Info.Kind == DiagnosticKind::Note && currentSnippet) {
       // If this is a note and we have an in-flight message, add it to that
       // instead of emitting it separately.
@@ -950,7 +951,9 @@ void PrintingDiagnosticConsumer::handleDiagnostic(SourceManager &SM,
           BufferedEducationalNotes.push_back(buffer->get()->getBuffer().str());
       }
     }
-  } else {
+    break;
+
+  case DiagnosticOptions::FormattingStyle::LLVM:
     printDiagnostic(SM, Info);
 
     if (PrintEducationalNotes) {
@@ -965,6 +968,7 @@ void PrintingDiagnosticConsumer::handleDiagnostic(SourceManager &SM,
     for (auto ChildInfo : Info.ChildDiagnosticInfo) {
       printDiagnostic(SM, *ChildInfo);
     }
+    break;
   }
 }
 
diff --git a/lib/FrontendTool/FrontendTool.cpp b/lib/FrontendTool/FrontendTool.cpp
index 704aa40bf3a5c..dfb1b9ab38d1c 100644
--- a/lib/FrontendTool/FrontendTool.cpp
+++ b/lib/FrontendTool/FrontendTool.cpp
@@ -2111,10 +2111,8 @@ int swift::performFrontend(ArrayRef<const char *> Args,
   PDC.setPrintEducationalNotes(
       Invocation.getDiagnosticOptions().PrintEducationalNotes);
 
-  // Temporarily stage the new diagnostic formatting style behind
-  // -enable-descriptive-diagnostics
-  if (Invocation.getDiagnosticOptions().EnableExperimentalFormatting)
-    PDC.enableExperimentalFormatting();
+  PDC.setFormattingStyle(
+      Invocation.getDiagnosticOptions().PrintedFormattingStyle);
 
   if (Invocation.getFrontendOptions().DebugTimeCompilation)
     SharedTimer::enableCompilationTimers();
diff --git a/test/Driver/color-diagnostics.swift b/test/Driver/color-diagnostics.swift
index fd51b4513e83e..1a17f0d05e3b7 100644
--- a/test/Driver/color-diagnostics.swift
+++ b/test/Driver/color-diagnostics.swift
@@ -1,4 +1,4 @@
-// RUN: not %target-swiftc_driver -color-diagnostics -emit-executable -o %t %s 2>&1 \
+// RUN: not %target-swiftc_driver -color-diagnostics -diagnostic-style=llvm -emit-executable -o %t %s 2>&1 \
 // RUN:     | %FileCheck -check-prefix=CHECK-CD %s
 // CHECK-CD: [0m1 = 2{{$}}
 
diff --git a/test/diagnostics/educational-notes.swift b/test/diagnostics/educational-notes.swift
index 2d5ecfaeeeba2..76af493b70b81 100644
--- a/test/diagnostics/educational-notes.swift
+++ b/test/diagnostics/educational-notes.swift
@@ -1,6 +1,6 @@
-// RUN: not %target-swift-frontend -color-diagnostics -print-educational-notes -diagnostic-documentation-path %S/test-docs/ -typecheck %s 2>&1 | %FileCheck %s --match-full-lines --strict-whitespace
+// RUN: not %target-swift-frontend -color-diagnostics -diagnostic-style=llvm -print-educational-notes -diagnostic-documentation-path %S/test-docs/ -typecheck %s 2>&1 | %FileCheck %s --match-full-lines --strict-whitespace
 // RUN: not %target-swift-frontend -no-color-diagnostics -print-educational-notes -diagnostic-documentation-path %S/test-docs/ -typecheck %s 2>&1 | %FileCheck %s --match-full-lines --strict-whitespace --check-prefix=NO-COLOR
-// RUN: not %target-swift-frontend -enable-experimental-diagnostic-formatting -print-educational-notes -diagnostic-documentation-path %S/test-docs/ -typecheck %s 2>&1 | %FileCheck %s --check-prefix=CHECK-DESCRIPTIVE
+// RUN: not %target-swift-frontend -diagnostic-style=swift -print-educational-notes -diagnostic-documentation-path %S/test-docs/ -typecheck %s 2>&1 | %FileCheck %s --check-prefix=CHECK-DESCRIPTIVE
 
 // A diagnostic with no educational notes
 let x = 1 +
diff --git a/test/diagnostics/pretty-printed-diagnostics.swift b/test/diagnostics/pretty-printed-diagnostics.swift
index a6c4f9d9e7501..56373bd5ee36e 100644
--- a/test/diagnostics/pretty-printed-diagnostics.swift
+++ b/test/diagnostics/pretty-printed-diagnostics.swift
@@ -1,4 +1,4 @@
-// RUN: not %target-swift-frontend -enable-experimental-diagnostic-formatting -typecheck %s 2>&1 | %FileCheck %s
+// RUN: not %target-swift-frontend -diagnostic-style=swift -typecheck %s 2>&1 | %FileCheck %s
 
 1 + 2
 
diff --git a/test/diagnostics/pretty-printed-diags-in-clang-buffer.swift b/test/diagnostics/pretty-printed-diags-in-clang-buffer.swift
index 049ca95278278..036141a59def7 100644
--- a/test/diagnostics/pretty-printed-diags-in-clang-buffer.swift
+++ b/test/diagnostics/pretty-printed-diags-in-clang-buffer.swift
@@ -1,5 +1,5 @@
 // RUN: %empty-directory(%t.mcp)
-// RUN: not %target-swift-frontend(mock-sdk: %clang-importer-sdk) -enable-experimental-diagnostic-formatting -I %S/Inputs/ -typecheck %s -module-cache-path %t.mcp 2>&1 | %FileCheck %s
+// RUN: not %target-swift-frontend(mock-sdk: %clang-importer-sdk) -diagnostic-style=swift -I %S/Inputs/ -typecheck %s -module-cache-path %t.mcp 2>&1 | %FileCheck %s
 
 // REQUIRES: objc_interop
 
diff --git a/test/diagnostics/pretty-printed-source-loc-directive-diags.swift b/test/diagnostics/pretty-printed-source-loc-directive-diags.swift
index cebc5a22884bb..e26b5c77524e3 100644
--- a/test/diagnostics/pretty-printed-source-loc-directive-diags.swift
+++ b/test/diagnostics/pretty-printed-source-loc-directive-diags.swift
@@ -1,4 +1,4 @@
-// RUN: not %target-swift-frontend -enable-experimental-diagnostic-formatting -typecheck %s 2>&1 | %FileCheck %s
+// RUN: not %target-swift-frontend -diagnostic-style=swift -typecheck %s 2>&1 | %FileCheck %s
 
 // Error split between the real file and a virtual one.
 #sourceLocation(file: "abc.swift", line: 9)