diff --git a/Sources/SwiftCompilerPluginMessageHandling/Diagnostics.swift b/Sources/SwiftCompilerPluginMessageHandling/Diagnostics.swift index 277212cd08c..2e68c1efd42 100644 --- a/Sources/SwiftCompilerPluginMessageHandling/Diagnostics.swift +++ b/Sources/SwiftCompilerPluginMessageHandling/Diagnostics.swift @@ -68,10 +68,12 @@ extension PluginMessage.Diagnostic.Severity { extension PluginMessage.Diagnostic { init(from syntaxDiag: SwiftDiagnostics.Diagnostic, in sourceManager: SourceManager) { - if let position = sourceManager.position( - of: syntaxDiag.node, - at: .afterLeadingTrivia - ) { + if let node = syntaxDiag.node, + let position = sourceManager.position( + of: node, + at: .afterLeadingTrivia + ) + { self.position = .init(fileName: position.fileName, offset: position.utf8Offset) } else { self.position = .invalid @@ -92,7 +94,7 @@ extension PluginMessage.Diagnostic { } self.notes = syntaxDiag.notes.compactMap { - guard let pos = sourceManager.position(of: $0.node, at: .afterLeadingTrivia) else { + guard let node = $0.node, let pos = sourceManager.position(of: node, at: .afterLeadingTrivia) else { return nil } let position = PluginMessage.Diagnostic.Position( diff --git a/Sources/SwiftDiagnostics/Diagnostic.swift b/Sources/SwiftDiagnostics/Diagnostic.swift index c4655bb85c3..ab57eba39aa 100644 --- a/Sources/SwiftDiagnostics/Diagnostic.swift +++ b/Sources/SwiftDiagnostics/Diagnostic.swift @@ -21,11 +21,11 @@ public struct Diagnostic: CustomDebugStringConvertible, Sendable { public let diagMessage: DiagnosticMessage /// The node at whose start location the message should be displayed. - public let node: Syntax + public let node: Syntax? /// The position at which the location should be anchored. /// By default, this is the start location of `node`. - public let position: AbsolutePosition + public let position: AbsolutePosition? /// Nodes that should be highlighted in the source code. public let highlights: [Syntax] @@ -40,17 +40,17 @@ public struct Diagnostic: CustomDebugStringConvertible, Sendable { /// If `highlights` is `nil` then `node` will be highlighted. This is a /// reasonable default for almost all diagnostics. public init( - node: some SyntaxProtocol, + node: (some SyntaxProtocol)?, position: AbsolutePosition? = nil, message: DiagnosticMessage, highlights: [Syntax]? = nil, notes: [Note] = [], fixIts: [FixIt] = [] ) { - self.node = Syntax(node) - self.position = position ?? node.positionAfterSkippingLeadingTrivia + self.node = node.map { Syntax($0) } + self.position = position ?? node?.positionAfterSkippingLeadingTrivia self.diagMessage = message - self.highlights = highlights ?? [Syntax(node)] + self.highlights = highlights ?? node.map { [Syntax($0)] } ?? [] self.notes = notes self.fixIts = fixIts } @@ -67,13 +67,24 @@ public struct Diagnostic: CustomDebugStringConvertible, Sendable { } /// The location at which the diagnostic should be displayed. - public func location(converter: SourceLocationConverter) -> SourceLocation { + public func location(converter: SourceLocationConverter) -> SourceLocation? { + guard let position else { + return nil + } + return converter.location(for: position) } public var debugDescription: String { + guard let node else { + return "\(DiagnosticsFormatter.unknownFileName):0:0: \(message)" + } + let locationConverter = SourceLocationConverter(fileName: "", tree: node.root) - let location = location(converter: locationConverter) + guard let location = location(converter: locationConverter) else { + return "\(DiagnosticsFormatter.unknownFileName):0:0: \(message)" + } + return "\(location.line):\(location.column): \(message)" } } diff --git a/Sources/SwiftDiagnostics/DiagnosticsFormatter.swift b/Sources/SwiftDiagnostics/DiagnosticsFormatter.swift index 28f4d18bf4e..51f8619dc3b 100644 --- a/Sources/SwiftDiagnostics/DiagnosticsFormatter.swift +++ b/Sources/SwiftDiagnostics/DiagnosticsFormatter.swift @@ -54,6 +54,9 @@ extension Sequence where Element == Range { } public struct DiagnosticsFormatter { + /// The string used to identify an unknown file name. + @_spi(Testing) + public static let unknownFileName = "" /// A wrapper struct for a source line, its diagnostics, and any /// non-diagnostic text that follows the line. @@ -215,14 +218,19 @@ public struct DiagnosticsFormatter { suffixTexts: [AbsolutePosition: String], sourceLocationConverter: SourceLocationConverter? = nil ) -> String { - let slc = sourceLocationConverter ?? SourceLocationConverter(fileName: "", tree: tree) + let slc = + sourceLocationConverter ?? SourceLocationConverter(fileName: DiagnosticsFormatter.unknownFileName, tree: tree) // First, we need to put each line and its diagnostics together var annotatedSourceLines = [AnnotatedSourceLine]() for (sourceLineIndex, sourceLine) in slc.sourceLines.enumerated() { let diagsForLine = diags.filter { diag in - return diag.location(converter: slc).line == (sourceLineIndex + 1) + guard let location = diag.location(converter: slc) else { + return false + } + + return location.line == (sourceLineIndex + 1) } let suffixText = suffixTexts.compactMap { (position, text) in if slc.location(for: position).line == (sourceLineIndex + 1) { @@ -299,12 +307,14 @@ public struct DiagnosticsFormatter { } let columnsWithDiagnostics = Set( - annotatedLine.diagnostics.map { - annotatedLine.characterColumn(ofUtf8Column: $0.location(converter: slc).column) + annotatedLine.diagnostics.compactMap { + $0.location(converter: slc).map { + annotatedLine.characterColumn(ofUtf8Column: $0.column) + } } ) let diagsPerColumn = Dictionary(grouping: annotatedLine.diagnostics) { diag in - annotatedLine.characterColumn(ofUtf8Column: diag.location(converter: slc).column) + annotatedLine.characterColumn(ofUtf8Column: diag.location(converter: slc)!.column) }.sorted { lhs, rhs in lhs.key > rhs.key } diff --git a/Sources/SwiftDiagnostics/GroupedDiagnostics.swift b/Sources/SwiftDiagnostics/GroupedDiagnostics.swift index f1f3059bc19..f25108152c2 100644 --- a/Sources/SwiftDiagnostics/GroupedDiagnostics.swift +++ b/Sources/SwiftDiagnostics/GroupedDiagnostics.swift @@ -57,6 +57,10 @@ public struct GroupedDiagnostics { /// source file IDs. var rootIndexes: [Syntax: SourceFileID] = [:] + /// Diagnostics that aren't associated with any source file because they have + /// no location information. + var floatingDiagnostics: [Diagnostic] = [] + public init() {} /// Add a new source file to the set of grouped diagnostics. @@ -122,8 +126,16 @@ public struct GroupedDiagnostics { _ diagnostic: Diagnostic, in knownSourceFileID: SourceFileID? = nil ) { - guard let sourceFileID = knownSourceFileID ?? findSourceFileContaining(diagnostic.node) else { - // Drop the diagnostic on the floor. + guard let node = diagnostic.node else { + // There is no node anchoring the diagnostic in a source file, so treat + // it as floating. + floatingDiagnostics.append(diagnostic) + return + } + + guard let sourceFileID = knownSourceFileID ?? findSourceFileContaining(node) else { + // This diagnostic is in source file, but we aren't producing diagnostics + // for that file. Drop the diagnostic on the floor. return } @@ -205,7 +217,9 @@ extension GroupedDiagnostics { // If there's a primary diagnostic, print it first. if let (primaryDiagSourceFile, primaryDiag) = findPrimaryDiagnostic(in: sourceFile) { let primaryDiagSLC = primaryDiagSourceFile.sourceLocationConverter - let location = primaryDiag.location(converter: primaryDiagSLC) + let location = + primaryDiag.location(converter: primaryDiagSLC) + ?? SourceLocation(line: 0, column: 0, offset: 0, file: DiagnosticsFormatter.unknownFileName) // Display file/line/column and diagnostic text for the primary diagnostic. prefixString = @@ -233,9 +247,13 @@ extension GroupedDiagnostics { prefixString += "`- \(bufferLoc.file):\(bufferLoc.line):\(bufferLoc.column): \(decoratedMessage)\n" } } + } else if let firstDiag = sourceFile.diagnostics.first, + let firstDiagLoc = firstDiag.location(converter: slc) + { + prefixString = "\(sourceFile.displayName): \(firstDiagLoc.line):" } else { - let firstLine = sourceFile.diagnostics.first.map { $0.location(converter: slc).line } ?? 0 - prefixString = "\(sourceFile.displayName): \(firstLine):" + // There are no diagnostics to print from this file. + return "" } suffixString = "" @@ -275,9 +293,25 @@ extension GroupedDiagnostics { extension DiagnosticsFormatter { /// Annotate all of the source files in the given set of grouped diagnostics. public func annotateSources(in group: GroupedDiagnostics) -> String { - return group.rootSourceFiles.map { rootSourceFileID in - group.annotateSource(rootSourceFileID, formatter: self, indentString: "") - }.joined(separator: "\n") + // Render all grouped diagnostics. + var renderedDiags = group.rootSourceFiles.compactMap { rootSourceFileID in + let annotated = group.annotateSource( + rootSourceFileID, + formatter: self, + indentString: "" + ) + return annotated.isEmpty ? nil : annotated + } + + // Render any floating diagnostics, which have no anchor. + renderedDiags.append( + contentsOf: group.floatingDiagnostics.map { diag in + let renderedMessage = diagnosticDecorator.decorateDiagnosticMessage(diag.diagMessage) + return "\(DiagnosticsFormatter.unknownFileName):0:0: \(renderedMessage)" + } + ) + + return renderedDiags.joined(separator: "\n") } public static func annotateSources( diff --git a/Sources/SwiftDiagnostics/Note.swift b/Sources/SwiftDiagnostics/Note.swift index 8a85d79a969..06e9a5b5d31 100644 --- a/Sources/SwiftDiagnostics/Note.swift +++ b/Sources/SwiftDiagnostics/Note.swift @@ -37,22 +37,22 @@ extension NoteMessage { /// A note that points to another node that's relevant for a Diagnostic. public struct Note: CustomDebugStringConvertible, Sendable { /// The node whose location the node is pointing. - public let node: Syntax + public let node: Syntax? /// The position at which the location should be anchored. /// By default, this is the start location of `node`. - public let position: AbsolutePosition + public let position: AbsolutePosition? /// A description of what this note is pointing at. public let noteMessage: NoteMessage public init( - node: Syntax, + node: Syntax?, position: AbsolutePosition? = nil, message: NoteMessage ) { self.node = node - self.position = position ?? node.positionAfterSkippingLeadingTrivia + self.position = position ?? node?.positionAfterSkippingLeadingTrivia self.noteMessage = message } @@ -62,17 +62,22 @@ public struct Note: CustomDebugStringConvertible, Sendable { } /// The location at which the note should be displayed. - public func location(converter: SourceLocationConverter) -> SourceLocation { + public func location(converter: SourceLocationConverter) -> SourceLocation? { + guard let position else { + return nil + } + return converter.location(for: position) } public var debugDescription: String { - if let root = node.root.as(SourceFileSyntax.self) { + if let root = node?.root.as(SourceFileSyntax.self) { let locationConverter = SourceLocationConverter(fileName: "", tree: root) - let location = location(converter: locationConverter) - return "\(location): \(message)" - } else { - return ": \(message)" + if let location = location(converter: locationConverter) { + return "\(location): \(message)" + } } + + return "\(DiagnosticsFormatter.unknownFileName):0:0: \(message)" } } diff --git a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift index 20b7c7702fd..c9e5d9198ea 100644 --- a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift +++ b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift @@ -107,19 +107,27 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { let diagProducer = ParseDiagnosticsGenerator() diagProducer.walk(tree) diagProducer.diagnostics.sort { - if $0.position != $1.position { - return $0.position < $1.position + guard let lhsPosition = $0.position, let rhsPosition = $1.position else { + return $0.position == nil + } + + if lhsPosition != rhsPosition { + return lhsPosition < rhsPosition + } + + guard let lhsNode = $0.node, let rhsNode = $1.node else { + return $0.node == nil } // Emit children diagnostics before parent diagnostics. // This makes sure that for missing declarations with attributes, we emit diagnostics on the attribute before we complain about the missing declaration. - if $0.node.hasParent($1.node) { + if lhsNode.hasParent(rhsNode) { return true - } else if $1.node.hasParent($0.node) { + } else if rhsNode.hasParent(lhsNode) { return false } else { // If multiple tokens are missing at the same location, emit diagnostics about nodes that occur earlier in the tree first. - return $0.node.id < $1.node.id + return lhsNode.id < rhsNode.id } } return diagProducer.diagnostics @@ -157,7 +165,16 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { if suppressRemainingDiagnostics { return } - diagnostics.removeAll(where: { handledNodes.contains($0.node.id) }) + + diagnostics.removeAll( + where: { + if let nodeId = $0.node?.id { + return handledNodes.contains(nodeId) + } + + return false + } + ) diagnostics.append(diagnostic) self.handledNodes.append(contentsOf: handledNodes) } diff --git a/Sources/SwiftSyntaxMacrosGenericTestSupport/Assertions.swift b/Sources/SwiftSyntaxMacrosGenericTestSupport/Assertions.swift index 7d7e490cbc0..32363c35962 100644 --- a/Sources/SwiftSyntaxMacrosGenericTestSupport/Assertions.swift +++ b/Sources/SwiftSyntaxMacrosGenericTestSupport/Assertions.swift @@ -157,7 +157,18 @@ func assertNote( location: spec.failureLocation.underlying, failureHandler: { failureHandler(TestFailureSpec(underlying: $0)) } ) - let location = expansionContext.location(for: note.position, anchoredAt: note.node, fileName: "") + guard let position = note.position, let node = note.node else { + failureHandler( + TestFailureSpec( + message: "note has no position", + location: spec.failureLocation + ) + ) + + return + } + + let location = expansionContext.location(for: position, anchoredAt: node, fileName: "") if location.line != spec.line { failureHandler( TestFailureSpec( @@ -387,7 +398,19 @@ public func assertDiagnostic( location: spec.failureLocation.underlying, failureHandler: { failureHandler(TestFailureSpec(underlying: $0)) } ) - let location = expansionContext.location(for: diag.position, anchoredAt: diag.node, fileName: "") + guard let position = diag.position, + let node = diag.node + else { + failureHandler( + TestFailureSpec( + message: "diagnostic missing location info", + location: spec.failureLocation + ) + ) + return + } + + let location = expansionContext.location(for: position, anchoredAt: node, fileName: "") if location.line != spec.line { failureHandler( TestFailureSpec( diff --git a/Tests/SwiftDiagnosticsTest/GroupDiagnosticsFormatterTests.swift b/Tests/SwiftDiagnosticsTest/GroupDiagnosticsFormatterTests.swift index d4892a26020..ff8244e20d1 100644 --- a/Tests/SwiftDiagnosticsTest/GroupDiagnosticsFormatterTests.swift +++ b/Tests/SwiftDiagnosticsTest/GroupDiagnosticsFormatterTests.swift @@ -85,6 +85,43 @@ final class GroupedDiagnosticsFormatterTests: XCTestCase { ) } + func testFloatingDiagnostics() { + var group = GroupedDiagnostics() + _ = group.addTestFile( + """ + func f() { } + """, + displayName: "main.swift", + diagnosticDescriptors: [] + ) + + group.addDiagnostic( + Diagnostic( + node: nil as Syntax?, + message: SimpleDiagnosticMessage( + message: "this is bad", + diagnosticID: MessageID( + domain: "swift-syntax", + id: "diagnostic test" + ), + severity: .error, + category: DiagnosticCategory( + name: "Badness", + documentationURL: nil + ) + ) + ) + ) + + let annotated = DiagnosticsFormatter.annotateSources(in: group) + assertStringsEqualWithDiff( + annotated, + """ + :0:0: error: this is bad [#Badness] + """ + ) + } + func testGroupingForMacroExpansion() { var group = GroupedDiagnostics() @@ -259,4 +296,5 @@ final class GroupedDiagnosticsFormatterTests: XCTestCase { """ ) } + } diff --git a/Tests/SwiftParserTest/Assertions.swift b/Tests/SwiftParserTest/Assertions.swift index cb537012a3c..42fdb4b1e46 100644 --- a/Tests/SwiftParserTest/Assertions.swift +++ b/Tests/SwiftParserTest/Assertions.swift @@ -10,7 +10,7 @@ // //===----------------------------------------------------------------------===// -import SwiftDiagnostics +@_spi(Testing) import SwiftDiagnostics @_spi(FixItApplier) import SwiftIDEUtils @_spi(Testing) @_spi(RawSyntax) @_spi(AlternateTokenIntrospection) @_spi(ExperimentalLanguageFeatures) import SwiftParser @_spi(RawSyntax) import SwiftParserDiagnostics @@ -324,7 +324,7 @@ func assertNote( XCTAssertEqual(note.message, spec.message, file: spec.file, line: spec.line) let locationConverter = SourceLocationConverter(fileName: "", tree: tree) assertLocation( - note.location(converter: locationConverter), + note.location(converter: locationConverter)!, in: tree, markerLocations: markerLocations, expectedLocationMarker: spec.locationMarker, @@ -343,7 +343,7 @@ func assertDiagnostic( ) { let locationConverter = SourceLocationConverter(fileName: "", tree: tree) assertLocation( - diag.location(converter: locationConverter), + diag.location(converter: locationConverter)!, in: tree, markerLocations: markerLocations, expectedLocationMarker: spec.locationMarker, @@ -368,7 +368,7 @@ func assertDiagnostic( ) } - let highlight = spec.highlight ?? diag.node.description + let highlight = spec.highlight ?? diag.node?.description ?? DiagnosticsFormatter.unknownFileName assertStringsEqualWithDiff( diag.highlights.map(\.description).joined().trimmingTrailingWhitespace(), highlight.trimmingTrailingWhitespace(),