-
Notifications
You must be signed in to change notification settings - Fork 128
Include fix-its in the diagnostic on a parameterized test when @Test is missing arguments:
#450
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
Changes from 2 commits
fc7b743
ac98958
2d38e16
f143759
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -432,32 +432,109 @@ 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", | ||
| message: "Attribute \(_macroName(attribute)) cannot specify arguments when used with function '\(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 function '\(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<LabeledExprSyntax>) { | ||
| var newAttribute = attribute | ||
| newAttribute.leftParen = .leftParenToken() | ||
| newAttribute.arguments = .argumentList(baseArguments + arguments) | ||
| 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), | ||
| 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( | ||
| "Add 'arguments:' with one collection", | ||
| appendingArguments: [LabeledExprSyntax(label: "arguments", expression: EditorPlaceholderExprSyntax(type: argumentsCollectionType))] | ||
| ) | ||
| } | ||
|
|
||
| // Fix-It to add 'arguments:' with all combinations of <N> collections, | ||
| // where <N> is the count of the function's parameters. Only offered for | ||
| // 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( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using our built-in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still would suggest using
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've attempted to do this but found that as it stands currently, the I'm open to it, but would ask for a bit more specific guidance on how you recommend applying it (once you're available again). In the mean time I'm planning to proceed with merging this since we've gone through a couple rounds and I think the PR is in good shape overall at this point |
||
| label: label.map { .identifier($0) }, | ||
| colon: label == nil ? nil : .colonToken(trailingTrivia: .space), | ||
| expression: EditorPlaceholderExprSyntax(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) | ||
stmontgomery marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| return fixIts | ||
| } | ||
|
|
||
| /// Create a diagnostic message stating that `@Test` or `@Suite` is | ||
|
|
@@ -565,7 +642,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)'"), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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] = [] | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.