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

Don't store @template constraint in a TypeParameterDeclaration node #26283

Merged
3 commits merged into from
Aug 10, 2018
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
27 changes: 20 additions & 7 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5948,7 +5948,8 @@ namespace ts {

/** A type parameter is thisless if its contraint is thisless, or if it has no constraint. */
function isThislessTypeParameter(node: TypeParameterDeclaration) {
return !node.constraint || isThislessType(node.constraint);
const constraint = getEffectiveConstraintOfTypeParameter(node);
return !constraint || isThislessType(constraint);
}

/**
Expand Down Expand Up @@ -6735,7 +6736,7 @@ namespace ts {
}

function getConstraintDeclarationForMappedType(type: MappedType) {
return type.declaration.typeParameter.constraint;
return getEffectiveConstraintOfTypeParameter(type.declaration.typeParameter);
}

function isMappedTypeWithKeyofConstraintDeclaration(type: MappedType) {
Expand Down Expand Up @@ -7872,7 +7873,7 @@ namespace ts {

function getConstraintDeclaration(type: TypeParameter) {
const decl = type.symbol && getDeclarationOfKind<TypeParameterDeclaration>(type.symbol, SyntaxKind.TypeParameter);
return decl && decl.constraint;
return decl && getEffectiveConstraintOfTypeParameter(decl);
}

function getInferredTypeParameterConstraint(typeParameter: TypeParameter) {
Expand Down Expand Up @@ -7936,7 +7937,9 @@ namespace ts {
}

function getParentSymbolOfTypeParameter(typeParameter: TypeParameter): Symbol | undefined {
return getSymbolOfNode(getDeclarationOfKind(typeParameter.symbol, SyntaxKind.TypeParameter)!.parent);
const tp = getDeclarationOfKind<TypeParameterDeclaration>(typeParameter.symbol, SyntaxKind.TypeParameter)!;
const host = isJSDocTemplateTag(tp.parent) ? getHostSignatureFromJSDoc(tp.parent) : tp.parent;
return host && getSymbolOfNode(host);
}

function getTypeListId(types: ReadonlyArray<Type> | undefined) {
Expand Down Expand Up @@ -22016,7 +22019,7 @@ namespace ts {
checkSourceElement(node.default);
const typeParameter = getDeclaredTypeOfTypeParameter(getSymbolOfNode(node));
if (!hasNonCircularBaseConstraint(typeParameter)) {
error(node.constraint, Diagnostics.Type_parameter_0_has_a_circular_constraint, typeToString(typeParameter));
error(getEffectiveConstraintOfTypeParameter(node), Diagnostics.Type_parameter_0_has_a_circular_constraint, typeToString(typeParameter));
}
if (!hasNonCircularTypeParameterDefault(typeParameter)) {
error(node.default, Diagnostics.Type_parameter_0_has_a_circular_default, typeToString(typeParameter));
Expand Down Expand Up @@ -22749,7 +22752,7 @@ namespace ts {

const type = <MappedType>getTypeFromMappedTypeNode(node);
const constraintType = getConstraintTypeFromMappedType(type);
checkTypeAssignableTo(constraintType, keyofConstraintType, node.typeParameter.constraint);
checkTypeAssignableTo(constraintType, keyofConstraintType, getEffectiveConstraintOfTypeParameter(node.typeParameter));
}

function checkThisType(node: ThisTypeNode) {
Expand Down Expand Up @@ -23640,6 +23643,13 @@ namespace ts {
checkSourceElement(node.typeExpression);
}

function checkJSDocTemplateTag(node: JSDocTemplateTag): void {
checkSourceElement(node.constraint);
for (const tp of node.typeParameters) {
checkSourceElement(tp);
}
}

function checkJSDocTypeTag(node: JSDocTypeTag) {
checkSourceElement(node.typeExpression);
}
Expand Down Expand Up @@ -25427,7 +25437,8 @@ namespace ts {

// If the type parameter node does not have an identical constraint as the resolved
// type parameter at this position, we report an error.
const sourceConstraint = source.constraint && getTypeFromTypeNode(source.constraint);
const constraint = getEffectiveConstraintOfTypeParameter(source);
const sourceConstraint = constraint && getTypeFromTypeNode(constraint);
const targetConstraint = getConstraintOfTypeParameter(target);
if (sourceConstraint) {
// relax check if later interface augmentation has no constraint
Expand Down Expand Up @@ -26647,6 +26658,8 @@ namespace ts {
case SyntaxKind.JSDocTypedefTag:
case SyntaxKind.JSDocCallbackTag:
return checkJSDocTypeAliasTag(node as JSDocTypedefTag);
case SyntaxKind.JSDocTemplateTag:
return checkJSDocTemplateTag(node as JSDocTemplateTag);
case SyntaxKind.JSDocTypeTag:
return checkJSDocTypeTag(node as JSDocTypeTag);
case SyntaxKind.JSDocParameterTag:
Expand Down
7 changes: 2 additions & 5 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ namespace ts {
case SyntaxKind.JSDocAugmentsTag:
return visitNode(cbNode, (<JSDocAugmentsTag>node).class);
case SyntaxKind.JSDocTemplateTag:
return visitNodes(cbNode, cbNodes, (<JSDocTemplateTag>node).typeParameters);
return visitNode(cbNode, (<JSDocTemplateTag>node).constraint) || visitNodes(cbNode, cbNodes, (<JSDocTemplateTag>node).typeParameters);
case SyntaxKind.JSDocTypedefTag:
if ((node as JSDocTypedefTag).typeExpression &&
(node as JSDocTypedefTag).typeExpression!.kind === SyntaxKind.JSDocTypeExpression) {
Expand Down Expand Up @@ -7049,13 +7049,10 @@ namespace ts {
typeParameters.push(typeParameter);
} while (parseOptionalJsdoc(SyntaxKind.CommaToken));

if (constraint) {
first(typeParameters).constraint = constraint.type;
}

const result = <JSDocTemplateTag>createNode(SyntaxKind.JSDocTemplateTag, atToken.pos);
result.atToken = atToken;
result.tagName = tagName;
result.constraint = constraint;
result.typeParameters = createNodeArray(typeParameters, typeParametersPos);
finishNode(result);
return result;
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,7 @@ namespace ts {
kind: SyntaxKind.TypeParameter;
parent: DeclarationWithTypeParameterChildren | InferTypeNode;
name: Identifier;
/** Note: Consider calling `getEffectiveConstraintOfTypeParameter` */
constraint?: TypeNode;
default?: TypeNode;

Expand Down Expand Up @@ -2363,6 +2364,7 @@ namespace ts {

export interface JSDocTemplateTag extends JSDocTag {
kind: SyntaxKind.JSDocTemplateTag;
constraint: TypeNode | undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Only the first type parameter in a list is constrained by the constraint. Can you add a test for that? That is, /** @template {number} T,U */ only constrains T.

Copy link
Author

Choose a reason for hiding this comment

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

That was done in #24600 for the reason that reusing a node would be bad -- maybe not necessary now?

Copy link
Author

Choose a reason for hiding this comment

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

The test jsdocTemplateTag3, added in #24600, will have fewer errors if this is changed, so it's already being tested.

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 like applying the constraint to multiple parameters even if there's no implementation reason that it's easier.

typeParameters: NodeArray<TypeParameterDeclaration>;
}

Expand Down
9 changes: 9 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,8 @@ namespace ts {
return !isExpressionWithTypeArgumentsInClassExtendsClause(parent);
case SyntaxKind.TypeParameter:
return node === (<TypeParameterDeclaration>parent).constraint;
case SyntaxKind.JSDocTemplateTag:
return node === (<JSDocTemplateTag>parent).constraint;
case SyntaxKind.PropertyDeclaration:
case SyntaxKind.PropertySignature:
case SyntaxKind.Parameter:
Expand Down Expand Up @@ -5104,6 +5106,13 @@ namespace ts {
}
return node.typeParameters || (isInJavaScriptFile(node) ? getJSDocTypeParameterDeclarations(node) : emptyArray);
}

export function getEffectiveConstraintOfTypeParameter(node: TypeParameterDeclaration): TypeNode | undefined {
return node.constraint ? node.constraint
: isJSDocTemplateTag(node.parent) && node === node.parent.typeParameters[0]
? node.parent.constraint
: undefined;
}
}

// Simple node tests of the form `node.kind === SyntaxKind.Foo`.
Expand Down
3 changes: 3 additions & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,7 @@ declare namespace ts {
kind: SyntaxKind.TypeParameter;
parent: DeclarationWithTypeParameterChildren | InferTypeNode;
name: Identifier;
/** Note: Consider calling `getEffectiveConstraintOfTypeParameter` */
constraint?: TypeNode;
default?: TypeNode;
expression?: Expression;
Expand Down Expand Up @@ -1568,6 +1569,7 @@ declare namespace ts {
}
interface JSDocTemplateTag extends JSDocTag {
kind: SyntaxKind.JSDocTemplateTag;
constraint: TypeNode | undefined;
typeParameters: NodeArray<TypeParameterDeclaration>;
}
interface JSDocReturnTag extends JSDocTag {
Expand Down Expand Up @@ -3267,6 +3269,7 @@ declare namespace ts {
* JavaScript file, gets the type parameters from the `@template` tag from JSDoc.
*/
function getEffectiveTypeParameterDeclarations(node: DeclarationWithTypeParameters): ReadonlyArray<TypeParameterDeclaration>;
function getEffectiveConstraintOfTypeParameter(node: TypeParameterDeclaration): TypeNode | undefined;
}
declare namespace ts {
function isNumericLiteral(node: Node): node is NumericLiteral;
Expand Down
3 changes: 3 additions & 0 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,7 @@ declare namespace ts {
kind: SyntaxKind.TypeParameter;
parent: DeclarationWithTypeParameterChildren | InferTypeNode;
name: Identifier;
/** Note: Consider calling `getEffectiveConstraintOfTypeParameter` */
constraint?: TypeNode;
default?: TypeNode;
expression?: Expression;
Expand Down Expand Up @@ -1568,6 +1569,7 @@ declare namespace ts {
}
interface JSDocTemplateTag extends JSDocTag {
kind: SyntaxKind.JSDocTemplateTag;
constraint: TypeNode | undefined;
typeParameters: NodeArray<TypeParameterDeclaration>;
}
interface JSDocReturnTag extends JSDocTag {
Expand Down Expand Up @@ -3267,6 +3269,7 @@ declare namespace ts {
* JavaScript file, gets the type parameters from the `@template` tag from JSDoc.
*/
function getEffectiveTypeParameterDeclarations(node: DeclarationWithTypeParameters): ReadonlyArray<TypeParameterDeclaration>;
function getEffectiveConstraintOfTypeParameter(node: TypeParameterDeclaration): TypeNode | undefined;
}
declare namespace ts {
function isNumericLiteral(node: Node): node is NumericLiteral;
Expand Down
5 changes: 4 additions & 1 deletion tests/baselines/reference/jsdocTemplateTag3.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ tests/cases/conformance/jsdoc/a.js(14,29): error TS2339: Property 'a' does not e
tests/cases/conformance/jsdoc/a.js(14,35): error TS2339: Property 'b' does not exist on type 'U'.
tests/cases/conformance/jsdoc/a.js(21,3): error TS2345: Argument of type '{ a: number; }' is not assignable to parameter of type '{ a: number; b: string; }'.
Property 'b' is missing in type '{ a: number; }'.
tests/cases/conformance/jsdoc/a.js(24,15): error TS2304: Cannot find name 'NoLongerAllowed'.
tests/cases/conformance/jsdoc/a.js(25,2): error TS1069: Unexpected token. A type parameter name was expected without curly braces.


==== tests/cases/conformance/jsdoc/a.js (4 errors) ====
==== tests/cases/conformance/jsdoc/a.js (5 errors) ====
/**
* @template {{ a: number, b: string }} T,U A Comment
* @template {{ c: boolean }} V uh ... are comments even supported??
Expand Down Expand Up @@ -37,6 +38,8 @@ tests/cases/conformance/jsdoc/a.js(25,2): error TS1069: Unexpected token. A type

/**
* @template {NoLongerAllowed}
~~~~~~~~~~~~~~~
!!! error TS2304: Cannot find name 'NoLongerAllowed'.
* @template T preceding line's syntax is no longer allowed
~
!!! error TS1069: Unexpected token. A type parameter name was expected without curly braces.
Expand Down
10 changes: 10 additions & 0 deletions tests/cases/fourslash/editTemplateConstraint.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/// <reference path='fourslash.ts' />

/////**
//// * @template {/**/
//// */
////function f() {}

goTo.marker("");
edit.insert("n");
edit.insert("u");
21 changes: 21 additions & 0 deletions tests/cases/fourslash/quickInfoTemplateTag.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/// <reference path='fourslash.ts'/>

// @allowJs: true
// @checkJs: true
// @Filename: /foo.js

/////**
//// * Doc
//// * @template {new (...args: any[]) => any} T
//// * @param {T} cls
//// */
////function /**/myMixin(cls) {
//// return class extends cls {}
////}

verify.quickInfoAt("",
`function myMixin<T extends new (...args: any[]) => any>(cls: T): {
new (...args: any[]): (Anonymous class);
prototype: myMixin<any>.(Anonymous class);
} & T`,
"Doc");