From b6fcad4496cce446faa8e4f79b7558a33710a842 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Ba=CC=A8k?= Date: Wed, 27 Sep 2023 16:22:37 +0200 Subject: [PATCH] Enhance diagnostic formatter to include trivia between highlight nodes - Separate logic for calculating highlight ranges from `DiagnosticsFormatter` into `SyntaxHighlightRangeCalculator`. - Introduce an `ExtendedHighlight` private struct to manage trivia settings for each highlight. - Extend logic to consider leading and trailing trivia when highlighting consecutive nodes. - The `computeHighlightRanges` method now takes into account special cases for trivia between consecutive highlight nodes. - Add unit tests to cover all paths in `SyntaxHighlightRangeCalculator`. - Add documentation about trivia handling, explaining the conditions under which trivia should be included or excluded. --- .../DiagnosticsFormatter.swift | 87 +---- .../SyntaxHighlightRangeCalculator.swift | 225 +++++++++++++ .../DiagnosticsFormatterTests.swift | 2 +- .../SyntaxHighlightRangeCalculatorTests.swift | 297 ++++++++++++++++++ 4 files changed, 529 insertions(+), 82 deletions(-) create mode 100644 Sources/SwiftDiagnostics/SyntaxHighlightRangeCalculator.swift create mode 100644 Tests/SwiftDiagnosticsTest/SyntaxHighlightRangeCalculatorTests.swift diff --git a/Sources/SwiftDiagnostics/DiagnosticsFormatter.swift b/Sources/SwiftDiagnostics/DiagnosticsFormatter.swift index f55c9b42411..bc7339f3fa3 100644 --- a/Sources/SwiftDiagnostics/DiagnosticsFormatter.swift +++ b/Sources/SwiftDiagnostics/DiagnosticsFormatter.swift @@ -12,43 +12,6 @@ import SwiftSyntax -extension Sequence where Element == Range { - /// Given a set of ranges that are sorted in order of nondecreasing lower - /// bound, merge any overlapping ranges to produce a sequence of - /// nonoverlapping ranges. - fileprivate func mergingOverlappingRanges() -> [Range] { - var result: [Range] = [] - - var prior: Range? = nil - for range in self { - // If this is the first range we've seen, note it as the prior and - // continue. - guard let priorRange = prior else { - prior = range - continue - } - - // If the ranges overlap, expand the prior range. - precondition(priorRange.lowerBound <= range.lowerBound) - if priorRange.overlaps(range) { - let lower = priorRange.lowerBound - let upper = Swift.max(priorRange.upperBound, range.upperBound) - prior = lower..] = annotatedLine.diagnostics.map { - $0.highlights - }.joined().compactMap { (highlight) -> Range? in - if highlight.root != Syntax(tree) { - return nil - } - - let startLoc = highlight.startLocation(converter: slc, afterLeadingTrivia: true) - let startLine = startLoc.line - - // Find the starting column. - let startColumn: Int - if startLine < lineNumber { - startColumn = 1 - } else if startLine == lineNumber { - startColumn = startLoc.column - } else { - return nil - } - - // Find the ending column. - let endLoc = highlight.endLocation(converter: slc, afterTrailingTrivia: false) - let endLine = endLoc.line - - let endColumn: Int - if endLine > lineNumber { - endColumn = annotatedLine.sourceString.count - } else if endLine == lineNumber { - endColumn = endLoc.column - } else { - return nil - } - - if startColumn == endColumn { - return nil - } - - return startColumn..` instances for the start and end columns. + /// + /// - Note: + /// 1. If the highlight is the first in the line, its leading trivia is always excluded. + /// 2. If a highlight's end aligns with the start of the next highlight, their shared trivia settings are adjusted. + /// 3. The last highlight in the line never includes trailing trivia. + @_spi(Testing) public static func computeHighlightRanges( + forLine annotatedSourceLine: (highlights: [Syntax], sourceString: String), + at lineNumber: Int, + fromTree tree: some SyntaxProtocol, + usingSourceLocationConverter sourceLocationConverter: SourceLocationConverter + ) -> [Range] { + // Initialize an array of extended highlights, based on provided Syntax highlights. + var extendedHighlights = annotatedSourceLine.highlights.map { ExtendedHighlight(highlight: $0) } + + // Loop through each extended highlight to fine-tune its trivia settings. + for extendedHighlightsIndex in extendedHighlights.indices { + adjustTriviaSettingsForHighlight( + atIndex: extendedHighlightsIndex, + in: &extendedHighlights, + usingSourceLocationConverter: sourceLocationConverter + ) + } + + // Compute the highlight ranges, one for each extended highlight. + let highlightRanges: [Range] = + extendedHighlights + .compactMap { extendedHighlight in + if extendedHighlight.highlight.root != Syntax(tree) { + return nil + } + + // Start and end locations considering the adjusted trivia settings. + let startLoc = extendedHighlight.highlight.startLocation(converter: sourceLocationConverter, afterLeadingTrivia: extendedHighlight.afterLeadingTrivia) + let endLoc = extendedHighlight.highlight.endLocation(converter: sourceLocationConverter, afterTrailingTrivia: extendedHighlight.afterTrailingTrivia) + + // Compute starting column based on the line number and start location. + let startColumn: Int + if startLoc.line < lineNumber { + startColumn = 1 + } else if startLoc.line == lineNumber { + startColumn = startLoc.column + } else { + return nil + } + + // Compute ending column based on the line number and end location. + let endColumn: Int + if endLoc.line > lineNumber { + endColumn = annotatedSourceLine.sourceString.count + } else if endLoc.line == lineNumber { + endColumn = endLoc.column + } else { + return nil + } + + // Skip highlights with identical start and end columns. + if startColumn == endColumn { + return nil + } + + return startColumn.. firstIndex { + let previousIndex = extendedHighlights.index(before: index) + // Get end location of the previous highlight. + let previousEndLoc = extendedHighlights[previousIndex].highlight.endLocation(converter: sourceLocationConverter, afterTrailingTrivia: true) + // Get start location of the current highlight without leading trivia. + let currentStartLoc = extendedHighlights[index].highlight.startLocation(converter: sourceLocationConverter, afterLeadingTrivia: false) + + // If the end of the previous highlight aligns with the start of the current, adjust trivia settings. + if previousEndLoc == currentStartLoc { + extendedHighlights[previousIndex].afterTrailingTrivia = true + afterLeadingTrivia = false + } + } + + // Check if the current highlight's end aligns with the start of the next highlight. + if index < lastIndex { + let nextIndex = extendedHighlights.index(after: index) + // Get end location of the current highlight with trailing trivia. + let currentEndLoc = extendedHighlights[index].highlight.endLocation(converter: sourceLocationConverter, afterTrailingTrivia: true) + // Get start location of the next highlight without leading trivia. + let nextStartLoc = extendedHighlights[nextIndex].highlight.startLocation(converter: sourceLocationConverter, afterLeadingTrivia: false) + + // If the end of the current highlight aligns with the start of the next, adjust trivia settings. + if currentEndLoc == nextStartLoc { + afterTrailingTrivia = true + extendedHighlights[nextIndex].afterLeadingTrivia = false + } + } + + // Update the trivia settings for the current highlight. + extendedHighlights[index].afterLeadingTrivia = afterLeadingTrivia + extendedHighlights[index].afterTrailingTrivia = afterTrailingTrivia + } +} + +extension Sequence where Element == Range { + /// Given a set of ranges that are sorted in order of nondecreasing lower + /// bound, merge any overlapping ranges to produce a sequence of + /// nonoverlapping ranges. + fileprivate func mergingOverlappingRanges() -> [Range] { + var result: [Range] = [] + + var prior: Range? = nil + for range in self { + // If this is the first range we've seen, note it as the prior and + // continue. + guard let priorRange = prior else { + prior = range + continue + } + + // If the ranges overlap, expand the prior range. + precondition(priorRange.lowerBound <= range.lowerBound) + if priorRange.overlaps(range) { + let lower = priorRange.lowerBound + let upper = Swift.max(priorRange.upperBound, range.upperBound) + prior = lower..], + file: StaticString = #file, + line: UInt = #line +) { + let sourceLocationConverter = SourceLocationConverter( + fileName: "", + tree: tree + ) + + let computedRanges = SyntaxHighlightRangeCalculator.computeHighlightRanges( + forLine: (highlights: highlights.map { Syntax(fromProtocol: $0.node) }, sourceString: lineSource ?? tree.description), + at: lineNumber, + fromTree: tree, + usingSourceLocationConverter: sourceLocationConverter + ) + + XCTAssertEqual(computedRanges, expectedRanges, file: file, line: line) + for highlight in highlights { + assertStringsEqualWithDiff( + highlight.node.description, + highlight.expectedNodeDescription, + """ + The description of the passed highlight node of type \(highlight.node.syntaxNodeType) does not \ + align with the expected description. + """, + additionalInfo: """ + Expected: + \(highlight.expectedNodeDescription) + + Actual: + \(highlight.node.description) + """, + file: file, + line: line + ) + } +} + +final class SyntaxHighlightRangeCalculatorTests: XCTestCase { + func testComputeHighlightRangesFirstHighlightExcludesLeadingTrivia() { + let decl: DeclSyntax = + "@PropertyWrapper private(set) var variable: Decimal = 123 * 456" + // 123456789012345678901234567890 + let variableDecl = decl.cast(VariableDeclSyntax.self) + + assertSyntaxHighlightRanges( + inSyntaxTree: variableDecl, + withHighlights: [ + (node: variableDecl.attributes, expectedNodeDescription: "@PropertyWrapper "), + (node: variableDecl.modifiers, expectedNodeDescription: "private(set) "), + ], + matchExpectedRanges: [1..<18, 18..<30] + ) + } + + func testComputeHighlightRangesAdjacentHighlightsShareTrivia() { + let decl: DeclSyntax = + "@PropertyWrapper private(set) var variable: Decimal = 123 * 456" + // 1234567890123456789012345678901234 + let variableDecl = decl.cast(VariableDeclSyntax.self) + + assertSyntaxHighlightRanges( + inSyntaxTree: variableDecl, + withHighlights: [ + (node: variableDecl.attributes, expectedNodeDescription: "@PropertyWrapper "), + (node: variableDecl.modifiers, expectedNodeDescription: "private(set) "), + ], + matchExpectedRanges: [1..<22, 22..<34] + ) + } + + func testComputeHighlightRangesAdjacentHighlightsShareTriviaWithModifiersAndTypeAnnotation() { + let decl: DeclSyntax = + "@PropertyWrapper private(set) var variable: Decimal = 123 * 456" + // 0123456789012 1234567890 + let variableDecl = decl.cast(VariableDeclSyntax.self) + + assertSyntaxHighlightRanges( + inSyntaxTree: variableDecl, + withHighlights: [ + (node: variableDecl.modifiers, expectedNodeDescription: "private(set) "), + (node: variableDecl.bindings.first!.typeAnnotation!, expectedNodeDescription: ": Decimal "), + ], + matchExpectedRanges: [20..<32, 51..<60] + ) + } + + func testComputeHighlightRangesOverlappingHighlightsMerge() { + let decl: DeclSyntax = + "@PropertyWrapper private(set) var variable: Decimal = 123 * 456" + // 1234567890123456 678 1234567890123456789012 + let variableDecl = decl.cast(VariableDeclSyntax.self) + + assertSyntaxHighlightRanges( + inSyntaxTree: variableDecl, + withHighlights: [ + (node: variableDecl.attributes, expectedNodeDescription: "@PropertyWrapper "), + (node: variableDecl.bindingSpecifier, expectedNodeDescription: "var "), + (node: variableDecl.bindings.first!.typeAnnotation!, expectedNodeDescription: ": Decimal "), + (node: variableDecl.bindings.first!.initializer!, expectedNodeDescription: "= 123 * 456"), + ], + matchExpectedRanges: [1..<17, 36..<39, 51..<61, 61..<72] + ) + } + + func testComputeHighlightRangesFirstHighlightExcludesLeadingTriviaWithSpaces() { + let decl: DeclSyntax = + " @PropertyWrapper private(set) var variable: Decimal = 123 * 456" + // 89012345678901234 + let variableDecl = decl.cast(VariableDeclSyntax.self) + + assertSyntaxHighlightRanges( + inSyntaxTree: variableDecl, + withHighlights: [ + (node: variableDecl.attributes, expectedNodeDescription: " @PropertyWrapper ") + ], + matchExpectedRanges: [8..<24] + ) + } + + func testComputeHighlightRangesOverlappingHighlightsMergeWithAttributesAndInitializers() { + let decl: DeclSyntax = + "@PropertyWrapper private(set) var variable: Decimal = 123 * 456" + // 1234567890123456789012 + let variableDecl = decl.cast(VariableDeclSyntax.self) + + assertSyntaxHighlightRanges( + inSyntaxTree: variableDecl, + withHighlights: [ + (node: variableDecl.bindings.first!.typeAnnotation!, expectedNodeDescription: ": Decimal "), + (node: variableDecl.bindings.first!.initializer!, expectedNodeDescription: "= 123 * 456"), + (node: variableDecl.bindings.first!.initializer!.value.cast(SequenceExprSyntax.self).elements.first!, expectedNodeDescription: "123 "), + (node: variableDecl.bindings.first!.typeAnnotation!, expectedNodeDescription: ": Decimal "), + (node: variableDecl.bindings.first!.initializer!.value.cast(SequenceExprSyntax.self).elements.last!, expectedNodeDescription: "456"), + (node: variableDecl.bindings.first!.typeAnnotation!, expectedNodeDescription: ": Decimal "), + ], + matchExpectedRanges: [51..<61, 61..<72] + ) + } + + func testComputeHighlightRangesLastHighlightExcludesTrailingTrivia() { + let decl: DeclSyntax = + "@PropertyWrapper private(set) var variable: Decimal = 123 * 456" + // 1234567890123456789012345678901234567890123456789012345678901234 + let variableDecl = decl.cast(VariableDeclSyntax.self) + + assertSyntaxHighlightRanges( + inSyntaxTree: variableDecl, + withHighlights: [ + (node: variableDecl, expectedNodeDescription: "@PropertyWrapper private(set) var variable: Decimal = 123 * 456") + ], + matchExpectedRanges: [1..<64] + ) + } + + func testComputeHighlightRangesFirstHighlightExcludesLeadingTriviaForSimpleVariable() { + let decl: DeclSyntax = + "let x=1" + // 56 + let variableDecl = decl.cast(VariableDeclSyntax.self) + + assertSyntaxHighlightRanges( + inSyntaxTree: variableDecl, + withHighlights: [ + (node: variableDecl.bindings.first!.pattern, expectedNodeDescription: "x") + ], + matchExpectedRanges: [5..<6] + ) + } + + func testComputeHighlightRangesEmptyHighlightsEmptyResult() { + let decl: DeclSyntax = "@PropertyWrapper private(set) var variable: Decimal = 123 * 456" + + assertSyntaxHighlightRanges( + inSyntaxTree: decl, + withHighlights: [], + matchExpectedRanges: [] + ) + } + + func testComputeHighlightRangesInvalidSyntaxTreeEmptyResult() { + let decl: DeclSyntax = "@PropertyWrapper private(set) var variable: Decimal = 123 * 456" + + assertSyntaxHighlightRanges( + inSyntaxTree: decl, + withHighlights: [ + (node: DeclSyntax("var variable: Decimal = 123 * 456"), expectedNodeDescription: "var variable: Decimal = 123 * 456") + ], + matchExpectedRanges: [] + ) + } + + func testNoHighlightsOnFirstLineWithVariableDeclaration() { + let decl: DeclSyntax = + """ + class MyClass { + var abc = 132 + } + """ + + let variableDecl = decl.cast(ClassDeclSyntax.self).memberBlock.members.first!.decl.cast(VariableDeclSyntax.self) + + assertSyntaxHighlightRanges( + inSyntaxTree: decl, + withHighlights: [ + (node: variableDecl, expectedNodeDescription: "\n var abc = 132") + ], + atLineNumber: 1, + usingLineSource: "class MyClass {\n", + matchExpectedRanges: [] + ) + } + + func testFullLineHighlightForSingleLineVariableDeclaration() { + let decl: DeclSyntax = + """ + class MyClass { + var abc = 132 + } + """ + + assertSyntaxHighlightRanges( + inSyntaxTree: decl, + withHighlights: [ + ( + node: decl, + expectedNodeDescription: """ + class MyClass { + var abc = 132 + } + """ + ) + ], + atLineNumber: 2, + usingLineSource: " var abc = 132\n", + matchExpectedRanges: [1..<16] + ) + } + + func testPartialHighlightForMultiLineVariableDeclarationStart() { + let decl: DeclSyntax = + """ + class MyClass { + var abc = + 132 + } + """ + + let variableDecl = decl.cast(ClassDeclSyntax.self).memberBlock.members.first!.decl.cast(VariableDeclSyntax.self) + + assertSyntaxHighlightRanges( + inSyntaxTree: decl, + withHighlights: [ + (node: variableDecl, expectedNodeDescription: "\n var abc =\n 132") + ], + atLineNumber: 2, + usingLineSource: " var abc =\n", + matchExpectedRanges: [3..<12] + ) + } +}