ci: give SwiftLint a config, so 3,104 violations become 35 that mean something - #442
Merged
Merged
Conversation
…something
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
There was no
.swiftlint.ymlanywhere. 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: 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 weren't wrong so much as not this project's:
identifier_names,v,c,k,din closures and math — a house-style opinion this codebase has declined 1,734 timesline_lengthtrailing_commaopening_braceThe change
only_rules— an allowlist — rather than a longdisabled_rules. 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.Also pins SwiftLint to 0.63.2.
brew install swiftlintwas unpinned — whileruff, three files away, carries a whole paragraph explaining why an unpinned linter is unacceptable ("a release that enables new rules turns a PR red for code it never touched… ten minutes went into proving the feature branch was innocent"). Same failure mode, same fix.What this deliberately does NOT do
The remaining 35 stay, and the job stays
continue-on-error: true. This PR touches no Swift at all, so it cannot conflict with the 22 files in the open #432. Two of the 35 are also traps:force_tryaretry!onNSRegularExpressionbuilt 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 gain. They need a reasoned// swiftlint:disable:next force_tryeach, not a blind rewrite.ProviderAccountDeletionOutbox.swift:19unneeded_synthesized_initializeris correct onmainand self-resolves when fix: harden provider account credential lifecycle #432 lands — that PR gives the init ageneration: UUID? = UUID()default, which the synthesized memberwise init would not provide. Deleting it today gets reverted tomorrow; deleting it after fix: harden provider account credential lifecycle #432 silently drops 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 — which is the whole point of this PR.
Verification
Measured locally with the pinned version (0.63.2, same as CI will now install), on the exact six paths CI lints. The CI job log should now report 35, not 3,104 — worth grepping rather than trusting the check colour, since the job is warning-only either way.
🤖 Generated with Claude Code