From 3319e6d0e3bf7ca8a2b43b0d2cd4c6304583f6f2 Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Mon, 31 Oct 2016 08:48:37 -0700 Subject: [PATCH] Fix for inlining async methods/getters. R=brianwilkerson@google.com BUG= Review URL: https://codereview.chromium.org/2463493002 . --- .../services/refactoring/inline_method.dart | 33 +++ .../refactoring/inline_method_test.dart | 188 ++++++++++++++++++ 2 files changed, 221 insertions(+) diff --git a/pkg/analysis_server/lib/src/services/refactoring/inline_method.dart b/pkg/analysis_server/lib/src/services/refactoring/inline_method.dart index ca243c4b067..486f724f937 100644 --- a/pkg/analysis_server/lib/src/services/refactoring/inline_method.dart +++ b/pkg/analysis_server/lib/src/services/refactoring/inline_method.dart @@ -216,6 +216,7 @@ class InlineMethodRefactoringImpl extends RefactoringImpl _SourcePart _methodExpressionPart; _SourcePart _methodStatementsPart; List<_ReferenceProcessor> _referenceProcessors = []; + Set _alreadyMadeAsync = new Set(); InlineMethodRefactoringImpl(this.searchEngine, this.unit, this.offset) { utils = new CorrectionUtils(unit); @@ -287,6 +288,11 @@ class InlineMethodRefactoringImpl extends RefactoringImpl result = new RefactoringStatus.fatal('Cannot inline operator.'); return new Future.value(result); } + // maybe [a]sync* + if (_methodElement.isGenerator) { + result = new RefactoringStatus.fatal('Cannot inline a generator.'); + return new Future.value(result); + } // analyze method body result.addStatus(_prepareMethodParts()); // process references @@ -560,6 +566,33 @@ class _ReferenceProcessor { if (!_shouldProcess()) { return; } + // If the element being inlined is async, ensure that the function + // body that encloses the method is also async. + if (ref._methodElement.isAsynchronous) { + FunctionBody body = _node.getAncestor((n) => n is FunctionBody); + if (body != null) { + if (body.isSynchronous) { + if (body.isGenerator) { + status.addFatalError( + 'Cannot inline async into sync*.', newLocation_fromNode(_node)); + return; + } + if (refElement is ExecutableElement) { + var executable = refElement as ExecutableElement; + if (!executable.returnType.isDartAsyncFuture) { + status.addFatalError( + 'Cannot inline async into a function that does not return a Future.', + newLocation_fromNode(_node)); + return; + } + } + if (ref._alreadyMadeAsync.add(body)) { + SourceRange bodyStart = rangeStartLength(body.offset, 0); + _addRefEdit(newSourceEdit_range(bodyStart, 'async ')); + } + } + } + } // may be invocation of inline method if (nodeParent is MethodInvocation) { MethodInvocation invocation = nodeParent; diff --git a/pkg/analysis_server/test/services/refactoring/inline_method_test.dart b/pkg/analysis_server/test/services/refactoring/inline_method_test.dart index 7c35f5ead85..bc1d29c3810 100644 --- a/pkg/analysis_server/test/services/refactoring/inline_method_test.dart +++ b/pkg/analysis_server/test/services/refactoring/inline_method_test.dart @@ -66,6 +66,56 @@ class A { expect(refactoring.isDeclaration, isTrue); } + test_bad_async_intoSyncStar() { + indexTestUnit(r''' +import 'dart:async'; +class A { + Future get test async => 42; + Iterable> foo() sync* { + yield test; + } +} +'''); + _createRefactoring('test async'); + // error + return _assertConditionsFatal('Cannot inline async into sync*.'); + } + + test_bad_async_targetIsSync_doesNotReturnFuture() { + indexTestUnit(r''' +import 'dart:async'; +class A { + Future get test async => 42; + double foo() { + test; + return 1.2; + } +} +'''); + _createRefactoring('test async'); + // error + return _assertConditionsFatal( + 'Cannot inline async into a function that does not return a Future.'); + } + + test_bad_asyncStar() { + indexTestUnit(r''' +import 'dart:async'; +class A { + Stream test() async* { + yield 1; + yield 2; + } + foo() { + test(); + } +} +'''); + _createRefactoring('test() async*'); + // error + return _assertConditionsFatal('Cannot inline a generator.'); + } + test_bad_cascadeInvocation() async { indexTestUnit(r''' class A { @@ -711,6 +761,100 @@ main() { '''); } + test_getter_async_targetIsAsync() { + indexTestUnit(r''' +import 'dart:async'; +class A { + Future get test async => 42; + Future foo() async { + return test; + } +} +'''); + _createRefactoring('test async'); + // validate change + return _assertSuccessfulRefactoring(r''' +import 'dart:async'; +class A { + Future foo() async { + return 42; + } +} +'''); + } + + test_getter_async_targetIsAsyncStar() { + indexTestUnit(r''' +import 'dart:async'; +class A { + Future get test async => 42; + Stream foo() async { + return await test; + } +} +'''); + _createRefactoring('test async'); + // validate change + return _assertSuccessfulRefactoring(r''' +import 'dart:async'; +class A { + Stream foo() async { + return await 42; + } +} +'''); + } + + test_getter_async_targetIsSync() { + indexTestUnit(r''' +import 'dart:async'; +class A { + Future get test async => 42; + Future foo() { + return test; + } +} +'''); + _createRefactoring('test async'); + // validate change + return _assertSuccessfulRefactoring(r''' +import 'dart:async'; +class A { + Future foo() async { + return 42; + } +} +'''); + } + + test_getter_async_targetIsSync2() { + indexTestUnit(r''' +import 'dart:async'; +class A { + Future get test async => 42; + Future foo1() { + return test; + } + Future foo2() { + return test; + } +} +'''); + _createRefactoring('test async'); + // validate change + return _assertSuccessfulRefactoring(r''' +import 'dart:async'; +class A { + Future foo1() async { + return 42; + } + Future foo2() async { + return 42; + } +} +'''); + } + test_getter_classMember_instance() { indexTestUnit(r''' class A { @@ -803,6 +947,50 @@ main() { expect(refactoring.inlineAll, false); } + test_method_async() { + indexTestUnit(r''' +import 'dart:async'; +class A { + Future test() async => 42; + Future foo() { + return test(); + } +} +'''); + _createRefactoring('test() async'); + // validate change + return _assertSuccessfulRefactoring(r''' +import 'dart:async'; +class A { + Future foo() async { + return 42; + } +} +'''); + } + + test_method_async2() { + indexTestUnit(r''' +import 'dart:async'; +class A { + Future test() async => 42; + Future foo() { + return [test(), test()]; + } +} +'''); + _createRefactoring('test() async'); + // validate change + return _assertSuccessfulRefactoring(r''' +import 'dart:async'; +class A { + Future foo() async { + return [42, 42]; + } +} +'''); + } + test_method_emptyBody() { indexTestUnit(r''' abstract class A {