Skip to content

Add JSDocFunctionTypeParameter node kind #18213

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

Closed
wants to merge 1 commit into from
Closed

Add JSDocFunctionTypeParameter node kind #18213

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 1, 2017

Fixes #17403
In order to keep ParameterDeclaration.name defined, we should stop reusing it for nodes that do not have names. A parameter in function(number, number): string
(Ref: #17074, #17326)

@ghost ghost requested a review from sandersn September 1, 2017 21:22
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

The main advantage of this change is that it's impossible to say JSDocFunctionTypeParameter.name now, right? The code bloats more than I expected with this change, so unless this makes using parameters more fool-proof, then I don't think it's worth it.

type: TypeNode;
}

export const enum JSDocFunctionTypeParameterSort {
Copy link
Member

Choose a reason for hiding this comment

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

-Kind is a better suffix than -Sort here.

Copy link
Member

Choose a reason for hiding this comment

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

Even better: name: JSDocParameterSpecialName, with Regular renamed to None.

Copy link
Member

Choose a reason for hiding this comment

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

or specialName so that it isn't structurally compatible with other named things.

export interface JSDocFunctionTypeParameterDeclaration extends Declaration {
kind: SyntaxKind.JSDocFunctionTypeParameter;
parent: JSDocFunctionType;
sort: JSDocFunctionTypeParameterSort;
Copy link
Member

Choose a reason for hiding this comment

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

jsdocKind or parameterKind or jsdocParameterKind.

Copy link
Member

Choose a reason for hiding this comment

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

even better, just name or maybe nameKind.

@@ -2047,6 +2050,8 @@ namespace ts {
return declareSymbolAndAddToSymbolTable(<Declaration>node, SymbolFlags.TypeParameter, SymbolFlags.TypeParameterExcludes);
case SyntaxKind.Parameter:
return bindParameter(<ParameterDeclaration>node);
case SyntaxKind.JSDocFunctionTypeParameter:
Copy link
Member

Choose a reason for hiding this comment

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

indentation

case JSDocFunctionTypeParameterSort.New:
return "new" as __String;
case JSDocFunctionTypeParameterSort.This:
return "this" as __String;
Copy link
Member

Choose a reason for hiding this comment

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

I don't think these names matter since won't be used later.

@@ -360,6 +360,7 @@ namespace ts {
JSDocNonNullableType,
JSDocOptionalType,
JSDocFunctionType,
JSDocFunctionTypeParameter,
Copy link
Member

Choose a reason for hiding this comment

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

JSDocParameter is a better name

@@ -6423,14 +6427,14 @@ namespace ts {
const classType = declaration.kind === SyntaxKind.Constructor ?
getDeclaredTypeOfClassOrInterface(getMergedSymbol((<ClassDeclaration>declaration.parent).symbol))
: undefined;
const typeParameters = classType ? classType.localTypeParameters : getTypeParametersFromDeclaration(declaration);
const typeParameters = classType ? classType.localTypeParameters : declaration.kind === SyntaxKind.JSDocFunctionType ? emptyArray : getTypeParametersFromDeclaration(declaration);
Copy link
Member

Choose a reason for hiding this comment

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

seems like getTypeParametersFromDeclaration could handle jsdoc functions too, since it calls getEffectiveTypeParameters to provide precisely that level of abstraction.

@@ -2085,8 +2086,23 @@ namespace ts {
type: TypeNode;
}

export interface JSDocFunctionType extends JSDocType, SignatureDeclaration {
export interface JSDocFunctionType extends JSDocType, Declaration {
Copy link
Member

Choose a reason for hiding this comment

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

need to remove SyntaxKind.JSDocFunctionType from SignatureDeclaration.kind.

This,
New,
}
export interface JSDocFunctionTypeParameterDeclaration extends Declaration {
Copy link
Member

Choose a reason for hiding this comment

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

similarly, rename to JSDocParameterDeclaration

@mhegazy
Copy link
Contributor

mhegazy commented Nov 8, 2017

@Andy-MS is this still needed?

@mhegazy
Copy link
Contributor

mhegazy commented Jan 8, 2018

Does not seem to be needed. closing for now. please reopen if that is not the case.

@mhegazy mhegazy closed this Jan 8, 2018
@ghost ghost deleted the jsdoc_parsing branch January 8, 2018 18:41
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants