swiftlang / sourcekit-lsp

Language Server Protocol implementation for Swift and C-based languages
Apache License 2.0
3.31k stars 277 forks source link

Make LSP report variable shadowing on hover events #1123

Open RussBaz opened 8 months ago

RussBaz commented 8 months ago

Hi.

It would be nice if the LSP reported next to the type information if the current variable is shadowing another variable.

(I previously raised this issue in the Swift Forum - Can we make LSP report variable shadowing on hover?)

The existing way is to highlight all the variables in the file with the same name and manually scan through them to find out if you are shadowing any of them or not.

It is not too big of an issue in the simple cases like (with just two scopes):

var shadowed: String?

// Few lines of code

if let shadowed {
    // Do something
}

However, it becomes much harder to track what is being shadowed when the number of possible scopes, times used and lines of code grow.

For example, when you have many mutable variables in different scopes with similar or identical names. (The variable called name was the biggest offender for me) If you add to this long switch statements in some functions, with a possibility with even more nested if statements in many cases, it can become very hard to know at a glance that you are mutating the right variable. Also, without a reminder, you might later forget that it is better to rename some shadowing variables to better describe what they are doing.

I have experienced this little inconvenience while writing a parser by hand. I modelled it like a state machine and when I was dealing with certain complicated states, I would often reuse some names in the inner scopes as I could not remember all the names in the outer scopes.

Therefore, I suggest making LSP add a note (e.g. when it reports a variable type) that the variable is shadowing another variable. Ideally, even with an ability to jump to the previous, shadowed declaration, but just this reminder will be sufficient for the most cases.

Variable shadowing in Swift is great, but it would be even grater if we were subtly reminded whenever it was used.

Thank you.

ahoppen commented 8 months ago

Tracked in Apple’s issue tracker as rdar://124370517

DougGregor commented 6 months ago

Neat idea! There's a Google Summer of Code project to introduce lexical scopes into swift-syntax that would make this implementable without a huge amount of work.