Skip to content

Decorators normative changes #52582

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

Merged
merged 7 commits into from
Feb 9, 2023
Merged

Decorators normative changes #52582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dfa80ca
Apply normative changes to 'context.access'
rbuckton Feb 2, 2023
6dc331d
Allow decorators before and after 'export' in JS files
rbuckton Feb 2, 2023
2473663
Improve error message for decorators before and after 'export'
rbuckton Feb 3, 2023
cf3262a
Fix lint
rbuckton Feb 3, 2023
60f931e
Accept missing baseline
rbuckton Feb 3, 2023
9ba73ed
Add tests for 'abstract' as well
rbuckton Feb 3, 2023
2cfd5b1
Merge branch 'main' into decorators-normative-changes
rbuckton Feb 9, 2023
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
28 changes: 27 additions & 1 deletion src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46445,6 +46445,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
let lastStatic: Node | undefined, lastDeclare: Node | undefined, lastAsync: Node | undefined, lastOverride: Node | undefined, firstDecorator: Decorator | undefined;
let flags = ModifierFlags.None;
let sawExportBeforeDecorators = false;
// We parse decorators and modifiers in four contiguous chunks:
// [...leadingDecorators, ...leadingModifiers, ...trailingDecorators, ...trailingModifiers]. It is an error to
// have both leading and trailing decorators.
let hasLeadingDecorators = false;
for (const modifier of (node as HasModifiers).modifiers!) {
if (isDecorator(modifier)) {
if (!nodeCanBeDecorated(legacyDecorators, node, node.parent, node.parent.parent)) {
Expand All @@ -46461,13 +46465,35 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return grammarErrorOnFirstToken(node, Diagnostics.Decorators_cannot_be_applied_to_multiple_get_Slashset_accessors_of_the_same_name);
}
}

// if we've seen any modifiers aside from `export`, `default`, or another decorator, then this is an invalid position
if (flags & ~(ModifierFlags.ExportDefault | ModifierFlags.Decorator)) {
return grammarErrorOnNode(modifier, Diagnostics.Decorators_are_not_valid_here);
}

// if we've already seen leading decorators and leading modifiers, then trailing decorators are an invalid position
if (hasLeadingDecorators && flags & ModifierFlags.Modifier) {
Debug.assertIsDefined(firstDecorator);
const sourceFile = getSourceFileOfNode(modifier);
if (!hasParseDiagnostics(sourceFile)) {
addRelatedInfo(
error(modifier, Diagnostics.Decorators_may_not_appear_after_export_or_export_default_if_they_also_appear_before_export),
createDiagnosticForNode(firstDecorator, Diagnostics.Decorator_used_before_export_here));
return true;
}
return false;
}

flags |= ModifierFlags.Decorator;
if (flags & ModifierFlags.Export) {

// if we have not yet seen a modifier, then these are leading decorators
if (!(flags & ModifierFlags.Modifier)) {
hasLeadingDecorators = true;
}
else if (flags & ModifierFlags.Export) {
sawExportBeforeDecorators = true;
}

firstDecorator ??= modifier;
}
else {
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1589,6 +1589,10 @@
"category": "Error",
"code": 1485
},
"Decorator used before 'export' here.": {
"category": "Error",
"code": 1486
},

"The types of '{0}' are incompatible between these types.": {
"category": "Error",
Expand Down Expand Up @@ -6560,7 +6564,7 @@
"category": "Error",
"code": 8037
},
"Decorators must come after 'export' or 'export default' in JavaScript files.": {
"Decorators may not appear after 'export' or 'export default' if they also appear before 'export'.": {
"category": "Error",
"code": 8038
},
Expand Down
245 changes: 93 additions & 152 deletions src/compiler/factory/emitHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
isComputedPropertyName,
isIdentifier,
memoize,
ObjectLiteralElementLike,
PrivateIdentifier,
ScriptTarget,
setEmitFlags,
Expand Down Expand Up @@ -250,165 +251,105 @@ export function createEmitHelperFactory(context: TransformationContext): EmitHel
]);
}

// Per https://github.com/tc39/proposal-decorators/issues/494, we may need to change the emit for the `access` object
// so that it does not need to be used via `.call`. The following two sections represent the options presented in
// tc39/proposal-decorators#494
//
// === Current approach (`access.get.call(obj)`, `access.set.call(obj, value)`) ===
//
// function createESDecorateClassElementAccessGetMethod(elementName: ESDecorateName) {
// const accessor = elementName.computed ?
// factory.createElementAccessExpression(factory.createThis(), elementName.name) :
// factory.createPropertyAccessExpression(factory.createThis(), elementName.name);
//
// return factory.createMethodDeclaration(
// /*modifiers*/ undefined,
// /*asteriskToken*/ undefined,
// "get",
// /*questionToken*/ undefined,
// /*typeParameters*/ undefined,
// [],
// /*type*/ undefined,
// factory.createBlock([factory.createReturnStatement(accessor)])
// );
// }
//
// function createESDecorateClassElementAccessSetMethod(elementName: ESDecorateName) {
// const accessor = elementName.computed ?
// factory.createElementAccessExpression(factory.createThis(), elementName.name) :
// factory.createPropertyAccessExpression(factory.createThis(), elementName.name);
//
// return factory.createMethodDeclaration(
// /*modifiers*/ undefined,
// /*asteriskToken*/ undefined,
// "set",
// /*questionToken*/ undefined,
// /*typeParameters*/ undefined,
// [factory.createParameterDeclaration(
// /*modifiers*/ undefined,
// /*dotDotDotToken*/ undefined,
// factory.createIdentifier("value")
// )],
// /*type*/ undefined,
// factory.createBlock([
// factory.createExpressionStatement(
// factory.createAssignment(
// accessor,
// factory.createIdentifier("value")
// )
// )
// ])
// );
// }
//
// function createESDecorateClassElementAccessObject(name: ESDecorateName, access: ESDecorateClassElementAccess) {
// const properties: ObjectLiteralElementLike[] = [];
// if (access.get) properties.push(createESDecorateClassElementAccessGetMethod(name));
// if (access.set) properties.push(createESDecorateClassElementAccessSetMethod(name));
// return factory.createObjectLiteralExpression(properties);
// }
//
// === Suggested approach (`access.get(obj)`, `access.set(obj, value)`, `access.has(obj)`) ===
//
// function createESDecorateClassElementAccessGetMethod(elementName: ESDecorateName) {
// const accessor = elementName.computed ?
// factory.createElementAccessExpression(factory.createIdentifier("obj"), elementName.name) :
// factory.createPropertyAccessExpression(factory.createIdentifier("obj"), elementName.name);
//
// return factory.createMethodDeclaration(
// /*modifiers*/ undefined,
// /*asteriskToken*/ undefined,
// "get",
// /*questionToken*/ undefined,
// /*typeParameters*/ undefined,
// [factory.createParameterDeclaration(
// /*modifiers*/ undefined,
// /*dotDotDotToken*/ undefined,
// factory.createIdentifier("obj")
// )],
// /*type*/ undefined,
// factory.createBlock([factory.createReturnStatement(accessor)])
// );
// }
//
// function createESDecorateClassElementAccessSetMethod(elementName: ESDecorateName) {
// const accessor = elementName.computed ?
// factory.createElementAccessExpression(factory.createIdentifier("obj"), elementName.name) :
// factory.createPropertyAccessExpression(factory.createIdentifier("obj"), elementName.name);
//
// return factory.createMethodDeclaration(
// /*modifiers*/ undefined,
// /*asteriskToken*/ undefined,
// "set",
// /*questionToken*/ undefined,
// /*typeParameters*/ undefined,
// [factory.createParameterDeclaration(
// /*modifiers*/ undefined,
// /*dotDotDotToken*/ undefined,
// factory.createIdentifier("obj")
// ),
// factory.createParameterDeclaration(
// /*modifiers*/ undefined,
// /*dotDotDotToken*/ undefined,
// factory.createIdentifier("value")
// )],
// /*type*/ undefined,
// factory.createBlock([
// factory.createExpressionStatement(
// factory.createAssignment(
// accessor,
// factory.createIdentifier("value")
// )
// )
// ])
// );
// }
//
// function createESDecorateClassElementAccessHasMethod(elementName: ESDecorateName) {
// const propertyName =
// elementName.computed ? elementName.name :
// isIdentifier(elementName.name) ? factory.createStringLiteralFromNode(elementName.name) :
// elementName.name;
//
// return factory.createMethodDeclaration(
// /*modifiers*/ undefined,
// /*asteriskToken*/ undefined,
// "has",
// /*questionToken*/ undefined,
// /*typeParameters*/ undefined,
// [factory.createParameterDeclaration(
// /*modifiers*/ undefined,
// /*dotDotDotToken*/ undefined,
// factory.createIdentifier("obj")
// )],
// /*type*/ undefined,
// factory.createBlock([factory.createReturnStatement(
// factory.createBinaryExpression(
// propertyName,
// SyntaxKind.InKeyword,
// factory.createIdentifier("obj")
// )
// )])
// );
// }
//
// function createESDecorateClassElementAccessObject(name: ESDecorateName, access: ESDecorateClassElementAccess) {
// const properties: ObjectLiteralElementLike[] = [];
// if (access.get) properties.push(createESDecorateClassElementAccessGetMethod(name));
// if (access.set) properties.push(createESDecorateClassElementAccessSetMethod(name));
// property.push(createESDecorateClassElementAccessHasMethod(name));
// return factory.createObjectLiteralExpression(properties);
// }

function createESDecorateClassElementAccessGetMethod(elementName: ESDecorateName) {
const accessor = elementName.computed ?
factory.createElementAccessExpression(factory.createIdentifier("obj"), elementName.name) :
factory.createPropertyAccessExpression(factory.createIdentifier("obj"), elementName.name);

return factory.createPropertyAssignment(
"get",
factory.createArrowFunction(
/*modifiers*/ undefined,
/*typeParameters*/ undefined,
[factory.createParameterDeclaration(
/*modifiers*/ undefined,
/*dotDotDotToken*/ undefined,
factory.createIdentifier("obj")
)],
/*type*/ undefined,
/*equalsGreaterThanToken*/ undefined,
accessor
)
);
}

function createESDecorateClassElementAccessSetMethod(elementName: ESDecorateName) {
const accessor = elementName.computed ?
factory.createElementAccessExpression(factory.createIdentifier("obj"), elementName.name) :
factory.createPropertyAccessExpression(factory.createIdentifier("obj"), elementName.name);

return factory.createPropertyAssignment(
"set",
factory.createArrowFunction(
/*modifiers*/ undefined,
/*typeParameters*/ undefined,
[factory.createParameterDeclaration(
/*modifiers*/ undefined,
/*dotDotDotToken*/ undefined,
factory.createIdentifier("obj")
),
factory.createParameterDeclaration(
/*modifiers*/ undefined,
/*dotDotDotToken*/ undefined,
factory.createIdentifier("value")
)],
/*type*/ undefined,
/*equalsGreaterThanToken*/ undefined,
factory.createBlock([
factory.createExpressionStatement(
factory.createAssignment(
accessor,
factory.createIdentifier("value")
)
)
])
)
);
}

function createESDecorateClassElementAccessHasMethod(elementName: ESDecorateName) {
const propertyName =
elementName.computed ? elementName.name :
isIdentifier(elementName.name) ? factory.createStringLiteralFromNode(elementName.name) :
elementName.name;

return factory.createPropertyAssignment(
"has",
factory.createArrowFunction(
/*modifiers*/ undefined,
/*typeParameters*/ undefined,
[factory.createParameterDeclaration(
/*modifiers*/ undefined,
/*dotDotDotToken*/ undefined,
factory.createIdentifier("obj")
)],
/*type*/ undefined,
/*equalsGreaterThanToken*/ undefined,
factory.createBinaryExpression(
propertyName,
SyntaxKind.InKeyword,
factory.createIdentifier("obj")
)
)
);
}

function createESDecorateClassElementAccessObject(name: ESDecorateName, access: ESDecorateClassElementAccess) {
const properties: ObjectLiteralElementLike[] = [];
properties.push(createESDecorateClassElementAccessHasMethod(name));
if (access.get) properties.push(createESDecorateClassElementAccessGetMethod(name));
if (access.set) properties.push(createESDecorateClassElementAccessSetMethod(name));
return factory.createObjectLiteralExpression(properties);
}

function createESDecorateClassElementContextObject(contextIn: ESDecorateClassElementContext) {
return factory.createObjectLiteralExpression([
factory.createPropertyAssignment(factory.createIdentifier("kind"), factory.createStringLiteral(contextIn.kind)),
factory.createPropertyAssignment(factory.createIdentifier("name"), contextIn.name.computed ? contextIn.name.name : factory.createStringLiteralFromNode(contextIn.name.name)),
factory.createPropertyAssignment(factory.createIdentifier("static"), contextIn.static ? factory.createTrue() : factory.createFalse()),
factory.createPropertyAssignment(factory.createIdentifier("private"), contextIn.private ? factory.createTrue() : factory.createFalse()),

// Disabled, pending resolution of https://github.com/tc39/proposal-decorators/issues/494
// factory.createPropertyAssignment(factory.createIdentifier("access"), createESDecorateClassElementAccessObject(contextIn.name, contextIn.access))
factory.createPropertyAssignment(factory.createIdentifier("access"), createESDecorateClassElementAccessObject(contextIn.name, contextIn.access))
]);
}

Expand Down
26 changes: 21 additions & 5 deletions src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7704,20 +7704,36 @@ namespace Parser {
function parseModifiers(allowDecorators: boolean, permitConstAsModifier?: boolean, stopOnStartOfClassStaticBlock?: boolean): NodeArray<ModifierLike> | undefined {
const pos = getNodePos();
let list: ModifierLike[] | undefined;
let modifier, hasSeenStaticModifier = false;
let decorator, modifier, hasSeenStaticModifier = false, hasLeadingModifier = false, hasTrailingDecorator = false;

// Decorators should be contiguous in a list of modifiers but can potentially appear in two places (i.e., `[...leadingDecorators, ...leadingModifiers, ...trailingDecorators, ...trailingModifiers]`).
// The leading modifiers *should* only contain `export` and `default` when trailingDecorators are present, but we'll handle errors for any other leading modifiers in the checker.
// It is illegal to have both leadingDecorators and trailingDecorators, but we will report that as a grammar check in the checker.

// parse leading decorators
if (allowDecorators && token() === SyntaxKind.AtToken) {
while (decorator = tryParseDecorator()) {
list = append(list, decorator);
}
}

// parse leading modifiers
while (modifier = tryParseModifier(hasSeenStaticModifier, permitConstAsModifier, stopOnStartOfClassStaticBlock)) {
if (modifier.kind === SyntaxKind.StaticKeyword) hasSeenStaticModifier = true;
list = append(list, modifier);
hasLeadingModifier = true;
}

// Decorators should be contiguous in a list of modifiers (i.e., `[...leadingModifiers, ...decorators, ...trailingModifiers]`).
// The leading modifiers *should* only contain `export` and `default` when decorators are present, but we'll handle errors for any other leading modifiers in the checker.
if (allowDecorators && token() === SyntaxKind.AtToken) {
let decorator;
// parse trailing decorators, but only if we parsed any leading modifiers
if (hasLeadingModifier && allowDecorators && token() === SyntaxKind.AtToken) {
while (decorator = tryParseDecorator()) {
list = append(list, decorator);
hasTrailingDecorator = true;
}
}

// parse trailing modifiers, but only if we parsed any trailing decorators
if (hasTrailingDecorator) {
while (modifier = tryParseModifier(hasSeenStaticModifier, permitConstAsModifier, stopOnStartOfClassStaticBlock)) {
if (modifier.kind === SyntaxKind.StaticKeyword) hasSeenStaticModifier = true;
list = append(list, modifier);
Expand Down
Loading