Replace danger-swift with a actions/github-script version (#6080)

* Replace danger-swift with a actions/github-script version
* Report PR check results in a comment

Annotations are only visible in the Checks tab, so the results were easy to
miss on the pull request itself. The workflow now keeps a single comment up to
date on the Conversation tab, listing failures and warnings together, and
deletes it again once there is nothing left to report.

Commenting on pull requests from forks needs a token with write access, which
the pull_request trigger does not provide, hence the switch to
pull_request_target. The job never checks out or executes any code from the
pull request, it only reads the event payload and the list of changed files.

.github/workflows/danger.yml

@@ -1,32 +0,0 @@
-name: Danger CI
-
-on:
-  pull_request:
-    types: [labeled, synchronize, opened, reopened]
-
-permissions: {}
-
-jobs:
-  build:
-    name: Danger
-    runs-on: ubuntu-latest
-    timeout-minutes: 15
-    permissions:
-      contents: read
-      pull-requests: write # The token is still read-only for forks.
-      statuses: write
-
-    concurrency:
-      # Only allow a single run of this workflow on each branch, automatically cancelling older runs.
-      group: ${{ format('danger-ci-{0}', github.ref) }}
-      cancel-in-progress: true
-
-    steps:
-      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
-        with:
-          persist-credentials: false
-
-      - name: Danger
-        uses: danger/swift@751aa740b2bd69ea1b54444a945961e64c38fc22 # 3.22.1
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/pr-checks.yml

@@ -0,0 +1,109 @@
+name: PR Checks
+
+on:
+  # Privilege escalation necessary to comment on pull requests from forks.
+  # 🚨 We must not check out or execute any code here, and be careful around use of user-controlled inputs.
+  pull_request_target: # zizmor: ignore[dangerous-triggers]
+    types: [opened, reopened, edited, synchronize, labeled, unlabeled]
+
+permissions: {}
+
+jobs:
+  pr-checks:
+    name: PR Checks
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+
+    concurrency:
+      # Only allow a single run of this workflow on each branch, automatically cancelling older runs.
+      group: ${{ format('pr-checks-{0}', github.ref) }}
+      cancel-in-progress: true
+
+    permissions:
+      pull-requests: write
+
+    steps:
+      - name: Check the pull request
+        uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
+        with:
+          script: |
+            const pr = context.payload.pull_request;
+            const body = pr.body ?? "";
+
+            const files = await github.paginate(github.rest.pulls.listFiles, {
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              pull_number: pr.number
+            });
+            const paths = files.map(file => file.filename.toLowerCase());
+
+            const warnings = [];
+            const failures = [];
+
+            if (pr.additions > 1000) {
+              warnings.push("This pull request seems relatively large. Please consider splitting it into multiple smaller ones.");
+            }
+
+            if (!body) {
+              warnings.push("Please provide a description for this PR.");
+            }
+
+            if (paths.some(path => path.includes("/view")) && !body.includes("user-attachments")) {
+              warnings.push("You seem to have made changes to views. Please consider adding screenshots.");
+            }
+
+            if (paths.some(path => path.includes(".xcassets") && path.endsWith(".png"))) {
+              warnings.push("You seem to have made changes to some resource images. Please consider using an SVG or PDF.");
+            }
+
+            if (pr.title.endsWith("…") || /^(Fixes|Fix) #\d+/.test(pr.title) || /^\w+\(\w+\):/.test(pr.title)) {
+              failures.push("Please provide a complete title that can be used as a changelog entry.");
+            }
+
+            if (pr.labels.filter(label => label.name.startsWith("pr-")).length !== 1) {
+              failures.push("Please add a `pr-` label to categorise the changelog entry.");
+            }
+
+            warnings.forEach(warning => core.warning(warning));
+            failures.forEach(failure => core.error(failure));
+
+            // Keep a single comment up to date rather than adding a new one on every run.
+            const marker = "<!-- pr-checks -->";
+            const comments = await github.paginate(github.rest.issues.listComments, {
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              issue_number: pr.number
+            });
+            const existing = comments.find(comment => comment.body.includes(marker));
+
+            if (warnings.length || failures.length) {
+              const lines = [...failures.map(failure => `- ❌ ${failure}`),
+                             ...warnings.map(warning => `- ⚠️ ${warning}`)];
+              const commentBody = `${marker}\n### PR Checks\n\n${lines.join("\n")}`;
+
+              if (existing) {
+                await github.rest.issues.updateComment({
+                  owner: context.repo.owner,
+                  repo: context.repo.repo,
+                  comment_id: existing.id,
+                  body: commentBody
+                });
+              } else {
+                await github.rest.issues.createComment({
+                  owner: context.repo.owner,
+                  repo: context.repo.repo,
+                  issue_number: pr.number,
+                  body: commentBody
+                });
+              }
+            } else if (existing) {
+              await github.rest.issues.deleteComment({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                comment_id: existing.id
+              });
+            }
+
+            if (failures.length) {
+              core.setFailed(`${failures.length} check(s) failed, see the comment on the pull request.`);
+            }

AGENTS.md

@@ -380,7 +380,7 @@ Same pattern for publishers.
 | `app.yml` | App-level XcodeGen config |
 | `.swiftlint.yml` | SwiftLint rules |
 | `.swiftformat` | SwiftFormat rules |
-| `Dangerfile.swift` | Danger PR checks |
+| `.github/workflows/pr-checks.yml` | PR metadata checks (title, labels, size, screenshots) |
 | `Package.swift` | SPM manifest (Tools CLI) |
 | `localazy.json` | Localazy translation config |
 | `codecov.yml` | Codecov config |

CONTRIBUTING.md

@@ -150,7 +150,7 @@ Our [changelog](CHANGES.md) is automatically generated by GitHub, based on the P
 For Swift coding style we use [SwiftLint](https://github.com/realm/SwiftLint) to check some conventions at compile time (rules are located in the `.swiftlint.yml` file). 
 Otherwise please have a look to [Apple Swift conventions](https://swift.org/documentation/api-design-guidelines.html#conventions). We are also using some of the conventions of [raywenderlich.com Swift style guide](https://github.com/raywenderlich/swift-style-guide).
 
-We enforce the coding style by running checks on the CI for every PR through [Danger](Dangerfile.swift), [SwiftLint](.swiftlint.yml), [SwiftFormat](.swiftformat) and [SonarCloud](https://sonarcloud.io/project/overview?id=element-x-ios)
+We enforce the coding style by running checks on the CI for every PR through [PR Checks](.github/workflows/pr-checks.yml), [SwiftLint](.swiftlint.yml), [SwiftFormat](.swiftformat) and [SonarCloud](https://sonarcloud.io/project/overview?id=element-x-ios)
 
 We also gather coverage reports on every PR through [Codecov](https://app.codecov.io/gh/element-hq/element-x-ios) and will eventually start enforcing minimums.
 

Dangerfile.swift

@@ -1,52 +0,0 @@
-import Danger
-import Foundation
-
-SwiftLint.lint(.modifiedAndCreatedFiles(directory: nil),
-               inline: true,
-               configFile: nil,
-               strict: false,
-               quiet: true,
-               swiftlintPath: nil,
-               markdownAction: { _ in })
-
-let danger = Danger()
-
-/// All of the new and modified files together.
-let editedFiles = danger.git.modifiedFiles + danger.git.createdFiles
-
-// Warn when there is a big PR
-if (danger.github.pullRequest.additions ?? 0) > 1000 {
-    warn("This pull request seems relatively large. Please consider splitting it into multiple smaller ones.")
-}
-
-// Check that the PR has a description
-if danger.github.pullRequest.body?.isEmpty ?? true {
-    warn("Please provide a description for this PR.")
-}
-
-/// Check for screenshots on view changes
-let hasChangedViews = editedFiles.contains { $0.lowercased().contains("/view") }
-if hasChangedViews {
-    if (danger.github.pullRequest.body?.contains("user-attachments") ?? false) == false {
-        warn("You seem to have made changes to views. Please consider adding screenshots.")
-    }
-}
-
-/// Check for pngs on resources
-let hasPngs = editedFiles.contains { $0.lowercased().contains(".xcassets") && $0.lowercased().hasSuffix(".png") }
-if hasPngs {
-    warn("You seem to have made changes to some resource images. Please consider using an SVG or PDF.")
-}
-
-// Check for nice PR titles
-let prTitle = danger.github.pullRequest.title
-let fixesRegex = try! Regex("(Fixes|Fix) #\\d+")
-let semanticRegex = try! Regex("\\w+\\(\\w+\\):")
-if prTitle.hasSuffix("…") || prTitle.starts(with: fixesRegex) || prTitle.starts(with: semanticRegex) {
-    fail("Please provide a complete title that can be used as a changelog entry.")
-}
-
-// Check for changelog tags
-if danger.github.issue.labels.filter({ $0.name.hasPrefix("pr-") }).count != 1 {
-    fail("Please add a `pr-` label to categorise the changelog entry.")
-}