Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove support for Flow comment types #13687

Merged
merged 3 commits into from
Oct 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thorn0 This should be marked as [BREAKING]?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, let's do that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #13703


Being a kind of preprocessor, [Flow comments](https://flow.org/blog/2015/02/20/Flow-Comments/) AKA [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, for some special cases, Prettier tried to detect that this syntax was used and to preserve it. As an attempt to solve an unsolvable problem, this limited 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