validation: unreachable code - special handling of nested function declarations

2 files changed, 69 insertions, 2 deletions

diff --git a/plugins/org.eclipse.dltk.javascript.core/src/org/eclipse/dltk/internal/javascript/validation/FlowValidation.java b/plugins/org.eclipse.dltk.javascript.core/src/org/eclipse/dltk/internal/javascript/validation/FlowValidation.java
index 780527ea..4be980a7 100644
--- a/plugins/org.eclipse.dltk.javascript.core/src/org/eclipse/dltk/internal/javascript/validation/FlowValidation.java
+++ b/plugins/org.eclipse.dltk.javascript.core/src/org/eclipse/dltk/internal/javascript/validation/FlowValidation.java
@@ -25,6 +25,7 @@ import org.eclipse.dltk.javascript.ast.CatchClause;
 import org.eclipse.dltk.javascript.ast.ContinueStatement;
 import org.eclipse.dltk.javascript.ast.DefaultClause;
 import org.eclipse.dltk.javascript.ast.DoWhileStatement;
+import org.eclipse.dltk.javascript.ast.Expression;
 import org.eclipse.dltk.javascript.ast.ForEachInStatement;
 import org.eclipse.dltk.javascript.ast.ForInStatement;
 import org.eclipse.dltk.javascript.ast.ForStatement;
@@ -38,6 +39,7 @@ import org.eclipse.dltk.javascript.ast.SwitchComponent;
 import org.eclipse.dltk.javascript.ast.SwitchStatement;
 import org.eclipse.dltk.javascript.ast.ThrowStatement;
 import org.eclipse.dltk.javascript.ast.TryStatement;
+import org.eclipse.dltk.javascript.ast.VoidExpression;
 import org.eclipse.dltk.javascript.ast.WhileStatement;
 import org.eclipse.dltk.javascript.core.JavaScriptProblems;
 import org.eclipse.dltk.javascript.parser.Reporter;
@@ -101,7 +103,9 @@ public class FlowValidation extends AbstractNavigationVisitor<FlowStatus>
 		int endRange = -1;
 		boolean firstBreak = true;
 		for (Statement statement : statements) {
-			if (status.isTerminated()) {
+			if (isFunctionDeclaration(statement)) {
+				visit(statement);
+			} else if (status.isTerminated()) {
 				if (isSwitch && statement instanceof BreakStatement
 						&& firstBreak && status.isReturned()) {
 					firstBreak = false;
@@ -125,6 +129,17 @@ public class FlowValidation extends AbstractNavigationVisitor<FlowStatus>
 		return status;
 	}
 
+	private boolean isFunctionDeclaration(Statement statement) {
+		if (statement instanceof VoidExpression) {
+			final Expression expression = ((VoidExpression) statement)
+					.getExpression();
+			return expression instanceof FunctionStatement
+					&& ((FunctionStatement) expression).isDeclaration();
+		} else {
+			return false;
+		}
+	}
+
 	@Override
 	public FlowStatus visitIfStatement(IfStatement node) {
 
diff --git a/tests/org.eclipse.dltk.javascript.core.tests/src/org/eclipse/dltk/javascript/core/tests/validation/FlowValidationTests.java b/tests/org.eclipse.dltk.javascript.core.tests/src/org/eclipse/dltk/javascript/core/tests/validation/FlowValidationTests.java
index 2d247122..014c0960 100644
--- a/tests/org.eclipse.dltk.javascript.core.tests/src/org/eclipse/dltk/javascript/core/tests/validation/FlowValidationTests.java
+++ b/tests/org.eclipse.dltk.javascript.core.tests/src/org/eclipse/dltk/javascript/core/tests/validation/FlowValidationTests.java
@@ -362,7 +362,7 @@ public class FlowValidationTests extends AbstractValidationTest {
 				Collections.singleton(JavaScriptProblems.UNREACHABLE_CODE),
 				problemIds);
 	}
-	
+
 	public void testIfAndSwitchWithNoReturns() throws Exception {
 		StringList code = new StringList();
 		code.add("function criteria_tree_node_text() {");
@@ -388,4 +388,56 @@ public class FlowValidationTests extends AbstractValidationTest {
 				.toString()));
 		assertEquals(problemIds.toString(), 0, problemIds.size());
 	}
+
+	public void testNestedFunctionDeclaration() {
+		final StringList code = new StringList();
+		code.add("function test() {");
+		code.add("  return 1");
+		code.add("  function nested() {");
+		code.add("    return 1;");
+		code.add("  }");
+		code.add("}");
+		final List<IProblem> problems = validate(code.toString());
+		assertEquals(problems.toString(), 0, problems.size());
+	}
+
+	public void testNestedFunctionDeclaration_nestedError() {
+		final StringList code = new StringList();
+		code.add("function test() {");
+		code.add("  return 1");
+		code.add("  function nested(x) {");
+		code.add("    if (a > 0) { return 1; }");
+		code.add("    else { return -1; }");
+		code.add("    return false;");
+		code.add("  }");
+		code.add("}");
+		final String sCode = code.toString();
+		final List<IProblem> problems = validate(sCode);
+		assertEquals(problems.toString(), 1, problems.size());
+		final IProblem problem = problems.get(0);
+		assertEquals(JavaScriptProblems.UNREACHABLE_CODE, problem.getID());
+		assertEquals(
+				"return false;",
+				sCode.substring(problem.getSourceStart(),
+						problem.getSourceEnd()));
+	}
+
+	public void testNestedFunctionDeclaration_outerError() {
+		final StringList code = new StringList();
+		code.add("function test() {");
+		code.add("  return 1");
+		code.add("  function nested() {");
+		code.add("  }");
+		code.add("  return false;");
+		code.add("}");
+		final String sCode = code.toString();
+		final List<IProblem> problems = validate(sCode);
+		assertEquals(problems.toString(), 1, problems.size());
+		final IProblem problem = problems.get(0);
+		assertEquals(JavaScriptProblems.UNREACHABLE_CODE, problem.getID());
+		assertEquals(
+				"return false;",
+				sCode.substring(problem.getSourceStart(),
+						problem.getSourceEnd()));
 	}
+}