Skip to content

Unify all parser tests to use an enhanced version AssertParse #641

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 6 commits into from
Aug 29, 2022
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 @@ -90,7 +90,7 @@ public enum StaticParserError: String, DiagnosticMessage {
}

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

public var message: String { self.rawValue }

Expand Down
49 changes: 20 additions & 29 deletions Sources/SwiftParser/SwiftParser.docc/FixingBugs.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@ Once you’ve written a test case (see below), set a breakpoint in `Parser.parse

1. Add a new test case in `SwiftParserTest` that looks like the following
```swift
try AssertParse({ $0.parseSourceFile() }) {
AssertParse(
"""
<#your code that does not round trip#>
"""
}
)
```
2. Run the test case, read the error message to figure out which part of the source file does not round-trip
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`


## Parse of Valid Source Failed
Expand All @@ -28,20 +27,14 @@ Diagnostics are produced when the parsed syntax tree contains missing or unexpec

1. Add a test case in `SwiftParserTest` that looks like the following
```swift
let source = """
<#your code that produces an invalid syntax tree#>
"""

let tree = withParser(source: source) {
Syntax(raw: $0.parseSourceFile().raw)
}
XCTAssertHasSubstructure(
tree,
<#create a syntax node that you expect the tree to have#>
AssertParse(
"""
<#your code that produces an invalid syntax tree#>
""",
substructure: <#create a syntax node that you expect the tree to have#>
)
```
2. 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`
3. Run the test case and navigate the debugger to the place that produced the invalid syntax node.
2. Run the test case and navigate the debugger to the place that produced the invalid syntax node.

## Unhelpful Diagnostic Produced

Expand All @@ -51,31 +44,29 @@ Unhelpful diagnostics can result from two reasons:

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.
```
swift-parser-test print-tree /path/to/file/with/bad/diagnostic
swift-parser-test print-tree /path/to/file/with/unhelpful/diagnostic.swift
```

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.

To add a new, more contextual diagnostic, perform the following steps.

1. Add a test case to `DiagnosticTests.swift` like the following:
1. Add a test case in `SwiftParserTest` that looks like the following

```swift
let source = """
<#your code that produces a bad diagnostic#>
}
"""
let loop = withParser(source: source) {
Syntax(raw: $0.parserSourceFile().raw)
}
AssertParse(
"""
<#your code that produced the unhelpful diagnostic#>
""",
diagnostics: [
DiagnosticSpec(message: "<#expected diagnostic message#>")
]
)
```
2. Optional: Call a more specific production than `parseSourceFile` in the test case.
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`.
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.
4. If the diagnostic message you want to emit does not exist yet, add a case to <doc:SwiftParser/DiagnosticKind> for the new diagnostic.
5. If the function does not already exist, write a new visit method on <doc:SwiftParser/ParseDiagnosticsGenerator>.
6. In that visitation method, detect the pattern for which the improved diagnostic should be emitted and emit it using `diagnostics.append`.
7. Mark the missing or garbage nodes that are covered by the new diagnostic as handled by adding their `SyntaxIdentifier`s to `handledNodes`.
8. Assert that the new diagnostic is emitted by addding the following to your test case:
```swift
XCTAssertSingleDiagnostic(in: tree, line: <#expected line#>, column: <#expected column#>, expectedKind: .<#expected diagnostic kind#>)
```
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`.
64 changes: 64 additions & 0 deletions Sources/_SwiftSyntaxTestSupport/LocationMarkers.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
//===--- LocationMarkers.swift --------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2022 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

/// Finds all marked ranges in the given text, see `Marker`.
fileprivate func findMarkedRanges(text: String) -> [Marker] {
var markers = [Marker]()
while let marker = nextMarkedRange(text: text, from: markers.last?.range.upperBound ?? text.startIndex) {
markers.append(marker)
}
return markers
}

fileprivate func nextMarkedRange(text: String, from: String.Index) -> Marker? {
guard let start = text.range(of: "#^", range: from ..< text.endIndex),
let end = text.range(of: "^#", range: start.upperBound ..< text.endIndex) else {
return nil
}

let markerRange = start.lowerBound ..< end.upperBound
let name = text[start.upperBound ..< end.lowerBound]

// Expand to the whole line if the line only contains the marker
let lineRange = text.lineRange(for: start)
if text[lineRange].trimmingCharacters(in: .whitespacesAndNewlines) == text[markerRange] {
return Marker(name: name, range: lineRange)
}
return Marker(name: name, range: markerRange)
}

fileprivate struct Marker {
/// The name of the marker without the `#^` and `^#` markup.
let name: Substring
/// The range of the marker.
///
/// If the marker contains all the the non-whitepace characters on the line,
/// this is the range of the entire line. Otherwise it's the range of the
/// marker itself, including the `#^` and `^#` markup.
let range: Range<String.Index>
}

public func extractMarkers(_ markedText: String) -> (markers: [String: Int], textWithoutMarkers: String) {
var text = ""
var markers = [String: Int]()
var lastIndex = markedText.startIndex
for marker in findMarkedRanges(text: markedText) {
text += markedText[lastIndex ..< marker.range.lowerBound]
lastIndex = marker.range.upperBound

assert(markers[String(marker.name)] == nil, "Marker names must be unique")
markers[String(marker.name)] = text.utf8.count
}
text += markedText[lastIndex ..< markedText.endIndex]

return (markers, text)
}
111 changes: 5 additions & 106 deletions Sources/_SwiftSyntaxTestSupport/Syntax+Assertions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,60 +33,6 @@ public func XCTAssertNextIsNil<Iterator: IteratorProtocol>(_ iterator: inout Ite
XCTAssertNil(iterator.next())
}

/// Verifies that the tree parsed from `actual` has the same structure as
/// `expected` when parsed with `parse`, ie. it has the same structure and
/// optionally the same trivia (if `includeTrivia` is set).
public func XCTAssertSameStructure(
_ actual: String,
parse: (String) throws -> Syntax,
_ expected: Syntax,
includeTrivia: Bool = false,
file: StaticString = #filePath, line: UInt = #line
) throws {
let actualTree = try parse(actual)
XCTAssertSameStructure(actualTree, expected, includeTrivia: includeTrivia, file: file, line: line)
}

/// Verifies that two trees are equivalent, ie. they have the same structure
/// and optionally the same trivia if `includeTrivia` is set.
public func XCTAssertSameStructure<ActualTree, ExpectedTree>(
_ actual: ActualTree,
_ expected: ExpectedTree,
includeTrivia: Bool = false,
file: StaticString = #filePath, line: UInt = #line
)
where ActualTree: SyntaxProtocol, ExpectedTree: SyntaxProtocol
{
let diff = actual.findFirstDifference(baseline: expected, includeTrivia: includeTrivia)
XCTAssertNil(diff, diff!.debugDescription, file: file, line: line)
}

/// See `SubtreeMatcher.assertSameStructure`.
public func XCTAssertHasSubstructure<ExpectedTree: SyntaxProtocol>(
_ markedText: String,
parse: (String) throws -> Syntax,
afterMarker: String? = nil,
_ expected: ExpectedTree,
includeTrivia: Bool = false,
file: StaticString = #filePath, line: UInt = #line
) throws {
let subtreeMatcher = try SubtreeMatcher(markedText, parse: parse)
try subtreeMatcher.assertSameStructure(afterMarker: afterMarker, Syntax(expected), file: file, line: line)
}

/// See `SubtreeMatcher.assertSameStructure`.
public func XCTAssertHasSubstructure<ActualTree, ExpectedTree>(
_ actualTree: ActualTree,
_ expected: ExpectedTree,
includeTrivia: Bool = false,
file: StaticString = #filePath, line: UInt = #line
) throws
where ActualTree: SyntaxProtocol, ExpectedTree: SyntaxProtocol
{
let subtreeMatcher = SubtreeMatcher(Syntax(actualTree))
try subtreeMatcher.assertSameStructure(Syntax(expected), file: file, line: line)
}

/// Allows matching a subtrees of the given `markedText` against
/// `baseline`/`expected` trees, where a combination of markers and the type
/// of the `expected` tree is used to first find the subtree to match. Note
Expand Down Expand Up @@ -135,24 +81,14 @@ public struct SubtreeMatcher {
private var actualTree: Syntax

public init(_ markedText: String, parse: (String) throws -> Syntax) throws {
var text = ""
var markers = [String: Int]()
var lastIndex = markedText.startIndex
for marker in findMarkedRanges(text: markedText) {
text += markedText[lastIndex ..< marker.range.lowerBound]
lastIndex = marker.range.upperBound

assert(markers[String(marker.name)] == nil, "Marker names must be unique")
markers[String(marker.name)] = text.utf8.count
}
text += markedText[lastIndex ..< markedText.endIndex]
let (markers, text) = extractMarkers(markedText)

self.markers = markers.isEmpty ? ["DEFAULT": 0] : markers
self.actualTree = try parse(text)
}

public init(_ actualTree: Syntax) {
self.markers = ["DEFAULT": 0]
public init(_ actualTree: Syntax, markers: [String: Int]) {
self.markers = markers.isEmpty ? ["DEFAULT": 0] : markers
self.actualTree = actualTree
}

Expand All @@ -172,8 +108,8 @@ public struct SubtreeMatcher {
return subtree.findFirstDifference(baseline: baseline, includeTrivia: includeTrivia)
}

/// Same as `XCTAssertSameStructure`, but uses the subtree found from parsing
/// the text passed into `init(markedText:)` as the `actual` tree.
/// Verifies that the the subtree found from parsing the text passed into
/// `init(markedText:)` has the same structure as `expected`.
public func assertSameStructure(afterMarker: String? = nil, _ expected: Syntax, includeTrivia: Bool = false,
file: StaticString = #filePath, line: UInt = #line) throws {
let diff = try findFirstDifference(afterMarker: afterMarker, baseline: expected, includeTrivia: includeTrivia)
Expand All @@ -195,43 +131,6 @@ public enum SubtreeError: Error, CustomStringConvertible {
}
}

/// Finds all marked ranges in the given text, see `Marker`.
fileprivate func findMarkedRanges(text: String) -> [Marker] {
var markers = [Marker]()
while let marker = nextMarkedRange(text: text, from: markers.last?.range.upperBound ?? text.startIndex) {
markers.append(marker)
}
return markers
}

fileprivate func nextMarkedRange(text: String, from: String.Index) -> Marker? {
guard let start = text.range(of: "#^", range: from ..< text.endIndex),
let end = text.range(of: "^#", range: start.upperBound ..< text.endIndex) else {
return nil
}

let markerRange = start.lowerBound ..< end.upperBound
let name = text[start.upperBound ..< end.lowerBound]

// Expand to the whole line if the line only contains the marker
let lineRange = text.lineRange(for: start)
if text[lineRange].trimmingCharacters(in: .whitespacesAndNewlines) == text[markerRange] {
return Marker(name: name, range: lineRange)
}
return Marker(name: name, range: markerRange)
}

fileprivate struct Marker {
/// The name of the marker without the `#^` and `^#` markup.
let name: Substring
/// The range of the marker.
///
/// If the marker contains all the the non-whitepace characters on the line,
/// this is the range of the entire line. Otherwise it's the range of the
/// marker itself, including the `#^` and `^#` markup.
let range: Range<String.Index>
}

fileprivate class SyntaxTypeFinder: SyntaxAnyVisitor {
private let offset: Int
private let type: SyntaxProtocol.Type
Expand Down
Loading