Merge pull request #13181 from github/redsun82/swift-diagnostics-enable-warnings

Swift: turn internal error into a TSP warning

Author: redsun82

swift/logging/SwiftDiagnostics.cpp

@@ -8,6 +8,22 @@
 
 namespace codeql {
 
+namespace {
+std::string_view severityToString(SwiftDiagnostic::Severity severity) {
+  using S = SwiftDiagnostic::Severity;
+  switch (severity) {
+    case S::note:
+      return "note";
+    case S::warning:
+      return "warning";
+    case S::error:
+      return "error";
+    default:
+      return "unknown";
+  }
+}
+}  // namespace
+
 nlohmann::json SwiftDiagnostic::json(const std::chrono::system_clock::time_point& timestamp,
                                      std::string_view message) const {
   nlohmann::json ret{
@@ -23,7 +39,7 @@ nlohmann::json SwiftDiagnostic::json(const std::chrono::system_clock::time_point
            {"cliSummaryTable", has(Visibility::cliSummaryTable)},
            {"telemetry", has(Visibility::telemetry)},
        }},
-      {"severity", "error"},
+      {"severity", severityToString(severity)},
       {"helpLinks", std::vector<std::string_view>(absl::StrSplit(helpLinks, ' '))},
       {format == Format::markdown ? "markdownMessage" : "plaintextMessage",
        absl::StrCat(message, ".\n\n", action)},

swift/logging/SwiftDiagnostics.h

@@ -46,34 +46,48 @@ struct SwiftDiagnostic {
     all = 0b111,
   };
 
+  // Notice that Tool Status Page severity is not necessarily the same as log severity, as the
+  // scope is different: TSP's scope is the whole analysis, log's scope is a single run
+  enum class Severity {
+    note,
+    warning,
+    error,
+  };
+
+  // wrapper for passing optional help links to constructor
+  struct HelpLinks {
+    std::string_view value;
+  };
+
+  static constexpr std::string_view extractorName = "swift";
+
   std::string_view id;
   std::string_view name;
-  static constexpr std::string_view extractorName = "swift";
-  Format format;
   std::string_view action;
-  // space separated if more than 1. Not a vector to allow constexpr
-  // TODO(C++20) with vector going constexpr this can be turned to `std::vector<std::string_view>`
-  std::string_view helpLinks;
-  // for the moment, we only output errors, so no need to store the severity
 
+  Format format{Format::markdown};
   Visibility visibility{Visibility::all};
+  Severity severity{Severity::error};
+
+  // space separated if more than 1. Not a vector to allow constexpr
+  // TODO(C++20) with vector going constexpr this can be turned to `std::vector<std::string_view>`
+  std::string_view helpLinks{""};
 
   std::optional<SwiftDiagnosticsLocation> location{};
 
   // notice help links are really required only for plaintext messages, otherwise they should be
   // directly embedded in the markdown message
+  // optional arguments can be any of HelpLinks, Severity, Visibility or Format to set the
+  // corresponding field
+  // TODO(C++20) this constructor won't really be necessary anymore with designated initializers
+  template <typename... OptionalArgs>
   constexpr SwiftDiagnostic(std::string_view id,
                             std::string_view name,
-                            Format format,
-                            std::string_view action = "",
-                            std::string_view helpLinks = "",
-                            Visibility visibility = Visibility::all)
-      : id{id},
-        name{name},
-        format{format},
-        action{action},
-        helpLinks{helpLinks},
-        visibility{visibility} {}
+                            std::string_view action,
+                            OptionalArgs... optionalArgs)
+      : id{id}, name{name}, action{action} {
+    (setOptionalArg(optionalArgs), ...);
+  }
 
   // create a JSON diagnostics for this source with the given `timestamp` and `message`
   // Depending on format, either a plaintextMessage or markdownMessage is used that includes both
@@ -97,6 +111,15 @@ struct SwiftDiagnostic {
 
  private:
   bool has(Visibility v) const;
+
+  constexpr void setOptionalArg(HelpLinks h) { helpLinks = h.value; }
+  constexpr void setOptionalArg(Format f) { format = f; }
+  constexpr void setOptionalArg(Visibility v) { visibility = v; }
+  constexpr void setOptionalArg(Severity s) { severity = s; }
+
+  // intentionally left undefined
+  template <typename T>
+  constexpr void setOptionalArg(T);
 };
 
 inline constexpr SwiftDiagnostic::Visibility operator|(SwiftDiagnostic::Visibility lhs,
@@ -114,9 +137,12 @@ inline constexpr SwiftDiagnostic::Visibility operator&(SwiftDiagnostic::Visibili
 constexpr SwiftDiagnostic internalError{
     "internal-error",
     "Internal error",
-    SwiftDiagnostic::Format::plaintext,
-    /* action=*/"",
-    /* helpLinks=*/"",
-    SwiftDiagnostic::Visibility::telemetry,
+    "Some or all of the Swift analysis may have failed.\n"
+    "\n"
+    "If the error persists, contact support, quoting the error message and describing what "
+    "happened, or [open an issue in our open source repository][1].\n"
+    "\n"
+    "[1]: https://github.com/github/codeql/issues/new?labels=bug&template=ql---general.md",
+    SwiftDiagnostic::Severity::warning,
 };
 }  // namespace codeql

swift/tools/autobuilder-diagnostics/IncompatibleOs.cpp

@@ -9,8 +9,8 @@
 const std::string_view codeql::programName = "autobuilder";
 
 constexpr codeql::SwiftDiagnostic incompatibleOs{
-    "incompatible-os", "Incompatible operating system (expected macOS)",
-    codeql::SwiftDiagnostic::Format::markdown,
+    "incompatible-os",
+    "Incompatible operating system (expected macOS)",
     "[Change the Actions runner][1] to run on macOS.\n"
     "\n"
     "You may be able to run analysis on Linux by setting up a [manual build command][2].\n"
@@ -22,7 +22,8 @@ constexpr codeql::SwiftDiagnostic incompatibleOs{
     "https://docs.github.com/en/enterprise-server/code-security/code-scanning/"
     "automatically-scanning-your-code-for-vulnerabilities-and-errors/"
     "configuring-the-codeql-workflow-for-compiled-languages#adding-build-steps-for-a-compiled-"
-    "language"};
+    "language",
+};
 
 static codeql::Logger& logger() {
   static codeql::Logger ret{"main"};

swift/xcode-autobuilder/CustomizingBuildDiagnostics.h

@@ -1,12 +1,13 @@
 #include <string_view>
+#include "swift/logging/SwiftDiagnostics.h"
 
 namespace codeql {
 constexpr std::string_view customizingBuildAction = "Set up a manual build command.";
-constexpr std::string_view customizingBuildHelpLinks =
+constexpr SwiftDiagnostic::HelpLinks customizingBuildHelpLinks{
     "https://docs.github.com/en/enterprise-server/code-security/code-scanning/"
     "automatically-scanning-your-code-for-vulnerabilities-and-errors/customizing-code-scanning "
     "https://docs.github.com/en/enterprise-server/code-security/code-scanning/"
     "automatically-scanning-your-code-for-vulnerabilities-and-errors/"
     "configuring-the-codeql-workflow-for-compiled-languages#adding-build-steps-for-a-compiled-"
-    "language";
+    "language"};
 }  // namespace codeql

swift/xcode-autobuilder/XcodeBuildRunner.cpp

@@ -9,9 +9,8 @@
 #include "swift/xcode-autobuilder/CustomizingBuildDiagnostics.h"
 
 constexpr codeql::SwiftDiagnostic buildCommandFailed{
-    "build-command-failed", "Detected build command failed",
-    codeql::SwiftDiagnostic::Format::plaintext, codeql::customizingBuildAction,
-    codeql::customizingBuildHelpLinks};
+    "build-command-failed", "Detected build command failed", codeql::customizingBuildAction,
+    codeql::SwiftDiagnostic::Format::plaintext, codeql::customizingBuildHelpLinks};
 
 static codeql::Logger& logger() {
   static codeql::Logger ret{"build"};

swift/xcode-autobuilder/xcode-autobuilder.cpp

@@ -13,18 +13,16 @@ static const char* unitTest = "com.apple.product-type.bundle.unit-test";
 const std::string_view codeql::programName = "autobuilder";
 
 constexpr codeql::SwiftDiagnostic noProjectFound{
-    "no-project-found", "No Xcode project or workspace detected",
-    codeql::SwiftDiagnostic::Format::plaintext, codeql::customizingBuildAction,
-    codeql::customizingBuildHelpLinks};
+    "no-project-found", "No Xcode project or workspace detected", codeql::customizingBuildAction,
+    codeql::SwiftDiagnostic::Format::plaintext, codeql::customizingBuildHelpLinks};
 
 constexpr codeql::SwiftDiagnostic noSwiftTarget{
-    "no-swift-target", "No Swift compilation target found",
-    codeql::SwiftDiagnostic::Format::plaintext, codeql::customizingBuildAction,
-    codeql::customizingBuildHelpLinks};
+    "no-swift-target", "No Swift compilation target found", codeql::customizingBuildAction,
+    codeql::SwiftDiagnostic::Format::plaintext, codeql::customizingBuildHelpLinks};
 
 constexpr codeql::SwiftDiagnostic spmNotSupported{
     "spm-not-supported", "Swift Package Manager build unsupported by autobuild",
-    codeql::SwiftDiagnostic::Format::plaintext, codeql::customizingBuildAction,
+    codeql::customizingBuildAction, codeql::SwiftDiagnostic::Format::plaintext,
     codeql::customizingBuildHelpLinks};
 
 static codeql::Logger& logger() {