Skip to content

Commit

Permalink
fix(go): missing methods for multiple inheritance of the same interfa…
Browse files Browse the repository at this point in the history
…ce (#2701)

If a class implements an interface from multiple paths (e.g. directly and via a base), then Go compilation fails with an ambiguity issue.

Simplify by always re-implementing all methods and properties in derived classes.

Fixes #2700
  • Loading branch information
Elad Ben-Israel authored Mar 15, 2021
1 parent b7a560d commit e31f0e1
Show file tree
Hide file tree
Showing 11 changed files with 1,525 additions and 97 deletions.
1 change: 1 addition & 0 deletions packages/jsii-calc/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ export * as module2647 from './module2647';
export * as module2689 from './module2689';
export * as module2692 from './module2692';
export * as module2530 from './module2530';
export * as module2700 from './module2700';
17 changes: 17 additions & 0 deletions packages/jsii-calc/lib/module2700/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
export interface IFoo {
bar(): string;
readonly baz: number;
}

export class Base implements IFoo {
public readonly baz = 120;
public bar() {
return 'bar';
}
}

export class Derived extends Base implements IFoo {
public zoo() {
return 'zoo';
}
}
154 changes: 153 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,12 @@
"line": 2
}
},
"jsii-calc.module2700": {
"locationInModule": {
"filename": "lib/index.ts",
"line": 18
}
},
"jsii-calc.nodirect": {
"locationInModule": {
"filename": "lib/index.ts",
Expand Down Expand Up @@ -14766,6 +14772,152 @@
}
]
},
"jsii-calc.module2700.Base": {
"assembly": "jsii-calc",
"docs": {
"stability": "stable"
},
"fqn": "jsii-calc.module2700.Base",
"initializer": {
"docs": {
"stability": "stable"
}
},
"interfaces": [
"jsii-calc.module2700.IFoo"
],
"kind": "class",
"locationInModule": {
"filename": "lib/module2700/index.ts",
"line": 6
},
"methods": [
{
"docs": {
"stability": "stable"
},
"locationInModule": {
"filename": "lib/module2700/index.ts",
"line": 8
},
"name": "bar",
"overrides": "jsii-calc.module2700.IFoo",
"returns": {
"type": {
"primitive": "string"
}
}
}
],
"name": "Base",
"namespace": "module2700",
"properties": [
{
"docs": {
"stability": "stable"
},
"immutable": true,
"locationInModule": {
"filename": "lib/module2700/index.ts",
"line": 7
},
"name": "baz",
"overrides": "jsii-calc.module2700.IFoo",
"type": {
"primitive": "number"
}
}
]
},
"jsii-calc.module2700.Derived": {
"assembly": "jsii-calc",
"base": "jsii-calc.module2700.Base",
"docs": {
"stability": "stable"
},
"fqn": "jsii-calc.module2700.Derived",
"initializer": {
"docs": {
"stability": "stable"
}
},
"interfaces": [
"jsii-calc.module2700.IFoo"
],
"kind": "class",
"locationInModule": {
"filename": "lib/module2700/index.ts",
"line": 13
},
"methods": [
{
"docs": {
"stability": "stable"
},
"locationInModule": {
"filename": "lib/module2700/index.ts",
"line": 14
},
"name": "zoo",
"returns": {
"type": {
"primitive": "string"
}
}
}
],
"name": "Derived",
"namespace": "module2700"
},
"jsii-calc.module2700.IFoo": {
"assembly": "jsii-calc",
"docs": {
"stability": "stable"
},
"fqn": "jsii-calc.module2700.IFoo",
"kind": "interface",
"locationInModule": {
"filename": "lib/module2700/index.ts",
"line": 1
},
"methods": [
{
"abstract": true,
"docs": {
"stability": "stable"
},
"locationInModule": {
"filename": "lib/module2700/index.ts",
"line": 2
},
"name": "bar",
"returns": {
"type": {
"primitive": "string"
}
}
}
],
"name": "IFoo",
"namespace": "module2700",
"properties": [
{
"abstract": true,
"docs": {
"stability": "stable"
},
"immutable": true,
"locationInModule": {
"filename": "lib/module2700/index.ts",
"line": 3
},
"name": "baz",
"type": {
"primitive": "number"
}
}
]
},
"jsii-calc.nodirect.sub1.TypeFromSub1": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -15522,5 +15674,5 @@
}
},
"version": "3.20.120",
"fingerprint": "ELEXfVI6U0WtgZz/uZCYxVmrnbgWKv+bjZ55LY5pjAE="
"fingerprint": "tN40QG7OJMwvZrncnCWdHHQjS9MvubkL128BGefSM1A="
}
44 changes: 2 additions & 42 deletions packages/jsii-pacmak/lib/targets/go/types/class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ export class GoClass extends GoType {
public readonly properties: GoProperty[];
public readonly staticProperties: GoProperty[];

private readonly reimplementedMethods?: readonly ClassMethod[];
private readonly reimplementedProperties?: readonly GoProperty[];

private _extends?: GoClass | null;
private _implements?: readonly GoInterface[];

Expand All @@ -40,7 +37,7 @@ export class GoClass extends GoType {

const methods = new Array<ClassMethod>();
const staticMethods = new Array<StaticMethod>();
for (const method of type.ownMethods) {
for (const method of type.allMethods) {
if (method.static) {
staticMethods.push(new StaticMethod(this, method));
} else {
Expand All @@ -53,7 +50,7 @@ export class GoClass extends GoType {

const properties = new Array<GoProperty>();
const staticProperties = new Array<GoProperty>();
for (const prop of type.ownProperties) {
for (const prop of type.allProperties) {
if (prop.static) {
staticProperties.push(new GoProperty(this, prop));
} else {
Expand All @@ -64,31 +61,6 @@ export class GoClass extends GoType {
this.properties = properties.sort(comparators.byName);
this.staticProperties = staticProperties.sort(comparators.byName);

// If there is more than one base, and any ancestor (including transitive)
// comes from a different assembly, we will re-implement all members on the
// proxy struct, as otherwise we run the risk of un-promotable methods
// caused by inheriting the same interface via multiple paths (since we have
// to represent those as embedded types).
const hasMultipleBases = type.interfaces.length > (type.base ? 0 : 1);
if (
hasMultipleBases &&
type
.getAncestors()
.some((ancestor) => ancestor.assembly.fqn !== type.assembly.fqn)
) {
this.reimplementedMethods = type.allMethods
.filter((method) => !method.static && method.definingType !== type)
.map((method) => new ClassMethod(this, method))
.sort(comparators.byName);

this.reimplementedProperties = type.allProperties
.filter(
(property) => !property.static && property.definingType !== type,
)
.map((property) => new GoProperty(this, property))
.sort(comparators.byName);
}

if (type.initializer) {
this.initializer = new GoClassConstructor(this, type.initializer);
}
Expand Down Expand Up @@ -143,10 +115,6 @@ export class GoClass extends GoType {
for (const method of this.methods) {
method.emit(context);
}

for (const method of this.reimplementedMethods ?? []) {
method.emit(context);
}
}

public emitRegistration(code: CodeMaker): void {
Expand Down Expand Up @@ -183,8 +151,6 @@ export class GoClass extends GoType {
...this.properties,
...this.staticMethods,
...this.staticProperties,
...(this.reimplementedMethods ?? []),
...(this.reimplementedProperties ?? []),
];
}

Expand Down Expand Up @@ -240,9 +206,6 @@ export class GoClass extends GoType {
for (const property of this.properties) {
property.emitGetterProxy(context);
}
for (const property of this.reimplementedProperties ?? []) {
property.emitGetterProxy(context);
}
context.code.line();
}

Expand Down Expand Up @@ -306,9 +269,6 @@ export class GoClass extends GoType {
for (const property of this.properties) {
property.emitSetterProxy(context);
}
for (const property of this.reimplementedProperties ?? []) {
property.emitSetterProxy(context);
}
}

public get dependencies(): Package[] {
Expand Down
Loading

0 comments on commit e31f0e1

Please sign in to comment.