From d747277edf6a9b0f2639953788f100201fe44bc4 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 7 Mar 2023 15:40:43 -0800 Subject: [PATCH] Skip derived override assignment declarations (#52603) --- src/compiler/checker.ts | 17 +++++-- ...erringClassMembersFromAssignments3.symbols | 2 +- ...erringClassMembersFromAssignments4.symbols | 2 +- .../thisPropertyAssignmentInherited.js | 39 +++++++++++++++ .../thisPropertyAssignmentInherited.symbols | 48 ++++++++++++++++++ .../thisPropertyAssignmentInherited.types | 50 +++++++++++++++++++ .../salsa/thisPropertyAssignmentInherited.ts | 23 +++++++++ 7 files changed, 176 insertions(+), 5 deletions(-) create mode 100644 tests/baselines/reference/thisPropertyAssignmentInherited.js create mode 100644 tests/baselines/reference/thisPropertyAssignmentInherited.symbols create mode 100644 tests/baselines/reference/thisPropertyAssignmentInherited.types create mode 100644 tests/cases/conformance/salsa/thisPropertyAssignmentInherited.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index cb2c8de93fa..661db37fd5f 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -244,6 +244,7 @@ import { getCombinedModifierFlags, getCombinedNodeFlags, getContainingClass, + getContainingClassStaticBlock, getContainingFunction, getContainingFunctionOrClassStaticBlock, getDeclarationModifierFlagsFromSymbol, @@ -12213,9 +12214,19 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { } function addInheritedMembers(symbols: SymbolTable, baseSymbols: Symbol[]) { - for (const s of baseSymbols) { - if (!symbols.has(s.escapedName) && !isStaticPrivateIdentifierProperty(s)) { - symbols.set(s.escapedName, s); + for (const base of baseSymbols) { + if (isStaticPrivateIdentifierProperty(base)) { + continue; + } + const derived = symbols.get(base.escapedName); + if (!derived + // non-constructor/static-block assignment declarations are ignored here; they're not treated as overrides + || derived.valueDeclaration + && isBinaryExpression(derived.valueDeclaration) + && !isConstructorDeclaredProperty(derived) + && !getContainingClassStaticBlock(derived.valueDeclaration)) { + symbols.set(base.escapedName, base); + symbols.set(base.escapedName, base); } } } diff --git a/tests/baselines/reference/inferringClassMembersFromAssignments3.symbols b/tests/baselines/reference/inferringClassMembersFromAssignments3.symbols index b4006600dbe..60a3e1b5b51 100644 --- a/tests/baselines/reference/inferringClassMembersFromAssignments3.symbols +++ b/tests/baselines/reference/inferringClassMembersFromAssignments3.symbols @@ -17,7 +17,7 @@ class Derived extends Base { >m : Symbol(Derived.m, Decl(a.js, 5, 28)) this.p = 1 ->this.p : Symbol(Derived.p, Decl(a.js, 6, 9)) +>this.p : Symbol(Base.p, Decl(a.js, 1, 19)) >this : Symbol(Derived, Decl(a.js, 4, 1)) >p : Symbol(Derived.p, Decl(a.js, 6, 9)) } diff --git a/tests/baselines/reference/inferringClassMembersFromAssignments4.symbols b/tests/baselines/reference/inferringClassMembersFromAssignments4.symbols index d0d438c3ed3..692864201cb 100644 --- a/tests/baselines/reference/inferringClassMembersFromAssignments4.symbols +++ b/tests/baselines/reference/inferringClassMembersFromAssignments4.symbols @@ -20,7 +20,7 @@ class Derived extends Base { // should be OK, and p should have type number | undefined from its base this.p = 1 ->this.p : Symbol(Derived.p, Decl(a.js, 6, 9)) +>this.p : Symbol(Base.p, Decl(a.js, 1, 9)) >this : Symbol(Derived, Decl(a.js, 4, 1)) >p : Symbol(Derived.p, Decl(a.js, 6, 9)) } diff --git a/tests/baselines/reference/thisPropertyAssignmentInherited.js b/tests/baselines/reference/thisPropertyAssignmentInherited.js new file mode 100644 index 00000000000..01cd7fc8b67 --- /dev/null +++ b/tests/baselines/reference/thisPropertyAssignmentInherited.js @@ -0,0 +1,39 @@ +//// [thisPropertyAssignmentInherited.js] +export class Element { + /** + * @returns {String} + */ + get textContent() { + return '' + } + set textContent(x) {} + cloneNode() { return this} +} +export class HTMLElement extends Element {} +export class TextElement extends HTMLElement { + get innerHTML() { return this.textContent; } + set innerHTML(html) { this.textContent = html; } + toString() { + } +} + + + + + +//// [thisPropertyAssignmentInherited.d.ts] +export class Element { + set textContent(arg: string); + /** + * @returns {String} + */ + get textContent(): string; + cloneNode(): Element; +} +export class HTMLElement extends Element { +} +export class TextElement extends HTMLElement { + set innerHTML(arg: string); + get innerHTML(): string; + toString(): void; +} diff --git a/tests/baselines/reference/thisPropertyAssignmentInherited.symbols b/tests/baselines/reference/thisPropertyAssignmentInherited.symbols new file mode 100644 index 00000000000..4874bfd9247 --- /dev/null +++ b/tests/baselines/reference/thisPropertyAssignmentInherited.symbols @@ -0,0 +1,48 @@ +=== tests/cases/conformance/salsa/thisPropertyAssignmentInherited.js === +export class Element { +>Element : Symbol(Element, Decl(thisPropertyAssignmentInherited.js, 0, 0)) + + /** + * @returns {String} + */ + get textContent() { +>textContent : Symbol(Element.textContent, Decl(thisPropertyAssignmentInherited.js, 0, 22), Decl(thisPropertyAssignmentInherited.js, 6, 3)) + + return '' + } + set textContent(x) {} +>textContent : Symbol(Element.textContent, Decl(thisPropertyAssignmentInherited.js, 0, 22), Decl(thisPropertyAssignmentInherited.js, 6, 3)) +>x : Symbol(x, Decl(thisPropertyAssignmentInherited.js, 7, 18)) + + cloneNode() { return this} +>cloneNode : Symbol(Element.cloneNode, Decl(thisPropertyAssignmentInherited.js, 7, 23)) +>this : Symbol(Element, Decl(thisPropertyAssignmentInherited.js, 0, 0)) +} +export class HTMLElement extends Element {} +>HTMLElement : Symbol(HTMLElement, Decl(thisPropertyAssignmentInherited.js, 9, 1)) +>Element : Symbol(Element, Decl(thisPropertyAssignmentInherited.js, 0, 0)) + +export class TextElement extends HTMLElement { +>TextElement : Symbol(TextElement, Decl(thisPropertyAssignmentInherited.js, 10, 43)) +>HTMLElement : Symbol(HTMLElement, Decl(thisPropertyAssignmentInherited.js, 9, 1)) + + get innerHTML() { return this.textContent; } +>innerHTML : Symbol(TextElement.innerHTML, Decl(thisPropertyAssignmentInherited.js, 11, 46), Decl(thisPropertyAssignmentInherited.js, 12, 46)) +>this.textContent : Symbol(Element.textContent, Decl(thisPropertyAssignmentInherited.js, 0, 22), Decl(thisPropertyAssignmentInherited.js, 6, 3)) +>this : Symbol(TextElement, Decl(thisPropertyAssignmentInherited.js, 10, 43)) +>textContent : Symbol(Element.textContent, Decl(thisPropertyAssignmentInherited.js, 0, 22), Decl(thisPropertyAssignmentInherited.js, 6, 3)) + + set innerHTML(html) { this.textContent = html; } +>innerHTML : Symbol(TextElement.innerHTML, Decl(thisPropertyAssignmentInherited.js, 11, 46), Decl(thisPropertyAssignmentInherited.js, 12, 46)) +>html : Symbol(html, Decl(thisPropertyAssignmentInherited.js, 13, 16)) +>this.textContent : Symbol(Element.textContent, Decl(thisPropertyAssignmentInherited.js, 0, 22), Decl(thisPropertyAssignmentInherited.js, 6, 3)) +>this : Symbol(TextElement, Decl(thisPropertyAssignmentInherited.js, 10, 43)) +>textContent : Symbol(TextElement.textContent, Decl(thisPropertyAssignmentInherited.js, 13, 23)) +>html : Symbol(html, Decl(thisPropertyAssignmentInherited.js, 13, 16)) + + toString() { +>toString : Symbol(TextElement.toString, Decl(thisPropertyAssignmentInherited.js, 13, 50)) + } +} + + diff --git a/tests/baselines/reference/thisPropertyAssignmentInherited.types b/tests/baselines/reference/thisPropertyAssignmentInherited.types new file mode 100644 index 00000000000..eefead898a0 --- /dev/null +++ b/tests/baselines/reference/thisPropertyAssignmentInherited.types @@ -0,0 +1,50 @@ +=== tests/cases/conformance/salsa/thisPropertyAssignmentInherited.js === +export class Element { +>Element : Element + + /** + * @returns {String} + */ + get textContent() { +>textContent : string + + return '' +>'' : "" + } + set textContent(x) {} +>textContent : string +>x : string + + cloneNode() { return this} +>cloneNode : () => this +>this : this +} +export class HTMLElement extends Element {} +>HTMLElement : HTMLElement +>Element : Element + +export class TextElement extends HTMLElement { +>TextElement : TextElement +>HTMLElement : HTMLElement + + get innerHTML() { return this.textContent; } +>innerHTML : string +>this.textContent : string +>this : this +>textContent : string + + set innerHTML(html) { this.textContent = html; } +>innerHTML : string +>html : string +>this.textContent = html : string +>this.textContent : string +>this : this +>textContent : string +>html : string + + toString() { +>toString : () => void + } +} + + diff --git a/tests/cases/conformance/salsa/thisPropertyAssignmentInherited.ts b/tests/cases/conformance/salsa/thisPropertyAssignmentInherited.ts new file mode 100644 index 00000000000..bd77274270b --- /dev/null +++ b/tests/cases/conformance/salsa/thisPropertyAssignmentInherited.ts @@ -0,0 +1,23 @@ +// @checkJs: true +// @strict: true +// @emitDeclarationOnly: true +// @declaration: true +// @Filename: thisPropertyAssignmentInherited.js +export class Element { + /** + * @returns {String} + */ + get textContent() { + return '' + } + set textContent(x) {} + cloneNode() { return this} +} +export class HTMLElement extends Element {} +export class TextElement extends HTMLElement { + get innerHTML() { return this.textContent; } + set innerHTML(html) { this.textContent = html; } + toString() { + } +} +