[L] Change in dart/sdk[main]: DAS: Support many primary constructor use cases in ConvertClassToEnum
0 views
Skip to first unread message
Samuel Rawlins (Gerrit)
Jan 21, 2026, 1:17:51 PM (11 hours ago) Jan 21
to Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Brian Wilkerson
Samuel Rawlins voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Brian Wilkerson
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brian Wilkerson (Gerrit)
Jan 21, 2026, 2:11:01 PM (10 hours ago) Jan 21
to Samuel Rawlins, Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Samuel Rawlins
Brian Wilkerson voted and added 2 comments![]( "Open in Gerrit")
Votes added by Brian Wilkerson
| Code-Review | +1 |
2 comments
File pkg/analysis_server/lib/src/services/correction/dart/convert_class_to_enum.dart
Line 128, Patchset 1 (Latest):
final AstNode /* ConstructorDeclaration | PrimaryConstructorDeclaration */Brian Wilkerson . unresolved
Consider moving this into an `assert` in the constructor and possibly documenting it in the field's docs.
File pkg/analysis_server/test/src/services/correction/assist/convert_class_to_enum_test.dart
Line 151, Patchset 1 (Latest):
enum const _E(final String code) {Brian Wilkerson . unresolved
Do we want to remove the `const` here?
The proposal reads "The proposal allows an `enum` declaration to include the modifier `const` just before the name of the declaration when it has a primary constructor, but it also allows this keyword to be omitted."
I'm guessing that we'll have a lint or dead code warning to encourage people to omit it since it's useless to have it there, so maybe we should just remove it as part of the assist?
Related details
Attention is currently required from:
- Samuel Rawlins
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Samuel Rawlins (Gerrit)
Jan 21, 2026, 5:09:50 PM (7 hours ago) Jan 21
to Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Samuel Rawlins added 2 comments![]( "Open in Gerrit")
File pkg/analysis_server/lib/src/services/correction/dart/convert_class_to_enum.dart
Line 128, Patchset 1:
final AstNode /* ConstructorDeclaration | PrimaryConstructorDeclaration */Brian Wilkerson . resolved
Consider moving this into an `assert` in the constructor and possibly documenting it in the field's docs.
Samuel Rawlins
Done
File pkg/analysis_server/test/src/services/correction/assist/convert_class_to_enum_test.dart
Do we want to remove the `const` here?
The proposal reads "The proposal allows an `enum` declaration to include the modifier `const` just before the name of the declaration when it has a primary constructor, but it also allows this keyword to be omitted."
I'm guessing that we'll have a lint or dead code warning to encourage people to omit it since it's useless to have it there, so maybe we should just remove it as part of the assist?
Samuel Rawlins
Sounds good to me. Removing 'const' can be a follow-up CL.
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Samuel Rawlins (Gerrit)
Jan 21, 2026, 7:51:27 PM (4 hours ago) Jan 21
to Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Samuel Rawlins voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Commit Queue (Gerrit)
Jan 21, 2026, 7:52:08 PM (4 hours ago) Jan 21
to Samuel Rawlins, Brian Wilkerson, dart-analys...@google.com, rev...@dartlang.org
Commit Queue submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: pkg/analysis_server/lib/src/services/correction/dart/convert_class_to_enum.dart
Insertions: 8, Deletions: 4.
@@ -124,9 +124,9 @@
/// Information about a single constructor (regular or primary) in the class
/// being converted.
class _Constructor {
- /// The declaration of the constructor.
- final AstNode /* ConstructorDeclaration | PrimaryConstructorDeclaration */
- declaration;
+ /// The declaration of the constructor, either a [ConstructorDeclaration] or a
+ /// [PrimaryConstructorDeclaration].
+ final AstNode declaration;
/// The parameter list for this constructor.
final FormalParameterList parameters;
@@ -134,7 +134,11 @@
/// The element representing the constructor.
final ConstructorElement element;
- _Constructor(this.declaration, this.parameters, this.element);
+ _Constructor(this.declaration, this.parameters, this.element)
+ : assert(
+ declaration is ConstructorDeclaration ||
+ declaration is PrimaryConstructorDeclaration,
+ );
}
/// A description of how to convert the class to an enum.
```
Change information
Commit message:
DAS: Support many primary constructor use cases in ConvertClassToEnum
Work towards https://github.com/dart-lang/sdk/issues/62274
Change-Id: I83e1f3c7dae3f3b80449dc8231bf544208c9ba94
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/474760
Commit-Queue: Samuel Rawlins <sraw...@google.com>
Reviewed-by: Brian Wilkerson <brianwi...@google.com>
Files:
- M pkg/analysis_server/lib/src/services/correction/dart/convert_class_to_enum.dart
- M pkg/analysis_server/test/src/services/correction/assist/convert_class_to_enum_test.dart
Change size: L
Delta: 2 files changed, 271 insertions(+), 88 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Brian Wilkerson
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages