diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 32965372e83fc..44bd4bafaaaa5 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -33823,8 +33823,7 @@ namespace ts { const derivedPropertyFlags = derived.flags & SymbolFlags.PropertyOrAccessor; if (basePropertyFlags && derivedPropertyFlags) { // property/accessor is overridden with property/accessor - if (!compilerOptions.useDefineForClassFields - || baseDeclarationFlags & ModifierFlags.Abstract && !(base.valueDeclaration && isPropertyDeclaration(base.valueDeclaration) && base.valueDeclaration.initializer) + if (baseDeclarationFlags & ModifierFlags.Abstract && !(base.valueDeclaration && isPropertyDeclaration(base.valueDeclaration) && base.valueDeclaration.initializer) || base.valueDeclaration && base.valueDeclaration.parent.kind === SyntaxKind.InterfaceDeclaration || derived.valueDeclaration && isBinaryExpression(derived.valueDeclaration)) { // when the base property is abstract or from an interface, base/derived flags don't need to match @@ -33840,7 +33839,7 @@ namespace ts { Diagnostics._0_is_defined_as_a_property_in_class_1_but_is_overridden_here_in_2_as_an_accessor; error(getNameOfDeclaration(derived.valueDeclaration) || derived.valueDeclaration, errorMessage, symbolToString(base), typeToString(baseType), typeToString(type)); } - else { + else if (compilerOptions.useDefineForClassFields) { const uninitialized = find(derived.declarations, d => d.kind === SyntaxKind.PropertyDeclaration && !(d as PropertyDeclaration).initializer); if (uninitialized && !(derived.flags & SymbolFlags.Transient) diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index ef46447faacdb..5c050494e367b 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -5705,6 +5705,10 @@ "category": "Message", "code": 95118 }, + "Generate 'get' and 'set' accessors for all overriding properties": { + "category": "Message", + "code": 95119 + }, "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": { "category": "Error", diff --git a/src/services/codefixes/fixAddMissingMember.ts b/src/services/codefixes/fixAddMissingMember.ts index 27c73306693f8..f39234cdf17d7 100644 --- a/src/services/codefixes/fixAddMissingMember.ts +++ b/src/services/codefixes/fixAddMissingMember.ts @@ -78,19 +78,6 @@ namespace ts.codefix { }, }); - function getAllSupers(decl: ClassOrInterface | undefined, checker: TypeChecker): readonly ClassOrInterface[] { - const res: ClassLikeDeclaration[] = []; - while (decl) { - const superElement = getClassExtendsHeritageElement(decl); - const superSymbol = superElement && checker.getSymbolAtLocation(superElement.expression); - const superDecl = superSymbol && find(superSymbol.declarations, isClassLike); - if (superDecl) { res.push(superDecl); } - decl = superDecl; - } - return res; - } - - type ClassOrInterface = ClassLikeDeclaration | InterfaceDeclaration; const enum InfoKind { Enum, ClassOrInterface } interface EnumInfo { readonly kind: InfoKind.Enum; diff --git a/src/services/codefixes/fixPropertyOverrideAccessor.ts b/src/services/codefixes/fixPropertyOverrideAccessor.ts new file mode 100644 index 0000000000000..1941b5ccd5826 --- /dev/null +++ b/src/services/codefixes/fixPropertyOverrideAccessor.ts @@ -0,0 +1,57 @@ +/* @internal */ +namespace ts.codefix { + const errorCodes = [ + Diagnostics._0_is_defined_as_an_accessor_in_class_1_but_is_overridden_here_in_2_as_an_instance_property.code, + Diagnostics._0_is_defined_as_a_property_in_class_1_but_is_overridden_here_in_2_as_an_accessor.code, + ]; + const fixId = "fixPropertyOverrideAccessor"; + registerCodeFix({ + errorCodes, + getCodeActions(context) { + const edits = doChange(context.sourceFile, context.span.start, context.span.length, context.errorCode, context); + if (edits) { + return [createCodeFixAction(fixId, edits, Diagnostics.Generate_get_and_set_accessors, fixId, Diagnostics.Generate_get_and_set_accessors_for_all_overriding_properties)]; + } + }, + fixIds: [fixId], + + getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => { + const edits = doChange(diag.file, diag.start, diag.length, diag.code, context); + if (edits) { + for (const edit of edits) { + changes.pushRaw(context.sourceFile, edit); + } + } + }), + }); + + function doChange(file: SourceFile, start: number, length: number, code: number, context: CodeFixContext | CodeFixAllContext) { + let startPosition: number; + let endPosition: number; + if (code === Diagnostics._0_is_defined_as_an_accessor_in_class_1_but_is_overridden_here_in_2_as_an_instance_property.code) { + startPosition = start; + endPosition = start + length; + } + else if (code === Diagnostics._0_is_defined_as_a_property_in_class_1_but_is_overridden_here_in_2_as_an_accessor.code) { + const checker = context.program.getTypeChecker(); + const node = getTokenAtPosition(file, start).parent; + Debug.assert(isAccessor(node), "error span of fixPropertyOverrideAccessor should only be on an accessor"); + const containingClass = node.parent; + Debug.assert(isClassLike(containingClass), "erroneous accessors should only be inside classes"); + const base = singleOrUndefined(getAllSupers(containingClass, checker)); + if (!base) return []; + + const name = unescapeLeadingUnderscores(getTextOfPropertyName(node.name)); + const baseProp = checker.getPropertyOfType(checker.getTypeAtLocation(base), name); + if (!baseProp || !baseProp.valueDeclaration) return []; + + startPosition = baseProp.valueDeclaration.pos; + endPosition = baseProp.valueDeclaration.end; + file = getSourceFileOfNode(baseProp.valueDeclaration); + } + else { + Debug.fail("fixPropertyOverrideAccessor codefix got unexpected error code " + code); + } + return generateAccessorFromProperty(file, startPosition, endPosition, context, Diagnostics.Generate_get_and_set_accessors.message); + } +} diff --git a/src/services/codefixes/generateAccessors.ts b/src/services/codefixes/generateAccessors.ts new file mode 100644 index 0000000000000..7fed793a2f1ac --- /dev/null +++ b/src/services/codefixes/generateAccessors.ts @@ -0,0 +1,243 @@ +/* @internal */ +namespace ts.codefix { + type AcceptedDeclaration = ParameterPropertyDeclaration | PropertyDeclaration | PropertyAssignment; + type AcceptedNameType = Identifier | StringLiteral; + type ContainerDeclaration = ClassLikeDeclaration | ObjectLiteralExpression; + + interface Info { + readonly container: ContainerDeclaration; + readonly isStatic: boolean; + readonly isReadonly: boolean; + readonly type: TypeNode | undefined; + readonly declaration: AcceptedDeclaration; + readonly fieldName: AcceptedNameType; + readonly accessorName: AcceptedNameType; + readonly originalName: string; + readonly renameAccessor: boolean; + } + + export function generateAccessorFromProperty(file: SourceFile, start: number, end: number, context: textChanges.TextChangesContext, _actionName: string): FileTextChanges[] | undefined { + const fieldInfo = getAccessorConvertiblePropertyAtPosition(file, start, end); + if (!fieldInfo) return undefined; + + const changeTracker = textChanges.ChangeTracker.fromContext(context); + const { isStatic, isReadonly, fieldName, accessorName, originalName, type, container, declaration } = fieldInfo; + + suppressLeadingAndTrailingTrivia(fieldName); + suppressLeadingAndTrailingTrivia(accessorName); + suppressLeadingAndTrailingTrivia(declaration); + suppressLeadingAndTrailingTrivia(container); + + let accessorModifiers: ModifiersArray | undefined; + let fieldModifiers: ModifiersArray | undefined; + if (isClassLike(container)) { + const modifierFlags = getEffectiveModifierFlags(declaration); + if (isSourceFileJS(file)) { + const modifiers = createModifiers(modifierFlags); + accessorModifiers = modifiers; + fieldModifiers = modifiers; + } + else { + accessorModifiers = createModifiers(prepareModifierFlagsForAccessor(modifierFlags)); + fieldModifiers = createModifiers(prepareModifierFlagsForField(modifierFlags)); + } + } + + updateFieldDeclaration(changeTracker, file, declaration, fieldName, fieldModifiers); + + const getAccessor = generateGetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container); + suppressLeadingAndTrailingTrivia(getAccessor); + insertAccessor(changeTracker, file, getAccessor, declaration, container); + + if (isReadonly) { + // readonly modifier only existed in classLikeDeclaration + const constructor = getFirstConstructorWithBody(container); + if (constructor) { + updateReadonlyPropertyInitializerStatementConstructor(changeTracker, file, constructor, fieldName.text, originalName); + } + } + else { + const setAccessor = generateSetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container); + suppressLeadingAndTrailingTrivia(setAccessor); + insertAccessor(changeTracker, file, setAccessor, declaration, container); + } + + return changeTracker.getChanges(); + } + + function isConvertibleName(name: DeclarationName): name is AcceptedNameType { + return isIdentifier(name) || isStringLiteral(name); + } + + function isAcceptedDeclaration(node: Node): node is AcceptedDeclaration { + return isParameterPropertyDeclaration(node, node.parent) || isPropertyDeclaration(node) || isPropertyAssignment(node); + } + + function createPropertyName(name: string, originalName: AcceptedNameType) { + return isIdentifier(originalName) ? createIdentifier(name) : createLiteral(name); + } + + function createAccessorAccessExpression(fieldName: AcceptedNameType, isStatic: boolean, container: ContainerDeclaration) { + const leftHead = isStatic ? (container).name! : createThis(); // TODO: GH#18217 + return isIdentifier(fieldName) ? createPropertyAccess(leftHead, fieldName) : createElementAccess(leftHead, createLiteral(fieldName)); + } + + function createModifiers(modifierFlags: ModifierFlags): ModifiersArray | undefined { + return modifierFlags ? createNodeArray(createModifiersFromModifierFlags(modifierFlags)) : undefined; + } + + function prepareModifierFlagsForAccessor(modifierFlags: ModifierFlags): ModifierFlags { + modifierFlags &= ~ModifierFlags.Readonly; // avoid Readonly modifier because it will convert to get accessor + modifierFlags &= ~ModifierFlags.Private; + + if (!(modifierFlags & ModifierFlags.Protected)) { + modifierFlags |= ModifierFlags.Public; + } + + return modifierFlags; + } + + function prepareModifierFlagsForField(modifierFlags: ModifierFlags): ModifierFlags { + modifierFlags &= ~ModifierFlags.Public; + modifierFlags &= ~ModifierFlags.Protected; + modifierFlags |= ModifierFlags.Private; + return modifierFlags; + } + + export function getAccessorConvertiblePropertyAtPosition(file: SourceFile, start: number, end: number): Info | undefined { + const node = getTokenAtPosition(file, start); + const declaration = findAncestor(node.parent, isAcceptedDeclaration); + // make sure declaration have AccessibilityModifier or Static Modifier or Readonly Modifier + const meaning = ModifierFlags.AccessibilityModifier | ModifierFlags.Static | ModifierFlags.Readonly; + if (!declaration || !nodeOverlapsWithStartEnd(declaration.name, file, start, end) + || !isConvertibleName(declaration.name) || (getEffectiveModifierFlags(declaration) | meaning) !== meaning) return undefined; + + const name = declaration.name.text; + const startWithUnderscore = startsWithUnderscore(name); + const fieldName = createPropertyName(startWithUnderscore ? name : getUniqueName(`_${name}`, file), declaration.name); + const accessorName = createPropertyName(startWithUnderscore ? getUniqueName(name.substring(1), file) : name, declaration.name); + return { + isStatic: hasStaticModifier(declaration), + isReadonly: hasEffectiveReadonlyModifier(declaration), + type: getTypeAnnotationNode(declaration), + container: declaration.kind === SyntaxKind.Parameter ? declaration.parent.parent : declaration.parent, + originalName: (declaration.name).text, + declaration, + fieldName, + accessorName, + renameAccessor: startWithUnderscore + }; + } + + function generateGetAccessor(fieldName: AcceptedNameType, accessorName: AcceptedNameType, type: TypeNode | undefined, modifiers: ModifiersArray | undefined, isStatic: boolean, container: ContainerDeclaration) { + return createGetAccessor( + /*decorators*/ undefined, + modifiers, + accessorName, + /*parameters*/ undefined!, // TODO: GH#18217 + type, + createBlock([ + createReturn( + createAccessorAccessExpression(fieldName, isStatic, container) + ) + ], /*multiLine*/ true) + ); + } + + function generateSetAccessor(fieldName: AcceptedNameType, accessorName: AcceptedNameType, type: TypeNode | undefined, modifiers: ModifiersArray | undefined, isStatic: boolean, container: ContainerDeclaration) { + return createSetAccessor( + /*decorators*/ undefined, + modifiers, + accessorName, + [createParameter( + /*decorators*/ undefined, + /*modifiers*/ undefined, + /*dotDotDotToken*/ undefined, + createIdentifier("value"), + /*questionToken*/ undefined, + type + )], + createBlock([ + createStatement( + createAssignment( + createAccessorAccessExpression(fieldName, isStatic, container), + createIdentifier("value") + ) + ) + ], /*multiLine*/ true) + ); + } + + function updatePropertyDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: PropertyDeclaration, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined) { + const property = updateProperty( + declaration, + declaration.decorators, + modifiers, + fieldName, + declaration.questionToken || declaration.exclamationToken, + declaration.type, + declaration.initializer + ); + changeTracker.replaceNode(file, declaration, property); + } + + function updatePropertyAssignmentDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: PropertyAssignment, fieldName: AcceptedNameType) { + const assignment = updatePropertyAssignment(declaration, fieldName, declaration.initializer); + changeTracker.replacePropertyAssignment(file, declaration, assignment); + } + + function updateFieldDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: AcceptedDeclaration, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined) { + if (isPropertyDeclaration(declaration)) { + updatePropertyDeclaration(changeTracker, file, declaration, fieldName, modifiers); + } + else if (isPropertyAssignment(declaration)) { + updatePropertyAssignmentDeclaration(changeTracker, file, declaration, fieldName); + } + else { + changeTracker.replaceNode(file, declaration, + updateParameter(declaration, declaration.decorators, modifiers, declaration.dotDotDotToken, cast(fieldName, isIdentifier), declaration.questionToken, declaration.type, declaration.initializer)); + } + } + + function insertAccessor(changeTracker: textChanges.ChangeTracker, file: SourceFile, accessor: AccessorDeclaration, declaration: AcceptedDeclaration, container: ContainerDeclaration) { + isParameterPropertyDeclaration(declaration, declaration.parent) ? changeTracker.insertNodeAtClassStart(file, container, accessor) : + isPropertyAssignment(declaration) ? changeTracker.insertNodeAfterComma(file, declaration, accessor) : + changeTracker.insertNodeAfter(file, declaration, accessor); + } + + function updateReadonlyPropertyInitializerStatementConstructor(changeTracker: textChanges.ChangeTracker, file: SourceFile, constructor: ConstructorDeclaration, fieldName: string, originalName: string) { + if (!constructor.body) return; + constructor.body.forEachChild(function recur(node) { + if (isElementAccessExpression(node) && + node.expression.kind === SyntaxKind.ThisKeyword && + isStringLiteral(node.argumentExpression) && + node.argumentExpression.text === originalName && + isWriteAccess(node)) { + changeTracker.replaceNode(file, node.argumentExpression, createStringLiteral(fieldName)); + } + if (isPropertyAccessExpression(node) && node.expression.kind === SyntaxKind.ThisKeyword && node.name.text === originalName && isWriteAccess(node)) { + changeTracker.replaceNode(file, node.name, createIdentifier(fieldName)); + } + if (!isFunctionLike(node) && !isClassLike(node)) { + node.forEachChild(recur); + } + }); + } + + export function getAllSupers(decl: ClassOrInterface | undefined, checker: TypeChecker): readonly ClassOrInterface[] { + const res: ClassLikeDeclaration[] = []; + while (decl) { + const superElement = getClassExtendsHeritageElement(decl); + const superSymbol = superElement && checker.getSymbolAtLocation(superElement.expression); + if (!superSymbol) break; + const symbol = superSymbol.flags & SymbolFlags.Alias ? checker.getAliasedSymbol(superSymbol) : superSymbol; + const superDecl = find(symbol.declarations, isClassLike); + if (!superDecl) break; + res.push(superDecl); + decl = superDecl; + } + return res; + } + + export type ClassOrInterface = ClassLikeDeclaration | InterfaceDeclaration; +} diff --git a/src/services/refactors/generateGetAccessorAndSetAccessor.ts b/src/services/refactors/generateGetAccessorAndSetAccessor.ts index 024a27f5a926e..e942305f6b92c 100644 --- a/src/services/refactors/generateGetAccessorAndSetAccessor.ts +++ b/src/services/refactors/generateGetAccessorAndSetAccessor.ts @@ -2,254 +2,35 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { const actionName = "Generate 'get' and 'set' accessors"; const actionDescription = Diagnostics.Generate_get_and_set_accessors.message; - registerRefactor(actionName, { getEditsForAction, getAvailableActions }); - - type AcceptedDeclaration = ParameterPropertyDeclaration | PropertyDeclaration | PropertyAssignment; - type AcceptedNameType = Identifier | StringLiteral; - type ContainerDeclaration = ClassLikeDeclaration | ObjectLiteralExpression; - - interface Info { - readonly container: ContainerDeclaration; - readonly isStatic: boolean; - readonly isReadonly: boolean; - readonly type: TypeNode | undefined; - readonly declaration: AcceptedDeclaration; - readonly fieldName: AcceptedNameType; - readonly accessorName: AcceptedNameType; - readonly originalName: string; - readonly renameAccessor: boolean; - } - - function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { - if (!getConvertibleFieldAtPosition(context)) return emptyArray; - - return [{ - name: actionName, - description: actionDescription, - actions: [ - { - name: actionName, - description: actionDescription - } - ] - }]; - } - - function getEditsForAction(context: RefactorContext, _actionName: string): RefactorEditInfo | undefined { - const { file } = context; - - const fieldInfo = getConvertibleFieldAtPosition(context); - if (!fieldInfo) return undefined; - - const changeTracker = textChanges.ChangeTracker.fromContext(context); - const { isStatic, isReadonly, fieldName, accessorName, originalName, type, container, declaration, renameAccessor } = fieldInfo; - - suppressLeadingAndTrailingTrivia(fieldName); - suppressLeadingAndTrailingTrivia(accessorName); - suppressLeadingAndTrailingTrivia(declaration); - suppressLeadingAndTrailingTrivia(container); - - let accessorModifiers: ModifiersArray | undefined; - let fieldModifiers: ModifiersArray | undefined; - if (isClassLike(container)) { - const modifierFlags = getEffectiveModifierFlags(declaration); - if (isSourceFileJS(file)) { - const modifiers = createModifiers(modifierFlags); - accessorModifiers = modifiers; - fieldModifiers = modifiers; - } - else { - accessorModifiers = createModifiers(prepareModifierFlagsForAccessor(modifierFlags)); - fieldModifiers = createModifiers(prepareModifierFlagsForField(modifierFlags)); - } - } - - updateFieldDeclaration(changeTracker, file, declaration, fieldName, fieldModifiers); - - const getAccessor = generateGetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container); - suppressLeadingAndTrailingTrivia(getAccessor); - insertAccessor(changeTracker, file, getAccessor, declaration, container); - - if (isReadonly) { - // readonly modifier only existed in classLikeDeclaration - const constructor = getFirstConstructorWithBody(container); - if (constructor) { - updateReadonlyPropertyInitializerStatementConstructor(changeTracker, file, constructor, fieldName.text, originalName); - } + registerRefactor(actionName, { + getEditsForAction(context, actionName) { + if (!context.endPosition) return undefined; + const info = codefix.getAccessorConvertiblePropertyAtPosition(context.file, context.startPosition, context.endPosition); + if (!info) return undefined; + const edits = codefix.generateAccessorFromProperty(context.file, context.startPosition, context.endPosition, context, actionName); + if (!edits) return undefined; + + const renameFilename = context.file.fileName; + const nameNeedRename = info.renameAccessor ? info.accessorName : info.fieldName; + const renameLocationOffset = isIdentifier(nameNeedRename) ? 0 : -1; + const renameLocation = renameLocationOffset + getRenameLocation(edits, renameFilename, nameNeedRename.text, /*preferLastLocation*/ isParameter(info.declaration)); + + return { renameFilename, renameLocation, edits }; + }, + getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { + if (!context.endPosition) return emptyArray; + if (!codefix.getAccessorConvertiblePropertyAtPosition(context.file, context.startPosition, context.endPosition)) return emptyArray; + + return [{ + name: actionName, + description: actionDescription, + actions: [ + { + name: actionName, + description: actionDescription + } + ] + }]; } - else { - const setAccessor = generateSetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container); - suppressLeadingAndTrailingTrivia(setAccessor); - insertAccessor(changeTracker, file, setAccessor, declaration, container); - } - - const edits = changeTracker.getChanges(); - const renameFilename = file.fileName; - - const nameNeedRename = renameAccessor ? accessorName : fieldName; - const renameLocationOffset = isIdentifier(nameNeedRename) ? 0 : -1; - const renameLocation = renameLocationOffset + getRenameLocation(edits, renameFilename, nameNeedRename.text, /*preferLastLocation*/ isParameter(declaration)); - return { renameFilename, renameLocation, edits }; - } - - function isConvertibleName(name: DeclarationName): name is AcceptedNameType { - return isIdentifier(name) || isStringLiteral(name); - } - - function isAcceptedDeclaration(node: Node): node is AcceptedDeclaration { - return isParameterPropertyDeclaration(node, node.parent) || isPropertyDeclaration(node) || isPropertyAssignment(node); - } - - function createPropertyName(name: string, originalName: AcceptedNameType) { - return isIdentifier(originalName) ? createIdentifier(name) : createLiteral(name); - } - - function createAccessorAccessExpression(fieldName: AcceptedNameType, isStatic: boolean, container: ContainerDeclaration) { - const leftHead = isStatic ? (container).name! : createThis(); // TODO: GH#18217 - return isIdentifier(fieldName) ? createPropertyAccess(leftHead, fieldName) : createElementAccess(leftHead, createLiteral(fieldName)); - } - - function createModifiers(modifierFlags: ModifierFlags): ModifiersArray | undefined { - return modifierFlags ? createNodeArray(createModifiersFromModifierFlags(modifierFlags)) : undefined; - } - - function prepareModifierFlagsForAccessor(modifierFlags: ModifierFlags): ModifierFlags { - modifierFlags &= ~ModifierFlags.Readonly; // avoid Readonly modifier because it will convert to get accessor - modifierFlags &= ~ModifierFlags.Private; - - if (!(modifierFlags & ModifierFlags.Protected)) { - modifierFlags |= ModifierFlags.Public; - } - - return modifierFlags; - } - - function prepareModifierFlagsForField(modifierFlags: ModifierFlags): ModifierFlags { - modifierFlags &= ~ModifierFlags.Public; - modifierFlags &= ~ModifierFlags.Protected; - modifierFlags |= ModifierFlags.Private; - return modifierFlags; - } - - function getConvertibleFieldAtPosition(context: RefactorContext): Info | undefined { - const { file, startPosition, endPosition } = context; - - const node = getTokenAtPosition(file, startPosition); - const declaration = findAncestor(node.parent, isAcceptedDeclaration); - // make sure declaration have AccessibilityModifier or Static Modifier or Readonly Modifier - const meaning = ModifierFlags.AccessibilityModifier | ModifierFlags.Static | ModifierFlags.Readonly; - if (!declaration || !nodeOverlapsWithStartEnd(declaration.name, file, startPosition, endPosition!) // TODO: GH#18217 - || !isConvertibleName(declaration.name) || (getEffectiveModifierFlags(declaration) | meaning) !== meaning) return undefined; - - const name = declaration.name.text; - const startWithUnderscore = startsWithUnderscore(name); - const fieldName = createPropertyName(startWithUnderscore ? name : getUniqueName(`_${name}`, file), declaration.name); - const accessorName = createPropertyName(startWithUnderscore ? getUniqueName(name.substring(1), file) : name, declaration.name); - return { - isStatic: hasStaticModifier(declaration), - isReadonly: hasEffectiveReadonlyModifier(declaration), - type: getTypeAnnotationNode(declaration), - container: declaration.kind === SyntaxKind.Parameter ? declaration.parent.parent : declaration.parent, - originalName: (declaration.name).text, - declaration, - fieldName, - accessorName, - renameAccessor: startWithUnderscore - }; - } - - function generateGetAccessor(fieldName: AcceptedNameType, accessorName: AcceptedNameType, type: TypeNode | undefined, modifiers: ModifiersArray | undefined, isStatic: boolean, container: ContainerDeclaration) { - return createGetAccessor( - /*decorators*/ undefined, - modifiers, - accessorName, - /*parameters*/ undefined!, // TODO: GH#18217 - type, - createBlock([ - createReturn( - createAccessorAccessExpression(fieldName, isStatic, container) - ) - ], /*multiLine*/ true) - ); - } - - function generateSetAccessor(fieldName: AcceptedNameType, accessorName: AcceptedNameType, type: TypeNode | undefined, modifiers: ModifiersArray | undefined, isStatic: boolean, container: ContainerDeclaration) { - return createSetAccessor( - /*decorators*/ undefined, - modifiers, - accessorName, - [createParameter( - /*decorators*/ undefined, - /*modifiers*/ undefined, - /*dotDotDotToken*/ undefined, - createIdentifier("value"), - /*questionToken*/ undefined, - type - )], - createBlock([ - createStatement( - createAssignment( - createAccessorAccessExpression(fieldName, isStatic, container), - createIdentifier("value") - ) - ) - ], /*multiLine*/ true) - ); - } - - function updatePropertyDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: PropertyDeclaration, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined) { - const property = updateProperty( - declaration, - declaration.decorators, - modifiers, - fieldName, - declaration.questionToken || declaration.exclamationToken, - declaration.type, - declaration.initializer - ); - changeTracker.replaceNode(file, declaration, property); - } - - function updatePropertyAssignmentDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: PropertyAssignment, fieldName: AcceptedNameType) { - const assignment = updatePropertyAssignment(declaration, fieldName, declaration.initializer); - changeTracker.replacePropertyAssignment(file, declaration, assignment); - } - - function updateFieldDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: AcceptedDeclaration, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined) { - if (isPropertyDeclaration(declaration)) { - updatePropertyDeclaration(changeTracker, file, declaration, fieldName, modifiers); - } - else if (isPropertyAssignment(declaration)) { - updatePropertyAssignmentDeclaration(changeTracker, file, declaration, fieldName); - } - else { - changeTracker.replaceNode(file, declaration, - updateParameter(declaration, declaration.decorators, modifiers, declaration.dotDotDotToken, cast(fieldName, isIdentifier), declaration.questionToken, declaration.type, declaration.initializer)); - } - } - - function insertAccessor(changeTracker: textChanges.ChangeTracker, file: SourceFile, accessor: AccessorDeclaration, declaration: AcceptedDeclaration, container: ContainerDeclaration) { - isParameterPropertyDeclaration(declaration, declaration.parent) ? changeTracker.insertNodeAtClassStart(file, container, accessor) : - isPropertyAssignment(declaration) ? changeTracker.insertNodeAfterComma(file, declaration, accessor) : - changeTracker.insertNodeAfter(file, declaration, accessor); - } - - function updateReadonlyPropertyInitializerStatementConstructor(changeTracker: textChanges.ChangeTracker, file: SourceFile, constructor: ConstructorDeclaration, fieldName: string, originalName: string) { - if (!constructor.body) return; - constructor.body.forEachChild(function recur(node) { - if (isElementAccessExpression(node) && - node.expression.kind === SyntaxKind.ThisKeyword && - isStringLiteral(node.argumentExpression) && - node.argumentExpression.text === originalName && - isWriteAccess(node)) { - changeTracker.replaceNode(file, node.argumentExpression, createStringLiteral(fieldName)); - } - if (isPropertyAccessExpression(node) && node.expression.kind === SyntaxKind.ThisKeyword && node.name.text === originalName && isWriteAccess(node)) { - changeTracker.replaceNode(file, node.name, createIdentifier(fieldName)); - } - if (!isFunctionLike(node) && !isClassLike(node)) { - node.forEachChild(recur); - } - }); - } + }); } diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index f3e6e5ce4566b..afa2e93c8d136 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -85,10 +85,12 @@ "codefixes/fixJSDocTypes.ts", "codefixes/fixMissingCallParentheses.ts", "codefixes/fixAwaitInSyncFunction.ts", + "codefixes/fixPropertyOverrideAccessor.ts", "codefixes/inferFromUsage.ts", "codefixes/fixReturnTypeInAsyncFunction.ts", "codefixes/disableJsDiagnostics.ts", "codefixes/helpers.ts", + "codefixes/generateAccessors.ts", "codefixes/fixInvalidImportSyntax.ts", "codefixes/fixStrictClassInitialization.ts", "codefixes/requireInTs.ts", diff --git a/tests/baselines/reference/accessorsOverrideProperty6.errors.txt b/tests/baselines/reference/accessorsOverrideProperty6.errors.txt new file mode 100644 index 0000000000000..ad3c13e6849e9 --- /dev/null +++ b/tests/baselines/reference/accessorsOverrideProperty6.errors.txt @@ -0,0 +1,24 @@ +tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty6.ts(5,9): error TS2611: 'p' is defined as a property in class 'A', but is overridden here in 'B' as an accessor. +tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty6.ts(12,9): error TS2611: 'p' is defined as a property in class 'C', but is overridden here in 'D' as an accessor. + + +==== tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty6.ts (2 errors) ==== + class A { + p = 'yep' + } + class B extends A { + get p() { return 'oh no' } // error + ~ +!!! error TS2611: 'p' is defined as a property in class 'A', but is overridden here in 'B' as an accessor. + } + class C { + p = 101 + } + class D extends C { + _secret = 11 + get p() { return this._secret } // error + ~ +!!! error TS2611: 'p' is defined as a property in class 'C', but is overridden here in 'D' as an accessor. + set p(value) { this._secret = value } // error + } + \ No newline at end of file diff --git a/tests/baselines/reference/inheritanceMemberAccessorOverridingProperty.errors.txt b/tests/baselines/reference/inheritanceMemberAccessorOverridingProperty.errors.txt index 98b9d349d64fe..0872ab47f2ec6 100644 --- a/tests/baselines/reference/inheritanceMemberAccessorOverridingProperty.errors.txt +++ b/tests/baselines/reference/inheritanceMemberAccessorOverridingProperty.errors.txt @@ -1,8 +1,9 @@ tests/cases/compiler/inheritanceMemberAccessorOverridingProperty.ts(6,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher. +tests/cases/compiler/inheritanceMemberAccessorOverridingProperty.ts(6,9): error TS2611: 'x' is defined as a property in class 'a', but is overridden here in 'b' as an accessor. tests/cases/compiler/inheritanceMemberAccessorOverridingProperty.ts(9,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher. -==== tests/cases/compiler/inheritanceMemberAccessorOverridingProperty.ts (2 errors) ==== +==== tests/cases/compiler/inheritanceMemberAccessorOverridingProperty.ts (3 errors) ==== class a { x: string; } @@ -11,6 +12,8 @@ tests/cases/compiler/inheritanceMemberAccessorOverridingProperty.ts(9,9): error get x() { ~ !!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher. + ~ +!!! error TS2611: 'x' is defined as a property in class 'a', but is overridden here in 'b' as an accessor. return "20"; } set x(aValue: string) { diff --git a/tests/baselines/reference/inheritanceMemberPropertyOverridingAccessor.errors.txt b/tests/baselines/reference/inheritanceMemberPropertyOverridingAccessor.errors.txt index ec8ec05a8aeb5..61e7e78942d35 100644 --- a/tests/baselines/reference/inheritanceMemberPropertyOverridingAccessor.errors.txt +++ b/tests/baselines/reference/inheritanceMemberPropertyOverridingAccessor.errors.txt @@ -1,8 +1,9 @@ tests/cases/compiler/inheritanceMemberPropertyOverridingAccessor.ts(3,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher. tests/cases/compiler/inheritanceMemberPropertyOverridingAccessor.ts(6,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher. +tests/cases/compiler/inheritanceMemberPropertyOverridingAccessor.ts(12,5): error TS2610: 'x' is defined as an accessor in class 'a', but is overridden here in 'b' as an instance property. -==== tests/cases/compiler/inheritanceMemberPropertyOverridingAccessor.ts (2 errors) ==== +==== tests/cases/compiler/inheritanceMemberPropertyOverridingAccessor.ts (3 errors) ==== class a { private __x: () => string; get x() { @@ -19,4 +20,6 @@ tests/cases/compiler/inheritanceMemberPropertyOverridingAccessor.ts(6,9): error class b extends a { x: () => string; + ~ +!!! error TS2610: 'x' is defined as an accessor in class 'a', but is overridden here in 'b' as an instance property. } \ No newline at end of file diff --git a/tests/baselines/reference/tscWatch/programUpdates/updates-diagnostics-and-emit-when-useDefineForClassFields-changes.js b/tests/baselines/reference/tscWatch/programUpdates/updates-diagnostics-and-emit-when-useDefineForClassFields-changes.js index 721df9bc34376..0e2e3b2a407c6 100644 --- a/tests/baselines/reference/tscWatch/programUpdates/updates-diagnostics-and-emit-when-useDefineForClassFields-changes.js +++ b/tests/baselines/reference/tscWatch/programUpdates/updates-diagnostics-and-emit-when-useDefineForClassFields-changes.js @@ -37,7 +37,13 @@ Output:: [12:00:13 AM] Starting compilation in watch mode... -[12:00:16 AM] Found 0 errors. Watching for file changes. +a.ts:2:21 - error TS2610: 'prop' is defined as an accessor in class 'C', but is overridden here in 'D' as an instance property. + +2 class D extends C { prop = 1; } +   ~~~~ + + +[12:00:16 AM] Found 1 error. Watching for file changes. diff --git a/tests/cases/fourslash/codeFixPropertyOverrideAccess.ts b/tests/cases/fourslash/codeFixPropertyOverrideAccess.ts new file mode 100644 index 0000000000000..d930f76e46838 --- /dev/null +++ b/tests/cases/fourslash/codeFixPropertyOverrideAccess.ts @@ -0,0 +1,27 @@ +/// + +// @strict: true + +//// class A { +//// get x() { return 1 } +//// } +//// class B extends A { +//// x = 2 +//// } + +verify.codeFix({ + description: `Generate 'get' and 'set' accessors`, + newFileContent: `class A { + get x() { return 1 } +} +class B extends A { + private _x = 2 + public get x() { + return this._x + } + public set x(value) { + this._x = value + } +}`, + index: 0 +}) diff --git a/tests/cases/fourslash/codeFixPropertyOverrideAccess2.ts b/tests/cases/fourslash/codeFixPropertyOverrideAccess2.ts new file mode 100644 index 0000000000000..3146d1364fece --- /dev/null +++ b/tests/cases/fourslash/codeFixPropertyOverrideAccess2.ts @@ -0,0 +1,27 @@ +/// + +// @strict: true + +//// class A { +//// x = 1 +//// } +//// class B extends A { +//// get x() { return 2 } +//// } + +verify.codeFix({ + description: `Generate 'get' and 'set' accessors`, + newFileContent: `class A { + private _x = 1 + public get x() { + return this._x + } + public set x(value) { + this._x = value + } +} +class B extends A { + get x() { return 2 } +}`, + index: 0 +}) diff --git a/tests/cases/fourslash/codeFixPropertyOverrideAccess3.ts b/tests/cases/fourslash/codeFixPropertyOverrideAccess3.ts new file mode 100644 index 0000000000000..cc83c8e067f09 --- /dev/null +++ b/tests/cases/fourslash/codeFixPropertyOverrideAccess3.ts @@ -0,0 +1,28 @@ +/// + +// @strict: true + +// @Filename: foo.ts +//// import { A } from './source' +//// class B extends A { +//// get x() { return 2 } +//// } +// @Filename: source.ts +//// export class A { +//// x = 1 +//// } + +verify.codeFix({ + description: `Generate 'get' and 'set' accessors`, + newFileContent: { + '/tests/cases/fourslash/source.ts': `export class A { + private _x = 1; + public get x() { + return this._x; + } + public set x(value) { + this._x = value; + } +}`}, + index: 0 +}) diff --git a/tests/cases/fourslash/codeFixPropertyOverrideAccess_all.ts b/tests/cases/fourslash/codeFixPropertyOverrideAccess_all.ts new file mode 100644 index 0000000000000..5cc9a7a8eaa75 --- /dev/null +++ b/tests/cases/fourslash/codeFixPropertyOverrideAccess_all.ts @@ -0,0 +1,45 @@ +/// + +// @strict: true + +//// class A { +//// get x() { return 1 } +//// } +//// class B extends A { +//// x = 2 +//// } +//// class C { +//// get x() { return 3 } +//// } +//// class D extends C { +//// x = 4 +//// } + +verify.codeFixAll({ + fixId: "fixPropertyOverrideAccessor", + fixAllDescription: "Generate 'get' and 'set' accessors for all overriding properties", + newFileContent: `class A { + get x() { return 1 } +} +class B extends A { + private _x = 2 + public get x() { + return this._x + } + public set x(value) { + this._x = value + } +} +class C { + get x() { return 3 } +} +class D extends C { + private _x = 4 + public get x() { + return this._x + } + public set x(value) { + this._x = value + } +}`, +})