diff --git a/pkg/analysis_server/lib/src/cider/rename.dart b/pkg/analysis_server/lib/src/cider/rename.dart index ad326495b3e..6fe2fb61f74 100644 --- a/pkg/analysis_server/lib/src/cider/rename.dart +++ b/pkg/analysis_server/lib/src/cider/rename.dart @@ -329,10 +329,6 @@ class CheckNameResponse { } else if (node is EnumDeclaration) { var utils = CorrectionUtils(resolvedUnit); var location = utils.prepareEnumNewConstructorLocation(node); - if (location == null) { - return null; - } - var header = 'const ${interfaceElement.name}.$newName();'; return CiderReplaceMatch(libraryPath, [ ReplaceInfo(location.prefix + header + location.suffix, diff --git a/pkg/analysis_server/lib/src/services/correction/dart/create_constructor.dart b/pkg/analysis_server/lib/src/services/correction/dart/create_constructor.dart index 63fdcd31028..bdf2d501485 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/create_constructor.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/create_constructor.dart @@ -132,9 +132,6 @@ class CreateConstructor extends CorrectionProducer { // prepare location var targetLocation = CorrectionUtils(targetUnit) .prepareEnumNewConstructorLocation(targetNode); - if (targetLocation == null) { - return; - } var arguments = parent.arguments; _constructorName = diff --git 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 index e2187b13d30..2fc7b3bbf7a 100644 --- 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 @@ -5,6 +5,7 @@ import 'package:analysis_server/src/services/correction/dart/abstract_producer.dart'; import 'package:analysis_server/src/services/correction/fix.dart'; import 'package:analysis_server/src/services/correction/util.dart'; +import 'package:analysis_server/src/utilities/extensions/object.dart'; import 'package:analyzer/dart/analysis/features.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/element/element.dart'; @@ -17,32 +18,50 @@ class CreateConstructorForFinalFields extends CorrectionProducer { @override FixKind get fixKind => DartFixKind.CREATE_CONSTRUCTOR_FOR_FINAL_FIELDS; + FieldDeclaration? get _errorFieldDeclaration { + if (node is VariableDeclaration) { + final fieldDeclaration = node.parent?.parent; + return fieldDeclaration?.ifTypeOrNull(); + } + return null; + } + @override Future compute(ChangeBuilder builder) async { - if (node is! VariableDeclaration) { + final fieldDeclaration = _errorFieldDeclaration; + if (fieldDeclaration == null) { return; } - var classDeclaration = node.thisOrAncestorOfType(); - if (classDeclaration == null) { - return; + final containerDeclaration = fieldDeclaration.parent; + switch (containerDeclaration) { + case ClassDeclaration(): + await _classDeclaration( + builder: builder, + classDeclaration: containerDeclaration, + ); + case EnumDeclaration(): + await _enumDeclaration( + builder: builder, + enumDeclaration: containerDeclaration, + ); } + } + Future _classDeclaration({ + required ChangeBuilder builder, + required ClassDeclaration classDeclaration, + }) async { var className = classDeclaration.name.lexeme; var superType = classDeclaration.declaredElement?.supertype; if (superType == null) { return; } - var variableLists = []; - for (var member in classDeclaration.members) { - if (member is FieldDeclaration) { - var variableList = member.fields; - if (variableList.isFinal && !variableList.isLate) { - variableLists.add(variableList); - } - } - } + final variableLists = _interestingVariableLists( + classDeclaration.members, + ); + // prepare location for a new constructor var targetLocation = utils.prepareNewConstructorLocation( resolvedResult.session, classDeclaration); @@ -50,52 +69,101 @@ class CreateConstructorForFinalFields extends CorrectionProducer { return; } + final fixContext = _FixContext( + builder: builder, + containerName: className, + location: targetLocation, + variableLists: variableLists, + ); + if (flutter.isExactlyStatelessWidgetType(superType) || flutter.isExactlyStatefulWidgetType(superType)) { - // Specialize for Flutter widgets. - var keyClass = await sessionHelper.getClass(flutter.widgetsUri, 'Key'); - if (keyClass == null) { - return; - } - - if (unit.featureSet.isEnabled(Feature.super_parameters)) { - await _withSuperParameters( - builder, targetLocation, className, variableLists); - } else { - await _withoutSuperParameters( - builder, targetLocation, className, keyClass, variableLists); - } + await _forFlutterClass(fixContext); } else { - var fieldNames = []; - for (var variableList in variableLists) { - fieldNames.addAll(variableList.variables - .where((v) => v.initializer == null) - .map((v) => v.name.lexeme)); - } - - await builder.addDartFileEdit(file, (builder) { - builder.addInsertion(targetLocation.offset, (builder) { - builder.write(targetLocation.prefix); - builder.writeConstructorDeclaration(className, - fieldNames: fieldNames); - builder.write(targetLocation.suffix); - }); - }); + await _notFlutter( + fixContext: fixContext, + isConst: false, + ); } } - Future _withoutSuperParameters( - ChangeBuilder builder, - InsertionLocation targetLocation, - String className, - ClassElement keyClass, - List variableLists) async { + Future _enumDeclaration({ + required ChangeBuilder builder, + required EnumDeclaration enumDeclaration, + }) async { + final enumName = enumDeclaration.name.lexeme; + final variableLists = _interestingVariableLists( + enumDeclaration.members, + ); + + final targetLocation = + utils.prepareEnumNewConstructorLocation(enumDeclaration); + + await _notFlutter( + fixContext: _FixContext( + builder: builder, + containerName: enumName, + location: targetLocation, + variableLists: variableLists, + ), + isConst: true, + ); + } + + Future _forFlutterClass(_FixContext fixContext) async { + // Specialize for Flutter widgets. + var keyClass = await sessionHelper.getClass(flutter.widgetsUri, 'Key'); + if (keyClass == null) { + return; + } + + if (unit.featureSet.isEnabled(Feature.super_parameters)) { + await _withSuperParameters(fixContext); + } else { + await _withoutSuperParameters( + fixContext: fixContext, + keyClass: keyClass, + ); + } + } + + Future _notFlutter({ + required _FixContext fixContext, + required bool isConst, + }) async { + final fieldNames = fixContext.variableLists + .expand((variableList) => variableList.variables) + .where((variable) => variable.initializer == null) + .map((variable) => variable.name.lexeme) + .toList(); + + 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.writeConstructorDeclaration( + fixContext.containerName, + fieldNames: fieldNames, + ); + builder.write(location.suffix); + }); + }); + } + + Future _withoutSuperParameters({ + required _FixContext fixContext, + required ClassElement keyClass, + }) async { + final location = fixContext.location; var isNonNullable = unit.featureSet.isEnabled(Feature.non_nullable); - await builder.addDartFileEdit(file, (builder) { - builder.addInsertion(targetLocation.offset, (builder) { - builder.write(targetLocation.prefix); + await fixContext.builder.addDartFileEdit(file, (builder) { + builder.addInsertion(location.offset, (builder) { + builder.write(location.prefix); builder.write('const '); - builder.write(className); + builder.write(fixContext.containerName); builder.write('({'); builder.writeType( keyClass.instantiate( @@ -107,37 +175,45 @@ class CreateConstructorForFinalFields extends CorrectionProducer { ); builder.write(' key'); - _writeParameters(builder, variableLists, isNonNullable); + _writeParameters( + builder: builder, + variableLists: fixContext.variableLists, + isNonNullable: isNonNullable, + ); builder.write('}) : super(key: key);'); - builder.write(targetLocation.suffix); + builder.write(location.suffix); }); }); } - Future _withSuperParameters( - ChangeBuilder builder, - InsertionLocation targetLocation, - String className, - List variableLists) async { - await builder.addDartFileEdit(file, (builder) { - builder.addInsertion(targetLocation.offset, (builder) { - builder.write(targetLocation.prefix); + Future _withSuperParameters(_FixContext fixContext) async { + await fixContext.builder.addDartFileEdit(file, (builder) { + final location = fixContext.location; + builder.addInsertion(location.offset, (builder) { + builder.write(location.prefix); builder.write('const '); - builder.write(className); + builder.write(fixContext.containerName); builder.write('({'); builder.write('super.key'); - _writeParameters(builder, variableLists, true); + _writeParameters( + builder: builder, + variableLists: fixContext.variableLists, + isNonNullable: true, + ); builder.write('});'); - builder.write(targetLocation.suffix); + builder.write(location.suffix); }); }); } - void _writeParameters(DartEditBuilder builder, - List variableLists, bool isNonNullable) { + void _writeParameters({ + required DartEditBuilder builder, + required List variableLists, + required bool isNonNullable, + }) { var childrenFields = []; var childrenNullables = []; for (var variableList in variableLists) { @@ -145,17 +221,17 @@ class CreateConstructorForFinalFields extends CorrectionProducer { .where((v) => v.initializer == null) .map((v) => v.name.lexeme); + final hasNullableType = variableList.type?.type?.nullabilitySuffix == + NullabilitySuffix.question; + for (var fieldName in fieldNames) { if (fieldName == 'child' || fieldName == 'children') { childrenFields.add(fieldName); - childrenNullables.add(variableList.type?.type?.nullabilitySuffix == - NullabilitySuffix.question); + childrenNullables.add(hasNullableType); continue; } builder.write(', '); - if (isNonNullable && - variableList.type?.type?.nullabilitySuffix != - NullabilitySuffix.question) { + if (isNonNullable && !hasNullableType) { builder.write('required '); } builder.write('this.'); @@ -173,4 +249,28 @@ class CreateConstructorForFinalFields extends CorrectionProducer { builder.write(fieldName); } } + + static List _interestingVariableLists( + List members, + ) { + return members + .whereType() + .map((e) => e.fields) + .where((e) => e.isFinal && !e.isLate) + .toList(); + } +} + +class _FixContext { + final ChangeBuilder builder; + final String containerName; + final InsertionLocation location; + final List variableLists; + + _FixContext({ + required this.builder, + required this.containerName, + required this.location, + required this.variableLists, + }); } diff --git a/pkg/analysis_server/lib/src/services/correction/util.dart b/pkg/analysis_server/lib/src/services/correction/util.dart index eb202bc8f73..86c23f46807 100644 --- a/pkg/analysis_server/lib/src/services/correction/util.dart +++ b/pkg/analysis_server/lib/src/services/correction/util.dart @@ -1108,7 +1108,7 @@ class CorrectionUtils { return null; } - InsertionLocation? prepareEnumNewConstructorLocation( + InsertionLocation prepareEnumNewConstructorLocation( EnumDeclaration enumDeclaration, ) { var indent = getIndent(1); diff --git a/pkg/analysis_server/lib/src/services/refactoring/legacy/rename_constructor.dart b/pkg/analysis_server/lib/src/services/refactoring/legacy/rename_constructor.dart index 44ef5bdf67b..521a87efc66 100644 --- a/pkg/analysis_server/lib/src/services/refactoring/legacy/rename_constructor.dart +++ b/pkg/analysis_server/lib/src/services/refactoring/legacy/rename_constructor.dart @@ -207,10 +207,6 @@ class RenameConstructorRefactoringImpl extends RenameRefactoringImpl { } else if (node is EnumDeclaration) { var utils = CorrectionUtils(resolvedUnit); var location = utils.prepareEnumNewConstructorLocation(node); - if (location == null) { - return; - } - var header = 'const ${classElement.name}.$newName();'; doSourceChange_addElementEdit( change, diff --git 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 index 7d31ed00e17..463e77836f4 100644 --- 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 @@ -23,7 +23,7 @@ class CreateConstructorForFinalFieldsTest extends FixProcessorTest { @override FixKind get kind => DartFixKind.CREATE_CONSTRUCTOR_FOR_FINAL_FIELDS; - Future test_excludesLate() async { + Future test_class_excludesLate() async { await resolveTestCode(''' class Test { final int a; @@ -40,7 +40,7 @@ class Test { '''); } - Future test_flutter() async { + Future test_class_flutter() async { writeTestPackageConfig(flutter: true); await resolveTestCode(''' import 'package:flutter/widgets.dart'; @@ -66,7 +66,7 @@ class MyWidget extends StatelessWidget { }); } - Future test_flutter_childLast() async { + Future test_class_flutter_childLast() async { writeTestPackageConfig(flutter: true); await resolveTestCode(''' import 'package:flutter/widgets.dart'; @@ -92,7 +92,7 @@ class MyWidget extends StatelessWidget { }); } - Future test_flutter_childrenLast() async { + Future test_class_flutter_childrenLast() async { writeTestPackageConfig(flutter: true); await resolveTestCode(''' import 'package:flutter/widgets.dart'; @@ -118,7 +118,7 @@ class MyWidget extends StatelessWidget { }); } - Future test_inTopLevelMethod() async { + Future test_class_inTopLevelMethod() async { await resolveTestCode(''' void f() { final int v; @@ -128,7 +128,7 @@ void f() { await assertNoFix(); } - Future test_lint_sortConstructorsFirst() async { + Future test_class_lint_sortConstructorsFirst() async { createAnalysisOptionsFile(lints: [LintNames.sort_constructors_first]); await resolveTestCode(''' class Test { @@ -150,7 +150,7 @@ class Test { }); } - Future test_simple() async { + Future test_class_simple() async { await resolveTestCode(''' class Test { final int a; @@ -171,6 +171,29 @@ class Test { }); } + Future test_enum_simple() async { + await resolveTestCode(''' +enum E { + v(0, 2); + final int a; + final int b = 1; + final int c; +} +'''); + await assertHasFix(''' +enum E { + v(0, 2); + final int a; + final int b = 1; + final int c; + + const E(this.a, this.c); +} +''', errorFilter: (error) { + return error.message.contains("'a'"); + }); + } + Future test_topLevelField() async { await resolveTestCode(''' final int v; @@ -188,7 +211,7 @@ class CreateConstructorForFinalFieldsWithoutNullSafetyTest @override String get testPackageLanguageVersion => '2.9'; - Future test_flutter() async { + Future test_class_flutter() async { writeTestPackageConfig(flutter: true); await resolveTestCode(''' import 'package:flutter/widgets.dart'; @@ -214,7 +237,7 @@ class MyWidget extends StatelessWidget { }); } - Future test_flutter_childLast() async { + Future test_class_flutter_childLast() async { writeTestPackageConfig(flutter: true); await resolveTestCode(''' import 'package:flutter/widgets.dart'; @@ -240,7 +263,7 @@ class MyWidget extends StatelessWidget { }); } - Future test_flutter_childrenLast() async { + Future test_class_flutter_childrenLast() async { writeTestPackageConfig(flutter: true); await resolveTestCode(''' import 'package:flutter/widgets.dart'; @@ -276,7 +299,7 @@ class CreateConstructorForFinalFieldsWithoutSuperParametersTest @override String get testPackageLanguageVersion => '2.16'; - Future test_flutter() async { + Future test_class_flutter() async { writeTestPackageConfig(flutter: true); await resolveTestCode(''' import 'package:flutter/widgets.dart'; @@ -302,7 +325,7 @@ class MyWidget extends StatelessWidget { }); } - Future test_flutter_childLast() async { + Future test_class_flutter_childLast() async { writeTestPackageConfig(flutter: true); await resolveTestCode(''' import 'package:flutter/widgets.dart'; @@ -328,7 +351,7 @@ class MyWidget extends StatelessWidget { }); } - Future test_flutter_childrenLast() async { + Future test_class_flutter_childrenLast() async { writeTestPackageConfig(flutter: true); await resolveTestCode(''' import 'package:flutter/widgets.dart';