Skip to content

Commit

Permalink
Allow accessors in ambient class declarations
Browse files Browse the repository at this point in the history
  • Loading branch information
@rbuckton
rbuckton committed Aug 9, 2019
1 parent f333684 commit 3d7d144
Show file tree
Hide file tree
Showing 25 changed files with 537 additions and 174 deletions.
57 changes: 25 additions & 32 deletions src/compiler/checker.ts
View file
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
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
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
type || ensureType(p, 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
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
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

0 comments on commit 3d7d144

Please sign in to comment.