From 7412a73e20d1b89b51433dede23f403319960e6c Mon Sep 17 00:00:00 2001 From: Rauhul Varma <rauhul@apple.com> Date: Mon, 18 Sep 2023 11:37:10 -0700 Subject: [PATCH 1/2] Improve assertMacroExpansion highlight verification Adds a new initializer to DiagnosticSpec which takes an optional array of highlights instead of a single optional highlight string. Updates existing initializer to map the string into an array of one. Updates assertMacroExpansion to check each highlight in the diagnostic spec against the actual produced diagnostic individually. Additionally changes the behavior to drop leading and trailing trivial when performing the highlight comparison to better match the Swift compiler's behavior. --- Package.swift | 5 + Release Notes/510.md | 6 ++ .../Assertions.swift | 84 +++++++++++---- .../AssertionsTests.swift | 101 ++++++++++++++++++ 4 files changed, 174 insertions(+), 22 deletions(-) create mode 100644 Tests/SwiftSyntaxMacrosTestSupportTests/AssertionsTests.swift diff --git a/Package.swift b/Package.swift index 01ea46869eb..401149007a6 100644 --- a/Package.swift +++ b/Package.swift @@ -246,6 +246,11 @@ let package = Package( dependencies: ["_SwiftSyntaxTestSupport", "SwiftDiagnostics", "SwiftParser", "SwiftSyntaxMacros", "SwiftSyntaxMacroExpansion"] ), + .testTarget( + name: "SwiftSyntaxMacrosTestSupportTests", + dependencies: ["SwiftDiagnostics", "SwiftSyntax", "SwiftSyntaxMacros", "SwiftSyntaxMacrosTestSupport"] + ), + // MARK: SwiftParser .target( diff --git a/Release Notes/510.md b/Release Notes/510.md index 8ae0439576b..24400d550a5 100644 --- a/Release Notes/510.md +++ b/Release Notes/510.md @@ -47,6 +47,12 @@ - Issue: https://github.com/apple/swift-syntax/issues/2261 - Pull Request: https://github.com/apple/swift-syntax/pull/2264 +- `DiagnosticSpec.highlight` replaced by `highlights` + - Description: The use of a single string `highlight` prevented users from asserting that a macro highlighted exactly the expected set of syntax nodes. Use of `DiagnosticSpec.init(...highlight:...)` is deprecated and forwards to `DiagnosticSpec.init(...highlights:...)`. Migrating from `highlight` to `highlights` is straightforward; any uses of `DiagnosticSpec.init` which do not specify a `highlight` do not need to change, otherwise: + - If the diagnostic highlights a single node, the `highlight` string should be replaced with a single element array containing the same string without any trailing trivia, e.g., `highlight: "let "` -> `highlights: ["let"]`. + - If the diagnostic highlights multiple nodes, the `highlight` string should be replaced with an array containing an element for each highlighted node, e.g., `highlight: "struct {}"` -> `highlights: ["struct", "{}"]`. + - Pull Request: https://github.com/apple/swift-syntax/pull/2213 + ## Template - *Affected API or two word description* diff --git a/Sources/SwiftSyntaxMacrosTestSupport/Assertions.swift b/Sources/SwiftSyntaxMacrosTestSupport/Assertions.swift index 0a347c36238..5a3fcdbd2c8 100644 --- a/Sources/SwiftSyntaxMacrosTestSupport/Assertions.swift +++ b/Sources/SwiftSyntaxMacrosTestSupport/Assertions.swift @@ -128,8 +128,8 @@ public struct DiagnosticSpec { /// 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? + /// If not `nil`, the text fragments the diagnostic is expected to highlight + public let highlights: [String]? /// The notes that are expected to be attached to the diagnostic public let notes: [NoteSpec] @@ -141,7 +141,7 @@ public struct DiagnosticSpec { 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. + /// Creates a new ``DiagnosticSpec`` that describes a diagnostic tests are expecting to be generated by a macro expansion. /// /// - Parameters: /// - id: If not `nil`, the ID, which the diagnostic is expected to have. @@ -149,7 +149,7 @@ public struct DiagnosticSpec { /// - 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 + /// - highlights: If not empty, the text fragments 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. @@ -160,7 +160,7 @@ public struct DiagnosticSpec { line: Int, column: Int, severity: DiagnosticSeverity = .error, - highlight: String? = nil, + highlights: [String]? = nil, notes: [NoteSpec] = [], fixIts: [FixItSpec] = [], originatorFile: StaticString = #file, @@ -171,7 +171,7 @@ public struct DiagnosticSpec { self.line = line self.column = column self.severity = severity - self.highlight = highlight + self.highlights = highlights self.notes = notes self.fixIts = fixIts self.originatorFile = originatorFile @@ -179,6 +179,42 @@ public struct DiagnosticSpec { } } +extension DiagnosticSpec { + @available(*, deprecated, message: "Use highlights instead") + public var highlight: String? { + guard let highlights else { + return nil + } + return highlights.joined(separator: " ") + } + + @_disfavoredOverload + @available(*, deprecated, message: "Use init(id:message:line:column:severity:highlights:notes:fixIts:originatorFile:originatorLine:) instead") + 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.init( + id: id, + message: message, + line: line, + column: column, + severity: severity, + highlights: highlight.map { [$0] }, + notes: notes, + fixIts: fixIts + ) + } +} + func assertDiagnostic( _ diag: Diagnostic, in expansionContext: BasicMacroExpansionContext, @@ -194,23 +230,27 @@ func assertDiagnostic( 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 ?? "") + if let highlights = spec.highlights { + if diag.highlights.count != highlights.count { + XCTFail( + """ + Expected \(highlights.count) highlights but received \(diag.highlights.count): + \(diag.highlights.map(\.trimmedDescription).joined(separator: "\n")) + """, + file: spec.originatorFile, + line: spec.originatorLine + ) + } else { + for (actual, expected) in zip(diag.highlights, highlights) { + assertStringsEqualWithDiff( + actual.trimmedDescription, + expected, + "highlight does not match", + file: spec.originatorFile, + line: spec.originatorLine + ) + } } - - assertStringsEqualWithDiff( - highlightedCode, - highlight, - "highlight does not match", - file: spec.originatorFile, - line: spec.originatorLine - ) } if diag.notes.count != spec.notes.count { XCTFail( diff --git a/Tests/SwiftSyntaxMacrosTestSupportTests/AssertionsTests.swift b/Tests/SwiftSyntaxMacrosTestSupportTests/AssertionsTests.swift new file mode 100644 index 00000000000..a7059f5d6e8 --- /dev/null +++ b/Tests/SwiftSyntaxMacrosTestSupportTests/AssertionsTests.swift @@ -0,0 +1,101 @@ +//===----------------------------------------------------------------------===// +// +// 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 SwiftDiagnostics +import SwiftSyntax +import SwiftSyntaxMacros +import SwiftSyntaxMacrosTestSupport +import XCTest + +final class AssertionsTests: XCTestCase { + struct OnlyStruct: DiagnosticMessage { + var message = "'@NoStruct' cannot be applied to struct types" + var diagnosticID = MessageID(domain: "\(AssertionsTests.self)", id: "\(OnlyStruct.self)") + var severity = DiagnosticSeverity.error + } + + struct NoStruct: MemberMacro { + static func expansion( + of node: AttributeSyntax, + providingMembersOf declaration: some DeclGroupSyntax, + conformingTo protocols: [TypeSyntax], + in context: some MacroExpansionContext + ) throws -> [DeclSyntax] { + if let structDecl = declaration.as(StructDeclSyntax.self) { + context.diagnose( + .init( + node: structDecl.structKeyword, + message: OnlyStruct() + ) + ) + } + return [] + } + } + + struct NoStructMultiHighlight: MemberMacro { + static func expansion( + of node: AttributeSyntax, + providingMembersOf declaration: some DeclGroupSyntax, + conformingTo protocols: [TypeSyntax], + in context: some MacroExpansionContext + ) throws -> [DeclSyntax] { + if let structDecl = declaration.as(StructDeclSyntax.self) { + context.diagnose( + .init( + node: structDecl.structKeyword, + message: OnlyStruct(), + highlights: [ + Syntax(structDecl.structKeyword), + Syntax(structDecl.name) + ] + ) + ) + } + return [] + } + } + + func testAssertMacroExpansionIgnoresHighlightMatchingIfNil() { + assertMacroExpansion( + "@NoStruct struct S { }", + expandedSource: "struct S { }", + diagnostics: [ + .init(message: OnlyStruct().message, line: 1, column: 11) + ], + macros: ["NoStruct": NoStruct.self] + ) + } + + @available(*, deprecated) + func testAssertMacroExpansionMatchesSingleStringHighlight() { + assertMacroExpansion( + "@NoStruct struct S { }", + expandedSource: "struct S { }", + diagnostics: [ + .init(message: OnlyStruct().message, line: 1, column: 11, highlight: "struct") + ], + macros: ["NoStruct": NoStruct.self] + ) + } + + func testAssertMacroExpansionMatchesArrayHighlight() { + assertMacroExpansion( + "@NoStruct struct S { }", + expandedSource: "struct S { }", + diagnostics: [ + .init(message: OnlyStruct().message, line: 1, column: 11, highlights: ["struct", "S"]) + ], + macros: ["NoStruct": NoStructMultiHighlight.self] + ) + } +} From b16a76f84ffc9eec30b035868d81b0ac4d6dc10a Mon Sep 17 00:00:00 2001 From: Rauhul Varma <rauhul@apple.com> Date: Sun, 26 Nov 2023 11:40:59 -0500 Subject: [PATCH 2/2] fix formatting --- Tests/SwiftSyntaxMacrosTestSupportTests/AssertionsTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/SwiftSyntaxMacrosTestSupportTests/AssertionsTests.swift b/Tests/SwiftSyntaxMacrosTestSupportTests/AssertionsTests.swift index a7059f5d6e8..862f3e0f3f7 100644 --- a/Tests/SwiftSyntaxMacrosTestSupportTests/AssertionsTests.swift +++ b/Tests/SwiftSyntaxMacrosTestSupportTests/AssertionsTests.swift @@ -56,7 +56,7 @@ final class AssertionsTests: XCTestCase { message: OnlyStruct(), highlights: [ Syntax(structDecl.structKeyword), - Syntax(structDecl.name) + Syntax(structDecl.name), ] ) )