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 naming tuple members #38234

Merged
merged 14 commits into from
May 19, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
254 changes: 190 additions & 64 deletions src/compiler/checker.ts

Large diffs are not rendered by default.

24 changes: 24 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3517,6 +3517,22 @@
"category": "Error",
"code": 5083
},
"Tuple members must all have names or all not have names.": {
"category": "Error",
"code": 5084
},
"A tuple member cannot be both optional and rest.": {
"category": "Error",
"code": 5085
},
"A labeled tuple element is declared as optional with a question mark after the name and before the colon, rather than after the type.": {
"category": "Error",
"code": 5086
},
"A labeled tuple element is declared as rest with a `...` before the name, rather than before the type.": {
"category": "Error",
"code": 5087
},

"Generates a sourcemap for each corresponding '.d.ts' file.": {
"category": "Message",
Expand Down Expand Up @@ -5677,6 +5693,14 @@
"category": "Message",
"code": 95116
},
"Move labeled tuple element modifiers to labels": {
"category": "Message",
"code": 95117
},
"Convert overload list to single signature": {
"category": "Message",
"code": 95118
},

"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
"category": "Error",
Expand Down
20 changes: 16 additions & 4 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1370,6 +1370,8 @@ namespace ts {
case SyntaxKind.RestType:
case SyntaxKind.JSDocVariadicType:
return emitRestOrJSDocVariadicType(node as RestTypeNode | JSDocVariadicType);
case SyntaxKind.NamedTupleMember:
return emitNamedTupleMember(node as NamedTupleMember);

// Binding patterns
case SyntaxKind.ObjectBindingPattern:
Expand Down Expand Up @@ -2099,9 +2101,19 @@ namespace ts {
}

function emitTupleType(node: TupleTypeNode) {
writePunctuation("[");
emitList(node, node.elementTypes, ListFormat.TupleTypeElements);
writePunctuation("]");
emitTokenWithComment(SyntaxKind.OpenBracketToken, node.pos, writePunctuation, node);
const flags = getEmitFlags(node) & EmitFlags.SingleLine ? ListFormat.SingleLineTupleTypeElements : ListFormat.MultiLineTupleTypeElements;
emitList(node, node.elements, flags | ListFormat.NoSpaceIfEmpty);
emitTokenWithComment(SyntaxKind.CloseBracketToken, node.elements.end, writePunctuation, node);
}

function emitNamedTupleMember(node: NamedTupleMember) {
emit(node.dotDotDotToken);
emit(node.name);
emit(node.questionToken);
emitTokenWithComment(SyntaxKind.ColonToken, node.name.end, writePunctuation, node);
writeSpace();
emit(node.type);
}

function emitOptionalType(node: OptionalTypeNode) {
Expand Down Expand Up @@ -4968,7 +4980,7 @@ namespace ts {
}

function emitLeadingSynthesizedComment(comment: SynthesizedComment) {
if (comment.kind === SyntaxKind.SingleLineCommentTrivia) {
if (comment.hasLeadingNewline || comment.kind === SyntaxKind.SingleLineCommentTrivia) {
writer.writeLine();
}
writeSynthesizedComment(comment);
Expand Down
43 changes: 38 additions & 5 deletions src/compiler/factoryPublic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -810,15 +810,15 @@ namespace ts {
: node;
}

export function createTupleTypeNode(elementTypes: readonly TypeNode[]) {
export function createTupleTypeNode(elements: readonly (TypeNode | NamedTupleMember)[]) {
const node = createSynthesizedNode(SyntaxKind.TupleType) as TupleTypeNode;
node.elementTypes = createNodeArray(elementTypes);
node.elements = createNodeArray(elements);
return node;
}

export function updateTupleTypeNode(node: TupleTypeNode, elementTypes: readonly TypeNode[]) {
return node.elementTypes !== elementTypes
? updateNode(createTupleTypeNode(elementTypes), node)
export function updateTupleTypeNode(node: TupleTypeNode, elements: readonly (TypeNode | NamedTupleMember)[]) {
return node.elements !== elements
? updateNode(createTupleTypeNode(elements), node)
: node;
}

Expand Down Expand Up @@ -934,6 +934,24 @@ namespace ts {
: node;
}

export function createNamedTupleMember(dotDotDotToken: Token<SyntaxKind.DotDotDotToken> | undefined, name: Identifier, questionToken: Token<SyntaxKind.QuestionToken> | undefined, type: TypeNode) {
const node = <NamedTupleMember>createSynthesizedNode(SyntaxKind.NamedTupleMember);
node.dotDotDotToken = dotDotDotToken;
node.name = name;
node.questionToken = questionToken;
node.type = type;
return node;
}

export function updateNamedTupleMember(node: NamedTupleMember, dotDotDotToken: Token<SyntaxKind.DotDotDotToken> | undefined, name: Identifier, questionToken: Token<SyntaxKind.QuestionToken> | undefined, type: TypeNode) {
return node.dotDotDotToken !== dotDotDotToken
|| node.name !== name
|| node.questionToken !== questionToken
|| node.type !== type
? updateNode(createNamedTupleMember(dotDotDotToken, name, questionToken, type), node)
: node;
}

export function createThisTypeNode() {
return <ThisTypeNode>createSynthesizedNode(SyntaxKind.ThisType);
}
Expand Down Expand Up @@ -2616,6 +2634,21 @@ namespace ts {
return node;
}


/* @internal */
export function createJSDocVariadicType(type: TypeNode): JSDocVariadicType {
const node = createSynthesizedNode(SyntaxKind.JSDocVariadicType) as JSDocVariadicType;
node.type = type;
return node;
}

/* @internal */
export function updateJSDocVariadicType(node: JSDocVariadicType, type: TypeNode): JSDocVariadicType {
return node.type !== type
? updateNode(createJSDocVariadicType(type), node)
: node;
}

// JSX

export function createJsxElement(openingElement: JsxOpeningElement, children: readonly JsxChild[], closingElement: JsxClosingElement) {
Expand Down
33 changes: 31 additions & 2 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ namespace ts {
case SyntaxKind.ArrayType:
return visitNode(cbNode, (<ArrayTypeNode>node).elementType);
case SyntaxKind.TupleType:
return visitNodes(cbNode, cbNodes, (<TupleTypeNode>node).elementTypes);
return visitNodes(cbNode, cbNodes, (<TupleTypeNode>node).elements);
case SyntaxKind.UnionType:
case SyntaxKind.IntersectionType:
return visitNodes(cbNode, cbNodes, (<UnionOrIntersectionTypeNode>node).types);
Expand Down Expand Up @@ -207,6 +207,11 @@ namespace ts {
visitNode(cbNode, (<MappedTypeNode>node).type);
case SyntaxKind.LiteralType:
return visitNode(cbNode, (<LiteralTypeNode>node).literal);
case SyntaxKind.NamedTupleMember:
return visitNode(cbNode, (<NamedTupleMember>node).dotDotDotToken) ||
visitNode(cbNode, (<NamedTupleMember>node).name) ||
visitNode(cbNode, (<NamedTupleMember>node).questionToken) ||
visitNode(cbNode, (<NamedTupleMember>node).type);
case SyntaxKind.ObjectBindingPattern:
case SyntaxKind.ArrayBindingPattern:
return visitNodes(cbNode, cbNodes, (<BindingPattern>node).elements);
Expand Down Expand Up @@ -3056,9 +3061,33 @@ namespace ts {
return type;
}

function isNextTokenColonOrQuestionColon() {
return nextToken() === SyntaxKind.ColonToken || (token() === SyntaxKind.QuestionToken && nextToken() === SyntaxKind.ColonToken);
}

function isTupleElementName() {
if (token() === SyntaxKind.DotDotDotToken) {
return tokenIsIdentifierOrKeyword(nextToken()) && isNextTokenColonOrQuestionColon();
}
return tokenIsIdentifierOrKeyword(token()) && isNextTokenColonOrQuestionColon();
}

function parseTupleElementNameOrTupleElementType() {
if (lookAhead(isTupleElementName)) {
const node = <NamedTupleMember>createNode(SyntaxKind.NamedTupleMember);
node.dotDotDotToken = parseOptionalToken(SyntaxKind.DotDotDotToken);
node.name = parseIdentifierName();
node.questionToken = parseOptionalToken(SyntaxKind.QuestionToken);
parseExpected(SyntaxKind.ColonToken);
node.type = parseTupleElementType();
return addJSDocComment(finishNode(node));
}
return parseTupleElementType();
}

function parseTupleType(): TupleTypeNode {
const node = <TupleTypeNode>createNode(SyntaxKind.TupleType);
node.elementTypes = parseBracketedList(ParsingContext.TupleElementTypes, parseTupleElementType, SyntaxKind.OpenBracketToken, SyntaxKind.CloseBracketToken);
node.elements = parseBracketedList(ParsingContext.TupleElementTypes, parseTupleElementNameOrTupleElementType, SyntaxKind.OpenBracketToken, SyntaxKind.CloseBracketToken);
return finishNode(node);
}

Expand Down
4 changes: 4 additions & 0 deletions src/compiler/transformers/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,10 @@ namespace ts {
}
}

if (isTupleTypeNode(input) && (getLineAndCharacterOfPosition(currentSourceFile, input.pos).line === getLineAndCharacterOfPosition(currentSourceFile, input.end).line)) {
setEmitFlags(input, EmitFlags.SingleLine);
}

return cleanup(visitEachChild(input, visitDeclarationSubtree, context));

function cleanup<T extends Node>(returnValue: T | undefined): T | undefined {
Expand Down
22 changes: 18 additions & 4 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ namespace ts {
IndexedAccessType,
MappedType,
LiteralType,
NamedTupleMember,
ImportType,
// Binding patterns
ObjectBindingPattern,
Expand Down Expand Up @@ -700,6 +701,7 @@ namespace ts {
| ConstructorTypeNode
| JSDocFunctionType
| ExportDeclaration
| NamedTupleMember
| EndOfFileToken;

export type HasType =
Expand Down Expand Up @@ -1274,7 +1276,15 @@ namespace ts {

export interface TupleTypeNode extends TypeNode {
kind: SyntaxKind.TupleType;
elementTypes: NodeArray<TypeNode>;
elements: NodeArray<TypeNode | NamedTupleMember>;
Copy link
Member Author

Choose a reason for hiding this comment

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

In the end, I made NamedTupleMember a TypeNode, which is inline with OptionalType and RestType, so I don't have to rename this field (or even change the type). If anyone feels strongly about it, I can change it back. However renaming it was very useful for figuring out where I needed to adjust/handle the new node; so "breaking" the API because of the new child kind might be worthwhile for other consumers, too. Depends on how strongly we want to maintain AST compatibility I suppose.

Copy link
Member

Choose a reason for hiding this comment

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

I like the rename.

Copy link
Contributor

Choose a reason for hiding this comment

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

This rename is missing in the API breaking changes for 4.0 - https://github.com/microsoft/TypeScript/wiki/API-Breaking-Changes

}

export interface NamedTupleMember extends TypeNode, JSDocContainer, Declaration {
kind: SyntaxKind.NamedTupleMember;
dotDotDotToken?: Token<SyntaxKind.DotDotDotToken>;
name: Identifier;
questionToken?: Token<SyntaxKind.QuestionToken>;
Copy link
Member

Choose a reason for hiding this comment

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

General design question: how do you decide between saving a reference to a token like this vs. saving a boolean property that indicates optionality, like ImportTypeNode['isTypeOf']? Is this token actually used anywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

In this case, mostly so the members of the same name are the same type as ParameterDeclaration. In the case of ParameterDeclaration, so comments can more easily be collected on the intervening tokens (iirc). (Types don't normally care about comment preservation on intervening tokens)

type: TypeNode;
}

export interface OptionalTypeNode extends TypeNode {
Expand Down Expand Up @@ -1478,6 +1488,7 @@ namespace ts {
kind: SyntaxKind.SyntheticExpression;
isSpread: boolean;
type: Type;
tupleNameSource?: ParameterDeclaration | NamedTupleMember;
}

// see: https://tc39.github.io/ecma262/#prod-ExponentiationExpression
Expand Down Expand Up @@ -2590,6 +2601,7 @@ namespace ts {
text: string;
pos: -1;
end: -1;
hasLeadingNewline?: boolean;
}

// represents a top level: { type } expression in a JSDoc comment.
Expand Down Expand Up @@ -3495,7 +3507,7 @@ namespace ts {
*/
getResolvedSignature(node: CallLikeExpression, candidatesOutArray?: Signature[], argumentCount?: number): Signature | undefined;
/* @internal */ getResolvedSignatureForSignatureHelp(node: CallLikeExpression, candidatesOutArray?: Signature[], argumentCount?: number): Signature | undefined;
/* @internal */ getExpandedParameters(sig: Signature): readonly Symbol[];
/* @internal */ getExpandedParameters(sig: Signature): readonly (readonly Symbol[])[];
/* @internal */ hasEffectiveRestParameter(sig: Signature): boolean;
getSignatureFromDeclaration(declaration: SignatureDeclaration): Signature | undefined;
isImplementationOfOverload(node: SignatureDeclaration): boolean | undefined;
Expand Down Expand Up @@ -4148,6 +4160,7 @@ namespace ts {
cjsExportMerged?: Symbol; // Version of the symbol with all non export= exports merged with the export= target
typeOnlyDeclaration?: TypeOnlyCompatibleAliasDeclaration | false; // First resolved alias declaration that makes the symbol only usable in type constructs
isConstructorDeclaredProperty?: boolean; // Property declared through 'this.x = ...' assignment in constructor
tupleLabelDeclaration?: NamedTupleMember | ParameterDeclaration; // Declaration associated with the tuple's label
}

/* @internal */
Expand Down Expand Up @@ -4618,7 +4631,7 @@ namespace ts {
minLength: number;
hasRestElement: boolean;
readonly: boolean;
associatedNames?: __String[];
labeledElementDeclarations?: readonly (NamedTupleMember | ParameterDeclaration)[];
}

export interface TupleTypeReference extends TypeReference {
Expand Down Expand Up @@ -6559,7 +6572,8 @@ namespace ts {
SingleLineTypeLiteralMembers = SingleLine | SpaceBetweenBraces | SpaceBetweenSiblings,
MultiLineTypeLiteralMembers = MultiLine | Indented | OptionalIfEmpty,

TupleTypeElements = CommaDelimited | SpaceBetweenSiblings | SingleLine,
SingleLineTupleTypeElements = CommaDelimited | SpaceBetweenSiblings | SingleLine,
MultiLineTupleTypeElements = CommaDelimited | Indented | SpaceBetweenSiblings | MultiLine,
UnionTypeConstituents = BarDelimited | SpaceBetweenSiblings | SingleLine,
IntersectionTypeConstituents = AmpersandDelimited | SpaceBetweenSiblings | SingleLine,
ObjectBindingPatternElements = SingleLine | AllowTrailingComma | SpaceBetweenBraces | CommaDelimited | SpaceBetweenSiblings | NoSpaceIfEmpty,
Expand Down
10 changes: 9 additions & 1 deletion src/compiler/visitorPublic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ namespace ts {

case SyntaxKind.TupleType:
return updateTupleTypeNode((<TupleTypeNode>node),
nodesVisitor((<TupleTypeNode>node).elementTypes, visitor, isTypeNode));
nodesVisitor((<TupleTypeNode>node).elements, visitor, isTypeNode));

case SyntaxKind.OptionalType:
return updateOptionalTypeNode((<OptionalTypeNode>node),
Expand Down Expand Up @@ -517,6 +517,14 @@ namespace ts {
(<ImportTypeNode>node).isTypeOf
);

case SyntaxKind.NamedTupleMember:
return updateNamedTupleMember(<NamedTupleMember>node,
visitNode((<NamedTupleMember>node).dotDotDotToken, visitor, isToken),
visitNode((<NamedTupleMember>node).name, visitor, isIdentifier),
visitNode((<NamedTupleMember>node).questionToken, visitor, isToken),
visitNode((<NamedTupleMember>node).type, visitor, isTypeNode),
);

case SyntaxKind.ParenthesizedType:
return updateParenthesizedType(<ParenthesizedTypeNode>node,
visitNode((<ParenthesizedTypeNode>node).type, visitor, isTypeNode));
Expand Down
52 changes: 52 additions & 0 deletions src/services/codefixes/fixIncorrectNamedTupleSyntax.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/* @internal */
namespace ts.codefix {
const fixId = "fixIncorrectNamedTupleSyntax";
const errorCodes = [
Diagnostics.A_labeled_tuple_element_is_declared_as_optional_with_a_question_mark_after_the_name_and_before_the_colon_rather_than_after_the_type.code,
Diagnostics.A_labeled_tuple_element_is_declared_as_rest_with_a_before_the_name_rather_than_before_the_type.code
];

registerCodeFix({
errorCodes,
getCodeActions: context => {
const { sourceFile, span } = context;
const namedTupleMember = getNamedTupleMember(sourceFile, span.start);
const changes = textChanges.ChangeTracker.with(context, t => doChange(t, sourceFile, namedTupleMember));
return [createCodeFixAction(fixId, changes, Diagnostics.Move_labeled_tuple_element_modifiers_to_labels, fixId, Diagnostics.Move_labeled_tuple_element_modifiers_to_labels)];
},
fixIds: [fixId]
});

function getNamedTupleMember(sourceFile: SourceFile, pos: number) {
const token = getTokenAtPosition(sourceFile, pos);
return findAncestor(token, t => t.kind === SyntaxKind.NamedTupleMember) as NamedTupleMember | undefined;
}
function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, namedTupleMember?: NamedTupleMember) {
if (!namedTupleMember) {
return;
}
let unwrappedType = namedTupleMember.type;
let sawOptional = false;
let sawRest = false;
while (unwrappedType.kind === SyntaxKind.OptionalType || unwrappedType.kind === SyntaxKind.RestType || unwrappedType.kind === SyntaxKind.ParenthesizedType) {
if (unwrappedType.kind === SyntaxKind.OptionalType) {
sawOptional = true;
}
else if (unwrappedType.kind === SyntaxKind.RestType) {
sawRest = true;
}
unwrappedType = (unwrappedType as OptionalTypeNode | RestTypeNode | ParenthesizedTypeNode).type;
Copy link
Member

Choose a reason for hiding this comment

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

Nit / nagging question: any reason not to use isOptionalType and friends in the while so as to avoid this type assertion?

Copy link
Member Author

Choose a reason for hiding this comment

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

Uhh, mostly just because iirc one of them doesn't exist. Yeah, I think there isn't a isRestType and mixing and matching looked odd.

}
const updated = updateNamedTupleMember(
namedTupleMember,
namedTupleMember.dotDotDotToken || (sawRest ? createToken(SyntaxKind.DotDotDotToken) : undefined),
namedTupleMember.name,
namedTupleMember.questionToken || (sawOptional ? createToken(SyntaxKind.QuestionToken) : undefined),
unwrappedType
);
if (updated === namedTupleMember) {
return;
}
changes.replaceNode(sourceFile, namedTupleMember, updated);
}
}
2 changes: 1 addition & 1 deletion src/services/formatting/smartIndenter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ namespace ts.formatting {
return childKind !== SyntaxKind.JsxClosingFragment;
case SyntaxKind.IntersectionType:
case SyntaxKind.UnionType:
if (childKind === SyntaxKind.TypeLiteral) {
if (childKind === SyntaxKind.TypeLiteral || childKind === SyntaxKind.TupleType) {
return false;
}
// falls through
Expand Down
Loading