Don't warn on test methods in ES6 classes in the missing JSDoc lint c…

…heck

This has been broken for a while but was unnoticed due to a separate bug, which prevented any missing JSDoc lint warnings on ES6 class methods in modules.

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

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

@@ -5355,7 +5355,7 @@ static Node getBestLValueOwner(@Nullable Node lValue) {
   }
 
   /** Get the name of the given l-value node. */
-  static String getBestLValueName(@Nullable Node lValue) {
+  public static String getBestLValueName(@Nullable Node lValue) {
     if (lValue == null || lValue.getParent() == null) {
       return null;
     }

src/com/google/javascript/jscomp/lint/CheckJSDocStyle.java

@@ -226,12 +226,8 @@ private void visitClass(NodeTraversal t, Node cls) {
   }
 
   private void checkMissingJsDoc(NodeTraversal t, Node function) {
-    if (isFunctionThatShouldHaveJsDoc(t, function)) {
-      String name = NodeUtil.getName(function);
-      // Don't warn for test functions, setUp, tearDown, etc.
-      if (name == null || !ExportTestFunctions.isTestFunction(name)) {
-        t.report(function, MISSING_JSDOC);
-      }
+    if (isFunctionThatShouldHaveJsDoc(t, function) && !isTestMethod(function)) {
+      t.report(function, MISSING_JSDOC);
     }
   }
 
@@ -241,6 +237,7 @@ private void checkMissingJsDoc(NodeTraversal t, Node function) {
    */
   private boolean isFunctionThatShouldHaveJsDoc(NodeTraversal t, Node function) {
     if (!(t.inGlobalHoistScope() || t.inModuleScope())) {
+      // TODO(b/233631820): this should check for the module hoist scope instead
       return false;
     }
     if (NodeUtil.isFunctionDeclaration(function)) {
@@ -272,6 +269,13 @@ private boolean isFunctionThatShouldHaveJsDoc(NodeTraversal t, Node function) {
     return false;
   }
 
+  /** Whether this is a test method (test* or setup/teardown) that does not require JSDoc */
+  private boolean isTestMethod(Node function) {
+    Node bestLValue = NodeUtil.getBestLValue(function);
+    String name = bestLValue != null ? NodeUtil.getBestLValueName(bestLValue) : null;
+    return name != null && ExportTestFunctions.isTestFunction(name);
+  }
+
   private boolean isConstructorWithoutParameters(Node function) {
     return NodeUtil.isEs6Constructor(function)
         && !NodeUtil.getFunctionParameters(function).hasChildren();

test/com/google/javascript/jscomp/lint/CheckJSDocStyleTest.java

@@ -318,6 +318,21 @@ public void testMissingJsDoc_noWarningIfNotTopLevelAndNoParams() {
         "})"));
   }
 
+  @Test
+  public void testMissingJsDoc_noWarning_wizConstructorAndDeps() {
+    // Exempt Wiz controller constructor and deps() method because Wiz automatically adds JSDoc
+    // NOTE(lharker@): right now this does not warn because of b/124061048: the behavior is correct
+    // but for the wrong reason.
+    testSame(
+        lines(
+            "goog.module('a.b.MyController');",
+            "class MyController extends SomeParentController {",
+            "  static deps() { return {model: 0}; }",
+            "  constructor({model}) {}",
+            "}",
+            "registerController(MY_CONTROLLER, MyController);"));
+  }
+
   @Test
   public void testMissingJsDoc_noWarningOnTestFunctions() {
     testSame("function testSomeFunctionality() {}");
@@ -332,6 +347,15 @@ public void testMissingJsDoc_noWarningOnTestFunctions() {
     testSame("var tearDown = function() {};");
   }
 
+  @Test
+  public void testMissingJsDoc_noWarningOnTestMethods() {
+    testSame("class MyClass { testSomeFunctionality() {} }");
+    testSame("goog.module('mod'); class MyClass { testSomeFunctionality() {} }");
+    testSame("a.b.c = class { testSomeFunctionality() {} }");
+    testSame("class MyClass { setUp() {} }");
+    testSame("class MyClass { tearDown() {} }");
+  }
+
   @Test
   public void testMissingJsDoc_noWarningOnTestFunctions_withES6Modules() {
     testSame("export function testSomeFunctionality() {}");
@@ -351,6 +375,9 @@ public void testMissingJsDoc_noWarningOnEmptyConstructor_withES6Modules() {
   public void testMissingJsDoc_googModule() {
     testWarning("goog.module('a.b.c'); function f() {}", MISSING_JSDOC);
     testWarning("goog.module('a.b.c'); var f = function() {};", MISSING_JSDOC);
+    // TODO(b/124061048): these should also warn for missing JSDoc
+    testSame("goog.module('a.b.c'); class Foo { constructor(x) {} }");
+    testSame("goog.module('a.b.c'); class Foo { someMethod() {} }");
   }
 
   @Test
@@ -382,6 +409,8 @@ public void testMissingJsDoc_ES6Module05() {
   public void testMissingJsDoc_googModule_noWarning() {
     testSame("goog.module('a.b.c'); /** @type {function()} */ function f() {}");
     testSame("goog.module('a.b.c'); /** @type {function()} */ var f = function() {};");
+    // No param constructors do not require JSDoc
+    testSame("goog.module('a.b.c'); class Foo { constructor() {} }");
   }
 
   @Test