Include fix-its in the diagnostic on a parameterized test when @Test is missing arguments:
#450
Conversation
stmontgomery
requested review from
SeanROlszewski,
briancroom,
dennisweissmann,
grynspan and
suzannaratcliff
as code owners
|
@swift-ci please test |
|
|
||
| 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 |
Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift
Outdated
Show resolved
Hide resolved
| let additionalArguments = parameters.indices.map { index in | ||
| let label = index == parameters.startIndex ? "arguments" : nil | ||
| let argumentsCollectionType = "[\(parameters[index].baseTypeName)]" | ||
| return LabeledExprSyntax( |
There was a problem hiding this comment.
Consider using our built-in Argument type instead, which when cast to LabeledExprSyntax ought to always do the right thing.
There was a problem hiding this comment.
I still would suggest using Argument here.
There was a problem hiding this comment.
I've attempted to do this but found that as it stands currently, the Argument helper type still requires me to do the same book-keeping for trailing trivia like commaToken on the LabeledExprSyntax nodes. I'm not quite sure what the benefit of this type is in this circumstance.
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
grynspan
left a comment
grynspan
left a comment
There was a problem hiding this comment.
Please add a new parameterized test that covers all the code paths for the new Fix-It code.
|
@swift-ci please test Windows |
I've now added a comprehensive unit test to validate the fix-its on this diagnostic under various scenarios |
|
@swift-ci please test |
…` is missing `arguments:` Resolves rdar://128396593
stmontgomery
force-pushed
the
arguments-fix-it
branch
from
d8bd0f7 to
ac98958
Compare
|
@swift-ci please test |
Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift
Outdated
Show resolved
Hide resolved
|
@swift-ci please test |
|
@swift-ci please test |
2 participants
This adds fix-its to the diagnostic emitted by the
@Testmacro when it has been applied to a function which includes parameters but does not specifyarguments: ....Motivation:
When transforming a non-parameterized
@Testfunction into a parameterized one, it's common to begin by adding one or more parameters to the function's signature. Immediately after doing that, the@Testmacro emits an error diagnostic ifarguments:is not passed to@Test. Since this is a common workflow and it may not always be obvious to new users where to addarguments:, it would be beneficial for this diagnostic to include a fix-it.Modifications:
EditorPlaceholderExprSyntaxto prefer typed placeholders (see code comment).Checklist:
Resolves rdar://128396593