Skip to content

Allow accessors in ambient class declarations #32787

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 2 commits into from
Aug 9, 2019
Merged

Allow accessors in ambient class declarations #32787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d7d144
Allow accessors in ambient class declarations
rbuckton Aug 9, 2019
c5683ac
Update src/compiler/transformers/declarations.ts
rbuckton Aug 9, 2019
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
57 changes: 25 additions & 32 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5935,7 +5935,9 @@ namespace ts {
// Otherwise, fall back to 'any'.
else {
if (setter) {
errorOrSuggestion(noImplicitAny, setter, Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation, symbolToString(symbol));
if (!isPrivateWithinAmbient(setter)) {
errorOrSuggestion(noImplicitAny, setter, Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation, symbolToString(symbol));
}
}
else {
Debug.assert(!!getter, "there must existed getter as we are current checking either setter or getter in this function");
Expand Down Expand Up @@ -33035,43 +33037,39 @@ namespace ts {
}

function checkGrammarAccessor(accessor: AccessorDeclaration): boolean {
const kind = accessor.kind;
if (languageVersion < ScriptTarget.ES5) {
return grammarErrorOnNode(accessor.name, Diagnostics.Accessors_are_only_available_when_targeting_ECMAScript_5_and_higher);
}
else if (accessor.flags & NodeFlags.Ambient) {
return grammarErrorOnNode(accessor.name, Diagnostics.An_accessor_cannot_be_declared_in_an_ambient_context);
}
else if (accessor.body === undefined && !hasModifier(accessor, ModifierFlags.Abstract)) {
return grammarErrorAtPos(accessor, accessor.end - 1, ";".length, Diagnostics._0_expected, "{");
if (!(accessor.flags & NodeFlags.Ambient)) {
if (languageVersion < ScriptTarget.ES5) {
return grammarErrorOnNode(accessor.name, Diagnostics.Accessors_are_only_available_when_targeting_ECMAScript_5_and_higher);
}
if (accessor.body === undefined && !hasModifier(accessor, ModifierFlags.Abstract)) {
return grammarErrorAtPos(accessor, accessor.end - 1, ";".length, Diagnostics._0_expected, "{");
}
}
else if (accessor.body && hasModifier(accessor, ModifierFlags.Abstract)) {
if (accessor.body && hasModifier(accessor, ModifierFlags.Abstract)) {
return grammarErrorOnNode(accessor, Diagnostics.An_abstract_accessor_cannot_have_an_implementation);
}
else if (accessor.typeParameters) {
if (accessor.typeParameters) {
return grammarErrorOnNode(accessor.name, Diagnostics.An_accessor_cannot_have_type_parameters);
}
else if (!doesAccessorHaveCorrectParameterCount(accessor)) {
if (!doesAccessorHaveCorrectParameterCount(accessor)) {
return grammarErrorOnNode(accessor.name,
kind === SyntaxKind.GetAccessor ?
accessor.kind === SyntaxKind.GetAccessor ?
Diagnostics.A_get_accessor_cannot_have_parameters :
Diagnostics.A_set_accessor_must_have_exactly_one_parameter);
}
else if (kind === SyntaxKind.SetAccessor) {
if (accessor.kind === SyntaxKind.SetAccessor) {
if (accessor.type) {
return grammarErrorOnNode(accessor.name, Diagnostics.A_set_accessor_cannot_have_a_return_type_annotation);
}
else {
const parameter = accessor.parameters[0];
if (parameter.dotDotDotToken) {
return grammarErrorOnNode(parameter.dotDotDotToken, Diagnostics.A_set_accessor_cannot_have_rest_parameter);
}
else if (parameter.questionToken) {
return grammarErrorOnNode(parameter.questionToken, Diagnostics.A_set_accessor_cannot_have_an_optional_parameter);
}
else if (parameter.initializer) {
return grammarErrorOnNode(accessor.name, Diagnostics.A_set_accessor_parameter_cannot_have_an_initializer);
}
const parameter = Debug.assertDefined(getSetAccessorValueParameter(accessor), "Return value does not match parameter count assertion.");
if (parameter.dotDotDotToken) {
return grammarErrorOnNode(parameter.dotDotDotToken, Diagnostics.A_set_accessor_cannot_have_rest_parameter);
}
if (parameter.questionToken) {
return grammarErrorOnNode(parameter.questionToken, Diagnostics.A_set_accessor_cannot_have_an_optional_parameter);
}
if (parameter.initializer) {
return grammarErrorOnNode(accessor.name, Diagnostics.A_set_accessor_parameter_cannot_have_an_initializer);
}
}
return false;
Expand Down Expand Up @@ -33559,14 +33557,9 @@ namespace ts {

function checkGrammarStatementInAmbientContext(node: Node): boolean {
if (node.flags & NodeFlags.Ambient) {
// An accessors is already reported about the ambient context
if (isAccessor(node.parent)) {
return getNodeLinks(node).hasReportedStatementInAmbientContext = true;
}

// Find containing block which is either Block, ModuleBlock, SourceFile
const links = getNodeLinks(node);
if (!links.hasReportedStatementInAmbientContext && isFunctionLike(node.parent)) {
if (!links.hasReportedStatementInAmbientContext && (isFunctionLike(node.parent) || isAccessor(node.parent))) {
return getNodeLinks(node).hasReportedStatementInAmbientContext = grammarErrorOnFirstToken(node, Diagnostics.An_implementation_cannot_be_declared_in_ambient_contexts);
}

Expand Down
4 changes: 0 additions & 4 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,6 @@
"category": "Error",
"code": 1085
},
"An accessor cannot be declared in an ambient context.": {
"category": "Error",
"code": 1086
},
"'{0}' modifier cannot appear on a constructor declaration.": {
"category": "Error",
"code": 1089
Expand Down
79 changes: 71 additions & 8 deletions src/compiler/transformers/declarations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ namespace ts {
}
}

function ensureParameter(p: ParameterDeclaration, modifierMask?: ModifierFlags): ParameterDeclaration {
function ensureParameter(p: ParameterDeclaration, modifierMask?: ModifierFlags, type?: TypeNode): ParameterDeclaration {
let oldDiag: typeof getSymbolAccessibilityDiagnostic | undefined;
if (!suppressNewDiagnosticContexts) {
oldDiag = getSymbolAccessibilityDiagnostic;
Expand All @@ -406,7 +406,7 @@ namespace ts {
p.dotDotDotToken,
filterBindingPatternInitializers(p.name),
resolver.isOptionalParameter(p) ? (p.questionToken || createToken(SyntaxKind.QuestionToken)) : undefined,
ensureType(p, p.type, /*ignorePrivate*/ true), // Ignore private param props, since this type is going straight back into a param
ensureType(p, type || p.type, /*ignorePrivate*/ true), // Ignore private param props, since this type is going straight back into a param
ensureNoInitializer(p)
);
if (!suppressNewDiagnosticContexts) {
Expand Down Expand Up @@ -535,6 +535,36 @@ namespace ts {
return createNodeArray(newParams, params.hasTrailingComma);
}

function updateAccessorParamsList(input: AccessorDeclaration, isPrivate: boolean) {
let newParams: ParameterDeclaration[] | undefined;
if (!isPrivate) {
const thisParameter = getThisParameter(input);
if (thisParameter) {
newParams = [ensureParameter(thisParameter)];
}
}
if (isSetAccessorDeclaration(input)) {
let newValueParameter: ParameterDeclaration | undefined;
if (!isPrivate) {
const valueParameter = getSetAccessorValueParameter(input);
if (valueParameter) {
const accessorType = getTypeAnnotationFromAllAccessorDeclarations(input, resolver.getAllAccessorDeclarations(input));
newValueParameter = ensureParameter(valueParameter, /*modifierMask*/ undefined, accessorType);
}
}
if (!newValueParameter) {
newValueParameter = createParameter(
/*decorators*/ undefined,
/*modifiers*/ undefined,
/*dotDotDotToken*/ undefined,
"value"
);
}
newParams = append(newParams, newValueParameter);
}
return createNodeArray(newParams || emptyArray) as NodeArray<ParameterDeclaration>;
}

function ensureTypeParams(node: Node, params: NodeArray<TypeParameterDeclaration> | undefined) {
return hasModifier(node, ModifierFlags.Private) ? undefined : visitNodes(params, visitDeclarationSubtree);
}
Expand Down Expand Up @@ -811,10 +841,33 @@ namespace ts {
return cleanup(sig);
}
case SyntaxKind.GetAccessor: {
// For now, only emit class accessors as accessors if they were already declared in an ambient context.
if (input.flags & NodeFlags.Ambient) {
const isPrivate = hasModifier(input, ModifierFlags.Private);
const accessorType = getTypeAnnotationFromAllAccessorDeclarations(input, resolver.getAllAccessorDeclarations(input));
return cleanup(updateGetAccessor(
input,
/*decorators*/ undefined,
ensureModifiers(input),
input.name,
updateAccessorParamsList(input, isPrivate),
!isPrivate ? ensureType(input, accessorType) : undefined,
/*body*/ undefined));
}
const newNode = ensureAccessor(input);
return cleanup(newNode);
}
case SyntaxKind.SetAccessor: {
// For now, only emit class accessors as accessors if they were already declared in an ambient context.
if (input.flags & NodeFlags.Ambient) {
return cleanup(updateSetAccessor(
input,
/*decorators*/ undefined,
ensureModifiers(input),
input.name,
updateAccessorParamsList(input, hasModifier(input, ModifierFlags.Private)),
/*body*/ undefined));
}
const newNode = ensureAccessor(input);
return cleanup(newNode);
}
Expand Down Expand Up @@ -1374,17 +1427,27 @@ namespace ts {
return maskModifierFlags(node, mask, additions);
}

function ensureAccessor(node: AccessorDeclaration): PropertyDeclaration | undefined {
const accessors = resolver.getAllAccessorDeclarations(node);
if (node.kind !== accessors.firstAccessor.kind) {
return;
}
function getTypeAnnotationFromAllAccessorDeclarations(node: AccessorDeclaration, accessors: AllAccessorDeclarations) {
let accessorType = getTypeAnnotationFromAccessor(node);
if (!accessorType && accessors.secondAccessor) {
if (!accessorType && node !== accessors.firstAccessor) {
accessorType = getTypeAnnotationFromAccessor(accessors.firstAccessor);
// If we end up pulling the type from the second accessor, we also need to change the diagnostic context to get the expected error message
getSymbolAccessibilityDiagnostic = createGetSymbolAccessibilityDiagnosticForNode(accessors.firstAccessor);
}
if (!accessorType && accessors.secondAccessor && node !== accessors.secondAccessor) {
accessorType = getTypeAnnotationFromAccessor(accessors.secondAccessor);
// If we end up pulling the type from the second accessor, we also need to change the diagnostic context to get the expected error message
getSymbolAccessibilityDiagnostic = createGetSymbolAccessibilityDiagnosticForNode(accessors.secondAccessor);
}
return accessorType;
}

function ensureAccessor(node: AccessorDeclaration): PropertyDeclaration | undefined {
const accessors = resolver.getAllAccessorDeclarations(node);
if (node.kind !== accessors.firstAccessor.kind) {
return;
}
const accessorType = getTypeAnnotationFromAllAccessorDeclarations(node, accessors);
const prop = createProperty(/*decorators*/ undefined, maskModifiers(node, /*mask*/ undefined, (!accessors.setAccessor) ? ModifierFlags.Readonly : ModifierFlags.None), node.name, node.questionToken, ensureType(node, accessorType), /*initializer*/ undefined);
const leadingsSyntheticCommentRanges = accessors.secondAccessor && getLeadingCommentRangesOfNode(accessors.secondAccessor, currentSourceFile);
if (leadingsSyntheticCommentRanges) {
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3518,7 +3518,7 @@ namespace ts {
return find(node.members, (member): member is ConstructorDeclaration & { body: FunctionBody } => isConstructorDeclaration(member) && nodeIsPresent(member.body));
}

function getSetAccessorValueParameter(accessor: SetAccessorDeclaration): ParameterDeclaration | undefined {
export function getSetAccessorValueParameter(accessor: SetAccessorDeclaration): ParameterDeclaration | undefined {
if (accessor && accessor.parameters.length > 0) {
const hasThis = accessor.parameters.length === 2 && parameterIsThisKeyword(accessor.parameters[0]);
return accessor.parameters[hasThis ? 1 : 0];
Expand Down Expand Up @@ -3553,7 +3553,7 @@ namespace ts {
return id.originalKeywordKind === SyntaxKind.ThisKeyword;
}

export function getAllAccessorDeclarations(declarations: NodeArray<Declaration>, accessor: AccessorDeclaration): AllAccessorDeclarations {
export function getAllAccessorDeclarations(declarations: readonly Declaration[], accessor: AccessorDeclaration): AllAccessorDeclarations {
// TODO: GH#18217
let firstAccessor!: AccessorDeclaration;
let secondAccessor!: AccessorDeclaration;
Expand Down
52 changes: 42 additions & 10 deletions src/services/codefixes/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,9 @@ namespace ts.codefix {
const modifiers = visibilityModifier ? createNodeArray([visibilityModifier]) : undefined;
const type = checker.getWidenedType(checker.getTypeOfSymbolAtLocation(symbol, enclosingDeclaration));
const optional = !!(symbol.flags & SymbolFlags.Optional);
const ambient = !!(enclosingDeclaration.flags & NodeFlags.Ambient);

switch (declaration.kind) {
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
case SyntaxKind.PropertySignature:
case SyntaxKind.PropertyDeclaration:
const typeNode = checker.typeToTypeNode(type, enclosingDeclaration, /*flags*/ undefined, getNoopSymbolTrackerWithResolver(context));
Expand All @@ -70,6 +69,37 @@ namespace ts.codefix {
typeNode,
/*initializer*/ undefined));
break;
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor: {
const allAccessors = getAllAccessorDeclarations(declarations, declaration as AccessorDeclaration);
const typeNode = checker.typeToTypeNode(type, enclosingDeclaration, /*flags*/ undefined, getNoopSymbolTrackerWithResolver(context));
const orderedAccessors = allAccessors.secondAccessor
? [allAccessors.firstAccessor, allAccessors.secondAccessor]
: [allAccessors.firstAccessor];
for (const accessor of orderedAccessors) {
if (isGetAccessorDeclaration(accessor)) {
out(createGetAccessor(
/*decorators*/ undefined,
modifiers,
name,
emptyArray,
typeNode,
ambient ? undefined : createStubbedMethodBody(preferences)));
}
else {
Debug.assertNode(accessor, isSetAccessorDeclaration);
const parameter = getSetAccessorValueParameter(accessor);
const parameterName = parameter && isIdentifier(parameter.name) ? idText(parameter.name) : undefined;
out(createSetAccessor(
/*decorators*/ undefined,
modifiers,
name,
createDummyParameters(1, [parameterName], [typeNode], 1, /*inJs*/ false),
ambient ? undefined : createStubbedMethodBody(preferences)));
}
}
break;
}
case SyntaxKind.MethodSignature:
case SyntaxKind.MethodDeclaration:
// The signature for the implementation appears as an entry in `signatures` iff
Expand All @@ -87,7 +117,7 @@ namespace ts.codefix {
if (declarations.length === 1) {
Debug.assert(signatures.length === 1);
const signature = signatures[0];
outputMethod(signature, modifiers, name, createStubbedMethodBody(preferences));
outputMethod(signature, modifiers, name, ambient ? undefined : createStubbedMethodBody(preferences));
break;
}

Expand All @@ -96,13 +126,15 @@ namespace ts.codefix {
outputMethod(signature, getSynthesizedDeepClones(modifiers, /*includeTrivia*/ false), getSynthesizedDeepClone(name, /*includeTrivia*/ false));
}

if (declarations.length > signatures.length) {
const signature = checker.getSignatureFromDeclaration(declarations[declarations.length - 1] as SignatureDeclaration)!;
outputMethod(signature, modifiers, name, createStubbedMethodBody(preferences));
}
else {
Debug.assert(declarations.length === signatures.length);
out(createMethodImplementingSignatures(signatures, name, optional, modifiers, preferences));
if (!ambient) {
if (declarations.length > signatures.length) {
const signature = checker.getSignatureFromDeclaration(declarations[declarations.length - 1] as SignatureDeclaration)!;
outputMethod(signature, modifiers, name, createStubbedMethodBody(preferences));
}
else {
Debug.assert(declarations.length === signatures.length);
out(createMethodImplementingSignatures(signatures, name, optional, modifiers, preferences));
}
}
break;
}
Expand Down
Loading