[SourceKit] Add local variables to structure (SR-5057)

[marcelofabri]

This PR includes local variables (source.lang.swift.decl.var.local) to structures provided by SourceKit. This also makes variables declared in for and while statements available in the structure.

Resolves SR-5057.

This is super important for SwiftLint, as you can see on realm/SwiftLint#136 and all duplicated issues. I'm not sure if there's a historical reason for this not be included until now.

[xwu]

Similar question for this: does it make sense to be emitting accessibility and setter accessibility for local vars? They don't properly have such, unless I'm mistaken.

[marcelofabri]

Does it make sense to be emitting accessibility and setter accessibility for local vars? They don't properly have such, unless I'm mistaken.

I thought about this and I wasn't able to come up with a good answer. It looks like they'll always be private (which isn't quite right), but at the same time, checking if key.setter_accessibility is present is the only way to check whether it's a let or a var declaration.

[xwu]

Hmm, yes, that's useful information, but surely there's a design possible that doesn't require describing the non-existent accessibility level of a non-existent setter?

[marcelofabri]

Hmm, yes, that's useful information, but surely there's a design possible that doesn't require describing the non-existent accessibility level of a non-existent setter?

I think I agree. What would be the best approach here? Leaving that for another PR?

[xwu]

I wouldn't have the ability to answer that, but I will ask--is it possible to suppress the accessibility output and to have two flavors of source.lang.swift.decl.var.local?

[marcelofabri]

is it possible to suppress the accessibility output and to have two flavors of source.lang.swift.decl.var.local

Wouldn't that be actually more inconsistent? This would be the only variable kind to be split in two based on its mutability.

Some alternatives:

• Keep the accessibility as private. Although not 100% accurate, at least it's consistent and convenient. It'd be just a matter of clients understanding this convention.
• Introduce a new source.lang.swift.accessibility.local. This could also be a bit inconsistent, because it's not something you can specify with a keyword.
• Introduce a new key.ismutable (or something like that) field that should be added to variables declarations, indicating whether it's a var or let.

[xwu]

Whatever key is used to indicate the difference between var and let, I feel pretty strongly it should not be accessibility, as this information simply has nothing to do with access levels.

This is why I suggested different flavors of [...]var.local. If it feels inconsistent, it ought to be possible to split every variable kind into two based on mutability: [...]var.global.constant, [...]var.global.variable, etc. ("variable" and "constant" being the official terminology used in TSPL).

However, another key such as key.valuebindingkind might be nice, too, and extensible when ownership features come into play.

[marcelofabri]

cc @nkcsgexi

[marcelofabri]

I'm thinking about removing the accessibility fields for local variables for now and leave the discussion for the (near) future.

[marcelofabri]

Rebased omitting the accessibility for local variables.

[marcelofabri]

I've removed these since the C-style for was removed from Swift and some cleanup was just merged (#11346)

[marcelofabri]

ping @nkcsgexi?

[nkcsgexi]

@swift-ci please test

[nkcsgexi]

lgtm! Sorry about the delay.

[nkcsgexi]

@swift-ci please smoke test

[nkcsgexi]

@shahmishal any idea why PR testing isn't responding?

[shahmishal]

@swift-ci please smoke test

[shahmishal]

@nkcsgexi @marcelofabri Is it possible to close this PR and re-create the PR? We have seen issues with few PR's, to workaround this issue we need to just create the PR.