Skip to content

Commit de5f7ed

Browse files
authored
Merge pull request #4635 from jmschonfeld/empty-run-equality-crash
Fix logic error in AttributedString.Runs.== for empty collections
2 parents a9bf8d5 + f9aade0 commit de5f7ed

File tree

2 files changed

+37
-6
lines changed

2 files changed

+37
-6
lines changed

Sources/Foundation/AttributedString/AttributedString.swift

+23-6
Original file line numberDiff line numberDiff line change
@@ -974,18 +974,35 @@ public struct AttributedString : AttributedStringProtocol {
974974
public static func == (lhs: Runs, rhs: Runs) -> Bool {
975975
let lhsSlice = lhs._guts.runs[lhs._startingRunIndex ..< lhs._endingRunIndex]
976976
let rhsSlice = rhs._guts.runs[rhs._startingRunIndex ..< rhs._endingRunIndex]
977-
guard lhsSlice.count == rhs.count else {
977+
978+
// If there are different numbers of runs, they aren't equal
979+
guard lhsSlice.count == rhsSlice.count else {
978980
return false
979981
}
980-
// Compare all inner runs (their lengths will not be limited by _range)
981-
if lhsSlice.count > 2 && !lhsSlice[lhsSlice.startIndex + 1 ..< lhsSlice.endIndex - 1].elementsEqual(rhsSlice[rhsSlice.startIndex + 1 ..< rhsSlice.endIndex - 1]) {
982+
983+
let runCount = lhsSlice.count
984+
985+
// Empty slices are always equal
986+
guard runCount > 0 else {
987+
return true
988+
}
989+
990+
// Compare the first run (clamping their ranges) since we know each has at least one run
991+
if lhs._guts.run(at: lhs.startIndex, clampedBy: lhs._range) != rhs._guts.run(at: rhs.startIndex, clampedBy: rhs._range) {
982992
return false
983993
}
984-
// If the inner runs are equivalent, check the first and last runs with clamped ranges
985-
if lhsSlice.count > 1 && lhs._guts.run(at: Index(rangeIndex: lhs._endingRunIndex - 1), clampedBy: lhs._range) != rhs._guts.run(at: Index(rangeIndex: rhs._endingRunIndex - 1), clampedBy: rhs._range) {
994+
995+
// Compare all inner runs if they exist without needing to clamp ranges
996+
if runCount > 2 && !lhsSlice[lhsSlice.startIndex + 1 ..< lhsSlice.endIndex - 1].elementsEqual(rhsSlice[rhsSlice.startIndex + 1 ..< rhsSlice.endIndex - 1]) {
986997
return false
987998
}
988-
return lhs._guts.run(at: lhs.startIndex, clampedBy: lhs._range) == rhs._guts.run(at: rhs.startIndex, clampedBy: rhs._range)
999+
1000+
// If there are more than one run (so we didn't already check this as the first run), check the last run (clamping its range)
1001+
if runCount > 1 && lhs._guts.run(at: Index(rangeIndex: lhs._endingRunIndex - 1), clampedBy: lhs._range) != rhs._guts.run(at: Index(rangeIndex: rhs._endingRunIndex - 1), clampedBy: rhs._range) {
1002+
return false
1003+
}
1004+
1005+
return true
9891006
}
9901007

9911008
public var description: String {

Tests/Foundation/Tests/TestAttributedString.swift

+14
Original file line numberDiff line numberDiff line change
@@ -989,6 +989,19 @@ E {
989989
XCTAssertFalse(desc.isEmpty)
990990
}
991991
}
992+
993+
func testSubstringEquality() {
994+
let str = AttributedString("")
995+
let range = str.startIndex ..< str.endIndex
996+
XCTAssertEqual(str[range], str[range])
997+
998+
let str2 = "A" + AttributedString("A", attributes: .init().testInt(2))
999+
let substringA = str2[str2.startIndex ..< str2.index(afterCharacter: str2.startIndex)]
1000+
let substringB = str2[str2.index(afterCharacter: str2.startIndex) ..< str2.endIndex]
1001+
XCTAssertNotEqual(substringA, substringB)
1002+
XCTAssertEqual(substringA, substringA)
1003+
XCTAssertEqual(substringB, substringB)
1004+
}
9921005

9931006
// MARK: - Coding Tests
9941007

@@ -2196,6 +2209,7 @@ E {
21962209
("testSubstringBase", testSubstringBase),
21972210
("testSubstringGetAttribute", testSubstringGetAttribute),
21982211
("testSubstringDescription", testSubstringDescription),
2212+
("testSubstringEquality", testSubstringEquality),
21992213
("testJSONEncoding", testJSONEncoding),
22002214
("testDecodingThenConvertingToNSAttributedString", testDecodingThenConvertingToNSAttributedString),
22012215
("testCustomAttributeCoding", testCustomAttributeCoding),

0 commit comments

Comments
 (0)