Skip to content

Template tag allows specification of constraints #24600

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

Merged
merged 13 commits into from
Jun 4, 2018
6 changes: 3 additions & 3 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28416,9 +28416,9 @@ namespace ts {
}

function checkGrammarConstructorTypeParameters(node: ConstructorDeclaration) {
const typeParameters = getEffectiveTypeParameterDeclarations(node);
if (isNodeArray(typeParameters)) {
const { pos, end } = typeParameters;
const jsdocTypeParameters = isInJavaScriptFile(node) && getJSDocTypeParameterDeclarations(node);
if (node.typeParameters || jsdocTypeParameters && jsdocTypeParameters.length) {
const { pos, end } = node.typeParameters || jsdocTypeParameters && jsdocTypeParameters[0] || node;
return grammarErrorAtPos(node, pos, end - pos, Diagnostics.Type_parameters_cannot_appear_on_a_constructor_declaration);
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,10 @@
"category": "Error",
"code": 1068
},
"Unexpected token. A type parameter name was expected without curly braces.": {
"category": "Error",
"code": 1069
},
"'{0}' modifier cannot appear on a type member.": {
"category": "Error",
"code": 1070
Expand Down
35 changes: 14 additions & 21 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7013,29 +7013,27 @@ namespace ts {
}

function parseTemplateTag(atToken: AtToken, tagName: Identifier): JSDocTemplateTag | undefined {
if (some(tags, isJSDocTemplateTag)) {
parseErrorAt(tagName.pos, scanner.getTokenPos(), Diagnostics._0_tag_already_specified, tagName.escapedText);
// the template tag looks like '@template {Constraint} T,U,V'
let constraint: JSDocTypeExpression | undefined;
if (token() === SyntaxKind.OpenBraceToken) {
constraint = parseJSDocTypeExpression();
skipWhitespace();
}

// Type parameter list looks like '@template T,U,V'
const typeParameters = [];
const typeParametersPos = getNodePos();

while (true) {
const typeParameter = <TypeParameterDeclaration>createNode(SyntaxKind.TypeParameter);
const name = parseJSDocIdentifierNameWithOptionalBraces();
skipWhitespace();
if (!name) {
parseErrorAtPosition(scanner.getStartPos(), 0, Diagnostics.Identifier_expected);
if (!tokenIsIdentifierOrKeyword(token())) {
parseErrorAtCurrentToken(Diagnostics.Unexpected_token_A_type_parameter_name_was_expected_without_curly_braces);
return undefined;
Copy link

Choose a reason for hiding this comment

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

Was already this way, but it looks like parseTemplateTag is the only tag-parsing method out of 9 that can return undefined. Might be better to call parseJSDocIdentifierName with createIfMissing set, and a new parameter to allow a custom diagnostic. Than parseTag could return JSDocTag (without | undefined).

Copy link
Member Author

Choose a reason for hiding this comment

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

parseTag returns undefined near the beginning if it can't parseJSDocIdentifierName, too. As-is the change is not worth it in my opinion. It might be worthwhile to have parseJSDocIdentifierName always return a missing node. I tried that and it looks a bit better, but there's a good bit of churn. Would you like to review it as part of this PR or would you like to see it in a separate one?

Copy link

Choose a reason for hiding this comment

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

Separate is good.

}

typeParameter.name = name;
typeParameter.name = parseJSDocIdentifierName()!;
skipWhitespace();
finishNode(typeParameter);

typeParameters.push(typeParameter);

if (token() === SyntaxKind.CommaToken) {
// need to look for more type parameters
nextJSDocToken();
skipWhitespace();
}
Expand All @@ -7044,6 +7042,10 @@ namespace ts {
}
}

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

const result = <JSDocTemplateTag>createNode(SyntaxKind.JSDocTemplateTag, atToken.pos);
result.atToken = atToken;
result.tagName = tagName;
Expand All @@ -7052,15 +7054,6 @@ namespace ts {
return result;
}

function parseJSDocIdentifierNameWithOptionalBraces(): Identifier | undefined {
const parsedBrace = parseOptional(SyntaxKind.OpenBraceToken);
const res = parseJSDocIdentifierName();
if (parsedBrace) {
parseExpected(SyntaxKind.CloseBraceToken);
}
return res;
}

function nextJSDocToken(): JsDocSyntaxKind {
return currentToken = scanner.scanJSDocToken();
}
Expand Down
17 changes: 7 additions & 10 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3166,21 +3166,18 @@ namespace ts {
}
if (isJSDocTypeAlias(node)) {
Debug.assert(node.parent.kind === SyntaxKind.JSDocComment);
const templateTags = flatMap(filter(node.parent.tags, isJSDocTemplateTag), tag => tag.typeParameters) as ReadonlyArray<TypeParameterDeclaration>;
const templateTagNodes = templateTags as NodeArray<TypeParameterDeclaration>;
templateTagNodes.pos = templateTagNodes.length > 0 ? first(templateTagNodes).pos : node.pos;
templateTagNodes.end = templateTagNodes.length > 0 ? last(templateTagNodes).end : node.end;
templateTagNodes.hasTrailingComma = false;
return templateTagNodes;
return flatMap(node.parent.tags, tag => isJSDocTemplateTag(tag) ? tag.typeParameters : undefined) as ReadonlyArray<TypeParameterDeclaration>;
}
return node.typeParameters || (isInJavaScriptFile(node) ? getJSDocTypeParameterDeclarations(node) : emptyArray);
}

export function getJSDocTypeParameterDeclarations(node: DeclarationWithTypeParameters): ReadonlyArray<TypeParameterDeclaration> {
// template tags are only available when a typedef isn't already using them
const tag = find(getJSDocTags(node), (tag): tag is JSDocTemplateTag =>
isJSDocTemplateTag(tag) && !(tag.parent.kind === SyntaxKind.JSDocComment && tag.parent.tags!.some(isJSDocTypeAlias)));
return (tag && tag.typeParameters) || emptyArray;
return flatMap(getJSDocTags(node), tag => isNonTypeAliasTemplate(tag) ? tag.typeParameters : undefined);
}

/** template tags are only available when a typedef isn't already using them */
function isNonTypeAliasTemplate(tag: JSDocTag): tag is JSDocTemplateTag {
return isJSDocTemplateTag(tag) && !(tag.parent.kind === SyntaxKind.JSDocComment && tag.parent.tags!.some(isJSDocTypeAlias));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/jsdocTemplateClass.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ tests/cases/conformance/jsdoc/templateTagOnClasses.js(25,1): error TS2322: Type

==== tests/cases/conformance/jsdoc/templateTagOnClasses.js (1 errors) ====
/**
* @template {T}
* @template T
* @typedef {(t: T) => T} Id
*/
/** @template T */
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/jsdocTemplateClass.symbols
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
=== tests/cases/conformance/jsdoc/templateTagOnClasses.js ===
/**
* @template {T}
* @template T
* @typedef {(t: T) => T} Id
*/
/** @template T */
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/jsdocTemplateClass.types
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
=== tests/cases/conformance/jsdoc/templateTagOnClasses.js ===
/**
* @template {T}
* @template T
* @typedef {(t: T) => T} Id
*/
/** @template T */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ tests/cases/conformance/jsdoc/templateTagOnConstructorFunctions.js(24,1): error

==== tests/cases/conformance/jsdoc/templateTagOnConstructorFunctions.js (1 errors) ====
/**
* @template {U}
* @template U
* @typedef {(u: U) => U} Id
*/
/**
* @param {T} t
* @template {T}
* @template T
*/
function Zet(t) {
/** @type {T} */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
=== tests/cases/conformance/jsdoc/templateTagOnConstructorFunctions.js ===
/**
* @template {U}
* @template U
* @typedef {(u: U) => U} Id
*/
/**
* @param {T} t
* @template {T}
* @template T
*/
function Zet(t) {
>Zet : Symbol(Zet, Decl(templateTagOnConstructorFunctions.js, 0, 0))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
=== tests/cases/conformance/jsdoc/templateTagOnConstructorFunctions.js ===
/**
* @template {U}
* @template U
* @typedef {(u: U) => U} Id
*/
/**
* @param {T} t
* @template {T}
* @template T
*/
function Zet(t) {
>Zet : typeof Zet
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ tests/cases/conformance/jsdoc/templateTagWithNestedTypeLiteral.js(26,15): error
==== tests/cases/conformance/jsdoc/templateTagWithNestedTypeLiteral.js (2 errors) ====
/**
* @param {T} t
* @template {T}
* @template T
*/
function Zet(t) {
/** @type {T} */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
=== tests/cases/conformance/jsdoc/templateTagWithNestedTypeLiteral.js ===
/**
* @param {T} t
* @template {T}
* @template T
*/
function Zet(t) {
>Zet : Symbol(Zet, Decl(templateTagWithNestedTypeLiteral.js, 0, 0))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
=== tests/cases/conformance/jsdoc/templateTagWithNestedTypeLiteral.js ===
/**
* @param {T} t
* @template {T}
* @template T
*/
function Zet(t) {
>Zet : typeof Zet
Expand Down
47 changes: 47 additions & 0 deletions tests/baselines/reference/jsdocTemplateTag3.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
tests/cases/conformance/jsdoc/a.js(14,29): error TS2339: Property 'a' does not exist on type 'U'.
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(25,2): error TS1069: Unexpected token. A type parameter name was expected without curly braces.


==== tests/cases/conformance/jsdoc/a.js (4 errors) ====
/**
* @template {{ a: number, b: string }} T,U A Comment
* @template {{ c: boolean }} V uh ... are comments even supported??
* @template W
* @template X That last one had no comment
* @param {T} t
* @param {U} u
* @param {V} v
* @param {W} w
* @param {X} x
* @return {W | X}
*/
function f(t, u, v, w, x) {
if(t.a + t.b.length > u.a - u.b.length && v.c) {
~
!!! error TS2339: Property 'a' does not exist on type 'U'.
~
!!! error TS2339: Property 'b' does not exist on type 'U'.
return w;
}
return x;
}

f({ a: 12, b: 'hi', c: null }, undefined, { c: false, d: 12, b: undefined }, 101, 'nope');
f({ a: 12 }, undefined, undefined, 101, 'nope');
~~~~~~~~~~
!!! error TS2345: Argument of type '{ a: number; }' is not assignable to parameter of type '{ a: number; b: string; }'.
!!! error TS2345: Property 'b' is missing in type '{ a: number; }'.

/**
* @template {NoLongerAllowed}
* @template T preceding line's syntax is no longer allowed
~
!!! error TS1069: Unexpected token. A type parameter name was expected without curly braces.
* @param {T} x
*/
function g(x) { }


70 changes: 70 additions & 0 deletions tests/baselines/reference/jsdocTemplateTag3.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
=== tests/cases/conformance/jsdoc/a.js ===
/**
* @template {{ a: number, b: string }} T,U A Comment
* @template {{ c: boolean }} V uh ... are comments even supported??
* @template W
* @template X That last one had no comment
* @param {T} t
* @param {U} u
* @param {V} v
* @param {W} w
* @param {X} x
* @return {W | X}
*/
function f(t, u, v, w, x) {
>f : Symbol(f, Decl(a.js, 0, 0))
>t : Symbol(t, Decl(a.js, 12, 11))
>u : Symbol(u, Decl(a.js, 12, 13))
>v : Symbol(v, Decl(a.js, 12, 16))
>w : Symbol(w, Decl(a.js, 12, 19))
>x : Symbol(x, Decl(a.js, 12, 22))

if(t.a + t.b.length > u.a - u.b.length && v.c) {
>t.a : Symbol(a, Decl(a.js, 1, 15))
>t : Symbol(t, Decl(a.js, 12, 11))
>a : Symbol(a, Decl(a.js, 1, 15))
>t.b.length : Symbol(String.length, Decl(lib.d.ts, --, --))
>t.b : Symbol(b, Decl(a.js, 1, 26))
>t : Symbol(t, Decl(a.js, 12, 11))
>b : Symbol(b, Decl(a.js, 1, 26))
>length : Symbol(String.length, Decl(lib.d.ts, --, --))
>u : Symbol(u, Decl(a.js, 12, 13))
>u : Symbol(u, Decl(a.js, 12, 13))
>v.c : Symbol(c, Decl(a.js, 2, 15))
>v : Symbol(v, Decl(a.js, 12, 16))
>c : Symbol(c, Decl(a.js, 2, 15))

return w;
>w : Symbol(w, Decl(a.js, 12, 19))
}
return x;
>x : Symbol(x, Decl(a.js, 12, 22))
}

f({ a: 12, b: 'hi', c: null }, undefined, { c: false, d: 12, b: undefined }, 101, 'nope');
>f : Symbol(f, Decl(a.js, 0, 0))
>a : Symbol(a, Decl(a.js, 19, 3))
>b : Symbol(b, Decl(a.js, 19, 10))
>c : Symbol(c, Decl(a.js, 19, 19))
>undefined : Symbol(undefined)
>c : Symbol(c, Decl(a.js, 19, 43))
>d : Symbol(d, Decl(a.js, 19, 53))
>b : Symbol(b, Decl(a.js, 19, 60))
>undefined : Symbol(undefined)

f({ a: 12 }, undefined, undefined, 101, 'nope');
>f : Symbol(f, Decl(a.js, 0, 0))
>a : Symbol(a, Decl(a.js, 20, 3))
>undefined : Symbol(undefined)
>undefined : Symbol(undefined)

/**
* @template {NoLongerAllowed}
* @template T preceding line's syntax is no longer allowed
* @param {T} x
*/
function g(x) { }
>g : Symbol(g, Decl(a.js, 20, 49))
>x : Symbol(x, Decl(a.js, 27, 11))


Loading