Make two changes to JSDoc matching

(1) when rewriting jsdoc type declarations first make sure the JSDoc actually contains a type before attempting anything (2) when attempting to rewrite JSDoc bailout when if the JSDoc object doesn't have an original text. The first prevents failures for cases like: /** @const */ var x = ... Where currently the code assumed if the type matched and there was a jsdoc object, it had to have a type declaration. The second handles cases like where the compiler generates a synthetic jsdoc object (where there isn't any jsdoc in the original source). ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=149667957
google · Mar 10, 2017 · fab4a78 · fab4a78
1 parent 8ac08c0
commit fab4a78
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 13 deletions.
diff --git a/src/com/google/javascript/refactoring/Matchers.java b/src/com/google/javascript/refactoring/Matchers.java
@@ -340,7 +340,7 @@ public static Matcher jsDocType(final String type) {
         JSDocInfo jsDoc = node.getParent().isVar()
             ? node.getParent().getJSDocInfo() : node.getJSDocInfo();
         JSType jsType = node.getJSType();
-        return jsDoc != null && jsType != null
+        return jsDoc != null && jsDoc.hasType() && jsType != null
             && providedJsType.isEquivalentTo(jsType.restrictByNotNullOrUndefined());
       }
     };

diff --git a/src/com/google/javascript/refactoring/SuggestedFix.java b/src/com/google/javascript/refactoring/SuggestedFix.java
@@ -465,17 +465,20 @@ public Builder changeJsDocType(Node n, AbstractCompiler compiler, String type) {
       String originalComment = info.getOriginalCommentString();
       int originalPosition = info.getOriginalCommentPosition();
 
-      // TODO(mknichel): Support multiline @type annotations.
+      // If there isn't an original comment, then it is generated and we can't make a change.
-      Pattern typeDocPattern = Pattern.compile(
+      if (originalComment != null) {
-          "@(type|private|protected|public|const|return) *\\{?[^\\s}]+\\}?");
+        // TODO(mknichel): Support multiline @type annotations.
-      Matcher m = typeDocPattern.matcher(originalComment);
+        Pattern typeDocPattern = Pattern.compile(
-      while (m.find()) {
+            "@(type|private|protected|public|const|return) *\\{?[^\\s}@]+\\}?");
-        replacements.put(
+        Matcher m = typeDocPattern.matcher(originalComment);
-            n.getSourceFileName(),
+        while (m.find()) {
-            new CodeReplacement(
+          replacements.put(
-                originalPosition + m.start(),
+              n.getSourceFileName(),
-                m.end() - m.start(),
+              new CodeReplacement(
-                "@" + m.group(1) + " {" + type + "}"));
+                  originalPosition + m.start(),
+                  m.end() - m.start(),
+                  "@" + m.group(1) + " {" + type + "}"));
+        }
       }
 
       return this;

diff --git a/test/com/google/javascript/refactoring/MatchersTest.java b/test/com/google/javascript/refactoring/MatchersTest.java
@@ -271,7 +271,7 @@ public void testPrototypeDeclarations() {
   }
 
   @Test
-  public void testJsDocType() {
+  public void testJsDocType1() {
     String input = "/** @type {number} */ var foo = 1;";
     Compiler compiler = getCompiler(input);
     Node root = compileToScriptRoot(compiler);
@@ -280,6 +280,46 @@ public void testJsDocType() {
     assertFalse(Matchers.jsDocType("string").matches(node, new NodeMetadata(compiler)));
   }
 
+  @Test
+  public void testJsDocType2() {
+    String input = "/** @type {number} */ let foo = 1;";
+    Compiler compiler = getCompiler(input);
+    Node root = compileToScriptRoot(compiler);
+    Node node = root.getFirstFirstChild();
+    assertTrue(Matchers.jsDocType("number").matches(node, new NodeMetadata(compiler)));
+    assertFalse(Matchers.jsDocType("string").matches(node, new NodeMetadata(compiler)));
+  }
+
+  @Test
+  public void testJsDocType3() {
+    String input = "/** @type {number} */ const foo = 1;";
+    Compiler compiler = getCompiler(input);
+    Node root = compileToScriptRoot(compiler);
+    Node node = root.getFirstFirstChild();
+    assertTrue(Matchers.jsDocType("number").matches(node, new NodeMetadata(compiler)));
+    assertFalse(Matchers.jsDocType("string").matches(node, new NodeMetadata(compiler)));
+  }
+
+  @Test
+  public void testJsDocTypeNoMatch1() {
+    String input = "/** @const */ var foo = 1;";
+    Compiler compiler = getCompiler(input);
+    Node root = compileToScriptRoot(compiler);
+    Node node = root.getFirstFirstChild();
+    assertFalse(Matchers.jsDocType("number").matches(node, new NodeMetadata(compiler)));
+    assertFalse(Matchers.jsDocType("string").matches(node, new NodeMetadata(compiler)));
+  }
+
+  @Test
+  public void testJsDocTypeNoMatch2() {
+    String input = "const foo = 1;";
+    Compiler compiler = getCompiler(input);
+    Node root = compileToScriptRoot(compiler);
+    Node node = root.getFirstFirstChild();
+    assertFalse(Matchers.jsDocType("number").matches(node, new NodeMetadata(compiler)));
+    assertFalse(Matchers.jsDocType("string").matches(node, new NodeMetadata(compiler)));
+  }
+
   @Test
   public void testPropertyAccess() {
     String input = "foo.bar.method();";