Produce meaningful diagnostic for '#else if' typo #1302
Conversation
There was a problem hiding this comment.
Hey @TiagoMaiaL,
thanks for working on this. I just took a look at your changes and noticed a few things to which I put comments inline. Can you take a look whether they make sense to you. If they don’t, feel free to reach out again.
|
@swift-ci please test |
There was a problem hiding this comment.
Thanks. This is looking great now. I’ve got two more minor comments inline. After you addressed those, we can ship this.
And in case you’re wondering why your test request didn’t trigger anything: Only people with Commit Access can trigger CI testing.
|
@swift-ci Please test |
|
Oh, one last request: Could you rebase and squash your commits on top of |
TiagoMaiaL
force-pushed
the
fix-else_if-diagnostic
branch
from
7745f36 to
1c8845d
Compare
|
Thanks for squashing the commits. Let’s 🚢it. @swift-ci Please test |
Generate unexpected and missing tokens for #else if typo Add diagnostics for poundElseSpaceIf typo Fix #else_if parsing test Add StaticParserError.unexpectedPoundSpaceIf for `#else if` diagnostic Start using `ReplaceTokensFixIt` in diagnostic for `IfConfigDeclSyntax` Fix generation of unexpected nodes for `#else if` typo Fix diagnostic generation for `#else if` typo Rename `StaticParserError.unexpectedPoundSpaceIf` to `unexpectedPoundElseSpaceIf` Fix `#else if` parsing and diagnostic test Adjust `#else if` FixIt to only replace unexpected tokens Remove `diagnosticSpec`s for handled nodes in `#else if` parsing test Add `fixedSource` assertion to `#else if` diagnostic test Fix `leadingTrivia` for `#else if` FixIt Assert for correct FixIt message in `#else if` diagnostic test Use new TokenConsumer.consume API for invalid if in `#else if` Fix formatting for code related to `#else if` diagnostic generation
TiagoMaiaL
force-pushed
the
fix-else_if-diagnostic
branch
from
1c8845d to
1ee6a45
Compare
|
@ahoppen I've fixed the formatting issues in the code by running |
|
Let’s run CI again and see. If it got to checking the formatting, everything should be good. @swift-ci Please test |
|
Thank you for your help, @ahoppen |
The purpose of this PR is to fix the first part of issue #1221: produce a meaningful diagnostic for the common
#else iftypo.I've followed the tips given by @bnbarham and the ones from the fixingBugs document. Here's what I did so far:
ifconfigExprTests(it's currently failing because the diagnostics include more errors than the one I'm expecting, I must be doing something wrong)#else iftypo, and a missing one for#elseif, inparsePoundIfDirective, insideDirectives.swift(I'm not sure if I'm using the right methods)IfConfigDeclSyntax, inParseDiagnosticsGenerator.swiftAny tip or help to get this right is really appreciated. Thank you in advance.