Fix problems with export and function/class declarations.

This stops Normalize from splitting export default + a function/class declarations with since default exports are unnamed.

It also stops rewriting of
  export function foo() {}
to
  export var foo = function() {];
as foo is block-scoped and hoisted (so #429 does not apply).

Also changes the behavior of some NodeUtil helpers and unit tests to acknowledge that export default can contain module-level function and class declarations, which is both technically correct and should aid future work on export statements. For example, this creates a function foo in the module scope:
  export default function foo() {};

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

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

@@ -1392,7 +1392,7 @@ public BanGlobalVars(AbstractCompiler compiler, Requirement requirement)
     @Override
     protected ConformanceResult checkConformance(NodeTraversal t, Node n) {
       if (t.inGlobalScope()
-          && isDeclaration(n)
+          && NodeUtil.isDeclaration(n)
           && !n.getBooleanProp(Node.IS_NAMESPACE)
           && !isWhitelisted(n)) {
         Node enclosingScript = NodeUtil.getEnclosingScript(n);
@@ -1404,12 +1404,6 @@ && isDeclaration(n)
       return ConformanceResult.CONFORMANCE;
     }
 
-    private boolean isDeclaration(Node n) {
-      return NodeUtil.isNameDeclaration(n)
-          || NodeUtil.isFunctionDeclaration(n)
-          || NodeUtil.isClassDeclaration(n);
-    }
-
     private boolean isWhitelisted(Node n) {
       return (n.isVar() || n.isFunction()) && isWhitelistedName(n.getFirstChild().getString());
     }

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

@@ -349,7 +349,7 @@ private void maybeAddFunction(Function fn, JSModule module) {
 
   /**
    * @param fnNode The function to inspect.
-   * @return Whether the function has parameters, var, or function declarations.
+   * @return Whether the function has parameters, var/const/let, class, or function declarations.
    */
   private static boolean hasLocalNames(Node fnNode) {
     Node block = NodeUtil.getFunctionBody(fnNode);

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

@@ -2650,9 +2650,8 @@ static boolean isStatementParent(Node parent) {
   private static boolean isDeclarationParent(Node parent) {
     switch (parent.getToken()) {
       case DECLARE:
-        return true;
       case EXPORT:
-        return !parent.getBooleanProp(Node.EXPORT_DEFAULT);
+        return true;
       default:
         return isStatementParent(parent);
     }
@@ -2931,20 +2930,29 @@ public static Node getFunctionBody(Node fn) {
     return fn.getLastChild();
   }
 
+
+  /**
+   * Is the node a var, const, let, function, or class declaration?
+   * See {@link #isFunctionDeclaration}, {@link #isClassDeclaration}, and {@link #isNameDeclaration}
+   */
+  static boolean isDeclaration(Node n) {
+    return isNameDeclaration(n) || isFunctionDeclaration(n) || isClassDeclaration(n);
+  }
+
   /**
    * Is this node a function declaration? A function declaration is a function
    * that has a name that is added to the current scope (i.e. a function that
    * is not part of a expression; see {@link #isFunctionExpression}).
    */
   public static boolean isFunctionDeclaration(Node n) {
-    return n.isFunction() && isDeclarationParent(n.getParent());
+    return n.isFunction() && isDeclarationParent(n.getParent()) && isNamedFunction(n);
   }
 
   /**
    * see {@link #isClassDeclaration}
    */
   public static boolean isClassDeclaration(Node n) {
-    return n.isClass() && isDeclarationParent(n.getParent());
+    return n.isClass() && isDeclarationParent(n.getParent()) && isNamedClass(n);
   }
 
   /**
@@ -2955,6 +2963,8 @@ public static boolean isClassDeclaration(Node n) {
   public static boolean isHoistedFunctionDeclaration(Node n) {
     if (isFunctionDeclaration(n)) {
       Node parent = n.getParent();
+      // TODO(lharker): should return true if parent is an export. Doing so breaks other tests, so
+      // I'm moving it into future CLs.
       return parent.isScript() || parent.isModuleBody() || parent.getParent().isFunction();
     }
     return false;
@@ -3024,6 +3034,33 @@ static boolean isClassExpression(Node n) {
     return n.isClass() && !isStatement(n);
   }
 
+  /**
+   * Returns whether n is a function with a nonempty name.
+   * Differs from {@link #isFunctionDeclaration} because the name might in a function expression
+   * and not be added to the current scope.
+   *
+   * Some named functions include
+   * <pre>
+   *   (function f() {})();
+   *   export default function f() {};
+   *   function f() {};
+   *   var f = function f() {};
+   * </pre>
+   */
+  static boolean isNamedFunction(Node n) {
+    return n.isFunction() && isReferenceName(n.getFirstChild());
+  }
+
+  /**
+   * see {@link #isNamedFunction}
+   *
+   * @param n A node
+   * @return Whether n is a named class
+   */
+  static boolean isNamedClass(Node n) {
+    return n.isClass() && isReferenceName(n.getFirstChild());
+  }
+
   /**
    * Returns whether this is a bleeding function (an anonymous named function
    * that bleeds into the inner scope).
@@ -4084,13 +4121,12 @@ public boolean apply(Node n) {
 
 
   /**
-   * A predicate for matching var, let, const, or function declarations.
-   * TODO(simranarora): Should this handle class declarations too?
+   * A predicate for matching var, let, const, class or function declarations.
    */
   static class MatchDeclaration implements Predicate<Node> {
     @Override
     public boolean apply(Node n) {
-      return isFunctionDeclaration(n) || NodeUtil.isNameDeclaration(n);
+      return isDeclaration(n);
     }
   }
 

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

@@ -489,17 +489,22 @@ private void rewriteExportSpecShorthand(Node n) {
      *
      */
     private void splitExportDeclaration(Node n) {
+      if (n.getBooleanProp(Node.EXPORT_DEFAULT)) {
+        return;
+      }
       Node c = n.getFirstChild();
-      if (NodeUtil.isNameDeclaration(c) || c.isClass()) {
+      if (NodeUtil.isDeclaration(c)) {
         n.removeChild(c);
 
         Node exportSpecs = new Node(Token.EXPORT_SPECS).srcref(n);
         n.addChildToFront(exportSpecs);
         Iterable<Node> names;
-        if (c.isClass()) {
+        if (c.isClass() || c.isFunction()) {
           names = Collections.singleton(c.getFirstChild());
           n.getParent().addChildBefore(c, n);
-          n.getParent().addChildBefore(new Node(Token.EMPTY).srcref(n), n);
+          if (c.isClass()) {
+            n.getParent().addChildBefore(new Node(Token.EMPTY).srcref(n), n);
+          }
         } else {
           names = NodeUtil.getLhsNodesOfDeclaration(c);
           // Split up var declarations onto separate lines.
@@ -570,7 +575,10 @@ private void rewriteEs6ObjectLiteralShorthandPropertySyntax(Node n) {
      */
     static boolean visitFunction(Node n, AbstractCompiler compiler) {
       checkState(n.isFunction(), n);
-      if (NodeUtil.isFunctionDeclaration(n) && !NodeUtil.isHoistedFunctionDeclaration(n)) {
+      // TODO(lharker): function declarations within exports are hoisted - after fixing
+      // NodeUtil.isHoistedFunctionDeclaration we shouldn't need the special case for export below
+      if (NodeUtil.isFunctionDeclaration(n) && !NodeUtil.isHoistedFunctionDeclaration(n)
+          && !n.getParent().isExport()) {
         rewriteFunctionDeclaration(n, compiler);
         return true;
       } else if (n.isFunction() && !NodeUtil.getFunctionBody(n).isNormalBlock()) {

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

@@ -394,7 +394,7 @@ public void testIsFunctionDeclaration() {
     assertFalse(NodeUtil.isFunctionDeclaration(getFunctionNode("var x = function(){}")));
     assertTrue(NodeUtil.isFunctionDeclaration(getFunctionNode("export function f() {}")));
     assertFalse(NodeUtil.isFunctionDeclaration(getFunctionNode("export default function() {}")));
-    assertFalse(
+    assertTrue(
         NodeUtil.isFunctionDeclaration(getFunctionNode("export default function foo() {}")));
     assertFalse(
         NodeUtil.isFunctionDeclaration(getFunctionNode("export default (foo) => { alert(foo); }")));
@@ -404,8 +404,9 @@ public void testIsClassDeclaration() {
     assertTrue(NodeUtil.isClassDeclaration(getClassNode("class Foo {}")));
     assertFalse(NodeUtil.isClassDeclaration(getClassNode("var Foo = class {}")));
     assertFalse(NodeUtil.isClassDeclaration(getClassNode("var Foo = class Foo{}")));
-    assertFalse(NodeUtil.isClassDeclaration(getClassNode("export default class Foo {}")));
+    assertTrue(NodeUtil.isClassDeclaration(getClassNode("export default class Foo {}")));
     assertTrue(NodeUtil.isClassDeclaration(getClassNode("export class Foo {}")));
+    assertFalse(NodeUtil.isClassDeclaration(getClassNode("export default class {}")));
   }
 
   private void assertSideEffect(boolean se, String js) {

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

@@ -1043,16 +1043,21 @@ public void testSplitExportDeclarationWithConst() {
   }
 
   public void testSplitExportDeclarationOfFunction() {
-    // TODO(lharker): Change this to expect "function bar$jscomp$1() {};" once the bug
-    // rewriting functions to be var declarations in exports is fixed.
     test("export function bar() {};",
         LINE_JOINER.join(
-            "var bar$jscomp$1 = function() {}",
+            "function bar$jscomp$1() {}",
             "export {bar$jscomp$1 as bar};"
         ));
+
+    // Don't need to split declarations in default exports since they are either unnamed, or the
+    // name is declared in the module scope only.
+    testSame("export default function() {}");
+    test("export default function foo() {}", "export default function foo$jscomp$1() {}");
   }
 
   public void testSplitExportDeclarationOfClass() {
     test("export class Foo {};", "class Foo {}; export {Foo as Foo};");
+    testSame("export default class Bar {}");
+    testSame("export default class {}");
   }
 }