Repository: jshint/jshint
Pull request #3382 (2019-03-31 20:49:25)

[[FIX]] Correct parsing of class method names
@rwaldron I found an unrelated bug while fixing gh-3381, and I've included a
patch for that here, too. gh-3381 only effected class methods, but I added a
regression tests for method defined in object initializers for good measure.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/22510525/badge)](https://coveralls.io/builds/22510525)

Coverage increased (+0.0003%) to 98.545% when pulling **b619b1c66ede0ed6888ba0d5183b13dc37c07ed7 on jugglinmike:class-method-names** into **07ff25d3f4379415f0b438d9fb7cc6f6cabb7f7a on jshint:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/22510525/badge)](https://coveralls.io/builds/22510525)

Coverage increased (+0.0003%) to 98.545% when pulling **b619b1c66ede0ed6888ba0d5183b13dc37c07ed7 on jugglinmike:class-method-names** into **07ff25d3f4379415f0b438d9fb7cc6f6cabb7f7a on jshint:master**.

rwaldron: https://github.com/jshint/jshint/pull/3379 has landed
jugglinmike: Rebased and passing as expected, @rwaldron 

Patch:
diff --git a/src/jshint.js b/src/jshint.js
index 40cc3db8..3d4ba500 100644
--- a/src/jshint.js
+++ b/src/jshint.js
@@ -2843,6 +2843,24 @@ var JSHINT = (function() {
       }
 
       token = state.tokens.next;
+
+      if ((token.value === "set" || token.value === "get") && !checkPunctuator(peek(), "(")) {
+        if (inGenerator) {
+          error("E024", token, token.value);
+        }
+        accessorType = token.value;
+        advance();
+        token = state.tokens.next;
+
+        if (!isStatic && token.value === "constructor") {
+          error("E049", token, "class " + accessorType + "ter method", token.value);
+        } else if (isStatic && token.value === "prototype") {
+          error("E049", token, "static class " + accessorType + "ter method", token.value);
+        }
+      } else {
+        accessorType = null;
+      }
+
       switch (token.value) {
         case ";":
           warning("W032", token);
@@ -2857,35 +2875,14 @@ var JSHINT = (function() {
           } else {
             if (inGenerator || context & prodParams.preAsync) {
               error("E024", token, token.value);
-            }
-            if (hasConstructor) {
+            } else if (hasConstructor) {
               error("E024", token, token.value);
+            } else {
+              hasConstructor = !accessorType && !isStatic;
             }
             advance();
             doMethod(classToken, context, state.nameStack.infer());
-            hasConstructor = true;
           }
-          break;
-        case "set":
-        case "get":
-          if (inGenerator) {
-            error("E024", token, token.value);
-          }
-          accessorType = token.value;
-          advance();
-
-          if (state.tokens.next.value === "[") {
-            name = computedPropertyName(context);
-            doMethod(classToken, context, name, false);
-          } else {
-            name = propertyName(context);
-            if (name === "prototype" || name === "constructor") {
-              error("E049", state.tokens.curr, "class " + accessorType + "ter method", name);
-            }
-            saveAccessor(accessorType, props, name, state.tokens.curr, true, isStatic);
-            doMethod(classToken, context, state.nameStack.infer(), false);
-          }
-
           break;
         case "[":
           name = computedPropertyName(context);
@@ -2894,15 +2891,23 @@ var JSHINT = (function() {
           break;
         default:
           name = propertyName(context);
-          if (!name) {
+          if (name === undefined) {
             error("E024", token, token.value);
             advance();
             break;
           }
-          if (name === "prototype") {
-            error("E049", token, "class method", name);
+
+          if (accessorType) {
+            saveAccessor(accessorType, props, name, token, true, isStatic);
+            name = state.nameStack.infer();
+          } else {
+            if (isStatic && name === "prototype") {
+              error("E049", token, "static class method", name);
+            }
+
+            saveProperty(props, name, token, true, isStatic);
           }
-          saveProperty(props, name, token, true, isStatic);
+
           doMethod(classToken, context, name, inGenerator);
           break;
       }
@@ -4135,12 +4140,13 @@ var JSHINT = (function() {
 
           // Due to visual symmetry with the array rest property (and the early
           // design of the language feature), developers may mistakenly assume
-          // any expression is valid in this position.  Parse an expression and
-          // issue an error in order to recover more gracefully from this
-          // condition.
-          expr = expression(context, 10);
-
-          if (expr.type !== "(identifier)") {
+          // any expression is valid in this position. If the next token is not
+          // an identifier, attempt to parse an expression and issue an error.
+          // order to recover more gracefully from this condition.
+          if (state.tokens.next.type === "(identifier)") {
+            id = identifier(context);
+          } else {
+            expr = expression(context, 10);
             error("E030", expr, expr.value);
           }
         } else {


Test patch:
diff --git a/tests/test262/expectations.txt b/tests/test262/expectations.txt
index d3803a5e..c79e55f6 100644
--- a/tests/test262/expectations.txt
+++ b/tests/test262/expectations.txt
@@ -10808,8 +10808,6 @@ test/language/expressions/class/elements/syntax/valid/grammar-privatename-no-ini
 test/language/expressions/class/elements/syntax/valid/grammar-privatename-no-initializer-with-method.js(strict mode)
 test/language/expressions/class/elements/syntax/valid/grammar-privatenames-multi-line.js(default)
 test/language/expressions/class/elements/syntax/valid/grammar-privatenames-multi-line.js(strict mode)
-test/language/expressions/class/elements/syntax/valid/grammar-static-ctor-accessor-meth-valid.js(default)
-test/language/expressions/class/elements/syntax/valid/grammar-static-ctor-accessor-meth-valid.js(strict mode)
 test/language/expressions/class/elements/syntax/valid/grammar-static-private-async-gen-meth-prototype.js(default)
 test/language/expressions/class/elements/syntax/valid/grammar-static-private-async-gen-meth-prototype.js(strict mode)
 test/language/expressions/class/elements/syntax/valid/grammar-static-private-async-meth-prototype.js(default)
@@ -10842,8 +10840,6 @@ test/language/statements/class/elements/syntax/valid/grammar-privatename-no-init
 test/language/statements/class/elements/syntax/valid/grammar-privatename-no-initializer-with-method.js(strict mode)
 test/language/statements/class/elements/syntax/valid/grammar-privatenames-multi-line.js(default)
 test/language/statements/class/elements/syntax/valid/grammar-privatenames-multi-line.js(strict mode)
-test/language/statements/class/elements/syntax/valid/grammar-static-ctor-accessor-meth-valid.js(default)
-test/language/statements/class/elements/syntax/valid/grammar-static-ctor-accessor-meth-valid.js(strict mode)
 test/language/statements/class/elements/syntax/valid/grammar-static-private-async-gen-meth-prototype.js(default)
 test/language/statements/class/elements/syntax/valid/grammar-static-private-async-gen-meth-prototype.js(strict mode)
 test/language/statements/class/elements/syntax/valid/grammar-static-private-async-meth-prototype.js(default)
diff --git a/tests/unit/objrestspread.js b/tests/unit/objrestspread.js
index f2489d16..d000a42c 100644
--- a/tests/unit/objrestspread.js
+++ b/tests/unit/objrestspread.js
@@ -115,5 +115,9 @@ exports.rest = function (test) {
     .addError(1, 7, "Expected an identifier and instead saw '{'.")
     .test("({ ...{} } = {});", { esversion: 9 });
 
+  TestRun(test, "gh-3377 - identifier interpreted as new binding, not reference")
+    .addError(1, 10, "'x' is defined but never used.")
+    .test("var { ...x } = {};", { esversion: 9, unused: true, undef: true });
+
   test.done();
 };
diff --git a/tests/unit/parser.js b/tests/unit/parser.js
index dcfd3919..b6ad6a5c 100644
--- a/tests/unit/parser.js
+++ b/tests/unit/parser.js
@@ -6256,6 +6256,13 @@ exports["object ComputedPropertyName"] = function (test) {
     .addError(19, 17, "Setter is defined without getter.")
     .test(code, { esnext: true });
 
+  TestRun(test, "regression test for gh-3381")
+    .test([
+      "void {",
+      "  set() {}",
+      "};"
+    ], {esversion: 6});
+
   TestRun(test)
     .addError(6, 1, "'computed property names' is only available in ES6 (use 'esversion: 6').")
     .addError(7, 8, "'computed property names' is only available in ES6 (use 'esversion: 6').")
@@ -6661,7 +6668,6 @@ exports["class and method naming"] = function (test) {
     .addError(2, 7, "Strict violation.")
     .addError(4, 7, "A class getter method cannot be named 'constructor'.")
     .addError(5, 7, "A class setter method cannot be named 'constructor'.")
-    .addError(6, 3, "A class method cannot be named 'prototype'.")
     .addError(7, 6, "Class properties must be methods. Expected '(' but instead saw 'extra'.")
     .addError(8, 11, "Class properties must be methods. Expected '(' but instead saw 'extraIdent1'.")
     .addError(9, 11, "Class properties must be methods. Expected '(' but instead saw 'extraIdent2'.")
@@ -6681,6 +6687,53 @@ exports["class and method naming"] = function (test) {
 
   run.test(code, {esnext: true});
 
+  TestRun(test, "valid uses of name `constructor`")
+    .test([
+      "void class {",
+      "  constructor() {}",
+      "};",
+      "void class {",
+      "  static constructor() {}",
+      "};",
+      "void class {",
+      "  static get constructor() {}",
+      "};"
+    ], {esversion: 6});
+
+  TestRun(test, "valid uses of name `prototype`")
+    .test([
+      "void class {",
+      "  get prototype() {}",
+      "};",
+      "void class {",
+      "  get prototype() {}",
+      "  set prototype(x) {}",
+      "};"
+    ], {esversion: 6});
+
+  TestRun(test, "invalid use of name `prototype`: static method")
+    .addError(2, 10, "A static class method cannot be named 'prototype'.")
+    .test([
+      "void class {",
+      "  static prototype() {}",
+      "};"
+    ], {esversion: 6});
+
+  TestRun(test, "invalid use of name `prototype`: static accessor method")
+    .addError(2, 14, "A static class getter method cannot be named 'prototype'.")
+    .test([
+      "void class {",
+      "  static get prototype() {}",
+      "};"
+    ], {esversion: 6});
+
+  TestRun(test, "regression test for gh-3381")
+    .test([
+      "void class {",
+      "  set() {}",
+      "};"
+    ], {esversion: 6});
+
   TestRun(test, "hazardous method names (see gh-3358)")
     .addError(3, 17, "'hasOwnProperty' is a really bad name.")
     .test([