Skip to content

Commit 127a71b

Browse files
committed
Unify AssertParse and XCTAssert[Single]Diagnostic
Update `AssertParse` to take the a list of expected diagnostic specifications that we are expected to produce.
1 parent 264dbd5 commit 127a71b

File tree

14 files changed

+719
-733
lines changed

14 files changed

+719
-733
lines changed

Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public enum StaticParserError: String, DiagnosticMessage {
9090
}
9191

9292
public enum StaticParserFixIt: String, FixItMessage {
93-
case moveThrowBeforeArrow = "Move 'throws' in before of '->'"
93+
case moveThrowBeforeArrow = "Move 'throws' before '->'"
9494

9595
public var message: String { self.rawValue }
9696

Sources/SwiftParser/SwiftParser.docc/FixingBugs.md

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,13 @@ Once you’ve written a test case (see below), set a breakpoint in `Parser.parse
1010

1111
1. Add a new test case in `SwiftParserTest` that looks like the following
1212
```swift
13-
try AssertParse({ $0.parseSourceFile() }) {
13+
AssertParse(
1414
"""
1515
<#your code that does not round trip#>
1616
"""
17-
}
17+
)
1818
```
1919
2. Run the test case, read the error message to figure out which part of the source file does not round-trip
20-
3. Optional: Reduce the test case even further by deleting more source code and calling into a specific production of the parser instead of `Parser.parseSourceFile`
2120

2221

2322
## Parse of Valid Source Failed
@@ -51,7 +50,7 @@ Unhelpful diagnostics can result from two reasons:
5150

5251
To distinguish these cases run the following command and look at the dumped syntax tree. Use your own judgment to decide whether this models the intended meaning of the source code reasonably well.
5352
```
54-
swift-parser-test print-tree /path/to/file/with/bad/diagnostic
53+
swift-parser-test print-tree /path/to/file/with/unhelpful/diagnostic.swift
5554
```
5655
5756
Fixing the first case where the parser does not recover according to the user’s intent is similar to [Parse of Valid Source Code Produced an Invalid Syntax Tree](#Parse-of-Valid-Source-Code-Produced-an-Invalid-Syntax-Tree). See <doc:SwiftParser/ParserRecovery> for documentation how parser recovery works and determine how to recover better from the invalid source code.
@@ -61,21 +60,19 @@ To add a new, more contextual diagnostic, perform the following steps.
6160
1. Add a test case to `DiagnosticTests.swift` like the following:
6261
6362
```swift
64-
let source = """
65-
<#your code that produces a bad diagnostic#>
66-
}
67-
"""
68-
let loop = withParser(source: source) {
69-
Syntax(raw: $0.parserSourceFile().raw)
70-
}
63+
AssertParse(
64+
"""
65+
<#your code that produced the unhelpful diagnostic#>
66+
""",
67+
diagnostics: [
68+
DiagnosticSpec(message: "<#expected diagnostic message#>")
69+
]
70+
)
7171
```
72-
2. Optional: Call a more specific production than `parseSourceFile` in the test case.
72+
2. Mark the location at which you expect the diagnostic to be produced with `#^DIAG^#`. If you expect multiple diagnostics to be produced, you can use multiple of these markers with different names and use these markers by passing a `locationMarker` to `DiagnosticSpec`.
7373
3. Determine which node encompasses all information that is necessary to produce the improved diagnostic – for example `FunctionSignatureSyntax` contains all information to diagnose if the `throws` keyword was written after the `->` instead of in front of it.
7474
4. If the diagnostic message you want to emit does not exist yet, add a case to <doc:SwiftParser/DiagnosticKind> for the new diagnostic.
7575
5. If the function does not already exist, write a new visit method on <doc:SwiftParser/ParseDiagnosticsGenerator>.
7676
6. In that visitation method, detect the pattern for which the improved diagnostic should be emitted and emit it using `diagnostics.append`.
7777
7. Mark the missing or garbage nodes that are covered by the new diagnostic as handled by adding their `SyntaxIdentifier`s to `handledNodes`.
78-
8. Assert that the new diagnostic is emitted by addding the following to your test case:
79-
```swift
80-
XCTAssertSingleDiagnostic(in: tree, line: <#expected line#>, column: <#expected column#>, expectedKind: .<#expected diagnostic kind#>)
81-
```
78+
8. If the diagnostic produces Fix-Its assert that they are generated by adding the Fix-It's message to the `DiagnosticSpec` with the `fixIt` parameter and asserting that applying the Fix-Its produces the correct source code by adding the `fixedSource` parameter to `AssertParse`.
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
//===--- LocationMarkers.swift --------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2022 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
/// Finds all marked ranges in the given text, see `Marker`.
14+
fileprivate func findMarkedRanges(text: String) -> [Marker] {
15+
var markers = [Marker]()
16+
while let marker = nextMarkedRange(text: text, from: markers.last?.range.upperBound ?? text.startIndex) {
17+
markers.append(marker)
18+
}
19+
return markers
20+
}
21+
22+
fileprivate func nextMarkedRange(text: String, from: String.Index) -> Marker? {
23+
guard let start = text.range(of: "#^", range: from ..< text.endIndex),
24+
let end = text.range(of: "^#", range: start.upperBound ..< text.endIndex) else {
25+
return nil
26+
}
27+
28+
let markerRange = start.lowerBound ..< end.upperBound
29+
let name = text[start.upperBound ..< end.lowerBound]
30+
31+
// Expand to the whole line if the line only contains the marker
32+
let lineRange = text.lineRange(for: start)
33+
if text[lineRange].trimmingCharacters(in: .whitespacesAndNewlines) == text[markerRange] {
34+
return Marker(name: name, range: lineRange)
35+
}
36+
return Marker(name: name, range: markerRange)
37+
}
38+
39+
fileprivate struct Marker {
40+
/// The name of the marker without the `#^` and `^#` markup.
41+
let name: Substring
42+
/// The range of the marker.
43+
///
44+
/// If the marker contains all the the non-whitepace characters on the line,
45+
/// this is the range of the entire line. Otherwise it's the range of the
46+
/// marker itself, including the `#^` and `^#` markup.
47+
let range: Range<String.Index>
48+
}
49+
50+
public func extractMarkers(_ markedText: String) -> (markers: [String: Int], textWithoutMarkers: String) {
51+
var text = ""
52+
var markers = [String: Int]()
53+
var lastIndex = markedText.startIndex
54+
for marker in findMarkedRanges(text: markedText) {
55+
text += markedText[lastIndex ..< marker.range.lowerBound]
56+
lastIndex = marker.range.upperBound
57+
58+
assert(markers[String(marker.name)] == nil, "Marker names must be unique")
59+
markers[String(marker.name)] = text.utf8.count
60+
}
61+
text += markedText[lastIndex ..< markedText.endIndex]
62+
63+
return (markers, text)
64+
}

Sources/_SwiftSyntaxTestSupport/Syntax+Assertions.swift

Lines changed: 1 addition & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -135,17 +135,7 @@ public struct SubtreeMatcher {
135135
private var actualTree: Syntax
136136

137137
public init(_ markedText: String, parse: (String) throws -> Syntax) throws {
138-
var text = ""
139-
var markers = [String: Int]()
140-
var lastIndex = markedText.startIndex
141-
for marker in findMarkedRanges(text: markedText) {
142-
text += markedText[lastIndex ..< marker.range.lowerBound]
143-
lastIndex = marker.range.upperBound
144-
145-
assert(markers[String(marker.name)] == nil, "Marker names must be unique")
146-
markers[String(marker.name)] = text.utf8.count
147-
}
148-
text += markedText[lastIndex ..< markedText.endIndex]
138+
let (markers, text) = extractMarkers(markedText)
149139

150140
self.markers = markers.isEmpty ? ["DEFAULT": 0] : markers
151141
self.actualTree = try parse(text)
@@ -195,43 +185,6 @@ public enum SubtreeError: Error, CustomStringConvertible {
195185
}
196186
}
197187

198-
/// Finds all marked ranges in the given text, see `Marker`.
199-
fileprivate func findMarkedRanges(text: String) -> [Marker] {
200-
var markers = [Marker]()
201-
while let marker = nextMarkedRange(text: text, from: markers.last?.range.upperBound ?? text.startIndex) {
202-
markers.append(marker)
203-
}
204-
return markers
205-
}
206-
207-
fileprivate func nextMarkedRange(text: String, from: String.Index) -> Marker? {
208-
guard let start = text.range(of: "#^", range: from ..< text.endIndex),
209-
let end = text.range(of: "^#", range: start.upperBound ..< text.endIndex) else {
210-
return nil
211-
}
212-
213-
let markerRange = start.lowerBound ..< end.upperBound
214-
let name = text[start.upperBound ..< end.lowerBound]
215-
216-
// Expand to the whole line if the line only contains the marker
217-
let lineRange = text.lineRange(for: start)
218-
if text[lineRange].trimmingCharacters(in: .whitespacesAndNewlines) == text[markerRange] {
219-
return Marker(name: name, range: lineRange)
220-
}
221-
return Marker(name: name, range: markerRange)
222-
}
223-
224-
fileprivate struct Marker {
225-
/// The name of the marker without the `#^` and `^#` markup.
226-
let name: Substring
227-
/// The range of the marker.
228-
///
229-
/// If the marker contains all the the non-whitepace characters on the line,
230-
/// this is the range of the entire line. Otherwise it's the range of the
231-
/// marker itself, including the `#^` and `^#` markup.
232-
let range: Range<String.Index>
233-
}
234-
235188
fileprivate class SyntaxTypeFinder: SyntaxAnyVisitor {
236189
private let offset: Int
237190
private let type: SyntaxProtocol.Type

Tests/SwiftParserTest/Assertions.swift

Lines changed: 136 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import XCTest
22
@_spi(RawSyntax) import SwiftSyntax
33
@_spi(Testing) @_spi(RawSyntax) import SwiftParser
4+
import SwiftDiagnostics
5+
import _SwiftSyntaxTestSupport
46

57
// MARK: Lexing Assertions
68

@@ -49,33 +51,147 @@ func AssertEqualTokens(_ actual: [Lexer.Lexeme], _ expected: [Lexer.Lexeme], fil
4951

5052
// MARK: Parsing Assertions
5153

54+
/// An abstract data structure to describe how a diagnostic produced by the parser should look like.
55+
struct DiagnosticSpec {
56+
/// The name of a maker (of the form `#^DIAG^#`) in the source code that marks the location where the diagnostis should be produced.
57+
let locationMarker: String
58+
/// If not `nil`, assert that the diagnostic has the given ID.
59+
let id: MessageID?
60+
/// If not `nil`, assert that the diagnostic has the given message.
61+
let message: String?
62+
/// If not `nil`, assert that the highlighted range has this content.
63+
let highlight: String?
64+
/// If not `nil`, assert that the diagnostic contains fix-its with these messages.
65+
/// Use the `fixedSource` parameter on `AssertParse` to check that applying the Fix-It yields the expected result.
66+
let fixIts: [String]?
67+
68+
init(locationMarker: String = "DIAG", id: MessageID? = nil, message: String?, highlight: String? = nil, fixIts: [String]? = nil) {
69+
self.locationMarker = locationMarker
70+
self.id = id
71+
self.message = message
72+
self.highlight = highlight
73+
self.fixIts = fixIts
74+
}
75+
}
76+
77+
class FixItApplier: SyntaxRewriter {
78+
var changes: [FixIt.Change]
79+
80+
init(diagnostics: [Diagnostic]) {
81+
self.changes = diagnostics.flatMap({ $0.fixIts }).flatMap({ $0.changes })
82+
}
83+
84+
public override func visitAny(_ node: Syntax) -> Syntax? {
85+
for change in changes {
86+
switch change {
87+
case .replace(oldNode: let oldNode, newNode: let newNode) where oldNode.id == node.id:
88+
return newNode
89+
default:
90+
break
91+
}
92+
}
93+
return nil
94+
}
95+
96+
/// Applies all Fix-Its in `diagnostics` to `tree` and returns the fixed syntax tree.
97+
public static func applyFixes<T: SyntaxProtocol>(in diagnostics: [Diagnostic], to tree: T) -> Syntax {
98+
let applier = FixItApplier(diagnostics: diagnostics)
99+
return applier.visit(Syntax(tree))
100+
}
101+
}
102+
103+
/// Assert that the diagnostic `diag` produced in `tree` matches `spec`,
104+
/// using `markerLocations` to translate markers to actual source locations.
105+
func AssertDiagnostic<T: SyntaxProtocol>(
106+
_ diag: Diagnostic,
107+
in tree: T,
108+
markerLocations: [String: Int],
109+
expected spec: DiagnosticSpec,
110+
file: StaticString = #filePath,
111+
line: UInt = #line
112+
) {
113+
if let markerLoc = markerLocations[spec.locationMarker] {
114+
let locationConverter = SourceLocationConverter(file: "/test.swift", source: tree.description)
115+
let actualLocation = diag.location(converter: locationConverter)
116+
let expectedLocation = locationConverter.location(for: AbsolutePosition(utf8Offset: markerLoc))
117+
if let actualLine = actualLocation.line,
118+
let actualColumn = actualLocation.column,
119+
let expectedLine = expectedLocation.line,
120+
let expectedColumn = expectedLocation.column {
121+
XCTAssertEqual(
122+
actualLine, expectedLine,
123+
"Expected diagnostic on line \(expectedLine) but got \(actualLine)",
124+
file: file, line: line
125+
)
126+
XCTAssertEqual(
127+
actualColumn, expectedColumn,
128+
"Expected diagnostic on column \(expectedColumn) but got \(actualColumn)",
129+
file: file, line: line
130+
)
131+
} else {
132+
XCTFail("Failed to resolve diagnostic location to line/column", file: file, line: line)
133+
}
134+
} else {
135+
XCTFail("Did not find marker #^\(spec.locationMarker)^# in the source code", file: file, line: line)
136+
}
137+
if let id = spec.id {
138+
XCTAssertEqual(diag.diagnosticID, id, file: file, line: line)
139+
}
140+
if let message = spec.message {
141+
XCTAssertEqual(diag.message, message, file: file, line: line)
142+
}
143+
if let highlight = spec.highlight {
144+
AssertStringsEqualWithDiff(diag.highlights.map(\.description).joined(), highlight, file: file, line: line)
145+
}
146+
if let fixIts = spec.fixIts {
147+
XCTAssertEqual(
148+
fixIts, diag.fixIts.map(\.message.message),
149+
"Fix-Its for diagnostic did not match expected Fix-Its",
150+
file: file, line: line
151+
)
152+
}
153+
}
154+
155+
/// Parse `markedSource` and perform the following assertions:
156+
/// - The parsed syntax tree should be printable back to the original source code (round-tripping)
157+
/// - Parsing produced the given `diagnostics` (`diagnostics = []` asserts that the parse was successful)
158+
/// - If `fixedSource` is not `nil`, assert that applying all fixes from the diagnostics produces `fixedSource`
159+
/// The source file can be marked with markers of the form `#^DIAG^#` to mark source locations that can be referred to by `diagnostics`.
160+
/// These markers are removed before parsing the source file.
161+
/// By default, `DiagnosticSpec` asserts that the diagnostics is produced at a location marked by `#^DIAG^#`.
162+
/// `parseSyntax` can be used to adjust the production that should be used as the entry point to parse the source code.
52163
func AssertParse<Node: RawSyntaxNodeProtocol>(
53-
_ parseSyntax: (inout Parser) -> Node,
54-
allowErrors: Bool = false,
164+
_ markedSource: String,
165+
_ parseSyntax: (inout Parser) -> Node = { $0.parseSourceFile() },
166+
diagnostics expectedDiagnostics: [DiagnosticSpec] = [],
167+
fixedSource expectedFixedSource: String? = nil,
55168
file: StaticString = #file,
56-
line: UInt = #line,
57-
_ source: () -> String
58-
) throws {
169+
line: UInt = #line
170+
) {
59171
// Verify the parser can round-trip the source
60-
let src = source()
61-
var source = src
62-
source.withUTF8 { buf in
172+
let (markerLocations, source) = extractMarkers(markedSource)
173+
var src = source
174+
src.withUTF8 { buf in
63175
var parser = Parser(buf)
64176
withExtendedLifetime(parser) {
65-
let parse = Syntax(raw: parseSyntax(&parser).raw)
66-
AssertStringsEqualWithDiff("\(parse)", src, additionalInfo: """
177+
let tree = Syntax(raw: parseSyntax(&parser).raw)
178+
AssertStringsEqualWithDiff("\(tree)", source, additionalInfo: """
179+
Source failed to round-trip.
180+
67181
Actual syntax tree:
68-
\(parse.recursiveDescription)
182+
\(tree.recursiveDescription)
183+
""", file: file, line: line)
184+
let diags = ParseDiagnosticsGenerator.diagnostics(for: tree)
185+
XCTAssertEqual(diags.count, expectedDiagnostics.count, """
186+
Expected \(expectedDiagnostics.count) diagnostics but received \(diags.count):
187+
\(diags.map(\.debugDescription).joined(separator: "\n"))
69188
""", file: file, line: line)
70-
if !allowErrors {
71-
let diagnostics = ParseDiagnosticsGenerator.diagnostics(for: Syntax(raw: parse.raw))
72-
XCTAssertEqual(
73-
diagnostics.count, 0,
74-
"""
75-
Received the following diagnostics while parsing the source code:
76-
\(diagnostics)
77-
""",
78-
file: file, line: line)
189+
for (diag, expectedDiag) in zip(diags, expectedDiagnostics) {
190+
AssertDiagnostic(diag, in: tree, markerLocations: markerLocations, expected: expectedDiag, file: file, line: line)
191+
}
192+
if let expectedFixedSource = expectedFixedSource {
193+
let fixedSource = FixItApplier.applyFixes(in: diags, to: tree).description
194+
AssertStringsEqualWithDiff(fixedSource, expectedFixedSource, file: file, line: line)
79195
}
80196
}
81197
}

0 commit comments

Comments
 (0)