Add correct diagnostic for missing parentheses

[kimdv]

Resolves #1925

[kimdv]

@swift-ci please test

[kimdv]

I added this test and it fails because we are peeking here.
When at this place current token is comparisonPredicate.

    assertParse(
      """
      withInvalidOrderings { (comparisonPredicate: @escaping (Int, Int) -> Bool) in
      }
      """
    )

So the peek seems not the correct solutions here.
Maybe we should do something like self.peek(isAt: .colon) && previousToken != .leftParen

What do you think @ahoppen

[kimdv]

@ahoppen 😬

[ahoppen]

Sorry, it’s been a busy week and I missed the comment. Thanks for the ping.

The issue happens because in canPareClosureSignature, we now stop at the arrow. Since we start doing skipSingle at comparisonPredicate. If you make the following change, then we’re calling skipSingle on the opening (, which consumes all the contents until the matching ) before the in, including the ->, which fixes the issue.

- if lookahead.consume(if: .leftParen) != nil {  // Consume the ')'.
+ if lookahead.at(.leftParen) {  // Consume the '('.

[kimdv]

Oh okay nice!
Thanks a lot.

And no worries! Take the time to reply when you can 😁

[kimdv]

@swift-ci please test

[kimdv]

@swift-ci please test macOS

[kimdv]

@ahoppen the job fails here: https://ci.swift.org/job/swift-syntax-PR-macOS/3177/console

Not sure why.
When I add the tests in the test suite they succeeds correctly.

Edit:
committed and added them now

[kimdv]

@swift-ci please test

[ahoppen]

It’s failing at the following test case. The in in the for statement is critical to reproduce the failure. Could you look into why it’s failing? If you can’t find it, I can try debugging as well.

state.withCriticalRegion { (1 + 2) }
for action in tracking {
  action()
}

And just to explain how I got to this: I clones the compiler repo and ran

swift-parser-cli print-diags stdlib/public/Observation/Sources/Observation/ObservationRegistrar.swift

Then I kept removing code from that file while still reproducing the failure until I ended up with the reduced example.

[kimdv]

@ahoppen found it.
We had inside canParseClosureSignature this !lookahead.at(.endOfFile, .rightParen, .keyword(.in)) and it didn't stop before hitting the in keyword.

Changed rightParen -> rightBrace

[kimdv]

@swift-ci please test

[ahoppen]

@swift-ci Please test Windows

[ahoppen]

Oh, yes, that does make sense. skipSingle is looking for the rightParen now that we previously consumed manually in lookahead.consume(if: .leftParen).