Support for private fields in CreateConstructorForFinalFields when using named formal parameters.

Change-Id: I246c8b9f9b5e26ad778648d2d5e12a952bc98395 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/308801 Commit-Queue: Konstantin Shcheglov <scheglov@google.com> Reviewed-by: Samuel Rawlins <srawlins@google.com>
2024-09-15 23:29:47 +00:00 · 2023-06-12 21:59:28 +00:00 · 2023-06-12 21:59:28 +00:00 · 379ab45f2c
parent 27e5ab180d
commit 379ab45f2c
2 changed files with 197 additions and 45 deletions
--- a/pkg/analysis_server/lib/src/services/correction/dart/create_constructor_for_final_fields.dart
+++ b/pkg/analysis_server/lib/src/services/correction/dart/create_constructor_for_final_fields.dart
@ -126,7 +126,7 @@ class CreateConstructorForFinalFields extends CorrectionProducer {
    );
  }

-  List<_Field> _fieldsToWrite(
+  List<_Field>? _fieldsToWrite(
    List<VariableDeclarationList> variableLists,
  ) {
    final result = <_Field>[];
@ -136,10 +136,22 @@ class CreateConstructorForFinalFields extends CorrectionProducer {
          type != null && typeSystem.isPotentiallyNonNullable(type);

      for (final field in variableList.variables) {
+        final typeAnnotation = variableList.type;
+        if (typeAnnotation == null) {
+          return null;
+        }
        if (field.initializer == null) {
+          final fieldName = field.name.lexeme;
+          final namedFormalParameterName =
+              _Field.computeNamedFormalParameterName(fieldName);
+          if (namedFormalParameterName == null) {
+            return null;
+          }
          result.add(
            _Field(
-              name: field.name.lexeme,
+              typeAnnotation: typeAnnotation,
+              fieldName: fieldName,
+              namedFormalParameterName: namedFormalParameterName,
              hasNonNullableType: hasNonNullableType,
            ),
          );
@ -221,7 +233,95 @@ class CreateConstructorForFinalFields extends CorrectionProducer {
    required bool isConst,
  }) async {
    final fields = _fieldsToWrite(fixContext.variableLists);
+    if (fields == null) {
+      return;
+    }
+
+    switch (_style) {
+      case _Style.requiredNamed:
+        await _notFlutterNamed(
+          fixContext: fixContext,
+          isConst: isConst,
+          fields: fields,
+        );
+      case _Style.requiredPositional:
+        await _notFlutterRequiredPositional(
+          fixContext: fixContext,
+          isConst: isConst,
+          fields: fields,
+        );
+    }
+  }
+
+  Future<void> _notFlutterNamed({
+    required _FixContext fixContext,
+    required bool isConst,
+    required List<_Field> fields,
+  }) async {
+    final fieldsForInitializers = <_Field>[];

+    final location = fixContext.location;
+    await fixContext.builder.addDartFileEdit(file, (builder) {
+      builder.addInsertion(location.offset, (builder) {
+        builder.write(location.prefix);
+        if (isConst) {
+          builder.write('const ');
+        }
+        builder.write(fixContext.containerName);
+        builder.write('({');
+        var hasWritten = false;
+        final superNamed = fixContext.superNamed;
+        if (superNamed != null) {
+          for (final formalParameter in superNamed) {
+            if (hasWritten) {
+              builder.write(', ');
+            }
+            if (formalParameter.isRequiredNamed) {
+              builder.write('required ');
+            }
+            builder.write('super.');
+            builder.write(formalParameter.name);
+            hasWritten = true;
+          }
+        }
+        for (final field in fields) {
+          if (hasWritten) {
+            builder.write(', ');
+          }
+          if (field.namedFormalParameterName == field.fieldName) {
+            builder.write('required this.');
+            builder.write(field.fieldName);
+          } else {
+            builder.write('required ');
+            builder.write(
+              utils.getNodeText(field.typeAnnotation),
+            );
+            builder.write(' ');
+            builder.write(field.namedFormalParameterName);
+            fieldsForInitializers.add(field);
+          }
+          hasWritten = true;
+        }
+        builder.write('})');
+
+        if (fieldsForInitializers.isNotEmpty) {
+          final code = fieldsForInitializers.map((field) {
+            return '${field.fieldName} = ${field.namedFormalParameterName}';
+          }).join(', ');
+          builder.write(' : $code');
+        }
+
+        builder.write(';');
+        builder.write(location.suffix);
+      });
+    });
+  }
+
+  Future<void> _notFlutterRequiredPositional({
+    required _FixContext fixContext,
+    required bool isConst,
+    required List<_Field> fields,
+  }) async {
    final location = fixContext.location;
    await fixContext.builder.addDartFileEdit(file, (builder) {
      builder.addInsertion(location.offset, (builder) {
@ -231,43 +331,14 @@ class CreateConstructorForFinalFields extends CorrectionProducer {
        }
        builder.write(fixContext.containerName);
        builder.write('(');
-        switch (_style) {
-          case _Style.requiredNamed:
-            builder.write('{');
-            var hasWritten = false;
-            final superNamed = fixContext.superNamed;
-            if (superNamed != null) {
-              for (final formalParameter in superNamed) {
-                if (hasWritten) {
-                  builder.write(', ');
-                }
-                if (formalParameter.isRequiredNamed) {
-                  builder.write('required ');
-                }
-                builder.write('super.');
-                builder.write(formalParameter.name);
-                hasWritten = true;
-              }
-            }
-            for (final field in fields) {
-              if (hasWritten) {
-                builder.write(', ');
-              }
-              builder.write('required this.');
-              builder.write(field.name);
-              hasWritten = true;
-            }
-            builder.write('}');
-          case _Style.requiredPositional:
-            var hasWritten = false;
-            for (final field in fields) {
-              if (hasWritten) {
-                builder.write(', ');
-              }
-              builder.write('this.');
-              builder.write(field.name);
-              hasWritten = true;
-            }
+        var hasWritten = false;
+        for (final field in fields) {
+          if (hasWritten) {
+            builder.write(', ');
+          }
+          builder.write('this.');
+          builder.write(field.fieldName);
+          hasWritten = true;
        }
        builder.write(');');
        builder.write(location.suffix);
@ -280,6 +351,10 @@ class CreateConstructorForFinalFields extends CorrectionProducer {
    required List<VariableDeclarationList> variableLists,
  }) {
    final fields = _fieldsToWrite(variableLists);
+    if (fields == null) {
+      return;
+    }
+
    final childrenLast = [
      ...fields.whereNot((field) => field.isChild),
      ...fields.where((field) => field.isChild),
@ -291,7 +366,7 @@ class CreateConstructorForFinalFields extends CorrectionProducer {
        builder.write('required ');
      }
      builder.write('this.');
-      builder.write(field.name);
+      builder.write(field.fieldName);
    }
  }

@ -307,16 +382,36 @@ class CreateConstructorForFinalFields extends CorrectionProducer {
 }

 class _Field {
-  final String name;
+  final TypeAnnotation typeAnnotation;
+  final String fieldName;
+  final String namedFormalParameterName;
  final bool hasNonNullableType;

  _Field({
-    required this.name,
+    required this.typeAnnotation,
+    required this.fieldName,
+    required this.namedFormalParameterName,
    required this.hasNonNullableType,
  });

  bool get isChild {
-    return const {'child', 'children'}.contains(name);
+    return const {'child', 'children'}.contains(fieldName);
+  }
+
+  /// Returns the name for the corresponding named formal parameters, or
+  /// `null` if such name cannot be computed, so that the quick fix cannot
+  /// be computed.
+  static String? computeNamedFormalParameterName(String fieldName) {
+    var result = fieldName;
+    while (true) {
+      if (result.isEmpty) {
+        return null;
+      } else if (result.startsWith('_')) {
+        result = result.substring(1);
+      } else {
+        return result;
+      }
+    }
  }
 }

--- a/pkg/analysis_server/test/src/services/correction/fix/create_constructor_for_final_fields_test.dart
+++ b/pkg/analysis_server/test/src/services/correction/fix/create_constructor_for_final_fields_test.dart
@ -4,6 +4,7 @@

 import 'package:analysis_server/src/services/correction/fix.dart';
 import 'package:analysis_server/src/services/linter/lint_names.dart';
+import 'package:analyzer/src/error/codes.dart';
 import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';

@ -132,7 +133,7 @@ class Test {
    });
  }

-  Future<void> test_class_simple() async {
+  Future<void> test_class_noSuperClass() async {
    await resolveTestCode('''
 class Test {
  final int a;
@ -153,7 +154,42 @@ class Test {
    });
  }

-  Future<void> test_enum_simple() async {
+  Future<void> test_class_noSuperClass_hasPrivate() async {
+    await resolveTestCode('''
+class Test {
+  final int _a;
+  final int _b;
+  final int c;
+}
+''');
+    await assertHasFix('''
+class Test {
+  final int _a;
+  final int _b;
+  final int c;
+
+  Test({required int a, required int b, required this.c}) : _a = a, _b = b;
+}
+''', errorFilter: (error) {
+      return error.errorCode == CompileTimeErrorCode.FINAL_NOT_INITIALIZED &&
+          error.message.contains("'_a'");
+    });
+  }
+
+  Future<void> test_class_noSuperClass_hasPrivate_onlyUnderscores() async {
+    await resolveTestCode('''
+class Test {
+  final int a;
+  final int _;
+  final int __;
+}
+''');
+    await assertNoFix(errorFilter: (error) {
+      return error.message.contains("'a'");
+    });
+  }
+
+  Future<void> test_enum() async {
    await resolveTestCode('''
 enum E {
  v(a: 0, c: 2);
@ -278,6 +314,27 @@ class MyWidget extends StatelessWidget {
    });
  }

+  Future<void> test_class_hasPrivate() async {
+    await resolveTestCode('''
+class Test {
+  final int a;
+  final int _b;
+  final int c;
+}
+''');
+    await assertHasFix('''
+class Test {
+  final int a;
+  final int _b;
+  final int c;
+
+  Test(this.a, this._b, this.c);
+}
+''', errorFilter: (error) {
+      return error.message.contains("'a'");
+    });
+  }
+
  Future<void> test_class_inTopLevelMethod() async {
    await resolveTestCode('''
 void f() {