LSP: Should renaming a field change fields in other definitions if they are unified

[hansbogert]

I started using the LSP server some more and testing out the renaming refactor capabilities. I had a situation which can be imagined as the following:

#A: {
	foo: "bar" | string
}

B: #A & {
  foo: "foobar"
}

I intuitively expected when renaming #A.foo that B.foo would also be renamed. Thinking about it some more this is tricky business. Did the CUE team think about this and if so, what was the rationale? Is my intuition wrong and would renaming fields of structs which merely unify with the field being renamed be a very bad idea?
I fear that if you -- a user/developer -- want to choose one renaming approach over the other that LSP is not flexible enough to cater to both use-cases.

A similar question could be, would you expect B.foo to show up if you ask for the references to #A.foo ?

[cuematthew]

I've implemented a fix for this. It's now landed on master 27f5797

Essentially, my current thinking is you're correct, and it should behave as you wish. Rename is built entirely on top of find-references, so fixing find-references is all that's needed. The interesting thing is that this does, in some ways, make find-references not the opposite of jump-to-definition - see my comments in the general area of https://github.com/cue-lang/cue/blob/master/internal/lsp/definitions/definitions_test.go#L3354 This non-opposite aspect doesn't upset me though because it only rears its head when you're doing jump-to-definition from a field declaration. And similarly find-references (or rename) to field declarations.

I guess our github integrations don't close things if they're discussions rather than issues. I'll close this, but please do reopen if you think there are remaining problems with this functionality.

[hansbogert]

This is working and i love it. Thanks