-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Reuse input nodes where possible when serializing jsdoc implements clauses #41783
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5860,6 +5860,32 @@ namespace ts { | |
return typeToTypeNodeHelper(type, context); | ||
} | ||
|
||
function trackExistingEntityName<T extends EntityNameOrEntityNameExpression>(node: T, context: NodeBuilderContext, includePrivateSymbol?: (s: Symbol) => void) { | ||
let introducesError = false; | ||
const leftmost = getFirstIdentifier(node); | ||
if (isInJSFile(node) && (isExportsIdentifier(leftmost) || isModuleExportsAccessExpression(leftmost.parent) || (isQualifiedName(leftmost.parent) && isModuleIdentifier(leftmost.parent.left) && isExportsIdentifier(leftmost.parent.right)))) { | ||
introducesError = true; | ||
return { introducesError, node }; | ||
} | ||
const sym = resolveEntityName(leftmost, SymbolFlags.All, /*ignoreErrors*/ true, /*dontResolveALias*/ true); | ||
if (sym) { | ||
if (isSymbolAccessible(sym, context.enclosingDeclaration, SymbolFlags.All, /*shouldComputeAliasesToMakeVisible*/ false).accessibility !== SymbolAccessibility.Accessible) { | ||
introducesError = true; | ||
} | ||
else { | ||
context.tracker?.trackSymbol?.(sym, context.enclosingDeclaration, SymbolFlags.All); | ||
includePrivateSymbol?.(sym); | ||
} | ||
if (isIdentifier(node)) { | ||
const name = sym.flags & SymbolFlags.TypeParameter ? typeParameterToName(getDeclaredTypeOfSymbol(sym), context) : factory.cloneNode(node); | ||
name.symbol = sym; // for quickinfo, which uses identifier symbol information | ||
return { introducesError, node: setEmitFlags(setOriginalNode(name, node), EmitFlags.NoAsciiEscaping) }; | ||
} | ||
} | ||
|
||
return { introducesError, node }; | ||
} | ||
|
||
function serializeExistingTypeNode(context: NodeBuilderContext, existing: TypeNode, includePrivateSymbol?: (s: Symbol) => void, bundled?: boolean) { | ||
if (cancellationToken && cancellationToken.throwIfCancellationRequested) { | ||
cancellationToken.throwIfCancellationRequested(); | ||
|
@@ -5983,25 +6009,10 @@ namespace ts { | |
} | ||
|
||
if (isEntityName(node) || isEntityNameExpression(node)) { | ||
const leftmost = getFirstIdentifier(node); | ||
if (isInJSFile(node) && (isExportsIdentifier(leftmost) || isModuleExportsAccessExpression(leftmost.parent) || (isQualifiedName(leftmost.parent) && isModuleIdentifier(leftmost.parent.left) && isExportsIdentifier(leftmost.parent.right)))) { | ||
hadError = true; | ||
return node; | ||
} | ||
const sym = resolveEntityName(leftmost, SymbolFlags.All, /*ignoreErrors*/ true, /*dontResolveALias*/ true); | ||
if (sym) { | ||
if (isSymbolAccessible(sym, context.enclosingDeclaration, SymbolFlags.All, /*shouldComputeAliasesToMakeVisible*/ false).accessibility !== SymbolAccessibility.Accessible) { | ||
hadError = true; | ||
} | ||
else { | ||
context.tracker?.trackSymbol?.(sym, context.enclosingDeclaration, SymbolFlags.All); | ||
includePrivateSymbol?.(sym); | ||
} | ||
if (isIdentifier(node)) { | ||
const name = sym.flags & SymbolFlags.TypeParameter ? typeParameterToName(getDeclaredTypeOfSymbol(sym), context) : factory.cloneNode(node); | ||
name.symbol = sym; // for quickinfo, which uses identifier symbol information | ||
return setEmitFlags(setOriginalNode(name, node), EmitFlags.NoAsciiEscaping); | ||
} | ||
const { introducesError, node: result } = trackExistingEntityName(node, context, includePrivateSymbol); | ||
hadError = hadError || introducesError; | ||
if (result !== node) { | ||
return result; | ||
} | ||
} | ||
|
||
|
@@ -6732,12 +6743,50 @@ namespace ts { | |
!(p.flags & SymbolFlags.Prototype || p.escapedName === "prototype" || p.valueDeclaration && getEffectiveModifierFlags(p.valueDeclaration) & ModifierFlags.Static && isClassLike(p.valueDeclaration.parent)); | ||
} | ||
|
||
function sanitizeJSDocImplements(clauses: readonly ExpressionWithTypeArguments[]): ExpressionWithTypeArguments[] | undefined { | ||
const result = mapDefined(clauses, e => { | ||
const oldEnclosing = context.enclosingDeclaration; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wtb dynamic scope macro There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that gives you the control flow, but not the scoping behaviour for variables. I'm talking something like |
||
context.enclosingDeclaration = e; | ||
let expr = e.expression; | ||
if (isEntityNameExpression(expr)) { | ||
if (isIdentifier(expr) && idText(expr) === "") { | ||
return cleanup(/*result*/ undefined); // Empty heritage clause, should be an error, but prefer emitting no heritage clauses to reemitting the empty one | ||
} | ||
let introducesError: boolean; | ||
({ introducesError, node: expr } = trackExistingEntityName(expr, context, includePrivateSymbol)); | ||
if (introducesError) { | ||
return cleanup(/*result*/ undefined); | ||
} | ||
} | ||
return cleanup(factory.createExpressionWithTypeArguments(expr, | ||
map(e.typeArguments, a => | ||
serializeExistingTypeNode(context, a, includePrivateSymbol, bundled) | ||
|| typeToTypeNodeHelper(getTypeFromTypeNode(a), context) | ||
) | ||
)); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. might be nice to break this line too |
||
function cleanup<T>(result: T): T { | ||
context.enclosingDeclaration = oldEnclosing; | ||
return result; | ||
} | ||
}); | ||
if (result.length === clauses.length) { | ||
return result; | ||
} | ||
return undefined; | ||
} | ||
|
||
function serializeAsClass(symbol: Symbol, localName: string, modifierFlags: ModifierFlags) { | ||
const originalDecl = find(symbol.declarations, isClassLike); | ||
const oldEnclosing = context.enclosingDeclaration; | ||
context.enclosingDeclaration = originalDecl || oldEnclosing; | ||
const localParams = getLocalTypeParametersOfClassOrInterfaceOrTypeAlias(symbol); | ||
const typeParamDecls = map(localParams, p => typeParameterToDeclaration(p, context)); | ||
const classType = getDeclaredTypeOfClassOrInterface(symbol); | ||
const baseTypes = getBaseTypes(classType); | ||
const implementsExpressions = mapDefined(getImplementsTypes(classType), serializeImplementedType); | ||
const originalImplements = originalDecl && getEffectiveImplementsTypeNodes(originalDecl); | ||
const implementsExpressions = originalImplements && sanitizeJSDocImplements(originalImplements) | ||
|| mapDefined(getImplementsTypes(classType), serializeImplementedType); | ||
const staticType = getTypeOfSymbol(symbol); | ||
const isClass = !!staticType.symbol?.valueDeclaration && isClassLike(staticType.symbol.valueDeclaration); | ||
const staticBaseType = isClass | ||
|
@@ -6790,6 +6839,7 @@ namespace ts { | |
[factory.createConstructorDeclaration(/*decorators*/ undefined, factory.createModifiersFromModifierFlags(ModifierFlags.Private), [], /*body*/ undefined)] : | ||
serializeSignatures(SignatureKind.Construct, staticType, staticBaseType, SyntaxKind.Constructor) as ConstructorDeclaration[]; | ||
const indexSignatures = serializeIndexSignatures(classType, baseTypes[0]); | ||
context.enclosingDeclaration = oldEnclosing; | ||
addResult(setTextRange(factory.createClassDeclaration( | ||
/*decorators*/ undefined, | ||
/*modifiers*/ undefined, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
//// [tests/cases/conformance/jsdoc/declarations/jsDeclarationsClassImplementsGenericsSerialization.ts] //// | ||
|
||
//// [interface.ts] | ||
export interface Encoder<T> { | ||
encode(value: T): Uint8Array | ||
} | ||
//// [lib.js] | ||
/** | ||
* @template T | ||
* @implements {IEncoder<T>} | ||
*/ | ||
export class Encoder { | ||
/** | ||
* @param {T} value | ||
*/ | ||
encode(value) { | ||
return new Uint8Array(0) | ||
} | ||
} | ||
|
||
|
||
/** | ||
* @template T | ||
* @typedef {import('./interface').Encoder<T>} IEncoder | ||
*/ | ||
|
||
//// [interface.js] | ||
"use strict"; | ||
Object.defineProperty(exports, "__esModule", { value: true }); | ||
//// [lib.js] | ||
"use strict"; | ||
Object.defineProperty(exports, "__esModule", { value: true }); | ||
exports.Encoder = void 0; | ||
/** | ||
* @template T | ||
* @implements {IEncoder<T>} | ||
*/ | ||
var Encoder = /** @class */ (function () { | ||
function Encoder() { | ||
} | ||
/** | ||
* @param {T} value | ||
*/ | ||
Encoder.prototype.encode = function (value) { | ||
return new Uint8Array(0); | ||
}; | ||
return Encoder; | ||
}()); | ||
exports.Encoder = Encoder; | ||
/** | ||
* @template T | ||
* @typedef {import('./interface').Encoder<T>} IEncoder | ||
*/ | ||
|
||
|
||
//// [interface.d.ts] | ||
export interface Encoder<T> { | ||
encode(value: T): Uint8Array; | ||
} | ||
//// [lib.d.ts] | ||
/** | ||
* @template T | ||
* @implements {IEncoder<T>} | ||
*/ | ||
export class Encoder<T> implements IEncoder<T> { | ||
/** | ||
* @param {T} value | ||
*/ | ||
encode(value: T): Uint8Array; | ||
} | ||
export type IEncoder<T> = import("./interface").Encoder<T>; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
=== tests/cases/conformance/jsdoc/declarations/interface.ts === | ||
export interface Encoder<T> { | ||
>Encoder : Symbol(Encoder, Decl(interface.ts, 0, 0)) | ||
>T : Symbol(T, Decl(interface.ts, 0, 25)) | ||
|
||
encode(value: T): Uint8Array | ||
>encode : Symbol(Encoder.encode, Decl(interface.ts, 0, 29)) | ||
>value : Symbol(value, Decl(interface.ts, 1, 11)) | ||
>T : Symbol(T, Decl(interface.ts, 0, 25)) | ||
>Uint8Array : Symbol(Uint8Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) | ||
} | ||
=== tests/cases/conformance/jsdoc/declarations/lib.js === | ||
/** | ||
* @template T | ||
* @implements {IEncoder<T>} | ||
*/ | ||
export class Encoder { | ||
>Encoder : Symbol(Encoder, Decl(lib.js, 0, 0)) | ||
|
||
/** | ||
* @param {T} value | ||
*/ | ||
encode(value) { | ||
>encode : Symbol(Encoder.encode, Decl(lib.js, 4, 22)) | ||
>value : Symbol(value, Decl(lib.js, 8, 11)) | ||
|
||
return new Uint8Array(0) | ||
>Uint8Array : Symbol(Uint8Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) | ||
} | ||
} | ||
|
||
|
||
/** | ||
* @template T | ||
* @typedef {import('./interface').Encoder<T>} IEncoder | ||
*/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
=== tests/cases/conformance/jsdoc/declarations/interface.ts === | ||
export interface Encoder<T> { | ||
encode(value: T): Uint8Array | ||
>encode : (value: T) => Uint8Array | ||
>value : T | ||
} | ||
=== tests/cases/conformance/jsdoc/declarations/lib.js === | ||
/** | ||
* @template T | ||
* @implements {IEncoder<T>} | ||
*/ | ||
export class Encoder { | ||
>Encoder : Encoder<T> | ||
|
||
/** | ||
* @param {T} value | ||
*/ | ||
encode(value) { | ||
>encode : (value: T) => Uint8Array | ||
>value : T | ||
|
||
return new Uint8Array(0) | ||
>new Uint8Array(0) : Uint8Array | ||
>Uint8Array : Uint8ArrayConstructor | ||
>0 : 0 | ||
} | ||
} | ||
|
||
|
||
/** | ||
* @template T | ||
* @typedef {import('./interface').Encoder<T>} IEncoder | ||
*/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
// @allowJs: true | ||
// @checkJs: true | ||
// @target: es5 | ||
// @outDir: ./out | ||
// @declaration: true | ||
// @filename: interface.ts | ||
export interface Encoder<T> { | ||
encode(value: T): Uint8Array | ||
} | ||
// @filename: lib.js | ||
/** | ||
* @template T | ||
* @implements {IEncoder<T>} | ||
*/ | ||
export class Encoder { | ||
/** | ||
* @param {T} value | ||
*/ | ||
encode(value) { | ||
return new Uint8Array(0) | ||
} | ||
} | ||
|
||
|
||
/** | ||
* @template T | ||
* @typedef {import('./interface').Encoder<T>} IEncoder | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is the best way to break up this line, but it's what I ended up doing so I could read it.