Rename(refactor) support for record's named fields

[hamsbrar]

({int a}) record = (a: 123);
//    ^ hovering over 'a' and pressing f2 shows the message "The element can't be renamed."

> dart --version
Dart SDK version: 3.0.6 (stable) (Tue Jul 11 18:49:07 2023 +0000) on "linux_x64"

> code --list-extensions --show-versions | grep dart-code
[email protected]

[bwilkerson]

I'm not claiming that it was the correct decision, but the choice to not support refactoring for record fields was intentional.

The rationale behind the decision is that, unlike classes, enums, and other nominal types, records are structurally typed. That means that the types of a and b in

void f(({int first, int last}) a, ({int first, int last}) b) {}

are the same type because of the shape of the record. There's actually nothing there, other than the shape to tell us that they're intended to be the same. In fact, if we change the name a to nameLengths and the name b to characterRange, then we'd immediately suspect that the length of the first name and the first character in the range were only coincidentally named the same.

In other words, for a compiler that doesn't understand the semantics of the names, there's no reason to think that the two record types have any semantic relationship, or that both names should be changed if one of the names is selected.

We could make an exception for an assignment and assume that the type on the left should always be associated with the type on the right, and that would likely always be true. But what we decided was that it would be confusing if 'rename' was sometimes available and sometimes not, so we decided it was better to never offer it.

Interested to see what the community thinks.

[hamsbrar]

I think rename could be applied on locations where a shape is used.

For example, let's take shape x1 that has a field a1. If I manually rename the field a1 to a2(which will result in a new shape x2), I can immediately see places where shape x1 is expected but not provided (thanks to warnings/errors). These broken places are the only places where shape x1 was being used so these are the only places where I've rename a1 to a2. If any of those broken places involves another shape, say y1, it's definitely the case y1 is structurally the same as x1(as long as shapes are sealed) so I've got to rename a1 to a2 in y1 too.

[bwilkerson]

I think that's an interesting idea.

I'd want to think through some concrete examples before committing to it in order to evaluate whether there are potentially confusing cases where using the shape would cause us to either rename too much or not rename enough, but I think it might be worth exploring.

@scheglov Do we have information about which field names needed to be the same to resolve everything within a function or initializer expression? If not, is it information that we could collect? (Just trying to evaluate the feasibility of the proposed approach.

[DanTup]

A slightly related issue came up at Dart-Code/Dart-Code#5294, which is that LSP's Document Highlights also doesn't know how to relate these things together, so you don't get the same highlighting you do for class fields.

I suspect if something was implemented to support renaming, it could also be used to improve that.

[FMorschel]

I'd love for this to happen so record typedefs work the same as function typedefs.

typedef MyFunc = void Function(int, {int value});
typedef MyRecord = (int, {int value});

The same rules applied to renaming the first should probably work to rename the second as well.

[bwilkerson]

I agree, feature parity would be good here.