From 0df433734ed41d982a69506f82702e96c45e9453 Mon Sep 17 00:00:00 2001 From: YE Date: Tue, 18 Aug 2026 11:47:49 +0900 Subject: [PATCH 1/2] ci: give SwiftLint a config, so 3,104 violations become 35 that mean something MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There was no `.swiftlint.yml` anywhere. CI therefore ran SwiftLint's DEFAULT rule set and reported 3,104 violations across 248 files, every run, for months. A check that is always red carries exactly as much information as one that is always green. It cost real time too: v1.49's PR had to verify by hand that its new files were clean, because the check itself could not say. The defaults were not wrong so much as not this project's: 1,734 (56%) identifier_name — objecting to `s`, `v`, `c`, `k`, `d` in closures and math. A house-style opinion this codebase has declined 1,734 times. 376 line_length 213 trailing_comma 153 opening_brace ~700 size/complexity rules, which measure "this file is big" `only_rules` (an allowlist) rather than a long `disabled_rules`, for two reasons: it states what we believe instead of what we tolerate, and a SwiftLint upgrade cannot silently add a rule and redden every PR. before: 3,104 violations / 248 files after: 35 violations / 15 files Also PINS SwiftLint to 0.63.2. `brew install swiftlint` was unpinned, while ruff three files away carries a paragraph explaining why that is unacceptable. Same failure mode, same fix. WHAT IS DELIBERATELY NOT DONE HERE The remaining 35 are left, and the job stays `continue-on-error: true`, because two of them must not be fixed mechanically and this PR touches no Swift at all (so it cannot conflict with the 22 files in the open #432): * 6 x force_try are `try!` on NSRegularExpression built from COMPILE-TIME CONSTANT patterns. The rule is right in general and wrong here; making the property optional would complicate every call site for zero safety. They need a reasoned `swiftlint:disable:next` each, not a blind rewrite. * ProviderAccountDeletionOutbox.swift:19 unneeded_synthesized_initializer is correct on main and SELF-RESOLVES when #432 lands — that PR gives the init a `generation: UUID? = UUID()` default, which the synthesized memberwise init would not provide. Deleting the init today would be reverted tomorrow; deleting it after #432 would silently drop a default. Zeroing the 35 and flipping the job to blocking is the follow-up, once #432 is in. Only then does turning it red mean anything. Co-Authored-By: Claude Opus 5 --- .github/workflows/lint-ci.yml | 16 +++++++-- CLI Pulse Bar/.swiftlint.yml | 61 +++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 CLI Pulse Bar/.swiftlint.yml diff --git a/.github/workflows/lint-ci.yml b/.github/workflows/lint-ci.yml index 40333d4d..3a48ff8a 100644 --- a/.github/workflows/lint-ci.yml +++ b/.github/workflows/lint-ci.yml @@ -33,8 +33,20 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v4 - - name: Install SwiftLint - run: brew install swiftlint + # PINNED, for the same reason ruff is pinned in helper-ci.yml: an + # unpinned linter turns PRs red for code they never touched the moment + # upstream enables a rule. `brew install swiftlint` was unpinned here + # while ruff carried a paragraph explaining why that is unacceptable. + # Raising this is a deliberate task — bump it, run the lint, fix or + # allowlist what the new version finds, in a PR that does only that. + - name: Install SwiftLint (pinned) + run: | + set -euo pipefail + curl -fsSL -o /tmp/swiftlint.zip \ + https://github.com/realm/SwiftLint/releases/download/0.63.2/portable_swiftlint.zip + unzip -q -o /tmp/swiftlint.zip -d /tmp/swiftlint + sudo install -m 0755 /tmp/swiftlint/swiftlint /usr/local/bin/swiftlint + swiftlint version - name: Run SwiftLint on CLI Pulse Bar targets working-directory: CLI Pulse Bar diff --git a/CLI Pulse Bar/.swiftlint.yml b/CLI Pulse Bar/.swiftlint.yml new file mode 100644 index 00000000..5ed8d423 --- /dev/null +++ b/CLI Pulse Bar/.swiftlint.yml @@ -0,0 +1,61 @@ +# SwiftLint configuration — opt-in, not opt-out. +# +# WHY THIS FILE EXISTS +# -------------------- +# There was no config at all, so CI ran SwiftLint's DEFAULT rule set and +# reported 3,104 violations across 248 files, every run, for months. A check +# that is always red carries exactly as much information as one that is always +# green: none. It cost real time too — v1.49's PR had to verify by hand that +# its new files were clean, because the check itself could not say. +# +# The defaults were not wrong so much as not this project's. 1,734 of the +# 3,104 — 56% — were `identifier_name` objecting to `s`, `v`, `c`, `k`, `d` +# in closures and math. That is a house-style opinion this codebase has +# declined 1,734 times. Another ~700 were size and line-length rules, which +# measure "this file is big", something everyone already knows. +# +# So this is `only_rules`, an ALLOWLIST, rather than a long `disabled_rules` +# list. Two reasons: +# * it states what we believe instead of what we tolerate, and +# * a SwiftLint upgrade cannot silently add a new rule and turn every PR +# red for code nobody touched. That failure has a precedent here: see the +# pinned-ruff comment in helper-ci.yml. +# +# Adding a rule is deliberate: add it, fix the fallout in a PR that does only +# that, and it becomes load-bearing from then on. +only_rules: + # Crashes waiting to happen. + - force_try + - force_cast + - implicitly_unwrapped_optional + + # Code that says something other than what it does. + - unused_optional_binding + - unused_closure_parameter + - unused_control_flow_label + - redundant_discardable_let + - redundant_string_enum_value + - redundant_void_return + - unneeded_synthesized_initializer + - unneeded_break_in_switch + - nsnumber_init_as_function_reference + + # Readability rules with no plausible false positive. + - control_statement + - prefer_for_where + - syntactic_sugar + - empty_count + - empty_string + - is_disjoint + - toggle_bool + - last_where + - first_where + - contains_over_filter_count + - contains_over_first_not_nil + + # Non-negotiable regardless of style. + - inclusive_language + +excluded: + - CLIPulseCore/.build + - build From 5b8c5d521ba317d909d3324326c2927e2eb174c0 Mon Sep 17 00:00:00 2001 From: YE Date: Tue, 18 Aug 2026 12:00:57 +0900 Subject: [PATCH 2/2] ci: make SwiftLint block, now that being red means something MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-on to the config in this branch's first commit. Three corrections the CI job log surfaced that a local `--quiet` run had hidden: 1. TWO RULE NAMES IN THE ALLOWLIST WERE INVALID. `nsnumber_init_as_function_reference` and `prefer_for_where` do not exist; the real name is `for_where`. SwiftLint only WARNS about an unknown identifier and carries on, so both rules were silently inactive — an allowlist that quietly ignores entries is its own trap. Fixed, and `for_where` immediately found 5 real sites. 2. force_try IS ERROR SEVERITY, and was the only thing making the job exit 2. All 6 sites are `try!` on NSRegularExpression built from compile-time string literals: they cannot fail at runtime, and making the properties optional would push a nil check onto every call site for no safety. Each now carries a reasoned `swiftlint:disable:next` — documentation of WHY it is safe, which is worth more than either silence or a blanket severity downgrade. The rule stays an error so an UNjustified `try!` still stops a merge. 3. I HAD BEEN READING THE WRONG CSV COLUMN. SwiftLint's CSV is file,line,char,severity,type,reason,rule_id — index 5 is the human message, index 6 is the rule id. Two annotation passes silently matched nothing and reported success. The earlier "715 distinct rules" was 715 distinct messages. Result: 3,104 violations -> 33 warnings, 0 errors, exit 0. So `continue-on-error: true` is removed and the job now BLOCKS. A red check that blocks nothing is what let 3,104 accumulate in the first place. VERIFIED THE GATE FIRES, not just that it passes: appending a deliberate `try!` to PrivacySettings.swift made the lint exit 2; restoring the file returned exit 0. The 33 remaining are warning-severity and do not fail the job. Zeroing them and adding --strict is the follow-up; one of them is in ProviderAccountDeletionOutbox and must wait for #432, which changes that initializer. Co-Authored-By: Claude Opus 5 --- .github/workflows/lint-ci.yml | 8 ++++++-- CLI Pulse Bar/.swiftlint.yml | 9 +++++++-- .../ClaudeConversationPreviewFormatter.swift | 8 ++++++++ .../CLIPulseCore/CodexConversationPreviewFormatter.swift | 8 ++++++++ .../GeminiConversationPreviewFormatter.swift | 8 ++++++++ 5 files changed, 37 insertions(+), 4 deletions(-) diff --git a/.github/workflows/lint-ci.yml b/.github/workflows/lint-ci.yml index 3a48ff8a..c405afcf 100644 --- a/.github/workflows/lint-ci.yml +++ b/.github/workflows/lint-ci.yml @@ -26,10 +26,14 @@ concurrency: jobs: swiftlint: - name: SwiftLint (warning-only) + # No longer "warning-only". With a config in place the error-severity rules + # (force_try, force_cast, implicitly_unwrapped_optional) now BLOCK, because + # a red check that blocks nothing is what let 3,104 violations accumulate. + # The remaining ~33 are warning-severity and do not fail the job; zeroing + # them and adding --strict is the follow-up. + name: SwiftLint runs-on: macos-14 timeout-minutes: 10 - continue-on-error: true steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v4 diff --git a/CLI Pulse Bar/.swiftlint.yml b/CLI Pulse Bar/.swiftlint.yml index 5ed8d423..92fa7d85 100644 --- a/CLI Pulse Bar/.swiftlint.yml +++ b/CLI Pulse Bar/.swiftlint.yml @@ -38,11 +38,10 @@ only_rules: - redundant_void_return - unneeded_synthesized_initializer - unneeded_break_in_switch - - nsnumber_init_as_function_reference # Readability rules with no plausible false positive. - control_statement - - prefer_for_where + - for_where - syntactic_sugar - empty_count - empty_string @@ -56,6 +55,12 @@ only_rules: # Non-negotiable regardless of style. - inclusive_language +# force_try defaults to ERROR severity, which is why it — and only it — made +# this job exit 2. Keeping it as an error is correct: an unjustified `try!` in +# this codebase should stop a merge. The justified ones carry a reasoned +# `swiftlint:disable:next` at the site, which is better documentation than +# silence and better safety than a blanket downgrade. + excluded: - CLIPulseCore/.build - build diff --git a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ClaudeConversationPreviewFormatter.swift b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ClaudeConversationPreviewFormatter.swift index 1d95e2e2..58ad6b5a 100644 --- a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ClaudeConversationPreviewFormatter.swift +++ b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ClaudeConversationPreviewFormatter.swift @@ -334,6 +334,10 @@ public enum ClaudeConversationPreviewFormatter { /// Only triggers when the punctuation is immediately followed by /// an uppercase ASCII letter (heuristic for sentence boundary). private static let sentencePunctPattern: NSRegularExpression = { + // The pattern is a compile-time string literal, so this cannot fail at + // runtime. Making the property optional would push a nil check onto every + // call site for no safety gain. + // swiftlint:disable:next force_try try! NSRegularExpression(pattern: "([\\.!?])([A-Z])", options: []) }() @@ -399,6 +403,10 @@ public enum ClaudeConversationPreviewFormatter { /// consume `[1a` and leave a dangling `]`. private static let orphanCsiPattern: NSRegularExpression = { let pattern = "\\[[0-9;:?<>=]+[ -/]*[a-zA-Z](?!\\])" + // The pattern is a compile-time string literal, so this cannot fail at + // runtime. Making the property optional would push a nil check onto every + // call site for no safety gain. + // swiftlint:disable:next force_try return try! NSRegularExpression(pattern: pattern, options: []) }() diff --git a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/CodexConversationPreviewFormatter.swift b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/CodexConversationPreviewFormatter.swift index 2bc09004..2abe49a0 100644 --- a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/CodexConversationPreviewFormatter.swift +++ b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/CodexConversationPreviewFormatter.swift @@ -192,6 +192,10 @@ public enum CodexConversationPreviewFormatter { /// emits them. private static let numberedMenuPattern: NSRegularExpression = { let pattern = "^›?\\s*(?:\\(\\d+\\)|\\[\\d+\\]|\\d+\\.)\\s+\\S" + // The pattern is a compile-time string literal, so this cannot fail at + // runtime. Making the property optional would push a nil check onto every + // call site for no safety gain. + // swiftlint:disable:next force_try return try! NSRegularExpression(pattern: pattern, options: []) }() @@ -278,6 +282,10 @@ public enum CodexConversationPreviewFormatter { // covers every real-world CSI final byte, and the lookahead // catches the `[1a]`-style footnote false-positive class. let pattern = "\\[[0-9;:?<>=]+[ -/]*[a-zA-Z](?!\\])" + // The pattern is a compile-time string literal, so this cannot fail at + // runtime. Making the property optional would push a nil check onto every + // call site for no safety gain. + // swiftlint:disable:next force_try return try! NSRegularExpression(pattern: pattern, options: []) }() diff --git a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/GeminiConversationPreviewFormatter.swift b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/GeminiConversationPreviewFormatter.swift index 55ea680c..06e8a89f 100644 --- a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/GeminiConversationPreviewFormatter.swift +++ b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/GeminiConversationPreviewFormatter.swift @@ -223,6 +223,10 @@ public enum GeminiConversationPreviewFormatter { /// optional leading whitespace. private static let numberedMenuPattern: NSRegularExpression = { let pattern = "^\\s*(?:\\(\\d+\\)|\\[\\d+\\]|\\d+\\.)\\s+\\S" + // The pattern is a compile-time string literal, so this cannot fail at + // runtime. Making the property optional would push a nil check onto every + // call site for no safety gain. + // swiftlint:disable:next force_try return try! NSRegularExpression(pattern: pattern, options: []) }() @@ -309,6 +313,10 @@ public enum GeminiConversationPreviewFormatter { // covers every real-world CSI final byte, and the lookahead // catches the `[1a]`-style footnote false-positive class. let pattern = "\\[[0-9;:?<>=]+[ -/]*[a-zA-Z](?!\\])" + // The pattern is a compile-time string literal, so this cannot fail at + // runtime. Making the property optional would push a nil check onto every + // call site for no safety gain. + // swiftlint:disable:next force_try return try! NSRegularExpression(pattern: pattern, options: []) }()