Repository: jshint/jshint
Pull request #3355 (2019-02-05 17:19:15)

[[FIX]] Do not add cls method names to env. record
A recent refactoring of the class parsing logic [1] removed the function
name inference logic (i.e. integration with the `nameStack` object) in
favor of a similar codepath within the generic `doFunction` routine. By
providing the method name to that routine, the change had an unintended
side effect: method names were used to create new entries in the new
environment record. This was observable from user code in the presence
of JSHint's `unused` linting option: when set, methods would incorrectly
shadow variables that shared the same name.

Continue to rely on the `doFunction` method for name inference (limiting
duplication), but disable the extension of the environment record for
that case.

This is intended to resolve gh-3354.

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

Coverage remained the same at 98.471% when pulling **38f683d69658ae5abd16c991bd16225d23f2c57a on jugglinmike:class-method-name** into **b7faa24675c38ce27cf783bc7f8841cf143d1641 on jshint:master**.

rwaldron: Cool bug and nice fix :D

Patch:
diff --git a/src/jshint.js b/src/jshint.js
index d452d239..3b2f5871 100644
--- a/src/jshint.js
+++ b/src/jshint.js
@@ -3691,7 +3691,7 @@ var JSHINT = (function() {
     // 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
     state.funct["(scope)"].stack("functionouter");
-    var internallyAccessibleName = name || classExprBinding;
+    var internallyAccessibleName = !isMethod && (name || classExprBinding);
     if (internallyAccessibleName) {
       state.funct["(scope)"].block.add(internallyAccessibleName,
         classExprBinding ? "class" : "function", state.tokens.curr, false);


Test patch:
diff --git a/tests/unit/options.js b/tests/unit/options.js
index 579fd54b..4202991f 100644
--- a/tests/unit/options.js
+++ b/tests/unit/options.js
@@ -1107,6 +1107,73 @@ exports.unused.crossBlocks = function (test) {
   test.done();
 };
 
+// Regression test for gh-3354
+exports.unused.methodNames = function (test) {
+  TestRun(test, "object methods - ES5")
+    .test([
+      "var p;",
+      "void {",
+      "  get p() { void p; },",
+      "  set p(_) { void p; void _; }",
+      "};"
+    ], { unused: true, esversion: 5 });
+
+  TestRun(test, "object methods - ES6")
+    .test([
+      "var m, g;",
+      "void {",
+      "  m() { void m; },",
+      "  *g() { yield g; }",
+      "};"
+    ], { unused: true, esversion: 6 });
+
+  TestRun(test, "object methods - ES8")
+    .test([
+      "var m;",
+      "void {",
+      "  async m() { void m; }",
+      "};"
+    ], { unused: true, esversion: 8 });
+
+  TestRun(test, "object methods - ES9")
+    .test([
+      "var m;",
+      "void {",
+      "  async * m() { yield m; }",
+      "};"
+    ], { unused: true, esversion: 9 });
+
+  TestRun(test, "class methods - ES6")
+    .test([
+      "var m, g, p, s;",
+      "void class {",
+      "  m() { void m; }",
+      "  *g() { yield g; }",
+      "  get p() { void p; }",
+      "  set p() { void p; }",
+      "  static s() { void s; }",
+      "};"
+    ], { unused: true, esversion: 6 });
+
+  TestRun(test, "class methods - ES8")
+    .test([
+      "var m;",
+      "void class {",
+      "  async m() { void m; }",
+      "};"
+    ], { unused: true, esversion: 8 });
+
+  TestRun(test, "class methods - ES9")
+    .test([
+      "var m;",
+      "void class {",
+      "  async * m() { yield m; }",
+      "};"
+    ], { unused: true, esversion: 9 });
+
+  test.done();
+};
+
 exports['param overrides function name expression'] = function (test) {
   TestRun(test)
     .test([