Skip to content

Improved 'nameless' JSDoc typedef support. #28162

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

Closed
Closed
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
8 changes: 6 additions & 2 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1755,7 +1755,7 @@ namespace ts {
bind(typeAlias.typeExpression);
if (!typeAlias.fullName || typeAlias.fullName.kind === SyntaxKind.Identifier) {
parent = typeAlias.parent;
bindBlockScopedDeclaration(typeAlias, SymbolFlags.TypeAlias, SymbolFlags.TypeAliasExcludes);
bindBlockScopedDeclaration(typeAlias, SymbolFlags.TypeAlias, SymbolFlags.TypeAliasExcludes & ~SymbolFlags.TypeAlias);
}
else {
bind(typeAlias.fullName);
Expand Down Expand Up @@ -2713,7 +2713,11 @@ namespace ts {
const isEnum = isInJSFile(node) && !!getJSDocEnumTag(node);
const enumFlags = (isEnum ? SymbolFlags.RegularEnum : SymbolFlags.None);
const enumExcludes = (isEnum ? SymbolFlags.RegularEnumExcludes : SymbolFlags.None);
if (isBlockOrCatchScoped(node)) {
const jsdocTypeDef = isInJSFile(node) && getJSDocNamelessTypedefTag(node);
if (jsdocTypeDef && !jsdocTypeDef.name) {
bindBlockScopedDeclaration(node, SymbolFlags.TypeAlias, SymbolFlags.TypeAliasExcludes);
}
else if (isBlockOrCatchScoped(node)) {
bindBlockScopedDeclaration(node, SymbolFlags.BlockScopedVariable | enumFlags, SymbolFlags.BlockScopedVariableExcludes | enumExcludes);
}
else if (isParameterDeclaration(node)) {
Expand Down
16 changes: 14 additions & 2 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4796,6 +4796,10 @@ namespace ts {
return addOptionality(declaredType, isOptional);
}

if (isNamelessJSDocTypeDef(declaration)) {
return getDeclaredTypeOfTypeAlias(getSymbolOfNode(declaration));
}

if ((noImplicitAny || isInJSFile(declaration)) &&
declaration.kind === SyntaxKind.VariableDeclaration && !isBindingPattern(declaration.name) &&
!(getCombinedModifierFlags(declaration) & ModifierFlags.Export) && !(declaration.flags & NodeFlags.Ambient)) {
Expand Down Expand Up @@ -5272,6 +5276,9 @@ namespace ts {
else if (isObjectLiteralMethod(declaration)) {
type = tryGetTypeFromEffectiveTypeNode(declaration) || checkObjectLiteralMethod(declaration, CheckMode.Normal);
}
else if (isNamelessJSDocTypeDef(declaration)) {
type = getDeclaredTypeOfTypeAlias(symbol);
}
else if (isParameter(declaration)
|| isPropertyDeclaration(declaration)
|| isPropertySignature(declaration)
Expand Down Expand Up @@ -5910,8 +5917,10 @@ namespace ts {
return errorType;
}

const declaration = <JSDocTypedefTag | JSDocCallbackTag | TypeAliasDeclaration>find(symbol.declarations, d =>
let declaration = <JSDocTypedefTag | JSDocCallbackTag | TypeAliasDeclaration>find(symbol.declarations, d =>
isJSDocTypeAlias(d) || d.kind === SyntaxKind.TypeAliasDeclaration);
if (!declaration) declaration = findMap(symbol.declarations, getJSDocNamelessTypedefTag);

const typeNode = isJSDocTypeAlias(declaration) ? declaration.typeExpression : declaration.type;
// If typeNode is missing, we will error in checkJSDocTypedefTag.
let type = typeNode ? getTypeFromTypeNode(typeNode) : errorType;
Expand Down Expand Up @@ -24861,7 +24870,7 @@ namespace ts {
}
}
}
else {
else if (!isNamelessJSDocTypeDef(node)) {
// Node is a secondary declaration, check that type is identical to primary declaration and check that
// initializer is consistent with type associated with the node
const declarationType = convertAutoToAny(getWidenedTypeForVariableLikeDeclaration(node));
Expand Down Expand Up @@ -29951,6 +29960,9 @@ namespace ts {
if (node.flags & NodeFlags.Ambient) {
checkAmbientInitializer(node);
}
else if (node.initializer && isNamelessJSDocTypeDef(node)) {
return grammarErrorOnNode(node, Diagnostics._0_only_refers_to_a_type_but_is_being_used_as_a_value_here, (node.name as Identifier).escapedText);
}
else if (!node.initializer) {
if (isBindingPattern(node.name) && !isBindingPattern(node.parent)) {
return grammarErrorOnNode(node, Diagnostics.A_destructuring_declaration_must_have_an_initializer);
Expand Down
9 changes: 9 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1240,6 +1240,10 @@ namespace ts {
return node && node.kind === SyntaxKind.MethodDeclaration && node.parent.kind === SyntaxKind.ObjectLiteralExpression;
}

export function isNamelessJSDocTypeDef(node: Node): boolean {
return !!getJSDocNamelessTypedefTag(node);
}

export function isObjectLiteralOrClassExpressionMethod(node: Node): node is MethodDeclaration {
return node.kind === SyntaxKind.MethodDeclaration &&
(node.parent.kind === SyntaxKind.ObjectLiteralExpression ||
Expand Down Expand Up @@ -5133,6 +5137,11 @@ namespace ts {
return getFirstJSDocTag(node, isJSDocEnumTag);
}

/** Gets the nameless JSDoc typedef tag for the node if present */
export function getJSDocNamelessTypedefTag(node: Node): JSDocTypedefTag | undefined {
return getFirstJSDocTag(node, (n): n is JSDocTypedefTag => isJSDocTypedefTag(n) && !n.name);
}

/** Gets the JSDoc this tag for the node if present */
export function getJSDocThisTag(node: Node): JSDocThisTag | undefined {
return getFirstJSDocTag(node, isJSDocThisTag);
Expand Down
2 changes: 2 additions & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3251,6 +3251,8 @@ declare namespace ts {
function getJSDocClassTag(node: Node): JSDocClassTag | undefined;
/** Gets the JSDoc enum tag for the node if present */
function getJSDocEnumTag(node: Node): JSDocEnumTag | undefined;
/** Gets the nameless JSDoc typedef tag for the node if present */
function getJSDocNamelessTypedefTag(node: Node): JSDocTypedefTag | undefined;
/** Gets the JSDoc this tag for the node if present */
function getJSDocThisTag(node: Node): JSDocThisTag | undefined;
/** Gets the JSDoc return tag for the node if present */
Expand Down
2 changes: 2 additions & 0 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3251,6 +3251,8 @@ declare namespace ts {
function getJSDocClassTag(node: Node): JSDocClassTag | undefined;
/** Gets the JSDoc enum tag for the node if present */
function getJSDocEnumTag(node: Node): JSDocEnumTag | undefined;
/** Gets the nameless JSDoc typedef tag for the node if present */
function getJSDocNamelessTypedefTag(node: Node): JSDocTypedefTag | undefined;
/** Gets the JSDoc this tag for the node if present */
function getJSDocThisTag(node: Node): JSDocThisTag | undefined;
/** Gets the JSDoc return tag for the node if present */
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/jsdocTypedefNoCrash.types
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
* }}
*/
export const foo = 5;
>foo : 5
>foo : error
>5 : 5

4 changes: 2 additions & 2 deletions tests/baselines/reference/jsdocTypedefNoCrash2.symbols
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
=== tests/cases/compiler/export.js ===
export type foo = 5;
>foo : Symbol(foo, Decl(export.js, 0, 0), Decl(export.js, 5, 12))
>foo : Symbol(foo, Decl(export.js, 0, 0), Decl(export.js, 2, 3))

/**
* @typedef {{
* }}
*/
export const foo = 5;
>foo : Symbol(foo, Decl(export.js, 0, 0), Decl(export.js, 5, 12))
>foo : Symbol(foo, Decl(export.js, 5, 12))

2 changes: 1 addition & 1 deletion tests/baselines/reference/jsdocTypedefNoCrash2.types
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ export type foo = 5;
* }}
*/
export const foo = 5;
>foo : 5
>foo : any
>5 : 5

70 changes: 70 additions & 0 deletions tests/baselines/reference/typedefNameless.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
tests/cases/conformance/jsdoc/a.js(11,1): error TS2693: 'nameless' only refers to a type, but is being used as a value here.
tests/cases/conformance/jsdoc/a.js(26,5): error TS2693: 'notOK' only refers to a type, but is being used as a value here.
tests/cases/conformance/jsdoc/a.js(32,7): error TS1155: 'const' declarations must be initialized.


==== tests/cases/conformance/jsdoc/a.js (3 errors) ====
/**
* @typedef {number}
*/
var nameless;

/**
* @typedef {number} named
*/
var this_is_not_the_name = true;

nameless = 123; // nameless is not a value
~~~~~~~~
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 this error. We can't change the semantics of JS or TS such that nameless no longer has a value meaning.

!!! error TS2693: 'nameless' only refers to a type, but is being used as a value here.

/**
* @param {named} p1
* @param {nameless} p2
*/
function abc(p1, p2) {}

/**
* @param {named} p1
* @param {nameless} p2
*/
export function breakThings(p1, p2) {}

/** @typedef {number} */
var notOK = 1;
~~~~~
!!! error TS2693: 'notOK' only refers to a type, but is being used as a value here.
Copy link
Member

Choose a reason for hiding this comment

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

same here


/** @typedef {string} */
let thisIsOK;

/** @typedef {{L: number}} */
const notLegalButShouldBe;
~~~~~~~~~~~~~~~~~~~
!!! error TS1155: 'const' declarations must be initialized.

==== tests/cases/conformance/jsdoc/b.js (0 errors) ====
/**
* @typedef {{
* p: string
* }}
*/
export var type1;

==== tests/cases/conformance/jsdoc/c.js (0 errors) ====
import { type1 as aliased } from './b';
Copy link
Member

Choose a reason for hiding this comment

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

This is the other change of this PR, I think. I'm not sure I like this one either -- it encourages checkJS users to import and export real variables at runtime just for the purposes of creating aliases at compile time. There's no general guarantee that these imports and exports will get optimised away.


/**
* @param {aliased} pt1
*/
function f1(pt1) {}

/** @type {{ p2?: any }} */
var k = {};

/**
* @typedef {aliased}
*/
k.p2;


79 changes: 79 additions & 0 deletions tests/baselines/reference/typedefNameless.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
=== tests/cases/conformance/jsdoc/a.js ===
/**
* @typedef {number}
*/
var nameless;
>nameless : Symbol(nameless, Decl(a.js, 3, 3), Decl(a.js, 1, 3))

/**
* @typedef {number} named
*/
var this_is_not_the_name = true;
>this_is_not_the_name : Symbol(this_is_not_the_name, Decl(a.js, 8, 3))

nameless = 123; // nameless is not a value

/**
* @param {named} p1
* @param {nameless} p2
*/
function abc(p1, p2) {}
>abc : Symbol(abc, Decl(a.js, 10, 15))
>p1 : Symbol(p1, Decl(a.js, 16, 13))
>p2 : Symbol(p2, Decl(a.js, 16, 16))

/**
* @param {named} p1
* @param {nameless} p2
*/
export function breakThings(p1, p2) {}
>breakThings : Symbol(breakThings, Decl(a.js, 16, 23))
>p1 : Symbol(p1, Decl(a.js, 22, 28))
>p2 : Symbol(p2, Decl(a.js, 22, 31))

/** @typedef {number} */
var notOK = 1;
>notOK : Symbol(notOK, Decl(a.js, 25, 3), Decl(a.js, 24, 4))

/** @typedef {string} */
let thisIsOK;
>thisIsOK : Symbol(thisIsOK, Decl(a.js, 28, 3), Decl(a.js, 27, 4))

/** @typedef {{L: number}} */
const notLegalButShouldBe;
>notLegalButShouldBe : Symbol(notLegalButShouldBe, Decl(a.js, 31, 5), Decl(a.js, 30, 4))

=== tests/cases/conformance/jsdoc/b.js ===
/**
* @typedef {{
* p: string
* }}
*/
export var type1;
>type1 : Symbol(type1, Decl(b.js, 5, 10), Decl(b.js, 1, 3))

=== tests/cases/conformance/jsdoc/c.js ===
import { type1 as aliased } from './b';
>type1 : Symbol(aliased, Decl(c.js, 0, 8))
>aliased : Symbol(aliased, Decl(c.js, 0, 8))

/**
* @param {aliased} pt1
*/
function f1(pt1) {}
>f1 : Symbol(f1, Decl(c.js, 0, 39))
>pt1 : Symbol(pt1, Decl(c.js, 5, 12))

/** @type {{ p2?: any }} */
var k = {};
>k : Symbol(k, Decl(c.js, 8, 3))

/**
* @typedef {aliased}
*/
k.p2;
>k.p2 : Symbol(p2, Decl(c.js, 7, 12))
>k : Symbol(k, Decl(c.js, 8, 3))
>p2 : Symbol(p2, Decl(c.js, 7, 12))


85 changes: 85 additions & 0 deletions tests/baselines/reference/typedefNameless.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
=== tests/cases/conformance/jsdoc/a.js ===
/**
* @typedef {number}
*/
var nameless;
>nameless : any

/**
* @typedef {number} named
*/
var this_is_not_the_name = true;
>this_is_not_the_name : boolean
>true : true

nameless = 123; // nameless is not a value
>nameless = 123 : 123
>nameless : any
>123 : 123

/**
* @param {named} p1
* @param {nameless} p2
*/
function abc(p1, p2) {}
>abc : (p1: number, p2: number) => void
>p1 : number
>p2 : number

/**
* @param {named} p1
* @param {nameless} p2
*/
export function breakThings(p1, p2) {}
>breakThings : (p1: number, p2: number) => void
>p1 : number
>p2 : number

/** @typedef {number} */
var notOK = 1;
>notOK : any
>1 : 1

/** @typedef {string} */
let thisIsOK;
>thisIsOK : any

/** @typedef {{L: number}} */
const notLegalButShouldBe;
>notLegalButShouldBe : any

=== tests/cases/conformance/jsdoc/b.js ===
/**
* @typedef {{
* p: string
* }}
*/
export var type1;
>type1 : any

=== tests/cases/conformance/jsdoc/c.js ===
import { type1 as aliased } from './b';
>type1 : any
>aliased : any

/**
* @param {aliased} pt1
*/
function f1(pt1) {}
>f1 : (pt1: { p: string; }) => void
>pt1 : { p: string; }

/** @type {{ p2?: any }} */
var k = {};
>k : { p2?: any; }
>{} : {}

/**
* @typedef {aliased}
*/
k.p2;
>k.p2 : any
>k : { p2?: any; }
>p2 : any


Loading