Skip to content

Commit 8902856

Browse files
enforce overload declaration ordering when computing the highlighted differences (#1250)
rdar://155975575
1 parent 65aaf92 commit 8902856

File tree

2 files changed

+104
-3
lines changed

2 files changed

+104
-3
lines changed

Sources/SwiftDocC/Model/Rendering/RenderSectionTranslator/DeclarationsSectionTranslator.swift

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,17 @@ struct DeclarationsSectionTranslator: RenderSectionTranslator {
4444
/// Fetch the common fragments for the given references, or compute it if necessary.
4545
func commonFragments(
4646
for mainDeclaration: OverloadDeclaration,
47-
overloadDeclarations: [OverloadDeclaration]
47+
overloadDeclarations: [OverloadDeclaration],
48+
mainDeclarationIndex: Int
4849
) -> [SymbolGraph.Symbol.DeclarationFragments.Fragment] {
4950
if let fragments = commonFragments(for: mainDeclaration.reference) {
5051
return fragments
5152
}
5253

53-
let preProcessedDeclarations = [mainDeclaration.declaration] + overloadDeclarations.map(\.declaration)
54+
var preProcessedDeclarations = overloadDeclarations.map(\.declaration)
55+
// Insert the main declaration according to the display index so the ordering is consistent
56+
// between overloaded symbols
57+
preProcessedDeclarations.insert(mainDeclaration.declaration, at: mainDeclarationIndex)
5458

5559
// Collect the "common fragments" so we can highlight the ones that are different
5660
// in each declaration
@@ -216,7 +220,9 @@ struct DeclarationsSectionTranslator: RenderSectionTranslator {
216220
// in each declaration
217221
let commonFragments = commonFragments(
218222
for: (mainDeclaration, renderNode.identifier, nil),
219-
overloadDeclarations: processedOverloadDeclarations)
223+
overloadDeclarations: processedOverloadDeclarations,
224+
mainDeclarationIndex: overloads.displayIndex
225+
)
220226

221227
renderedTokens = translateDeclaration(
222228
mainDeclaration,

Tests/SwiftDocCTests/Rendering/DeclarationsRenderSectionTests.swift

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import Foundation
1212
import XCTest
1313
@testable import SwiftDocC
1414
import SwiftDocCTestUtilities
15+
import SymbolKit
1516

1617
class DeclarationsRenderSectionTests: XCTestCase {
1718
func testDecodingTokens() throws {
@@ -322,6 +323,91 @@ class DeclarationsRenderSectionTests: XCTestCase {
322323
}
323324
}
324325

326+
func testInconsistentHighlightDiff() throws {
327+
enableFeatureFlag(\.isExperimentalOverloadedSymbolPresentationEnabled)
328+
329+
// Generate a symbol graph with many overload groups that share declarations.
330+
// The overloaded declarations have two legitimate solutions for their longest common subsequence:
331+
// one that ends in a close-parenthesis, and one that ends in a space.
332+
// By alternating the order in which these declarations appear,
333+
// the computed difference highlighting can differ
334+
// unless the declarations are sorted prior to the calculation.
335+
// Ensure that the overload difference highlighting is consistent for these declarations.
336+
337+
// init(_ content: MyClass) throws
338+
let declaration1: SymbolGraph.Symbol.DeclarationFragments = .init(declarationFragments: [
339+
.init(kind: .keyword, spelling: "init", preciseIdentifier: nil),
340+
.init(kind: .text, spelling: "(", preciseIdentifier: nil),
341+
.init(kind: .externalParameter, spelling: "_", preciseIdentifier: nil),
342+
.init(kind: .text, spelling: " ", preciseIdentifier: nil),
343+
.init(kind: .internalParameter, spelling: "content", preciseIdentifier: nil),
344+
.init(kind: .text, spelling: ": ", preciseIdentifier: nil),
345+
.init(kind: .typeIdentifier, spelling: "MyClass", preciseIdentifier: "s:MyClass"),
346+
.init(kind: .text, spelling: ") ", preciseIdentifier: nil),
347+
.init(kind: .keyword, spelling: "throws", preciseIdentifier: nil),
348+
])
349+
350+
// init(_ content: some ConvertibleToMyClass)
351+
let declaration2: SymbolGraph.Symbol.DeclarationFragments = .init(declarationFragments: [
352+
.init(kind: .keyword, spelling: "init", preciseIdentifier: nil),
353+
.init(kind: .text, spelling: "(", preciseIdentifier: nil),
354+
.init(kind: .externalParameter, spelling: "_", preciseIdentifier: nil),
355+
.init(kind: .text, spelling: " ", preciseIdentifier: nil),
356+
.init(kind: .internalParameter, spelling: "content", preciseIdentifier: nil),
357+
.init(kind: .text, spelling: ": ", preciseIdentifier: nil),
358+
.init(kind: .keyword, spelling: "some", preciseIdentifier: nil),
359+
.init(kind: .text, spelling: " ", preciseIdentifier: nil),
360+
.init(kind: .typeIdentifier, spelling: "ConvertibleToMyClass", preciseIdentifier: "s:ConvertibleToMyClass"),
361+
.init(kind: .text, spelling: ")", preciseIdentifier: nil),
362+
])
363+
let overloadsCount = 10
364+
let symbols = (0...overloadsCount).flatMap({ index in
365+
let reverseDeclarations = index % 2 != 0
366+
return [
367+
makeSymbol(
368+
id: "overload-\(index)-1",
369+
kind: .func,
370+
pathComponents: ["overload-\(index)"],
371+
otherMixins: [reverseDeclarations ? declaration2 : declaration1]),
372+
makeSymbol(
373+
id: "overload-\(index)-2",
374+
kind: .func,
375+
pathComponents: ["overload-\(index)"],
376+
otherMixins: [reverseDeclarations ? declaration1 : declaration2]),
377+
]
378+
})
379+
let symbolGraph = makeSymbolGraph(moduleName: "FancierOverloads", symbols: symbols)
380+
381+
let catalog = Folder(name: "unit-test.docc", content: [
382+
InfoPlist(displayName: "FancierOverloads", identifier: "com.test.example"),
383+
JSONFile(name: "FancierOverloads.symbols.json", content: symbolGraph),
384+
])
385+
386+
let (bundle, context) = try loadBundle(catalog: catalog)
387+
388+
func assertDeclarations(for USR: String, file: StaticString = #filePath, line: UInt = #line) throws {
389+
let reference = try XCTUnwrap(context.documentationCache.reference(symbolID: USR), file: file, line: line)
390+
let symbol = try XCTUnwrap(context.entity(with: reference).semantic as? Symbol, file: file, line: line)
391+
var translator = RenderNodeTranslator(context: context, bundle: bundle, identifier: reference)
392+
let renderNode = try XCTUnwrap(translator.visitSymbol(symbol) as? RenderNode, file: file, line: line)
393+
let declarationsSection = try XCTUnwrap(renderNode.primaryContentSections.compactMap({ $0 as? DeclarationsRenderSection }).first, file: file, line: line)
394+
XCTAssertEqual(declarationsSection.declarations.count, 1, file: file, line: line)
395+
let declarations = try XCTUnwrap(declarationsSection.declarations.first, file: file, line: line)
396+
397+
XCTAssertEqual(declarationsAndHighlights(for: declarations), [
398+
"init(_ content: MyClass) throws",
399+
" ~~~~~~~~ ~~~~~~",
400+
"init(_ content: some ConvertibleToMyClass)",
401+
" ~~~~ ~~~~~~~~~~~~~~~~~~~~~",
402+
], file: file, line: line)
403+
}
404+
405+
for i in 0...overloadsCount {
406+
try assertDeclarations(for: "overload-\(i)-1")
407+
try assertDeclarations(for: "overload-\(i)-2")
408+
}
409+
}
410+
325411
func testDontHighlightWhenOverloadsAreDisabled() throws {
326412
let symbolGraphFile = Bundle.module.url(
327413
forResource: "FancyOverloads",
@@ -403,3 +489,12 @@ func declarationAndHighlights(for tokens: [DeclarationRenderSection.Token]) -> [
403489
tokens.map({ String(repeating: $0.highlight == .changed ? "~" : " ", count: $0.text.count) }).joined()
404490
]
405491
}
492+
493+
func declarationsAndHighlights(for section: DeclarationRenderSection) -> [String] {
494+
guard let otherDeclarations = section.otherDeclarations else {
495+
return []
496+
}
497+
var declarations = otherDeclarations.declarations.map(\.tokens)
498+
declarations.insert(section.tokens, at: otherDeclarations.displayIndex)
499+
return declarations.flatMap(declarationAndHighlights(for:))
500+
}

0 commit comments

Comments
 (0)