Skip to content

Fix handling of colons on Flow function types #1089

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fix handling of colons on Flow function types #1089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d2e3181
Reformat lib/printer.ts with Prettier.
gnprice Apr 26, 2022
4cee7b2
Split up test/flow.ts cases in the Mocha metadata.
gnprice Apr 26, 2022
c1cf486
In Flow round-trip tests, compare text before re-parsing.
gnprice Apr 26, 2022
2e9d9f3
Fix expected vs. actual in test/flow.ts.
gnprice Apr 26, 2022
3cf2432
Straighten out handling of Flow function types.
gnprice Apr 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 25 additions & 52 deletions lib/printer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,15 +619,11 @@ function genericPrintNoParens(path: any, options: any, print: any) {
}

case "ImportAttribute":
return concat([
path.call(print, "key"),
": ",
path.call(print, "value"),
]);
return concat([path.call(print, "key"), ": ", path.call(print, "value")]);

case "StaticBlock":
parts.push("static ");
// Intentionally fall through to BlockStatement below.
// Intentionally fall through to BlockStatement below.

case "BlockStatement": {
const naked = path.call(
Expand Down Expand Up @@ -705,7 +701,7 @@ function genericPrintNoParens(path: any, options: any, print: any) {

case "RecordExpression":
parts.push("#");
// Intentionally fall through to printing the object literal...
// Intentionally fall through to printing the object literal...
case "ObjectExpression":
case "ObjectPattern":
case "ObjectTypeAnnotation": {
Expand Down Expand Up @@ -846,7 +842,7 @@ function genericPrintNoParens(path: any, options: any, print: any) {

case "TupleExpression":
parts.push("#");
// Intentionally fall through to printing the tuple elements...
// Intentionally fall through to printing the tuple elements...
case "ArrayExpression":
case "ArrayPattern": {
const elems: any[] = n.elements;
Expand Down Expand Up @@ -924,31 +920,20 @@ function genericPrintNoParens(path: any, options: any, print: any) {
);

case "BigIntLiteral": // Babel 7 Literal split
return fromString(
getPossibleRaw(n) || (n.value + "n"),
options,
);
return fromString(getPossibleRaw(n) || n.value + "n", options);

case "NumericLiteral": // Babel 6 Literal Split
return fromString(
getPossibleRaw(n) || n.value,
options,
);
return fromString(getPossibleRaw(n) || n.value, options);

case "DecimalLiteral":
return fromString(
getPossibleRaw(n) || (n.value + "m"),
options,
);
return fromString(getPossibleRaw(n) || n.value + "m", options);

case "BooleanLiteral": // Babel 6 Literal split
case "StringLiteral": // Babel 6 Literal split
case "Literal":
return fromString(
getPossibleRaw(n) || (
typeof n.value === "string"
? nodeStr(n.value, options)
: n.value),
getPossibleRaw(n) ||
(typeof n.value === "string" ? nodeStr(n.value, options) : n.value),
options,
);

Expand Down Expand Up @@ -1578,9 +1563,7 @@ function genericPrintNoParens(path: any, options: any, print: any) {
// transformed away before printing.
case "TypeAnnotation":
if (n.typeAnnotation) {
if (n.typeAnnotation.type !== "FunctionTypeAnnotation") {
parts.push(": ");
}
parts.push(": ");
parts.push(path.call(print, "typeAnnotation"));
return concat(parts);
}
Expand Down Expand Up @@ -1682,7 +1665,8 @@ function genericPrintNoParens(path: any, options: any, print: any) {
case "DeclareFunction":
return printFlowDeclaration(path, [
"function ",
path.call(print, "id"),
path.call(print, "id", "name"),
path.call(print, "id", "typeAnnotation", "typeAnnotation"),
";",
]);

Expand Down Expand Up @@ -1767,15 +1751,6 @@ function genericPrintNoParens(path: any, options: any, print: any) {
namedTypes.DeclareFunction.check(path.getParentNode(2))
);

const needsColon =
isArrowFunctionTypeAnnotation &&
!namedTypes.FunctionTypeParam.check(parent) &&
!namedTypes.TypeAlias.check(parent);

if (needsColon) {
parts.push(": ");
}

const hasTypeParameters = !!n.typeParameters;
const needsParens =
hasTypeParameters || n.params.length !== 1 || n.params[0].name;
Expand Down Expand Up @@ -1906,7 +1881,7 @@ function genericPrintNoParens(path: any, options: any, print: any) {
path.call(print, "id"),
"]]",
n.optional ? "?" : "",
n.value.type !== "FunctionTypeAnnotation" ? ": " : "",
!n.method ? ": " : "",
path.call(print, "value"),
]);

Expand Down Expand Up @@ -2895,7 +2870,7 @@ function maybePrintImportAssertions(
"\n}",
);
} else {
parts.push(" ", flat, " }")
parts.push(" ", flat, " }");
}
return concat(parts);
}
Expand Down Expand Up @@ -2979,7 +2954,8 @@ function printExportDeclaration(path: any, options: any, print: any) {

if (decl.source) {
parts.push(
" from ", path.call(print, "source"),
" from ",
path.call(print, "source"),
maybePrintImportAssertions(path, options, print),
);
}
Expand Down Expand Up @@ -3059,22 +3035,19 @@ function swapQuotes(str: string) {
return str.replace(/['"]/g, (m) => (m === '"' ? "'" : '"'));
}

function getPossibleRaw(node:
| types.namedTypes.Literal
| types.namedTypes.NumericLiteral
| types.namedTypes.StringLiteral
| types.namedTypes.RegExpLiteral
| types.namedTypes.BigIntLiteral
| types.namedTypes.DecimalLiteral
function getPossibleRaw(
node:
| types.namedTypes.Literal
| types.namedTypes.NumericLiteral
| types.namedTypes.StringLiteral
| types.namedTypes.RegExpLiteral
| types.namedTypes.BigIntLiteral
| types.namedTypes.DecimalLiteral,
): string | void {
const value = types.getFieldValue(node, "value");
const extra = types.getFieldValue(node, "extra");

if (
extra &&
typeof extra.raw === "string" &&
value == extra.rawValue
) {
if (extra && typeof extra.raw === "string" && value == extra.rawValue) {
return extra.raw;
}

Expand Down
75 changes: 60 additions & 15 deletions test/flow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Printer } from "../lib/printer";
import * as types from "ast-types";
import { EOL as eol } from "os";

describe("type syntax", function () {
describe("Flow type syntax", function () {
const printer = new Printer({
tabWidth: 2,
quote: "single",
Expand All @@ -18,15 +18,25 @@ describe("type syntax", function () {
};

function check(source: string, parseOptions?: any) {
parseOptions = parseOptions || esprimaParserParseOptions;
const ast1 = parse(source, parseOptions);
const code = printer.printGenerically(ast1).code;
const ast2 = parse(code, parseOptions);
types.astNodesAreEquivalent.assert(ast1, ast2);
assert.strictEqual(source, code);
it(`handles: ${source}`, () => {
parseOptions = parseOptions || esprimaParserParseOptions;
const ast1 = parse(source, parseOptions);
const code = printer.printGenerically(ast1).code;
assert.strictEqual(code, source);
const ast2 = parse(code, parseOptions);
types.astNodesAreEquivalent.assert(ast1, ast2);
});
}

it("should parse and print type annotations correctly", function () {
function checkEquiv(a: string, b: string) {
it(`handles equivalently \`${a}\` vs. \`${b}\``, () => {
const aAst = parse(a, flowParserParseOptions);
const bAst = parse(b, flowParserParseOptions);
types.astNodesAreEquivalent.assert(aAst, bAst);
});
}

describe("should parse and print type annotations correctly", function () {
// Import type annotations
check("import type foo from 'foo';");
check("import typeof foo from 'foo';");
Expand Down Expand Up @@ -123,6 +133,24 @@ describe("type syntax", function () {
// Return types
check("function a(): number {}");
check("var a: () => X = fn;");
check("function f(): () => void {}", flowParserParseOptions);
check("function f(): () => () => void {}", flowParserParseOptions);
check(
"function f(): (cb: () => void) => () => void {}",
flowParserParseOptions,
);
// check( // TODO this breaks
// "function f(): (() => void) => () => void {}",
// flowParserParseOptions,
// );
check(
"function f(m: (cb: () => void) => () => void): void {}",
flowParserParseOptions,
);
// check( // TODO this breaks
// "function f((() => void) => () => void): void {}",
// flowParserParseOptions,
// );

// Object
check(
Expand Down Expand Up @@ -154,7 +182,28 @@ describe("type syntax", function () {
check("declare function foo(c: C, b: B): void;");
check("declare function foo(c: (e: Event) => void, b: B): void;");
check("declare function foo(c: C, d?: Array<D>): void;");
check("declare function f(): () => void;", flowParserParseOptions);
check(
"declare function f(): (cb: () => void) => () => void;",
flowParserParseOptions,
);
check(
"declare function f(m: (cb: () => void) => () => void): void;",
flowParserParseOptions,
);
// check( // TODO breaks
// "declare function f(): (() => void) => () => void;",
// flowParserParseOptions,
// );
// check( // TODO breaks
// "declare function f((() => void) => () => void): void;",
// flowParserParseOptions,
// );

check("declare class C { x: string }");
// check("declare class C { constructor(): void }"); // TODO broken
// check("declare class D { f(): D }"); // TODO broken

check(
"declare module M {" +
eol +
Expand All @@ -178,6 +227,8 @@ describe("type syntax", function () {
check("class A {" + eol + " a: number;" + eol + "}");
check("class A {" + eol + " foo(a: number): string {}" + eol + "}");
check("class A {" + eol + " static foo(a: number): string {}" + eol + "}");
check(`class C {${eol} constructor() {}${eol}}`);
check(`class C {${eol} f(): C {}${eol}}`);

// Type parameters
check("class A<T> {}");
Expand Down Expand Up @@ -213,7 +264,7 @@ describe("type syntax", function () {
check("function myFunction([param1]: Params) {}", flowParserParseOptions);
});

it("can pretty-print [Optional]IndexedAccessType AST nodes", () => {
describe("can pretty-print [Optional]IndexedAccessType AST nodes", () => {
check("type A = Obj?.['a'];", flowParserParseOptions);
check("type B = Array<string>?.[number];", flowParserParseOptions);
check("type C = Obj?.['bar']['baz'];", flowParserParseOptions);
Expand All @@ -224,12 +275,6 @@ describe("type syntax", function () {
check("type H = (Obj?.['bar'])[string][];", flowParserParseOptions);
check("type I = Obj?.['bar']?.[string][];", flowParserParseOptions);

function checkEquiv(a: string, b: string) {
const aAst = parse(a, flowParserParseOptions);
const bAst = parse(b, flowParserParseOptions);
types.astNodesAreEquivalent.assert(aAst, bAst);
}

// Since FastPath#needsParens does not currently add any parentheses to
// these expressions, make sure they do not matter for parsing the AST.

Expand Down