[[FIX]] Classes are not hoisted

Classes are not hoisted and have a TDZ like let and const.
In addition logic around when a class/function name is exposed has been
tidied up so that function/class expressions do not have their name made
available outside the expression.
Fixes #1934

src/jshint.js

@@ -2926,6 +2926,18 @@ var JSHINT = (function() {
 
     functions.push(state.funct);
 
+    // So that the function is available to itself and referencing itself is not
+    // seen as a closure, add the function name to a new scope, but do not
+    // test for unused (unused: false)
+    // it is a new block scope so that params can override it, it can be block scoped
+    // but declarations inside the function don't cause already declared error
+    var internallyAccessibleName = name || classExprBinding;
+    if (internallyAccessibleName) {
+      state.funct["(scope)"].stack();
+      state.funct["(scope)"].block.add(internallyAccessibleName,
+        classExprBinding ? "class" : "function", state.tokens.curr, false);
+    }
+
     var params = functionparams(options);
     var paramsIds;
     if (params) {
@@ -2939,18 +2951,6 @@ var JSHINT = (function() {
     state.funct["(params)"] = paramsIds;
     state.funct["(metrics)"].verifyMaxParametersPerFunction(paramsIds);
 
-    if (name) {
-      // So that the function is available to itself and referencing itself is not
-      // seen as a closure, add the function name to the current scope, but do not
-      // test for unused (unused: false)
-      state.funct["(scope)"].funct.add(name, "function", state.tokens.curr, false);
-    }
-
-    if (classExprBinding) {
-      // if we are inside a class expression, make the class name available to sub functions
-      state.funct["(scope)"].funct.add(classExprBinding, "function", state.tokens.curr, false);
-    }
-
     if (isArrow) {
       if (!state.option.esnext) {
         warning("W119", state.tokens.curr, "arrow function syntax (=>)");
@@ -2978,6 +2978,11 @@ var JSHINT = (function() {
 
     state.funct["(scope)"].unstack(); // also does usage and label checks
 
+    // if we have a name, unstack that scope (see above where it is stacked)
+    if (internallyAccessibleName) {
+      state.funct["(scope)"].unstack();
+    }
+
     state.funct = state.funct["(context)"];
 
     return f;
@@ -4437,26 +4442,25 @@ var JSHINT = (function() {
     }
 
     if (state.tokens.next.type === "default") {
-      // ExportDeclaration :: export default HoistableDeclaration
-      // ExportDeclaration :: export default ClassDeclaration
+      // ExportDeclaration ::
+      //      export default [lookahead  { function, class }] AssignmentExpression[In] ;
+      //      export default HoistableDeclaration
+      //      export default ClassDeclaration
       state.nameStack.set(state.tokens.next);
       advance("default");
-      if (state.tokens.next.id === "function" || state.tokens.next.id === "class") {
+      var exportType = state.tokens.next.id;
+      if (exportType === "function" || exportType === "class") {
         this.block = true;
       }
 
       token = peek();
 
       expression(10);
 
-      if (state.tokens.next.id === "class") {
-        identifier = token.name;
-      } else {
-        identifier = token.value;
-      }
+      identifier = token.value;
 
       state.funct["(scope)"].addlabel(identifier, {
-        type: "function",
+        type: exportType,
         token: token });
 
       state.funct["(scope)"].setExported(identifier, token);
@@ -4527,8 +4531,9 @@ var JSHINT = (function() {
       // ExportDeclaration :: export Declaration
       this.block = true;
       advance("class");
-      state.funct["(scope)"].setExported(state.tokens.next.value, state.tokens.next);
+      var classNameToken = state.tokens.next;
       state.syntax["class"].fud();
+      state.funct["(scope)"].setExported(classNameToken.value, classNameToken);
     } else {
       error("E024", state.tokens.next, state.tokens.next.value);
     }

src/messages.js

@@ -96,7 +96,7 @@ var warnings = {
   W018: "Confusing use of '{a}'.",
   W019: "Use the isNaN function to compare with NaN.",
   W020: "Read only.",
-  W021: "'{a}' is a function.",
+  W021: "'{a}' is a {b}.", // is a class or is a function
   W022: "Do not assign to the exception parameter.",
   W023: "Expected an identifier in an assignment and instead saw a function invocation.",
   W024: "Expected an identifier and instead saw '{a}' (a reserved word).",

src/scope-manager.js

@@ -272,6 +272,7 @@ var scopeManager = function(state, predefined, exported, declared) {
         var usage = currentUsages[usedLabelName];
         var usedLabel = currentLabels[usedLabelName];
         if (usedLabel) {
+          var usedLabelType = usedLabel["(type)"];
 
           if (usedLabel["(useOutsideOfScope)"] && !state.option.funcscope) {
             var usedTokens = usage["(tokens)"];
@@ -289,16 +290,17 @@ var scopeManager = function(state, predefined, exported, declared) {
           _current["(labels)"][usedLabelName]["(unused)"] = false;
 
           // check for modifying a const
-          if (usedLabel["(type)"] === "const" && usage["(modified)"]) {
+          if (usedLabelType === "const" && usage["(modified)"]) {
             for (j = 0; j < usage["(modified)"].length; j++) {
               error("E013", usage["(modified)"][j], usedLabelName);
             }
           }
 
           // check for re-assigning a function declaration
-          if (usedLabel["(type)"] === "function" && usage["(reassigned)"]) {
+          if ((usedLabelType === "function" || usedLabelType === "class") &&
+              usage["(reassigned)"]) {
             for (j = 0; j < usage["(reassigned)"].length; j++) {
-              error("W021", usage["(reassigned)"][j], usedLabelName);
+              error("W021", usage["(reassigned)"][j], usedLabelName, usedLabelType);
             }
           }
           continue;
@@ -532,7 +534,7 @@ var scopeManager = function(state, predefined, exported, declared) {
 
       var type  = opts.type;
       var token = opts.token;
-      var isblockscoped = type === "let" || type === "const";
+      var isblockscoped = type === "let" || type === "const" || type === "class";
 
       // outer shadow check (inner is only on non-block scoped)
       _checkOuterShadow(labelName, token, type);
@@ -661,7 +663,7 @@ var scopeManager = function(state, predefined, exported, declared) {
 
       /**
        * Adds a new function scoped variable
-       * see current.add for block scoped
+       * see block.add for block scoped
        */
       add: function(labelName, type, tok, unused) {
         _current["(labels)"][labelName] = {

tests/unit/core.js

@@ -1005,7 +1005,7 @@ exports.testConstModification = function (test) {
   test.done();
 };
 
-exports["class declaration export (default)"] = function (test) {
+exports["class declaration export"] = function (test) {
   var source = fs.readFileSync(__dirname + "/fixtures/class-declaration.js", "utf8");
 
   TestRun(test).test(source, {
@@ -1016,7 +1016,7 @@ exports["class declaration export (default)"] = function (test) {
   test.done();
 };
 
-exports["function declaration export (default)"] = function (test) {
+exports["function declaration export"] = function (test) {
   var source = fs.readFileSync(__dirname + "/fixtures/function-declaration.js", "utf8");
 
   TestRun(test).test(source, {
@@ -1027,6 +1027,35 @@ exports["function declaration export (default)"] = function (test) {
   test.done();
 };
 
+exports.classIsBlockScoped = function (test) {
+  var code = [
+    "new A();", // use in TDZ
+    "class A {}",
+    "class B extends C {}", // use in TDZ
+    "class C {}",
+    "new D();", // not defined
+    "let E = class D {" +
+    "  constructor() { D.static(); }",
+    "  myfunc() { return D; }",
+    "};",
+    "new D();", // not defined
+    "if (true) {",
+    "  class F {}",
+    "}",
+    "new F();" // not defined
+  ];
+
+  TestRun(test)
+    .addError(2, "'A' was used before it was declared, which is illegal for 'class' variables.")
+    .addError(4, "'C' was used before it was declared, which is illegal for 'class' variables.")
+    .addError(5, "'D' is not defined.")
+    .addError(9, "'D' is not defined.")
+    .addError(13, "'F' is not defined.")
+    .test(code, { esnext: true, undef: true });
+
+  test.done();
+};
+
 exports.testES6ModulesNamedExportsAffectUndef = function (test) {
   // The identifier "foo" is expected to have been defined in the scope
   // of this file in order to be exported.

tests/unit/fixtures/class-declaration.js

@@ -13,3 +13,6 @@ export default class Header extends Section {
 Object.defineProperty(Header, "CONST", {
   value: 1
 });
+
+export class NotDefault extends Header {}
+new NotDefault();

tests/unit/fixtures/function-declaration.js

@@ -3,3 +3,9 @@ export default function Header() {}
 Object.defineProperty(Header, "CONST", {
   value: 1
 });
+
+export function Header() {}
+
+Object.defineProperty(Header, "CONST", {
+  value: 1
+});

tests/unit/options.js

@@ -853,6 +853,33 @@ exports.unused = function (test) {
   test.done();
 };
 
+exports['param overrides function name expression'] = function (test) {
+  TestRun(test)
+    .test([
+      "var A = function B(B) {",
+      "  return B;",
+      "};",
+      "A();"
+    ], { undef: true, unused: "strict" });
+
+  test.done();
+};
+
+exports['let can re-use function and class name'] = function (test) {
+  TestRun(test)
+    .test([
+      "var A = function B(C) {",
+      "  let B = C;",
+      "  return B;",
+      "};",
+      "A();",
+      "var D = class E { constructor(F) { let E = F; return E; }};",
+      "D();"
+    ], { undef: true, unused: "strict", esnext: true });
+
+  test.done();
+};
+
 exports['unused data with options'] = function (test) {
 
   // see gh-1894 for discussion on this test

tests/unit/parser.js

@@ -5522,7 +5522,13 @@ exports.classes = function (test) {
 
   var run = TestRun(test)
     .addError(cdecl + 4, "Expected an identifier and instead saw 'package' (a reserved word).")
-    .addError(cexpr + 4, "Expected an identifier and instead saw 'package' (a reserved word).");
+    .addError(cexpr + 4, "Expected an identifier and instead saw 'package' (a reserved word).")
+    .addError(cdeclAssn + 4, "'Foo15' is a class.")
+    .addError(cdeclAssn + 7, "'Foo18' is a class.")
+    .addError(cdeclAssn + 7, "'Foo17' is a class.")
+    .addError(cexprAssn + 4, "'Foo15' is a class.")
+    .addError(cexprAssn + 7, "'Foo18' is a class.")
+    .addError(cexprAssn + 7, "'Foo17' is a class.");
 
   run.test(code, {esnext: true});
   run.test(code, {moz: true});
@@ -5539,6 +5545,14 @@ exports.classes = function (test) {
     .addError(cexpr + 3, "Expected an identifier and instead saw 'protected' (a reserved word).")
     .addError(cexpr + 4, "'package' is defined but never used.");
 
+  run
+    .addError(cdeclAssn + 4, "'Foo15' is a class.")
+    .addError(cdeclAssn + 7, "'Foo18' is a class.")
+    .addError(cdeclAssn + 7, "'Foo17' is a class.")
+    .addError(cexprAssn + 4, "'Foo15' is a class.")
+    .addError(cexprAssn + 7, "'Foo18' is a class.")
+    .addError(cexprAssn + 7, "'Foo17' is a class.");
+
   code[0] = "'use strict';" + code[0];
   run.test(code, {unused: true, globalstrict: true, esnext: true});
   run.test(code, {unused: true, globalstrict: true, moz: true});
@@ -5675,11 +5689,11 @@ exports.classExpression = function (test) {
   ];
 
   TestRun(test)
-    .addError(2, "'MyClass' is a function.")
-    .addError(3, "'MyClass' is a function.")
-    .addError(4, "'MyClass' is a function.")
-    .addError(5, "'MyClass' is a function.")
-    .addError(6, "'MyClass' is a function.")
+    .addError(2, "'MyClass' is a class.")
+    .addError(3, "'MyClass' is a class.")
+    .addError(4, "'MyClass' is a class.")
+    .addError(5, "'MyClass' is a class.")
+    .addError(6, "'MyClass' is a class.")
     .addError(8, "'MyClass' is not defined.")
     .test(code, { esnext: true, undef: true });