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.

Sign up for GitHub

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

Jump to bottom

Remove getAllAccessorDeclarations from the EmitResolver #57993

Merged
merged 1 commit into from
Mar 29, 2024
Merged

Remove getAllAccessorDeclarations from the EmitResolver #57993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
73 changes: 52 additions & 21 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,6 @@ import {
GenericType,
GetAccessorDeclaration,
getAliasDeclarationFromName,
getAllAccessorDeclarations,
getAllJSDocTags,
getAllowSyntheticDefaultImports,
getAncestor,
Expand Down Expand Up @@ -8507,7 +8506,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function getDeclarationWithTypeAnnotation(symbol: Symbol, enclosingDeclaration: Node | undefined) {
return symbol.declarations && find(symbol.declarations, s => !!getEffectiveTypeAnnotationNode(s) && (!enclosingDeclaration || !!findAncestor(s, n => n === enclosingDeclaration)));
return symbol.declarations && find(symbol.declarations, s => !!getNonlocalEffectiveTypeAnnotationNode(s) && (!enclosingDeclaration || !!findAncestor(s, n => n === enclosingDeclaration)));
}

function existingTypeNodeIsNotReferenceOrIsReferenceWithCompatibleTypeArgumentCount(existing: TypeNode, type: Type) {
Expand All @@ -8530,7 +8529,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const declWithExistingAnnotation = getDeclarationWithTypeAnnotation(symbol, getEnclosingDeclarationIgnoringFakeScope(enclosingDeclaration));
if (declWithExistingAnnotation && !isFunctionLikeDeclaration(declWithExistingAnnotation) && !isGetAccessorDeclaration(declWithExistingAnnotation)) {
// try to reuse the existing annotation
const existing = getEffectiveTypeAnnotationNode(declWithExistingAnnotation)!;
const existing = getNonlocalEffectiveTypeAnnotationNode(declWithExistingAnnotation)!;
const result = tryReuseExistingTypeNode(context, existing, type, declWithExistingAnnotation, addUndefined, includePrivateSymbol, bundled);
if (result) {
return result;
Expand Down Expand Up @@ -8583,7 +8582,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const typePredicate = getTypePredicateOfSignature(signature);
const type = getReturnTypeOfSignature(signature);
if (!isErrorType(type) && context.enclosingDeclaration) {
const annotation = signature.declaration && getEffectiveReturnTypeNode(signature.declaration);
const annotation = signature.declaration && getNonlocalEffectiveReturnTypeAnnotationNode(signature.declaration);
const enclosingDeclarationIgnoringFakeScope = getEnclosingDeclarationIgnoringFakeScope(context.enclosingDeclaration);
if (!!findAncestor(annotation, n => n === enclosingDeclarationIgnoringFakeScope) && annotation) {
const annotated = getTypeFromTypeNode(annotation);
Expand Down Expand Up @@ -48696,6 +48695,22 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return isFunctionLike(declaration) || isExportAssignment(declaration) || isVariableLike(declaration);
}

function getAllAccessorDeclarationsForDeclaration(accessor: AccessorDeclaration): AllAccessorDeclarations {
accessor = getParseTreeNode(accessor, isGetOrSetAccessorDeclaration)!; // TODO: GH#18217
const otherKind = accessor.kind === SyntaxKind.SetAccessor ? SyntaxKind.GetAccessor : SyntaxKind.SetAccessor;
const otherAccessor = getDeclarationOfKind<AccessorDeclaration>(getSymbolOfDeclaration(accessor), otherKind);
const firstAccessor = otherAccessor && (otherAccessor.pos < accessor.pos) ? otherAccessor : accessor;
const secondAccessor = otherAccessor && (otherAccessor.pos < accessor.pos) ? accessor : otherAccessor;
const setAccessor = accessor.kind === SyntaxKind.SetAccessor ? accessor : otherAccessor as SetAccessorDeclaration;
const getAccessor = accessor.kind === SyntaxKind.GetAccessor ? accessor : otherAccessor as GetAccessorDeclaration;
return {
firstAccessor,
secondAccessor,
setAccessor,
getAccessor,
};
}

function getPossibleTypeNodeReuseExpression(declaration: DeclarationWithPotentialInnerNodeReuse) {
return isFunctionLike(declaration) && !isSetAccessor(declaration)
? getSingleReturnExpression(declaration)
Expand All @@ -48704,7 +48719,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
: !!(declaration as HasInitializer).initializer
? (declaration as HasInitializer & typeof declaration).initializer
: isParameter(declaration) && isSetAccessor(declaration.parent)
? getSingleReturnExpression(getAllAccessorDeclarations(getSymbolOfDeclaration(declaration.parent)?.declarations, declaration.parent).getAccessor)
? getSingleReturnExpression(getAllAccessorDeclarationsForDeclaration(declaration.parent).getAccessor)
: undefined;
}

Expand Down Expand Up @@ -48894,6 +48909,37 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
}

function getNonlocalEffectiveTypeAnnotationNode(node: Node) {
const direct = getEffectiveTypeAnnotationNode(node);
if (direct) {
return direct;
}
if (node.kind === SyntaxKind.Parameter && node.parent.kind === SyntaxKind.SetAccessor) {
const other = getAllAccessorDeclarationsForDeclaration(node.parent as SetAccessorDeclaration).getAccessor;
if (other) {
return getEffectiveReturnTypeNode(other);
}
}
Comment on lines +48917 to +48922
Copy link
Member

Choose a reason for hiding this comment

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

Just to be clear, this code is a noop, right? Since we're not actually handling these and no baselines change? This is brand new code, so, checking.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, for now. My goal is to move the full "should this node be reused" logic check into the node builder, to minimize the EmitResolver's API surface. Today, the node builder never has to emit nonlocal types for get and set accessors. If one is missing an annotation, it gets the type the other one has, and if both have the same type, it prints them as a property, rather than accessors. Meanwhile, declaration emit proper obviously leaves accessor declarations as-is nowadays, and uses logic like this to pull the type node from the other accessor when one is missing an annotation. So this is slightly future-looking, in that it enables the node builder to properly reuse nodes for get/set pairs that are missing one type node, even though the declaration emitter doesn't yet ask it to do that (and you can't get an inferred type with a get/set pair without them having differing types, and thus both annotated).

return undefined;
}

function getNonlocalEffectiveReturnTypeAnnotationNode(node: SignatureDeclaration | JSDocSignature) {
const direct = getEffectiveReturnTypeNode(node);
if (direct) {
return direct;
}
if (node.kind === SyntaxKind.GetAccessor) {
const other = getAllAccessorDeclarationsForDeclaration(node).setAccessor;
if (other) {
const param = getSetAccessorValueParameter(other);
if (param) {
return getEffectiveTypeAnnotationNode(param);
}
}
}
return undefined;
}

function createResolver(): EmitResolver {
return {
getReferencedExportContainer,
Expand Down Expand Up @@ -48949,21 +48995,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
},
getJsxFactoryEntity,
getJsxFragmentFactoryEntity,
getAllAccessorDeclarations(accessor: AccessorDeclaration): AllAccessorDeclarations {
accessor = getParseTreeNode(accessor, isGetOrSetAccessorDeclaration)!; // TODO: GH#18217
const otherKind = accessor.kind === SyntaxKind.SetAccessor ? SyntaxKind.GetAccessor : SyntaxKind.SetAccessor;
const otherAccessor = getDeclarationOfKind<AccessorDeclaration>(getSymbolOfDeclaration(accessor), otherKind);
const firstAccessor = otherAccessor && (otherAccessor.pos < accessor.pos) ? otherAccessor : accessor;
const secondAccessor = otherAccessor && (otherAccessor.pos < accessor.pos) ? accessor : otherAccessor;
const setAccessor = accessor.kind === SyntaxKind.SetAccessor ? accessor : otherAccessor as SetAccessorDeclaration;
const getAccessor = accessor.kind === SyntaxKind.GetAccessor ? accessor : otherAccessor as GetAccessorDeclaration;
return {
firstAccessor,
secondAccessor,
setAccessor,
getAccessor,
};
},
isBindingCapturedByNode: (node, decl) => {
const parseNode = getParseTreeNode(node);
const parseDecl = getParseTreeNode(decl);
Expand Down Expand Up @@ -49280,7 +49311,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
}
else if (legacyDecorators && (node.kind === SyntaxKind.GetAccessor || node.kind === SyntaxKind.SetAccessor)) {
const accessors = getAllAccessorDeclarations((node.parent as ClassDeclaration).members, node as AccessorDeclaration);
const accessors = getAllAccessorDeclarationsForDeclaration(node as AccessorDeclaration);
if (hasDecorators(accessors.firstAccessor) && node === accessors.secondAccessor) {
return grammarErrorOnFirstToken(node, Diagnostics.Decorators_cannot_be_applied_to_multiple_get_Slashset_accessors_of_the_same_name);
}
Expand Down
1 change: 0 additions & 1 deletion src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1112,7 +1112,6 @@ export const notImplementedResolver: EmitResolver = {
isLiteralConstDeclaration: notImplemented,
getJsxFactoryEntity: notImplemented,
getJsxFragmentFactoryEntity: notImplemented,
getAllAccessorDeclarations: notImplemented,
isBindingCapturedByNode: notImplemented,
getDeclarationStatementsForSourceFile: notImplemented,
isImportRequiredByAugmentation: notImplemented,
Expand Down
6 changes: 4 additions & 2 deletions src/compiler/transformers/declarations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
FunctionTypeNode,
GeneratedIdentifierFlags,
GetAccessorDeclaration,
getAllAccessorDeclarations,
getCommentRange,
getDirectoryPath,
getEffectiveBaseTypeNode,
Expand Down Expand Up @@ -119,6 +120,7 @@ import {
isMethodSignature,
isModifier,
isModuleDeclaration,
isObjectLiteralExpression,
isOmittedExpression,
isPrivateIdentifier,
isSemicolonClassElement,
Expand Down Expand Up @@ -728,7 +730,7 @@ export function transformDeclarations(context: TransformationContext) {
if (!isPrivate) {
const valueParameter = getSetAccessorValueParameter(input);
if (valueParameter) {
const accessorType = getTypeAnnotationFromAllAccessorDeclarations(input, resolver.getAllAccessorDeclarations(input));
const accessorType = getTypeAnnotationFromAllAccessorDeclarations(input, getAllAccessorDeclarations(isObjectLiteralExpression(input.parent) ? input.parent.properties : input.parent.members, input));
newValueParameter = ensureParameter(valueParameter, /*modifierMask*/ undefined, accessorType);
}
}
Expand Down Expand Up @@ -1037,7 +1039,7 @@ export function transformDeclarations(context: TransformationContext) {
if (isPrivateIdentifier(input.name)) {
return cleanup(/*returnValue*/ undefined);
}
const accessorType = getTypeAnnotationFromAllAccessorDeclarations(input, resolver.getAllAccessorDeclarations(input));
const accessorType = getTypeAnnotationFromAllAccessorDeclarations(input, getAllAccessorDeclarations(isObjectLiteralExpression(input.parent) ? input.parent.properties : input.parent.members, input));
return cleanup(factory.updateGetAccessorDeclaration(
input,
ensureModifiers(input),
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/transformers/ts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1122,7 +1122,7 @@ export function transformTypeScript(context: TransformationContext) {
if (typeSerializer) {
let decorators: Decorator[] | undefined;
if (shouldAddTypeMetadata(node)) {
const typeMetadata = emitHelpers().createMetadataHelper("design:type", typeSerializer.serializeTypeOfNode({ currentLexicalScope, currentNameScope: container }, node));
const typeMetadata = emitHelpers().createMetadataHelper("design:type", typeSerializer.serializeTypeOfNode({ currentLexicalScope, currentNameScope: container }, node, container));
decorators = append(decorators, factory.createDecorator(typeMetadata));
}
if (shouldAddParamTypesMetadata(node)) {
Expand All @@ -1141,7 +1141,7 @@ export function transformTypeScript(context: TransformationContext) {
if (typeSerializer) {
let properties: ObjectLiteralElementLike[] | undefined;
if (shouldAddTypeMetadata(node)) {
const typeProperty = factory.createPropertyAssignment("type", factory.createArrowFunction(/*modifiers*/ undefined, /*typeParameters*/ undefined, [], /*type*/ undefined, factory.createToken(SyntaxKind.EqualsGreaterThanToken), typeSerializer.serializeTypeOfNode({ currentLexicalScope, currentNameScope: container }, node)));
const typeProperty = factory.createPropertyAssignment("type", factory.createArrowFunction(/*modifiers*/ undefined, /*typeParameters*/ undefined, [], /*type*/ undefined, factory.createToken(SyntaxKind.EqualsGreaterThanToken), typeSerializer.serializeTypeOfNode({ currentLexicalScope, currentNameScope: container }, node, container)));
properties = append(properties, typeProperty);
}
if (shouldAddParamTypesMetadata(node)) {
Expand Down
14 changes: 7 additions & 7 deletions src/compiler/transformers/typeSerializer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export interface RuntimeTypeSerializer {
* Serializes the type of a node for use with decorator type metadata.
* @param node The node that should have its type serialized.
*/
serializeTypeOfNode(serializerContext: RuntimeTypeSerializerContext, node: PropertyDeclaration | ParameterDeclaration | AccessorDeclaration | ClassLikeDeclaration | MethodDeclaration): Expression;
serializeTypeOfNode(serializerContext: RuntimeTypeSerializerContext, node: PropertyDeclaration | ParameterDeclaration | AccessorDeclaration | ClassLikeDeclaration | MethodDeclaration, container: ClassLikeDeclaration): Expression;
/**
* Serializes the types of the parameters of a node for use with decorator type metadata.
* @param node The node that should have its parameter types serialized.
Expand Down Expand Up @@ -145,7 +145,7 @@ export function createRuntimeTypeSerializer(context: TransformationContext): Run

return {
serializeTypeNode: (serializerContext, node) => setSerializerContextAnd(serializerContext, serializeTypeNode, node),
serializeTypeOfNode: (serializerContext, node) => setSerializerContextAnd(serializerContext, serializeTypeOfNode, node),
serializeTypeOfNode: (serializerContext, node, container) => setSerializerContextAnd(serializerContext, serializeTypeOfNode, node, container),
serializeParameterTypesOfNode: (serializerContext, node, container) => setSerializerContextAnd(serializerContext, serializeParameterTypesOfNode, node, container),
serializeReturnTypeOfNode: (serializerContext, node) => setSerializerContextAnd(serializerContext, serializeReturnTypeOfNode, node),
};
Expand All @@ -166,8 +166,8 @@ export function createRuntimeTypeSerializer(context: TransformationContext): Run
return result;
}

function getAccessorTypeNode(node: AccessorDeclaration) {
const accessors = resolver.getAllAccessorDeclarations(node);
function getAccessorTypeNode(node: AccessorDeclaration, container: ClassLikeDeclaration) {
const accessors = getAllAccessorDeclarations(container.members, node);
return accessors.setAccessor && getSetAccessorTypeAnnotationNode(accessors.setAccessor)
|| accessors.getAccessor && getEffectiveReturnTypeNode(accessors.getAccessor);
}
Expand All @@ -176,14 +176,14 @@ export function createRuntimeTypeSerializer(context: TransformationContext): Run
* Serializes the type of a node for use with decorator type metadata.
* @param node The node that should have its type serialized.
*/
function serializeTypeOfNode(node: PropertyDeclaration | ParameterDeclaration | AccessorDeclaration | ClassLikeDeclaration | MethodDeclaration): SerializedTypeNode {
function serializeTypeOfNode(node: PropertyDeclaration | ParameterDeclaration | AccessorDeclaration | ClassLikeDeclaration | MethodDeclaration, container: ClassLikeDeclaration): SerializedTypeNode {
switch (node.kind) {
case SyntaxKind.PropertyDeclaration:
case SyntaxKind.Parameter:
return serializeTypeNode(node.type);
case SyntaxKind.SetAccessor:
case SyntaxKind.GetAccessor:
return serializeTypeNode(getAccessorTypeNode(node));
return serializeTypeNode(getAccessorTypeNode(node, container));
case SyntaxKind.ClassDeclaration:
case SyntaxKind.ClassExpression:
case SyntaxKind.MethodDeclaration:
Expand Down Expand Up @@ -217,7 +217,7 @@ export function createRuntimeTypeSerializer(context: TransformationContext): Run
expressions.push(serializeTypeNode(getRestParameterElementType(parameter.type)));
}
else {
expressions.push(serializeTypeOfNode(parameter));
expressions.push(serializeTypeOfNode(parameter, container));
}
}
}
Expand Down
1 change: 0 additions & 1 deletion src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5635,7 +5635,6 @@ export interface EmitResolver {
isLiteralConstDeclaration(node: VariableDeclaration | PropertyDeclaration | PropertySignature | ParameterDeclaration): boolean;
getJsxFactoryEntity(location?: Node): EntityName | undefined;
getJsxFragmentFactoryEntity(location?: Node): EntityName | undefined;
getAllAccessorDeclarations(declaration: AccessorDeclaration): AllAccessorDeclarations;
isBindingCapturedByNode(node: Node, decl: VariableDeclaration | BindingElement): boolean;
getDeclarationStatementsForSourceFile(node: SourceFile, flags: NodeBuilderFlags, tracker: SymbolTracker, bundled?: boolean): Statement[] | undefined;
isImportRequiredByAugmentation(decl: ImportDeclaration): boolean;
Expand Down