Add inline variable refactoring

[somiljain2006]

Fixes #2507

Implements Inline Variable for local let declarations with lexical lookup

[ahoppen]

Thanks for tackling this, @somiljain2006. I left a few suggestions inline.

[ahoppen]

Thanks! Looks a lot simpler now already. I have left a few more suggestions inline.

[ahoppen]

I have a couple more nitpicky comments comments, otherwise looks great to me 👍🏽

[ahoppen]

One last nitpick, then we’re ready to ship.

[ahoppen]

Let’s 🚢 it

[ahoppen]

@swift-ci Please test

[somiljain2006]

@ahoppen I'm having trouble identifying the exact cause of the Windows CI failure. The build fails during CMake generation because the SwiftSyntax::SwiftLexicalLookup target cannot be found. I checked the top-level CMakeLists.txt, where SwiftSyntax is already brought in with find_package(SwiftSyntax CONFIG REQUIRED), but I haven't been able to figure out whether the issue is with how the dependency is declared in SourceKit-LSP or with the SwiftSyntax CMake package used by the Windows Stage2 toolchain. Can you tell me what I'm missing?

[ahoppen]

Seems like we need to expose SwiftLexicalLookup before we can use it because this is the first change that’s using it. Opened swiftlang/swift#91985 to do so.

[ahoppen]

The Windows issue should be fixed now. Let's try again.

@swift-ci Please test Windows

[somiljain2006]

@ahoppen How about extending the inline variable code action to support var declarations?

[ahoppen]

I don’t think extending it to var is all that important and reasoning about whether the variable is never modified is non-trivial (eg. need to also consider if it’s ever passed as inout to a function). But if you have a good proposal of how to implement it, I won’t object.

[somiljain2006]

@ahoppen I was thinking of this approach: only offer the action when the variable has an initializer and all resolved references are read-only. I would reject direct mutations (=, +=, etc.), inout uses such as &x, and any cases where a known mutation pattern is detected. More complex cases like closure captures and mutable pointer APIs could remain unsupported initially.

[ahoppen]

I think += actually raises a good point where you can’t fully decide if a variable is read-only. You could define your own operator ++= that takes a first argument as inout. Then, if you write, x ++= [2], x would be mutated but you had no way of knowing that without running the type checker.

I personally think that this would be a fairly big investment to do correctly with marginal pay-offs, so I wouldn’t do it.