Suggest to add missing equal token to variable declarations

[whiteio]

When a variable declaration is followed by an expression on the same line, it should be suggested to insert an equal token.

Resolves #1610

[ahoppen]

Thanks @whiteio. I’ve got one question regarding the implementation. Also: Could you add a test case from the example in the issue?

[ahoppen]

Can’t you just inline isProbablyVarDecl here? If you can, you could merge the !self.at(.leftBrace) check into it?

Also, instead of checking self.currentToken.flags.contains(.isAtStartOfLine) it’s nicer to check

self.at(TokenSpec(.identifier, allowAtStartOfLine: false), TokenSpec(.wildcard, allowAtStartOfLine: false))`

Also: What do we need the isProbablyVarDecl check for?

[whiteio]

@ahoppen Yes I've added in the test case from the example in the issue. isProbablyVarDecl can be inlined, I've inlined it and merged in !self.at(.leftBrace).

self.at(TokenSpec(.identifier, allowAtStartOfLine: false), TokenSpec(.wildcard, allowAtStartOfLine: false))`

Would this also need to include keywords? For example, if I change it to the above it would suggest inserting an '=' if the variable declaration is followed by an initializer on a new line, such as:

struct S {
  let _value: Int
                 ╰─ error: expected '=' in variable
  init() {
  }
}

Also: What do we need the isProbablyVarDecl check for?

I added this in to make sure that the correct fixits are suggested when there are consecutive variable declarations on the same line.

For example, without the isProbablyVarDecl check it would suggest adding an '=' instead of a ';' before 'i' in the following case

let x: Int i = 10

[ahoppen]

self.at(TokenSpec(.identifier, allowAtStartOfLine: false), TokenSpec(.wildcard, allowAtStartOfLine: false))

I am sorry, I missed that you had a negation in front of self.at. Forget what I said.

I added this in to make sure that the correct fixits are suggested when there are consecutive variable declarations on the same line.

Hmm. But this also means that you don’t generate the diagnostic to add the missing = if you write the following, right?

let x: Int i

I’m not sure if it’s really beneficial to optimize for the case where someone incorrectly writes two variable declarations on the same line. Declaring two variables on the same line isn’t very common in Swift in my experience.

So: My take on this would be to just remove

!self.at(.identifier, .wildcard, .leftBrace), !self.peek().rawTokenKind.is(.colon, .equal, .comma)

entirely. The only thing you might need to keep is !self.at(.leftBrace) so that we parse the upcoming init accessor syntax correctly

struct Foo {
  let x = 1 {
    init {
      …
    }
  }
}

[whiteio]

So: My take on this would be to just remove
!self.at(.identifier, .wildcard, .leftBrace), !self.peek().rawTokenKind.is(.colon, .equal, .comma)

Yes, you're right. I've made your suggested change to remove this, I've added !self.at(.leftBrace) as it's needed for scenarios such as the one you've mentioned. 👍

[ahoppen]

Am I missing something or do you know why we aren’t interesting a = after Int as the Fix-It says?

[whiteio]

Looks like "insert '='" wasn't included in the applyFixits parameter. I've added it in now.

[ahoppen]

Oh, that makes sense.

[ahoppen]

Just one more comment, then we’re ready to 🚢

[ahoppen]

The idea is that the a test inserts newline and the b test below adds the ;. So, I think you should be able to remove testRecovery7b.

[whiteio]

Ok, that's it removed now.

[ahoppen]

Thank you @whiteio.

[ahoppen]

@swift-ci Please test