From fae7752e9e5087aa51d8eaaf19aa949a0337ca8b Mon Sep 17 00:00:00 2001 From: FMorschel Date: Thu, 5 Jun 2025 16:58:40 -0700 Subject: [PATCH] [DAS] Adds tests for fix priorities and new priority for `Create class` The related issue asked for a new priority for the `Create class` fix that would be lower if the undefined name was lowercase, giving the `Create method` and `Create function` fixes a higher relative priority. This change also adds a new abstract class to test the relative priority between fix kinds. It is also used to test agains the merge combinators fixes. Fixes: https://github.com/dart-lang/sdk/issues/60523 Change-Id: I938f52a577ecf1b6bb8dd66c94fd45395a011ffa Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422321 Auto-Submit: Felipe Morschel Reviewed-by: Samuel Rawlins Reviewed-by: Brian Wilkerson Commit-Queue: Samuel Rawlins --- .../correction/dart/create_class.dart | 165 +++++++++++----- .../lib/src/services/correction/fix.dart | 62 +++--- .../src/services/correction/fix_internal.dart | 85 ++++----- pkg/analysis_server/test/edit/fixes_test.dart | 6 +- .../shared_code_actions_fixes_tests.dart | 10 +- .../correction/fix/create_class_test.dart | 178 +++++++++++++++++- .../correction/fix/fix_processor.dart | 54 +++++- .../fix/merge_combinators_test.dart | 39 +++- 8 files changed, 450 insertions(+), 149 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/create_class.dart b/pkg/analysis_server/lib/src/services/correction/dart/create_class.dart index 3299ded6d45..cb812c26430 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/create_class.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/create_class.dart @@ -11,24 +11,13 @@ import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dar import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; import 'package:analyzer_plugin/utilities/range_factory.dart'; -class CreateClass extends ResolvedCorrectionProducer { - String className = ''; +class CreateClass extends MultiCorrectionProducer { + static final _lowerCaseRegex = RegExp(r'([_\$]||[_\$]+[0-9])*[a-z]'); CreateClass({required super.context}); @override - CorrectionApplicability get applicability => - // TODO(applicability): comment on why. - CorrectionApplicability.singleLocation; - - @override - List get fixArguments => [className]; - - @override - FixKind get fixKind => DartFixKind.CREATE_CLASS; - - @override - Future compute(ChangeBuilder builder) async { + Future> get producers async { var targetNode = node; Element? prefixElement; ArgumentList? arguments; @@ -41,7 +30,7 @@ class CreateClass extends ResolvedCorrectionProducer { if (name.element != null || arguments == null) { // TODO(brianwilkerson): Consider supporting creating a class when the // arguments are missing by also adding an empty argument list. - return; + return const []; } targetNode = name; requiresConstConstructor = true; @@ -51,7 +40,7 @@ class CreateClass extends ResolvedCorrectionProducer { if (importPrefix != null) { prefixElement = importPrefix.element2; if (prefixElement == null) { - return; + return const []; } } className = targetNode.name.lexeme; @@ -59,32 +48,124 @@ class CreateClass extends ResolvedCorrectionProducer { } else if (targetNode case SimpleIdentifier( :var parent, ) when parent is! PropertyAccess && parent is! PrefixedIdentifier) { - className = targetNode.nameOfType; + className = targetNode.nameOfType ?? targetNode.name; requiresConstConstructor |= _requiresConstConstructor(targetNode); } else if (targetNode is PrefixedIdentifier) { prefixElement = targetNode.prefix.element; if (prefixElement == null) { - return; + return const []; } - className = targetNode.identifier.nameOfType; + className = + targetNode.identifier.nameOfType ?? targetNode.identifier.name; } else { - return; + return const []; } - if (className == null) { - return; + if (className.isEmpty) { + return const []; } - this.className = className; + // Lowercase class names are valid but not idiomatic so lower the priority. + if (className.startsWith(_lowerCaseRegex)) { + return [ + _CreateClass.lowercase( + context: context, + targetNode: targetNode, + prefixElement: prefixElement, + className: className, + requiresConstConstructor: requiresConstConstructor, + arguments: arguments, + ), + ]; + } else { + return [ + _CreateClass.uppercase( + context: context, + targetNode: targetNode, + prefixElement: prefixElement, + className: className, + requiresConstConstructor: requiresConstConstructor, + arguments: arguments, + ), + ]; + } + } + static bool _requiresConstConstructor(AstNode node) { + var parent = node.parent; + // TODO(scheglov): remove after NamedType refactoring. + if (node is SimpleIdentifier && parent is NamedType) { + return _requiresConstConstructor(parent); + } + if (node is SimpleIdentifier && parent is MethodInvocation) { + return parent.inConstantContext; + } + if (node is NamedType && parent is ConstructorName) { + return _requiresConstConstructor(parent); + } + if (node is ConstructorName && parent is InstanceCreationExpression) { + return parent.isConst; + } + return false; + } +} + +class _CreateClass extends ResolvedCorrectionProducer { + final ArgumentList? _arguments; + final bool _requiresConstConstructor; + final AstNode _targetNode; + final Element? _prefixElement; + final String _className; + + @override + final FixKind fixKind; + + _CreateClass.lowercase({ + required super.context, + required ArgumentList? arguments, + required bool requiresConstConstructor, + required AstNode targetNode, + required Element? prefixElement, + required String className, + }) : _className = className, + _prefixElement = prefixElement, + _targetNode = targetNode, + _requiresConstConstructor = requiresConstConstructor, + _arguments = arguments, + fixKind = DartFixKind.CREATE_CLASS_LOWERCASE; + + _CreateClass.uppercase({ + required super.context, + required ArgumentList? arguments, + required bool requiresConstConstructor, + required AstNode targetNode, + required Element? prefixElement, + required String className, + }) : _className = className, + _prefixElement = prefixElement, + _targetNode = targetNode, + _requiresConstConstructor = requiresConstConstructor, + _arguments = arguments, + fixKind = DartFixKind.CREATE_CLASS_UPPERCASE; + + @override + CorrectionApplicability get applicability => + // TODO(applicability): comment on why. + CorrectionApplicability.singleLocation; + + @override + List get fixArguments => [_className]; + + @override + Future compute(ChangeBuilder builder) async { // prepare environment LibraryFragment targetUnit; var prefix = ''; var suffix = ''; var offset = -1; String? filePath; - if (prefixElement == null) { + if (_prefixElement == null) { targetUnit = unit.declaredFragment!; - var enclosingMember = targetNode.thisOrAncestorMatching( + var enclosingMember = _targetNode.thisOrAncestorMatching( (node) => node is CompilationUnitMember && node.parent is CompilationUnit, ); @@ -96,8 +177,8 @@ class CreateClass extends ResolvedCorrectionProducer { prefix = '$eol$eol'; } else { for (var import in libraryElement2.firstFragment.libraryImports2) { - if (prefixElement is PrefixElement && - import.prefix2?.element == prefixElement) { + if (_prefixElement is PrefixElement && + import.prefix2?.element == _prefixElement) { var library = import.importedLibrary2; if (library != null) { targetUnit = library.firstFragment; @@ -120,11 +201,11 @@ class CreateClass extends ResolvedCorrectionProducer { return; } - var className2 = className; + var className2 = _className; await builder.addDartFileEdit(filePath, (builder) { builder.addInsertion(offset, (builder) { builder.write(prefix); - if (arguments == null && !requiresConstConstructor) { + if (_arguments == null && !_requiresConstConstructor) { builder.writeClassDeclaration(className2, nameGroupName: 'NAME'); } else { builder.writeClassDeclaration( @@ -134,9 +215,9 @@ class CreateClass extends ResolvedCorrectionProducer { builder.write(' '); builder.writeConstructorDeclaration( className2, - argumentList: arguments, + argumentList: _arguments, classNameGroupName: 'NAME', - isConst: requiresConstConstructor, + isConst: _requiresConstConstructor, ); builder.writeln(); }, @@ -144,29 +225,11 @@ class CreateClass extends ResolvedCorrectionProducer { } builder.write(suffix); }); - if (prefixElement == null) { - builder.addLinkedPosition(range.node(targetNode), 'NAME'); + if (_prefixElement == null) { + builder.addLinkedPosition(range.node(_targetNode), 'NAME'); } }); } - - static bool _requiresConstConstructor(AstNode node) { - var parent = node.parent; - // TODO(scheglov): remove after NamedType refactoring. - if (node is SimpleIdentifier && parent is NamedType) { - return _requiresConstConstructor(parent); - } - if (node is SimpleIdentifier && parent is MethodInvocation) { - return parent.inConstantContext; - } - if (node is NamedType && parent is ConstructorName) { - return _requiresConstConstructor(parent); - } - if (node is ConstructorName && parent is InstanceCreationExpression) { - return parent.isConst; - } - return false; - } } extension on AstNode { diff --git a/pkg/analysis_server/lib/src/services/correction/fix.dart b/pkg/analysis_server/lib/src/services/correction/fix.dart index 269f0a5a940..bac13196bb5 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix.dart @@ -10,22 +10,22 @@ import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; abstract final class AnalysisOptionsFixKind { static const REMOVE_LINT = FixKind( 'analysisOptions.fix.removeLint', - 50, + DartFixKindPriority.standard, "Remove '{0}'", ); static const REMOVE_SETTING = FixKind( 'analysisOptions.fix.removeSetting', - 50, + DartFixKindPriority.standard, "Remove '{0}'", ); static const REPLACE_WITH_STRICT_CASTS = FixKind( 'analysisOptions.fix.replaceWithStrictCasts', - 50, + DartFixKindPriority.standard, 'Replace with the strict-casts analysis mode', ); static const REPLACE_WITH_STRICT_RAW_TYPES = FixKind( 'analysisOptions.fix.replaceWithStrictRawTypes', - 50, + DartFixKindPriority.standard, 'Replace with the strict-raw-types analysis mode', ); } @@ -159,7 +159,7 @@ abstract final class DartFixKind { ); static const ADD_INITIALIZING_FORMAL_PARAMETERS = FixKind( 'dart.fix.add.initializingFormalParameters', - 70, + DartFixKindPriority.standard + 20, 'Add final initializing formal parameters', ); static const ADD_KEY_TO_CONSTRUCTORS = FixKind( @@ -194,22 +194,22 @@ abstract final class DartFixKind { ); static const ADD_MISSING_PARAMETER_NAMED = FixKind( 'dart.fix.add.missingParameterNamed', - 70, + DartFixKindPriority.standard + 20, "Add named parameter '{0}'", ); static const ADD_MISSING_PARAMETER_POSITIONAL = FixKind( 'dart.fix.add.missingParameterPositional', - 69, + DartFixKindPriority.standard + 19, 'Add optional positional parameter', ); static const ADD_MISSING_PARAMETER_REQUIRED = FixKind( 'dart.fix.add.missingParameterRequired', - 70, + DartFixKindPriority.standard + 20, 'Add required positional parameter', ); static const ADD_MISSING_REQUIRED_ARGUMENT = FixKind( 'dart.fix.add.missingRequiredArgument', - 70, + DartFixKindPriority.standard + 20, 'Add {0} required argument{1}', ); static const ADD_MISSING_SWITCH_CASES = FixKind( @@ -334,7 +334,7 @@ abstract final class DartFixKind { ); static const CHANGE_ARGUMENT_NAME = FixKind( 'dart.fix.change.argumentName', - 60, + DartFixKindPriority.standard + 10, "Change to '{0}'", ); static const CHANGE_TO = FixKind( @@ -455,7 +455,7 @@ abstract final class DartFixKind { ); static const CONVERT_TO_CONSTANT_PATTERN = FixKind( 'dart.fix.convert.toConstantPattern', - 49, + DartFixKindPriority.standard - 1, 'Convert to constant pattern', ); static const CONVERT_TO_CONTAINS = FixKind( @@ -700,12 +700,12 @@ abstract final class DartFixKind { ); static const CONVERT_TO_SUPER_PARAMETERS = FixKind( 'dart.fix.convert.toSuperParameters', - 30, + DartFixKindPriority.ignore, 'Convert to using super parameters', ); static const CONVERT_TO_SUPER_PARAMETERS_MULTI = FixKind( 'dart.fix.convert.toSuperParameters.multi', - 30, + DartFixKindPriority.ignore, 'Convert to using super parameters everywhere in file', ); static const CONVERT_TO_WHERE_TYPE = FixKind( @@ -728,9 +728,14 @@ abstract final class DartFixKind { DartFixKindPriority.standard, 'Convert to wildcard variable', ); - static const CREATE_CLASS = FixKind( - 'dart.fix.create.class', - DartFixKindPriority.standard, + static const CREATE_CLASS_UPPERCASE = FixKind( + 'dart.fix.create.class.uppercase', + DartFixKindPriority.standard + 2, + "Create class '{0}'", + ); + static const CREATE_CLASS_LOWERCASE = FixKind( + 'dart.fix.create.class.lowercase', + DartFixKindPriority.standard - 5, "Create class '{0}'", ); static const CREATE_CONSTRUCTOR = FixKind( @@ -755,27 +760,27 @@ abstract final class DartFixKind { ); static const CREATE_EXTENSION_GETTER = FixKind( 'dart.fix.create.extension.getter', - DartFixKindPriority.standard - 20, + DartFixKindPriority.ignore, "Create extension getter '{0}'", ); static const CREATE_EXTENSION_METHOD = FixKind( 'dart.fix.create.extension.method', - DartFixKindPriority.standard - 20, + DartFixKindPriority.ignore, "Create extension method '{0}'", ); static const CREATE_EXTENSION_OPERATOR = FixKind( 'dart.fix.create.extension.operator', - DartFixKindPriority.standard - 20, + DartFixKindPriority.ignore, "Create extension operator '{0}'", ); static const CREATE_EXTENSION_SETTER = FixKind( 'dart.fix.create.extension.setter', - DartFixKindPriority.standard - 20, + DartFixKindPriority.ignore, "Create extension setter '{0}'", ); static const CREATE_FIELD = FixKind( 'dart.fix.create.field', - 49, + DartFixKindPriority.standard - 1, "Create field '{0}'", ); static const CREATE_FILE = FixKind( @@ -785,7 +790,7 @@ abstract final class DartFixKind { ); static const CREATE_FUNCTION = FixKind( 'dart.fix.create.function', - 49, + DartFixKindPriority.standard - 1, "Create function '{0}'", ); static const CREATE_GETTER = FixKind( @@ -822,7 +827,7 @@ abstract final class DartFixKind { ); static const CREATE_NO_SUCH_METHOD = FixKind( 'dart.fix.create.noSuchMethod', - 49, + DartFixKindPriority.standard - 1, "Create 'noSuchMethod' method", ); static const CREATE_PARAMETER = FixKind( @@ -850,11 +855,6 @@ abstract final class DartFixKind { DartFixKindPriority.standard, 'Extract local variable', ); - static const IMPORT_ASYNC = FixKind( - 'dart.fix.import.async', - 49, - "Import 'dart:async'", - ); static const IMPORT_LIBRARY_COMBINATOR = FixKind( 'dart.fix.import.libraryCombinator', DartFixKindPriority.standard + 5, @@ -962,7 +962,7 @@ abstract final class DartFixKind { ); static const INLINE_INVOCATION = FixKind( 'dart.fix.inlineInvocation', - DartFixKindPriority.standard - 20, + DartFixKindPriority.ignore, "Inline invocation of '{0}'", ); static const INLINE_INVOCATION_MULTI = FixKind( @@ -972,7 +972,7 @@ abstract final class DartFixKind { ); static const INLINE_TYPEDEF = FixKind( 'dart.fix.inlineTypedef', - DartFixKindPriority.standard - 20, + DartFixKindPriority.ignore, "Inline the definition of '{0}'", ); static const INLINE_TYPEDEF_MULTI = FixKind( @@ -1542,7 +1542,7 @@ abstract final class DartFixKind { ); static const REMOVE_TYPE_ARGUMENTS = FixKind( 'dart.fix.remove.typeArguments', - 49, + DartFixKindPriority.standard - 1, 'Remove type arguments', ); static const REMOVE_TYPE_CHECK = FixKind( diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 43e52120bd6..14459d91c9c 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -604,7 +604,6 @@ final _builtInNonLintGenerators = >{ CompileTimeErrorCode.BODY_MIGHT_COMPLETE_NORMALLY: [AddAsync.missingReturn], CompileTimeErrorCode.CAST_TO_NON_TYPE: [ ChangeTo.classOrMixin, - CreateClass.new, CreateMixin.new, ], CompileTimeErrorCode.CLASS_INSTANTIATION_ACCESS_TO_STATIC_MEMBER: [ @@ -621,10 +620,7 @@ final _builtInNonLintGenerators = >{ ], CompileTimeErrorCode.CONST_INSTANCE_FIELD: [AddStatic.new], CompileTimeErrorCode.CONST_WITH_NON_CONST: [RemoveConst.new], - CompileTimeErrorCode.CONST_WITH_NON_TYPE: [ - ChangeTo.classOrMixin, - CreateClass.new, - ], + CompileTimeErrorCode.CONST_WITH_NON_TYPE: [ChangeTo.classOrMixin], CompileTimeErrorCode.CONSTANT_PATTERN_WITH_NON_CONSTANT_EXPRESSION: [ AddConst.new, ], @@ -644,7 +640,6 @@ final _builtInNonLintGenerators = >{ ], CompileTimeErrorCode.EXTENDS_NON_CLASS: [ ChangeTo.classOrMixin, - CreateClass.new, RemoveNameFromDeclarationClause.new, ], CompileTimeErrorCode.EXTENDS_TYPE_ALIAS_EXPANDS_TO_TYPE_PARAMETER: [ @@ -710,10 +705,7 @@ final _builtInNonLintGenerators = >{ CompileTimeErrorCode.IMPLEMENTS_DISALLOWED_CLASS: [ RemoveNameFromDeclarationClause.new, ], - CompileTimeErrorCode.IMPLEMENTS_NON_CLASS: [ - ChangeTo.classOrMixin, - CreateClass.new, - ], + CompileTimeErrorCode.IMPLEMENTS_NON_CLASS: [ChangeTo.classOrMixin], CompileTimeErrorCode.IMPLEMENTS_REPEATED: [ RemoveNameFromDeclarationClause.new, ], @@ -742,10 +734,7 @@ final _builtInNonLintGenerators = >{ CompileTimeErrorCode.INTEGER_LITERAL_IMPRECISE_AS_DOUBLE: [ ChangeToNearestPreciseValue.new, ], - CompileTimeErrorCode.INVALID_ANNOTATION: [ - ChangeTo.annotation, - CreateClass.new, - ], + CompileTimeErrorCode.INVALID_ANNOTATION: [ChangeTo.annotation], CompileTimeErrorCode.INVALID_ASSIGNMENT: [ AddExplicitCast.new, AddNullCheck.new, @@ -800,20 +789,14 @@ final _builtInNonLintGenerators = >{ CompileTimeErrorCode.MIXIN_OF_DISALLOWED_CLASS: [ RemoveNameFromDeclarationClause.new, ], - CompileTimeErrorCode.MIXIN_OF_NON_CLASS: [ - ChangeTo.classOrMixin, - CreateClass.new, - ], + CompileTimeErrorCode.MIXIN_OF_NON_CLASS: [ChangeTo.classOrMixin], CompileTimeErrorCode.MIXIN_SUPER_CLASS_CONSTRAINT_DISALLOWED_CLASS: [ RemoveNameFromDeclarationClause.new, ], CompileTimeErrorCode.MIXIN_SUPER_CLASS_CONSTRAINT_NON_INTERFACE: [ RemoveNameFromDeclarationClause.new, ], - CompileTimeErrorCode.NEW_WITH_NON_TYPE: [ - ChangeTo.classOrMixin, - CreateClass.new, - ], + CompileTimeErrorCode.NEW_WITH_NON_TYPE: [ChangeTo.classOrMixin], CompileTimeErrorCode.NEW_WITH_UNDEFINED_CONSTRUCTOR: [CreateConstructor.new], CompileTimeErrorCode.NO_ANNOTATION_CONSTRUCTOR_ARGUMENTS: [ AddEmptyArgumentList.new, @@ -870,15 +853,8 @@ final _builtInNonLintGenerators = >{ AddMissingSwitchCases.new, ], CompileTimeErrorCode.NON_FINAL_FIELD_IN_ENUM: [MakeFinal.new], - CompileTimeErrorCode.NON_TYPE_AS_TYPE_ARGUMENT: [ - CreateClass.new, - CreateMixin.new, - ], - CompileTimeErrorCode.NOT_A_TYPE: [ - ChangeTo.classOrMixin, - CreateClass.new, - CreateMixin.new, - ], + CompileTimeErrorCode.NON_TYPE_AS_TYPE_ARGUMENT: [CreateMixin.new], + CompileTimeErrorCode.NOT_A_TYPE: [ChangeTo.classOrMixin, CreateMixin.new], CompileTimeErrorCode.NOT_INITIALIZED_NON_NULLABLE_INSTANCE_FIELD: [ AddLate.new, ], @@ -935,7 +911,6 @@ final _builtInNonLintGenerators = >{ CompileTimeErrorCode.SWITCH_CASE_COMPLETES_NORMALLY: [AddSwitchCaseBreak.new], CompileTimeErrorCode.TYPE_TEST_WITH_UNDEFINED_NAME: [ ChangeTo.classOrMixin, - CreateClass.new, CreateMixin.new, ], CompileTimeErrorCode.UNCHECKED_INVOCATION_OF_NULLABLE_VALUE: [ @@ -971,13 +946,9 @@ final _builtInNonLintGenerators = >{ CompileTimeErrorCode.UNCHECKED_USE_OF_NULLABLE_VALUE_IN_YIELD_EACH: [ AddNullCheck.new, ], - CompileTimeErrorCode.UNDEFINED_ANNOTATION: [ - ChangeTo.annotation, - CreateClass.new, - ], + CompileTimeErrorCode.UNDEFINED_ANNOTATION: [ChangeTo.annotation], CompileTimeErrorCode.UNDEFINED_CLASS: [ ChangeTo.classOrMixin, - CreateClass.new, CreateMixin.new, ], CompileTimeErrorCode.UNDEFINED_CLASS_BOOLEAN: [ReplaceBooleanWithBool.new], @@ -1007,12 +978,10 @@ final _builtInNonLintGenerators = >{ ], CompileTimeErrorCode.UNDEFINED_FUNCTION: [ ChangeTo.function, - CreateClass.new, CreateFunction.new, ], CompileTimeErrorCode.UNDEFINED_GETTER: [ ChangeTo.getterOrSetter, - CreateClass.new, CreateExtensionGetter.new, CreateField.new, CreateGetter.new, @@ -1022,7 +991,6 @@ final _builtInNonLintGenerators = >{ ], CompileTimeErrorCode.UNDEFINED_IDENTIFIER: [ ChangeTo.getterOrSetter, - CreateClass.new, CreateField.new, CreateGetter.new, CreateLocalVariable.new, @@ -1037,7 +1005,6 @@ final _builtInNonLintGenerators = >{ CompileTimeErrorCode.UNDEFINED_IDENTIFIER_AWAIT: [AddAsync.new], CompileTimeErrorCode.UNDEFINED_METHOD: [ ChangeTo.method, - CreateClass.new, CreateExtensionMethod.new, CreateFunction.new, CreateMethod.method, @@ -1338,11 +1305,16 @@ final _builtInNonLintMultiGenerators = { CompileTimeErrorCode.AMBIGUOUS_IMPORT: [AmbiguousImportFix.new], CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE: [DataDriven.new], CompileTimeErrorCode.CAST_TO_NON_TYPE: [ + CreateClass.new, DataDriven.new, ImportLibrary.forType, ], - CompileTimeErrorCode.CONST_WITH_NON_TYPE: [ImportLibrary.forType], + CompileTimeErrorCode.CONST_WITH_NON_TYPE: [ + CreateClass.new, + ImportLibrary.forType, + ], CompileTimeErrorCode.EXTENDS_NON_CLASS: [ + CreateClass.new, DataDriven.new, ImportLibrary.forType, ], @@ -1355,6 +1327,7 @@ final _builtInNonLintMultiGenerators = { DataDriven.new, ], CompileTimeErrorCode.IMPLEMENTS_NON_CLASS: [ + CreateClass.new, DataDriven.new, ImportLibrary.forType, ], @@ -1362,6 +1335,7 @@ final _builtInNonLintMultiGenerators = { AddSuperConstructorInvocation.new, ], CompileTimeErrorCode.INVALID_ANNOTATION: [ + CreateClass.new, ImportLibrary.forTopLevelVariable, ImportLibrary.forType, ], @@ -1369,10 +1343,14 @@ final _builtInNonLintMultiGenerators = { CompileTimeErrorCode.INVALID_OVERRIDE_SETTER: [DataDriven.new], CompileTimeErrorCode.MISSING_REQUIRED_ARGUMENT: [DataDriven.new], CompileTimeErrorCode.MIXIN_OF_NON_CLASS: [ + CreateClass.new, DataDriven.new, ImportLibrary.forType, ], - CompileTimeErrorCode.NEW_WITH_NON_TYPE: [ImportLibrary.forType], + CompileTimeErrorCode.NEW_WITH_NON_TYPE: [ + CreateClass.new, + ImportLibrary.forType, + ], CompileTimeErrorCode.NEW_WITH_UNDEFINED_CONSTRUCTOR_DEFAULT: [DataDriven.new], CompileTimeErrorCode.NO_DEFAULT_SUPER_CONSTRUCTOR_EXPLICIT: [ AddSuperConstructorInvocation.new, @@ -1383,10 +1361,11 @@ final _builtInNonLintMultiGenerators = { ], CompileTimeErrorCode.NON_TYPE_IN_CATCH_CLAUSE: [ImportLibrary.forType], CompileTimeErrorCode.NON_TYPE_AS_TYPE_ARGUMENT: [ - ImportLibrary.forType, + CreateClass.new, DataDriven.new, + ImportLibrary.forType, ], - CompileTimeErrorCode.NOT_A_TYPE: [ImportLibrary.forType], + CompileTimeErrorCode.NOT_A_TYPE: [CreateClass.new, ImportLibrary.forType], CompileTimeErrorCode.NOT_ENOUGH_POSITIONAL_ARGUMENTS_NAME_PLURAL: [ DataDriven.new, ], @@ -1397,17 +1376,26 @@ final _builtInNonLintMultiGenerators = { CompileTimeErrorCode.NOT_ENOUGH_POSITIONAL_ARGUMENTS_SINGULAR: [ DataDriven.new, ], - CompileTimeErrorCode.TYPE_TEST_WITH_UNDEFINED_NAME: [ImportLibrary.forType], + CompileTimeErrorCode.TYPE_TEST_WITH_UNDEFINED_NAME: [ + CreateClass.new, + ImportLibrary.forType, + ], CompileTimeErrorCode.UNDEFINED_ANNOTATION: [ + CreateClass.new, ImportLibrary.forTopLevelVariable, ImportLibrary.forType, ], - CompileTimeErrorCode.UNDEFINED_CLASS: [DataDriven.new, ImportLibrary.forType], + CompileTimeErrorCode.UNDEFINED_CLASS: [ + CreateClass.new, + DataDriven.new, + ImportLibrary.forType, + ], CompileTimeErrorCode.UNDEFINED_CONSTRUCTOR_IN_INITIALIZER_DEFAULT: [ AddSuperConstructorInvocation.new, ], CompileTimeErrorCode.UNDEFINED_EXTENSION_GETTER: [DataDriven.new], CompileTimeErrorCode.UNDEFINED_FUNCTION: [ + CreateClass.new, DataDriven.new, ImportLibrary.forExtension, ImportLibrary.forExtensionType, @@ -1415,12 +1403,14 @@ final _builtInNonLintMultiGenerators = { ImportLibrary.forType, ], CompileTimeErrorCode.UNDEFINED_GETTER: [ + CreateClass.new, DataDriven.new, ImportLibrary.forExtensionMember, ImportLibrary.forTopLevelVariable, ImportLibrary.forType, ], CompileTimeErrorCode.UNDEFINED_IDENTIFIER: [ + CreateClass.new, DataDriven.new, ImportLibrary.forExtension, ImportLibrary.forExtensionMember, @@ -1429,6 +1419,7 @@ final _builtInNonLintMultiGenerators = { ImportLibrary.forType, ], CompileTimeErrorCode.UNDEFINED_METHOD: [ + CreateClass.new, DataDriven.new, ImportLibrary.forExtensionMember, ImportLibrary.forFunction, diff --git a/pkg/analysis_server/test/edit/fixes_test.dart b/pkg/analysis_server/test/edit/fixes_test.dart index 64a85430398..efed4ec1145 100644 --- a/pkg/analysis_server/test/edit/fixes_test.dart +++ b/pkg/analysis_server/test/edit/fixes_test.dart @@ -86,9 +86,9 @@ void f() { } '''); await waitForTasksFinished(); - var errorFixes = await _getFixesAt(testFile, 'Completer'); - expect(errorFixes, hasLength(1)); - var fixes = errorFixes[0].fixes; + var errors = await _getFixesAt(testFile, 'Completer'); + expect(errors, hasLength(1)); + var fixes = errors.first.fixes; expect(fixes, hasLength(4)); expect(fixes[0].message, matches('Import library')); expect(fixes[1].message, matches("Import library .+ with 'show'")); diff --git a/pkg/analysis_server/test/shared/shared_code_actions_fixes_tests.dart b/pkg/analysis_server/test/shared/shared_code_actions_fixes_tests.dart index 204f8e93678..73e807935aa 100644 --- a/pkg/analysis_server/test/shared/shared_code_actions_fixes_tests.dart +++ b/pkg/analysis_server/test/shared/shared_code_actions_fixes_tests.dart @@ -334,9 +334,11 @@ var b = bar(); var allFixes = await getCodeActions(testFileUri, range: code.range.range); // Expect only the single-fix, there should be no apply-all. - expect(allFixes, hasLength(1)); + expect(allFixes, hasLength(2)); var fixTitle = allFixes.first.map((f) => f.title, (f) => f.title); expect(fixTitle, equals("Create function 'foo'")); + var fixTitle2 = allFixes.last.map((f) => f.title, (f) => f.title); + expect(fixTitle2, equals("Create class 'foo'")); } Future test_fixAll_notWhenSingle() async { @@ -432,7 +434,7 @@ void main() { containsAllInOrder([ // Non-ignore fixes (order doesn't matter here, but this is what // server produces). - 'quickfix.create.class', + 'quickfix.create.class.uppercase', 'quickfix.create.mixin', 'quickfix.create.localVariable', 'quickfix.remove.unusedLocalVariable', @@ -609,7 +611,7 @@ var a = [Test, Test, Te[!!]st]; findCodeActionLiteral( codeActions, title: "Create class 'Test'", - kind: CodeActionKind('quickfix.create.class'), + kind: CodeActionKind('quickfix.create.class.uppercase'), )!; expect(createClassAction.diagnostics, hasLength(3)); @@ -631,7 +633,7 @@ var a = [Test, Test, Te[!!]st]; findCodeActionLiteral( codeActions, title: "Create class 'Test'", - kind: CodeActionKind('quickfix.create.class'), + kind: CodeActionKind('quickfix.create.class.uppercase'), )!; expect(createClassActions.diagnostics, hasLength(3)); diff --git a/pkg/analysis_server/test/src/services/correction/fix/create_class_test.dart b/pkg/analysis_server/test/src/services/correction/fix/create_class_test.dart index 11dfdf9e903..034966fb782 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/create_class_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/create_class_test.dart @@ -12,14 +12,170 @@ import 'fix_processor.dart'; void main() { defineReflectiveSuite(() { - defineReflectiveTests(CreateClassTest); + defineReflectiveTests(CreateClassLowercaseTest); + defineReflectiveTests(CreateClassPriorityTest); + defineReflectiveTests(CreateClassUppercaseTest); }); } @reflectiveTest -class CreateClassTest extends FixProcessorTest { +class CreateClassLowercaseTest extends FixProcessorTest { @override - FixKind get kind => DartFixKind.CREATE_CLASS; + FixKind get kind => DartFixKind.CREATE_CLASS_LOWERCASE; + + Future test_lowercaseAssignment() async { + await resolveTestCode(''' +void f() { + var _ = newName(); +} +'''); + await assertHasFix(''' +void f() { + var _ = newName(); +} + +class newName { +} +'''); + } + + Future test_multiple() async { + await resolveTestCode(r''' +void f() { + var _ = _$_newName(); +} +'''); + await assertHasFix(r''' +void f() { + var _ = _$_newName(); +} + +class _$_newName { +} +'''); + } + + Future test_number() async { + await resolveTestCode(r''' +void f() { + var _ = _0newName(); +} +'''); + await assertHasFix(r''' +void f() { + var _ = _0newName(); +} + +class _0newName { +} +'''); + } + + Future test_startWithDollarSign() async { + await resolveTestCode(r''' +void f() { + var _ = $newName(); +} +'''); + await assertHasFix(r''' +void f() { + var _ = $newName(); +} + +class $newName { +} +'''); + } + + Future test_startWithUnderscore() async { + await resolveTestCode(''' +void f() { + var _ = _newName(); +} +'''); + await assertHasFix(''' +void f() { + var _ = _newName(); +} + +class _newName { +} +'''); + } +} + +@reflectiveTest +class CreateClassPriorityTest extends FixPriorityTest { + Future test_classFirst_function() async { + await resolveTestCode(''' +void f() { + var _ = NewName(); +} +'''); + await assertFixPriorityOrder([ + DartFixKind.CREATE_CLASS_UPPERCASE, + DartFixKind.CREATE_FUNCTION, + ]); + } + + Future test_classFirst_method() async { + await resolveTestCode(''' +class A { + void m() { + var _ = NewName(); + } +} +'''); + await assertFixPriorityOrder([ + DartFixKind.CREATE_CLASS_UPPERCASE, + DartFixKind.CREATE_METHOD, + ]); + } + + Future test_classLast_function() async { + await resolveTestCode(''' +void f() { + var _ = newName(); +} +'''); + await assertFixPriorityOrder([ + DartFixKind.CREATE_FUNCTION, + DartFixKind.CREATE_CLASS_LOWERCASE, + ]); + } + + Future test_classLast_import() async { + newFile('$testPackageLibPath/lib.dart', r''' +class A {} +'''); + await resolveTestCode(''' +A? a; +'''); + await assertFixPriorityOrder([ + DartFixKind.IMPORT_LIBRARY_PROJECT1, + DartFixKind.CREATE_CLASS_UPPERCASE, + ]); + } + + Future test_classLast_method() async { + await resolveTestCode(''' +class A { + void m() { + var _ = newName(); + } +} +'''); + await assertFixPriorityOrder([ + DartFixKind.CREATE_METHOD, + DartFixKind.CREATE_CLASS_LOWERCASE, + ]); + } +} + +@reflectiveTest +class CreateClassUppercaseTest extends FixProcessorTest { + @override + FixKind get kind => DartFixKind.CREATE_CLASS_UPPERCASE; Future test_annotation() async { await resolveTestCode(''' @@ -302,6 +458,22 @@ class Test { assertLinkedGroup(change.linkedEditGroups[0], ['Test v =', 'Test {']); } + Future test_startWithUnderscore() async { + await resolveTestCode(''' +void f() { + var _ = _NewName(); +} +'''); + await assertHasFix(''' +void f() { + var _ = _NewName(); +} + +class _NewName { +} +'''); + } + Future test_with() async { await resolveTestCode(''' class MyClass with BaseClass {} diff --git a/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart b/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart index 7e89aa38eb2..305e93397cd 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart @@ -38,6 +38,22 @@ abstract class BaseFixProcessorTest extends AbstractSingleUnitTest { return DartChangeWorkspace([await session]); } + /// Computes fixes for the given [error] in [testUnit]. + Future> _computeFixes(Diagnostic error) async { + var libraryResult = testLibraryResult; + if (libraryResult == null) { + return const []; + } + var context = DartFixContext( + instrumentationService: TestInstrumentationService(), + workspace: await workspace, + libraryResult: libraryResult, + unitResult: testAnalysisResult, + error: error, + ); + return await computeFixes(context); + } + /// Find the error that is to be fixed by computing the errors in the file, /// using the [errorFilter] to filter out errors that should be ignored, and /// expecting that there is a single remaining error. The error filter should @@ -271,6 +287,7 @@ abstract class FixInFileProcessorTest extends BaseFixProcessorTest { } /// Computes fixes for the given [diagnostic] in [testUnit]. + @override Future> _computeFixes( Diagnostic diagnostic, { Set? alreadyCalculated, @@ -287,12 +304,30 @@ abstract class FixInFileProcessorTest extends BaseFixProcessorTest { error: diagnostic, ); - var fixes = - await FixInFileProcessor( - context, - alreadyCalculated: alreadyCalculated, - ).compute(); - return fixes; + return await FixInFileProcessor( + context, + alreadyCalculated: alreadyCalculated, + ).compute(); + } +} + +abstract class FixPriorityTest extends BaseFixProcessorTest { + Future assertFixPriorityOrder( + List fixKinds, { + ErrorFilter? errorFilter, + }) async { + var error = await _findErrorToFix(errorFilter: errorFilter); + var computedFixes = await _computeFixes(error); + var kinds = computedFixes.map((fix) => fix.kind).toList(); + kinds.sort((a, b) => b.priority.compareTo(a.priority)); + expect(kinds, containsAllInOrder(fixKinds)); + } + + @override + void setUp() { + super.setUp(); + verifyNoTestUnitErrors = false; + useLineEndingsForPlatform = true; } } @@ -613,8 +648,9 @@ abstract class FixProcessorTest extends BaseFixProcessorTest { } } - /// Computes fixes for the given [diagnostic] in [testUnit]. - Future> _computeFixes(Diagnostic diagnostic) async { + /// Computes fixes for the given [error] in [testUnit]. + @override + Future> _computeFixes(Diagnostic error) async { var libraryResult = testLibraryResult; if (libraryResult == null) { return const []; @@ -624,7 +660,7 @@ abstract class FixProcessorTest extends BaseFixProcessorTest { workspace: await workspace, libraryResult: libraryResult, unitResult: testAnalysisResult, - error: diagnostic, + error: error, ); return await computeFixes(context); } diff --git a/pkg/analysis_server/test/src/services/correction/fix/merge_combinators_test.dart b/pkg/analysis_server/test/src/services/correction/fix/merge_combinators_test.dart index 8471cb94009..a2cac95d749 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/merge_combinators_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/merge_combinators_test.dart @@ -14,6 +14,7 @@ import 'fix_processor.dart'; void main() { defineReflectiveSuite(() { + defineReflectiveTests(MergeCombinatorsPriorityTest); defineReflectiveTests(MergeHideUsingHideTest); defineReflectiveTests(MergeHideUsingShowTest); defineReflectiveTests(MergeShowUsingHideTest); @@ -21,6 +22,39 @@ void main() { }); } +@reflectiveTest +class MergeCombinatorsPriorityTest extends FixPriorityTest { + Future test_atLeastOneShow() async { + await resolveTestCode(''' +import 'other.dart' show Stream, Future hide Stream; +'''); + await assertFixPriorityOrder( + [ + DartFixKind.MERGE_COMBINATORS_SHOW_SHOW, + DartFixKind.MERGE_COMBINATORS_HIDE_SHOW, + ], + errorFilter: (error) { + return error.errorCode == WarningCode.MULTIPLE_COMBINATORS; + }, + ); + } + + Future test_onlyHide() async { + await resolveTestCode(''' +import 'other.dart' hide Stream hide Future; +'''); + await assertFixPriorityOrder( + [ + DartFixKind.MERGE_COMBINATORS_HIDE_HIDE, + DartFixKind.MERGE_COMBINATORS_SHOW_HIDE, + ], + errorFilter: (error) { + return error.errorCode == WarningCode.MULTIPLE_COMBINATORS; + }, + ); + } +} + @reflectiveTest class MergeHideUsingHideTest extends _MergeCombinatorTest { @override @@ -554,7 +588,10 @@ import 'other.dart' show FutureOr, Completer, Timer; } } -abstract class _MergeCombinatorTest extends FixProcessorErrorCodeTest { +abstract class _MergeCombinatorTest extends FixProcessorErrorCodeTest + with _MergeCombinatorTestMixin {} + +mixin _MergeCombinatorTestMixin on FixProcessorErrorCodeTest { bool diagnosticCodeFilter(Diagnostic d) { return d.errorCode == diagnosticCode; }