Roll-forward the changes for block-scope-declared functions being tra…

…nspiled to Es3 and Es5.

Changes: prevented block-scope-declared feature from causing unnecessary transpilation

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=161249020

src/com/google/javascript/jscomp/Es6ExternsCheck.java

@@ -18,6 +18,8 @@
 import static com.google.common.base.Preconditions.checkState;
 
 import com.google.javascript.jscomp.NodeTraversal.AbstractPreOrderCallback;
+import com.google.javascript.jscomp.parsing.parser.FeatureSet;
+import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature;
 import com.google.javascript.rhino.Node;
 
 /**
@@ -40,7 +42,13 @@ private boolean hasEs6Syntax(Node root) {
     for (Node script : root.children()) {
       checkState(script.isScript());
       if (TranspilationPasses.isScriptEs6OrHigher(script)) {
-        return true;
+        FeatureSet features = (FeatureSet) script.getProp(Node.FEATURE_SET);
+        if (!features.has(Feature.BLOCK_SCOPED_FUNCTION_DECLARATION)) {
+          return true;
+        } else {
+          features = features.without(Feature.BLOCK_SCOPED_FUNCTION_DECLARATION);
+          return !FeatureSet.ES5.contains(features);
+        }
       }
     }
     return false;

src/com/google/javascript/jscomp/Es6RewriteBlockScopedDeclaration.java

@@ -108,8 +108,6 @@ && inLoop(n)) {
 
   @Override
   public void process(Node externs, Node root) {
-    // Since block-scoped function declarations can appear in any language mode, we need to run
-    // this pass unconditionally.
     NodeTraversal.traverseEs6(compiler, root, new CollectUndeclaredNames());
     NodeTraversal.traverseEs6(compiler, root, this);
     // Needed for let / const declarations in .d.ts externs.
@@ -119,9 +117,13 @@ public void process(Node externs, Node root) {
     NodeTraversal.traverseEs6(compiler, root, transformer);
     transformer.transformLoopClosure();
     varify();
-    TranspilationPasses.processTranspile(
-        compiler, externs, new RewriteBlockScopedFunctionDeclaration());
-    NodeTraversal.traverseEs6(compiler, root, new RewriteBlockScopedFunctionDeclaration());
+
+    // Block scoped function declarations can occur in any language mode, however for
+    // transpilation to ES3 and ES5, we want to hoist the functions from the block-scope by
+    // redeclaring them in var assignments
+    RewriteBlockScopedFunctionDeclaration rewriteFunction =
+        new RewriteBlockScopedFunctionDeclaration();
+    TranspilationPasses.processTranspile(compiler, root, rewriteFunction);
   }
 
   @Override

src/com/google/javascript/jscomp/TranspilationPasses.java

@@ -21,6 +21,7 @@
 import com.google.javascript.jscomp.NodeTraversal.Callback;
 import com.google.javascript.jscomp.PassFactory.HotSwapPassFactory;
 import com.google.javascript.jscomp.parsing.parser.FeatureSet;
+import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature;
 import com.google.javascript.rhino.Node;
 import java.util.List;
 
@@ -336,11 +337,17 @@ static void processTranspile(
       AbstractCompiler compiler, Node combinedRoot, Callback... callbacks) {
     if (compiler.getOptions().needsTranspilationFrom(FeatureSet.ES6)) {
       for (Node singleRoot : combinedRoot.children()) {
+        FeatureSet features = (FeatureSet) singleRoot.getProp(Node.FEATURE_SET);
         if (isScriptEs6OrHigher(singleRoot)) {
           for (Callback callback : callbacks) {
             singleRoot.putBooleanProp(Node.TRANSPILED, true);
             NodeTraversal.traverseEs6(compiler, singleRoot, callback);
           }
+        } else if (features != null && features.has(Feature.BLOCK_SCOPED_FUNCTION_DECLARATION)) {
+          // We want to traverse, but not give this node the transpiled property.
+          for (Callback callback : callbacks) {
+            NodeTraversal.traverseEs6(compiler, singleRoot, callback);
+          }
         }
       }
     }

src/com/google/javascript/jscomp/parsing/IRFactory.java

@@ -279,7 +279,7 @@ class IRFactory {
   private boolean currentFileIsExterns = false;
   private boolean hasJsDocTypeAnnotations = false;
 
-  private FeatureSet features = FeatureSet.ES3;
+  private FeatureSet features = FeatureSet.BARE_MINIMUM;
   private Node resultNode;
 
   private IRFactory(String sourceString,
@@ -1256,6 +1256,12 @@ Node processFunction(FunctionDeclarationTree functionTree) {
         maybeWarnForFeature(functionTree, Feature.ASYNC_FUNCTIONS);
       }
 
+      // Add a feature so that transpilation process hoists block scoped functions through
+      // var redeclaration in ES3 and ES5
+      if (isDeclaration) {
+        features = features.with(Feature.BLOCK_SCOPED_FUNCTION_DECLARATION);
+      }
+
       IdentifierToken name = functionTree.name;
       Node newName;
       if (name != null) {

src/com/google/javascript/jscomp/parsing/parser/FeatureSet.java

@@ -43,8 +43,11 @@
 public final class FeatureSet implements Serializable {
   private final ImmutableSet<Feature> features;
 
-  /** The bare minimum set of features in ES3. */
-  public static final FeatureSet ES3 = new FeatureSet(ImmutableSet.<Feature>of());
+  /** The bare minimum set of features. */
+  public static final FeatureSet BARE_MINIMUM  = new FeatureSet(ImmutableSet.<Feature>of());
+
+  /**Features from ES3.*/
+  public static final FeatureSet ES3 = BARE_MINIMUM.with(LangVersion.ES3.features());
 
   /** Features from ES5 only. */
   public static final FeatureSet ES5 = ES3.with(LangVersion.ES5.features());
@@ -66,7 +69,7 @@ public final class FeatureSet implements Serializable {
   public static final FeatureSet TYPESCRIPT = ES8_MODULES.with(LangVersion.TYPESCRIPT.features());
 
   private enum LangVersion {
-    ES5, ES6, ES7, ES8, TYPESCRIPT;
+    ES3, ES5, ES6, ES7, ES8, TYPESCRIPT;
 
     private Set<Feature> features() {
       Set<Feature> set = new HashSet<>();
@@ -81,6 +84,15 @@ private Set<Feature> features() {
 
   /** Specific features that can be included in a FeatureSet. */
   public enum Feature {
+    //ES3 features
+    //Functions can be declared in the block scope for all ES versions. However, for ES3 and ES5,
+    //we want to hoist the functions from the block scope by redeclaring them as vars (i.e.
+    //{ function f() {} } becomes { var f = function {} }. To prevent this feature from causing
+    //code to run additional transpilation passes beyond rewriting block scoped functions, we mark
+    //block scoped functions as an ES3 feature and then manually determine whether to rewrite
+    //functions inside the processTranspile method of TranspilationPasses.java
+    BLOCK_SCOPED_FUNCTION_DECLARATION("block function", LangVersion.ES3),
+
     // ES5 features
     ES3_KEYWORDS_AS_IDENTIFIERS("ES3 keywords as identifiers", LangVersion.ES5),
     GETTER("getters", LangVersion.ES5),

src/com/google/javascript/jscomp/parsing/parser/Parser.java

@@ -182,7 +182,7 @@ private enum FunctionFlavor {
   private final Config config;
   private final CommentRecorder commentRecorder = new CommentRecorder();
   private final ArrayDeque<FunctionFlavor> functionContextStack = new ArrayDeque<>();
-  private FeatureSet features = FeatureSet.ES3;
+  private FeatureSet features = FeatureSet.BARE_MINIMUM;
   private SourcePosition lastSourcePosition;
   @Nullable private String sourceMapURL;
 

test/com/google/javascript/jscomp/Es6RewriteBlockScopedDeclarationEs6LangOutTest.java

@@ -0,0 +1,86 @@
+/*
+ * Copyright 2017 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.javascript.jscomp;
+
+import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
+
+/**
+ * Test case for {@link Es6RewriteBlockScopedDeclaration} with Es6 as the Language Out
+ *
+ * Created by simranarora on 7/6/17.
+ * @author simranarora@google.com (Simran Arora)
+ */
+public class Es6RewriteBlockScopedDeclarationEs6LangOutTest extends CompilerTestCase {
+
+  @Override
+  protected void setUp() throws Exception {
+    super.setUp();
+    setAcceptedLanguage(LanguageMode.ECMASCRIPT_2015);
+    enableRunTypeCheckAfterProcessing();
+  }
+
+  @Override
+  protected CompilerOptions getOptions() {
+    CompilerOptions options = super.getOptions();
+    options.setLanguageOut(LanguageMode.ECMASCRIPT_2015);
+    return options;
+  }
+
+  @Override
+  protected CompilerPass getProcessor(Compiler compiler) {
+    return new Es6RewriteBlockScopedDeclaration(compiler);
+  }
+
+  @Override
+  protected int getNumRepetitions() {
+    return 1;
+  }
+
+  public void testNoTranspilationForBlockScopedFunctionDeclarations() {
+    testSame(
+        LINE_JOINER.join(
+            "for (var x of y) {",
+            "  function f() {",
+            "    x;",
+            "  }",
+            "}"));
+  }
+
+  // TODO (simarora) Correct output is commented out for now - transpilation mode is being entered
+  // to convert lets and consts to vars even though output is ES6
+  public void testNoTranspilationForLetConstDeclarations() {
+   /*testSame(
+        LINE_JOINER.join(
+            "for (var x of y) {",
+            "  function f() {",
+            "    let z;",
+            "  }",
+            "}"));*/
+    test(
+        LINE_JOINER.join(
+            "for (var x of y) {",
+            "  function f() {",
+            "    let z;",
+            "  }",
+            "}"),
+        LINE_JOINER.join(
+            "for (var x of y) {",
+            "  function f() {",
+            "    var z;",
+            "  }",
+            "}"));
+  }
+}

test/com/google/javascript/jscomp/NodeTraversalTest.java

@@ -283,7 +283,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
         + "visit FUNCTION foo [source_file: [testcode]] @2:0\n"
         + "visit SCRIPT [source_file: [testcode]] "
         + "[input_id: InputId: [testcode]] "
-        + "[feature_set: []] @1:0\n";
+        + "[feature_set: [block function]] @1:0\n";
 
     assertEquals(expectedResult, builder.toString());
   }

test/com/google/javascript/jscomp/parsing/ParserTest.java

@@ -82,7 +82,7 @@ protected void setUp() throws Exception {
     mode = LanguageMode.ECMASCRIPT3;
     strictMode = SLOPPY;
     isIdeMode = false;
-    expectedFeatures = FeatureSet.ES3;
+    expectedFeatures = FeatureSet.BARE_MINIMUM;
   }
 
   public void testExponentOperator() {
@@ -1795,6 +1795,18 @@ public void testLetForbidden2() {
         getRequiresEs6Message(Feature.LET_DECLARATIONS));
   }
 
+  // Ensure that we only add the feature for function declarations (i.e.
+  // function f() {} ) and not function expressions (e.g. var f = function() {} )
+  public void testBlockScopeFunctionDeclaration() {
+    expectFeatures(Feature.BLOCK_SCOPED_FUNCTION_DECLARATION);
+    parse("if (1) { function f() {} }");
+
+    expectFeatures();
+    Node result = parse("if (1) { var f = function() {} }");
+    FeatureSet features = (FeatureSet) result.getProp(Node.FEATURE_SET);
+    assertFS(features).doesNotHave(Feature.BLOCK_SCOPED_FUNCTION_DECLARATION);
+  }
+
   public void testLetForbidden3() {
     mode = LanguageMode.ECMASCRIPT5;
     strictMode = STRICT;
@@ -3800,7 +3812,7 @@ private Config createConfig() {
 
   /** Sets expectedFeatures based on the list of features. */
   private void expectFeatures(Feature... features) {
-    expectedFeatures = FeatureSet.ES3.with(features);
+    expectedFeatures = FeatureSet.BARE_MINIMUM.with(features);
   }
 
   private static class ParserResult {