Skip to content

Add a public function that can be used to test macro expansions #1550

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 2 commits into from
Apr 24, 2023
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
10 changes: 9 additions & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ let package = Package(
.library(name: "SwiftSyntax", type: .static, targets: ["SwiftSyntax"]),
.library(name: "SwiftSyntaxBuilder", type: .static, targets: ["SwiftSyntaxBuilder"]),
.library(name: "SwiftSyntaxMacros", type: .static, targets: ["SwiftSyntaxMacros"]),
.library(name: "SwiftSyntaxMacrosTestSupport", type: .static, targets: ["SwiftSyntaxMacrosTestSupport"]),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add this to the CMake build as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think there’s any reason why this should be in the toolchain. So no need to add it to the CMake build…

],
targets: [
// MARK: - Internal helper targets
Expand Down Expand Up @@ -178,7 +179,14 @@ let package = Package(

.testTarget(
name: "SwiftSyntaxMacrosTest",
dependencies: ["_SwiftSyntaxTestSupport", "SwiftDiagnostics", "SwiftOperators", "SwiftParser", "SwiftSyntaxBuilder", "SwiftSyntaxMacros"]
dependencies: ["_SwiftSyntaxTestSupport", "SwiftDiagnostics", "SwiftOperators", "SwiftParser", "SwiftSyntaxBuilder", "SwiftSyntaxMacros", "SwiftSyntaxMacrosTestSupport"]
),

// MARK: SwiftSyntaxMacrosTestSupport

.target(
name: "SwiftSyntaxMacrosTestSupport",
dependencies: ["_SwiftSyntaxTestSupport", "SwiftDiagnostics", "SwiftParser", "SwiftSyntaxMacros"]
),

// MARK: SwiftParser
Expand Down
11 changes: 10 additions & 1 deletion Sources/SwiftBasicFormat/BasicFormat.swift
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ open class BasicFormat: SyntaxRewriter {
(.regexLiteralPattern, _),
(.regexSlash, .extendedRegexDelimiter), // closing extended regex delimiter should never be separate by a space
(.rightAngle, .leftParen), // func foo<T>(x: T)
(.rightBrace, .leftParen), // { return 1 }()
(.rightParen, .leftParen), // returnsClosure()()
(.rightParen, .period), // foo().bar
(.rightSquareBracket, .period), // myArray[1].someProperty
Expand Down Expand Up @@ -310,6 +311,14 @@ open class BasicFormat: SyntaxRewriter {
return false
}()

lazy var nextTokenWillStartWithWhitespace: Bool = {
guard let nextToken = nextToken else {
return false
}
return nextToken.leadingTrivia.startsWithWhitespace
|| (requiresLeadingNewline(nextToken) && isMutable(nextToken))
}()

lazy var nextTokenWillStartWithNewline: Bool = {
guard let nextToken = nextToken else {
return false
Expand Down Expand Up @@ -359,7 +368,7 @@ open class BasicFormat: SyntaxRewriter {
// because newlines should be preferred to spaces as a whitespace
if requiresWhitespace(between: token, and: nextToken)
&& !trailingTrivia.endsWithWhitespace
&& !nextTokenWillStartWithNewline
&& !nextTokenWillStartWithWhitespace
{
trailingTrivia += .space
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftSyntaxMacros/MacroSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
}
)
} catch {
context.addDiagnostics(from: error, node: node)
context.addDiagnostics(from: error, node: declExpansion)
}

continue
Expand Down
305 changes: 305 additions & 0 deletions Sources/SwiftSyntaxMacrosTestSupport/Assertions.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,305 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2023 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
//
//===----------------------------------------------------------------------===//

import _SwiftSyntaxTestSupport
import SwiftBasicFormat
import SwiftDiagnostics
import SwiftParser
import SwiftSyntax
import SwiftSyntaxMacros
import XCTest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... or is this why we don't have it in the CMake build?


private extension String {
// This implementation is really slow; to use it outside a test it should be optimized.
func trimmingTrailingWhitespace() -> String {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this if we've fixed BasicFormat?

return self.replacingOccurrences(of: "[ ]+\\n", with: "\n", options: .regularExpression)
}
}

// MARK: - Note

/// Describes a diagnostic note that tests expect to be created by a macro expansion.
public struct NoteSpec {
/// The expected message of the note
public let message: String

/// The line to which the note is expected to point
public let line: Int

/// The column to which the note is expected to point
public let column: Int

/// The file and line at which this `NoteSpec` was created, so that assertion failures can be reported at its location.
internal let originatorFile: StaticString
internal let originatorLine: UInt

/// Creates a new `NoteSpec` that describes a note tests are expecting to be generated by a macro expansion.
///
/// - Parameters:
/// - message: The expected message of the note
/// - line: The line to which the note is expected to point
/// - column: The column to which the note is expected to point
/// - originatorFile: The file at which this `NoteSpec` was created, so that assertion failures can be reported at its location.
/// - originatorLine: The line at which this `NoteSpec` was created, so that assertion failures can be reported at its location.
public init(
message: String,
line: Int,
column: Int,
originatorFile: StaticString = #file,
originatorLine: UInt = #line
) {
self.message = message
self.line = line
self.column = column
self.originatorFile = originatorFile
self.originatorLine = originatorLine
}
}

func assertNote<T: SyntaxProtocol>(
_ note: Note,
in tree: T,
expected spec: NoteSpec
) {
assertStringsEqualWithDiff(note.message, spec.message, "message of note does not match", file: spec.originatorFile, line: spec.originatorLine)
let location = note.location(converter: SourceLocationConverter(file: "", source: tree.description))
XCTAssertEqual(location.line, spec.line, "line of note does not match", file: spec.originatorFile, line: spec.originatorLine)
XCTAssertEqual(location.column, spec.column, "column of note does not match", file: spec.originatorFile, line: spec.originatorLine)
}

// MARK: - Fix-It

/// Describes a Fix-It that tests expect to be created by a macro expansion.
///
/// Currently, it only compares the message of the Fix-It. In the future, it might
/// also compare the expected changes that should be performed by the Fix-It.
public struct FixItSpec {
/// The expected message of the Fix-It
public let message: String

/// The file and line at which this `NoteSpec` was created, so that assertion failures can be reported at its location.
internal let originatorFile: StaticString
internal let originatorLine: UInt

/// Creates a new `FixItSpec` that describes a Fix-It tests are expecting to be generated by a macro expansion.
///
/// - Parameters:
/// - message: The expected message of the note
/// - originatorFile: The file at which this `NoteSpec` was created, so that assertion failures can be reported at its location.
/// - originatorLine: The line at which this `NoteSpec` was created, so that assertion failures can be reported at its location.
public init(
message: String,
originatorFile: StaticString = #file,
originatorLine: UInt = #line
) {
self.message = message
self.originatorFile = originatorFile
self.originatorLine = originatorLine
}
}

func assertFixIt(
_ fixIt: FixIt,
expected spec: FixItSpec
) {
assertStringsEqualWithDiff(fixIt.message.message, spec.message, "message of Fix-It does not match", file: spec.originatorFile, line: spec.originatorLine)
}

// MARK: - Diagnostic

/// Describes a diagnostic that tests expect to be created by a macro expansion.
public struct DiagnosticSpec {
/// If not `nil`, the ID, which the diagnostic is expected to have.
public let id: MessageID?

/// The expected message of the diagnostic
public let message: String

/// The line to which the diagnostic is expected to point
public let line: Int

/// The column to which the diagnostic is expected to point
public let column: Int

/// The expected severity of the diagnostic
public let severity: DiagnosticSeverity

/// If not `nil`, the text the diagnostic is expected to highlight
public let highlight: String?

/// The notes that are expected to be attached to the diagnostic
public let notes: [NoteSpec]

/// The messages of the Fix-Its the diagnostic is expected to produce
public let fixIts: [FixItSpec]

/// The file and line at which this `NoteSpec` was created, so that assertion failures can be reported at its location.
internal let originatorFile: StaticString
internal let originatorLine: UInt

/// Creates a new `DiagnosticSpec` that describes a diagnsotic tests are expecting to be generated by a macro expansion.
///
/// - Parameters:
/// - id: If not `nil`, the ID, which the diagnostic is expected to have.
/// - message: The expected message of the diagnostic
/// - line: The line to which the diagnostic is expected to point
/// - column: The column to which the diagnostic is expected to point
/// - severity: The expected severity of the diagnostic
/// - highlight: If not `nil`, the text the diagnostic is expected to highlight
/// - notes: The notes that are expected to be attached to the diagnostic
/// - fixIts: The messages of the Fix-Its the diagnostic is expected to produce
/// - originatorFile: The file at which this `NoteSpec` was created, so that assertion failures can be reported at its location.
/// - originatorLine: The line at which this `NoteSpec` was created, so that assertion failures can be reported at its location.
public init(
id: MessageID? = nil,
message: String,
line: Int,
column: Int,
severity: DiagnosticSeverity = .error,
highlight: String? = nil,
notes: [NoteSpec] = [],
fixIts: [FixItSpec] = [],
originatorFile: StaticString = #file,
originatorLine: UInt = #line
) {
self.id = id
self.message = message
self.line = line
self.column = column
self.severity = severity
self.highlight = highlight
self.notes = notes
self.fixIts = fixIts
self.originatorFile = originatorFile
self.originatorLine = originatorLine
}
}

func assertDiagnostic<T: SyntaxProtocol>(
_ diag: Diagnostic,
in tree: T,
expected spec: DiagnosticSpec
) {
if let id = spec.id {
XCTAssertEqual(diag.diagnosticID, id, "diagnostic ID does not match", file: spec.originatorFile, line: spec.originatorLine)
}
assertStringsEqualWithDiff(diag.message, spec.message, "message does not match", file: spec.originatorFile, line: spec.originatorLine)
let location = diag.location(converter: SourceLocationConverter(file: "", source: tree.description))
XCTAssertEqual(location.line, spec.line, "line does not match", file: spec.originatorFile, line: spec.originatorLine)
XCTAssertEqual(location.column, spec.column, "column does not match", file: spec.originatorFile, line: spec.originatorLine)

XCTAssertEqual(spec.severity, diag.diagMessage.severity, "severity does not match", file: spec.originatorFile, line: spec.originatorLine)

if let highlight = spec.highlight {
var highlightedCode = ""
highlightedCode.append(diag.highlights.first?.with(\.leadingTrivia, []).description ?? "")
for highlight in diag.highlights.dropFirst().dropLast() {
highlightedCode.append(highlight.description)
}
if diag.highlights.count > 1 {
highlightedCode.append(diag.highlights.last?.with(\.trailingTrivia, []).description ?? "")
}

assertStringsEqualWithDiff(
highlightedCode.trimmingTrailingWhitespace(),
highlight.trimmingTrailingWhitespace(),
"highlight does not match",
file: spec.originatorFile,
line: spec.originatorLine
)
}
if diag.notes.count != spec.notes.count {
XCTFail(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we output both notes, ie. actual and expected? Same with fix-it's below.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think it’s worth outputting the expected notes because the diagnostic will point right into the source that defines them. We used to output the expected diagnostics if the count did not match and I found that verbose without giving any value.

"""
Expected \(spec.notes.count) notes but received \(diag.notes.count):
\(diag.notes.map(\.debugDescription).joined(separator: "\n"))
""",
file: spec.originatorFile,
line: spec.originatorLine
)
} else {
for (note, expectedNote) in zip(diag.notes, spec.notes) {
assertNote(note, in: tree, expected: expectedNote)
}
}
if diag.fixIts.count != spec.fixIts.count {
XCTFail(
"""
Expected \(spec.fixIts.count) Fix-Its but received \(diag.fixIts.count):
\(diag.fixIts.map(\.message.message).joined(separator: "\n"))
""",
file: spec.originatorFile,
line: spec.originatorLine
)
} else {
for (fixIt, expectedFixIt) in zip(diag.fixIts, spec.fixIts) {
assertFixIt(fixIt, expected: expectedFixIt)
}
}
}

/// Assert that expanding the given macros in the original source produces
/// the given expanded source code.
///
/// - Parameters:
/// - originalSource: The original source code, which is expected to contain
/// macros in various places (e.g., `#stringify(x + y)`).
/// - expandedSource: The source code that we expect to see after performing
/// macro expansion on the original source.
/// - diagnostics:
/// - macros: The macros that should be expanded, provided as a dictionary
/// mapping macro names (e.g., `"stringify"`) to implementation types
/// (e.g., `StringifyMacro.self`).
/// - testModuleName: The name of the test module to use.
/// - testFileName: The name of the test file name to use.
Comment on lines +262 to +263
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it useful to expose these? It doesn't actually matter for anything right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use them in diagnostic messages, right?

public func assertMacroExpansion(
_ originalSource: String,
expandedSource: String,
diagnostics: [DiagnosticSpec] = [],
macros: [String: Macro.Type],
testModuleName: String = "TestModule",
testFileName: String = "test.swift",
indentationWidth: Trivia = .spaces(4),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your thoughts on indentWidth vs indentationWidth?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t have strong feelings. Do you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Define "strong". I prefer indentWidth because it's shorter. But 🤷

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just checked. SwiftSyntax general prefers indentation over indent, so I’d prefer to stick with indentation for consistency.

file: StaticString = #file,
line: UInt = #line
) {
// Parse the original source file.
let origSourceFile = Parser.parse(source: originalSource)

// Expand all macros in the source.
let context = BasicMacroExpansionContext(
sourceFiles: [origSourceFile: .init(moduleName: testModuleName, fullFilePath: testFileName)]
)
let expandedSourceFile = origSourceFile.expand(macros: macros, in: context).formatted(using: BasicFormat(indentationWidth: indentationWidth))

assertStringsEqualWithDiff(
expandedSourceFile.description.trimmingTrailingWhitespace(),
expandedSource.trimmingTrailingWhitespace(),
file: file,
line: line
)

if context.diagnostics.count != diagnostics.count {
XCTFail(
"""
Expected \(diagnostics.count) diagnostics but received \(context.diagnostics.count):
\(context.diagnostics.map(\.debugDescription).joined(separator: "\n"))
""",
file: file,
line: line
)
} else {
for (actualDiag, expectedDiag) in zip(context.diagnostics, diagnostics) {
assertDiagnostic(actualDiag, in: origSourceFile, expected: expectedDiag)
}
}
}
Loading