From f4b6fa85eae9ffde044fe479999974585ca000da Mon Sep 17 00:00:00 2001 From: Ahmed Ashour Date: Mon, 23 Aug 2021 23:56:07 +0000 Subject: [PATCH] Add a quick fix to `return_of_invalid_type` Bug: 46785 Change-Id: Ided0ebd07935289b9b7fdfaaf5a38a992646c9d4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/209962 Reviewed-by: Brian Wilkerson Commit-Queue: Brian Wilkerson --- .../correction/dart/replace_return_type.dart | 111 +++++++++ .../lib/src/services/correction/fix.dart | 4 + .../src/services/correction/fix_internal.dart | 3 + .../fix/replace_return_type_test.dart | 235 ++++++++++++++++++ .../src/services/correction/fix/test_all.dart | 2 + 5 files changed, 355 insertions(+) create mode 100644 pkg/analysis_server/lib/src/services/correction/dart/replace_return_type.dart create mode 100644 pkg/analysis_server/test/src/services/correction/fix/replace_return_type_test.dart diff --git a/pkg/analysis_server/lib/src/services/correction/dart/replace_return_type.dart b/pkg/analysis_server/lib/src/services/correction/dart/replace_return_type.dart new file mode 100644 index 00000000000..89adc9991cc --- /dev/null +++ b/pkg/analysis_server/lib/src/services/correction/dart/replace_return_type.dart @@ -0,0 +1,111 @@ +// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analysis_server/src/services/correction/dart/abstract_producer.dart'; +import 'package:analysis_server/src/services/correction/fix.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/syntactic_entity.dart'; +import 'package:analyzer/dart/element/type.dart'; +import 'package:analyzer/src/dart/element/inheritance_manager3.dart'; +import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:analyzer_plugin/utilities/range_factory.dart'; + +class ReplaceReturnType extends CorrectionProducer { + String _newType = ''; + + @override + List get fixArguments => [_newType]; + + @override + FixKind get fixKind => DartFixKind.REPLACE_RETURN_TYPE; + + @override + Future compute(ChangeBuilder builder) async { + final node = this.node; + if (node is Expression) { + final typeSystem = libraryElement.typeSystem; + + var newType = node.staticType; + + void updateNewType(SyntacticEntity entity) { + if (entity is FunctionExpression) { + return; + } else if (entity is ReturnStatement) { + var type = entity.expression?.staticType; + if (type != null) { + if (newType == null) { + newType = type; + } else { + newType = typeSystem.leastUpperBound(newType!, type); + } + } + } else if (entity is AstNode) { + entity.childEntities.forEach(updateNewType); + } + } + + var functionBody = node.thisOrAncestorOfType(); + var parent = functionBody?.parent; + var grandParent = parent?.parent; + + TypeAnnotation? returnType; + if (grandParent is FunctionDeclaration) { + updateNewType(grandParent.functionExpression.body); + returnType = grandParent.returnType; + } else if (parent is MethodDeclaration) { + updateNewType(parent.body); + if (_isCompatibleWithReturnType(parent, newType)) { + returnType = parent.returnType; + } + } + + if (returnType != null && newType != null) { + if (functionBody!.isAsynchronous) { + newType = typeProvider.futureType(newType!); + } + + _newType = newType!.getDisplayString(withNullability: true); + + await builder.addDartFileEdit(file, (builder) { + if (builder.canWriteType(newType)) { + builder.addReplacement(range.node(returnType!), (builder) { + builder.writeType(newType); + }); + } + }); + } + } + } + + bool _isCompatibleWithReturnType( + MethodDeclaration method, DartType? newType) { + if (newType != null) { + var clazz = method.thisOrAncestorOfType(); + if (clazz != null) { + var classElement = clazz.declaredElement!; + var overriddenList = InheritanceManager3().getOverridden2( + classElement, + Name( + classElement.library.source.uri, + method.declaredElement!.name, + )); + + if (overriddenList != null) { + var notSubtype = overriddenList.any((element) => !libraryElement + .typeSystem + .isSubtypeOf(newType, element.returnType)); + if (notSubtype) { + return false; + } + } + } + return true; + } + return false; + } + + /// Return an instance of this class. Used as a tear-off in `FixProcessor`. + static ReplaceReturnType newInstance() => ReplaceReturnType(); +} diff --git a/pkg/analysis_server/lib/src/services/correction/fix.dart b/pkg/analysis_server/lib/src/services/correction/fix.dart index 73c37792fa1..83b6c8b69c5 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix.dart @@ -745,6 +745,10 @@ class DartFixKind { 'dart.fix.replace.nullWithVoid.multi', DartFixKindPriority.DEFAULT, "Replace 'Null' with 'void' everywhere in file"); + static const REPLACE_RETURN_TYPE = FixKind( + 'dart.fix.replace.returnType', + DartFixKindPriority.DEFAULT, + "Replace the return type with '{0}'"); static const REPLACE_RETURN_TYPE_FUTURE = FixKind( 'dart.fix.replace.returnTypeFuture', DartFixKindPriority.DEFAULT, 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 e0ef0b5e168..a67305fb170 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -140,6 +140,7 @@ import 'package:analysis_server/src/services/correction/dart/replace_final_with_ import 'package:analysis_server/src/services/correction/dart/replace_final_with_var.dart'; import 'package:analysis_server/src/services/correction/dart/replace_new_with_const.dart'; import 'package:analysis_server/src/services/correction/dart/replace_null_with_closure.dart'; +import 'package:analysis_server/src/services/correction/dart/replace_return_type.dart'; import 'package:analysis_server/src/services/correction/dart/replace_return_type_future.dart'; import 'package:analysis_server/src/services/correction/dart/replace_var_with_dynamic.dart'; import 'package:analysis_server/src/services/correction/dart/replace_with_brackets.dart'; @@ -919,9 +920,11 @@ class FixProcessor extends BaseProcessor { ], CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_FUNCTION: [ MakeReturnTypeNullable.newInstance, + ReplaceReturnType.newInstance, ], CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_METHOD: [ MakeReturnTypeNullable.newInstance, + ReplaceReturnType.newInstance, ], CompileTimeErrorCode.TYPE_TEST_WITH_UNDEFINED_NAME: [ ChangeTo.classOrMixin, diff --git a/pkg/analysis_server/test/src/services/correction/fix/replace_return_type_test.dart b/pkg/analysis_server/test/src/services/correction/fix/replace_return_type_test.dart new file mode 100644 index 00000000000..d3c41f70a76 --- /dev/null +++ b/pkg/analysis_server/test/src/services/correction/fix/replace_return_type_test.dart @@ -0,0 +1,235 @@ +// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analysis_server/src/services/correction/fix.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'; + +import 'fix_processor.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(ReplaceReturnTypeTest); + }); +} + +@reflectiveTest +class ReplaceReturnTypeTest extends FixProcessorTest { + @override + FixKind get kind => DartFixKind.REPLACE_RETURN_TYPE; + + Future test_async_method() async { + await resolveTestCode(''' +class A { + Future m() async { + return ''; + } +} +'''); + await assertHasFix(''' +class A { + Future m() async { + return ''; + } +} +'''); + } + + Future test_closure() async { + await resolveTestCode(''' +class A { + int m() { + var list = []; + list.map((e) { + return 0; + }); + return 2.4; + } +} +'''); + await assertHasFix(''' +class A { + double m() { + var list = []; + list.map((e) { + return 0; + }); + return 2.4; + } +} +'''); + } + + Future test_function() async { + await resolveTestCode(''' +int f() { + return ''; +} +'''); + await assertHasFix(''' +String f() { + return ''; +} +'''); + } + + Future test_function_local() async { + await resolveTestCode(''' +void top() { + int f() { + return ''; + } +} +'''); + await assertHasFix(''' +void top() { + String f() { + return ''; + } +} +''', errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_FUNCTION; + }); + } + + Future test_method() async { + await resolveTestCode(''' +class A { + int m() { + return ''; + } +} +'''); + await assertHasFix(''' +class A { + String m() { + return ''; + } +} +'''); + } + + Future test_methodOverride() async { + await resolveTestCode(''' +class A { + A m() => this; +} +class B extends A { + @override + int m() => this; +} +'''); + await assertHasFix(''' +class A { + A m() => this; +} +class B extends A { + @override + B m() => this; +} +''', errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_METHOD; + }); + } + + Future test_methodOverride_multiple_subtype() async { + await resolveTestCode(''' +class A {} +class B extends A {} + +class Parent { + A m() => A(); +} + +class I { + B m() => B(); +} + +class D extends Parent implements I { + @override + B m() => A(); +} +'''); + + await assertNoFix(); + } + + Future test_methodOverride_subtype() async { + await resolveTestCode(''' +class A { + B m() => B(); +} +class B extends A { + @override + B m() => A(); +} +'''); + await assertNoFix(); + } + + Future test_privateType() async { + addSource('/home/test/lib/a.dart', ''' +class A { + _B b => _B(); +} +class _B {} +'''); + + await resolveTestCode(''' +import 'package:test/a.dart'; + +int f(A a) { + return a.b(); +} +'''); + await assertNoFix(); + } + + Future test_upperBound_function() async { + await resolveTestCode(''' +int f() { + if (true) { + return 3; + } + return 2.4; +} +'''); + await assertHasFix(''' +num f() { + if (true) { + return 3; + } + return 2.4; +} +''', errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_FUNCTION; + }); + } + + Future test_upperBound_method() async { + await resolveTestCode(''' +class A { + int m() { + if (true) { + return 3; + } + return 2.4; + } +} +'''); + await assertHasFix(''' +class A { + num m() { + if (true) { + return 3; + } + return 2.4; + } +} +''', errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_METHOD; + }); + } +} diff --git a/pkg/analysis_server/test/src/services/correction/fix/test_all.dart b/pkg/analysis_server/test/src/services/correction/fix/test_all.dart index 6178199771d..5ff6f0de22f 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/test_all.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/test_all.dart @@ -167,6 +167,7 @@ import 'replace_final_with_var_test.dart' as replace_final_with_var; import 'replace_new_with_const_test.dart' as replace_new_with_const; import 'replace_null_with_closure_test.dart' as replace_null_with_closure; import 'replace_return_type_future_test.dart' as replace_return_type_future; +import 'replace_return_type_test.dart' as replace_return_type; import 'replace_var_with_dynamic_test.dart' as replace_var_with_dynamic; import 'replace_with_brackets_test.dart' as replace_with_brackets; import 'replace_with_conditional_assignment_test.dart' @@ -339,6 +340,7 @@ void main() { replace_new_with_const.main(); replace_null_with_closure.main(); replace_null_with_void.main(); + replace_return_type.main(); replace_return_type_future.main(); replace_var_with_dynamic.main(); replace_with_brackets.main();