Make it easier to add new final fields to existing constructors

[DanTup]

This code:

class A {
  final String a;
  final String b;
}

produces diagnostics for field_not_initialized and has a convenient fix to add a constructor:

If I use that fix and later add a new field c:

class A {
  final String a;
  final String b;
  final String c;

  A({required this.a, required this.b});
}

There is no diagnostic on c, nor are there any code actions to add C to the constructor (from the declaration of c, where I was typing). Instead, there is a fix on the constructor:

This is less convenient because it requires moving to the constructor and there's no option to add it as a named field (you have to use "Add final field formal parameters" and then "Convert selected formal parameter(s) to named" or "Convert all formal parameters to name" (although that one doesn't move it to the end where you might prefer it)).

It would be useful if a field that is not assigned in a constructor (causing final_not_initialized_constructor) provided a fix/assist at the location of the field (with both named and positional options).

(@bwilkerson one thing I thought about here was having context messages on final_not_initialized_constructor that point at the field, and showing fixes against the locations of context messages in addition to where the diagnostic is rendered... although it may fall down when there are two constructors with this diagnostic pointing at the same field as that would result in two fixes on the field declaration)

[DanTup]

(unsure if this is really a request for a refactoring or a quick-fix)

[bwilkerson]

It would be useful if a field that is not assigned in a constructor (causing final_not_initialized_constructor) provided a fix/assist at the location of the field (with both named and positional options).

I can see how that would be useful.

The reason we put the diagnostic on the constructors rather than the field is that every constructor needs to be looked at to decide the right way to add the initialization. I think the same reasoning applies to a fix.

Maybe it's the case in practice that either (a) there is typically a single constructor, so the options could be presented at the field declaration, or (b) one of the fixes, such as adding a parameter, is common enough that having a way to apply it to all constructors would be useful. I don't know whether either of those is the case, but we could certainly consider adding this kind of support for those special cases; I don't think we'd want to add every combination of edit x constructor.

unsure if this is really a request for a refactoring or a quick-fix

The same consideration would apply for an "add field" refactoring.

[DanTup]

or (b) one of the fixes, such as adding a parameter, is common enough that having a way to apply it to all constructors would be useful. I don't know whether either of those is the case, but we could certainly consider adding this kind of support for those special cases; I don't think we'd want to add every combination of edit x constructor.

My thinking was that the same two options from the first screenshot would be useful, and they're added to all constructors that have a diagnostic because this field is missing:

• add as positional parameter to all constructors that have final_not_initialized_constructor because of this field
• add as [required if non-nullable] named parameter to all constructors that have final_not_initialized_constructor because of this field

[FMorschel]

Just to point out the similar interest with #45821

[eernstg]

Here is a thing which is somewhat relevant: If we add support for primary constructors (something like this proposal) and inferred required) then the change would be as follows:

// First version.
class const A({String a, String b});

// Second version.
class const A({String a, String b, String c});

Of course, this isn't a solution to the original request because it wouldn't do anything to fix all the other constructors (if any). However, it would make the transition cost lower in some cases that might be common.

It might even be useful to use this together with a refactoring that transforms a regular constructor to a primary constructor, or vice versa.

[FMorschel]

I also think this could this be the same as #53301.

[FMorschel]

Just a full review for us to reference. Coming from #54156 (comment):

I've tested some combinations of fixes.

Things I noticed:

class C {
  final int a;

  C({required this.a});
}

class D extends C {
  final int b;
}

1. When using the fix on D the generated constructor ignores b.
   When doing this, on the generated constructor D({required super.a}); there is a fix on D with one option to add "final parameters", this is the output: D(this.b, {required super.a});
2. When using the fix on b "required named" all works fine
3. When using the fix on b for "final fields" (no required named) it doesn't add a.
   When doing this, on the generated constructor D(this.b); there is a fix on D there are two options:
   a. "Add super constructor invocation" which could be what you meant
   b. "Add required parameter" which produces invalid code D(, {required super.a}this.b); (latest master)

Note: If a was nullable or had a default value making it not required it doesn't show the option to fix (no need) when the constructor is created with the third option.

---

Some things do change with widgets:

class MyWidget extends StatelessWidget {
  final String text;

  @override
  Widget build(BuildContext context) => const SizedBox();
}

1. If you fix with "required named" on text you get const MyWidget({super.key, required this.text}); which could trigger always_put_required_named_parameters_first if it's on.
2. By using "add key to constructors on MyWidget the generated one is const MyWidget({super.key}); which has a new fix to "Add final formal parameters" that results finally in the same as above: const MyWidget({super.key, required this.text});.

[FMorschel]

@keertip since you just close #56182, do you want to try and fix this (or part)? Please take a look at my above comment too.

[FMorschel]

Here is a CL to fix 3.b above https://dart-review.googlesource.com/c/sdk/+/422440.

Here is a CL to fix Widgets 2 (the "Add final formal parameters") case, create constructors ordering when always_put_required_named_parameters_first is set and the original request from this issue at the same time https://dart-review.googlesource.com/c/sdk/+/422369

---

@bwilkerson, what are your takes on 1 and 3?

1. When using the fix on D, the generated constructor ignores b.
   [T]he generated constructor [is] D({required super.a})

Also consider the second case for Widgets here (ignores local variables too):

2. By using "add key to constructors" on MyWidget, the generated [constructor] is const MyWidget({super.key});

And 3:

3. When using the fix on b for "final fields" (no required named), it doesn't add a.
   [T]he generated constructor [is] D(this.b)

I'd personally say both could generate D(this.b, {required super.a}).