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

Rewrite TypeAnnotation and TSTypeAnnotation print #14171

Merged
merged 30 commits into from Jan 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
8d71f97
Rewrite `TypeAnnotation` and `TSTypeAnnotation` print
fisker Jan 13, 2023
1ba6e20
Minor tweak
fisker Jan 13, 2023
167df54
Add test
fisker Jan 13, 2023
4344691
Add test
fisker Jan 13, 2023
e4161f1
Add test for all effected changes
fisker Jan 13, 2023
f5ba6e5
Fix
fisker Jan 13, 2023
af5be4b
Fix
fisker Jan 13, 2023
242d44e
Add changelog
fisker Jan 13, 2023
3e38f74
Fix changelog
fisker Jan 13, 2023
48f1f1b
Space doesn't belong to `TSTypeAnnotation`
fisker Jan 13, 2023
f22e9b3
Indent with spaces
fisker Jan 13, 2023
4c8a94b
Add comment
fisker Jan 13, 2023
ae99735
Add comment
fisker Jan 13, 2023
f4ed781
Add space before comment
fisker Jan 16, 2023
af65908
Update src/language-js/print/type-annotation.js
fisker Jan 17, 2023
567d901
Add type to `CommentCheckFlags`
fisker Jan 17, 2023
b8fd44b
Make sure printed by `printTypeAnnotationProperty`
fisker Jan 17, 2023
2cdbaf5
Fix
fisker Jan 17, 2023
5fd1a47
Fix
fisker Jan 17, 2023
a913332
Fix
fisker Jan 17, 2023
5a2aec5
Merge branch 'next' into type-annotation
fisker Jan 17, 2023
42cae04
Fix
fisker Jan 17, 2023
a28862d
Move space before `=>` into `printTypeAnnotationProperty`
fisker Jan 17, 2023
6000212
Update changelog
fisker Jan 17, 2023
c254296
Add test
fisker Jan 17, 2023
0b77a7c
Add test
fisker Jan 17, 2023
fdf65f8
Fix fixture file name
fisker Jan 17, 2023
e42feca
Refine changelog
fisker Jan 19, 2023
2ad3031
Merge branch 'next' into type-annotation
fisker Jan 25, 2023
0c7cfa2
Merge branch 'next' into type-annotation
fisker Jan 28, 2023
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
33 changes: 33 additions & 0 deletions changelog_unreleased/typescript/14171.md
@@ -0,0 +1,33 @@
#### Improve comment print and cursor tracking around type annotation (#14171 by @fisker)

<!-- prettier-ignore -->
```tsx
// Input
let foo /* comment */ : number;

// Prettier stable
let foo: /* comment */ number;

// Prettier main
<Same as input>
fisker marked this conversation as resolved.
Show resolved Hide resolved
```

```js
// Prettier stable
prettier.formatWithCursor("let foo: number", {
cursorOffset: 7,
parser: "babel",
}).cursorOffset;

// -> 9

// Prettier main
(
await prettier.formatWithCursor("let foo: number", {
cursorOffset: 7,
parser: "babel",
})
).cursorOffset;

// -> 7
```
44 changes: 38 additions & 6 deletions src/language-js/needs-parens.js
Expand Up @@ -218,6 +218,20 @@ function needsParens(path, options) {
return true;
}
break;

case "TypeAnnotation":
if (
path.match(
undefined,
undefined,
(node, key) =>
key === "returnType" && node.type === "ArrowFunctionExpression"
) &&
includesFunctionTypeInObjectType(node)
) {
return true;
}
break;
}

switch (node.type) {
Expand Down Expand Up @@ -454,6 +468,18 @@ function needsParens(path, options) {
}
// fallthrough
case "TSFunctionType":
if (
path.match(
(node) => node.type === "TSFunctionType",
(node, key) =>
key === "typeAnnotation" && node.type === "TSTypeAnnotation",
(node, key) =>
key === "returnType" && node.type === "ArrowFunctionExpression"
)
) {
return true;
}
// fallthrough
case "TSConstructorType":
if (key === "extendsType" && parent.type === "TSConditionalType") {
const returnTypeAnnotation = (node.returnType || node.typeAnnotation)
Expand Down Expand Up @@ -524,6 +550,18 @@ function needsParens(path, options) {
);

case "FunctionTypeAnnotation": {
if (
path.match(
undefined,
(node, key) =>
key === "typeAnnotation" && node.type === "TypeAnnotation",
(node, key) =>
key === "returnType" && node.type === "ArrowFunctionExpression"
)
) {
return true;
}

const ancestor =
parent.type === "NullableTypeAnnotation" ? path.grandparent : parent;

Expand Down Expand Up @@ -832,12 +870,6 @@ function needsParens(path, options) {
parent.type !== "VariableDeclarator" &&
parent.type !== "YieldExpression")
);
case "TypeAnnotation":
return (
key === "returnType" &&
parent.type === "ArrowFunctionExpression" &&
includesFunctionTypeInObjectType(node)
);
}

return false;
Expand Down
5 changes: 3 additions & 2 deletions src/language-js/print/array.js
Expand Up @@ -21,7 +21,8 @@ import {
} from "../utils/index.js";
import { locStart } from "../loc.js";

import { printOptionalToken, printTypeAnnotation } from "./misc.js";
import { printOptionalToken } from "./misc.js";
import { printTypeAnnotationProperty } from "./type-annotation.js";

/** @typedef {import("../../document/builders.js").Doc} Doc */

Expand Down Expand Up @@ -139,7 +140,7 @@ function printArray(path, options, print) {

parts.push(
printOptionalToken(path),
printTypeAnnotation(path, options, print)
printTypeAnnotationProperty(path, print)
);

return parts;
Expand Down
4 changes: 2 additions & 2 deletions src/language-js/print/class.js
Expand Up @@ -19,13 +19,13 @@ import { getTypeParametersGroupId } from "./type-parameters.js";
import { printMethod } from "./function.js";
import {
printOptionalToken,
printTypeAnnotation,
printDefiniteToken,
printDeclareToken,
} from "./misc.js";
import { printPropertyKey } from "./property.js";
import { printAssignment } from "./assignment.js";
import { printClassMemberDecorators } from "./decorators.js";
import { printTypeAnnotationProperty } from "./type-annotation.js";

/**
* @typedef {import("../../document/builders.js").Doc} Doc
Expand Down Expand Up @@ -245,7 +245,7 @@ function printClassProperty(path, options, print) {
printPropertyKey(path, options, print),
printOptionalToken(path),
printDefiniteToken(path),
printTypeAnnotation(path, options, print)
printTypeAnnotationProperty(path, print)
);

const isAbstractProperty =
Expand Down
17 changes: 12 additions & 5 deletions src/language-js/print/flow.js
Expand Up @@ -17,6 +17,8 @@ import {
printUnionType,
printFunctionType,
printIndexedAccessType,
printTypeAnnotation,
printTypeAnnotationProperty,
} from "./type-annotation.js";
import { printInterface } from "./interface.js";
import { printTypeParameter, printTypeParameters } from "./type-parameters.js";
Expand All @@ -32,7 +34,6 @@ import {
import { printBigInt } from "./literal.js";
import {
printOptionalToken,
printTypeAnnotation,
printRestSpread,
printDeclareToken,
} from "./misc.js";
Expand All @@ -56,7 +57,11 @@ function printFlow(path, options, print) {
case "DeclareModule":
return ["declare module ", print("id"), " ", print("body")];
case "DeclareModuleExports":
return ["declare module.exports", ": ", print("typeAnnotation"), semi];
return [
"declare module.exports",
printTypeAnnotationProperty(path, print),
semi,
];
case "DeclareVariable":
return [printDeclareToken(path), "var ", print("id"), semi];
case "DeclareExportDeclaration":
Expand Down Expand Up @@ -90,7 +95,7 @@ function printFlow(path, options, print) {
// Type Annotations for Facebook Flow, typically stripped out or
// transformed away before printing.
case "TypeAnnotation":
return print("typeAnnotation");
return printTypeAnnotation(path, options, print);
case "TypeParameter":
return printTypeParameter(path, options, print);
case "TypeofTypeAnnotation":
Expand Down Expand Up @@ -131,6 +136,8 @@ function printFlow(path, options, print) {
return [
name,
printOptionalToken(path),
// `flow` doesn't wrap the `typeAnnotation` with `TypeAnnotation`, so the colon
// needs to be added separately.
name ? ": " : "",
print("typeAnnotation"),
];
Expand Down Expand Up @@ -198,7 +205,7 @@ function printFlow(path, options, print) {
];
// Same as `RestElement`
case "ObjectTypeSpreadProperty":
return printRestSpread(path, options, print);
return printRestSpread(path, print);
case "QualifiedTypeofIdentifier":
case "QualifiedTypeIdentifier":
return [print("qualification"), ".", print("id")];
Expand All @@ -212,7 +219,7 @@ function printFlow(path, options, print) {
return [
"(",
print("expression"),
printTypeAnnotation(path, options, print),
printTypeAnnotationProperty(path, print),
")",
];

Expand Down
8 changes: 2 additions & 6 deletions src/language-js/print/function.js
Expand Up @@ -44,6 +44,7 @@ import {
} from "./function-parameters.js";
import { printPropertyKey } from "./property.js";
import { printFunctionTypeParameters, printDeclareToken } from "./misc.js";
import { printTypeAnnotationProperty } from "./type-annotation.js";

const isMethod = (node) =>
node.type === "ObjectMethod" ||
Expand Down Expand Up @@ -436,15 +437,10 @@ function shouldPrintParamsWithoutParens(path, options) {
/** @returns {Doc} */
function printReturnType(path, print) {
const { node } = path;
const returnType = print("returnType");
const returnType = printTypeAnnotationProperty(path, print, "returnType");

const parts = [returnType];

// prepend colon to TypeScript type annotation
if (node.returnType?.typeAnnotation) {
parts.unshift(": ");
}

if (node.predicate) {
// The return type will already add the colon, but otherwise we
// need to do it ourselves
Expand Down
18 changes: 3 additions & 15 deletions src/language-js/print/misc.js
@@ -1,5 +1,6 @@
import { isNonEmptyArray } from "../../common/util.js";
import { indent, join, line } from "../../document/builders.js";
import { printTypeAnnotationProperty } from "./type-annotation.js";

function printOptionalToken(path) {
const { node } = path;
Expand Down Expand Up @@ -68,18 +69,6 @@ function printFunctionTypeParameters(path, options, print) {
return "";
}

function printTypeAnnotation(path, options, print) {
const { node, parent, key } = path;
if (!node.typeAnnotation) {
return "";
}

const isFunctionDeclarationIdentifier =
parent.type === "DeclareFunction" && key === "id";

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

function printBindExpressionCallee(path, options, print) {
return ["::", print("callee")];
}
Expand All @@ -104,8 +93,8 @@ function adjustClause(node, clause, forceSpace) {
return indent([line, clause]);
}

function printRestSpread(path, options, print) {
return ["...", print("argument"), printTypeAnnotation(path, options, print)];
function printRestSpread(path, print) {
return ["...", print("argument"), printTypeAnnotationProperty(path, print)];
}

function printDirective(rawText, options) {
Expand Down Expand Up @@ -133,7 +122,6 @@ export {
printFunctionTypeParameters,
printBindExpressionCallee,
printTypeScriptModifiers,
printTypeAnnotation,
printRestSpread,
adjustClause,
printDirective,
Expand Down
9 changes: 5 additions & 4 deletions src/language-js/print/object.js
Expand Up @@ -22,9 +22,10 @@ import {
} from "../utils/index.js";
import { locStart, locEnd } from "../loc.js";

import { printOptionalToken, printTypeAnnotation } from "./misc.js";
import { printOptionalToken } from "./misc.js";
import { shouldHugTheOnlyFunctionParameter } from "./function-parameters.js";
import { printHardlineAfterHeritage } from "./class.js";
import { printTypeAnnotationProperty } from "./type-annotation.js";

/** @typedef {import("../../document/builders.js").Doc} Doc */

Expand Down Expand Up @@ -170,7 +171,7 @@ function printObject(path, options, print) {
let content;
if (props.length === 0) {
if (!hasComment(node, CommentCheckFlags.Dangling)) {
return [leftBrace, rightBrace, printTypeAnnotation(path, options, print)];
return [leftBrace, rightBrace, printTypeAnnotationProperty(path, print)];
}

content = group([
Expand All @@ -179,7 +180,7 @@ function printObject(path, options, print) {
softline,
rightBrace,
printOptionalToken(path),
printTypeAnnotation(path, options, print),
printTypeAnnotationProperty(path, print),
]);
} else {
content = [
Expand All @@ -197,7 +198,7 @@ function printObject(path, options, print) {
options.bracketSpacing ? line : softline,
rightBrace,
printOptionalToken(path),
printTypeAnnotation(path, options, print),
printTypeAnnotationProperty(path, print),
];
}

Expand Down