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

Support conditional type and infer type in Flow #14573

Merged
merged 8 commits into from Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
19 changes: 19 additions & 0 deletions changelog_unreleased/flow/14573.md
@@ -0,0 +1,19 @@
#### Support conditional type and infer type (#14573 by @SamChou19815)

<!-- Optional description if it makes sense. -->

<!-- prettier-ignore -->
```jsx
// Input
type TestReturnType<T extends (...args: any[]) => any> = T extends (...args: any[]) => infer R ? R : any;

// Prettier stable
// Does not parse

// Prettier main
type TestReturnType<T extends (...args: any[]) => any> = T extends (
...args: any[]
) => infer R
? R
: any;
```
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -62,7 +62,7 @@
"file-entry-cache": "6.0.1",
"find-cache-dir": "4.0.0",
"find-parent-dir": "0.3.1",
"flow-parser": "0.202.0",
"flow-parser": "0.202.1",
"get-stdin": "9.0.0",
"graphql": "16.6.0",
"hermes-eslint": "0.10.0",
Expand Down
19 changes: 18 additions & 1 deletion src/language-js/needs-parens.js
Expand Up @@ -559,7 +559,7 @@ function needsParens(path, options) {
(parent.type === "IndexedAccessType" ||
parent.type === "OptionalIndexedAccessType"))
);

case "InferTypeAnnotation":
case "NullableTypeAnnotation":
return (
parent.type === "ArrayTypeAnnotation" ||
Expand Down Expand Up @@ -591,6 +591,9 @@ function needsParens(path, options) {
(key === "objectType" &&
(ancestor.type === "IndexedAccessType" ||
ancestor.type === "OptionalIndexedAccessType")) ||
(key === "checkType" && parent.type === "ConditionalTypeAnnotation") ||
(node.returnType.type === "InferTypeAnnotation" &&
node.returnType.typeParameter.bound) ||
// We should check ancestor's parent to know whether the parentheses
// are really needed, but since ??T doesn't make sense this check
// will almost never be true.
Expand All @@ -604,6 +607,20 @@ function needsParens(path, options) {
);
}

case "ConditionalTypeAnnotation":
if (
key === "extendsType" &&
parent.type === "ConditionalTypeAnnotation" &&
node.type === "ConditionalTypeAnnotation"
) {
return true;
}

if (key === "checkType" && parent.type === "ConditionalTypeAnnotation") {
return true;
}
Copy link
Member

Choose a reason for hiding this comment

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

Loos like we are missing cases, it's bad we are not sharing logic between TS and Flow.
Maybe better extract some code first to reuse?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed the logic and pass the test. I'm not sure how to better extract the code, since there are very little code sharing currently in the file.

My personal opinion is that the existing fallthrough logic for TS nodes already made it harder to understand, and I actually like the more explicit logic among those Flow nodes that don't use fallthrough...


// fallthrough
case "OptionalIndexedAccessType":
return key === "objectType" && parent.type === "IndexedAccessType";

Expand Down
6 changes: 6 additions & 0 deletions src/language-js/print/flow.js
Expand Up @@ -16,6 +16,7 @@ import {
printOpaqueType,
printTypeAlias,
printIntersectionType,
printInferType,
printUnionType,
printFunctionType,
printIndexedAccessType,
Expand Down Expand Up @@ -43,6 +44,7 @@ import {
printRestSpread,
printDeclareToken,
} from "./misc.js";
import { printTernary } from "./ternary.js";

function printFlow(path, options, print) {
const { node } = path;
Expand Down Expand Up @@ -98,6 +100,10 @@ function printFlow(path, options, print) {
return printIntersectionType(path, options, print);
case "UnionTypeAnnotation":
return printUnionType(path, options, print);
case "ConditionalTypeAnnotation":
return printTernary(path, options, print);
SamChou19815 marked this conversation as resolved.
Show resolved Hide resolved
case "InferTypeAnnotation":
return printInferType(path, options, print);
case "FunctionTypeAnnotation":
return printFunctionType(path, options, print);
case "TupleTypeAnnotation":
Expand Down
4 changes: 2 additions & 2 deletions src/language-js/print/ternary.js
Expand Up @@ -182,8 +182,8 @@ function shouldExtraIndentForConditionalExpression(path) {

/**
* The following is the shared logic for
* ternary operators, namely ConditionalExpression
* and TSConditionalType
* ternary operators, namely ConditionalExpression,
* ConditionalTypeAnnotation and TSConditionalType
* @param {AstPath} path - The path to the ConditionalExpression/TSConditionalType node.
* @param {Options} options - Prettier options
* @param {Function} print - Print function to call recursively
Expand Down
9 changes: 9 additions & 0 deletions src/language-js/print/type-annotation.js
Expand Up @@ -324,6 +324,14 @@ function printIndexedAccessType(path, options, print) {
];
}

/*
- `TSInferType`(TypeScript)
- `InferTypeAnnotation`(flow)
*/
function printInferType(path, options, print) {
return ["infer", " ", print("typeParameter")];
}

// `TSJSDocNullableType`, `TSJSDocNonNullableType`
function printJSDocType(path, print, token) {
const { node } = path;
Expand Down Expand Up @@ -504,6 +512,7 @@ export {
printUnionType,
printFunctionType,
printIndexedAccessType,
printInferType,
shouldHugType,
printJSDocType,
printRestType,
Expand Down
6 changes: 5 additions & 1 deletion src/language-js/print/type-parameters.js
Expand Up @@ -152,7 +152,11 @@ function printTypeParameter(path, options, print) {
parts.push(name);

if (node.bound) {
parts.push(printTypeAnnotationProperty(path, print, "bound"));
if (node.usesExtendsBound) {
parts.push(" extends ", print(["bound", "typeAnnotation"]));
Copy link
Member

Choose a reason for hiding this comment

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

node.bound is skipped, better not to do that, there could be comments, cursors.

This won't work?

parts.push(" extends ", print("bound"));

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I understand the problem, we should still use printTypeAnnotationProperty. I'll fix it and add comments to explain why it's needed and how it works on monday.

Copy link
Member

Choose a reason for hiding this comment

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

Found a case

Prettier pr-14573
Playground link

--parser flow

Input:

type A = () => infer R extends/* comment */ string

Output:

Error: Comment "comment" was not printed. Please report this error!
    at dn (https://deploy-preview-14573--prettier.netlify.app/lib/standalone.js:29:5817)
    at Ze (https://deploy-preview-14573--prettier.netlify.app/lib/standalone.js:29:7308)
    at async kn (https://deploy-preview-14573--prettier.netlify.app/lib/standalone.js:29:11045)
    at async ju (https://deploy-preview-14573--prettier.netlify.app/lib/standalone.js:34:581)
    at async formatCode (https://deploy-preview-14573--prettier.netlify.app/worker.js:159:12)
    at async handleFormatMessage (https://deploy-preview-14573--prettier.netlify.app/worker.js:83:24)
    at async self.onmessage (https://deploy-preview-14573--prettier.netlify.app/worker.js:38:14)

} else {
parts.push(printTypeAnnotationProperty(path, print, "bound"));
}
}

if (node.constraint) {
Expand Down
3 changes: 2 additions & 1 deletion src/language-js/print/typescript.js
Expand Up @@ -47,6 +47,7 @@ import {
printUnionType,
printFunctionType,
printIndexedAccessType,
printInferType,
printJSDocType,
printRestType,
printNamedTupleMember,
Expand Down Expand Up @@ -372,7 +373,7 @@ function printTypescript(path, options, print) {
return printTernary(path, options, print);

case "TSInferType":
return ["infer ", print("typeParameter")];
return printInferType(path, options, print);
case "TSIntersectionType":
return printIntersectionType(path, options, print);
case "TSUnionType":
Expand Down
7 changes: 7 additions & 0 deletions src/language-js/traverse/visitor-keys.evaluate.js
Expand Up @@ -34,7 +34,14 @@ const additionalVisitorKeys = {
QualifiedTypeofIdentifier: ["id", "qualification"],
ClassProperty: ["variance"],
ClassPrivateProperty: ["variance"],
ConditionalTypeAnnotation: [
"checkType",
"extendsType",
"trueType",
"falseType",
],
DeclareEnum: flowVisitorKeys.EnumDeclaration,
InferTypeAnnotation: ["typeParameter"],
TupleTypeAnnotation: ["elementTypes"],
TupleTypeSpreadElement: ["label", "typeAnnotation"],
TupleTypeLabeledElement: ["label", "elementType", "variance"],
Expand Down
@@ -0,0 +1,161 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`comments.js [babel-flow] format 1`] = `
"Unexpected token (3:12)
1 | // @flow
2 |
> 3 | type A = B extends T
| ^
4 | ? // comment
5 | foo
6 | : bar;"
`;

exports[`comments.js format 1`] = `
====================================options=====================================
parsers: ["flow"]
printWidth: 80
| printWidth
=====================================input======================================
// @flow

type A = B extends T
? // comment
foo
: bar;

type A = B extends test /* comment
comment
comment
*/
? foo
: bar;

type T = test extends B
? /* comment
comment
comment
comment
*/
foo
: bar;

type T = test extends B
? /* comment
comment
comment
comment
*/
foo
: test extends B
? /* comment
comment
comment */
foo
: bar;

type T = test extends B
? /* comment */
foo
: bar;

type T = test extends B
? foo
: /* comment
comment
comment
comment
*/
bar;

type T = test extends B
? foo
: /* comment
comment
comment
comment
*/
test extends B
? foo
: /* comment
comment
comment
*/
bar;

type T = test extends B
? foo
: /* comment */
bar;

type T = test extends B ? test extends B /* c
c */? foo : bar : bar;

=====================================output=====================================
// @flow

type A = B extends T // comment
? foo
: bar;

type A = B extends test /* comment
comment
comment
*/
? foo
: bar;

type T = test extends B /* comment
comment
comment
comment
*/
? foo
: bar;

type T = test extends B /* comment
comment
comment
comment
*/
? foo
: test extends B /* comment
comment
comment */
? foo
: bar;

type T = test extends B /* comment */ ? foo : bar;

type T = test extends B
? foo /* comment
comment
comment
comment
*/
: bar;

type T = test extends B
? foo /* comment
comment
comment
comment
*/
: test extends B
? foo /* comment
comment
comment
*/
: bar;

type T = test extends B ? foo /* comment */ : bar;

type T = test extends B
? test extends B /* c
c */
? foo
: bar
: bar;

================================================================================
`;