Fix closure compiler type casts

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.
+
+  <!-- prettier-ignore -->
+  ```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);
+  ```

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;
   }
 

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.<string>} */ (['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.<string>} */ (["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"]

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.<string>} */ (['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.<string>} */ (["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
+});
+
+================================================================================
+`;

tests/comments/closure-compiler-type-cast.js → tests/comments_closure_typecast/closure-compiler-type-cast.js

@@ -16,10 +16,21 @@ function returnValue() {
   return /** @type {!Array.<string>} */ (['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,

tests/comments_closure_typecast/jsfmt.spec.js

@@ -0,0 +1 @@
+run_spec(__dirname, ["babel"]);