Skip to content

Commit 77a46d5

Browse files
authored
Fix an incorrect equality implementation for ResolvedTopicReference (#1140)
rdar://130698358
1 parent 0675ac7 commit 77a46d5

File tree

2 files changed

+32
-12
lines changed

2 files changed

+32
-12
lines changed

Sources/SwiftDocC/Model/Identifier.swift

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -441,30 +441,22 @@ public struct ResolvedTopicReference: Hashable, Codable, Equatable, CustomString
441441
return url.absoluteString
442442
}
443443

444-
// Note: The source language of a `ResolvedTopicReference` is not considered when
445-
// hashing and checking for equality. This is intentional as DocC uses a single
446-
// ResolvedTopicReference to refer to all source language variants of a topic.
447-
//
448-
// This allows clients to look up topic references without knowing ahead of time
449-
// which languages they are available in.
450-
451444
public func hash(into hasher: inout Hasher) {
452-
hasher.combine(_storage.identifierPathAndFragment)
445+
hasher.combine(_storage)
453446
}
454447

455448
public static func == (lhs: ResolvedTopicReference, rhs: ResolvedTopicReference) -> Bool {
456-
return lhs._storage.identifierPathAndFragment == rhs._storage.identifierPathAndFragment
449+
return lhs._storage == rhs._storage
457450
}
458451

459452
/// Storage for a resolved topic reference's state.
460453
///
461454
/// This is a reference type which allows ``ResolvedTopicReference`` to have copy-on-write behavior.
462-
class Storage {
455+
class Storage: Hashable {
463456
let bundleID: DocumentationBundle.Identifier
464457
let path: String
465458
let fragment: String?
466459
let sourceLanguages: Set<SourceLanguage>
467-
let identifierPathAndFragment: String
468460

469461
let url: URL
470462

@@ -482,7 +474,6 @@ public struct ResolvedTopicReference: Hashable, Codable, Equatable, CustomString
482474
self.path = path
483475
self.fragment = fragment
484476
self.sourceLanguages = sourceLanguages
485-
self.identifierPathAndFragment = "\(bundleID)\(path)\(fragment ?? "")"
486477

487478
var components = URLComponents()
488479
components.scheme = ResolvedTopicReference.urlScheme
@@ -493,6 +484,28 @@ public struct ResolvedTopicReference: Hashable, Codable, Equatable, CustomString
493484
self.pathComponents = self.url.pathComponents
494485
self.absoluteString = self.url.absoluteString
495486
}
487+
488+
// Note: The source language of a `ResolvedTopicReference` is not considered when
489+
// hashing and checking for equality. This is intentional as DocC uses a single
490+
// ResolvedTopicReference to refer to all source language variants of a topic.
491+
//
492+
// This allows clients to look up topic references without knowing ahead of time
493+
// which languages they are available in.
494+
495+
func hash(into hasher: inout Hasher) {
496+
hasher.combine(path)
497+
hasher.combine(fragment)
498+
// Ignore the bundle ID in the hash even if it's used for equality (below).
499+
// _Almost_ all content should be local, with the same bundle ID, so hashing it doesn't contribute to meaningful uniqueness
500+
// and since the module name is already included in the path, paths very rarely collide across archives (where the bundle ID would help).
501+
}
502+
503+
static func == (lhs: Storage, rhs: Storage) -> Bool {
504+
lhs.path == rhs.path &&
505+
lhs.fragment == rhs.fragment &&
506+
lhs.bundleID == rhs.bundleID // Compare bundle ID last since it's expected to be the same for _almost_ all content in the build.
507+
508+
}
496509
}
497510

498511
// For testing the caching

Tests/SwiftDocCTests/Infrastructure/ResolvedTopicReferenceTests.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,11 @@ class ResolvedTopicReferenceTests: XCTestCase {
8383
_ = topicReference.absoluteString
8484
}
8585
}
86+
87+
func testDifferentReferencesAreNotEqual() {
88+
XCTAssertNotEqual(
89+
ResolvedTopicReference(bundleID: "com.example", path: "/OneTwo", fragment: nil, sourceLanguage: .swift),
90+
ResolvedTopicReference(bundleID: "com.example", path: "/One", fragment: "Two", sourceLanguage: .swift)
91+
)
92+
}
8693
}

0 commit comments

Comments
 (0)