Quick Assist for creating a constructor for selected final fields.

Quick Assist for creating a constructor for selected final fields.

R=brianwilkerson@google.com
BUG=

Committed: https://code.google.com/p/dart/source/detail?r=44504

[Brian Wilkerson, 2015-03-15 23:43:22]

This seems strange to me because it allows the creation of a constructor that
only initializes some of the final fields, leaving others with error markers,
and requires that the fields to be initialized be contiguous.

Every final field without an initializer will have a warning associated with it.
Would this be better as a quick fix that creates a constructor that initializes
all currently uninitialized final fields?

If this is the right solution to the problem then with the exception of the
possible bug, the code changes themselves LGTM.

https://codereview.chromium.org/1009833002/diff/1/pkg/analysis_server/test/se...
File pkg/analysis_server/test/services/correction/assist_test.dart (right):

https://codereview.chromium.org/1009833002/diff/1/pkg/analysis_server/test/se...
pkg/analysis_server/test/services/correction/assist_test.dart:1092: void
test_createFinalFieldsConstructor() {
What happens with the following test input:

class A {
  final int a;
// start
  final int b;
  final int c = 3;
  final int d;
// end
  final int e;
}

Does it generate "A(this.b, this.d);" or "A(this.b, this.c, this.d);"?

[scheglov, 2015-03-15 23:49:52]

You're right.
I will rework this CL into a Quick Fix.


On 2015/03/15 23:43:22, Brian Wilkerson wrote:
> This seems strange to me because it allows the creation of a constructor that
> only initializes some of the final fields, leaving others with error markers,
> and requires that the fields to be initialized be contiguous.
> 
> Every final field without an initializer will have a warning associated with
it.
> Would this be better as a quick fix that creates a constructor that
initializes
> all currently uninitialized final fields?
> 
> If this is the right solution to the problem then with the exception of the
> possible bug, the code changes themselves LGTM.
> 
>
https://codereview.chromium.org/1009833002/diff/1/pkg/analysis_server/test/se...
> File pkg/analysis_server/test/services/correction/assist_test.dart (right):
> 
>
https://codereview.chromium.org/1009833002/diff/1/pkg/analysis_server/test/se...
> pkg/analysis_server/test/services/correction/assist_test.dart:1092: void
> test_createFinalFieldsConstructor() {
> What happens with the following test input:
> 
> class A {
>   final int a;
> // start
>   final int b;
>   final int c = 3;
>   final int d;
> // end
>   final int e;
> }
> 
> Does it generate "A(this.b, this.d);" or "A(this.b, this.c, this.d);"?

[scheglov, 2015-03-16 00:43:21]

PTAL

On 2015/03/15 23:49:52, scheglov wrote:
> You're right.
> I will rework this CL into a Quick Fix.
> 
> 
> On 2015/03/15 23:43:22, Brian Wilkerson wrote:
> > This seems strange to me because it allows the creation of a constructor
that
> > only initializes some of the final fields, leaving others with error
markers,
> > and requires that the fields to be initialized be contiguous.
> > 
> > Every final field without an initializer will have a warning associated with
> it.
> > Would this be better as a quick fix that creates a constructor that
> initializes
> > all currently uninitialized final fields?
> > 
> > If this is the right solution to the problem then with the exception of the
> > possible bug, the code changes themselves LGTM.
> > 
> >
>
https://codereview.chromium.org/1009833002/diff/1/pkg/analysis_server/test/se...
> > File pkg/analysis_server/test/services/correction/assist_test.dart (right):
> > 
> >
>
https://codereview.chromium.org/1009833002/diff/1/pkg/analysis_server/test/se...
> > pkg/analysis_server/test/services/correction/assist_test.dart:1092: void
> > test_createFinalFieldsConstructor() {
> > What happens with the following test input:
> > 
> > class A {
> >   final int a;
> > // start
> >   final int b;
> >   final int c = 3;
> >   final int d;
> > // end
> >   final int e;
> > }
> > 
> > Does it generate "A(this.b, this.d);" or "A(this.b, this.c, this.d);"?

[Brian Wilkerson, 2015-03-16 13:17:55]

LGTM

https://codereview.chromium.org/1009833002/diff/20001/pkg/analysis_server/tes...
File pkg/analysis_server/test/services/correction/fix_test.dart (right):

https://codereview.chromium.org/1009833002/diff/20001/pkg/analysis_server/tes...
pkg/analysis_server/test/services/correction/fix_test.dart:290: void
test_createConstructor_forFinalFields() {
Does this handle the case where there is already an unnamed constructor?

class Test {
  final int a;
  final int b;
  final int c;

  Test(this.b);
}

[scheglov, 2015-03-16 14:55:22]

https://codereview.chromium.org/1009833002/diff/20001/pkg/analysis_server/tes...
File pkg/analysis_server/test/services/correction/fix_test.dart (right):

https://codereview.chromium.org/1009833002/diff/20001/pkg/analysis_server/tes...
pkg/analysis_server/test/services/correction/fix_test.dart:290: void
test_createConstructor_forFinalFields() {
On 2015/03/16 13:17:55, Brian Wilkerson wrote:
> Does this handle the case where there is already an unnamed constructor?
> 
> class Test {
>   final int a;
>   final int b;
>   final int c;
> 
>   Test(this.b);
> }

No, not this fix.
I plan to add a separate one for updating existing constructors.
This location has a different location (a constructor vs. a field) and a quite
different implementation.

[scheglov, 2015-03-16 15:00:36]

Committed patchset #2 (id:20001) manually as 44504 (presubmit successful).