diff --git a/src/com/google/javascript/jscomp/JSDocInfoPrinter.java b/src/com/google/javascript/jscomp/JSDocInfoPrinter.java index 4ee0d6c52fe..d8d9ee068dc 100644 --- a/src/com/google/javascript/jscomp/JSDocInfoPrinter.java +++ b/src/com/google/javascript/jscomp/JSDocInfoPrinter.java @@ -38,9 +38,19 @@ public final class JSDocInfoPrinter { private final boolean useOriginalName; + private final boolean printDesc; - JSDocInfoPrinter(boolean useOriginalName) { + public JSDocInfoPrinter(boolean useOriginalName) { + this(useOriginalName, false); + } + + /** + * @param useOriginalName Whether to use the original name field when printing types. + * @param printDesc Whether to print block, param, and return descriptions. + */ + public JSDocInfoPrinter(boolean useOriginalName, boolean printDesc) { this.useOriginalName = useOriginalName; + this.printDesc = printDesc; } public String print(JSDocInfo info) { @@ -108,7 +118,10 @@ public String print(JSDocInfo info) { String description = info.getDescription(); if (description != null) { - parts.add("@desc " + description + '\n'); + if (description.contains("\n")) { + multiline = true; + } + parts.add("@desc " + description); } if (info.makesDicts()) { @@ -162,13 +175,14 @@ public String print(JSDocInfo info) { if (info.getParameterCount() > 0) { multiline = true; for (String name : info.getParameterNames()) { - parts.add("@param " + buildParamType(name, info.getParameterType(name))); + parts.add("@param " + buildParamType(info, name)); } } if (info.hasReturnType()) { multiline = true; - parts.add(buildAnnotationWithType("return", info.getReturnType())); + parts.add( + buildAnnotationWithType("return", info.getReturnType(), info.getReturnDescription())); } if (!info.getThrownTypes().isEmpty()) { @@ -177,7 +191,7 @@ public String print(JSDocInfo info) { ImmutableList names = info.getTemplateTypeNames(); if (!names.isEmpty()) { - parts.add("@template " + Joiner.on(',').join(names)); + parts.add("@template " + Joiner.on(", ").join(names)); multiline = true; } @@ -262,16 +276,36 @@ public String print(JSDocInfo info) { parts.add("@closurePrimitive {" + info.getClosurePrimitiveId() + "}"); } - parts.add("*/"); + if (printDesc && info.getBlockDescription() != null) { + String cleaned = info.getBlockDescription().replaceAll("\n\\s*\\*\\s*", "\n"); + if (!cleaned.isEmpty()) { + multiline = true; + cleaned = cleaned.trim(); + if (parts.size() > 1) { + // If there is more than one part - the opening "/**" - then add blank line between the + // description and everything else. + cleaned += '\n'; + } + parts.add(1, cleaned); + } + } StringBuilder sb = new StringBuilder(); if (multiline) { - Joiner.on("\n ").appendTo(sb, parts); + Joiner.on("\n").appendTo(sb, parts); } else { Joiner.on(" ").appendTo(sb, parts); + sb.append(" */"); } - sb.append((multiline) ? "\n" : " "); - return sb.toString(); + // Ensure all lines start with " *", and then ensure all non blank lines have a space after + // the *. + String s = sb.toString().replaceAll("\n", "\n *").replaceAll("\n \\*([^ \n])", "\n * $1"); + if (multiline) { + s += "\n */\n"; + } else { + s += " "; + } + return s; } private Node stripBang(Node typeNode) { @@ -282,22 +316,45 @@ private Node stripBang(Node typeNode) { } private String buildAnnotationWithType(String annotation, JSTypeExpression type) { - return buildAnnotationWithType(annotation, type.getRoot()); + return buildAnnotationWithType(annotation, type, null); + } + + private String buildAnnotationWithType( + String annotation, JSTypeExpression type, String description) { + return buildAnnotationWithType(annotation, type.getRoot(), description); } private String buildAnnotationWithType(String annotation, Node type) { + return buildAnnotationWithType(annotation, type, null); + } + + private String buildAnnotationWithType(String annotation, Node type, String description) { StringBuilder sb = new StringBuilder(); sb.append("@"); sb.append(annotation); sb.append(" {"); appendTypeNode(sb, type); sb.append("}"); + if (description != null) { + sb.append(" "); + sb.append(description); + } return sb.toString(); } - private String buildParamType(String name, JSTypeExpression type) { + private String buildParamType(JSDocInfo info, String name) { + JSTypeExpression type = info.getParameterType(name); if (type != null) { - return "{" + typeNode(type.getRoot()) + "} " + name; + String p = + "{" + + typeNode(type.getRoot()) + + "} " + + name + + (printDesc && info.getDescriptionForParameter(name) != null + // Don't add a leading space; the parser retained it. + ? info.getDescriptionForParameter(name) + : ""); + return p.trim(); } else { return name; } diff --git a/test/com/google/javascript/jscomp/CodePrinterTest.java b/test/com/google/javascript/jscomp/CodePrinterTest.java index 83508ded32d..1ab53f7170d 100644 --- a/test/com/google/javascript/jscomp/CodePrinterTest.java +++ b/test/com/google/javascript/jscomp/CodePrinterTest.java @@ -1790,7 +1790,7 @@ public void testReturnWithTypeAnnotation() { lines( "function f() {", " return (/**", - " @return {number}", + " * @return {number}", " */", " function() {", " return 42;", @@ -1804,8 +1804,8 @@ public void testDeprecatedAnnotationIncludesNewline() { String js = LINE_JOINER.join( "/**", - " @type {number}", - " @deprecated See {@link replacementClass} for more details.", + " * @type {number}", + " * @deprecated See {@link replacementClass} for more details.", " */", "var x;", ""); @@ -2651,9 +2651,9 @@ public void testPreserveTypeAnnotations2() { assertPrintSame( LINE_JOINER.join( - "/**", - " @const", - " @suppress {const,duplicate}", + "/**", // + " * @const", + " * @suppress {const,duplicate}", " */", "var ns={}")); } @@ -3336,25 +3336,26 @@ public void testGoogScope() { + " alert(Quux.someProperty);\n" + "}\n" + "}); // goog.scope\n"; - String expectedCode = "" - + "/** @const */ var $jscomp = $jscomp || {};\n" - + "/** @const */ $jscomp.scope = {};\n" - + "/** @const */ var foo = {};\n" - + "/** @const */ foo.bar = {};\n" - + "goog.provide('foo.bar');\n" - + "goog.require('baz.qux.Quux');\n" - + "goog.require('foo.ScopedType');\n" - + "/**\n" - + " @param {ScopedType} obj\n" - + " */\n" - + "var fn = /**\n" - + " @param {ScopedType} obj\n" - + " */\n" - + "function(obj) {\n" - + " alert(STR);\n" - + " alert(Quux.someProperty);\n" - + "};\n" - + "var STR = '3';\n"; + String expectedCode = + "" + + "/** @const */ var $jscomp = $jscomp || {};\n" + + "/** @const */ $jscomp.scope = {};\n" + + "/** @const */ var foo = {};\n" + + "/** @const */ foo.bar = {};\n" + + "goog.provide('foo.bar');\n" + + "goog.require('baz.qux.Quux');\n" + + "goog.require('foo.ScopedType');\n" + + "/**\n" + + " * @param {ScopedType} obj\n" + + " */\n" + + "var fn = /**\n" + + " * @param {ScopedType} obj\n" + + " */\n" + + "function(obj) {\n" + + " alert(STR);\n" + + " alert(Quux.someProperty);\n" + + "};\n" + + "var STR = '3';\n"; CompilerOptions compilerOptions = new CompilerOptions(); compilerOptions.setClosurePass(true); diff --git a/test/com/google/javascript/jscomp/ExternExportsPassTest.java b/test/com/google/javascript/jscomp/ExternExportsPassTest.java index 36b9e837272..f221f5d3801 100644 --- a/test/com/google/javascript/jscomp/ExternExportsPassTest.java +++ b/test/com/google/javascript/jscomp/ExternExportsPassTest.java @@ -1692,13 +1692,13 @@ public void testNamespaceDefinitionInExterns() { "goog.exportSymbol('ns.subns.Foo', ns.subns.Foo);"), lines( "/**", - " @const", - " @suppress {const,duplicate}", + " * @const", + " * @suppress {const,duplicate}", " */", "var ns = {};", "/**", - " @const", - " @suppress {const,duplicate}", + " * @const", + " * @suppress {const,duplicate}", " */", "ns.subns = {};", "/**", diff --git a/test/com/google/javascript/jscomp/JSDocInfoPrinterTest.java b/test/com/google/javascript/jscomp/JSDocInfoPrinterTest.java index 1f79eb077c1..b5fdd87847d 100644 --- a/test/com/google/javascript/jscomp/JSDocInfoPrinterTest.java +++ b/test/com/google/javascript/jscomp/JSDocInfoPrinterTest.java @@ -44,7 +44,7 @@ public final class JSDocInfoPrinterTest { @Before public void setUp() { builder = new JSDocInfoBuilder(true); - jsDocInfoPrinter = new JSDocInfoPrinter(false); + jsDocInfoPrinter = new JSDocInfoPrinter(/* useOriginalName= */ false, /* printDesc= */ true); } @Test @@ -58,7 +58,7 @@ public void testBasic() { builder.recordSuppressions(ImmutableSet.of("globalThis", "uselessCode")); info = builder.buildAndReset(); assertThat(jsDocInfoPrinter.print(info)) - .isEqualTo("/**\n @suppress {globalThis,uselessCode}\n */\n"); + .isEqualTo("/**\n * @suppress {globalThis,uselessCode}\n */\n"); } @Test @@ -81,7 +81,21 @@ public void testFinal() { public void testDescTag() { builder.recordDescription("foo"); JSDocInfo info = builder.buildAndReset(); - assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/** @desc foo\n */ "); + assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/** @desc foo */ "); + } + + @Test + public void testMultilineDescTag() { + builder.recordDescription("foo\nbar"); + JSDocInfo info = builder.buildAndReset(); + assertThat(jsDocInfoPrinter.print(info)) + .isEqualTo( + LINE_JOINER.join( + "/**", // + " * @desc foo", + " * bar", + " */", + "")); } @Test @@ -96,7 +110,7 @@ public void testTemplate() { builder.recordTemplateTypeName("T"); builder.recordTemplateTypeName("U"); JSDocInfo info = builder.buildAndReset(); - assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n @template T,U\n */\n"); + assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n * @template T, U\n */\n"); } @Test @@ -104,7 +118,7 @@ public void testTypeTransformationLanguageTemplate() { builder.recordTypeTransformation("T", IR.string("Promise")); JSDocInfo info = builder.buildAndReset(); assertThat(jsDocInfoPrinter.print(info)) - .isEqualTo("/**\n @template T := \"Promise\" =:\n */\n"); + .isEqualTo("/**\n * @template T := \"Promise\" =:\n */\n"); } @Test @@ -115,26 +129,26 @@ public void testParam() { new JSTypeExpression(JsDocInfoParser.parseTypeString("string"), "")); JSDocInfo info = builder.buildAndReset(); assertThat(jsDocInfoPrinter.print(info)) - .isEqualTo("/**\n @param {number} foo\n @param {string} bar\n */\n"); + .isEqualTo("/**\n * @param {number} foo\n * @param {string} bar\n */\n"); builder.recordParameter("foo", new JSTypeExpression(new Node(Token.EQUALS, IR.string("number")), "")); info = builder.buildAndReset(); - assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n @param {number=} foo\n */\n"); + assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n * @param {number=} foo\n */\n"); builder.recordParameter("foo", new JSTypeExpression(new Node(Token.ELLIPSIS, IR.string("number")), "")); info = builder.buildAndReset(); - assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n @param {...number} foo\n */\n"); + assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n * @param {...number} foo\n */\n"); builder.recordParameter("foo", new JSTypeExpression(new Node(Token.ELLIPSIS, IR.empty()), "")); info = builder.buildAndReset(); - assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n @param {...} foo\n */\n"); + assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n * @param {...} foo\n */\n"); builder.recordParameter("foo", null); info = builder.buildAndReset(); - assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n @param foo\n */\n"); + assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n * @param foo\n */\n"); } @Test @@ -178,12 +192,12 @@ public void testTypes() { builder.recordReturnType( new JSTypeExpression(JsDocInfoParser.parseTypeString("number|string"), "")); JSDocInfo info = builder.buildAndReset(); - assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n @return {(number|string)}\n */\n"); + assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n * @return {(number|string)}\n */\n"); builder.recordParameter("foo", new JSTypeExpression(new Node(Token.ELLIPSIS, IR.string("number")), "")); info = builder.buildAndReset(); - assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n @param {...number} foo\n */\n"); + assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n * @param {...number} foo\n */\n"); builder.recordThrowType(new JSTypeExpression(new Node(Token.STAR), "")); info = builder.buildAndReset(); assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/** @throws {*} */ "); @@ -248,17 +262,17 @@ public void testInheritance() { builder.recordImplementedInterface( new JSTypeExpression(JsDocInfoParser.parseTypeString("Foo"), "")); JSDocInfo info = builder.buildAndReset(); - assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n @implements {Foo}\n */\n"); + assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n * @implements {Foo}\n */\n"); builder.recordImplementedInterface( new JSTypeExpression(JsDocInfoParser.parseTypeString("!Foo"), "")); info = builder.buildAndReset(); - assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n @implements {Foo}\n */\n"); + assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n * @implements {Foo}\n */\n"); builder.recordBaseType( new JSTypeExpression(JsDocInfoParser.parseTypeString("Foo"), "")); info = builder.buildAndReset(); - assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n @extends {Foo}\n */\n"); + assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n * @extends {Foo}\n */\n"); builder.recordBaseType( new JSTypeExpression(JsDocInfoParser.parseTypeString("!Foo"), "")); @@ -268,7 +282,7 @@ public void testInheritance() { new JSTypeExpression(JsDocInfoParser.parseTypeString("Bar.Baz"), "")); info = builder.buildAndReset(); assertThat(jsDocInfoPrinter.print(info)) - .isEqualTo("/**\n @extends {Foo}\n @implements {Bar}\n @implements {Bar.Baz}\n */\n"); + .isEqualTo("/**\n * @extends {Foo}\n * @implements {Bar}\n * @implements {Bar.Baz}\n */\n"); } @Test @@ -280,7 +294,7 @@ public void testInterfaceInheritance() { new JSTypeExpression(JsDocInfoParser.parseTypeString("Bar"), "")); JSDocInfo info = builder.buildAndReset(); assertThat(jsDocInfoPrinter.print(info)) - .isEqualTo("/**\n @interface\n @extends {Foo}\n @extends {Bar}\n */\n"); + .isEqualTo("/**\n * @interface\n * @extends {Foo}\n * @extends {Bar}\n */\n"); } @Test @@ -353,6 +367,71 @@ public void testConstDefines() { assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/** @define {string} */ "); } + @Test + public void testBlockDescription() { + builder.recordBlockDescription("Description of the thing"); + JSDocInfo info = builder.buildAndReset(); + assertThat(jsDocInfoPrinter.print(info)).isEqualTo("/**\n * Description of the thing\n */\n"); + } + + @Test + public void testParamDescriptions() { + builder.recordParameter( + "foo", new JSTypeExpression(JsDocInfoParser.parseTypeString("number"), "")); + builder.recordParameter( + "bar", new JSTypeExpression(JsDocInfoParser.parseTypeString("string"), "")); + // The parser will retain leading whitespace for descriptions. + builder.recordParameterDescription("foo", " A number for foo"); + builder.recordParameterDescription("bar", " A multline\n description for bar"); + JSDocInfo info = builder.buildAndReset(); + assertThat(jsDocInfoPrinter.print(info)) + .isEqualTo( + LINE_JOINER.join( + "/**", + " * @param {number} foo A number for foo", + " * @param {string} bar A multline", + " * description for bar", + " */", + "")); + } + + @Test + public void testReturnDescription() { + builder.recordReturnType( + new JSTypeExpression(JsDocInfoParser.parseTypeString("boolean"), "")); + builder.recordReturnDescription("The return value"); + JSDocInfo info = builder.buildAndReset(); + assertThat(jsDocInfoPrinter.print(info)) + .isEqualTo(LINE_JOINER.join("/**", " * @return {boolean} The return value", " */", "")); + } + + @Test + public void testAllDescriptions() { + builder.recordBlockDescription("Description of the thing"); + builder.recordParameter( + "foo", new JSTypeExpression(JsDocInfoParser.parseTypeString("number"), "")); + builder.recordParameter( + "bar", new JSTypeExpression(JsDocInfoParser.parseTypeString("string"), "")); + builder.recordParameterDescription("foo", " A number for foo"); + builder.recordParameterDescription("bar", " A multline\n description for bar"); + builder.recordReturnType( + new JSTypeExpression(JsDocInfoParser.parseTypeString("boolean"), "")); + builder.recordReturnDescription("The return value"); + JSDocInfo info = builder.buildAndReset(); + assertThat(jsDocInfoPrinter.print(info)) + .isEqualTo( + LINE_JOINER.join( + "/**", + " * Description of the thing", + " *", + " * @param {number} foo A number for foo", + " * @param {string} bar A multline", + " * description for bar", + " * @return {boolean} The return value", + " */", + "")); + } + @Test public void testDeprecated() { builder.recordDeprecated(); @@ -364,8 +443,8 @@ public void testDeprecated() { .isEqualTo( LINE_JOINER.join( "/**", - " @type {string}", - " @deprecated See {@link otherClass} for more info.", + " * @type {string}", + " * @deprecated See {@link otherClass} for more info.", " */", "")); }