Skip to content

Commit

Permalink
Remove support for Flow comment types
Browse files Browse the repository at this point in the history
  • Loading branch information
thorn0 committed Oct 20, 2022
1 parent c5cda4d commit 17a1d3a
Show file tree
Hide file tree
Showing 14 changed files with 56 additions and 187 deletions.
27 changes: 27 additions & 0 deletions changelog_unreleased/flow/13687.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#### Remove support for Flow comments (#13687 by @thorn0)

Being a kind of preprocessor, [comment types](https://flow.org/en/docs/types/comments/) are processed on the token level and can't be represented in an AST in the general case. Flow builds the AST as if these special comment tokens didn't exist. Example:

<!-- prettier-ignore -->
```js
/*:: if */ (x) + y;
```

This is parsed as `if (x) +y;` by Flow and as `x + y;` by JS parsers that don't support Flow.

Previously, Prettier had a limited support for this syntax in some specific cases. Prettier tried to detect that this syntax was used and to preserve it. As an attempt to solve an unsolvable problem, this support was fragile and riddled with bugs, so it has been removed. Now if the `parser` option is set to `flow` or `babel-flow`, the special comments will be parsed and reprinted like normal code. If a parser that doesn't support Flow is used, they will be treated like usual comments.

<!-- prettier-ignore -->
```js
// Input
let a /*: foo */ = b;

// Prettier stable
let a /*: foo */ = b;

// Prettier main with --parser flow
let a: foo = b;

// Prettier main with --parser babel
let a /*: foo */ = b;
```
14 changes: 1 addition & 13 deletions src/language-js/comments/printer-methods.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
import isNonEmptyArray from "../../utils/is-non-empty-array.js";
import { hasJsxIgnoreComment } from "../print/jsx.js";
import {
CommentCheckFlags,
getComments,
getFunctionParameters,
hasFlowAnnotationComment,
hasFlowShorthandAnnotationComment,
hasNodeIgnoreComment,
isCallExpression,
isJsxNode,
isLineComment,
} from "../utils/index.js";
Expand Down Expand Up @@ -71,15 +66,8 @@ function hasPrettierIgnore(path) {
* @returns {boolean}
*/
function willPrintOwnComments({ node, parent }) {
const hasFlowAnnotations = (node) =>
hasFlowAnnotationComment(getComments(node, CommentCheckFlags.Leading)) ||
hasFlowAnnotationComment(getComments(node, CommentCheckFlags.Trailing));

return (
((node &&
(isJsxNode(node) ||
hasFlowShorthandAnnotationComment(node) ||
(isCallExpression(parent) && hasFlowAnnotations(node)))) ||
((node && isJsxNode(node)) ||
(parent &&
(parent.type === "JSXSpreadAttribute" ||
parent.type === "JSXSpreadChild" ||
Expand Down
11 changes: 0 additions & 11 deletions src/language-js/needs-parens.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import isNonEmptyArray from "../utils/is-non-empty-array.js";
import {
getFunctionParameters,
getLeftSidePathName,
hasFlowShorthandAnnotationComment,
hasNakedLeftSide,
hasNode,
isBitwiseOperator,
Expand Down Expand Up @@ -36,16 +35,6 @@ function needsParens(path, options) {
return false;
}

if (
// Preserve parens if we have a Flow annotation comment, unless we're using the Flow
// parser. The Flow parser turns Flow comments into type annotation nodes in its
// AST, which we handle separately.
options.parser !== "flow" &&
hasFlowShorthandAnnotationComment(node)
) {
return true;
}

// Identifiers never need parentheses.
if (node.type === "Identifier") {
// ...unless those identifiers are embed placeholders. They might be substituted by complex
Expand Down
2 changes: 1 addition & 1 deletion src/language-js/parse/babel.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ function createParse(parseMethod, ...optionsCombinations) {
const parse = createParse("parse", appendPlugins(["jsx", "flow"]));
const parseFlow = createParse(
"parse",
appendPlugins(["jsx", ["flow", { all: true, enums: true }]])
appendPlugins(["jsx", ["flow", { all: true, enums: true }], "flowComments"])
);
const parseTypeScript = createParse(
"parse",
Expand Down
19 changes: 0 additions & 19 deletions src/language-js/print/call-expression.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { join, group } from "../../document/builders.js";
import pathNeedsParens from "../needs-parens.js";
import {
getCallArguments,
hasFlowAnnotationComment,
isCallExpression,
isMemberish,
isStringLiteral,
Expand Down Expand Up @@ -56,21 +55,6 @@ function printCallExpression(path, options, print) {
}
}

// Inline Flow annotation comments following Identifiers in Call nodes need to
// stay with the Identifier. For example:
//
// foo /*:: <SomeGeneric> */(bar);
//
// Here, we ensure that such comments stay between the Identifier and the Callee.
const isIdentifierWithFlowAnnotation =
(options.parser === "babel" || options.parser === "babel-flow") &&
node.callee &&
node.callee.type === "Identifier" &&
hasFlowAnnotationComment(node.callee.trailingComments);
if (isIdentifierWithFlowAnnotation) {
node.callee.trailingComments[0].printed = true;
}

// We detect calls on member lookups and possibly print them in a
// special chain format. See `printMemberChain` for more info.
if (
Expand All @@ -86,9 +70,6 @@ function printCallExpression(path, options, print) {
isNew ? "new " : "",
isDynamicImport ? "import" : print("callee"),
optional,
isIdentifierWithFlowAnnotation
? `/*:: ${node.callee.trailingComments[0].value.slice(2).trim()} */`
: "",
printFunctionTypeParameters(path, options, print),
printCallArguments(path, options, print),
];
Expand Down
9 changes: 1 addition & 8 deletions src/language-js/print/comment.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,7 @@ function printComment(commentPath, options) {
return printIndentableBlockComment(comment);
}

const commentEnd = locEnd(comment);
const isInsideFlowComment =
options.originalText.slice(commentEnd - 3, commentEnd) === "*-/";
return [
"/*",
replaceEndOfLine(comment.value),
isInsideFlowComment ? "*-/" : "*/",
];
return ["/*", replaceEndOfLine(comment.value), "*/"];
}

/* istanbul ignore next */
Expand Down
30 changes: 2 additions & 28 deletions src/language-js/print/flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
rawText,
shouldPrintComma,
} from "../utils/index.js";
import { locStart, locEnd } from "../loc.js";
import { printClass } from "./class.js";
import {
printOpaqueType,
Expand Down Expand Up @@ -287,36 +286,11 @@ function printFlow(path, options, print) {
];

case "TypeParameterDeclaration":
case "TypeParameterInstantiation": {
const printed = printTypeParameters(path, options, print, "params");

if (options.parser === "flow") {
const start = locStart(node);
const end = locEnd(node);
const commentStartIndex = options.originalText.lastIndexOf("/*", start);
const commentEndIndex = options.originalText.indexOf("*/", end);
if (commentStartIndex !== -1 && commentEndIndex !== -1) {
const comment = options.originalText
.slice(commentStartIndex + 2, commentEndIndex)
.trim();
if (
comment.startsWith("::") &&
!comment.includes("/*") &&
!comment.includes("*/")
) {
return ["/*:: ", printed, " */"];
}
}
}

return printed;
}
case "TypeParameterInstantiation":
return printTypeParameters(path, options, print, "params");

case "InferredPredicate":
return "%checks";
// Unhandled types below. If encountered, nodes of these types should
// be either left alone or desugared into AST types that are fully
// supported by the pretty-printer.
case "DeclaredPredicate":
return ["%checks(", print("value"), ")"];
case "AnyTypeAnnotation":
Expand Down
16 changes: 4 additions & 12 deletions src/language-js/print/function.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import { ArgExpansionBailout } from "../../common/errors.js";
import {
getFunctionParameters,
hasLeadingOwnLineComment,
isFlowAnnotationComment,
isJsxNode,
isTemplateOnItsOwnLine,
shouldPrintComma,
Expand Down Expand Up @@ -95,7 +94,7 @@ function printFunction(path, print, options, args) {
options,
expandArg
);
const returnTypeDoc = printReturnType(path, print, options);
const returnTypeDoc = printReturnType(path, print);
const shouldGroupParameters = shouldGroupFunctionParameters(
node,
returnTypeDoc
Expand Down Expand Up @@ -160,7 +159,7 @@ function printMethod(path, options, print) {
function printMethodInternal(path, options, print) {
const { node } = path;
const parametersDoc = printFunctionParameters(path, print, options);
const returnTypeDoc = printReturnType(path, print, options);
const returnTypeDoc = printReturnType(path, print);
const shouldGroupParameters = shouldGroupFunctionParameters(
node,
returnTypeDoc
Expand Down Expand Up @@ -194,7 +193,7 @@ function printArrowFunctionSignature(path, options, print, args) {
parts.push(print(["params", 0]));
} else {
const expandArg = args && (args.expandLastArg || args.expandFirstArg);
let returnTypeDoc = printReturnType(path, print, options);
let returnTypeDoc = printReturnType(path, print);
if (expandArg) {
if (willBreak(returnTypeDoc)) {
throw new ArgExpansionBailout();
Expand Down Expand Up @@ -423,17 +422,10 @@ function shouldPrintParamsWithoutParens(path, options) {
}

/** @returns {Doc} */
function printReturnType(path, print, options) {
function printReturnType(path, print) {
const { node } = path;
const returnType = print("returnType");

if (
node.returnType &&
isFlowAnnotationComment(options.originalText, node.returnType)
) {
return [" /*: ", returnType, " */"];
}

const parts = [returnType];

// prepend colon to TypeScript type annotation
Expand Down
5 changes: 0 additions & 5 deletions src/language-js/print/misc.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { isNonEmptyArray } from "../../common/util.js";
import { indent, join, line } from "../../document/builders.js";
import { isFlowAnnotationComment } from "../utils/index.js";

function printOptionalToken(path) {
const { node } = path;
Expand Down Expand Up @@ -54,10 +53,6 @@ function printTypeAnnotation(path, options, print) {
const isFunctionDeclarationIdentifier =
parentNode.type === "DeclareFunction" && parentNode.id === node;

if (isFlowAnnotationComment(options.originalText, node.typeAnnotation)) {
return [" /*: ", print("typeAnnotation"), " */"];
}

return [isFunctionDeclarationIdentifier ? "" : ": ", print("typeAnnotation")];
}

Expand Down
7 changes: 0 additions & 7 deletions src/language-js/printer-estree.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import * as commentsRelatedPrinterMethods from "./comments/printer-methods.js";
import pathNeedsParens from "./needs-parens.js";
import preprocess from "./print-preprocess.js";
import {
hasFlowShorthandAnnotationComment,
hasComment,
CommentCheckFlags,
isTheOnlyJsxElementInMarkdown,
Expand Down Expand Up @@ -154,12 +153,6 @@ function genericPrint(path, options, print, args) {
parts.unshift(";");
}

if (hasFlowShorthandAnnotationComment(node)) {
const [comment] = node.trailingComments;
parts.push(" /*", comment.value.trimStart(), "*/");
comment.printed = true;
}

if (isClassExpressionWithDecorators) {
parts.push(line);
}
Expand Down
60 changes: 0 additions & 60 deletions src/language-js/utils/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import esutils from "esutils";
import {
hasNewline,
skipWhitespace,
isNonEmptyArray,
isNextLineEmptyAfterIndex,
getStringWidth,
Expand Down Expand Up @@ -30,47 +29,6 @@ const isIdentifierName = esutils.keyword.isIdentifierNameES5;
* @typedef {import("../../common/ast-path.js").default} AstPath
*/

// We match any whitespace except line terminators because
// Flow annotation comments cannot be split across lines. For example:
//
// (this /*
// : any */).foo = 5;
//
// is not picked up by Flow (see https://github.com/facebook/flow/issues/7050), so
// removing the newline would create a type annotation that the user did not intend
// to create.
const NON_LINE_TERMINATING_WHITE_SPACE = "(?:(?=.)\\s)";
const FLOW_SHORTHAND_ANNOTATION = new RegExp(
`^${NON_LINE_TERMINATING_WHITE_SPACE}*:`
);
const FLOW_ANNOTATION = new RegExp(`^${NON_LINE_TERMINATING_WHITE_SPACE}*::`);

/**
* @param {Node} node
* @returns {boolean}
*/
function hasFlowShorthandAnnotationComment(node) {
// https://flow.org/en/docs/types/comments/
// Syntax example: const r = new (window.Request /*: Class<Request> */)("");

return (
node.extra?.parenthesized &&
isBlockComment(node.trailingComments?.[0]) &&
FLOW_SHORTHAND_ANNOTATION.test(node.trailingComments[0].value)
);
}

/**
* @param {Comment[]} comments
* @returns {boolean}
*/
function hasFlowAnnotationComment(comments) {
const firstComment = comments?.[0];
return (
isBlockComment(firstComment) && FLOW_ANNOTATION.test(firstComment.value)
);
}

/**
* @param {Node} node
* @param {(Node) => boolean} fn
Expand Down Expand Up @@ -619,21 +577,6 @@ function getTypeScriptMappedTypeModifier(tokenNode, keyword) {
return keyword;
}

/**
* @param {string} text
* @param {Node} typeAnnotation
* @returns {boolean}
*/
function isFlowAnnotationComment(text, typeAnnotation) {
const start = locStart(typeAnnotation);
const end = skipWhitespace(text, locEnd(typeAnnotation));
return (
end !== false &&
text.slice(start, start + 2) === "/*" &&
text.slice(end, end + 2) === "*/"
);
}

/**
* @param {string} text
* @param {Node} node
Expand Down Expand Up @@ -1288,8 +1231,6 @@ export {
getLeftSidePathName,
getParentExportDeclaration,
getTypeScriptMappedTypeModifier,
hasFlowAnnotationComment,
hasFlowShorthandAnnotationComment,
hasLeadingOwnLineComment,
hasNakedLeftSide,
hasNode,
Expand All @@ -1303,7 +1244,6 @@ export {
isCallExpression,
isMemberExpression,
isExportDeclaration,
isFlowAnnotationComment,
isFunctionCompositionArgs,
isFunctionNotation,
isFunctionOrArrowExpression,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type Children = Array<Child>;
class Thunk {}
class VirtualNode {
children: Child | Children;
constructor(type, children /*: Children */) {
constructor(type, children: Children) {
this.children = children.length === 1 ? children[0] : children;
}
}
Expand Down

0 comments on commit 17a1d3a

Please sign in to comment.