Skip to content

Commit 1da2a48

Browse files
committed
fix SimplifyGenericConstraints diagnostic location; preserve link reference definitions in ReflowComments
1 parent 2aa550f commit 1da2a48

10 files changed

Lines changed: 338 additions & 17 deletions
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
---
2+
# 83k-hv9
3+
title: wrapTernaryBranches fires on ternary that fits on one line
4+
status: completed
5+
type: bug
6+
priority: normal
7+
created_at: 2026-05-01T21:49:13Z
8+
updated_at: 2026-05-01T21:54:32Z
9+
sync:
10+
github:
11+
issue_number: "614"
12+
synced_at: "2026-05-01T23:12:04Z"
13+
---
14+
15+
## Repro
16+
17+
```swift
18+
var urlEncoded: String {
19+
isEmpty ? "" : "?"
20+
+ map { key, value in "\(key)=\(value.description.urlEncoded)" }
21+
.joined(separator: "&")
22+
}
23+
```
24+
25+
Xcode shows a `wrapTernaryBranches` warning on the `isEmpty ? "" : "?"` line: "wrap ternary branch onto a new line". The ternary itself (`isEmpty ? "" : "?"`) fits comfortably on one line — the rule should not fire.
26+
27+
The likely confusion is that the ternary is the **left operand** of a `+` expression whose right operand is a multi-line chain. The rule may be looking at the enclosing expression's wrapped state rather than the ternary expression itself, or measuring the ternary's containing line including the trailing `+ map { … }` continuation.
28+
29+
## Expected
30+
31+
`wrapTernaryBranches` should only fire when the ternary expression's *own* operands (condition / then / else) cause it to overflow the print width on a single line. `isEmpty ? "" : "?"` is well under width and should be left alone.
32+
33+
## Files
34+
35+
- `Sources/SwiftiomaticKit/Rules/Wrap/WrapTernaryBranches.swift`
36+
- Look at how the rule measures fit and whether it considers the ternary in isolation or as part of the enclosing infix expression.
37+
38+
## Test plan
39+
40+
- Add a fixture with a ternary used as one operand of a multi-line infix expression where the ternary itself is short — assert no finding.
41+
- Existing wrap-when-overflowing case stays green.
42+
43+
44+
45+
## Summary of Changes
46+
47+
- `Sources/SwiftiomaticKit/Rules/Wrap/WrapTernaryBranches.swift` — added a short-circuit before the collapsed-length calculation: if the source lines that already host `?` and `:` both fit within `lineLength`, skip the wrap. New helper `sourceLineLength(at:converter:)` measures actual source-line length via `SourceLocationConverter.position(ofLine:column:)`.
48+
- `Tests/SwiftiomaticTests/Rules/Wrap/WrapTernaryBranchesTests.swift` — added `ternaryWithMultiLineRHSOperandNotWrapped` covering the user's exact `isEmpty ? "" : "?" + map { … }.joined(...)` pattern.
49+
50+
## Root cause
51+
52+
Ternary `?:` precedence is lower than `+` in Swift, so `cond ? a : b + chain` parses as `cond ? a : (b + chain)`. When the `+ chain` part was a multi-line method chain, the ternary's `else` branch span included all of it, and `singleLineLength(of:)` (which collapses internal whitespace) measured the whole ternary as a single huge string that overflowed `lineLength`. The fix bypasses that calculation when both operator lines already fit in source — the operands' wrapping is independent and not the ternary's concern.
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
# slg-5gh
3+
title: SimplifyGenericConstraints diagnostics anchor to wrong source line
4+
status: completed
5+
type: bug
6+
priority: normal
7+
created_at: 2026-05-01T21:58:31Z
8+
updated_at: 2026-05-01T22:14:40Z
9+
sync:
10+
github:
11+
issue_number: "615"
12+
synced_at: "2026-05-01T23:12:04Z"
13+
---
14+
15+
The rule's `transform` extracts the where-clause `conformance` node from `visited` (the rewritten subtree) and calls `Self.diagnose(on: conformance, ...)`. The static `emitFinding` helper resolves the source location via `node.startLocation(converter: context.sourceLocationConverter)` — but the converter is built from the original source while `conformance`'s absolute byte offset reflects its position inside the detached rewritten subtree. Result: when a triggering decl is preceded by other source content, the finding lands on whatever original-source line shares that byte offset (typealiases, doc comments, URLs above the decl).
16+
17+
## Reproduction
18+
19+
A file like:
20+
21+
```
22+
typealias MoveRowHandler = (IndexSet, Int) -> Void
23+
24+
/// doc
25+
/// comment
26+
struct OutlineList<Data, RowContent>: View where Data: RecursiveCollection, RowContent: View { ... }
27+
```
28+
29+
…produces `simplifyGenericConstraints` warnings on the typealias and doc-comment lines instead of on the `where` clause.
30+
31+
## Tasks
32+
33+
- [x] Add a failing test that places the triggering struct after preceding content and asserts the finding line/column lands on the original `where` clause
34+
- [x] Fix `SimplifyGenericConstraints.simplifyConstraints` to diagnose against the original (not the rewritten) node
35+
- [x] Re-run the SimplifyGenericConstraints test suite
36+
37+
38+
39+
## Summary of Changes
40+
41+
`Sources/SwiftiomaticKit/Rules/Generics/SimplifyGenericConstraints.swift`: thread the `original` decl through `simplifyConstraints`, look up the matching `GenericRequirementSyntax` in the original where clause by the same enumeration index, and pass that attached node as the diagnose anchor. `original` carries correct positions because it is the node passed to the parent rewriter's `visit(_:)` while the tree is still attached, whereas `visited` (the result of `super.visit(node)`) is a detached subtree whose `position` is offset relative to its own root.
42+
43+
Verified end-to-end against `OutlineList.swift` (the user's repro). Before: warnings landed on lines 4, 6, 22. After: warnings land on lines 23 and 79, correctly anchored to each `where` clause. Full test suite passes (3161 tests).
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
---
2+
# t8p-jfj
3+
title: Doc-comment reflow merges Markdown footnote definitions into prose
4+
status: completed
5+
type: bug
6+
priority: normal
7+
created_at: 2026-05-01T22:14:55Z
8+
updated_at: 2026-05-01T23:10:52Z
9+
sync:
10+
github:
11+
issue_number: "613"
12+
synced_at: "2026-05-01T23:12:04Z"
13+
---
14+
15+
The doc-comment reflow rule treats a footnote/link reference definition as a regular continuation paragraph and reflows it onto the previous line, breaking the Markdown.
16+
17+
## Reproduction
18+
19+
Input:
20+
21+
```
22+
/// Text view for [iOS][uiv] or [MacOS][nsv]
23+
///
24+
/// [uiv]: https://developer.apple.com/documentation/uikit/uitextview
25+
/// [nsv]: https://developer.apple.com/documentation/appkit/nstextview
26+
```
27+
28+
Output:
29+
30+
```
31+
/// Text view for [iOS][uiv] or [MacOS][nsv]
32+
///
33+
/// [uiv]: https://developer.apple.com/documentation/uikit/uitextview [nsv]:
34+
/// https://developer.apple.com/documentation/appkit/nstextview
35+
```
36+
37+
The reflow joins the two `[name]: url` definitions into a single line, which makes them invalid Markdown link references. Footnote/reference definitions of the form `[label]: url` (with optional title) must remain on their own line.
38+
39+
## Tasks
40+
41+
- [x] Find the doc-comment reflow rule and locate where paragraph wrapping decides what counts as a continuation line
42+
- [x] Add a failing test that exercises the `[label]: url` line pattern across two adjacent definition lines and asserts no reflow
43+
- [x] Treat `^\s*\[[^\]]+\]:\s` (CommonMark link/reference definition) as a hard break — never merge into the previous or next line, never wrap mid-URL
44+
- [x] Re-run the reflow rule's test suite and full suite
45+
46+
47+
48+
## Summary of Changes
49+
50+
`Sources/SwiftiomaticKit/Rules/Comments/CommentReflowEngine.swift`: added `isLinkReferenceDefinition(_:)` that recognizes a CommonMark link reference definition (optional 0–3 leading spaces, `[label]: dest`). `parseBlocks` now (a) emits such a line as a `.verbatim` block at the top-level dispatch, and (b) breaks paragraph collection when one is encountered, so adjacent definitions can never be folded into a single paragraph and reflowed.
51+
52+
Added regression test `preservesAdjacentLinkReferenceDefinitions` in `Tests/SwiftiomaticTests/Rules/ReflowCommentsTests.swift` covering the exact reproduction (two `[uiv]:` / `[nsv]:` lines after a paragraph). Full suite passes (3162 tests).

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@
4343

4444
### 🐞 Fixes
4545

46+
- `SimplifyGenericConstraints`; anchor diagnostics to the original `where` clause instead of the rewritten subtree's detached position ([#615](https://github.com/toba/swiftiomatic/issues/615))
47+
- `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))
48+
- `ReflowComments`; preserve CommonMark link reference definitions (`[label]: url`) instead of merging adjacent definitions into one paragraph ([#613](https://github.com/toba/swiftiomatic/issues/613))
4649
- `wrapTernary`; skip ternaries inside single-line string interpolations to avoid producing invalid Swift ([#602](https://github.com/toba/swiftiomatic/issues/602))
4750
- `PreferIfElseChain`; rewrite guard-return + trailing return to `if/else` expression in implicit-return positions ([#563](https://github.com/toba/swiftiomatic/issues/563))
4851
- `extension` where-clause; apply continuation indent when `where` wraps to its own line

Sources/SwiftiomaticKit/Rules/Comments/CommentReflowEngine.swift

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,13 @@ package enum CommentReflowEngine {
137137
i += 1
138138
continue
139139
}
140+
// CommonMark link reference definition (`[label]: url`). Preserve verbatim — wrapping
141+
// the URL or merging adjacent definitions onto one line breaks the Markdown.
142+
if isLinkReferenceDefinition(line) {
143+
blocks.append(.verbatim(line: line))
144+
i += 1
145+
continue
146+
}
140147
// Block quote: contiguous lines starting with ">".
141148
if line.hasPrefix(">") {
142149
var quoted: [String] = []
@@ -167,6 +174,7 @@ package enum CommentReflowEngine {
167174
if l.hasPrefix(">") { break }
168175
if listMarker(l) != nil { break }
169176
if fenceOpener(l) != nil { break }
177+
if isLinkReferenceDefinition(l) { break }
170178
paraLines.append(l.trimmingCharacters(in: .whitespaces))
171179
i += 1
172180
}
@@ -175,6 +183,34 @@ package enum CommentReflowEngine {
175183
return blocks
176184
}
177185

186+
/// Returns `true` if `line` is a CommonMark link reference definition of the form
187+
/// `[label]: destination`, allowing up to 3 leading spaces of indent. These lines must remain
188+
/// on their own — wrapping the destination or merging an adjacent definition into the previous
189+
/// line invalidates the reference.
190+
private static func isLinkReferenceDefinition(_ line: String) -> Bool {
191+
var idx = line.startIndex
192+
var leading = 0
193+
194+
while idx < line.endIndex, line[idx] == " ", leading < 3 {
195+
idx = line.index(after: idx)
196+
leading += 1
197+
}
198+
guard idx < line.endIndex, line[idx] == "[" else { return false }
199+
let afterOpen = line.index(after: idx)
200+
guard let closeBracket = line[afterOpen...].firstIndex(of: "]") else { return false }
201+
// Label must be non-empty and contain no `[` (CommonMark forbids unescaped brackets).
202+
guard closeBracket > afterOpen,
203+
!line[afterOpen..<closeBracket].contains("[") else { return false }
204+
let afterClose = line.index(after: closeBracket)
205+
guard afterClose < line.endIndex, line[afterClose] == ":" else { return false }
206+
// Must be followed by whitespace (or end of line in lazy parsers — but we require non-empty
207+
// destination on the same line, which is the common case in Swift docs).
208+
let afterColon = line.index(after: afterClose)
209+
guard afterColon < line.endIndex, line[afterColon] == " " else { return false }
210+
let dest = line[afterColon...].drop(while: { $0 == " " })
211+
return !dest.isEmpty
212+
}
213+
178214
/// Returns the fence string (e.g. "` ` ` " or "~~~") if ` line` opens a fenced code block.
179215
private static func fenceOpener(_ line: String) -> String? {
180216
let trimmed = line.drop(while: { $0 == " " })

Sources/SwiftiomaticKit/Rules/Generics/SimplifyGenericConstraints.swift

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@ final class SimplifyGenericConstraints: StaticFormatRule<BasicRuleValue>, @unche
1818

1919
static func transform(
2020
_ visited: FunctionDeclSyntax,
21-
original _: FunctionDeclSyntax,
21+
original: FunctionDeclSyntax,
2222
parent _: Syntax?,
2323
context: Context
2424
) -> DeclSyntax {
2525
var result = simplifyConstraints(
2626
visited,
27+
original: original,
2728
genericParamsKeyPath: \.genericParameterClause,
2829
whereClauseKeyPath: \.genericWhereClause,
2930
context: context
@@ -38,13 +39,14 @@ final class SimplifyGenericConstraints: StaticFormatRule<BasicRuleValue>, @unche
3839

3940
static func transform(
4041
_ visited: StructDeclSyntax,
41-
original _: StructDeclSyntax,
42+
original: StructDeclSyntax,
4243
parent _: Syntax?,
4344
context: Context
4445
) -> DeclSyntax {
4546
DeclSyntax(
4647
simplifyConstraints(
4748
visited,
49+
original: original,
4850
genericParamsKeyPath: \.genericParameterClause,
4951
whereClauseKeyPath: \.genericWhereClause,
5052
context: context
@@ -53,13 +55,14 @@ final class SimplifyGenericConstraints: StaticFormatRule<BasicRuleValue>, @unche
5355

5456
static func transform(
5557
_ visited: ClassDeclSyntax,
56-
original _: ClassDeclSyntax,
58+
original: ClassDeclSyntax,
5759
parent _: Syntax?,
5860
context: Context
5961
) -> DeclSyntax {
6062
DeclSyntax(
6163
simplifyConstraints(
6264
visited,
65+
original: original,
6366
genericParamsKeyPath: \.genericParameterClause,
6467
whereClauseKeyPath: \.genericWhereClause,
6568
context: context
@@ -68,13 +71,14 @@ final class SimplifyGenericConstraints: StaticFormatRule<BasicRuleValue>, @unche
6871

6972
static func transform(
7073
_ visited: EnumDeclSyntax,
71-
original _: EnumDeclSyntax,
74+
original: EnumDeclSyntax,
7275
parent _: Syntax?,
7376
context: Context
7477
) -> DeclSyntax {
7578
DeclSyntax(
7679
simplifyConstraints(
7780
visited,
81+
original: original,
7882
genericParamsKeyPath: \.genericParameterClause,
7983
whereClauseKeyPath: \.genericWhereClause,
8084
context: context
@@ -83,13 +87,14 @@ final class SimplifyGenericConstraints: StaticFormatRule<BasicRuleValue>, @unche
8387

8488
static func transform(
8589
_ visited: ActorDeclSyntax,
86-
original _: ActorDeclSyntax,
90+
original: ActorDeclSyntax,
8791
parent _: Syntax?,
8892
context: Context
8993
) -> DeclSyntax {
9094
DeclSyntax(
9195
simplifyConstraints(
9296
visited,
97+
original: original,
9398
genericParamsKeyPath: \.genericParameterClause,
9499
whereClauseKeyPath: \.genericWhereClause,
95100
context: context
@@ -98,6 +103,7 @@ final class SimplifyGenericConstraints: StaticFormatRule<BasicRuleValue>, @unche
98103

99104
private static func simplifyConstraints<D>(
100105
_ decl: D,
106+
original: D,
101107
genericParamsKeyPath: WritableKeyPath<D, GenericParameterClauseSyntax?>,
102108
whereClauseKeyPath: WritableKeyPath<D, GenericWhereClauseSyntax?>,
103109
context: Context
@@ -113,7 +119,12 @@ final class SimplifyGenericConstraints: StaticFormatRule<BasicRuleValue>, @unche
113119
.map { $0.name.text }
114120
)
115121

116-
// Identify constraints to inline
122+
// Identify constraints to inline. The diagnostic is anchored to the *original* (attached)
123+
// conformance node so its source location maps through `context.sourceLocationConverter`
124+
// (which is built from the original tree) to the correct line/column. Anchoring on the
125+
// visited (rewritten) subtree gives a position relative to the detached subtree's root,
126+
// which floats up to whatever earlier source line shares that byte offset.
127+
let originalRequirements: [GenericRequirementSyntax] = (original[keyPath: whereClauseKeyPath]?.requirements).map(Array.init) ?? []
117128
var consumedIndices: Set<Int> = []
118129
var inlineMap: [String: TypeSyntax] = [:]
119130

@@ -129,9 +140,12 @@ final class SimplifyGenericConstraints: StaticFormatRule<BasicRuleValue>, @unche
129140
inlineMap[leftIdent.name.text] = conformance.rightType
130141
consumedIndices.insert(index)
131142

143+
let anchor = originalRequirements.indices.contains(index)
144+
? Syntax(originalRequirements[index].requirement)
145+
: Syntax(conformance)
132146
Self.diagnose(
133147
.simplifyGenericConstraint(param: leftIdent.name.text),
134-
on: conformance,
148+
on: anchor,
135149
context: context
136150
)
137151
}

0 commit comments

Comments
 (0)