Skip to content
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
50 changes: 39 additions & 11 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7594,11 +7594,18 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return typeParameterToDeclarationWithConstraint(type, context, constraintNode);
}

function symbolToParameterDeclaration(parameterSymbol: Symbol, context: NodeBuilderContext, preserveModifierFlags?: boolean, privateSymbolVisitor?: (s: Symbol) => void, bundledImports?: boolean): ParameterDeclaration {
let parameterDeclaration: ParameterDeclaration | JSDocParameterTag | undefined = getDeclarationOfKind<ParameterDeclaration>(parameterSymbol, SyntaxKind.Parameter);
if (!parameterDeclaration && !isTransientSymbol(parameterSymbol)) {
parameterDeclaration = getDeclarationOfKind<JSDocParameterTag>(parameterSymbol, SyntaxKind.JSDocParameterTag);
function getEffectiveParameterDeclaration(parameterSymbol: Symbol): ParameterDeclaration | JSDocParameterTag | undefined {
const parameterDeclaration: ParameterDeclaration | JSDocParameterTag | undefined = getDeclarationOfKind<ParameterDeclaration>(parameterSymbol, SyntaxKind.Parameter);
if (parameterDeclaration) {
return parameterDeclaration;
}
if (!isTransientSymbol(parameterSymbol)) {
return getDeclarationOfKind<JSDocParameterTag>(parameterSymbol, SyntaxKind.JSDocParameterTag);
}
}

function symbolToParameterDeclaration(parameterSymbol: Symbol, context: NodeBuilderContext, preserveModifierFlags?: boolean, privateSymbolVisitor?: (s: Symbol) => void, bundledImports?: boolean): ParameterDeclaration {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to extract some useful bits to separate helpers instead of reusing symbolToParameterDeclaration. To reuse symbolToParameterDeclaration in full I'd have to introduce some extra parameters there (like preserveType, preserveQuestionToken and I don't think it's worth it. And even with that, I'd have to check if it would for bizarre setters declared using rest params and potentially put even more work into supporting that.

const parameterDeclaration = getEffectiveParameterDeclaration(parameterSymbol);

let parameterType = getTypeOfSymbol(parameterSymbol);
if (parameterDeclaration && isRequiredInitializedParameter(parameterDeclaration)) {
Expand All @@ -7609,12 +7616,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const modifiers = !(context.flags & NodeBuilderFlags.OmitParameterModifiers) && preserveModifierFlags && parameterDeclaration && canHaveModifiers(parameterDeclaration) ? map(getModifiers(parameterDeclaration), factory.cloneNode) : undefined;
const isRest = parameterDeclaration && isRestParameter(parameterDeclaration) || getCheckFlags(parameterSymbol) & CheckFlags.RestParameter;
const dotDotDotToken = isRest ? factory.createToken(SyntaxKind.DotDotDotToken) : undefined;
const name = parameterDeclaration ? parameterDeclaration.name ?
parameterDeclaration.name.kind === SyntaxKind.Identifier ? setEmitFlags(factory.cloneNode(parameterDeclaration.name), EmitFlags.NoAsciiEscaping) :
parameterDeclaration.name.kind === SyntaxKind.QualifiedName ? setEmitFlags(factory.cloneNode(parameterDeclaration.name.right), EmitFlags.NoAsciiEscaping) :
cloneBindingName(parameterDeclaration.name) :
symbolName(parameterSymbol) :
symbolName(parameterSymbol);
const name = parameterToParameterDeclarationName(parameterSymbol, parameterDeclaration, context);
const isOptional = parameterDeclaration && isOptionalParameter(parameterDeclaration) || getCheckFlags(parameterSymbol) & CheckFlags.OptionalParameter;
const questionToken = isOptional ? factory.createToken(SyntaxKind.QuestionToken) : undefined;
const parameterNode = factory.createParameterDeclaration(
Expand All @@ -7627,6 +7629,15 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
);
context.approximateLength += symbolName(parameterSymbol).length + 3;
return parameterNode;
}

function parameterToParameterDeclarationName(parameterSymbol: Symbol, parameterDeclaration: ParameterDeclaration | JSDocParameterTag | undefined, context: NodeBuilderContext) {
return parameterDeclaration ? parameterDeclaration.name ?
parameterDeclaration.name.kind === SyntaxKind.Identifier ? setEmitFlags(factory.cloneNode(parameterDeclaration.name), EmitFlags.NoAsciiEscaping) :
parameterDeclaration.name.kind === SyntaxKind.QualifiedName ? setEmitFlags(factory.cloneNode(parameterDeclaration.name.right), EmitFlags.NoAsciiEscaping) :
cloneBindingName(parameterDeclaration.name) :
symbolName(parameterSymbol) :
symbolName(parameterSymbol);

function cloneBindingName(node: BindingName): BindingName {
return elideInitializerAndSetEmitFlags(node) as BindingName;
Expand Down Expand Up @@ -9779,14 +9790,31 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (p.flags & SymbolFlags.Accessor && useAccessors) {
const result: AccessorDeclaration[] = [];
if (p.flags & SymbolFlags.SetAccessor) {
const setter = p.declarations && forEach(p.declarations, d => {
if (d.kind === SyntaxKind.SetAccessor) {
return d as SetAccessorDeclaration;
}
if (isCallExpression(d) && isBindableObjectDefinePropertyCall(d)) {
return forEach(d.arguments[2].properties, propDecl => {
const id = getNameOfDeclaration(propDecl);
if (!!id && isIdentifier(id) && idText(id) === "set") {
return propDecl;
}
});
}
});

Debug.assert(setter && isFunctionLikeDeclaration(setter));
const paramSymbol: Symbol | undefined = getSignatureFromDeclaration(setter).parameters[0];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This annotation isn't enforced, it acts as a documentation hint~. The problem is that indexed access here won't return | undefined and TypeScript prefers the expression type here over the declared type (it's a known design decision). In practice, this array might be empty and I handle it in the implementation to make the truthiness check less weird I decided to annotate this here.


result.push(setTextRange(
factory.createSetAccessorDeclaration(
factory.createModifiersFromModifierFlags(flag),
name,
[factory.createParameterDeclaration(
/*modifiers*/ undefined,
/*dotDotDotToken*/ undefined,
"arg",
paramSymbol ? parameterToParameterDeclarationName(paramSymbol, getEffectiveParameterDeclaration(paramSymbol), context) : "value",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to use "value" as a new fallback here to make it more consistent with:

if (!newValueParameter) {
newValueParameter = factory.createParameterDeclaration(
/*modifiers*/ undefined,
/*dotDotDotToken*/ undefined,
"value",
);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to use "value" as a new fallback here to make it more consistent with:

if (!newValueParameter) {
newValueParameter = factory.createParameterDeclaration(
/*modifiers*/ undefined,
/*dotDotDotToken*/ undefined,
"value",
);
}

/*questionToken*/ undefined,
isPrivate ? undefined : serializeTypeForDeclaration(context, getTypeOfSymbol(p), p, enclosingDeclaration, includePrivateSymbol, bundled),
)],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//// [tests/cases/compiler/declarationEmitClassSetAccessorParamNameInJs.ts] ////

//// [foo.js]
// https://github.com/microsoft/TypeScript/issues/55391

export class Foo {
/**
* Bar.
*
* @param {string} baz Baz.
*/
set bar(baz) {}
}




//// [foo.d.ts]
export class Foo {
/**
* Bar.
*
* @param {string} baz Baz.
*/
set bar(baz: string);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//// [tests/cases/compiler/declarationEmitClassSetAccessorParamNameInJs.ts] ////

=== foo.js ===
// https://github.com/microsoft/TypeScript/issues/55391

export class Foo {
>Foo : Symbol(Foo, Decl(foo.js, 0, 0))

/**
* Bar.
*
* @param {string} baz Baz.
*/
set bar(baz) {}
>bar : Symbol(Foo.bar, Decl(foo.js, 2, 18))
>baz : Symbol(baz, Decl(foo.js, 8, 12))
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//// [tests/cases/compiler/declarationEmitClassSetAccessorParamNameInJs.ts] ////

=== foo.js ===
// https://github.com/microsoft/TypeScript/issues/55391

export class Foo {
>Foo : Foo

/**
* Bar.
*
* @param {string} baz Baz.
*/
set bar(baz) {}
>bar : string
>baz : string
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//// [tests/cases/compiler/declarationEmitClassSetAccessorParamNameInJs2.ts] ////

//// [foo.js]
export class Foo {
/**
* Bar.
*
* @param {{ prop: string }} baz Baz.
*/
set bar({}) {}
}




//// [foo.d.ts]
export class Foo {
/**
* Bar.
*
* @param {{ prop: string }} baz Baz.
*/
set bar({}: {
prop: string;
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//// [tests/cases/compiler/declarationEmitClassSetAccessorParamNameInJs2.ts] ////

=== foo.js ===
export class Foo {
>Foo : Symbol(Foo, Decl(foo.js, 0, 0))

/**
* Bar.
*
* @param {{ prop: string }} baz Baz.
*/
set bar({}) {}
>bar : Symbol(Foo.bar, Decl(foo.js, 0, 18))
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//// [tests/cases/compiler/declarationEmitClassSetAccessorParamNameInJs2.ts] ////

=== foo.js ===
export class Foo {
>Foo : Foo

/**
* Bar.
*
* @param {{ prop: string }} baz Baz.
*/
set bar({}) {}
>bar : { prop: string; }
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//// [tests/cases/compiler/declarationEmitClassSetAccessorParamNameInJs3.ts] ////

//// [foo.js]
export class Foo {
/**
* Bar.
*
* @param {{ prop: string | undefined }} baz Baz.
*/
set bar({ prop = 'foo' }) {}
}




//// [foo.d.ts]
export class Foo {
/**
* Bar.
*
* @param {{ prop: string | undefined }} baz Baz.
*/
set bar({ prop }: {
prop: string | undefined;
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//// [tests/cases/compiler/declarationEmitClassSetAccessorParamNameInJs3.ts] ////

=== foo.js ===
export class Foo {
>Foo : Symbol(Foo, Decl(foo.js, 0, 0))

/**
* Bar.
*
* @param {{ prop: string | undefined }} baz Baz.
*/
set bar({ prop = 'foo' }) {}
>bar : Symbol(Foo.bar, Decl(foo.js, 0, 18))
>prop : Symbol(prop, Decl(foo.js, 6, 13))
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//// [tests/cases/compiler/declarationEmitClassSetAccessorParamNameInJs3.ts] ////

=== foo.js ===
export class Foo {
>Foo : Foo

/**
* Bar.
*
* @param {{ prop: string | undefined }} baz Baz.
*/
set bar({ prop = 'foo' }) {}
>bar : { prop: string | undefined; }
>prop : string
>'foo' : "foo"
}

2 changes: 1 addition & 1 deletion tests/baselines/reference/genericSetterInClassTypeJsDoc.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ new Box(3).value = 3;
declare class Box<T> {
/** @param {T} initialValue */
constructor(initialValue: T);
set value(arg: T);
set value(value: T);
/** @type {T} */
get value(): T;
#private;
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/jsDeclarationsClasses.js
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ export class E<T, U> {
/**
* @param {string} _p
*/
static set s1(arg: string);
static set s1(_p: string);
/**
* @return {string}
*/
Expand All @@ -499,7 +499,7 @@ export class E<T, U> {
/**
* @param {string} _p
*/
static set s3(arg: string);
static set s3(_p: string);
/**
* @param {T} a
* @param {U} b
Expand All @@ -518,7 +518,7 @@ export class E<T, U> {
/**
* @param {U} _p
*/
set f1(arg: U);
set f1(_p: U);
/**
* @return {U}
*/
Expand All @@ -530,7 +530,7 @@ export class E<T, U> {
/**
* @param {U} _p
*/
set f3(arg: U);
set f3(_p: U);
}
/**
* @template T,U
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,12 @@ export class Point2D {
/**
* @param {number} x
*/
set x(arg: number);
set x(x: number);
get x(): number;
/**
* @param {number} y
*/
set y(arg: number);
set y(y: number);
get y(): number;
__proto__: typeof Vec;
}
Expand Down
Loading