Issue 22188. Verify that none or all execution flows return.

R=brianwilkerson@google.com, paulberry@google.com
BUG= https://code.google.com/p/dart/issues/detail?id=22188

Review URL: https://codereview.chromium.org//871013012

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@43663 260f80e4-7a28-3924-810f-c04153c831b5

pkg/analysis_server/lib/src/services/refactoring/extract_method.dart

@@ -25,6 +25,7 @@ import 'package:analyzer/src/generated/element.dart';
 import 'package:analyzer/src/generated/java_core.dart';
 import 'package:analyzer/src/generated/scanner.dart';
 import 'package:analyzer/src/generated/source.dart';
+import 'package:analyzer/src/generated/resolver.dart' show ExitDetector;
 
 
 const String _TOKEN_SEPARATOR = '\uFFFF';
@@ -57,6 +58,10 @@ Map<String, String> _inverseMap(Map map) {
  */
 class ExtractMethodRefactoringImpl extends RefactoringImpl implements
     ExtractMethodRefactoring {
+  static const ERROR_EXITS =
+      'Selected statements contain a return statement, but not all possible '
+          'execuion flows exit. Semantics may not be preserved.';
+
   final SearchEngine searchEngine;
   final CompilationUnit unit;
   final int selectionOffset;
@@ -572,7 +577,14 @@ class ExtractMethodRefactoringImpl extends RefactoringImpl implements
     if (_selectionExpression != null) {
       _returnType = _selectionExpression.bestType;
     }
-    // may be ends with "return" statement
+    // verify that none or all execution flows end with a "return"
+    if (_selectionStatements != null) {
+      bool hasReturn = _selectionStatements.any(_mayEndWithReturnStatement);
+      if (hasReturn && !ExitDetector.exits(_selectionStatements.last)) {
+        result.addError(ERROR_EXITS);
+      }
+    }
+    // maybe ends with "return" statement
     if (_selectionStatements != null) {
       _ReturnTypeComputer returnTypeComputer = new _ReturnTypeComputer();
       _selectionStatements.forEach((statement) {
@@ -580,7 +592,7 @@ class ExtractMethodRefactoringImpl extends RefactoringImpl implements
       });
       _returnType = returnTypeComputer.returnType;
     }
-    // may be single variable to return
+    // maybe single variable to return
     if (assignedUsedVariables.length == 1) {
       // we cannot both return variable and have explicit return statement
       if (_returnType != null) {
@@ -698,6 +710,15 @@ class ExtractMethodRefactoringImpl extends RefactoringImpl implements
     node.accept(visitor);
     return visitor.result;
   }
+
+  /**
+   * Returns `true` if the given [statement] may end with a [ReturnStatement].
+   */
+  static bool _mayEndWithReturnStatement(Statement statement) {
+    _HasReturnStatementVisitor visitor = new _HasReturnStatementVisitor();
+    statement.accept(visitor);
+    return visitor.hasReturn;
+  }
 }
 
 
@@ -865,6 +886,20 @@ class _HasMethodInvocationVisitor extends RecursiveAstVisitor {
 }
 
 
+class _HasReturnStatementVisitor extends RecursiveAstVisitor {
+  bool hasReturn = false;
+
+  @override
+  visitBlockFunctionBody(BlockFunctionBody node) {
+  }
+
+  @override
+  visitReturnStatement(ReturnStatement node) {
+    hasReturn = true;
+  }
+}
+
+
 class _InitializeOccurrencesVisitor extends GeneralizingAstVisitor<Object> {
   final ExtractMethodRefactoringImpl ref;
   final _SourcePattern selectionPattern;
@@ -972,7 +1007,6 @@ class _InitializeOccurrencesVisitor extends GeneralizingAstVisitor<Object> {
   }
 }
 
-
 class _InitializeParametersVisitor extends GeneralizingAstVisitor<Object> {
   final ExtractMethodRefactoringImpl ref;
   final List<VariableElement> assignedUsedVariables;
@@ -1031,6 +1065,7 @@ class _InitializeParametersVisitor extends GeneralizingAstVisitor<Object> {
   }
 }
 
+
 class _IsUsedAfterSelectionVisitor extends GeneralizingAstVisitor {
   final ExtractMethodRefactoringImpl ref;
   final VariableElement element;

pkg/analysis_server/test/services/refactoring/extract_method_test.dart

@@ -424,6 +424,21 @@ main() {
             "Extend selection to a valid range.");
   }
 
+  test_bad_statements_exit_notAllExecutionFlows() {
+    indexTestUnit('''
+main(int p) {
+// start
+  if (p == 0) {
+    return;
+  }
+// end
+  print(p);
+}
+''');
+    _createRefactoringForStartEndComments();
+    return _assertConditionsError(ExtractMethodRefactoringImpl.ERROR_EXITS);
+  }
+
   test_bad_statements_return_andAssignsVariable() {
     indexTestUnit('''
 main() {
@@ -2080,6 +2095,21 @@ void res() {
 ''');
   }
 
+  test_statements_exit_throws() async {
+    indexTestUnit('''
+main(int p) {
+// start
+  if (p == 0) {
+    return;
+  }
+  throw 'boo!';
+// end
+}
+''');
+    _createRefactoringForStartEndComments();
+    await assertRefactoringConditionsOK();
+  }
+
   test_statements_inSwitchMember() {
     indexTestUnit('''
 class A {
@@ -2292,8 +2322,10 @@ int res() {
 main(bool b) {
 // start
   if (b) {
+    print(true);
     return <int>[];
   } else {
+    print(false);
     return <String>[];
   }
 // end
@@ -2310,8 +2342,10 @@ main(bool b) {
 
 List res(bool b) {
   if (b) {
+    print(true);
     return <int>[];
   } else {
+    print(false);
     return <String>[];
   }
 }

pkg/analyzer/lib/src/generated/resolver.dart

@@ -4331,6 +4331,13 @@ class ExitDetector extends GeneralizingAstVisitor<bool> {
     }
     return false;
   }
+
+  /**
+   * Return `true` if the given [node] exits.
+   */
+  static bool exits(AstNode node) {
+    return new ExitDetector()._nodeExits(node);
+  }
 }
 
 /**