You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Rename(refactor) support for record's named fields #53024
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
voidf(({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.
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.
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.
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.
Sorry, I should've been clearer. I'm not saying that it does work today. I was saying that it would be great if both did.
The only thing that does work in one that doesn't in the other is comment references, but renaming between those two (comment reference and actual parameter in Functiontypedefs) is still broken (see #60580). I've tried working on that, but I'm not familiar enough with analyzer yet to understand where I should fix it. It seems that both elements are different.
Maybe that is the same underlying problem with your example, but I've not debugged that case yet.
Activity
bwilkerson commentedon Aug 6, 2023
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
aandbinare 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
atonameLengthsand the namebtocharacterRange, 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 commentedon Aug 6, 2023
I think rename could be applied on locations where a shape is used.
For example, let's take shape
x1that has a fielda1. If I manually rename the fielda1toa2(which will result in a new shapex2), I can immediately see places where shapex1is expected but not provided (thanks to warnings/errors). These broken places are the only places where shapex1was being used so these are the only places where I've renamea1toa2. If any of those broken places involves another shape, sayy1, it's definitely the casey1is structurally the same asx1(as long as shapes are sealed) so I've got to renamea1toa2iny1too.bwilkerson commentedon Aug 6, 2023
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 commentedon Oct 3, 2024
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 commentedon Apr 24, 2025
I'd love for this to happen so record typedefs work the same as function typedefs.
The same rules applied to renaming the first should probably work to rename the second as well.
bwilkerson commentedon Apr 24, 2025
I agree, feature parity would be good here.
scheglov commentedon Jul 21, 2025
What does work for function typedefs?
Renaming
valueinint valuedoes not updatevalue: 1.FMorschel commentedon Jul 21, 2025
Sorry, I should've been clearer. I'm not saying that it does work today. I was saying that it would be great if both did.
The only thing that does work in one that doesn't in the other is comment references, but renaming between those two (comment reference and actual parameter in
Functiontypedefs) is still broken (see #60580). I've tried working on that, but I'm not familiar enough with analyzer yet to understand where I should fix it. It seems that both elements are different.Maybe that is the same underlying problem with your example, but I've not debugged that case yet.