Skip to content

Fixes to @augments handling #18775

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
4 commits merged into from
Sep 28, 2017
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
56 changes: 45 additions & 11 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4882,7 +4882,16 @@ namespace ts {
}

function getBaseTypeNodeOfClass(type: InterfaceType): ExpressionWithTypeArguments {
return getClassExtendsHeritageClauseElement(<ClassLikeDeclaration>type.symbol.valueDeclaration);
const decl = <ClassLikeDeclaration>type.symbol.valueDeclaration;
if (isInJavaScriptFile(decl)) {
// Prefer an @augments tag because it may have type parameters.
const tag = getJSDocAugmentsTag(decl);
if (tag) {
return tag.class;
}
}

return getClassExtendsHeritageClauseElement(decl);
}

function getConstructorsForTypeArguments(type: Type, typeArgumentNodes: ReadonlyArray<TypeNode>, location: Node): Signature[] {
Expand Down Expand Up @@ -4986,15 +4995,6 @@ namespace ts {
baseType = getReturnTypeOfSignature(constructors[0]);
}

// In a JS file, you can use the @augments jsdoc tag to specify a base type with type parameters
const valueDecl = type.symbol.valueDeclaration;
if (valueDecl && isInJavaScriptFile(valueDecl)) {
const augTag = getJSDocAugmentsTag(type.symbol.valueDeclaration);
if (augTag && augTag.typeExpression && augTag.typeExpression.type) {
baseType = getTypeFromTypeNode(augTag.typeExpression.type);
}
}

if (baseType === unknownType) {
return;
}
Expand All @@ -5003,7 +5003,7 @@ namespace ts {
return;
}
if (type === baseType || hasBaseType(baseType, type)) {
error(valueDecl, Diagnostics.Type_0_recursively_references_itself_as_a_base_type,
error(type.symbol.valueDeclaration, Diagnostics.Type_0_recursively_references_itself_as_a_base_type,
typeToString(type, /*enclosingDeclaration*/ undefined, TypeFormatFlags.WriteArrayAsGenericType));
return;
}
Expand Down Expand Up @@ -19789,6 +19789,38 @@ namespace ts {
}
}

function checkJSDocAugmentsTag(node: JSDocAugmentsTag): void {
const cls = getJSDocHost(node);
if (!isClassDeclaration(cls) && !isClassExpression(cls)) {
error(cls, Diagnostics.JSDoc_augments_is_not_attached_to_a_class_declaration);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this message mention class expressions?

return;
}

const name = getIdentifierFromEntityNameExpression(node.class.expression);
const extend = getClassExtendsHeritageClauseElement(cls);
if (extend) {
const className = getIdentifierFromEntityNameExpression(extend.expression);
if (className && name.escapedText !== className.escapedText) {
error(name, Diagnostics.JSDoc_augments_0_does_not_match_the_extends_1_clause,
unescapeLeadingUnderscores(name.escapedText),
unescapeLeadingUnderscores(className.escapedText));
}
}
}

function getIdentifierFromEntityNameExpression(node: Identifier | PropertyAccessExpression): Identifier;
function getIdentifierFromEntityNameExpression(node: Expression): Identifier | undefined;
function getIdentifierFromEntityNameExpression(node: Expression): Identifier | undefined {
switch (node.kind) {
case SyntaxKind.Identifier:
return node as Identifier;
case SyntaxKind.PropertyAccessExpression:
return (node as PropertyAccessExpression).name;
default:
return undefined;
}
}

function checkFunctionOrMethodDeclaration(node: FunctionDeclaration | MethodDeclaration): void {
checkDecorators(node);
checkSignatureDeclaration(node);
Expand Down Expand Up @@ -22483,6 +22515,8 @@ namespace ts {
case SyntaxKind.ParenthesizedType:
case SyntaxKind.TypeOperator:
return checkSourceElement((<ParenthesizedTypeNode | TypeOperatorNode>node).type);
case SyntaxKind.JSDocAugmentsTag:
return checkJSDocAugmentsTag(node as JSDocAugmentsTag);
case SyntaxKind.JSDocTypedefTag:
return checkJSDocTypedefTag(node as JSDocTypedefTag);
case SyntaxKind.JSDocParameterTag:
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3511,6 +3511,14 @@
"category": "Error",
"code": 8021
},
"JSDoc '@augments' is not attached to a class declaration.": {
"category": "Error",
"code": 8022
},
"JSDoc '@augments {0}' does not match the 'extends {1}' clause.": {
"category": "Error",
"code": 8023
},
"Only identifiers/qualified-names with optional type arguments are currently supported in a class 'extends' clause.": {
"category": "Error",
"code": 9002
Expand Down
40 changes: 32 additions & 8 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ namespace ts {
case SyntaxKind.JSDocTypeTag:
return visitNode(cbNode, (<JSDocTypeTag>node).typeExpression);
case SyntaxKind.JSDocAugmentsTag:
return visitNode(cbNode, (<JSDocAugmentsTag>node).typeExpression);
return visitNode(cbNode, (<JSDocAugmentsTag>node).class);
case SyntaxKind.JSDocTemplateTag:
return visitNodes(cbNode, cbNodes, (<JSDocTemplateTag>node).typeParameters);
case SyntaxKind.JSDocTypedefTag:
Expand Down Expand Up @@ -5624,13 +5624,16 @@ namespace ts {
function parseExpressionWithTypeArguments(): ExpressionWithTypeArguments {
const node = <ExpressionWithTypeArguments>createNode(SyntaxKind.ExpressionWithTypeArguments);
node.expression = parseLeftHandSideExpressionOrHigher();
if (token() === SyntaxKind.LessThanToken) {
node.typeArguments = parseBracketedList(ParsingContext.TypeArguments, parseType, SyntaxKind.LessThanToken, SyntaxKind.GreaterThanToken);
}

node.typeArguments = tryParseTypeArguments();
return finishNode(node);
}

function tryParseTypeArguments(): NodeArray<TypeNode> | undefined {
return token() === SyntaxKind.LessThanToken
? parseBracketedList(ParsingContext.TypeArguments, parseType, SyntaxKind.LessThanToken, SyntaxKind.GreaterThanToken)
: undefined;
}

function isHeritageClause(): boolean {
return token() === SyntaxKind.ExtendsKeyword || token() === SyntaxKind.ImplementsKeyword;
}
Expand Down Expand Up @@ -6604,15 +6607,36 @@ namespace ts {
}

function parseAugmentsTag(atToken: AtToken, tagName: Identifier): JSDocAugmentsTag {
const typeExpression = parseJSDocTypeExpression(/*requireBraces*/ true);

const result = <JSDocAugmentsTag>createNode(SyntaxKind.JSDocAugmentsTag, atToken.pos);
result.atToken = atToken;
result.tagName = tagName;
result.typeExpression = typeExpression;
result.class = parseExpressionWithTypeArgumentsForAugments();
return finishNode(result);
}

function parseExpressionWithTypeArgumentsForAugments(): ExpressionWithTypeArguments & { expression: Identifier | PropertyAccessEntityNameExpression } {
const usedBrace = parseOptional(SyntaxKind.OpenBraceToken);
const node = createNode(SyntaxKind.ExpressionWithTypeArguments) as ExpressionWithTypeArguments & { expression: Identifier | PropertyAccessEntityNameExpression };
node.expression = parsePropertyAccessEntityNameExpression();
node.typeArguments = tryParseTypeArguments();
const res = finishNode(node);
if (usedBrace) {
parseExpected(SyntaxKind.CloseBraceToken);
}
return res;
}

function parsePropertyAccessEntityNameExpression() {
let node: Identifier | PropertyAccessEntityNameExpression = parseJSDocIdentifierName(/*createIfMissing*/ true);
while (token() === SyntaxKind.DotToken) {
const prop: PropertyAccessEntityNameExpression = createNode(SyntaxKind.PropertyAccessExpression, node.pos) as PropertyAccessEntityNameExpression;
prop.expression = node;
prop.name = parseJSDocIdentifierName();
node = finishNode(prop);
}
return node;
}

function parseClassTag(atToken: AtToken, tagName: Identifier): JSDocClassTag {
const tag = <JSDocClassTag>createNode(SyntaxKind.JSDocClassTag, atToken.pos);
tag.atToken = atToken;
Expand Down
6 changes: 6 additions & 0 deletions src/compiler/scanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1856,6 +1856,12 @@ namespace ts {
case CharacterCodes.closeBracket:
pos++;
return token = SyntaxKind.CloseBracketToken;
case CharacterCodes.lessThan:
Copy link
Member

Choose a reason for hiding this comment

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

can you make sure that we have some tests that use < and > inside jsdoc comments in various places? it should be allowed everywhere in comment text, especially in misaligned cases like

  /**
   * @param x hi
< > still part of the previous comment
   */

And of course it should cause error in places like the tag name, etc.

Copy link
Author

Choose a reason for hiding this comment

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

Added a test.

pos++;
return token = SyntaxKind.LessThanToken;
case CharacterCodes.greaterThan:
pos++;
return token = SyntaxKind.GreaterThanToken;
case CharacterCodes.equals:
pos++;
return token = SyntaxKind.EqualsToken;
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2161,7 +2161,7 @@ namespace ts {

export interface JSDocAugmentsTag extends JSDocTag {
kind: SyntaxKind.JSDocAugmentsTag;
typeExpression: JSDocTypeExpression;
class: ExpressionWithTypeArguments & { expression: Identifier | PropertyAccessEntityNameExpression };
Copy link
Member

Choose a reason for hiding this comment

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

don't you want a subclass of ExpressionWithTypeArguments that has the override expression: Identifier | PropertyAccessEntityNameExpression ? Seems like intersection would be too permissive.

Copy link
Author

Choose a reason for hiding this comment

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

How is intersection permissive where extends isn't? Could you give an example?

Copy link
Member

Choose a reason for hiding this comment

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

I forgot that Identifier is assignable to Expression & Identifier but (for example) BinaryExpression isn't. Never mind.

}

export interface JSDocClassTag extends JSDocTag {
Expand Down
8 changes: 6 additions & 2 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1581,8 +1581,7 @@ namespace ts {
return undefined;
}
const name = node.name.escapedText;
Debug.assert(node.parent!.kind === SyntaxKind.JSDocComment);
const func = node.parent!.parent!;
const func = getJSDocHost(node);
if (!isFunctionLike(func)) {
return undefined;
}
Expand All @@ -1591,6 +1590,11 @@ namespace ts {
return parameter && parameter.symbol;
}

export function getJSDocHost(node: JSDocTag): HasJSDoc {
Debug.assert(node.parent!.kind === SyntaxKind.JSDocComment);
return node.parent!.parent!;
}

export function getTypeParameterFromJsDoc(node: TypeParameterDeclaration & { parent: JSDocTemplateTag }): TypeParameterDeclaration | undefined {
const name = node.name.escapedText;
const { typeParameters } = (node.parent.parent.parent as ts.SignatureDeclaration | ts.InterfaceDeclaration | ts.ClassDeclaration);
Expand Down
5 changes: 5 additions & 0 deletions src/harness/unittests/jsDocParsing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,11 @@ namespace ts {
* @property {number} age
* @property {string} name
*/`);
parsesCorrectly("<> characters",
`/**
* @param x hi
< > still part of the previous comment
*/`);
});
});
describe("getFirstToken", () => {
Expand Down
3 changes: 1 addition & 2 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -581,11 +581,10 @@ namespace ts.Completions {

return { symbols, isGlobalCompletion, isMemberCompletion, allowStringLiteral, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), request, keywordFilters };

type JSDocTagWithTypeExpression = JSDocAugmentsTag | JSDocParameterTag | JSDocPropertyTag | JSDocReturnTag | JSDocTypeTag | JSDocTypedefTag;
type JSDocTagWithTypeExpression = JSDocParameterTag | JSDocPropertyTag | JSDocReturnTag | JSDocTypeTag | JSDocTypedefTag;

function isTagWithTypeExpression(tag: JSDocTag): tag is JSDocTagWithTypeExpression {
switch (tag.kind) {
case SyntaxKind.JSDocAugmentsTag:
case SyntaxKind.JSDocParameterTag:
case SyntaxKind.JSDocPropertyTag:
case SyntaxKind.JSDocReturnTag:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"kind": "JSDocComment",
"pos": 0,
"end": 61,
"tags": {
"0": {
"kind": "JSDocParameterTag",
"pos": 7,
"end": 16,
"atToken": {
"kind": "AtToken",
"pos": 7,
"end": 8
},
"tagName": {
"kind": "Identifier",
"pos": 8,
"end": 13,
"escapedText": "param"
},
"name": {
"kind": "Identifier",
"pos": 14,
"end": 15,
"escapedText": "x"
},
"isNameFirst": true,
"isBracketed": false,
"comment": "hi\n< > still part of the previous comment"
},
"length": 1,
"pos": 7,
"end": 16
}
}
14 changes: 10 additions & 4 deletions tests/baselines/reference/jsdocAugmentsMissingType.errors.txt
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
/a.js(2,14): error TS1005: '{' expected.
/a.js(2,14): error TS1003: Identifier expected.
/a.js(2,14): error TS8023: JSDoc '@augments ' does not match the 'extends A' clause.
/a.js(5,14): error TS2339: Property 'x' does not exist on type 'B'.


==== /a.js (1 errors) ====
==== /a.js (3 errors) ====
class A { constructor() { this.x = 0; } }
/** @augments */
~
!!! error TS1005: '{' expected.

!!! error TS1003: Identifier expected.

!!! error TS8023: JSDoc '@augments ' does not match the 'extends A' clause.
class B extends A {
m() {
this.x
~
!!! error TS2339: Property 'x' does not exist on type 'B'.
}
}

2 changes: 0 additions & 2 deletions tests/baselines/reference/jsdocAugmentsMissingType.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ class B extends A {
>m : Symbol(B.m, Decl(a.js, 2, 19))

this.x
>this.x : Symbol(A.x, Decl(a.js, 0, 25))
>this : Symbol(B, Decl(a.js, 0, 41))
>x : Symbol(A.x, Decl(a.js, 0, 25))
}
}

6 changes: 3 additions & 3 deletions tests/baselines/reference/jsdocAugmentsMissingType.types
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ class A { constructor() { this.x = 0; } }
/** @augments */
class B extends A {
>B : B
>A : A
>A : typeof A

m() {
>m : () => void

this.x
>this.x : number
>this.x : any
>this : this
>x : number
>x : any
}
}

12 changes: 12 additions & 0 deletions tests/baselines/reference/jsdocAugments_nameMismatch.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/b.js(4,15): error TS8023: JSDoc '@augments A' does not match the 'extends B' clause.


==== /b.js (1 errors) ====
class A {}
class B {}

/** @augments A */
~
!!! error TS8023: JSDoc '@augments A' does not match the 'extends B' clause.
class C extends B {}

12 changes: 12 additions & 0 deletions tests/baselines/reference/jsdocAugments_nameMismatch.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
=== /b.js ===
class A {}
>A : Symbol(A, Decl(b.js, 0, 0))

class B {}
>B : Symbol(B, Decl(b.js, 0, 10))

/** @augments A */
class C extends B {}
>C : Symbol(C, Decl(b.js, 1, 10))
>B : Symbol(B, Decl(b.js, 0, 10))

12 changes: 12 additions & 0 deletions tests/baselines/reference/jsdocAugments_nameMismatch.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
=== /b.js ===
class A {}
>A : A

class B {}
>B : B

/** @augments A */
class C extends B {}
>C : C
>B : A

21 changes: 21 additions & 0 deletions tests/baselines/reference/jsdocAugments_noExtends.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
=== /b.js ===
class A { constructor() { this.x = 0; } }
>A : Symbol(A, Decl(b.js, 0, 0))
>this.x : Symbol(A.x, Decl(b.js, 0, 25))
>this : Symbol(A, Decl(b.js, 0, 0))
>x : Symbol(A.x, Decl(b.js, 0, 25))

/** @augments A */
class B {
>B : Symbol(B, Decl(b.js, 0, 41))

m() {
>m : Symbol(B.m, Decl(b.js, 3, 9))

return this.x;
>this.x : Symbol(A.x, Decl(b.js, 0, 25))
>this : Symbol(B, Decl(b.js, 0, 41))
>x : Symbol(A.x, Decl(b.js, 0, 25))
}
}

Loading