ci: gate PRs on the iOS conventions they introduce, not on main's state

.github/workflows/ci-guards.yml

@@ -126,6 +126,27 @@ jobs:
         if: ${{ matrix.group == 'preflight' }}
         run: python3 tests/test_web_complexity_scope.py
 
+      - name: Validate iOS package conventions for this change
+        if: ${{ matrix.group == 'preflight' }}
+        env:
+          BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}
+        run: |
+          set -euo pipefail
+          python3 tests/test_ci_ios_conventions_diff.py
+          # The conventions lint scans the whole repository, so gating a PR on
+          # it directly turns every open PR red whenever main carries one
+          # unrelated violation (#10409). Judge the change instead: report only
+          # violations absent from the base. Skipped outside PR/merge-group,
+          # where there is no base to compare against.
+          if [ -n "${BASE_SHA:-}" ]; then
+            if ! git cat-file -e "$BASE_SHA^{commit}" 2>/dev/null; then
+              git fetch --no-tags --depth=1 origin "$BASE_SHA"
+            fi
+            ./scripts/ci/lint-ios-conventions-diff.sh "$BASE_SHA"
+          else
+            echo "No base revision for this event; conventions diff skipped."
+          fi
+
       - name: Validate reload shared DerivedData default
         if: ${{ matrix.group == 'preflight' }}
         run: ./tests/test_reload_shared_derived_data.sh

scripts/ci/lint-ios-conventions-diff.sh

@@ -0,0 +1,96 @@
+#!/usr/bin/env bash
+# Report only the iOS convention violations a change introduces.
+#
+# scripts/lint-ios-package-conventions.sh scans the whole repository, so running
+# it on pull requests sends every open PR red whenever main carries one
+# unrelated violation. That is the complaint in #10409, and it is why the check
+# now lives only in the dispatch-only iOS lane, where nothing sees it until a
+# violation has already landed. This compares the violations at HEAD against
+# those at the base commit and fails only on the difference, so a pull request
+# is judged on what it adds rather than on the state of main.
+#
+# Findings are keyed by (rule, file, text), not line number, so code that moves
+# without changing is not reported as new.
+set -uo pipefail
+
+ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
+cd "$ROOT"
+
+BASE_SHA="${1:-}"
+if [ -z "$BASE_SHA" ]; then
+  echo "usage: $(basename "$0") <base-sha>" >&2
+  exit 2
+fi
+
+# ERROR lines are "ERROR   <rule>   <path>:<line>  <text>". Both the shell
+# rules and lint_swift_namespaces.py emit that shape on stdout.
+fingerprints() {
+  ./scripts/lint-ios-package-conventions.sh 2>/dev/null | awk '
+    $1 == "ERROR" {
+      rule = $2; loc = $3
+      $1 = ""; $2 = ""; $3 = ""
+      sub(/^[[:space:]]+/, "")
+      split(loc, parts, ":")
+      print rule "\t" parts[1] "\t" $0
+    }' | sort -u
+}
+
+base_tree=""
+head_list="$(mktemp)"
+base_list="$(mktemp)"
+cleanup() {
+  rm -f "$head_list" "$base_list"
+  if [ -n "$base_tree" ]; then
+    git worktree remove --force "$base_tree" >/dev/null 2>&1 || rm -rf "$base_tree"
+    git worktree prune >/dev/null 2>&1 || true
+  fi
+}
+trap cleanup EXIT
+
+fingerprints > "$head_list"
+
+if ! git cat-file -e "${BASE_SHA}^{commit}" 2>/dev/null; then
+  git fetch --no-tags --depth=1 origin "$BASE_SHA" >/dev/null 2>&1 || true
+fi
+if ! git cat-file -e "${BASE_SHA}^{commit}" 2>/dev/null; then
+  # Without a base to compare against, judging "new" is not possible. Report
+  # the current state rather than passing silently on an unknown comparison.
+  echo "::warning::base $BASE_SHA unavailable; reporting all current violations"
+  if [ -s "$head_list" ]; then
+    cut -f1,2 "$head_list" | sed 's/^/NEW  /'
+    exit 1
+  fi
+  exit 0
+fi
+
+base_tree="$(mktemp -d)/base"
+git worktree add --detach "$base_tree" "$BASE_SHA" >/dev/null 2>&1 || {
+  echo "::error::could not check out base $BASE_SHA"
+  exit 2
+}
+( cd "$base_tree" && ./scripts/lint-ios-package-conventions.sh 2>/dev/null | awk '
+    $1 == "ERROR" {
+      rule = $2; loc = $3
+      $1 = ""; $2 = ""; $3 = ""
+      sub(/^[[:space:]]+/, "")
+      split(loc, parts, ":")
+      print rule "\t" parts[1] "\t" $0
+    }' | sort -u ) > "$base_list"
+
+new_count=0
+while IFS=$'\t' read -r rule file text; do
+  [ -z "$rule" ] && continue
+  printf 'NEW  %-24s %s  %s\n' "$rule" "$file" "$text"
+  new_count=$((new_count + 1))
+done < <(comm -13 "$base_list" "$head_list")
+
+carried="$(wc -l < "$base_list" | tr -d ' ')"
+if [ "$new_count" -gt 0 ]; then
+  echo
+  echo "FAIL: $new_count convention violation(s) introduced by this change."
+  echo "($carried pre-existing violation(s) on the base are not this change's to fix.)"
+  exit 1
+fi
+
+echo "OK: no new convention violations ($carried pre-existing on the base)."
+exit 0

tests/test-execution.toml

@@ -1066,3 +1066,7 @@ lane = "linux-guard"
 [[test]]
 path = "tests/test_web_complexity_scope.py"
 lane = "linux-guard"
+
+[[test]]
+path = "tests/test_ci_ios_conventions_diff.py"
+lane = "linux-guard"

tests/test_ci_ios_conventions_diff.py

@@ -0,0 +1,105 @@
+#!/usr/bin/env python3
+"""The iOS conventions diff gate must judge a change, not the state of main.
+
+scripts/lint-ios-package-conventions.sh scans the whole repository. Running it
+directly on pull requests is what #10409 complains about: one unrelated
+violation on main turns every open PR red. scripts/ci/lint-ios-conventions-diff.sh
+compares HEAD against the base and reports only the difference, which is what
+makes the check safe to run per-PR.
+"""
+import os
+import subprocess
+import tempfile
+import unittest
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parents[1]
+GATE = ROOT / "scripts/ci/lint-ios-conventions-diff.sh"
+
+
+def _violation(rule, path, text):
+    return f"ERROR   {rule:<28} {path}  {text}"
+
+
+class ConventionsDiffGate(unittest.TestCase):
+    def build(self, base_findings, head_findings):
+        """A repo whose stub linter reports one set at base and another at HEAD."""
+        tmp = Path(tempfile.mkdtemp())
+        self.addCleanup(subprocess.run, ["rm", "-rf", str(tmp)], check=False)
+        (tmp / "scripts/ci").mkdir(parents=True)
+        (tmp / "scripts/ci/lint-ios-conventions-diff.sh").write_bytes(GATE.read_bytes())
+        os.chmod(tmp / "scripts/ci/lint-ios-conventions-diff.sh", 0o755)
+
+        def write_stub(findings):
+            body = "\n".join(f'echo "{line}"' for line in findings)
+            stub = tmp / "scripts/lint-ios-package-conventions.sh"
+            stub.write_text(f"#!/usr/bin/env bash\n{body}\nexit 0\n")
+            os.chmod(stub, 0o755)
+
+        git = lambda *a: subprocess.run(["git", "-C", str(tmp), *a], check=True,
+                                        capture_output=True)
+        git("init", "-q")
+        git("config", "user.email", "t@example.com")
+        git("config", "user.name", "t")
+        write_stub(base_findings)
+        git("add", "-A")
+        git("commit", "-qm", "base")
+        base = subprocess.run(["git", "-C", str(tmp), "rev-parse", "HEAD"],
+                              capture_output=True, text=True, check=True).stdout.strip()
+        write_stub(head_findings)
+        git("add", "-A")
+        git("commit", "-qm", "head", "--allow-empty")
+        return tmp, base
+
+    def run_gate(self, base_findings, head_findings):
+        tmp, base = self.build(base_findings, head_findings)
+        return subprocess.run([str(tmp / "scripts/ci/lint-ios-conventions-diff.sh"), base],
+                              capture_output=True, text=True, cwd=str(tmp))
+
+    def test_pre_existing_violations_on_the_base_do_not_fail_a_change(self):
+        """The #10409 case: main is dirty, the change adds nothing."""
+        carried = [_violation("free-function", "Packages/iOS/A.swift:10", "func a()"),
+                   _violation("lock", "Packages/iOS/B.swift:3", "let l = NSLock()")]
+        result = self.run_gate(carried, carried)
+        self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+        self.assertIn("2 pre-existing", result.stdout)
+        self.assertNotIn("NEW", result.stdout)
+
+    def test_a_violation_the_change_introduces_fails_and_is_named(self):
+        carried = [_violation("free-function", "Packages/iOS/A.swift:10", "func a()")]
+        result = self.run_gate(carried, carried + [
+            _violation("lock", "Packages/iOS/B.swift:3", "let l = NSLock()")])
+        self.assertEqual(result.returncode, 1, result.stdout)
+        self.assertIn("NEW", result.stdout)
+        self.assertIn("Packages/iOS/B.swift", result.stdout)
+        self.assertNotIn("Packages/iOS/A.swift", result.stdout.split("FAIL")[0])
+
+    def test_moving_code_is_not_a_new_violation(self):
+        """Findings key on (rule, file, text), so a line shift is not 'new'."""
+        result = self.run_gate(
+            [_violation("free-function", "Packages/iOS/A.swift:10", "func a()")],
+            [_violation("free-function", "Packages/iOS/A.swift:870", "func a()")])
+        self.assertEqual(result.returncode, 0, result.stdout)
+        self.assertNotIn("NEW", result.stdout)
+
+    def test_fixing_a_violation_still_passes(self):
+        result = self.run_gate(
+            [_violation("free-function", "Packages/iOS/A.swift:10", "func a()")], [])
+        self.assertEqual(result.returncode, 0, result.stdout)
+
+    def test_an_unavailable_base_reports_rather_than_passing_silently(self):
+        tmp, _ = self.build([], [_violation("lock", "Packages/iOS/B.swift:3", "NSLock()")])
+        result = subprocess.run(
+            [str(tmp / "scripts/ci/lint-ios-conventions-diff.sh"), "0" * 40],
+            capture_output=True, text=True, cwd=str(tmp))
+        self.assertEqual(result.returncode, 1, result.stdout)
+        self.assertIn("unavailable", result.stdout + result.stderr)
+
+    def test_a_missing_base_argument_is_a_usage_error(self):
+        result = subprocess.run([str(GATE)], capture_output=True, text=True)
+        self.assertEqual(result.returncode, 2)
+        self.assertIn("usage", result.stderr)
+
+
+if __name__ == "__main__":
+    unittest.main()