Skip to content

Commit 4fbad16

Browse files
d-ronnqvistsofiaromorales
d-ronnqvist
and
sofiaromorales
authored
Fix a bug where the diagnostics file dropped all conversion problems (#707)
* Fix a bug where the diagnostics file dropped all conversion problems rdar://115521475 * Fix a related issue where diagnostics are repeated in the console rdar://115521475 * Apply suggestions from code review Co-authored-by: Sofía Rodríguez <sofilin2000@icloud.com> * Add deprecation annotation to deprecated protocol requirement * Update tests to use NSString API instead of new String API that's only available on newer platforms --------- Co-authored-by: Sofía Rodríguez <sofilin2000@icloud.com>
1 parent 7ddabfe commit 4fbad16

File tree

7 files changed

+85
-18
lines changed

7 files changed

+85
-18
lines changed

‎Sources/SwiftDocC/Infrastructure/Diagnostics/DiagnosticConsoleWriter.swift‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,16 +60,23 @@ public final class DiagnosticConsoleWriter: DiagnosticFormattingConsumer {
6060
}
6161
}
6262

63-
public func finalize() throws {
63+
public func flush() throws {
6464
if formattingOptions.contains(.formatConsoleOutputForTools) {
6565
// For tools, the console writer writes each diagnostic as they are received.
6666
} else {
6767
let text = self.diagnosticFormatter.formattedDescription(for: problems)
6868
outputStream.write(text)
6969
}
70+
problems = [] // `flush()` is called more than once. Don't emit the same problems again.
7071
self.diagnosticFormatter.finalize()
7172
}
7273

74+
// This is deprecated but still necessary to implement.
75+
@available(*, deprecated, renamed: "flush()")
76+
public func finalize() throws {
77+
try flush()
78+
}
79+
7380
private static func makeDiagnosticFormatter(
7481
_ options: DiagnosticFormattingOptions,
7582
baseURL: URL?,

‎Sources/SwiftDocC/Infrastructure/Diagnostics/DiagnosticConsumer.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public protocol DiagnosticFormattingConsumer: DiagnosticConsumer {
3131
}
3232

3333
public extension DiagnosticConsumer {
34-
// Deprecated for supressing the warning emitted when calling `finalize()`
34+
// Deprecated for suppressing the warning emitted when calling `finalize()`
3535
@available(*, deprecated)
3636
func flush() throws {
3737
try finalize()

‎Sources/SwiftDocC/Infrastructure/Diagnostics/DiagnosticFileWriter.swift‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,16 @@ public final class DiagnosticFileWriter: DiagnosticConsumer {
2929
receivedProblems.append(contentsOf: problems)
3030
}
3131

32-
public func finalize() throws {
32+
public func flush() throws {
3333
let fileContent = DiagnosticFile(problems: receivedProblems)
34-
receivedProblems = []
34+
// Don't clear the received problems, `flush()` is called more than once.
3535
let encoder = RenderJSONEncoder.makeEncoder(emitVariantOverrides: false)
3636
try encoder.encode(fileContent).write(to: outputPath, options: .atomic)
3737
}
38+
39+
// This is deprecated but still necessary to implement.
40+
@available(*, deprecated, renamed: "flush()")
41+
public func finalize() throws {
42+
try flush()
43+
}
3844
}

‎Tests/SwiftDocCTests/Diagnostics/DiagnosticConsoleWriterDefaultFormattingTest.swift‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class DiagnosticConsoleWriterDefaultFormattingTest: XCTestCase {
3636
let diagnostic = Diagnostic(source: source, severity: .error, range: range, identifier: identifier, summary: summary, explanation: explanation)
3737
let problem = Problem(diagnostic: diagnostic, possibleSolutions: [])
3838
consumer.receive([problem])
39-
try? consumer.finalize()
39+
try? consumer.flush()
4040
XCTAssertEqual(logger.output, """
4141
\u{001B}[1;31merror: \(summary)\u{001B}[0;0m
4242
\(explanation)
@@ -50,7 +50,7 @@ class DiagnosticConsoleWriterDefaultFormattingTest: XCTestCase {
5050
let diagnostic = Diagnostic(source: source, severity: .warning, range: range, identifier: identifier, summary: summary, explanation: explanation)
5151
let problem = Problem(diagnostic: diagnostic, possibleSolutions: [])
5252
consumer.receive([problem])
53-
try? consumer.finalize()
53+
try? consumer.flush()
5454
XCTAssertEqual(logger.output, """
5555
\u{001B}[1;33mwarning: \(summary)\u{001B}[0;0m
5656
\(explanation)
@@ -64,7 +64,7 @@ class DiagnosticConsoleWriterDefaultFormattingTest: XCTestCase {
6464
let diagnostic = Diagnostic(source: source, severity: .hint, range: range, identifier: identifier, summary: summary, explanation: explanation)
6565
let problem = Problem(diagnostic: diagnostic, possibleSolutions: [])
6666
consumer.receive([problem])
67-
try? consumer.finalize()
67+
try? consumer.flush()
6868
XCTAssertEqual(logger.output, """
6969
\u{001B}[1;39mnotice: \(summary)\u{001B}[0;0m
7070
\(explanation)
@@ -78,7 +78,7 @@ class DiagnosticConsoleWriterDefaultFormattingTest: XCTestCase {
7878
let diagnostic = Diagnostic(source: source, severity: .information, range: range, identifier: identifier, summary: summary, explanation: explanation)
7979
let problem = Problem(diagnostic: diagnostic, possibleSolutions: [])
8080
consumer.receive([problem])
81-
try? consumer.finalize()
81+
try? consumer.flush()
8282
XCTAssertEqual(logger.output, """
8383
\u{001B}[1;39mnote: \(summary)\u{001B}[0;0m
8484
\(explanation)
@@ -100,7 +100,7 @@ class DiagnosticConsoleWriterDefaultFormattingTest: XCTestCase {
100100
let diagnostic = Diagnostic(source: source, severity: .warning, range: range, identifier: identifier, summary: summary, explanation: explanation)
101101
let problem = Problem(diagnostic: diagnostic, possibleSolutions: [])
102102
consumer.receive([problem])
103-
try? consumer.finalize()
103+
try? consumer.flush()
104104
XCTAssertEqual(logger.output, """
105105
\u{001B}[1;33mwarning: \(summary)\u{001B}[0;0m
106106
\(explanation)
@@ -132,7 +132,7 @@ class DiagnosticConsoleWriterDefaultFormattingTest: XCTestCase {
132132
)
133133
let problem = Problem(diagnostic: diagnostic, possibleSolutions: [])
134134
consumer.receive([problem])
135-
try? consumer.finalize()
135+
try? consumer.flush()
136136

137137
XCTAssertEqual(logger.output, """
138138
\u{001B}[1;33mwarning: \(summary)\u{001B}[0;0m
@@ -200,7 +200,7 @@ class DiagnosticConsoleWriterDefaultFormattingTest: XCTestCase {
200200
let consumer = DiagnosticConsoleWriter(logger, highlight: true)
201201

202202
consumer.receive([firstProblem, secondProblem, thirdProblem])
203-
try? consumer.finalize()
203+
try? consumer.flush()
204204
XCTAssertEqual(logger.output, """
205205
\u{001B}[1;33mwarning: First diagnostic summary\u{001B}[0;0m
206206
First diagnostic explanation
@@ -231,7 +231,7 @@ class DiagnosticConsoleWriterDefaultFormattingTest: XCTestCase {
231231
let diagnostic = Diagnostic(source: source, severity: .warning, range: range, identifier: identifier, summary: summary, explanation: explanation)
232232
let problem = Problem(diagnostic: diagnostic, possibleSolutions: [])
233233
consumer.receive([problem])
234-
try? consumer.finalize()
234+
try? consumer.flush()
235235
print(logger.output)
236236
XCTAssertEqual(logger.output, """
237237
\u{001B}[1;33mwarning: \(summary)\u{001B}[0;0m
@@ -260,7 +260,7 @@ class DiagnosticConsoleWriterDefaultFormattingTest: XCTestCase {
260260
let diagnostic = Diagnostic(source: source, severity: .warning, range: range, identifier: identifier, summary: summary, explanation: explanation)
261261
let problem = Problem(diagnostic: diagnostic, possibleSolutions: [])
262262
consumer.receive([problem])
263-
try? consumer.finalize()
263+
try? consumer.flush()
264264
print(logger.output)
265265
XCTAssertEqual(logger.output, """
266266
\u{001B}[1;33mwarning: \(summary)\u{001B}[0;0m
@@ -302,7 +302,7 @@ class DiagnosticConsoleWriterDefaultFormattingTest: XCTestCase {
302302

303303
let problem = Problem(diagnostic: diagnostic, possibleSolutions: [solution, otherSolution])
304304
consumer.receive([problem])
305-
try? consumer.finalize()
305+
try? consumer.flush()
306306

307307
print(logger.output)
308308
XCTAssertEqual(logger.output, """
@@ -327,7 +327,7 @@ class DiagnosticConsoleWriterDefaultFormattingTest: XCTestCase {
327327

328328
let problem = Problem(diagnostic: diagnostic, possibleSolutions: [solution])
329329
consumer.receive([problem])
330-
try? consumer.finalize()
330+
try? consumer.flush()
331331

332332
print(logger.output)
333333
XCTAssertEqual(logger.output, """
@@ -359,7 +359,7 @@ class DiagnosticConsoleWriterDefaultFormattingTest: XCTestCase {
359359

360360
let problem = Problem(diagnostic: diagnostic, possibleSolutions: [solution])
361361
consumer.receive([problem])
362-
try? consumer.finalize()
362+
try? consumer.flush()
363363

364364
print(logger.output)
365365
XCTAssertEqual(logger.output, """

‎Tests/SwiftDocCTests/Diagnostics/DiagnosticFileWriterTests.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class DiagnosticFileWriterTests: XCTestCase {
6565
XCTAssertFalse(FileManager.default.fileExists(atPath: diagnosticFileURL.pathExtension))
6666
}
6767

68-
try writer.finalize()
68+
try writer.flush()
6969
XCTAssert(FileManager.default.fileExists(atPath: diagnosticFileURL.path))
7070

7171
let diagnosticFile = try JSONDecoder().decode(DiagnosticFile.self, from: Data(contentsOf: diagnosticFileURL))

‎Tests/SwiftDocCUtilitiesTests/ArgumentParsing/ConvertSubcommandTests.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ class ConvertSubcommandTests: XCTestCase {
457457
}
458458
}
459459

460-
func testTreatWarningAsrror() throws {
460+
func testTreatWarningAsError() throws {
461461
SetEnvironmentVariable(TemplateOption.environmentVariableKey, testTemplateURL.path)
462462
do {
463463
// Passing no argument should default to the current working directory.

‎Tests/SwiftDocCUtilitiesTests/ConvertActionTests.swift‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3055,6 +3055,60 @@ class ConvertActionTests: XCTestCase {
30553055
XCTAssert(engine.problems.contains(where: { $0.diagnostic.severity == .warning }))
30563056
}
30573057

3058+
func testWrittenDiagnosticsAfterConvert() throws {
3059+
let bundle = Folder(name: "unit-test.docc", content: [
3060+
InfoPlist(displayName: "TestBundle", identifier: "com.test.example"),
3061+
TextFile(name: "Documentation.md", utf8Content: """
3062+
# ``ModuleThatDoesNotExist``
3063+
3064+
This will result in two errors from two different phases of the build
3065+
""")
3066+
])
3067+
let testDataProvider = try TestFileSystem(folders: [bundle, Folder.emptyHTMLTemplateDirectory])
3068+
let targetDirectory = URL(fileURLWithPath: testDataProvider.currentDirectoryPath).appendingPathComponent("target", isDirectory: true)
3069+
let diagnosticFile = try createTemporaryDirectory().appendingPathComponent("test-diagnostics.json")
3070+
let fileConsumer = DiagnosticFileWriter(outputPath: diagnosticFile)
3071+
3072+
let engine = DiagnosticEngine()
3073+
engine.add(fileConsumer)
3074+
3075+
let logStorage = LogHandle.LogStorage()
3076+
let consoleConsumer = DiagnosticConsoleWriter(LogHandle.memory(logStorage), formattingOptions: [], baseURL: nil, highlight: false)
3077+
engine.add(consoleConsumer)
3078+
3079+
var action = try ConvertAction(
3080+
documentationBundleURL: bundle.absoluteURL,
3081+
outOfProcessResolver: nil,
3082+
analyze: false,
3083+
targetDirectory: targetDirectory,
3084+
htmlTemplateDirectory: Folder.emptyHTMLTemplateDirectory.absoluteURL,
3085+
emitDigest: false,
3086+
currentPlatforms: nil,
3087+
dataProvider: testDataProvider,
3088+
fileManager: testDataProvider,
3089+
temporaryDirectory: createTemporaryDirectory(),
3090+
diagnosticEngine: engine
3091+
)
3092+
3093+
let _ = try action.perform(logHandle: .standardOutput)
3094+
XCTAssertEqual(engine.problems.count, 2)
3095+
3096+
XCTAssert(FileManager.default.fileExists(atPath: diagnosticFile.path))
3097+
3098+
let diagnosticFileContent = try JSONDecoder().decode(DiagnosticFile.self, from: Data(contentsOf: diagnosticFile))
3099+
XCTAssertEqual(diagnosticFileContent.diagnostics.count, 2)
3100+
3101+
XCTAssertEqual(diagnosticFileContent.diagnostics.map(\.summary).sorted(), [
3102+
"No TechnologyRoot to organize article-only documentation.",
3103+
"No symbol matched 'ModuleThatDoesNotExist'. Can't resolve 'ModuleThatDoesNotExist'."
3104+
])
3105+
3106+
let logLines = logStorage.text.splitByNewlines
3107+
XCTAssertEqual(logLines.filter { ($0 as NSString).contains("warning:") }.count, 2, "There should be two warnings printed to the console")
3108+
XCTAssertEqual(logLines.filter { ($0 as NSString).contains("No TechnologyRoot to organize article-only documentation.") }.count, 1, "The root page warning shouldn't be repeated.")
3109+
XCTAssertEqual(logLines.filter { ($0 as NSString).contains("No symbol matched 'ModuleThatDoesNotExist'. Can't resolve 'ModuleThatDoesNotExist'.") }.count, 1, "The link warning shouldn't be repeated.")
3110+
}
3111+
30583112
#endif
30593113
}
30603114

0 commit comments

Comments
 (0)