[analyzer] fix for use_key_in_widget_constructors should use super parameters when possible.

Fixes #48946

Change-Id: I45afc5717eb17845f50ef4aa77fb554fb88e2ec6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/244242
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>

pkg/analysis_server/lib/src/services/correction/dart/add_key_to_constructors.dart

@@ -13,6 +13,7 @@ import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
 import 'package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart';
 import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
+import 'package:analyzer_plugin/utilities/range_factory.dart';
 import 'package:collection/collection.dart';
 
 class AddKeyToConstructors extends CorrectionProducer {
@@ -49,8 +50,12 @@ class AddKeyToConstructors extends CorrectionProducer {
           }
           builder.write(className);
           builder.write('({');
-          builder.writeType(keyType);
-          builder.write(' key}) : super(key: key);');
+          if (libraryElement.featureSet.isEnabled(Feature.super_parameters)) {
+            builder.write('super.key});');
+          } else {
+            builder.writeType(keyType);
+            builder.write(' key}) : super(key: key);');
+          }
           builder.write(targetLocation.suffix);
         });
       });
@@ -61,6 +66,18 @@ class AddKeyToConstructors extends CorrectionProducer {
       if (keyType == null) {
         return;
       }
+      var superParameters =
+          libraryElement.featureSet.isEnabled(Feature.super_parameters);
+
+      void writeKey(DartEditBuilder builder) {
+        if (superParameters) {
+          builder.write('super.key');
+        } else {
+          builder.writeType(keyType);
+          builder.write(' key');
+        }
+      }
+
       var parameterList = parent.parameters;
       var parameters = parameterList.parameters;
       if (parameters.isEmpty) {
@@ -68,10 +85,10 @@ class AddKeyToConstructors extends CorrectionProducer {
         await builder.addDartFileEdit(file, (builder) {
           builder.addInsertion(parameterList.leftParenthesis.end, (builder) {
             builder.write('{');
-            builder.writeType(keyType);
-            builder.write(' key}');
+            writeKey(builder);
+            builder.write('}');
           });
-          _updateSuper(builder, parent);
+          _updateSuper(builder, parent, superParameters);
         });
         return;
       }
@@ -81,19 +98,19 @@ class AddKeyToConstructors extends CorrectionProducer {
         await builder.addDartFileEdit(file, (builder) {
           builder.addInsertion(parameters.last.end, (builder) {
             builder.write(', {');
-            builder.writeType(keyType);
-            builder.write(' key}');
+            writeKey(builder);
+            builder.write('}');
           });
-          _updateSuper(builder, parent);
+          _updateSuper(builder, parent, superParameters);
         });
       } else if (leftDelimiter.type == TokenType.OPEN_CURLY_BRACKET) {
         // There are other named parameters, so add the new named parameter.
         await builder.addDartFileEdit(file, (builder) {
           builder.addInsertion(leftDelimiter.end, (builder) {
-            builder.writeType(keyType);
-            builder.write(' key, ');
+            writeKey(builder);
+            builder.write(', ');
           });
-          _updateSuper(builder, parent);
+          _updateSuper(builder, parent, superParameters);
         });
       }
     }
@@ -140,8 +157,8 @@ class AddKeyToConstructors extends CorrectionProducer {
     );
   }
 
-  void _updateSuper(
-      DartFileEditBuilder builder, ConstructorDeclaration constructor) {
+  void _updateSuper(DartFileEditBuilder builder,
+      ConstructorDeclaration constructor, bool superParameters) {
     if (constructor.factoryKeyword != null ||
         constructor.redirectedConstructor != null) {
       // Can't have a super constructor invocation.
@@ -159,6 +176,16 @@ class AddKeyToConstructors extends CorrectionProducer {
         return;
       }
     }
+    if (superParameters) {
+      if (invocation != null && invocation.argumentList.arguments.isEmpty) {
+        var previous = initializers.length == 1
+            ? constructor.parameters
+            : initializers[initializers.indexOf(invocation) - 1];
+        builder.addDeletion(range.endStart(previous, constructor.body));
+      }
+      return;
+    }
+
     if (invocation == null) {
       // There is no super constructor invocation, so add one.
       if (initializers.isEmpty) {

pkg/analysis_server/test/src/services/correction/fix/add_key_to_constructors_test.dart

@@ -15,6 +15,7 @@ void main() {
     defineReflectiveTests(AddKeyToConstructorsTest);
     defineReflectiveTests(
         AddKeyToConstructorsWithoutNamedArgumentsAnywhereTest);
+    defineReflectiveTests(AddKeyToConstructorsWithoutSuperParametersTest);
   });
 }
 
@@ -44,6 +45,449 @@ class MyWidget extends StatelessWidget {
     await assertHasFix('''
 import 'package:flutter/material.dart';
 
+class MyWidget extends StatelessWidget {
+  const MyWidget({super.key});
+}
+''');
+  }
+
+  Future<void> test_class_noNewline() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  const MyWidget({super.key});
+}
+''');
+  }
+
+  Future<void> test_constructor_namedParameters_withoutSuper() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  MyWidget({required String s});
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  MyWidget({super.key, required String s});
+}
+''');
+  }
+
+  Future<void> test_constructor_namedParameters_withSuper() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  MyWidget({required String s}) : super();
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  MyWidget({super.key, required String s});
+}
+''');
+  }
+
+  Future<void> test_constructor_namedParameters_withSuper_assert() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  MyWidget({required String s}) : assert(s.isNotEmpty), super();
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  MyWidget({super.key, required String s}) : assert(s.isNotEmpty);
+}
+''');
+  }
+
+  Future<void> test_constructor_noNamedParameters_withoutSuper() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  MyWidget(String s);
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  MyWidget(String s, {super.key});
+}
+''');
+  }
+
+  Future<void> test_constructor_noNamedParameters_withSuper() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  MyWidget(String s) : super();
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  MyWidget(String s, {super.key});
+}
+''');
+  }
+
+  Future<void> test_constructor_noParameters_withoutSuper() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  MyWidget();
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  MyWidget({super.key});
+}
+''');
+  }
+
+  Future<void> test_constructor_noParameters_withSuper_empty() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  MyWidget() : super();
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  MyWidget({super.key});
+}
+''');
+  }
+
+  Future<void> test_constructor_noParameters_withSuper_nonEmpty() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class A extends StatelessWidget {
+  const A({required this.text, Key? key}) : super(key: key);
+
+  final String text;
+}
+
+class MyWidget extends A {
+  MyWidget() : super(text: '');
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class A extends StatelessWidget {
+  const A({required this.text, Key? key}) : super(key: key);
+
+  final String text;
+}
+
+class MyWidget extends A {
+  MyWidget({super.key}) : super(text: '');
+}
+''', errorFilter: (error) => error.errorCode is LintCode);
+  }
+
+  Future<void>
+      test_constructor_noParameters_withSuper_nonEmpty_tailingComma() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class A extends StatelessWidget {
+  const A({required this.text, Key? key}) : super(key: key);
+
+  final String text;
+}
+
+class MyWidget extends A {
+  MyWidget() : super(text: '',);
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class A extends StatelessWidget {
+  const A({required this.text, Key? key}) : super(key: key);
+
+  final String text;
+}
+
+class MyWidget extends A {
+  MyWidget({super.key}) : super(text: '',);
+}
+''', errorFilter: (error) => error.errorCode is LintCode);
+  }
+
+  Future<void>
+      test_constructor_noParameters_withSuper_nonNamed_trailingComma() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class A extends StatelessWidget {
+  const A(this.widget, {Key? key}) : super(key: key);
+
+  final Widget widget;
+}
+
+class B extends A {
+  B() : super(const Text(''),);
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class A extends StatelessWidget {
+  const A(this.widget, {Key? key}) : super(key: key);
+
+  final Widget widget;
+}
+
+class B extends A {
+  B({super.key}) : super(const Text(''),);
+}
+''',
+        //TODO(asashour) there should be no other errors
+        errorFilter: (error) => error.errorCode is LintCode);
+  }
+
+  Future<void> test_initializer_final_constant() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  final t = const Text('');
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  final t = const Text('');
+
+  const MyWidget({super.key});
+}
+''',
+        //TODO(asashour) there should be no other errors
+        errorFilter: (error) => error.errorCode is LintCode);
+  }
+
+  Future<void> test_initializer_final_not_constant() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  final c = Container();
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  final c = Container();
+
+  MyWidget({super.key});
+}
+''');
+  }
+
+  Future<void> test_initializer_not_final_constant() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  var t = const Text('');
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  var t = const Text('');
+
+  MyWidget({super.key});
+}
+''',
+        //TODO(asashour) there should be no other errors
+        errorFilter: (error) => error.errorCode is LintCode);
+  }
+
+  Future<void> test_initializer_not_final_not_constant() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  var c = Container();
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  var c = Container();
+
+  MyWidget({super.key});
+}
+''');
+  }
+
+  Future<void> test_initializer_static() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  static Text t = const Text('');
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+  static Text t = const Text('');
+
+  const MyWidget({super.key});
+}
+''',
+        //TODO(asashour) there should be no other errors
+        errorFilter: (error) => error.errorCode is LintCode);
+  }
+
+  Future<void> test_super_not_constant() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class ParentWidget extends StatelessWidget {
+  final c = Container();
+
+  ParentWidget({Key? key}) : super(key: key);
+}
+
+class MyWidget extends ParentWidget {
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class ParentWidget extends StatelessWidget {
+  final c = Container();
+
+  ParentWidget({Key? key}) : super(key: key);
+}
+
+class MyWidget extends ParentWidget {
+  MyWidget({super.key});
+}
+''', errorFilter: (error) => error.errorCode is LintCode);
+  }
+}
+
+@reflectiveTest
+class AddKeyToConstructorsWithoutNamedArgumentsAnywhereTest
+    extends FixProcessorLintTest {
+  @override
+  FixKind get kind => DartFixKind.ADD_KEY_TO_CONSTRUCTORS;
+
+  @override
+  String get lintCode => LintNames.use_key_in_widget_constructors;
+
+  @override
+  String get testPackageLanguageVersion => '2.16';
+
+  @override
+  void setUp() {
+    super.setUp();
+    writeTestPackageConfig(
+      flutter: true,
+    );
+  }
+
+  Future<void> test_constructor_noParameters_withSuper_nonEmpty() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class A extends StatelessWidget {
+  const A(this.widget, {Key? key}) : super(key: key);
+
+  final Widget widget;
+}
+
+class B extends A {
+  B() : super(const Text(''));
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
+class A extends StatelessWidget {
+  const A(this.widget, {Key? key}) : super(key: key);
+
+  final Widget widget;
+}
+
+class B extends A {
+  B({Key? key}) : super(const Text(''), key: key);
+}
+''',
+        //TODO(asashour) there should be no other errors
+        errorFilter: (error) => error.errorCode is LintCode);
+  }
+}
+
+@reflectiveTest
+class AddKeyToConstructorsWithoutSuperParametersTest
+    extends FixProcessorLintTest {
+  @override
+  FixKind get kind => DartFixKind.ADD_KEY_TO_CONSTRUCTORS;
+
+  @override
+  String get lintCode => LintNames.use_key_in_widget_constructors;
+
+  @override
+  String get testPackageLanguageVersion => '2.16';
+
+  @override
+  void setUp() {
+    super.setUp();
+    writeTestPackageConfig(
+      flutter: true,
+    );
+  }
+
+  Future<void> test_class_newline() async {
+    await resolveTestCode('''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+}
+''');
+    await assertHasFix('''
+import 'package:flutter/material.dart';
+
 class MyWidget extends StatelessWidget {
   const MyWidget({Key? key}) : super(key: key);
 }
@@ -387,55 +831,3 @@ class MyWidget extends ParentWidget {
 ''', errorFilter: (error) => error.errorCode is LintCode);
   }
 }
-
-@reflectiveTest
-class AddKeyToConstructorsWithoutNamedArgumentsAnywhereTest
-    extends FixProcessorLintTest {
-  @override
-  FixKind get kind => DartFixKind.ADD_KEY_TO_CONSTRUCTORS;
-
-  @override
-  String get lintCode => LintNames.use_key_in_widget_constructors;
-
-  @override
-  String get testPackageLanguageVersion => '2.16';
-
-  @override
-  void setUp() {
-    super.setUp();
-    writeTestPackageConfig(
-      flutter: true,
-    );
-  }
-
-  Future<void> test_constructor_noParameters_withSuper_nonEmpty() async {
-    await resolveTestCode('''
-import 'package:flutter/material.dart';
-
-class A extends StatelessWidget {
-  const A(this.widget, {Key? key}) : super(key: key);
-
-  final Widget widget;
-}
-
-class B extends A {
-  B() : super(const Text(''));
-}
-''');
-    await assertHasFix('''
-import 'package:flutter/material.dart';
-
-class A extends StatelessWidget {
-  const A(this.widget, {Key? key}) : super(key: key);
-
-  final Widget widget;
-}
-
-class B extends A {
-  B({Key? key}) : super(const Text(''), key: key);
-}
-''',
-        //TODO(asashour) there should be no other errors
-        errorFilter: (error) => error.errorCode is LintCode);
-  }
-}