Skip to content

Commit 70c2ef4

Browse files
committed
fix LintCache fingerprint to invalidate on binary rebuild; add sm:ignore:next directive; cover ternary case 2
1 parent 8b4d1e0 commit 70c2ef4

8 files changed

Lines changed: 246 additions & 16 deletions

File tree

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
---
2+
# pel-a08
3+
title: WrapTernaryBranches still fires on single-line ternary after multi-line call
4+
status: completed
5+
type: bug
6+
priority: normal
7+
created_at: 2026-05-02T00:28:00Z
8+
updated_at: 2026-05-02T00:47:56Z
9+
sync:
10+
github:
11+
issue_number: "621"
12+
synced_at: "2026-05-02T00:48:50Z"
13+
---
14+
15+
Recent commit (8b4d1e03) claimed to fix this via 'scalar-pass WrapTernaryBranches.singleLineLength', but the rule still flags ternaries that fit on one line when the LHS is a multi-line function call.
16+
17+
## Repro
18+
19+
```swift
20+
return attribute(
21+
key,
22+
at: index,
23+
longestEffectiveRange: &valueRange,
24+
in: fullRange,
25+
) != nil ? valueRange : nil
26+
```
27+
28+
The ternary line `) != nil ? valueRange : nil` fits well within the line limit and should not be wrapped. WrapTernaryBranches fires twice (once for each branch).
29+
30+
## Expected
31+
32+
No findings — the ternary fits on a single line, so the singleLineLength scalar pass should suppress the diagnostic.
33+
34+
## Notes
35+
36+
- The LHS of the ternary spans multiple lines (the `attribute(...)` call), which may be confusing the line-length calculation.
37+
- Check whether the scalar pass measures from the start of the ternary expression vs the start of its line, and whether multi-line LHS triggers an early bail-out that skips the singleLineLength check.
38+
39+
40+
41+
## Summary of Changes
42+
43+
The fix from commit `8b4d1e03` is **already working correctly** — both reported cases produce zero findings when linted via the installed `sm` binary, and the existing + new test pass:
44+
45+
- Case 1 (`attribute(...) != nil ? valueRange : nil`) — covered by existing `ternaryAfterMultiLineConditionDoesNotWrapWhenOperatorLineFits`.
46+
- Case 2 (`isValidQuery || !matches.isEmpty ? .top(...) : .all(...)` inside a function-call argument) — added regression test `ternaryWithMultiLineDisjunctionConditionDoesNotWrapWhenOperatorLineFits` in `Tests/SwiftiomaticTests/Rules/Wrap/WrapTernaryBranchesTests.swift`.
47+
48+
Verified: `/opt/homebrew/bin/sm lint` on both inputs emits no findings. `WrapTernaryBranchesTests` 7/7 green.
49+
50+
The warnings shown in Xcode are **stale lint diagnostics** cached by SourceKit/the "Lint Source Code" SPM plugin. They will clear on the next explicit lint invocation (right-click → Lint Source Code) or after editing+saving the file. No code change needed beyond the added test.
51+
52+
53+
54+
## Actual Root Cause (correction to earlier summary)
55+
56+
The rule fix from `8b4d1e03` was correct — but the user was seeing **stale findings replayed from `.build/sm-lint-cache/`**, not fresh ones. The cache fingerprint hashed sorted rule type names + configuration, but **not** the rule logic. So a binary rebuild that changed rule behavior without renaming/adding/removing rules left the fingerprint unchanged → cache hit → previous (pre-fix) findings replayed.
57+
58+
### Fix
59+
60+
`Sources/SwiftiomaticKit/Support/LintCache.swift``ruleSetIdentifier` now also mixes in the running executable's path + size + mtime. Any rebuild changes mtime+size, which cycles the fingerprint and orphans every prior cache subtree. Bumped the fingerprint prefix from `rules.v1``rules.v2` to flush all existing caches one final time.
61+
62+
### User-side cleanup
63+
64+
Removed the stale cache at `/Users/jason/Developer/toba/thesis/.build/sm-lint-cache/`. After the next Xcode build the inline warnings should be gone.
65+
66+
### Verification
67+
68+
- Filtered test: `LintCache|WrapTernaryBranches` 17/17 passed.
69+
- Reproduced and confirmed fix: with `--no-cache` the rule never fires; with the new binary the cache stays consistent across rebuilds.
70+
- New `sm` binary deployed to `/opt/homebrew/Cellar/sm/3.0.5/bin/sm`.
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
# w0n-ctq
3+
title: Add sm:ignore:next directive scoped to next line only
4+
status: completed
5+
type: feature
6+
priority: normal
7+
created_at: 2026-05-02T00:32:08Z
8+
updated_at: 2026-05-02T00:35:00Z
9+
sync:
10+
github:
11+
issue_number: "620"
12+
synced_at: "2026-05-02T00:48:50Z"
13+
---
14+
15+
Add a new ignore directive form: `// sm:ignore:next <rules>` that applies only to the next statement/member, not from the comment to EOF.
16+
17+
## Tasks
18+
- [x] Add tests in RuleMaskTests for the new `:next` form
19+
- [x] Update regex in RuleStatusCollectionVisitor to capture optional `:next` scope
20+
- [x] When directive has `:next` scope, record nodeRange instead of restOfFileRange
21+
- [x] Update doc comment in RuleMask.swift
22+
- [x] Verify full test suite passes
23+
24+
25+
## Summary of Changes
26+
27+
- New directive form `// sm:ignore:next [rules]` applies the ignore only to the immediately following statement or member, instead of extending to EOF.
28+
- Updated regex in `RuleStatusCollectionVisitor` to capture optional `:next` scope; threaded scope through to range selection in `applyDirectives` (uses `nodeRange` for `:next`, `restOfFileRange` otherwise).
29+
- Updated `isFormatterIgnorePresent` in `CommentMovingRewriter` to also recognize the bare `// sm:ignore:next` form for skipping pretty-printing of the next node.
30+
- Added 4 new tests covering `:next` with rule names, bare `:next`, multi-line statement scope, and member scope.
31+
- Updated `RuleMask.swift` doc comment to describe the three forms.
32+
- Full test suite (3179 tests) passes.

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,11 @@
4040
- `PreferTypedThrowsOverResult`; lint `Result<T, E>` returns with a single do/catch ([#572](https://github.com/toba/swiftiomatic/issues/572))
4141
- `WarnSwapThenRemoveAll`; lint `swap(&a, &b)` followed by `a.removeAll`/`b.removeAll` ([#577](https://github.com/toba/swiftiomatic/issues/577))
4242
- `// sm:ignore`; unified directive replaces `// sm:ignore-file`; applies from comment to end of file (lone-line) or to the line only (trailing) ([#595](https://github.com/toba/swiftiomatic/issues/595))
43+
- `// sm:ignore:next`; new directive scoped to the next line only ([#620](https://github.com/toba/swiftiomatic/issues/620))
4344

4445
### 🐞 Fixes
4546

47+
- `LintCache`; mix the running executable's path/size/mtime into the rule-set fingerprint so binary rebuilds invalidate stale cached findings ([#621](https://github.com/toba/swiftiomatic/issues/621))
4648
- `SimplifyGenericConstraints`; anchor diagnostics to the original `where` clause instead of the rewritten subtree's detached position ([#615](https://github.com/toba/swiftiomatic/issues/615))
4749
- `wrapTernaryBranches`; skip when both `?` and `:` source lines already fit, even when an operand is a multi-line chain ([#614](https://github.com/toba/swiftiomatic/issues/614))
4850
- `ReflowComments`; preserve CommonMark link reference definitions (`[label]: url`) instead of merging adjacent definitions into one paragraph ([#613](https://github.com/toba/swiftiomatic/issues/613))

Sources/SwiftiomaticKit/Layout/Tokens/CommentMovingRewriter.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,13 +138,14 @@ final class CommentMovingRewriter: SyntaxRewriter {
138138
/// Returns whether the given trivia includes a directive to ignore formatting for the next node.
139139
///
140140
/// - Parameter trivia: Leading trivia for a node that the formatter supports ignoring.
141-
/// - Returns: Whether the trivia contains a bare `sm:ignore` directive (no rule names).
141+
/// - Returns: Whether the trivia contains a bare `sm:ignore` (or `sm:ignore:next`) directive
142+
/// (no rule names).
142143
func isFormatterIgnorePresent(inTrivia trivia: Trivia) -> Bool {
143144
func isFormatterIgnore(in commentText: String, prefix: String, suffix: String) -> Bool {
144145
let trimmed = commentText.dropFirst(prefix.count)
145146
.dropLast(suffix.count)
146147
.trimmingCharacters(in: .whitespaces)
147-
return trimmed == "sm:ignore"
148+
return trimmed == "sm:ignore" || trimmed == "sm:ignore:next"
148149
}
149150

150151
for piece in trivia {

Sources/SwiftiomaticKit/Support/LintCache.swift

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,19 +99,38 @@ package final class LintCache: Sendable {
9999

100100
/// A binary-stable identifier for the rule set compiled into this `sm` .
101101
///
102-
/// Computed once per process from sorted rule type names. When the binary gains, loses, or
103-
/// renames a rule the value changes, which combined with the per-configuration JSON hash
104-
/// produces a new fingerprint and orphans every prior cache subtree.
102+
/// Computed once per process from sorted rule type names plus the running executable's
103+
/// (path, size, mtime). When the binary gains, loses, or renames a rule, *or* when the
104+
/// executable is rebuilt with no surface change but altered rule logic, the value changes,
105+
/// which combined with the per-configuration JSON hash produces a new fingerprint and
106+
/// orphans every prior cache subtree.
105107
private static let ruleSetIdentifier: String = {
106108
var hasher = SHA256()
107-
hasher.update(data: Data("rules.v1\n".utf8))
109+
hasher.update(data: Data("rules.v2\n".utf8))
108110
let names = ConfigurationRegistry.allRuleTypes
109111
.map { String(reflecting: $0) }
110112
.sorted()
111113
for name in names {
112114
hasher.update(data: Data(name.utf8))
113115
hasher.update(data: Data([0]))
114116
}
117+
// Mix in the running executable's identity so that rebuilding `sm` (which can change
118+
// rule logic without changing rule names) invalidates the cache. Falls back to the
119+
// unmodified rule-name digest if the executable's attributes are unreadable.
120+
if let exePath = CommandLine.arguments.first,
121+
let attrs = try? FileManager.default.attributesOfItem(atPath: exePath)
122+
{
123+
hasher.update(data: Data(exePath.utf8))
124+
hasher.update(data: Data([0]))
125+
if let size = attrs[.size] as? NSNumber {
126+
hasher.update(data: Data("\(size.uint64Value)".utf8))
127+
hasher.update(data: Data([0]))
128+
}
129+
if let mtime = attrs[.modificationDate] as? Date {
130+
hasher.update(data: Data("\(mtime.timeIntervalSince1970)".utf8))
131+
hasher.update(data: Data([0]))
132+
}
133+
}
115134
return hexEncode(hasher.finalize())
116135
}()
117136

Sources/SwiftiomaticKit/Syntax/RuleMask.swift

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,22 @@ import Foundation
1414
import SwiftSyntax
1515

1616
/// Scans the source for `// sm:ignore` directives and records which rules are disabled in which
17-
/// ranges. There are two forms:
17+
/// ranges. There are three forms:
1818
///
1919
/// - **Lone-line** `// sm:ignore [Rule1, Rule2]` on a line by itself → rules disabled from the
2020
/// comment's position through end of file. Placing it at the top of a file therefore
2121
/// disables those rules for the whole file (replaces the older `sm:ignore-file`).
22+
/// - **Lone-line scoped** `// sm:ignore:next [Rule1, Rule2]` on a line by itself → rules
23+
/// disabled only for the immediately following statement (or member).
2224
/// - **Trailing** `// sm:ignore [Rule1, Rule2]` on the same line as a statement (or member) →
2325
/// rules disabled for that statement only.
2426
///
2527
/// Examples:
2628
///
2729
/// // sm:ignore — ignore all rules from here to EOF
2830
/// // sm:ignore fileLength, typeBodyLength — those rules off from here to EOF
31+
/// // sm:ignore:next — ignore all rules for the next statement
32+
/// // sm:ignore:next Rule1 — ignore Rule1 for the next statement
2933
/// let x = "trouble" // sm:ignore — ignore all rules for this line only
3034
/// let x = 1 // sm:ignore Rule1 — ignore Rule1 for this line only
3135
///
@@ -99,14 +103,14 @@ private final class RuleStatusCollectionVisitor: SyntaxVisitor {
99103
case subset(ruleNames: [String])
100104
}
101105

102-
typealias RegexExpression = Regex<(Substring, ruleNames: Substring?)>
106+
typealias RegexExpression = Regex<(Substring, scope: Substring?, ruleNames: Substring?)>
103107

104108
/// Cached regex object for the unified `sm:ignore` directive.
105109
///
106110
/// Note: We are using a string-based regex instead of a regex literal ( `#/regex/#` ) because
107111
/// Windows did not have full support for regex literals until Swift 5.10.
108112
private static nonisolated(unsafe) let ignoreRegex: RegexExpression = {
109-
let pattern = #"^\s*\/\/\s*sm:ignore(?:\s+(?<ruleNames>\S.*))?$"#
113+
let pattern = #"^\s*\/\/\s*sm:ignore(?<scope>:next)?(?:\s+(?<ruleNames>\S.*))?$"#
110114
return try! Regex(pattern).matchingSemantics(.unicodeScalar)
111115
}()
112116

@@ -171,8 +175,9 @@ private final class RuleStatusCollectionVisitor: SyntaxVisitor {
171175

172176
let isFirstInFile = firstToken.previousToken(viewMode: .sourceAccurate) == nil
173177
for comment in loneLineComments(in: firstToken.leadingTrivia, isFirstToken: isFirstInFile) {
174-
guard let match = ruleStatusDirectiveMatch(in: comment) else { continue }
175-
record(match, range: restOfFileRange)
178+
guard let (match, scope) = ruleStatusDirectiveMatch(in: comment) else { continue }
179+
// `:next` scopes the directive to this single node; bare lone-line extends to EOF.
180+
record(match, range: scope == .next ? nodeRange : restOfFileRange)
176181
}
177182

178183
for token in node.tokens(viewMode: .sourceAccurate) {
@@ -181,7 +186,7 @@ private final class RuleStatusCollectionVisitor: SyntaxVisitor {
181186
// on a struct member would leak up to the enclosing type, etc.
182187
if isInsideDescendantItem(token, of: node) { continue }
183188
for comment in trailingLineComments(in: token.trailingTrivia) {
184-
guard let match = ruleStatusDirectiveMatch(in: comment) else { continue }
189+
guard let (match, _) = ruleStatusDirectiveMatch(in: comment) else { continue }
185190
record(match, range: nodeRange)
186191
}
187192
}
@@ -211,11 +216,17 @@ private final class RuleStatusCollectionVisitor: SyntaxVisitor {
211216
}
212217
}
213218

219+
/// Scope of a matched directive.
220+
enum DirectiveScope { case eof, next }
221+
214222
/// Checks if a comment containing the given text matches a rule status directive. When it does
215-
/// match, its contents (e.g. list of rule names) are returned.
216-
private func ruleStatusDirectiveMatch(in text: String) -> RuleStatusDirectiveMatch? {
223+
/// match, its contents (rule names and scope) are returned.
224+
private func ruleStatusDirectiveMatch(
225+
in text: String
226+
) -> (match: RuleStatusDirectiveMatch, scope: DirectiveScope)? {
217227
guard let match = text.firstMatch(of: Self.ignoreRegex) else { return nil }
218-
guard let matchedRuleNames = match.output.ruleNames else { return .all }
228+
let scope: DirectiveScope = match.output.scope != nil ? .next : .eof
229+
guard let matchedRuleNames = match.output.ruleNames else { return (.all, scope) }
219230

220231
let rules = matchedRuleNames.split(separator: ",").compactMap { segment -> String? in
221232
let name = segment.trimmingCharacters(in: .whitespaces)
@@ -227,7 +238,7 @@ private final class RuleStatusCollectionVisitor: SyntaxVisitor {
227238
// Resolve custom keys (e.g. SortImports → "imports" not "sortImports")
228239
return ConfigurationRegistry.typeNameToKey[derived] ?? derived
229240
}
230-
return .subset(ruleNames: rules)
241+
return (.subset(ruleNames: rules), scope)
231242
}
232243

233244
/// Returns the list of line comments in the given trivia that are on a line by themselves

Tests/SwiftiomaticTests/Core/RuleMaskTests.swift

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,76 @@ struct RuleMaskTests {
351351
#expect(mask.ruleState("anyRule", at: location(ofLine: 12, in: converter)) == .disabled)
352352
}
353353

354+
// MARK: - `:next` directive: applies only to the next statement.
355+
356+
@Test func nextDirectiveAppliesOnlyToNextStatement() {
357+
let text =
358+
"""
359+
let a = 123
360+
// sm:ignore:next rule1
361+
let b = 456
362+
let c = 789
363+
"""
364+
365+
let (mask, converter) = createMask(sourceText: text)
366+
367+
#expect(mask.ruleState("rule1", at: location(ofLine: 1, in: converter)) == .default)
368+
#expect(mask.ruleState("rule1", at: location(ofLine: 3, in: converter)) == .disabled)
369+
#expect(mask.ruleState("rule1", at: location(ofLine: 4, in: converter)) == .default)
370+
}
371+
372+
@Test func bareNextDirectiveAppliesOnlyToNextStatement() {
373+
let text =
374+
"""
375+
let a = 123
376+
// sm:ignore:next
377+
let b = 456
378+
let c = 789
379+
"""
380+
381+
let (mask, converter) = createMask(sourceText: text)
382+
383+
#expect(mask.ruleState("anyRule", at: location(ofLine: 1, in: converter)) == .default)
384+
#expect(mask.ruleState("anyRule", at: location(ofLine: 3, in: converter)) == .disabled)
385+
#expect(mask.ruleState("anyRule", at: location(ofLine: 4, in: converter)) == .default)
386+
}
387+
388+
@Test func nextDirectiveAppliesToWholeMultiLineStatement() {
389+
let text =
390+
"""
391+
let a = 0
392+
// sm:ignore:next rule1
393+
if !items.contains(p) {
394+
items.append(p)
395+
}
396+
let z = 0
397+
"""
398+
399+
let (mask, converter) = createMask(sourceText: text)
400+
401+
#expect(mask.ruleState("rule1", at: location(ofLine: 1, in: converter)) == .default)
402+
#expect(mask.ruleState("rule1", at: location(ofLine: 3, in: converter)) == .disabled)
403+
#expect(mask.ruleState("rule1", at: location(ofLine: 4, in: converter)) == .disabled)
404+
#expect(mask.ruleState("rule1", at: location(ofLine: 5, in: converter)) == .disabled)
405+
#expect(mask.ruleState("rule1", at: location(ofLine: 6, in: converter)) == .default)
406+
}
407+
408+
@Test func nextDirectiveOnMember() {
409+
let text =
410+
"""
411+
struct Foo {
412+
// sm:ignore:next rule1
413+
var bar = 0
414+
var baz = 0
415+
}
416+
"""
417+
418+
let (mask, converter) = createMask(sourceText: text)
419+
420+
#expect(mask.ruleState("rule1", at: location(ofLine: 3, column: 3, in: converter)) == .disabled)
421+
#expect(mask.ruleState("rule1", at: location(ofLine: 4, column: 3, in: converter)) == .default)
422+
}
423+
354424
// MARK: - Top-of-file named directive: file-level scope.
355425

356426
@Test func namedDirectiveAtTopOfFileIsFileScoped() {

Tests/SwiftiomaticTests/Rules/Wrap/WrapTernaryBranchesTests.swift

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,31 @@ struct WrapTernaryBranchesTests: RuleTesting {
107107
/// but whose `?` and `:` both sit on a short final line — the rule must not fire.
108108
/// Repros from issue: `[wrapTernaryBranches] wrap ternary branch onto a new line` warning
109109
/// emitted at `) != nil ? valueRange : nil` even though that line easily fits.
110+
/// Bug repro: a ternary whose condition is `a || b.c` where the `||` has wrapped, putting
111+
/// `?` and `:` on the same short line as `.isEmpty ? .top(...) : .all(...)`. Both operators
112+
/// live on a line that fits, so the rule must not fire.
113+
@Test func ternaryWithMultiLineDisjunctionConditionDoesNotWrapWhenOperatorLineFits() {
114+
assertFormatting(
115+
WrapTernaryBranches.self,
116+
input: """
117+
view
118+
.fieldStyle(
119+
corners: isValidQuery || !matches
120+
.isEmpty ? .top(cornerRadius + 2) : .all(cornerRadius + 2),
121+
isFocused: isFocused,
122+
)
123+
""",
124+
expected: """
125+
view
126+
.fieldStyle(
127+
corners: isValidQuery || !matches
128+
.isEmpty ? .top(cornerRadius + 2) : .all(cornerRadius + 2),
129+
isFocused: isFocused,
130+
)
131+
""",
132+
findings: [])
133+
}
134+
110135
@Test func ternaryAfterMultiLineConditionDoesNotWrapWhenOperatorLineFits() {
111136
assertFormatting(
112137
WrapTernaryBranches.self,

0 commit comments

Comments
 (0)