Skip to content

[NFC] Fix diagnostic message query formatting for compatibility with existing tests, and fix additional tests #1614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ import class Foundation.JSONEncoder
import class Foundation.JSONDecoder
import var Foundation.EXIT_SUCCESS

private extension String {
func stripNewline() -> String {
return self.hasSuffix("\n") ? String(self.dropLast()) : self
}
}

extension Diagnostic.Message {
static func warn_scan_dylib_not_found() -> Diagnostic.Message {
.warning("Unable to locate libSwiftScan. Fallback to `swift-frontend` dependency scanner invocation.")
Expand All @@ -39,16 +33,16 @@ extension Diagnostic.Message {
.error("Swift Caching enabled - libSwiftScan load failed (\(libPath)).")
}
static func scanner_diagnostic_error(_ message: String) -> Diagnostic.Message {
return .error(message.stripNewline())
return .error(message)
}
static func scanner_diagnostic_warn(_ message: String) -> Diagnostic.Message {
.warning(message.stripNewline())
.warning(message)
}
static func scanner_diagnostic_note(_ message: String) -> Diagnostic.Message {
.note(message.stripNewline())
.note(message)
}
static func scanner_diagnostic_remark(_ message: String) -> Diagnostic.Message {
.remark(message.stripNewline())
.remark(message)
}
}

Expand Down
25 changes: 13 additions & 12 deletions Sources/SwiftDriver/SwiftScan/SwiftScan.swift
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ internal extension swiftscan_diagnostic_severity_t {
}
}

private extension String {
func stripNewline() -> String {
return self.hasSuffix("\n") ? String(self.dropLast()) : self
}
}

/// Wrapper for libSwiftScan, taking care of initialization, shutdown, and dispatching dependency scanning queries.
@_spi(Testing) public final class SwiftScan {
/// The path to the libSwiftScan dylib.
Expand Down Expand Up @@ -159,6 +165,8 @@ internal extension swiftscan_diagnostic_severity_t {
guard let importSetRef = importSetRefOrNull else {
throw DependencyScanningError.dependencyScanFailed("Unable to produce import set")
}
defer { api.swiftscan_import_set_dispose(importSetRef) }

if canQueryPerScanDiagnostics {
let diagnosticsSetRefOrNull = api.swiftscan_import_set_get_diagnostics(importSetRef)
guard let diagnosticsSetRef = diagnosticsSetRefOrNull else {
Expand All @@ -167,11 +175,7 @@ internal extension swiftscan_diagnostic_severity_t {
diagnostics = try mapToDriverDiagnosticPayload(diagnosticsSetRef)
}

let importSet = try constructImportSet(from: importSetRef, with: moduleAliases)
// Free the memory allocated for the in-memory representation of the import set
// returned by the scanner, now that we have translated it.
api.swiftscan_import_set_dispose(importSetRef)
return importSet
return try constructImportSet(from: importSetRef, with: moduleAliases)
}

func scanDependencies(workingDirectory: AbsolutePath,
Expand All @@ -195,6 +199,8 @@ internal extension swiftscan_diagnostic_severity_t {
guard let graphRef = graphRefOrNull else {
throw DependencyScanningError.dependencyScanFailed("Unable to produce dependency graph")
}
defer { api.swiftscan_dependency_graph_dispose(graphRef) }

if canQueryPerScanDiagnostics {
let diagnosticsSetRefOrNull = api.swiftscan_dependency_graph_get_diagnostics(graphRef)
guard let diagnosticsSetRef = diagnosticsSetRefOrNull else {
Expand All @@ -203,12 +209,7 @@ internal extension swiftscan_diagnostic_severity_t {
diagnostics = try mapToDriverDiagnosticPayload(diagnosticsSetRef)
}

let dependencyGraph = try constructGraph(from: graphRef, moduleAliases: moduleAliases)
// Free the memory allocated for the in-memory representation of the dependency
// graph returned by the scanner, now that we have translated it into an
// `InterModuleDependencyGraph`.
api.swiftscan_dependency_graph_dispose(graphRef)
return dependencyGraph
return try constructGraph(from: graphRef, moduleAliases: moduleAliases)
}

func batchScanDependencies(workingDirectory: AbsolutePath,
Expand Down Expand Up @@ -385,7 +386,7 @@ internal extension swiftscan_diagnostic_severity_t {
guard let diagnosticRef = diagnosticRefOrNull else {
throw DependencyScanningError.dependencyScanFailed("Unable to produce scanner diagnostics")
}
let message = try toSwiftString(api.swiftscan_diagnostic_get_message(diagnosticRef))
let message = try toSwiftString(api.swiftscan_diagnostic_get_message(diagnosticRef)).stripNewline()
let severity = api.swiftscan_diagnostic_get_severity(diagnosticRef)

var sourceLoc: ScannerDiagnosticSourceLocation? = nil
Expand Down
31 changes: 25 additions & 6 deletions Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1470,7 +1470,6 @@ final class ExplicitModuleBuildTests: XCTestCase {
Unable to find module dependency: 'unknown_module'
import unknown_module
^

""")
let sourceLoc = try XCTUnwrap(error.sourceLocation)
XCTAssertTrue(sourceLoc.bufferIdentifier.hasSuffix("I.swiftinterface"))
Expand All @@ -1495,7 +1494,6 @@ final class ExplicitModuleBuildTests: XCTestCase {
a dependency of main module 'testDependencyScanning'
import unknown_module
^

"""
)
} else {
Expand Down Expand Up @@ -1545,7 +1543,6 @@ final class ExplicitModuleBuildTests: XCTestCase {
Unable to find module dependency: 'FooBar'
import FooBar
^

""")

let sourceLoc = try XCTUnwrap(error.sourceLocation)
Expand All @@ -1562,7 +1559,6 @@ final class ExplicitModuleBuildTests: XCTestCase {
a dependency of main module 'testDependencyScanning'
import FooBar
^

"""
)
} else {
Expand Down Expand Up @@ -1817,8 +1813,31 @@ final class ExplicitModuleBuildTests: XCTestCase {
for scanIndex in 0..<numFiles {
let diagnostics = scanDiagnostics[scanIndex]
XCTAssertEqual(diagnostics.count, 2)
XCTAssertEqual(diagnostics[0].message, "Unable to find module dependency: 'UnknownModule\(scanIndex)'")
XCTAssertEqual(diagnostics[1].message, "a dependency of main module 'testParallelDependencyScanningDiagnostics\(scanIndex)'")
// Diagnostic source locations came after per-scan diagnostics, only meaningful
// on this test code-path
if try dependencyOracle.supportsDiagnosticSourceLocations() {
let sourceLoc = try XCTUnwrap(diagnostics[0].sourceLocation)
XCTAssertEqual(sourceLoc.lineNumber, 1)
XCTAssertEqual(sourceLoc.columnNumber, 8)
XCTAssertEqual(diagnostics[0].message,
"""
Unable to find module dependency: 'UnknownModule\(scanIndex)'
import UnknownModule\(scanIndex);
^
""")
let noteSourceLoc = try XCTUnwrap(diagnostics[1].sourceLocation)
XCTAssertEqual(noteSourceLoc.lineNumber, 1)
XCTAssertEqual(noteSourceLoc.columnNumber, 8)
XCTAssertEqual(diagnostics[1].message,
"""
a dependency of main module 'testParallelDependencyScanningDiagnostics\(scanIndex)'
import UnknownModule\(scanIndex);
^
""")
} else {
XCTAssertEqual(diagnostics[0].message, "Unable to find module dependency: 'UnknownModule\(scanIndex)'")
XCTAssertEqual(diagnostics[1].message, "a dependency of main module 'testParallelDependencyScanningDiagnostics\(scanIndex)'")
}
}
} else {
let globalDiagnostics = try dependencyOracle.getScannerDiagnostics()
Expand Down