Skip to content

Add JSDocFunctionTypeParameter node kind#18213

Closed
ghost wants to merge 1 commit into
masterfrom
jsdoc_parsing
Closed

Add JSDocFunctionTypeParameter node kind#18213
ghost wants to merge 1 commit into
masterfrom
jsdoc_parsing

Conversation

@ghost
Copy link
Copy Markdown

@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
Copy Markdown
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.

Comment thread src/compiler/types.ts
type: TypeNode;
}

export const enum JSDocFunctionTypeParameterSort {
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

Comment thread src/compiler/types.ts
export interface JSDocFunctionTypeParameterDeclaration extends Declaration {
kind: SyntaxKind.JSDocFunctionTypeParameter;
parent: JSDocFunctionType;
sort: JSDocFunctionTypeParameterSort;
Copy link
Copy Markdown
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
Copy Markdown
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.

Comment thread src/compiler/binder.ts
return declareSymbolAndAddToSymbolTable(<Declaration>node, SymbolFlags.TypeParameter, SymbolFlags.TypeParameterExcludes);
case SyntaxKind.Parameter:
return bindParameter(<ParameterDeclaration>node);
case SyntaxKind.JSDocFunctionTypeParameter:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

indentation

Comment thread src/compiler/binder.ts
case JSDocFunctionTypeParameterSort.New:
return "new" as __String;
case JSDocFunctionTypeParameterSort.This:
return "this" as __String;
Copy link
Copy Markdown
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.

Comment thread src/compiler/types.ts
JSDocNonNullableType,
JSDocOptionalType,
JSDocFunctionType,
JSDocFunctionTypeParameter,
Copy link
Copy Markdown
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

Comment thread src/compiler/checker.ts
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
Copy Markdown
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.

Comment thread src/compiler/types.ts
}

export interface JSDocFunctionType extends JSDocType, SignatureDeclaration {
export interface JSDocFunctionType extends JSDocType, Declaration {
Copy link
Copy Markdown
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.

Comment thread src/compiler/types.ts
This,
New,
}
export interface JSDocFunctionTypeParameterDeclaration extends Declaration {
Copy link
Copy Markdown
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
Copy Markdown
Contributor

mhegazy commented Nov 8, 2017

@Andy-MS is this still needed?

@mhegazy
Copy link
Copy Markdown
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