fix(go,python): bad code for members with same name with different ca…

…pitalization

In TypeScript, it is possible to give two members (methods/properties) the same name but with different capitalization. This results in duplicate members in Go and Python.

In Go, where the member is expected to be PascalCase, use the same conversion used in .NET, in which we only capitalize the first letter (assuming TypeScript uses camelCase). This offers more tolerance.

In Python, where members use snake_case, we implemented a different heuristic in which we only render the non-deprecated member and fail if there is more than one deprecated member.

Fixes #2508

packages/jsii-calc/lib/compliance.ts

@@ -2955,3 +2955,36 @@ export class Default {
     return;
   }
 }
+
+/**
+ * In TypeScript it is possible to have two methods with the same name but
+ * different capitalization.
+ *
+ * @see https://github.com/aws/jsii/issues/2508
+ */
+export class TwoMethodsWithSimilarCapitalization {
+  public toIsoString() {
+    return 'toIsoString';
+  }
+
+  /**
+   * @deprecated python requires that all alternatives are deprecated
+   */
+  public toISOString() {
+    return 'toISOString';
+  }
+
+  /**
+   * @deprecated python requires that all alternatives are deprecated
+   */
+  public toIsOString() {
+    return 'toIsoString';
+  }
+
+  public readonly fooBar = 123;
+
+  /**
+   * @deprecated YES
+   */
+  public readonly fooBAR = 111;
+}

packages/jsii-calc/test/assembly.jsii

@@ -13228,6 +13228,106 @@
         }
       ]
     },
+    "jsii-calc.TwoMethodsWithSimilarCapitalization": {
+      "assembly": "jsii-calc",
+      "docs": {
+        "see": "https://github.com/aws/jsii/issues/2508",
+        "stability": "stable",
+        "summary": "In TypeScript it is possible to have two methods with the same name but different capitalization."
+      },
+      "fqn": "jsii-calc.TwoMethodsWithSimilarCapitalization",
+      "initializer": {
+        "docs": {
+          "stability": "stable"
+        }
+      },
+      "kind": "class",
+      "locationInModule": {
+        "filename": "lib/compliance.ts",
+        "line": 2965
+      },
+      "methods": [
+        {
+          "docs": {
+            "stability": "stable"
+          },
+          "locationInModule": {
+            "filename": "lib/compliance.ts",
+            "line": 2966
+          },
+          "name": "toIsoString",
+          "returns": {
+            "type": {
+              "primitive": "string"
+            }
+          }
+        },
+        {
+          "docs": {
+            "deprecated": "python requires that all alternatives are deprecated",
+            "stability": "deprecated"
+          },
+          "locationInModule": {
+            "filename": "lib/compliance.ts",
+            "line": 2980
+          },
+          "name": "toIsOString",
+          "returns": {
+            "type": {
+              "primitive": "string"
+            }
+          }
+        },
+        {
+          "docs": {
+            "deprecated": "python requires that all alternatives are deprecated",
+            "stability": "deprecated"
+          },
+          "locationInModule": {
+            "filename": "lib/compliance.ts",
+            "line": 2973
+          },
+          "name": "toISOString",
+          "returns": {
+            "type": {
+              "primitive": "string"
+            }
+          }
+        }
+      ],
+      "name": "TwoMethodsWithSimilarCapitalization",
+      "properties": [
+        {
+          "docs": {
+            "stability": "stable"
+          },
+          "immutable": true,
+          "locationInModule": {
+            "filename": "lib/compliance.ts",
+            "line": 2984
+          },
+          "name": "fooBar",
+          "type": {
+            "primitive": "number"
+          }
+        },
+        {
+          "docs": {
+            "deprecated": "YES",
+            "stability": "deprecated"
+          },
+          "immutable": true,
+          "locationInModule": {
+            "filename": "lib/compliance.ts",
+            "line": 2989
+          },
+          "name": "fooBAR",
+          "type": {
+            "primitive": "number"
+          }
+        }
+      ]
+    },
     "jsii-calc.UmaskCheck": {
       "assembly": "jsii-calc",
       "docs": {
@@ -15422,5 +15522,5 @@
     }
   },
   "version": "3.20.120",
-  "fingerprint": "hBZRskNm0XPeSO9U+PMqKKVZ2faqmKnE9eVN2tY1Wik="
+  "fingerprint": "ELEXfVI6U0WtgZz/uZCYxVmrnbgWKv+bjZ55LY5pjAE="
 }

packages/jsii-pacmak/lib/naming-util.ts

@@ -0,0 +1,13 @@
+/**
+ * Converts a jsii method/property names to pascal-case.
+ *
+ * This is different from `toPascalCase()` since it only capitalizes the first
+ * letter. This handles avoids duplicates of things like `toIsoString()` and `toISOString()`.
+ * The assumption is that the jsii name is camelCased.
+ *
+ * @param camelCase The original jsii method name
+ * @returns A pascal-cased method name.
+ */
+export function jsiiToPascalCase(camelCase: string) {
+  return camelCase.charAt(0).toUpperCase() + camelCase.slice(1);
+}

packages/jsii-pacmak/lib/targets/dotnet/nameutils.ts

@@ -1,6 +1,8 @@
 import * as spec from '@jsii/spec';
 import { toCamelCase } from 'codemaker';
 
+import { jsiiToPascalCase } from '../../naming-util';
+
 export class DotNetNameUtils {
   public convertPropertyName(original: string) {
     if (this.isInvalidName(original)) {
@@ -87,7 +89,7 @@ export class DotNetNameUtils {
   }
 
   public capitalizeWord(original: string) {
-    return original.charAt(0).toUpperCase() + original.slice(1);
+    return jsiiToPascalCase(original);
   }
 
   /* We only want valid names for members */

packages/jsii-pacmak/lib/targets/go/types/class.ts

@@ -1,6 +1,7 @@
-import { CodeMaker, toPascalCase } from 'codemaker';
+import { CodeMaker } from 'codemaker';
 import { Method, ClassType, Initializer } from 'jsii-reflect';
 
+import { jsiiToPascalCase } from '../../../naming-util';
 import * as comparators from '../comparators';
 import { EmitContext } from '../emit-context';
 import { Package } from '../package';
@@ -280,7 +281,7 @@ export class GoClass extends GoType {
   private emitStaticProperty({ code }: EmitContext, prop: GoProperty): void {
     const getCaller = new StaticGetProperty(prop);
 
-    const propertyName = toPascalCase(prop.name);
+    const propertyName = jsiiToPascalCase(prop.name);
     const name = `${this.name}_${propertyName}`;
 
     code.openBlock(`func ${name}() ${prop.returnType}`);

packages/jsii-pacmak/lib/targets/go/types/go-type.ts

@@ -1,4 +1,4 @@
-import { CodeMaker, toPascalCase } from 'codemaker';
+import { CodeMaker } from 'codemaker';
 import { Type } from 'jsii-reflect';
 
 import { EmitContext } from '../emit-context';
@@ -13,15 +13,15 @@ export abstract class GoType {
   public readonly proxyName: string;
 
   public constructor(public pkg: Package, public type: Type) {
-    this.name = toPascalCase(type.name);
+    this.name = type.name;
 
     // Prefix with the nesting parent name(s), using an _ delimiter.
     for (
       let parent = type.nestingParent;
       parent != null;
       parent = parent.nestingParent
     ) {
-      this.name = `${toPascalCase(parent.name)}_${this.name}`;
+      this.name = `${parent.name}_${this.name}`;
     }
 
     // Add "jsiiProxy_" prefix to private struct name to avoid keyword conflicts

packages/jsii-pacmak/lib/targets/go/types/type-member.ts

@@ -1,6 +1,6 @@
-import { toPascalCase } from 'codemaker';
 import { Callable, Method, Parameter, Property } from 'jsii-reflect';
 
+import { jsiiToPascalCase } from '../../../naming-util';
 import { EmitContext } from '../emit-context';
 import { GetProperty, JSII_RT_ALIAS, SetProperty } from '../runtime';
 import { substituteReservedWords } from '../util';
@@ -33,7 +33,7 @@ export class GoProperty implements GoTypeMember {
     public parent: GoType,
     public readonly property: Property,
   ) {
-    this.name = toPascalCase(this.property.name);
+    this.name = jsiiToPascalCase(this.property.name);
     this.immutable = property.immutable;
 
     if (property.type) {
@@ -147,7 +147,7 @@ export abstract class GoMethod implements GoTypeMember {
     public readonly parent: GoClass | GoInterface,
     public readonly method: Callable,
   ) {
-    this.name = toPascalCase(method.name);
+    this.name = jsiiToPascalCase(method.name);
     if (Method.isMethod(method) && method.returns.type) {
       this.reference = new GoTypeRef(parent.pkg.root, method.returns.type);
     }

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

@@ -184,6 +184,61 @@ const setDifference = <T>(setA: Set<T>, setB: Set<T>): Set<T> => {
   return result;
 };
 
+/**
+ * Prepare python members for emission.
+ *
+ * If there are multiple members of the same name, they will all map to the same python
+ * name, so we will filter all deprecated members and expect that there will be only one
+ * left.
+ *
+ * Returns the members in a sorted list.
+ */
+function prepareMembers(members: PythonBase[], resolver: TypeResolver) {
+  // create a map from python name to list of members
+  const map: { [pythonName: string]: PythonBase[] } = {};
+  for (const m of members) {
+    let list = map[m.pythonName];
+    if (!list) {
+      list = map[m.pythonName] = [];
+    }
+
+    list.push(m);
+  }
+
+  for (const [name, list] of Object.entries(map)) {
+    // if the list of members for a name is 1, we are good
+    if (list.length === 1) {
+      continue;
+    }
+
+    // we found more than one member with the same python name, filter all
+    // deprecated versions and check that we are left with exactly one.
+    // otherwise, they will overwrite each other
+    // see https://github.com/aws/jsii/issues/2508
+    const nonDeprecated = list.filter((x) => !isDeprecated(x));
+    if (nonDeprecated.length > 1) {
+      throw new Error(
+        `Multiple non-deprecated members which map to the Python name "${name}"`,
+      );
+    }
+
+    // only retain the 1 non-deprecated member
+    map[name] = nonDeprecated;
+  }
+
+  // now return all the members
+  const ret = new Array<PythonBase>();
+  for (const v of Object.values(map)) {
+    if (v.length !== 1) {
+      throw new Error('assertion failed');
+    }
+
+    ret.push(v[0]);
+  }
+
+  return sortMembers(ret, resolver);
+}
+
 const sortMembers = (
   members: PythonBase[],
   resolver: TypeResolver,
@@ -235,6 +290,7 @@ const sortMembers = (
 
 interface PythonBase {
   readonly pythonName: string;
+  readonly docs?: spec.Docs;
 
   emit(code: CodeMaker, context: EmitContext, opts?: any): void;
 
@@ -271,7 +327,7 @@ abstract class BasePythonClassType implements PythonType, ISortableType {
     public readonly pythonName: string,
     public readonly fqn: string | undefined,
     opts: PythonTypeOpts,
-    protected readonly docs: spec.Docs | undefined,
+    public readonly docs: spec.Docs | undefined,
   ) {
     const { bases = [] } = opts;
 
@@ -342,7 +398,7 @@ abstract class BasePythonClassType implements PythonType, ISortableType {
     if (this.members.length > 0) {
       const resolver = this.boundResolver(context.resolver);
       let shouldSeparate = preamble != null;
-      for (const member of sortMembers(this.members, resolver)) {
+      for (const member of prepareMembers(this.members, resolver)) {
         if (shouldSeparate) {
           code.line();
         }
@@ -405,7 +461,7 @@ abstract class BaseMethod implements PythonBase {
     private readonly jsName: string | undefined,
     private readonly parameters: spec.Parameter[],
     private readonly returns: spec.OptionalValue | undefined,
-    private readonly docs: spec.Docs | undefined,
+    public readonly docs: spec.Docs | undefined,
     opts: BaseMethodOpts,
   ) {
     this.abstract = !!opts.abstract;
@@ -746,7 +802,7 @@ abstract class BaseProperty implements PythonBase {
     public readonly pythonName: string,
     private readonly jsName: string,
     private readonly type: spec.OptionalValue,
-    private readonly docs: spec.Docs | undefined,
+    public readonly docs: spec.Docs | undefined,
     opts: BasePropertyOpts,
   ) {
     const { abstract = false, immutable = false, isStatic = false } = opts;
@@ -1372,7 +1428,7 @@ class EnumMember implements PythonBase {
     private readonly generator: PythonGenerator,
     public readonly pythonName: string,
     private readonly value: string,
-    private readonly docs: spec.Docs | undefined,
+    public readonly docs: spec.Docs | undefined,
   ) {
     this.pythonName = pythonName;
     this.value = value;
@@ -1486,7 +1542,7 @@ class PythonModule implements PythonType {
     }
 
     // Emit all of our members.
-    for (const member of sortMembers(this.members, resolver)) {
+    for (const member of prepareMembers(this.members, resolver)) {
       code.line();
       code.line();
       member.emit(code, context);
@@ -2911,3 +2967,5 @@ function nestedContext(
         : context.surroundingTypeFqns,
   };
 }
+
+const isDeprecated = (x: PythonBase) => x.docs?.deprecated !== undefined;