clojure-lsp / clojure-lsp

Clojure & ClojureScript Language Server (LSP) implementation
https://clojure-lsp.io
MIT License
1.17k stars 153 forks source link

Rename symbol does not change symbols in map destructuring form with `:or` directive #1562

Closed DeLaGuardo closed 1 year ago

DeLaGuardo commented 1 year ago

Is your feature request related to a problem? Please describe. Consider this block:

(let [{:keys [foo] :or {foo 42}} {:foo 0}]
  (inc foo))

When I use the action rename symbol foo it works mostly as expected, except it doesn't change the symbol inside :or {foo 42} destructuring form. So, after renaming foo to bar the same snippet will look like this:

(let [{:keys [bar] :or {foo 42}} {:foo 0}]
  (inc bar))

In some huge bindings blocks that might end up with tricky hidden bug.

Describe the solution you'd like Because I expect rename symbol to alter all occurrences of the symbol foo it would be great if the action will change "defaults" as well.

Describe alternatives you've considered Some sort of interactive mode questioning how to proceed in case clojure-lsp detects that rename suppose to target "defaults".

ericdallo commented 1 year ago

Good catch, we should fix it, the issue is that we don't have any analysis for the foo inside the :or, so we would need probably a change in clj-kondo first to include those analysis so LSP can query and know what are them.

@borkdude I'd say those are locals as well but with some flag saying they come from :or, WDYT?

borkdude commented 1 year ago

I'd say the foo in :or is a local usage. Issue (+ PR) welcome.

ericdallo commented 1 year ago

@DeLaGuardo feel free to open a issue in clj-kondo about that, otherwise I should do it in some days

DeLaGuardo commented 1 year ago

Was unable to submit one yesterday due to github troubles :) https://github.com/clj-kondo/clj-kondo/issues/2068

ericdallo commented 1 year ago

Fixed on nightly/master, should be available on the next release