diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index 7b5a8f5be608..a89a80824180 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -41,3 +41,22 @@ Examples: ``` --> + +- JavaScript: Fix closure compiler typecasts ([#5947] by [@jridgewell]) + + If a closing parenthesis follows after a typecast in an inner expression, the typecast would wrap everything to the that following parenthesis. + + + ```js + // Input + test(/** @type {!Array} */(arrOrString).length); + test(/** @type {!Array} */((arrOrString)).length + 1); + + // Output (Prettier stable) + test(/** @type {!Array} */ (arrOrString.length)); + test(/** @type {!Array} */ (arrOrString.length + 1)); + + // Output (Prettier master) + test(/** @type {!Array} */ (arrOrString).length); + test(/** @type {!Array} */ (arrOrString).length + 1); + ``` diff --git a/src/language-js/needs-parens.js b/src/language-js/needs-parens.js index 4ad67228afa9..51135fcae3e5 100644 --- a/src/language-js/needs-parens.js +++ b/src/language-js/needs-parens.js @@ -6,23 +6,21 @@ const util = require("../common/util"); const comments = require("./comments"); const { hasFlowShorthandAnnotationComment } = require("./utils"); -function hasClosureCompilerTypeCastComment(text, path, locStart, locEnd) { +function hasClosureCompilerTypeCastComment(text, path) { // https://github.com/google/closure-compiler/wiki/Annotating-Types#type-casts // Syntax example: var x = /** @type {string} */ (fruit); const n = path.getValue(); return ( - util.getNextNonSpaceNonCommentCharacter(text, n, locEnd) === ")" && + isParenthesized(n) && (hasTypeCastComment(n) || hasAncestorTypeCastComment(0)) ); // for sub-item: /** @type {array} */ (numberOrString).map(x => x); function hasAncestorTypeCastComment(index) { const ancestor = path.getParentNode(index); - return ancestor && - util.getNextNonSpaceNonCommentCharacter(text, ancestor, locEnd) !== ")" && - /^[\s(]*$/.test(text.slice(locStart(ancestor), locStart(n))) + return ancestor && !isParenthesized(ancestor) ? hasTypeCastComment(ancestor) || hasAncestorTypeCastComment(index + 1) : false; } @@ -34,12 +32,18 @@ function hasClosureCompilerTypeCastComment(text, path, locStart, locEnd) { comment => comment.leading && comments.isBlockComment(comment) && - isTypeCastComment(comment.value) && - util.getNextNonSpaceNonCommentCharacter(text, comment, locEnd) === "(" + isTypeCastComment(comment.value) ) ); } + function isParenthesized(node) { + // Closure typecast comments only really make sense when _not_ using + // typescript or flow parsers, so we take advantage of the babel parser's + // parenthesized expressions. + return node.extra && node.extra.parenthesized; + } + function isTypeCastComment(comment) { const trimmed = comment.trim(); if (!/^\*\s*@type\s*\{[^]+\}$/.test(trimmed)) { @@ -100,14 +104,7 @@ function needsParens(path, options) { // Closure compiler requires that type casted expressions to be surrounded by // parentheses. - if ( - hasClosureCompilerTypeCastComment( - options.originalText, - path, - options.locStart, - options.locEnd - ) - ) { + if (hasClosureCompilerTypeCastComment(options.originalText, path)) { return true; } diff --git a/tests/comments/__snapshots__/jsfmt.spec.js.snap b/tests/comments/__snapshots__/jsfmt.spec.js.snap index 9198e1dc3efc..1cc4bce165fe 100644 --- a/tests/comments/__snapshots__/jsfmt.spec.js.snap +++ b/tests/comments/__snapshots__/jsfmt.spec.js.snap @@ -323,114 +323,6 @@ React.render( ================================================================================ `; -exports[`closure-compiler-type-cast.js 1`] = ` -====================================options===================================== -parsers: ["flow", "babel"] -printWidth: 80 - | printWidth -=====================================input====================================== -// test to make sure comments are attached correctly -let inlineComment = /* some comment */ ( - someReallyLongFunctionCall(withLots, ofArguments)); - -let object = { - key: /* some comment */ (someReallyLongFunctionCall(withLots, ofArguments)) -}; - -// preserve parens only for type casts -let assignment = /** @type {string} */ (getValue()); -let value = /** @type {string} */ (this.members[0]).functionCall(); - -functionCall(1 + /** @type {string} */ (value), /** @type {!Foo} */ ({})); - -function returnValue() { - return /** @type {!Array.} */ (['hello', 'you']); -} - -var newArray = /** @type {array} */ (numberOrString).map(x => x); -var newArray = /** @type {array} */ ((numberOrString)).map(x => x); -var newArray = /** @type {array} */ ((numberOrString).map(x => x)); - - -const data = functionCall( - arg1, - arg2, - /** @type {{height: number, width: number}} */ (arg3)); - -// Invalid type casts -const v = /** @type {} */ (value); -const v = /** @type {}} */ (value); -const v = /** @type } */ (value); -const v = /** @type { */ (value); -const v = /** @type {{} */ (value); - -const style = /** @type {{ - width: number, - height: number, - marginTop: number, - marginLeft: number, - marginRight: number, - marginBottom: number, -}} */ ({ - width, - height, - ...margins, -}); - -=====================================output===================================== -// test to make sure comments are attached correctly -let inlineComment = /* some comment */ someReallyLongFunctionCall( - withLots, - ofArguments -); - -let object = { - key: /* some comment */ someReallyLongFunctionCall(withLots, ofArguments) -}; - -// preserve parens only for type casts -let assignment = /** @type {string} */ (getValue()); -let value = /** @type {string} */ (this.members[0]).functionCall(); - -functionCall(1 + /** @type {string} */ (value), /** @type {!Foo} */ ({})); - -function returnValue() { - return /** @type {!Array.} */ (["hello", "you"]); -} - -var newArray = /** @type {array} */ (numberOrString).map(x => x); -var newArray = /** @type {array} */ (numberOrString).map(x => x); -var newArray = /** @type {array} */ (numberOrString.map(x => x)); - -const data = functionCall( - arg1, - arg2, - /** @type {{height: number, width: number}} */ (arg3) -); - -// Invalid type casts -const v = /** @type {} */ value; -const v = /** @type {}} */ value; -const v = /** @type } */ value; -const v = /** @type { */ value; -const v = /** @type {{} */ value; - -const style = /** @type {{ - width: number, - height: number, - marginTop: number, - marginLeft: number, - marginRight: number, - marginBottom: number, -}} */ ({ - width, - height, - ...margins -}); - -================================================================================ -`; - exports[`dangling.js 1`] = ` ====================================options===================================== parsers: ["flow", "babel"] diff --git a/tests/comments_closure_typecast/__snapshots__/jsfmt.spec.js.snap b/tests/comments_closure_typecast/__snapshots__/jsfmt.spec.js.snap new file mode 100644 index 000000000000..a2549b7b2306 --- /dev/null +++ b/tests/comments_closure_typecast/__snapshots__/jsfmt.spec.js.snap @@ -0,0 +1,132 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`closure-compiler-type-cast.js 1`] = ` +====================================options===================================== +parsers: ["babel"] +printWidth: 80 + | printWidth +=====================================input====================================== +// test to make sure comments are attached correctly +let inlineComment = /* some comment */ ( + someReallyLongFunctionCall(withLots, ofArguments)); + +let object = { + key: /* some comment */ (someReallyLongFunctionCall(withLots, ofArguments)) +}; + +// preserve parens only for type casts +let assignment = /** @type {string} */ (getValue()); +let value = /** @type {string} */ (this.members[0]).functionCall(); + +functionCall(1 + /** @type {string} */ (value), /** @type {!Foo} */ ({})); + +function returnValue() { + return /** @type {!Array.} */ (['hello', 'you']); +} + +// Only numberOrString is typecast +var newArray = /** @type {array} */ (numberOrString).map(x => x); +var newArray = /** @type {array} */ ((numberOrString)).map(x => x); +var newArray = test(/** @type {array} */ (numberOrString).map(x => x)); +var newArray = test(/** @type {array} */ ((numberOrString)).map(x => x)); + +// The numberOrString.map CallExpression is typecast +var newArray = /** @type {array} */ (numberOrString.map(x => x)); +var newArray = /** @type {array} */ ((numberOrString).map(x => x)); +var newArray = test(/** @type {array} */ (numberOrString.map(x => x))); +var newArray = test(/** @type {array} */ ((numberOrString).map(x => x))); + +test(/** @type {number} */(num) + 1); +test(/** @type {!Array} */(arrOrString).length + 1); +test(/** @type {!Array} */((arrOrString)).length + 1); + +const data = functionCall( + arg1, + arg2, + /** @type {{height: number, width: number}} */ (arg3)); + +// Invalid type casts +const v = /** @type {} */ (value); +const v = /** @type {}} */ (value); +const v = /** @type } */ (value); +const v = /** @type { */ (value); +const v = /** @type {{} */ (value); + +const style = /** @type {{ + width: number, + height: number, + marginTop: number, + marginLeft: number, + marginRight: number, + marginBottom: number, +}} */ ({ + width, + height, + ...margins, +}); + +=====================================output===================================== +// test to make sure comments are attached correctly +let inlineComment = /* some comment */ someReallyLongFunctionCall( + withLots, + ofArguments +); + +let object = { + key: /* some comment */ someReallyLongFunctionCall(withLots, ofArguments) +}; + +// preserve parens only for type casts +let assignment = /** @type {string} */ (getValue()); +let value = /** @type {string} */ (this.members[0]).functionCall(); + +functionCall(1 + /** @type {string} */ (value), /** @type {!Foo} */ ({})); + +function returnValue() { + return /** @type {!Array.} */ (["hello", "you"]); +} + +// Only numberOrString is typecast +var newArray = /** @type {array} */ (numberOrString).map(x => x); +var newArray = /** @type {array} */ (numberOrString).map(x => x); +var newArray = test(/** @type {array} */ (numberOrString).map(x => x)); +var newArray = test(/** @type {array} */ (numberOrString).map(x => x)); + +// The numberOrString.map CallExpression is typecast +var newArray = /** @type {array} */ (numberOrString.map(x => x)); +var newArray = /** @type {array} */ (numberOrString.map(x => x)); +var newArray = test(/** @type {array} */ (numberOrString.map(x => x))); +var newArray = test(/** @type {array} */ (numberOrString.map(x => x))); + +test(/** @type {number} */ (num) + 1); +test(/** @type {!Array} */ (arrOrString).length + 1); +test(/** @type {!Array} */ (arrOrString).length + 1); + +const data = functionCall( + arg1, + arg2, + /** @type {{height: number, width: number}} */ (arg3) +); + +// Invalid type casts +const v = /** @type {} */ value; +const v = /** @type {}} */ value; +const v = /** @type } */ value; +const v = /** @type { */ value; +const v = /** @type {{} */ value; + +const style = /** @type {{ + width: number, + height: number, + marginTop: number, + marginLeft: number, + marginRight: number, + marginBottom: number, +}} */ ({ + width, + height, + ...margins +}); + +================================================================================ +`; diff --git a/tests/comments/closure-compiler-type-cast.js b/tests/comments_closure_typecast/closure-compiler-type-cast.js similarity index 68% rename from tests/comments/closure-compiler-type-cast.js rename to tests/comments_closure_typecast/closure-compiler-type-cast.js index 8fce7d6f21c7..52ed42dd0f0d 100644 --- a/tests/comments/closure-compiler-type-cast.js +++ b/tests/comments_closure_typecast/closure-compiler-type-cast.js @@ -16,10 +16,21 @@ function returnValue() { return /** @type {!Array.} */ (['hello', 'you']); } +// Only numberOrString is typecast var newArray = /** @type {array} */ (numberOrString).map(x => x); var newArray = /** @type {array} */ ((numberOrString)).map(x => x); +var newArray = test(/** @type {array} */ (numberOrString).map(x => x)); +var newArray = test(/** @type {array} */ ((numberOrString)).map(x => x)); + +// The numberOrString.map CallExpression is typecast +var newArray = /** @type {array} */ (numberOrString.map(x => x)); var newArray = /** @type {array} */ ((numberOrString).map(x => x)); +var newArray = test(/** @type {array} */ (numberOrString.map(x => x))); +var newArray = test(/** @type {array} */ ((numberOrString).map(x => x))); +test(/** @type {number} */(num) + 1); +test(/** @type {!Array} */(arrOrString).length + 1); +test(/** @type {!Array} */((arrOrString)).length + 1); const data = functionCall( arg1, diff --git a/tests/comments_closure_typecast/jsfmt.spec.js b/tests/comments_closure_typecast/jsfmt.spec.js new file mode 100644 index 000000000000..8382eddeb1db --- /dev/null +++ b/tests/comments_closure_typecast/jsfmt.spec.js @@ -0,0 +1 @@ +run_spec(__dirname, ["babel"]);