From fc7b7435991c9932a6ed2e72e60587a097b16ae4 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Mon, 3 Jun 2024 12:39:55 -0500 Subject: [PATCH 1/4] Include fix-its in the diagnostic on a parameterized test when `@Test` is missing `arguments:` Resolves rdar://128396593 --- ...EditorPlaceholderExprSyntaxAdditions.swift | 24 ++++- .../FunctionDeclSyntaxAdditions.swift | 18 ++-- .../Support/DiagnosticMessage.swift | 98 ++++++++++++++++--- .../TestDeclarationMacroTests.swift | 4 +- 4 files changed, 120 insertions(+), 24 deletions(-) diff --git a/Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift b/Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift index b6a918ea5..cc6c15525 100644 --- a/Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift +++ b/Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift @@ -11,12 +11,28 @@ import SwiftSyntax extension EditorPlaceholderExprSyntax { - /// Initialize an instance of this type with the given placeholder string. + /// Initialize an instance of this type with the given display name string and + /// optional type. /// /// - Parameters: - /// - placeholder: The placeholder string, not including surrounding angle + /// - displayName: The display name string, not including surrounding angle /// brackets or pound characters. - init(_ placeholder: String) { - self.init(placeholder: .identifier("<# \(placeholder) #" + ">")) + /// - type: The type which this placeholder have, if any. When non-`nil`, + /// the expression will use typed placeholder syntax. + init(_ displayName: String, type: String? = nil) { + let placeholderString = if let type { + // This uses typed placeholder syntax, which allows the compiler to + // type-check the expression successfully. The resulting code still does + // not compile due to the placeholder, but it makes the diagnostic more + // clear. See + // https://developer.apple.com/documentation/swift-playgrounds/specifying-editable-regions-in-a-playground-page#Mark-Editable-Areas-with-Placeholder-Tokens + "T##\(displayName)##\(type)" + } else { + displayName + } + + // Manually concatenate the string to avoid it being interpreted as a + // placeholder when editing this file. + self.init(placeholder: .identifier("<#\(placeholderString)#" + ">")) } } diff --git a/Sources/TestingMacros/Support/Additions/FunctionDeclSyntaxAdditions.swift b/Sources/TestingMacros/Support/Additions/FunctionDeclSyntaxAdditions.swift index 70c4f7baf..790c94a75 100644 --- a/Sources/TestingMacros/Support/Additions/FunctionDeclSyntaxAdditions.swift +++ b/Sources/TestingMacros/Support/Additions/FunctionDeclSyntaxAdditions.swift @@ -136,12 +136,8 @@ extension FunctionParameterSyntax { /// For example, for the parameter `y` of `func x(y: Int)`, the value of this /// property is an expression equivalent to `Int.self`. private var typeMetatypeExpression: some ExprSyntaxProtocol { - // Discard any specifiers such as `inout` or `borrowing`, since we're only - // trying to obtain the base type to reference it in an expression. - let baseType = type.as(AttributedTypeSyntax.self)?.baseType ?? type - - // Construct a member access expression, referencing the base type above. - let baseTypeDeclReferenceExpr = DeclReferenceExprSyntax(baseName: .identifier(baseType.trimmedDescription)) + // Construct a member access expression, referencing the base type name. + let baseTypeDeclReferenceExpr = DeclReferenceExprSyntax(baseName: .identifier(baseTypeName)) // Enclose the base type declaration reference in a 1-element tuple, e.g. // `()`. It will be used in a member access expression below, and @@ -155,3 +151,13 @@ extension FunctionParameterSyntax { return MemberAccessExprSyntax(base: metatypeMemberAccessBase, name: .identifier("self")) } } + +extension FunctionParameterSyntax { + /// The base type name of this parameter. + var baseTypeName: String { + // Discard any specifiers such as `inout` or `borrowing`, since we're only + // trying to obtain the base type to reference it in an expression. + let baseType = type.as(AttributedTypeSyntax.self)?.baseType ?? type + return baseType.trimmedDescription + } +} diff --git a/Sources/TestingMacros/Support/DiagnosticMessage.swift b/Sources/TestingMacros/Support/DiagnosticMessage.swift index 50a6911cc..051f5d689 100644 --- a/Sources/TestingMacros/Support/DiagnosticMessage.swift +++ b/Sources/TestingMacros/Support/DiagnosticMessage.swift @@ -432,32 +432,106 @@ struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage { /// number of arguments when applied to the given function declaration. /// /// - Parameters: - /// - attribute: The `@Test` or `@Suite` attribute. + /// - attribute: The `@Test` attribute. /// - functionDecl: The declaration in question. /// /// - Returns: A diagnostic message. static func attributeArgumentCountIncorrect(_ attribute: AttributeSyntax, on functionDecl: FunctionDeclSyntax) -> Self { let expectedArgumentCount = functionDecl.signature.parameterClause.parameters.count - switch expectedArgumentCount { - case 0: + if expectedArgumentCount == 0 { return Self( syntax: Syntax(functionDecl), message: "Attribute \(_macroName(attribute)) cannot specify arguments when used with '\(functionDecl.completeName)' because it does not take any", severity: .error ) - case 1: + } else { return Self( syntax: Syntax(functionDecl), - message: "Attribute \(_macroName(attribute)) must specify an argument when used with '\(functionDecl.completeName)'", - severity: .error + message: "Attribute \(_macroName(attribute)) must specify arguments when used with '\(functionDecl.completeName)'", + severity: .error, + fixIts: _addArgumentsFixIts(for: attribute, given: functionDecl.signature.parameterClause.parameters) ) - default: - return Self( - syntax: Syntax(functionDecl), - message: "Attribute \(_macroName(attribute)) must specify \(expectedArgumentCount) arguments when used with '\(functionDecl.completeName)'", - severity: .error + } + } + + /// Create fix-its for a diagnostic stating that the given attribute must + /// specify arguments since it is applied to a function which has parameters. + /// + /// - Parameters: + /// - attribute: The `@Test` attribute. + /// - parameters: The parameter list of the function `attribute` is applied + /// to. + /// + /// - Returns: An array of fix-its to include in a diagnostic. + private static func _addArgumentsFixIts(for attribute: AttributeSyntax, given parameters: FunctionParameterListSyntax) -> [FixIt] { + let baseArguments: LabeledExprListSyntax + if let existingArguments = attribute.arguments { + guard case var .argumentList(existingLabeledArguments) = existingArguments else { + // If there are existing arguments but they are of an unexpected type, + // don't attempt to provide any fix-its. + return [] + } + + // If the existing argument list is non-empty, ensure the last argument + // has a trailing comma and space. + if !existingLabeledArguments.isEmpty { + let lastIndex = existingLabeledArguments.index(before: existingLabeledArguments.endIndex) + existingLabeledArguments[lastIndex].trailingComma = .commaToken(trailingTrivia: .space) + } + + baseArguments = existingLabeledArguments + } else { + baseArguments = .init() + } + + var fixIts: [FixIt] = [] + func addFixIt(_ message: String, appendingArguments arguments: some Collection) { + var newAttribute = attribute + newAttribute.attributeName = newAttribute.attributeName.trimmed + newAttribute.leftParen = .leftParenToken() + newAttribute.arguments = .argumentList(baseArguments + arguments) + newAttribute.rightParen = .rightParenToken() + + fixIts.append(FixIt( + message: MacroExpansionFixItMessage(message), + changes: [.replace(oldNode: Syntax(attribute), newNode: Syntax(newAttribute))] + )) + } + + // Fix-It to add 'arguments:' with one collection. If the function has 2 or + // more parameters, the elements of the placeholder collection are of tuple + // type. + do { + let argumentsCollectionType = if parameters.count == 1, let parameter = parameters.first { + "[\(parameter.baseTypeName)]" + } else { + "[(\(parameters.map(\.baseTypeName).joined(separator: ", ")))]" + } + + addFixIt( + parameters.count == 1 ? "Add 'arguments:'" : "Add 'arguments:' with one collection", + appendingArguments: [LabeledExprSyntax(label: "arguments", expression: EditorPlaceholderExprSyntax(argumentsCollectionType, type: argumentsCollectionType))] ) } + + // Fix-It to add 'arguments:' with all combinations of collections, + // where is the count of the function's parameters. Only offered for + // functions with 2 or more parameters. + if parameters.count >= 2 { + let additionalArguments = parameters.indices.map { index in + let label = index == parameters.startIndex ? "arguments" : nil + let argumentsCollectionType = "[\(parameters[index].baseTypeName)]" + return LabeledExprSyntax( + label: label.map { .identifier($0) }, + colon: label == nil ? nil : .colonToken(trailingTrivia: .space), + expression: EditorPlaceholderExprSyntax(argumentsCollectionType, type: argumentsCollectionType), + trailingComma: parameters.index(after: index) < parameters.endIndex ? .commaToken(trailingTrivia: .space) : nil + ) + } + addFixIt("Add 'arguments:' with all combinations of \(parameters.count) collections", appendingArguments: additionalArguments) + } + + return fixIts } /// Create a diagnostic message stating that `@Test` or `@Suite` is @@ -565,7 +639,7 @@ struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage { fixIts: [ FixIt( message: MacroExpansionFixItMessage(#"Replace "\#(urlString)" with URL"#), - changes: [.replace(oldNode: Syntax(urlExpr), newNode: Syntax(EditorPlaceholderExprSyntax("url")))] + changes: [.replace(oldNode: Syntax(urlExpr), newNode: Syntax(EditorPlaceholderExprSyntax("url", type: "String")))] ), FixIt( message: MacroExpansionFixItMessage("Remove trait '\(traitName.trimmed)'"), diff --git a/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift b/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift index 7f6e68f2e..ed73a2d6e 100644 --- a/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift +++ b/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift @@ -99,9 +99,9 @@ struct TestDeclarationMacroTests { // Argument count mismatches. "@Test func f(i: Int) {}": - "Attribute 'Test' must specify an argument when used with 'f(i:)'", + "Attribute 'Test' must specify arguments when used with 'f(i:)'", "@Test func f(i: Int, j: Int) {}": - "Attribute 'Test' must specify 2 arguments when used with 'f(i:j:)'", + "Attribute 'Test' must specify arguments when used with 'f(i:j:)'", "@Test(arguments: []) func f() {}": "Attribute 'Test' cannot specify arguments when used with 'f()' because it does not take any", From ac989580fc829976083d98976f2523451caad49c Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Wed, 12 Jun 2024 23:43:31 -0500 Subject: [PATCH 2/4] PR feedback and add comprehensive new unit test --- ...EditorPlaceholderExprSyntaxAdditions.swift | 72 ++++++++++++--- .../Support/DiagnosticMessage.swift | 21 +++-- .../TestDeclarationMacroTests.swift | 91 ++++++++++++++++++- .../TestSupport/FixIts.swift | 28 ++++++ 4 files changed, 183 insertions(+), 29 deletions(-) create mode 100644 Tests/TestingMacrosTests/TestSupport/FixIts.swift diff --git a/Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift b/Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift index cc6c15525..0636491b8 100644 --- a/Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift +++ b/Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift @@ -11,28 +11,70 @@ import SwiftSyntax extension EditorPlaceholderExprSyntax { - /// Initialize an instance of this type with the given display name string and + /// Initialize an instance of this type with the given placeholder string and /// optional type. /// /// - Parameters: - /// - displayName: The display name string, not including surrounding angle + /// - placeholder: The placeholder string, not including surrounding angle /// brackets or pound characters. /// - type: The type which this placeholder have, if any. When non-`nil`, /// the expression will use typed placeholder syntax. - init(_ displayName: String, type: String? = nil) { - let placeholderString = if let type { - // This uses typed placeholder syntax, which allows the compiler to - // type-check the expression successfully. The resulting code still does - // not compile due to the placeholder, but it makes the diagnostic more - // clear. See - // https://developer.apple.com/documentation/swift-playgrounds/specifying-editable-regions-in-a-playground-page#Mark-Editable-Areas-with-Placeholder-Tokens - "T##\(displayName)##\(type)" + init(_ placeholder: String, type: String? = nil) { + self.init(placeholder: .identifier(_editorPlaceholder(placeholder, type: type))) + } + + /// Initialize an instance of this type with the given type, using that as the + /// placeholder string. + /// + /// - Parameters: + /// - type: The type to use both as the placeholder text and as the + /// expression's type. + init(type: String) { + self.init(placeholder: .identifier(editorPlaceholder(forType: type))) + } +} + +/// Format a string to be included in an editor placeholder expression using the +/// specified placeholder text and optional type information. +/// +/// - Parameters: +/// - placeholder: The placeholder string, not including surrounding angle +/// brackets or pound characters. +/// - type: The type which this placeholder have, if any. When non-`nil`, +/// the expression will use typed placeholder syntax. +/// +/// - Returns: A formatted editor placeholder string. +private func _editorPlaceholder(_ placeholder: String, type: String? = nil) -> String { + let placeholderContent = if let type { + // These use typed placeholder syntax, which allows the compiler to + // type-check the expression successfully. The resulting code still does + // not compile due to the placeholder, but it makes the diagnostic more + // clear. See + // https://developer.apple.com/documentation/swift-playgrounds/specifying-editable-regions-in-a-playground-page#Mark-Editable-Areas-with-Placeholder-Tokens + if placeholder == type { + // When the placeholder string is exactly the same as the type string, + // use the shorter typed placeholder format. + "T##\(placeholder)" } else { - displayName + "T##\(placeholder)##\(type)" } - - // Manually concatenate the string to avoid it being interpreted as a - // placeholder when editing this file. - self.init(placeholder: .identifier("<#\(placeholderString)#" + ">")) + } else { + placeholder } + + // Manually concatenate the string to avoid it being interpreted as a + // placeholder when editing this file. + return "<#\(placeholderContent)#" + ">" +} + +/// Format a string to be included in an editor placeholder expression using the +/// specified type, using that type as the placeholder text. +/// +/// - Parameters: +/// - type: The type to use both as the placeholder text and as the +/// expression's type. +/// +/// - Returns: A formatted editor placeholder string. +func editorPlaceholder(forType type: String) -> String { + _editorPlaceholder(type, type: type) } diff --git a/Sources/TestingMacros/Support/DiagnosticMessage.swift b/Sources/TestingMacros/Support/DiagnosticMessage.swift index 051f5d689..0d3f0b63b 100644 --- a/Sources/TestingMacros/Support/DiagnosticMessage.swift +++ b/Sources/TestingMacros/Support/DiagnosticMessage.swift @@ -441,13 +441,13 @@ struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage { if expectedArgumentCount == 0 { return Self( syntax: Syntax(functionDecl), - message: "Attribute \(_macroName(attribute)) cannot specify arguments when used with '\(functionDecl.completeName)' because it does not take any", + message: "Attribute \(_macroName(attribute)) cannot specify arguments when used with function '\(functionDecl.completeName)' because it does not take any", severity: .error ) } else { return Self( syntax: Syntax(functionDecl), - message: "Attribute \(_macroName(attribute)) must specify arguments when used with '\(functionDecl.completeName)'", + message: "Attribute \(_macroName(attribute)) must specify arguments when used with function '\(functionDecl.completeName)'", severity: .error, fixIts: _addArgumentsFixIts(for: attribute, given: functionDecl.signature.parameterClause.parameters) ) @@ -487,10 +487,13 @@ struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage { var fixIts: [FixIt] = [] func addFixIt(_ message: String, appendingArguments arguments: some Collection) { var newAttribute = attribute - newAttribute.attributeName = newAttribute.attributeName.trimmed newAttribute.leftParen = .leftParenToken() newAttribute.arguments = .argumentList(baseArguments + arguments) - newAttribute.rightParen = .rightParenToken() + let trailingTrivia = newAttribute.rightParen?.trailingTrivia + ?? newAttribute.attributeName.as(IdentifierTypeSyntax.self)?.name.trailingTrivia + ?? .space + newAttribute.rightParen = .rightParenToken(trailingTrivia: trailingTrivia) + newAttribute.attributeName = newAttribute.attributeName.trimmed fixIts.append(FixIt( message: MacroExpansionFixItMessage(message), @@ -509,22 +512,22 @@ struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage { } addFixIt( - parameters.count == 1 ? "Add 'arguments:'" : "Add 'arguments:' with one collection", - appendingArguments: [LabeledExprSyntax(label: "arguments", expression: EditorPlaceholderExprSyntax(argumentsCollectionType, type: argumentsCollectionType))] + "Add 'arguments:' with one collection", + appendingArguments: [LabeledExprSyntax(label: "arguments", expression: EditorPlaceholderExprSyntax(type: argumentsCollectionType))] ) } // Fix-It to add 'arguments:' with all combinations of collections, // where is the count of the function's parameters. Only offered for - // functions with 2 or more parameters. - if parameters.count >= 2 { + // functions with 2 parameters. + if parameters.count == 2 { let additionalArguments = parameters.indices.map { index in let label = index == parameters.startIndex ? "arguments" : nil let argumentsCollectionType = "[\(parameters[index].baseTypeName)]" return LabeledExprSyntax( label: label.map { .identifier($0) }, colon: label == nil ? nil : .colonToken(trailingTrivia: .space), - expression: EditorPlaceholderExprSyntax(argumentsCollectionType, type: argumentsCollectionType), + expression: EditorPlaceholderExprSyntax(type: argumentsCollectionType), trailingComma: parameters.index(after: index) < parameters.endIndex ? .commaToken(trailingTrivia: .space) : nil ) } diff --git a/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift b/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift index ed73a2d6e..0791d5609 100644 --- a/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift +++ b/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift @@ -98,12 +98,8 @@ struct TestDeclarationMacroTests { "Attribute 'Test' cannot be applied to a function with a parameter marked '_const'", // Argument count mismatches. - "@Test func f(i: Int) {}": - "Attribute 'Test' must specify arguments when used with 'f(i:)'", - "@Test func f(i: Int, j: Int) {}": - "Attribute 'Test' must specify arguments when used with 'f(i:j:)'", "@Test(arguments: []) func f() {}": - "Attribute 'Test' cannot specify arguments when used with 'f()' because it does not take any", + "Attribute 'Test' cannot specify arguments when used with function 'f()' because it does not take any", // Invalid lexical contexts "struct S { func f() { @Test func g() {} } }": @@ -158,6 +154,91 @@ struct TestDeclarationMacroTests { } } + @Test("Error diagnostics which include fix-its emitted on API misuse", arguments: [ + // 'Test' attribute must specify arguments to parameterized test functions. + "@Test func f(i: Int) {}": + ( + message: "Attribute 'Test' must specify arguments when used with function 'f(i:)'", + fixIts: [ + ExpectedFixIt( + message: "Add 'arguments:' with one collection", + changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(editorPlaceholder(forType: "[Int]"))) ")] + ), + ] + ), + "@Test func f(i: Int, j: String) {}": + ( + message: "Attribute 'Test' must specify arguments when used with function 'f(i:j:)'", + fixIts: [ + ExpectedFixIt( + message: "Add 'arguments:' with one collection", + changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(editorPlaceholder(forType: "[(Int, String)]"))) ")] + ), + ExpectedFixIt( + message: "Add 'arguments:' with all combinations of 2 collections", + changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(editorPlaceholder(forType: "[Int]")), \(editorPlaceholder(forType: "[String]"))) ")] + ), + ] + ), + "@Test func f(i: Int, j: String, k: Double) {}": + ( + message: "Attribute 'Test' must specify arguments when used with function 'f(i:j:k:)'", + fixIts: [ + ExpectedFixIt( + message: "Add 'arguments:' with one collection", + changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(editorPlaceholder(forType: "[(Int, String, Double)]"))) ")] + ), + ] + ), + #"@Test("Some display name") func f(i: Int) {}"#: + ( + message: "Attribute 'Test' must specify arguments when used with function 'f(i:)'", + fixIts: [ + ExpectedFixIt( + message: "Add 'arguments:' with one collection", + changes: [.replace(oldSourceCode: #"@Test("Some display name") "#, newSourceCode: #"@Test("Some display name", arguments: \#(editorPlaceholder(forType: "[Int]"))) "#)] + ), + ] + ), + #"@Test /*comment*/ func f(i: Int) {}"#: + ( + message: "Attribute 'Test' must specify arguments when used with function 'f(i:)'", + fixIts: [ + ExpectedFixIt( + message: "Add 'arguments:' with one collection", + changes: [.replace(oldSourceCode: #"@Test /*comment*/ "#, newSourceCode: #"@Test(arguments: \#(editorPlaceholder(forType: "[Int]"))) /*comment*/ "#)] + ), + ] + ), + ]) + func apiMisuseErrorsIncludingFixIts(input: String, expectedDiagnostic: (message: String, fixIts: [ExpectedFixIt])) throws { + let (_, diagnostics) = try parse(input) + + #expect(diagnostics.count == 1) + let diagnostic = try #require(diagnostics.first) + #expect(diagnostic.diagMessage.severity == .error) + #expect(diagnostic.message == expectedDiagnostic.message) + + try #require(diagnostic.fixIts.count == expectedDiagnostic.fixIts.count) + for (fixIt, expectedFixIt) in zip(diagnostic.fixIts, expectedDiagnostic.fixIts) { + #expect(fixIt.message.message == expectedFixIt.message) + + try #require(fixIt.changes.count == expectedFixIt.changes.count) + for (change, expectedChange) in zip(fixIt.changes, expectedFixIt.changes) { + switch (change, expectedChange) { + case let (.replace(oldNode, newNode), .replace(expectedOldSourceCode, expectedNewSourceCode)): + let oldSourceCode = String(describing: oldNode.formatted()) + #expect(oldSourceCode == expectedOldSourceCode) + + let newSourceCode = String(describing: newNode.formatted()) + #expect(newSourceCode == expectedNewSourceCode) + default: + Issue.record("Change \(change) differs from expected change \(expectedChange)") + } + } + } + } + @Test("Warning diagnostics emitted on API misuse", arguments: [ // return types diff --git a/Tests/TestingMacrosTests/TestSupport/FixIts.swift b/Tests/TestingMacrosTests/TestSupport/FixIts.swift new file mode 100644 index 000000000..36e322821 --- /dev/null +++ b/Tests/TestingMacrosTests/TestSupport/FixIts.swift @@ -0,0 +1,28 @@ +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2024 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 Swift project authors +// + +/// A type representing a fix-it which is expected to be included in a +/// diagnostic from a macro. +struct ExpectedFixIt { + /// A description of what this expected fix-it performs. + var message: String + + /// An enumeration describing a change to be performed by a fix-it. + /// + /// - Note: Not all changes in the real `FixIt` type are currently supported + /// and included in this list. + enum Change { + /// Replace `oldSourceCode` by `newSourceCode`. + case replace(oldSourceCode: String, newSourceCode: String) + } + + /// The changes that would be performed when this expected fix-it is applied. + var changes: [Change] = [] +} From 2d38e16977c142d27d83b2479672025821c22981 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Thu, 13 Jun 2024 11:08:17 -0500 Subject: [PATCH 3/4] Remove need for helper func --- ...EditorPlaceholderExprSyntaxAdditions.swift | 68 ++++++------------- .../TestDeclarationMacroTests.swift | 14 ++-- 2 files changed, 28 insertions(+), 54 deletions(-) diff --git a/Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift b/Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift index 0636491b8..a8b5063cc 100644 --- a/Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift +++ b/Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift @@ -20,7 +20,26 @@ extension EditorPlaceholderExprSyntax { /// - type: The type which this placeholder have, if any. When non-`nil`, /// the expression will use typed placeholder syntax. init(_ placeholder: String, type: String? = nil) { - self.init(placeholder: .identifier(_editorPlaceholder(placeholder, type: type))) + let placeholderContent = if let type { + // These use typed placeholder syntax, which allows the compiler to + // type-check the expression successfully. The resulting code still does + // not compile due to the placeholder, but it makes the diagnostic more + // clear. See + // https://developer.apple.com/documentation/swift-playgrounds/specifying-editable-regions-in-a-playground-page#Mark-Editable-Areas-with-Placeholder-Tokens + if placeholder == type { + // When the placeholder string is exactly the same as the type string, + // use the shorter typed placeholder format. + "T##\(placeholder)" + } else { + "T##\(placeholder)##\(type)" + } + } else { + placeholder + } + + // Manually concatenate the string to avoid it being interpreted as a + // placeholder when editing this file. + self.init(placeholder: .identifier("<#\(placeholderContent)#" + ">")) } /// Initialize an instance of this type with the given type, using that as the @@ -30,51 +49,6 @@ extension EditorPlaceholderExprSyntax { /// - type: The type to use both as the placeholder text and as the /// expression's type. init(type: String) { - self.init(placeholder: .identifier(editorPlaceholder(forType: type))) + self.init(type, type: type) } } - -/// Format a string to be included in an editor placeholder expression using the -/// specified placeholder text and optional type information. -/// -/// - Parameters: -/// - placeholder: The placeholder string, not including surrounding angle -/// brackets or pound characters. -/// - type: The type which this placeholder have, if any. When non-`nil`, -/// the expression will use typed placeholder syntax. -/// -/// - Returns: A formatted editor placeholder string. -private func _editorPlaceholder(_ placeholder: String, type: String? = nil) -> String { - let placeholderContent = if let type { - // These use typed placeholder syntax, which allows the compiler to - // type-check the expression successfully. The resulting code still does - // not compile due to the placeholder, but it makes the diagnostic more - // clear. See - // https://developer.apple.com/documentation/swift-playgrounds/specifying-editable-regions-in-a-playground-page#Mark-Editable-Areas-with-Placeholder-Tokens - if placeholder == type { - // When the placeholder string is exactly the same as the type string, - // use the shorter typed placeholder format. - "T##\(placeholder)" - } else { - "T##\(placeholder)##\(type)" - } - } else { - placeholder - } - - // Manually concatenate the string to avoid it being interpreted as a - // placeholder when editing this file. - return "<#\(placeholderContent)#" + ">" -} - -/// Format a string to be included in an editor placeholder expression using the -/// specified type, using that type as the placeholder text. -/// -/// - Parameters: -/// - type: The type to use both as the placeholder text and as the -/// expression's type. -/// -/// - Returns: A formatted editor placeholder string. -func editorPlaceholder(forType type: String) -> String { - _editorPlaceholder(type, type: type) -} diff --git a/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift b/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift index 0791d5609..4fc796eab 100644 --- a/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift +++ b/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift @@ -162,7 +162,7 @@ struct TestDeclarationMacroTests { fixIts: [ ExpectedFixIt( message: "Add 'arguments:' with one collection", - changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(editorPlaceholder(forType: "[Int]"))) ")] + changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(EditorPlaceholderExprSyntax(type: "[Int]"))) ")] ), ] ), @@ -172,11 +172,11 @@ struct TestDeclarationMacroTests { fixIts: [ ExpectedFixIt( message: "Add 'arguments:' with one collection", - changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(editorPlaceholder(forType: "[(Int, String)]"))) ")] + changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(EditorPlaceholderExprSyntax(type: "[(Int, String)]"))) ")] ), ExpectedFixIt( message: "Add 'arguments:' with all combinations of 2 collections", - changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(editorPlaceholder(forType: "[Int]")), \(editorPlaceholder(forType: "[String]"))) ")] + changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(EditorPlaceholderExprSyntax(type: "[Int]")), \(EditorPlaceholderExprSyntax(type: "[String]"))) ")] ), ] ), @@ -186,7 +186,7 @@ struct TestDeclarationMacroTests { fixIts: [ ExpectedFixIt( message: "Add 'arguments:' with one collection", - changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(editorPlaceholder(forType: "[(Int, String, Double)]"))) ")] + changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(EditorPlaceholderExprSyntax(type: "[(Int, String, Double)]"))) ")] ), ] ), @@ -196,7 +196,7 @@ struct TestDeclarationMacroTests { fixIts: [ ExpectedFixIt( message: "Add 'arguments:' with one collection", - changes: [.replace(oldSourceCode: #"@Test("Some display name") "#, newSourceCode: #"@Test("Some display name", arguments: \#(editorPlaceholder(forType: "[Int]"))) "#)] + changes: [.replace(oldSourceCode: #"@Test("Some display name") "#, newSourceCode: #"@Test("Some display name", arguments: \#(EditorPlaceholderExprSyntax(type: "[Int]"))) "#)] ), ] ), @@ -206,11 +206,11 @@ struct TestDeclarationMacroTests { fixIts: [ ExpectedFixIt( message: "Add 'arguments:' with one collection", - changes: [.replace(oldSourceCode: #"@Test /*comment*/ "#, newSourceCode: #"@Test(arguments: \#(editorPlaceholder(forType: "[Int]"))) /*comment*/ "#)] + changes: [.replace(oldSourceCode: #"@Test /*comment*/ "#, newSourceCode: #"@Test(arguments: \#(EditorPlaceholderExprSyntax(type: "[Int]"))) /*comment*/ "#)] ), ] ), - ]) + ] as [String: (message: String, fixIts: [ExpectedFixIt])]) func apiMisuseErrorsIncludingFixIts(input: String, expectedDiagnostic: (message: String, fixIts: [ExpectedFixIt])) throws { let (_, diagnostics) = try parse(input) From f14375998bfb88b6a932d494afc950363e498017 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Sun, 16 Jun 2024 14:50:03 -0500 Subject: [PATCH 4/4] Resolve CI build failure --- .../TestDeclarationMacroTests.swift | 118 +++++++++--------- 1 file changed, 61 insertions(+), 57 deletions(-) diff --git a/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift b/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift index 4fc796eab..451a66bba 100644 --- a/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift +++ b/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift @@ -154,63 +154,67 @@ struct TestDeclarationMacroTests { } } - @Test("Error diagnostics which include fix-its emitted on API misuse", arguments: [ - // 'Test' attribute must specify arguments to parameterized test functions. - "@Test func f(i: Int) {}": - ( - message: "Attribute 'Test' must specify arguments when used with function 'f(i:)'", - fixIts: [ - ExpectedFixIt( - message: "Add 'arguments:' with one collection", - changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(EditorPlaceholderExprSyntax(type: "[Int]"))) ")] - ), - ] - ), - "@Test func f(i: Int, j: String) {}": - ( - message: "Attribute 'Test' must specify arguments when used with function 'f(i:j:)'", - fixIts: [ - ExpectedFixIt( - message: "Add 'arguments:' with one collection", - changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(EditorPlaceholderExprSyntax(type: "[(Int, String)]"))) ")] - ), - ExpectedFixIt( - message: "Add 'arguments:' with all combinations of 2 collections", - changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(EditorPlaceholderExprSyntax(type: "[Int]")), \(EditorPlaceholderExprSyntax(type: "[String]"))) ")] - ), - ] - ), - "@Test func f(i: Int, j: String, k: Double) {}": - ( - message: "Attribute 'Test' must specify arguments when used with function 'f(i:j:k:)'", - fixIts: [ - ExpectedFixIt( - message: "Add 'arguments:' with one collection", - changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(EditorPlaceholderExprSyntax(type: "[(Int, String, Double)]"))) ")] - ), - ] - ), - #"@Test("Some display name") func f(i: Int) {}"#: - ( - message: "Attribute 'Test' must specify arguments when used with function 'f(i:)'", - fixIts: [ - ExpectedFixIt( - message: "Add 'arguments:' with one collection", - changes: [.replace(oldSourceCode: #"@Test("Some display name") "#, newSourceCode: #"@Test("Some display name", arguments: \#(EditorPlaceholderExprSyntax(type: "[Int]"))) "#)] - ), - ] - ), - #"@Test /*comment*/ func f(i: Int) {}"#: - ( - message: "Attribute 'Test' must specify arguments when used with function 'f(i:)'", - fixIts: [ - ExpectedFixIt( - message: "Add 'arguments:' with one collection", - changes: [.replace(oldSourceCode: #"@Test /*comment*/ "#, newSourceCode: #"@Test(arguments: \#(EditorPlaceholderExprSyntax(type: "[Int]"))) /*comment*/ "#)] - ), - ] - ), - ] as [String: (message: String, fixIts: [ExpectedFixIt])]) + static var errorsWithFixIts: [String: (message: String, fixIts: [ExpectedFixIt])] { + [ + // 'Test' attribute must specify arguments to parameterized test functions. + "@Test func f(i: Int) {}": + ( + message: "Attribute 'Test' must specify arguments when used with function 'f(i:)'", + fixIts: [ + ExpectedFixIt( + message: "Add 'arguments:' with one collection", + changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(EditorPlaceholderExprSyntax(type: "[Int]"))) ")] + ), + ] + ), + "@Test func f(i: Int, j: String) {}": + ( + message: "Attribute 'Test' must specify arguments when used with function 'f(i:j:)'", + fixIts: [ + ExpectedFixIt( + message: "Add 'arguments:' with one collection", + changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(EditorPlaceholderExprSyntax(type: "[(Int, String)]"))) ")] + ), + ExpectedFixIt( + message: "Add 'arguments:' with all combinations of 2 collections", + changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(EditorPlaceholderExprSyntax(type: "[Int]")), \(EditorPlaceholderExprSyntax(type: "[String]"))) ")] + ), + ] + ), + "@Test func f(i: Int, j: String, k: Double) {}": + ( + message: "Attribute 'Test' must specify arguments when used with function 'f(i:j:k:)'", + fixIts: [ + ExpectedFixIt( + message: "Add 'arguments:' with one collection", + changes: [.replace(oldSourceCode: "@Test ", newSourceCode: "@Test(arguments: \(EditorPlaceholderExprSyntax(type: "[(Int, String, Double)]"))) ")] + ), + ] + ), + #"@Test("Some display name") func f(i: Int) {}"#: + ( + message: "Attribute 'Test' must specify arguments when used with function 'f(i:)'", + fixIts: [ + ExpectedFixIt( + message: "Add 'arguments:' with one collection", + changes: [.replace(oldSourceCode: #"@Test("Some display name") "#, newSourceCode: #"@Test("Some display name", arguments: \#(EditorPlaceholderExprSyntax(type: "[Int]"))) "#)] + ), + ] + ), + #"@Test /*comment*/ func f(i: Int) {}"#: + ( + message: "Attribute 'Test' must specify arguments when used with function 'f(i:)'", + fixIts: [ + ExpectedFixIt( + message: "Add 'arguments:' with one collection", + changes: [.replace(oldSourceCode: #"@Test /*comment*/ "#, newSourceCode: #"@Test(arguments: \#(EditorPlaceholderExprSyntax(type: "[Int]"))) /*comment*/ "#)] + ), + ] + ), + ] + } + + @Test("Error diagnostics which include fix-its emitted on API misuse", arguments: errorsWithFixIts) func apiMisuseErrorsIncludingFixIts(input: String, expectedDiagnostic: (message: String, fixIts: [ExpectedFixIt])) throws { let (_, diagnostics) = try parse(input)