Skip to content

Fix isPropertyDeclaredInAncestorClass function #2376

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
ahejlsberg merged 2 commits into from
Dec 14, 2025
Merged

Fix isPropertyDeclaredInAncestorClass function #2376

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
9ffc6d9
Fix isPropertyDeclaredInAncestorClass function
ahejlsberg Dec 13, 2025
a740f6c
Add regression test
ahejlsberg Dec 13, 2025
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
18 changes: 5 additions & 13 deletions internal/checker/checker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11368,21 +11368,13 @@ func (c *Checker) isOptionalPropertyDeclaration(node *ast.Node) bool {
}

func (c *Checker) isPropertyDeclaredInAncestorClass(prop *ast.Symbol) bool {
if prop.Parent.Flags&ast.SymbolFlagsClass == 0 {
return false
}
classType := c.getDeclaredTypeOfSymbol(prop.Parent)
for {
Copy link
Member Author

@ahejlsberg ahejlsberg Dec 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loop here isn't necessary since getPropertyOfType on the base class will get any inherited property (and not just a property in the immediate base class as the loop seems to imply).

baseTypes := c.getBaseTypes(classType)
if len(baseTypes) == 0 {
return false
}
classType = baseTypes[0]
superProperty := c.getPropertyOfType(classType, prop.Name)
if superProperty != nil && superProperty.ValueDeclaration != nil {
return true
if prop.Parent.Flags&ast.SymbolFlagsClass != 0 {
if baseTypes := c.getBaseTypes(c.getDeclaredTypeOfSymbol(prop.Parent)); len(baseTypes) != 0 {
superProperty := c.getPropertyOfType(baseTypes[0], prop.Name)
return superProperty != nil && superProperty.ValueDeclaration != nil
Comment on lines +11371 to +11374
Copy link

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refactored implementation only checks the immediate base type and no longer traverses the entire inheritance hierarchy. The original code correctly looped through all ancestor classes to check if the property was declared in any ancestor. This change will cause incorrect behavior when a property is declared in a grandparent or higher ancestor class, as those declarations will not be detected.

The loop should be restored to properly check all ancestors in the inheritance chain, not just the immediate parent.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@ahejlsberg ahejlsberg Dec 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong. getPropertyOfType on the immediate base type checks all inherited properties, not just those from the immediate base. Thus, the loop is unnecessary and actually causes a panic when a base type is an intersection.

}
}
return false
}

/**
Expand Down
17 changes: 17 additions & 0 deletions testdata/baselines/reference/compiler/checkInheritedProperty.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
checkInheritedProperty.ts(7,14): error TS2729: Property 'b' is used before its initialization.


==== checkInheritedProperty.ts (1 errors) ====
class Base {
}

declare const BaseFactory: new() => Base & { c: string }

class Derived extends BaseFactory {
a = this.b
~
!!! error TS2729: Property 'b' is used before its initialization.
!!! related TS2728 checkInheritedProperty.ts:8:5: 'b' is declared here.
b = "abc"
}

26 changes: 26 additions & 0 deletions testdata/baselines/reference/compiler/checkInheritedProperty.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//// [tests/cases/compiler/checkInheritedProperty.ts] ////

=== checkInheritedProperty.ts ===
class Base {
>Base : Symbol(Base, Decl(checkInheritedProperty.ts, 0, 0))
}

declare const BaseFactory: new() => Base & { c: string }
>BaseFactory : Symbol(BaseFactory, Decl(checkInheritedProperty.ts, 3, 13))
>Base : Symbol(Base, Decl(checkInheritedProperty.ts, 0, 0))
>c : Symbol(c, Decl(checkInheritedProperty.ts, 3, 44))

class Derived extends BaseFactory {
>Derived : Symbol(Derived, Decl(checkInheritedProperty.ts, 3, 56))
>BaseFactory : Symbol(BaseFactory, Decl(checkInheritedProperty.ts, 3, 13))

a = this.b
>a : Symbol(Derived.a, Decl(checkInheritedProperty.ts, 5, 35))
>this.b : Symbol(Derived.b, Decl(checkInheritedProperty.ts, 6, 14))
>this : Symbol(Derived, Decl(checkInheritedProperty.ts, 3, 56))
>b : Symbol(Derived.b, Decl(checkInheritedProperty.ts, 6, 14))

b = "abc"
>b : Symbol(Derived.b, Decl(checkInheritedProperty.ts, 6, 14))
}

26 changes: 26 additions & 0 deletions testdata/baselines/reference/compiler/checkInheritedProperty.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//// [tests/cases/compiler/checkInheritedProperty.ts] ////

=== checkInheritedProperty.ts ===
class Base {
>Base : Base
}

declare const BaseFactory: new() => Base & { c: string }
>BaseFactory : new () => Base & { c: string; }
>c : string

class Derived extends BaseFactory {
>Derived : Derived
>BaseFactory : Base & { c: string; }

a = this.b
>a : string
>this.b : string
>this : this
>b : string

b = "abc"
>b : string
>"abc" : "abc"
}

12 changes: 12 additions & 0 deletions testdata/tests/cases/compiler/checkInheritedProperty.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// @strict: true
// @noEmit: true

class Base {
}

declare const BaseFactory: new() => Base & { c: string }

class Derived extends BaseFactory {
a = this.b
b = "abc"
}