Improve support for refactoring JSDoc annotations:

* Handle stacked annotations such as @const @Private {Blah}
* Add support for @exports and @Package
* Correct bad replaces for annotations that are optionally followed by a type

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

src/com/google/javascript/refactoring/SuggestedFix.java

@@ -452,8 +452,6 @@ public Builder addOrReplaceJsDoc(Node n, String newJsDoc) {
      * Changes the JS Doc Type of the given node.
      */
     public Builder changeJsDocType(Node n, AbstractCompiler compiler, String type) {
-      JSDocInfo info = NodeUtil.getBestJSDocInfo(n);
-      Preconditions.checkNotNull(info, "Node %s does not have JS Doc associated with it.", n);
       Node typeNode = JsDocInfoParser.parseTypeString(type);
       Preconditions.checkNotNull(typeNode, "Invalid type: %s", type);
       JSTypeExpression typeExpr = new JSTypeExpression(typeNode, "jsflume");
@@ -462,15 +460,30 @@ public Builder changeJsDocType(Node n, AbstractCompiler compiler, String type) {
         throw new RuntimeException("JS Compiler does not recognize type: " + type);
       }
 
+      // TODO(mknichel): Use the JSDocInfoParser to find the end of the type declaration. This
+      // would also handle multiple lines, and record types (which contain '{')
+
+      // Only "@type" allows type names without "{}"
+      replaceTypePattern(n, type, Pattern.compile(
+          "@(type) *\\{?[^@\\s}]+\\}?"));
+
+      // Text following other annotations may be a comment, not a type.
+      replaceTypePattern(n, type, Pattern.compile(
+          "@(export|package|private|protected|public|const|return) *\\{[^}]+\\}"));
+
+      return this;
+    }
+
+    // The pattern supplied here should have one matching group, the annotation with
+    // associated the type expression, the entire pattern should match the annotation and
+    // the type expression to be replaced.
+    private void replaceTypePattern(Node n, String type, Pattern pattern) {
+      JSDocInfo info = NodeUtil.getBestJSDocInfo(n);
+      Preconditions.checkNotNull(info, "Node %s does not have JS Doc associated with it.", n);
       String originalComment = info.getOriginalCommentString();
       int originalPosition = info.getOriginalCommentPosition();
-
-      // If there isn't an original comment, then it is generated and we can't make a change.
       if (originalComment != null) {
-        // TODO(mknichel): Support multiline @type annotations.
-        Pattern typeDocPattern = Pattern.compile(
-            "@(type|private|protected|public|const|return) *\\{?[^\\s}@]+\\}?");
-        Matcher m = typeDocPattern.matcher(originalComment);
+        Matcher m = pattern.matcher(originalComment);
         while (m.find()) {
           replacements.put(
               n.getSourceFileName(),
@@ -480,8 +493,6 @@ public Builder changeJsDocType(Node n, AbstractCompiler compiler, String type) {
                   "@" + m.group(1) + " {" + type + "}"));
         }
       }
-
-      return this;
     }
 
     /**

test/com/google/javascript/refactoring/SuggestedFixTest.java

@@ -382,16 +382,72 @@ public void testChangeJsDocType2() {
   }
 
   @Test
-  public void testChangeJsDocType_privateType() {
+  public void testChangeJsDocType_packageType1() {
     String before = "/** ";
-    String after = "@private Foo */\nvar foo = new Foo()";
+    String after = "@package {Foo} */\nvar foo = new Foo()";
     Compiler compiler = getCompiler(before + after);
     Node root = compileToScriptRoot(compiler);
     SuggestedFix fix = new SuggestedFix.Builder()
         .changeJsDocType(root.getFirstChild(), compiler, "Object")
         .build();
     CodeReplacement replacement = new CodeReplacement(
-        before.length(), "@private Foo".length(), "@private {Object}");
+        before.length(), "@package {Foo}".length(), "@package {Object}");
+    assertReplacement(fix, replacement);
+  }
+
+  @Test
+  public void testChangeJsDocType_privateType1() {
+    String before = "/** ";
+    String after = "@private {Foo} */\nvar foo = new Foo()";
+    Compiler compiler = getCompiler(before + after);
+    Node root = compileToScriptRoot(compiler);
+    SuggestedFix fix = new SuggestedFix.Builder()
+        .changeJsDocType(root.getFirstChild(), compiler, "Object")
+        .build();
+    CodeReplacement replacement = new CodeReplacement(
+        before.length(), "@private {Foo}".length(), "@private {Object}");
+    assertReplacement(fix, replacement);
+  }
+
+  @Test
+  public void testChangeJsDocType_privateType2() {
+    String before = "/** @private ";
+    String after = "@const {Foo} */\nvar foo = new Foo()";
+    Compiler compiler = getCompiler(before + after);
+    Node root = compileToScriptRoot(compiler);
+    SuggestedFix fix = new SuggestedFix.Builder()
+        .changeJsDocType(root.getFirstChild(), compiler, "Object")
+        .build();
+    CodeReplacement replacement = new CodeReplacement(
+        before.length(), "@const {Foo}".length(), "@const {Object}");
+    assertReplacement(fix, replacement);
+  }
+
+  @Test
+  public void testChangeJsDocType_privateType3() {
+    String before = "/** @private ";
+    String after = "@const {Foo} */\nvar foo = new Foo()";
+    Compiler compiler = getCompiler(before + after);
+    Node root = compileToScriptRoot(compiler);
+    SuggestedFix fix = new SuggestedFix.Builder()
+        .changeJsDocType(root.getFirstChild(), compiler, "Object")
+        .build();
+    CodeReplacement replacement = new CodeReplacement(
+        before.length(), "@const {Foo}".length(), "@const {Object}");
+    assertReplacement(fix, replacement);
+  }
+
+  @Test
+  public void testChangeJsDocType_privateType4() {
+    String before = "/** ";
+    String after = "@const {Foo} */\nvar foo = new Foo()";
+    Compiler compiler = getCompiler(before + after);
+    Node root = compileToScriptRoot(compiler);
+    SuggestedFix fix = new SuggestedFix.Builder()
+        .changeJsDocType(root.getFirstChild(), compiler, "Object")
+        .build();
+    CodeReplacement replacement = new CodeReplacement(
+        before.length(), "@const {Foo}".length(), "@const {Object}");
     assertReplacement(fix, replacement);
   }