[XL] Change in dart/sdk[main]: DeCo. Use primary initializer scope for field initializers.
Konstantin Shcheglov (Gerrit)
Konstantin Shcheglov voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Johnni Winther
- Paul Berry
- Samuel Rawlins
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Paul Berry (Gerrit)
Paul Berry added 6 comments![]( "Open in Gerrit")
/// The primary constructor of this element.Clarify what it means if this getter returns `null`, e.g.:
```suggestion
/// The primary constructor of this element, or `null` if this element has no
/// primary constructor.
```
void nonLateInstanceField_enter({The name of this method, and its type signature, suggest that it should be called whenever entering a non-late instance field, regardless of whether there are primary constructor parameters. But the call site in `VariableDeclarationResolver.resolve` only calls it if `primaryConstructorParameters` is non-null.
I suggest either:
- Change the name to something that reflects the fact that the methods purpose is to declare primary constructor parameters (e.g. `declarePrimaryConstructorParameters()`),
- Make the `primaryConstructorParameters` argument non-nullable,
- And remove the `if` test on line 330.
Or:
- Remove the `if` test at the call site (line 63 of `variable_declaration_resolver.dart`) so that this method is always called.
List<FormalParameterElementImpl>? primaryConstructorParameters;The name suggests that this value will be non-null whenever there are primary constructor parameters. But the behavior below is that this value will be non-null whenever there are primary constructor parameters *that are in scope*. How about renaming this to `inScopePrimaryConstructorParameters`?
List<FormalParameterElementImpl>? primaryConstructorParameters,The name suggests that this value will be non-null whenever there are primary constructor parameters. But the behavior at the call site is that this value will be non-null whenever there are primary constructor parameters *that are in scope*. How about renaming this to `inScopePrimaryConstructorParameters`?
List<FormalParameterElementImpl>? primaryConstructorParameters;The name suggests that this value will be non-null whenever there are primary constructor parameters. But the behavior below is that this value will be non-null whenever there are primary constructor parameters *that are in scope*. How about renaming this to `inScopePrimaryConstructorParameters`?
List<FormalParameterElementImpl>? primaryConstructorParameters;The name suggests that this value will be non-null whenever there are primary constructor parameters. But the behavior below is that this value will be non-null whenever there are primary constructor parameters *that are in scope*. How about renaming this to `inScopePrimaryConstructorParameters`?
Related details
- Johnni Winther
- Konstantin Shcheglov
- Samuel Rawlins
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Konstantin Shcheglov (Gerrit)
Konstantin Shcheglov added 6 comments![]( "Open in Gerrit")
Clarify what it means if this getter returns `null`, e.g.:
```suggestion
/// The primary constructor of this element, or `null` if this element has no
/// primary constructor.
```
Done
The name of this method, and its type signature, suggest that it should be called whenever entering a non-late instance field, regardless of whether there are primary constructor parameters. But the call site in `VariableDeclarationResolver.resolve` only calls it if `primaryConstructorParameters` is non-null.
I suggest either:
- Change the name to something that reflects the fact that the methods purpose is to declare primary constructor parameters (e.g. `declarePrimaryConstructorParameters()`),
- Make the `primaryConstructorParameters` argument non-nullable,
- And remove the `if` test on line 330.
Or:
- Remove the `if` test at the call site (line 63 of `variable_declaration_resolver.dart`) so that this method is always called.
Done
List<FormalParameterElementImpl>? primaryConstructorParameters;The name suggests that this value will be non-null whenever there are primary constructor parameters. But the behavior below is that this value will be non-null whenever there are primary constructor parameters *that are in scope*. How about renaming this to `inScopePrimaryConstructorParameters`?
Done
List<FormalParameterElementImpl>? primaryConstructorParameters,The name suggests that this value will be non-null whenever there are primary constructor parameters. But the behavior at the call site is that this value will be non-null whenever there are primary constructor parameters *that are in scope*. How about renaming this to `inScopePrimaryConstructorParameters`?
Done
List<FormalParameterElementImpl>? primaryConstructorParameters;The name suggests that this value will be non-null whenever there are primary constructor parameters. But the behavior below is that this value will be non-null whenever there are primary constructor parameters *that are in scope*. How about renaming this to `inScopePrimaryConstructorParameters`?
Done
List<FormalParameterElementImpl>? primaryConstructorParameters;The name suggests that this value will be non-null whenever there are primary constructor parameters. But the behavior below is that this value will be non-null whenever there are primary constructor parameters *that are in scope*. How about renaming this to `inScopePrimaryConstructorParameters`?
- Johnni Winther
- Paul Berry
- Samuel Rawlins
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Paul Berry (Gerrit)
Paul Berry voted and added 1 comment![]( "Open in Gerrit")
- Johnni Winther
- Konstantin Shcheglov
- Samuel Rawlins
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Samuel Rawlins (Gerrit)
Samuel Rawlins voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Johnni Winther
- Konstantin Shcheglov
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Konstantin Shcheglov (Gerrit)
Konstantin Shcheglov voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
- Johnni Winther
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Commit Queue (Gerrit)
Commit Queue submitted the change![]( "Open in Gerrit")
Change information
DeCo. Use primary initializer scope for field initializers.
- M pkg/analyzer/api.txt
- M pkg/analyzer/lib/dart/element/element.dart
- M pkg/analyzer/lib/src/dart/element/element.dart
- M pkg/analyzer/lib/src/dart/resolver/flow_analysis_visitor.dart
- M pkg/analyzer/lib/src/dart/resolver/variable_declaration_resolver.dart
- M pkg/analyzer/lib/src/generated/resolver.dart
- M pkg/analyzer/lib/src/summary2/ast_resolver.dart
- M pkg/analyzer/lib/src/summary2/reference_resolver.dart
- M pkg/analyzer/lib/src/summary2/top_level_inference.dart
- M pkg/analyzer/test/src/dart/resolution/class_test.dart
- M pkg/analyzer/test/src/dart/resolution/enum_test.dart
- M pkg/analyzer/test/src/summary/elements/class_test.dart
- M pkg/analyzer/test/src/summary/elements/enum_test.dart
Code-Review: +1 by Paul Berry, +1 by Samuel Rawlins
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |