diff --git a/internal/checker/emitresolver.go b/internal/checker/emitresolver.go index 7bf3719e58..f5b68e1434 100644 --- a/internal/checker/emitresolver.go +++ b/internal/checker/emitresolver.go @@ -1270,9 +1270,20 @@ func (r *EmitResolver) TryJSTypeNodeToTypeNode(emitContext *printer.EmitContext, return requestNodeBuilder.TryJSTypeNodeToTypeNode(typeNode, enclosingDeclaration, flags, internalFlags, tracker) } -func (r *EmitResolver) GetBaseDeclarationsForPropertyDeclaration(node *ast.Node) []*ast.Node { +// IsThisPropertyAssignmentDeclarationRedundant reports whether a JS `this. = ...` expando +// assignment should be omitted from declaration emit because the member it would synthesize is +// already provided by an `extends` base type. This mirrors the skip condition in the checker's +// serializePropertySymbol: an inherited member is redundant when it is identical to the assigned +// one (same readonly-ness, optionality and type). Inherited accessors and methods are always +// treated as redundant here, since accessors merge oddly with value assignments (and run via the +// accessor at runtime), and `this`-expando props carry the ReplaceableByMethod contract, so a +// rebind such as `this.method = this.method.bind(this)` must not override the base method. +// +// Only `extends` base types are considered. Members coming from `implements` clauses are not +// inherited, so the class must redeclare them, and they are always emitted. +func (r *EmitResolver) IsThisPropertyAssignmentDeclarationRedundant(node *ast.Node) bool { if node == nil { - return nil + return false } r.checkerMu.Lock() @@ -1280,19 +1291,25 @@ func (r *EmitResolver) GetBaseDeclarationsForPropertyDeclaration(node *ast.Node) s := r.checker.getSymbolOfDeclaration(node) if s == nil || s.Parent == nil { - return nil + return false } parentType := r.checker.getDeclaredTypeOfSymbol(s.Parent) if parentType == nil { - return nil + return false } - bases := r.checker.getBaseTypes(parentType) - for _, b := range bases { - baseProp := r.checker.getPropertyOfObjectType(b, s.Name) - if baseProp != nil { - return baseProp.Declarations - // TODO: return base declarations from all base types if any callers actually look at the list + for _, base := range r.checker.getBaseTypes(parentType) { + baseProp := r.checker.getPropertyOfType(base, s.Name) + if baseProp == nil { + continue + } + if baseProp.Flags&(ast.SymbolFlagsAccessor|ast.SymbolFlagsMethod|ast.SymbolFlagsFunction) != 0 { + return true + } + if r.checker.isReadonlySymbol(baseProp) == r.checker.isReadonlySymbol(s) && + (s.Flags&ast.SymbolFlagsOptional) == (baseProp.Flags&ast.SymbolFlagsOptional) && + r.checker.isTypeIdenticalTo(r.checker.getTypeOfSymbol(s), r.checker.getTypeOfSymbol(baseProp)) { + return true } } - return nil + return false } diff --git a/internal/printer/emitresolver.go b/internal/printer/emitresolver.go index 189fe88cce..8ac8a00d9a 100644 --- a/internal/printer/emitresolver.go +++ b/internal/printer/emitresolver.go @@ -111,7 +111,7 @@ type EmitResolver interface { GetEnumMemberValue(node *ast.Node) evaluator.Result IsLateBound(node *ast.Node) bool IsOptionalParameter(node *ast.Node) bool - GetBaseDeclarationsForPropertyDeclaration(node *ast.Node) []*ast.Node + IsThisPropertyAssignmentDeclarationRedundant(node *ast.Node) bool // isolatedDeclarations-specific declaration emit GetPropertiesOfContainerFunction(node *ast.Node) []*ast.Symbol diff --git a/internal/transformers/declarations/transform.go b/internal/transformers/declarations/transform.go index b6596e0d77..3322b437cf 100644 --- a/internal/transformers/declarations/transform.go +++ b/internal/transformers/declarations/transform.go @@ -2066,9 +2066,8 @@ caseBlock: if thisTarget.ClassLikeData().HeritageClauses != nil && len(thisTarget.ClassLikeData().HeritageClauses.Nodes) > 0 && !isClassExtendingNull(thisTarget) { // there is a base type any assignments might be "from" tx.tracker.ReportInferenceFallback(thisTarget) // Add an isolated declarations error on this class - we can't know how to transform this prop into an assignment without referring to type information - decls := tx.resolver.GetBaseDeclarationsForPropertyDeclaration(node) - if len(decls) > 0 { - break caseBlock // property lightly overrides a property in a base type - skip it + if tx.resolver.IsThisPropertyAssignmentDeclarationRedundant(node) { + break caseBlock // skip assignments whose member is already provided by an `extends` base type (an inherited accessor/method, or an identical inherited property) // TODO: If the property has an explicit `@type` annotation, we should probably emit it (maybe with an `override` modifier) instead of skipping it } } diff --git a/testdata/baselines/reference/compiler/jsDeclarationEmitThisAssignmentWithBaseProperty.js b/testdata/baselines/reference/compiler/jsDeclarationEmitThisAssignmentWithBaseProperty.js new file mode 100644 index 0000000000..874fb37548 --- /dev/null +++ b/testdata/baselines/reference/compiler/jsDeclarationEmitThisAssignmentWithBaseProperty.js @@ -0,0 +1,112 @@ +//// [tests/cases/compiler/jsDeclarationEmitThisAssignmentWithBaseProperty.ts] //// + +//// [component.d.ts] +export class Component { + state: any; + constructor(props?: any); +} + +export class WithAccessor { + get value(): number; + set value(v: number); +} + +export class WithMethod { + method(): void; +} + +//// [main.js] +import { Component, WithAccessor, WithMethod } from "./component"; + +export class C1 extends Component { + state = { count: 0 }; +} + +export class C2 extends Component { + constructor() { + super({}); + this.state = { count: 0 }; + } +} + +export class C3 extends Component { + update() { + this.state = { count: 1 }; + } +} + +export class C4 extends WithAccessor { + constructor() { + super(); + this.value = 1; + } +} + +/** @implements {WithAccessor} */ +export class C5 { + constructor() { + this.value = 1; + } +} + +export class C6 extends WithMethod { + constructor() { + super(); + this.method = this.method.bind(this); + } +} + +//// [mainTs.ts] +import { Component } from "./component"; + +export class C1 extends Component { + state = { count: 0 }; +} + +export class C2 extends Component { + constructor() { + super({}); + this.state = { count: 0 }; + } +} + + + + +//// [main.d.ts] +import { Component, WithAccessor, WithMethod } from "./component"; +export declare class C1 extends Component { + state: { + count: number; + }; +} +export declare class C2 extends Component { + state: { + count: number; + }; + constructor(); +} +export declare class C3 extends Component { + update(): void; +} +export declare class C4 extends WithAccessor { + constructor(); +} +/** @implements {WithAccessor} */ +export declare class C5 implements WithAccessor { + value: number; + constructor(); +} +export declare class C6 extends WithMethod { + constructor(); +} +//// [mainTs.d.ts] +import { Component } from "./component"; +export declare class C1 extends Component { + state: { + count: number; + }; +} +export declare class C2 extends Component { + constructor(); +} diff --git a/testdata/baselines/reference/compiler/jsDeclarationEmitThisAssignmentWithBaseProperty.symbols b/testdata/baselines/reference/compiler/jsDeclarationEmitThisAssignmentWithBaseProperty.symbols new file mode 100644 index 0000000000..f01d9a8d48 --- /dev/null +++ b/testdata/baselines/reference/compiler/jsDeclarationEmitThisAssignmentWithBaseProperty.symbols @@ -0,0 +1,154 @@ +//// [tests/cases/compiler/jsDeclarationEmitThisAssignmentWithBaseProperty.ts] //// + +=== component.d.ts === +export class Component { +>Component : Symbol(Component, Decl(component.d.ts, 0, 0)) + + state: any; +>state : Symbol(Component.state, Decl(component.d.ts, 0, 24)) + + constructor(props?: any); +>props : Symbol(props, Decl(component.d.ts, 2, 16)) +} + +export class WithAccessor { +>WithAccessor : Symbol(WithAccessor, Decl(component.d.ts, 3, 1)) + + get value(): number; +>value : Symbol(WithAccessor.value, Decl(component.d.ts, 5, 27), Decl(component.d.ts, 6, 24)) + + set value(v: number); +>value : Symbol(WithAccessor.value, Decl(component.d.ts, 5, 27), Decl(component.d.ts, 6, 24)) +>v : Symbol(v, Decl(component.d.ts, 7, 14)) +} + +export class WithMethod { +>WithMethod : Symbol(WithMethod, Decl(component.d.ts, 8, 1)) + + method(): void; +>method : Symbol(WithMethod.method, Decl(component.d.ts, 10, 25)) +} + +=== main.js === +import { Component, WithAccessor, WithMethod } from "./component"; +>Component : Symbol(Component, Decl(main.js, 0, 8)) +>WithAccessor : Symbol(WithAccessor, Decl(main.js, 0, 19)) +>WithMethod : Symbol(WithMethod, Decl(main.js, 0, 33)) + +export class C1 extends Component { +>C1 : Symbol(C1, Decl(main.js, 0, 66)) +>Component : Symbol(Component, Decl(main.js, 0, 8)) + + state = { count: 0 }; +>state : Symbol(C1.state, Decl(main.js, 2, 35)) +>count : Symbol(count, Decl(main.js, 3, 13)) +} + +export class C2 extends Component { +>C2 : Symbol(C2, Decl(main.js, 4, 1)) +>Component : Symbol(Component, Decl(main.js, 0, 8)) + + constructor() { + super({}); +>super : Symbol(Component, Decl(component.d.ts, 0, 0)) + + this.state = { count: 0 }; +>this.state : Symbol(C2.state, Decl(main.js, 8, 18)) +>this : Symbol(C2, Decl(main.js, 4, 1)) +>state : Symbol(C2.state, Decl(main.js, 8, 18)) +>count : Symbol(count, Decl(main.js, 9, 22)) + } +} + +export class C3 extends Component { +>C3 : Symbol(C3, Decl(main.js, 11, 1)) +>Component : Symbol(Component, Decl(main.js, 0, 8)) + + update() { +>update : Symbol(C3.update, Decl(main.js, 13, 35)) + + this.state = { count: 1 }; +>this.state : Symbol(C3.state, Decl(main.js, 14, 14)) +>this : Symbol(C3, Decl(main.js, 11, 1)) +>state : Symbol(C3.state, Decl(main.js, 14, 14)) +>count : Symbol(count, Decl(main.js, 15, 22)) + } +} + +export class C4 extends WithAccessor { +>C4 : Symbol(C4, Decl(main.js, 17, 1)) +>WithAccessor : Symbol(WithAccessor, Decl(main.js, 0, 19)) + + constructor() { + super(); +>super : Symbol(WithAccessor, Decl(component.d.ts, 3, 1)) + + this.value = 1; +>this.value : Symbol(C4.value, Decl(main.js, 21, 16)) +>this : Symbol(C4, Decl(main.js, 17, 1)) +>value : Symbol(C4.value, Decl(main.js, 21, 16)) + } +} + +/** @implements {WithAccessor} */ +export class C5 { +>C5 : Symbol(C5, Decl(main.js, 24, 1)) + + constructor() { + this.value = 1; +>this.value : Symbol(C5.value, Decl(main.js, 28, 19)) +>this : Symbol(C5, Decl(main.js, 24, 1)) +>value : Symbol(C5.value, Decl(main.js, 28, 19)) + } +} + +export class C6 extends WithMethod { +>C6 : Symbol(C6, Decl(main.js, 31, 1)) +>WithMethod : Symbol(WithMethod, Decl(main.js, 0, 33)) + + constructor() { + super(); +>super : Symbol(WithMethod, Decl(component.d.ts, 8, 1)) + + this.method = this.method.bind(this); +>this.method : Symbol(C6.method, Decl(main.js, 35, 16)) +>this : Symbol(C6, Decl(main.js, 31, 1)) +>method : Symbol(C6.method, Decl(main.js, 35, 16)) +>this.method.bind : Symbol(CallableFunction.bind, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) +>this.method : Symbol(C6.method, Decl(main.js, 35, 16)) +>this : Symbol(C6, Decl(main.js, 31, 1)) +>method : Symbol(C6.method, Decl(main.js, 35, 16)) +>bind : Symbol(CallableFunction.bind, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --)) +>this : Symbol(C6, Decl(main.js, 31, 1)) + } +} + +=== mainTs.ts === +import { Component } from "./component"; +>Component : Symbol(Component, Decl(mainTs.ts, 0, 8)) + +export class C1 extends Component { +>C1 : Symbol(C1, Decl(mainTs.ts, 0, 40)) +>Component : Symbol(Component, Decl(mainTs.ts, 0, 8)) + + state = { count: 0 }; +>state : Symbol(C1.state, Decl(mainTs.ts, 2, 35)) +>count : Symbol(count, Decl(mainTs.ts, 3, 13)) +} + +export class C2 extends Component { +>C2 : Symbol(C2, Decl(mainTs.ts, 4, 1)) +>Component : Symbol(Component, Decl(mainTs.ts, 0, 8)) + + constructor() { + super({}); +>super : Symbol(Component, Decl(component.d.ts, 0, 0)) + + this.state = { count: 0 }; +>this.state : Symbol(Component.state, Decl(component.d.ts, 0, 24)) +>this : Symbol(C2, Decl(mainTs.ts, 4, 1)) +>state : Symbol(Component.state, Decl(component.d.ts, 0, 24)) +>count : Symbol(count, Decl(mainTs.ts, 9, 22)) + } +} + diff --git a/testdata/baselines/reference/compiler/jsDeclarationEmitThisAssignmentWithBaseProperty.types b/testdata/baselines/reference/compiler/jsDeclarationEmitThisAssignmentWithBaseProperty.types new file mode 100644 index 0000000000..68b7e075a2 --- /dev/null +++ b/testdata/baselines/reference/compiler/jsDeclarationEmitThisAssignmentWithBaseProperty.types @@ -0,0 +1,179 @@ +//// [tests/cases/compiler/jsDeclarationEmitThisAssignmentWithBaseProperty.ts] //// + +=== component.d.ts === +export class Component { +>Component : Component + + state: any; +>state : any + + constructor(props?: any); +>props : any +} + +export class WithAccessor { +>WithAccessor : WithAccessor + + get value(): number; +>value : number + + set value(v: number); +>value : number +>v : number +} + +export class WithMethod { +>WithMethod : WithMethod + + method(): void; +>method : () => void +} + +=== main.js === +import { Component, WithAccessor, WithMethod } from "./component"; +>Component : typeof Component +>WithAccessor : typeof WithAccessor +>WithMethod : typeof WithMethod + +export class C1 extends Component { +>C1 : C1 +>Component : Component + + state = { count: 0 }; +>state : { count: number; } +>{ count: 0 } : { count: number; } +>count : number +>0 : 0 +} + +export class C2 extends Component { +>C2 : C2 +>Component : Component + + constructor() { + super({}); +>super({}) : void +>super : typeof Component +>{} : {} + + this.state = { count: 0 }; +>this.state = { count: 0 } : { count: number; } +>this.state : any +>this : this +>state : any +>{ count: 0 } : { count: number; } +>count : number +>0 : 0 + } +} + +export class C3 extends Component { +>C3 : C3 +>Component : Component + + update() { +>update : () => void + + this.state = { count: 1 }; +>this.state = { count: 1 } : { count: number; } +>this.state : any +>this : this +>state : any +>{ count: 1 } : { count: number; } +>count : number +>1 : 1 + } +} + +export class C4 extends WithAccessor { +>C4 : C4 +>WithAccessor : WithAccessor + + constructor() { + super(); +>super() : void +>super : typeof WithAccessor + + this.value = 1; +>this.value = 1 : 1 +>this.value : any +>this : this +>value : any +>1 : 1 + } +} + +/** @implements {WithAccessor} */ +export class C5 { +>C5 : C5 + + constructor() { + this.value = 1; +>this.value = 1 : 1 +>this.value : any +>this : this +>value : any +>1 : 1 + } +} + +export class C6 extends WithMethod { +>C6 : C6 +>WithMethod : WithMethod + + constructor() { + super(); +>super() : void +>super : typeof WithMethod + + this.method = this.method.bind(this); +>this.method = this.method.bind(this) : () => void +>this.method : any +>this : this +>method : any +>this.method.bind(this) : () => void +>this.method.bind : { (this: T, thisArg: ThisParameterType): OmitThisParameter; (this: (this: T, ...args: [...A, ...B]) => R, thisArg: T, ...args: A): (...args: B) => R; } +>this.method : () => void +>this : this +>method : () => void +>bind : { (this: T, thisArg: ThisParameterType): OmitThisParameter; (this: (this: T, ...args: [...A, ...B]) => R, thisArg: T, ...args: A): (...args: B) => R; } +>this : this + } +} + +=== mainTs.ts === +import { Component } from "./component"; +>Component : typeof Component + +export class C1 extends Component { +>C1 : C1 +>Component : Component + + state = { count: 0 }; +>state : { count: number; } +>{ count: 0 } : { count: number; } +>count : number +>0 : 0 +} + +export class C2 extends Component { +>C2 : C2 +>Component : Component + + constructor() { + super({}); +>super({}) : void +>super : typeof Component +>{} : {} + + this.state = { count: 0 }; +>this.state = { count: 0 } : { count: number; } +>this.state : any +>this : this +>state : any +>{ count: 0 } : { count: number; } +>count : number +>0 : 0 + } +} + diff --git a/testdata/tests/cases/compiler/jsDeclarationEmitThisAssignmentWithBaseProperty.ts b/testdata/tests/cases/compiler/jsDeclarationEmitThisAssignmentWithBaseProperty.ts new file mode 100644 index 0000000000..c5c42c0f70 --- /dev/null +++ b/testdata/tests/cases/compiler/jsDeclarationEmitThisAssignmentWithBaseProperty.ts @@ -0,0 +1,75 @@ +// @allowJs: true +// @checkJs: true +// @declaration: true +// @emitDeclarationOnly: true +// @target: esnext + +// @filename: component.d.ts +export class Component { + state: any; + constructor(props?: any); +} + +export class WithAccessor { + get value(): number; + set value(v: number); +} + +export class WithMethod { + method(): void; +} + +// @filename: main.js +import { Component, WithAccessor, WithMethod } from "./component"; + +export class C1 extends Component { + state = { count: 0 }; +} + +export class C2 extends Component { + constructor() { + super({}); + this.state = { count: 0 }; + } +} + +export class C3 extends Component { + update() { + this.state = { count: 1 }; + } +} + +export class C4 extends WithAccessor { + constructor() { + super(); + this.value = 1; + } +} + +/** @implements {WithAccessor} */ +export class C5 { + constructor() { + this.value = 1; + } +} + +export class C6 extends WithMethod { + constructor() { + super(); + this.method = this.method.bind(this); + } +} + +// @filename: mainTs.ts +import { Component } from "./component"; + +export class C1 extends Component { + state = { count: 0 }; +} + +export class C2 extends Component { + constructor() { + super({}); + this.state = { count: 0 }; + } +}