fix(java): invalid code when multi-inheriting optional properties (#2591)

The generated code for interfaces includes default implementations for
optional properties (since adding a new optional property is a non
breaking change in TypeScript, this allows us to preserve this property
in Java, too).

However, if two distinct super-interfaces declare the same property, the
child inherits two unrelated default implementations for the same
method, which results in dispatch ambiguity that is illegal.

In order to remove the ambiguity, a new default implementation must be
provided on the child interface, so that the dispatch is unambiguous
again.

Fixes #22556



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0

Author: RomainMuller

packages/@scope/jsii-calc-lib/lib/duplicate-inherited-prop.ts

@@ -0,0 +1,19 @@
+// This is half of the contraption, the rest is in jsii-calc
+//
+// @see https://github.com/aws/jsii/issues/2256
+
+/**
+ * This struct is intentionally private. Any type that implements it will get
+ * a copy of it's properties hoisted in by jsii.
+ */
+interface InternalDiamondTip {
+  readonly hoistedTop?: string;
+}
+
+export interface DiamondLeft extends InternalDiamondTip {
+  readonly left?: number;
+}
+
+export interface DiamondRight extends InternalDiamondTip {
+  readonly right?: boolean;
+}

packages/@scope/jsii-calc-lib/lib/index.ts

@@ -110,3 +110,4 @@ export interface IThreeLevelsInterface extends base.IBaseInterface {
 }
 
 export * as submodule from './submodule';
+export * from './duplicate-inherited-prop';

packages/@scope/jsii-calc-lib/test/assembly.jsii

@@ -133,6 +133,102 @@
     }
   },
   "types": {
+    "@scope/jsii-calc-lib.DiamondLeft": {
+      "assembly": "@scope/jsii-calc-lib",
+      "datatype": true,
+      "docs": {
+        "stability": "deprecated"
+      },
+      "fqn": "@scope/jsii-calc-lib.DiamondLeft",
+      "kind": "interface",
+      "locationInModule": {
+        "filename": "lib/duplicate-inherited-prop.ts",
+        "line": 13
+      },
+      "name": "DiamondLeft",
+      "properties": [
+        {
+          "abstract": true,
+          "docs": {
+            "stability": "deprecated"
+          },
+          "immutable": true,
+          "locationInModule": {
+            "filename": "lib/duplicate-inherited-prop.ts",
+            "line": 10
+          },
+          "name": "hoistedTop",
+          "optional": true,
+          "type": {
+            "primitive": "string"
+          }
+        },
+        {
+          "abstract": true,
+          "docs": {
+            "stability": "deprecated"
+          },
+          "immutable": true,
+          "locationInModule": {
+            "filename": "lib/duplicate-inherited-prop.ts",
+            "line": 14
+          },
+          "name": "left",
+          "optional": true,
+          "type": {
+            "primitive": "number"
+          }
+        }
+      ]
+    },
+    "@scope/jsii-calc-lib.DiamondRight": {
+      "assembly": "@scope/jsii-calc-lib",
+      "datatype": true,
+      "docs": {
+        "stability": "deprecated"
+      },
+      "fqn": "@scope/jsii-calc-lib.DiamondRight",
+      "kind": "interface",
+      "locationInModule": {
+        "filename": "lib/duplicate-inherited-prop.ts",
+        "line": 17
+      },
+      "name": "DiamondRight",
+      "properties": [
+        {
+          "abstract": true,
+          "docs": {
+            "stability": "deprecated"
+          },
+          "immutable": true,
+          "locationInModule": {
+            "filename": "lib/duplicate-inherited-prop.ts",
+            "line": 10
+          },
+          "name": "hoistedTop",
+          "optional": true,
+          "type": {
+            "primitive": "string"
+          }
+        },
+        {
+          "abstract": true,
+          "docs": {
+            "stability": "deprecated"
+          },
+          "immutable": true,
+          "locationInModule": {
+            "filename": "lib/duplicate-inherited-prop.ts",
+            "line": 18
+          },
+          "name": "right",
+          "optional": true,
+          "type": {
+            "primitive": "boolean"
+          }
+        }
+      ]
+    },
     "@scope/jsii-calc-lib.EnumFromScopedModule": {
       "assembly": "@scope/jsii-calc-lib",
       "docs": {
@@ -776,5 +872,5 @@
     }
   },
   "version": "0.0.0",
-  "fingerprint": "7STgxc8/b+pajng4R4fEptbcOF2aQJBiQkO4bMAWpI4="
+  "fingerprint": "YlmxPtOusVTgKe2YUzZDpB4UnmnU3zZ0UUdXwSDcgOo="
 }

packages/jsii-calc/lib/duplicate-inherited-prop.ts

@@ -0,0 +1,9 @@
+import { DiamondLeft, DiamondRight } from '@scope/jsii-calc-lib';
+
+// This is half of the contraption, the rest is in @scope/jsii-calc-lib.
+//
+// @see https://github.com/aws/jsii/issues/2256
+
+export interface DiamondBottom extends DiamondLeft, DiamondRight {
+  readonly bottom?: Date;
+}

packages/jsii-calc/lib/index.ts

@@ -2,6 +2,7 @@ export * from './calculator';
 export * from './compliance';
 export * from './date';
 export * from './documented';
+export * from './duplicate-inherited-prop';
 export * from './erasures';
 export * from './nested-class';
 export * from './stability';

packages/jsii-calc/test/assembly.jsii

@@ -209,7 +209,7 @@
     "jsii-calc.submodule": {
       "locationInModule": {
         "filename": "lib/index.ts",
-        "line": 10
+        "line": 11
       },
       "readme": {
         "markdown": "Read you, read me\n=================\n\nThis is the readme of the `jsii-calc.submodule` module.\n"
@@ -3821,6 +3821,42 @@
         }
       ]
     },
+    "jsii-calc.DiamondBottom": {
+      "assembly": "jsii-calc",
+      "datatype": true,
+      "docs": {
+        "stability": "stable"
+      },
+      "fqn": "jsii-calc.DiamondBottom",
+      "interfaces": [
+        "@scope/jsii-calc-lib.DiamondLeft",
+        "@scope/jsii-calc-lib.DiamondRight"
+      ],
+      "kind": "interface",
+      "locationInModule": {
+        "filename": "lib/duplicate-inherited-prop.ts",
+        "line": 7
+      },
+      "name": "DiamondBottom",
+      "properties": [
+        {
+          "abstract": true,
+          "docs": {
+            "stability": "stable"
+          },
+          "immutable": true,
+          "locationInModule": {
+            "filename": "lib/duplicate-inherited-prop.ts",
+            "line": 8
+          },
+          "name": "bottom",
+          "optional": true,
+          "type": {
+            "primitive": "date"
+          }
+        }
+      ]
+    },
     "jsii-calc.DiamondInheritanceBaseLevelStruct": {
       "assembly": "jsii-calc",
       "datatype": true,
@@ -14441,5 +14477,5 @@
     }
   },
   "version": "3.20.120",
-  "fingerprint": "DELAKIDgUwCEVXhZflQBdllcDRnscledlYW0UXrjzbM="
+  "fingerprint": "DPUvV1cBxUeRNl5GK1DVzpU7/802/Hb8znpxKRttRUk="
 }

packages/jsii-pacmak/lib/targets/java.ts

@@ -833,6 +833,8 @@ class JavaGenerator extends Generator {
   }
 
   protected onEndInterface(ifc: spec.InterfaceType) {
+    this.emitMultiplyInheritedOptionalProperties(ifc);
+
     if (ifc.datatype) {
       this.emitDataType(ifc);
     } else {
@@ -883,6 +885,9 @@ class JavaGenerator extends Generator {
     this.addJavaDocs(prop);
     this.emitStabilityAnnotations(prop);
     if (prop.optional) {
+      if (prop.overrides) {
+        this.code.line('@Override');
+      }
       this.code.openBlock(`default ${getterType} get${propName}()`);
       this.code.line('return null;');
       this.code.closeBlock();
@@ -896,6 +901,9 @@ class JavaGenerator extends Generator {
         this.code.line();
         this.addJavaDocs(prop);
         if (prop.optional) {
+          if (prop.overrides) {
+            this.code.line('@Override');
+          }
           this.code.line('@software.amazon.jsii.Optional');
           this.code.openBlock(
             `default void set${propName}(final ${type} value)`,
@@ -911,6 +919,61 @@ class JavaGenerator extends Generator {
     }
   }
 
+  /**
+   * Emits a local default implementation for optional properties inherited from
+   * multiple distinct parent types. This remvoes the default method dispatch
+   * ambiguity that would otherwise exist.
+   *
+   * @param ifc            the interface to be processed.
+
+   *
+   * @see https://github.com/aws/jsii/issues/2256
+   */
+  private emitMultiplyInheritedOptionalProperties(ifc: spec.InterfaceType) {
+    if (ifc.interfaces == null || ifc.interfaces.length <= 1) {
+      // Nothing to do if we don't have parent interfaces, or if we have exactly one
+      return;
+    }
+    const inheritedOptionalProps = ifc.interfaces
+      .map(allOptionalProps.bind(this))
+      // Calculate how many direct parents brought a given optional property
+      .reduce((histogram, entry) => {
+        for (const [name, spec] of Object.entries(entry)) {
+          histogram[name] = histogram[name] ?? { spec, count: 0 };
+          histogram[name].count += 1;
+        }
+        return histogram;
+      }, {} as Record<string, { readonly spec: spec.Property; count: number }>);
+
+    const localProps = new Set(ifc.properties?.map((prop) => prop.name) ?? []);
+    for (const { spec, count } of Object.values(inheritedOptionalProps)) {
+      if (count < 2 || localProps.has(spec.name)) {
+        continue;
+      }
+      this.onInterfaceProperty(ifc, spec);
+    }
+
+    function allOptionalProps(this: JavaGenerator, fqn: string) {
+      const type = this.findType(fqn) as spec.InterfaceType;
+      const result: Record<string, spec.Property> = {};
+      for (const prop of type.properties ?? []) {
+        // Adding artifical "overrides" here for code-gen quality's sake.
+        result[prop.name] = { ...prop, overrides: type.fqn };
+      }
+      // Include optional properties of all super interfaces in the result
+      for (const base of type.interfaces ?? []) {
+        for (const [name, prop] of Object.entries(
+          allOptionalProps.call(this, base),
+        )) {
+          if (!(name in result)) {
+            result[name] = prop;
+          }
+        }
+      }
+      return result;
+    }
+  }
+
   private emitPackageInfo(mod: spec.Assembly) {
     if (!mod.docs) {
       return;