Merge pull request #13224 from github/sashabu/tsp-empty-help-links

sashabu · web-flow · commit f943502e413d · 2023-05-19T15:44:44.000+01:00
Swift: Drop support for plaintext diagnostics (and `helpLinks`).
diff --git a/swift/integration-tests/linux-only/autobuilder/unsupported-os/diagnostics.expected b/swift/integration-tests/linux-only/autobuilder/unsupported-os/diagnostics.expected
@@ -1,7 +1,4 @@
 {
-  "helpLinks": [
-    ""
-  ],
   "markdownMessage": "Currently, `autobuild` for Swift analysis is only supported on macOS.\n\n[Change the Actions runner][1] to run on macOS.\n\nYou may be able to run analysis on Linux by setting up a [manual build command][2].\n\n[1]: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idruns-on\n[2]: 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",
   "severity": "error",
   "source": {
diff --git a/swift/integration-tests/osx-only/autobuilder/failure/diagnostics.expected b/swift/integration-tests/osx-only/autobuilder/failure/diagnostics.expected
@@ -1,7 +1,4 @@
 {
-  "helpLinks": [
-    ""
-  ],
   "markdownMessage": "`autobuild` failed to run the detected build command:\n\n```\n/usr/bin/xcodebuild build -project <test-root-directory>/hello-failure.xcodeproj -target hello-failure CODE_SIGNING_REQUIRED=NO CODE_SIGNING_ALLOWED=NO\n```\n\nSet up a [manual build command][1] or [check the logs of the autobuild step][2].\n\n[1]: 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\n[2]: https://docs.github.com/en/actions/monitoring-and-troubleshooting-workflows/using-workflow-run-logs",
   "severity": "error",
   "source": {
diff --git a/swift/integration-tests/osx-only/autobuilder/no-build-system/diagnostics.expected b/swift/integration-tests/osx-only/autobuilder/no-build-system/diagnostics.expected
@@ -1,7 +1,4 @@
 {
-  "helpLinks": [
-    ""
-  ],
   "markdownMessage": "`autobuild` could not detect an Xcode project or workspace.\n\nSet up a [manual build command][1].\n\n[1]: 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",
   "severity": "error",
   "source": {
diff --git a/swift/integration-tests/osx-only/autobuilder/no-swift-with-spm/diagnostics.expected b/swift/integration-tests/osx-only/autobuilder/no-swift-with-spm/diagnostics.expected
@@ -1,7 +1,4 @@
 {
-  "helpLinks": [
-    ""
-  ],
   "markdownMessage": "A Swift package was detected, but no viable Xcode target was found.\n\nSwift Package Manager builds are not currently supported by `autobuild`. Set up a [manual build command][1].\n\n[1]: 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",
   "severity": "error",
   "source": {
diff --git a/swift/integration-tests/osx-only/autobuilder/no-swift/diagnostics.expected b/swift/integration-tests/osx-only/autobuilder/no-swift/diagnostics.expected
@@ -1,7 +1,4 @@
 {
-  "helpLinks": [
-    ""
-  ],
   "markdownMessage": "All targets found within Xcode projects or workspaces either contain no Swift source files, or are tests.\n\nTo analyze a custom set of source files, set up a [manual build command][1].\n\n[1]: 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",
   "severity": "error",
   "source": {
diff --git a/swift/integration-tests/osx-only/autobuilder/no-xcode-with-spm/diagnostics.expected b/swift/integration-tests/osx-only/autobuilder/no-xcode-with-spm/diagnostics.expected
@@ -1,7 +1,4 @@
 {
-  "helpLinks": [
-    ""
-  ],
   "markdownMessage": "A Swift package was detected, but no viable Xcode target was found.\n\nSwift Package Manager builds are not currently supported by `autobuild`. Set up a [manual build command][1].\n\n[1]: 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",
   "severity": "error",
   "source": {
diff --git a/swift/integration-tests/osx-only/autobuilder/only-tests-with-spm/diagnostics.expected b/swift/integration-tests/osx-only/autobuilder/only-tests-with-spm/diagnostics.expected
@@ -1,7 +1,4 @@
 {
-  "helpLinks": [
-    ""
-  ],
   "markdownMessage": "A Swift package was detected, but no viable Xcode target was found.\n\nSwift Package Manager builds are not currently supported by `autobuild`. Set up a [manual build command][1].\n\n[1]: 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",
   "severity": "error",
   "source": {
diff --git a/swift/integration-tests/osx-only/autobuilder/only-tests/diagnostics.expected b/swift/integration-tests/osx-only/autobuilder/only-tests/diagnostics.expected
@@ -1,7 +1,4 @@
 {
-  "helpLinks": [
-    ""
-  ],
   "markdownMessage": "All targets found within Xcode projects or workspaces either contain no Swift source files, or are tests.\n\nTo analyze a custom set of source files, set up a [manual build command][1].\n\n[1]: 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",
   "severity": "error",
   "source": {
diff --git a/swift/logging/SwiftDiagnostics.cpp b/swift/logging/SwiftDiagnostics.cpp
@@ -40,9 +40,7 @@ nlohmann::json SwiftDiagnostic::json(const std::chrono::system_clock::time_point
            {"telemetry", has(Visibility::telemetry)},
        }},
       {"severity", severityToString(severity)},
-      {"helpLinks", std::vector<std::string_view>(absl::StrSplit(helpLinks, ' '))},
-      {format == Format::markdown ? "markdownMessage" : "plaintextMessage",
-       absl::StrCat(message, "\n\n", action)},
+      {"markdownMessage", absl::StrCat(message, "\n\n", action)},
       {"timestamp", fmt::format("{:%FT%T%z}", timestamp)},
   };
   if (location) {
diff --git a/swift/logging/SwiftDiagnostics.h b/swift/logging/SwiftDiagnostics.h
@@ -35,11 +35,6 @@ struct SwiftDiagnosticsLocation {
 // These are internally stored into a map on id's. A specific error log can use binlog's category
 // as id, which will then be used to recover the diagnostic source while dumping.
 struct SwiftDiagnostic {
-  enum class Format {
-    plaintext,
-    markdown,
-  };
-
   enum class Visibility : unsigned char {
     none = 0b000,
     statusPage = 0b001,
@@ -56,31 +51,18 @@ struct SwiftDiagnostic {
     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;
   std::string_view action;
 
-  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
+  // optional arguments can be either Severity or Visibility 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,
@@ -91,10 +73,9 @@ struct SwiftDiagnostic {
     (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
-  // the message and the action to take. The id is used to construct the source id in the form
-  // `swift/<prog name>/<id>`
+  // create a JSON diagnostics for this source with the given `timestamp` and Markdown `message`
+  // A markdownMessage is emitted that includes both the message and the action to take. The id is
+  // used to construct the source id in the form `swift/<prog name>/<id>`
   nlohmann::json json(const std::chrono::system_clock::time_point& timestamp,
                       std::string_view message) const;
 
@@ -114,8 +95,6 @@ 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; }
 
diff --git a/swift/logging/tests/assertion-diagnostics/diagnostics.expected b/swift/logging/tests/assertion-diagnostics/diagnostics.expected
@@ -1,7 +1,4 @@
 {
-  "helpLinks": [
-   ""
-  ],
   "markdownMessage": "CodeQL encountered an unexpected internal error with the following message:\n> Assertion failed: `false`. Format the int 1234 and string myString if this assertion fails.\n\nSome or all of the Swift analysis may have failed.\n\nIf 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",
   "severity": "warning",
   "source": {

Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,4 @@`
`1`	`1`	`{`
`2`		`- "helpLinks": [`
`3`		`- ""`
`4`		`- ],`
`5`	`2`	"markdownMessage": "Currently, `autobuild` for Swift analysis is only supported on macOS.\n\n[Change the Actions runner][1] to run on macOS.\n\nYou may be able to run analysis on Linux by setting up a [manual build command][2].\n\n[1]: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idruns-on\n[2]: 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",
`6`	`3`	`"severity": "error",`
`7`	`4`	`"source": {`