-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Support naming tuple members #38234
Changes from all commits
086e01f
4c50574
c7e115a
6b754a9
ac57f0a
f0cf8d4
71b3f04
fcb324d
cc8d392
71a587f
a7aa566
e3a1cb1
9adfb14
05d398e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -328,6 +328,7 @@ namespace ts { | |
IndexedAccessType, | ||
MappedType, | ||
LiteralType, | ||
NamedTupleMember, | ||
ImportType, | ||
// Binding patterns | ||
ObjectBindingPattern, | ||
|
@@ -700,6 +701,7 @@ namespace ts { | |
| ConstructorTypeNode | ||
| JSDocFunctionType | ||
| ExportDeclaration | ||
| NamedTupleMember | ||
| EndOfFileToken; | ||
|
||
export type HasType = | ||
|
@@ -1274,7 +1276,15 @@ namespace ts { | |
|
||
export interface TupleTypeNode extends TypeNode { | ||
kind: SyntaxKind.TupleType; | ||
elementTypes: NodeArray<TypeNode>; | ||
elements: NodeArray<TypeNode | NamedTupleMember>; | ||
} | ||
|
||
export interface NamedTupleMember extends TypeNode, JSDocContainer, Declaration { | ||
kind: SyntaxKind.NamedTupleMember; | ||
dotDotDotToken?: Token<SyntaxKind.DotDotDotToken>; | ||
name: Identifier; | ||
questionToken?: Token<SyntaxKind.QuestionToken>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
type: TypeNode; | ||
} | ||
|
||
export interface OptionalTypeNode extends TypeNode { | ||
|
@@ -1478,6 +1488,7 @@ namespace ts { | |
kind: SyntaxKind.SyntheticExpression; | ||
isSpread: boolean; | ||
type: Type; | ||
tupleNameSource?: ParameterDeclaration | NamedTupleMember; | ||
} | ||
|
||
// see: https://tc39.github.io/ecma262/#prod-ExponentiationExpression | ||
|
@@ -2590,6 +2601,7 @@ namespace ts { | |
text: string; | ||
pos: -1; | ||
end: -1; | ||
hasLeadingNewline?: boolean; | ||
} | ||
|
||
// represents a top level: { type } expression in a JSDoc comment. | ||
|
@@ -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; | ||
|
@@ -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 */ | ||
|
@@ -4618,7 +4631,7 @@ namespace ts { | |
minLength: number; | ||
hasRestElement: boolean; | ||
readonly: boolean; | ||
associatedNames?: __String[]; | ||
labeledElementDeclarations?: readonly (NamedTupleMember | ParameterDeclaration)[]; | ||
} | ||
|
||
export interface TupleTypeReference extends TypeReference { | ||
|
@@ -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, | ||
|
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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit / nagging question: any reason not to use There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
} | ||
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); | ||
} | ||
} |
There was a problem hiding this comment.
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
aTypeNode
, which is inline withOptionalType
andRestType
, 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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the rename.
There was a problem hiding this comment.
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