Skip to content

Commit

Permalink
fix(go,python/java): bad code for members with same name with differe…
Browse files Browse the repository at this point in the history
…nt casing (#2699)

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.

In Java, this issue existed only for property names.

Fixes #2508

BREAKING CHANGE: if multiple members have the same name with different capitalization, only one is allowed to be non-deprecated. This will currently only manifest when producing python bindings, but will be added as a jsii compiler error in the future.
  • Loading branch information
Elad Ben-Israel authored Mar 16, 2021
1 parent 6b2bbe8 commit 25528fb
Show file tree
Hide file tree
Showing 19 changed files with 2,156 additions and 182 deletions.
6 changes: 3 additions & 3 deletions packages/@jsii/go-runtime-test/project/compliance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ func (suite *ComplianceSuite) TestStructs_nonOptionalequals() {
func (suite *ComplianceSuite) TestTestInterfaceParameter() {
assert := suite.Assert()

obj := calc.NewJsObjectLiteralForInterface()
obj := calc.NewJSObjectLiteralForInterface()
friendly := obj.GiveMeFriendly()
assert.Equal("I am literally friendly!", friendly.Hello())

Expand Down Expand Up @@ -1022,7 +1022,7 @@ func (suite *ComplianceSuite) TestConsts() {
obj := calc.Statics_ConstObj();
assert.Equal("world", obj.Hello());

assert.Equal(float64(1234), calc.Statics_Bar());
assert.Equal(float64(1234), calc.Statics_BAR());
assert.Equal("world", calc.Statics_ZooBar()["hello"]);

}
Expand Down Expand Up @@ -1084,7 +1084,7 @@ func (suite *ComplianceSuite) TestStructs_NonOptionalhashCode() {
func (suite *ComplianceSuite) TestTestLiteralInterface() {

assert := suite.Assert()
obj := calc.NewJsObjectLiteralForInterface();
obj := calc.NewJSObjectLiteralForInterface();
friendly := obj.GiveMeFriendly();
assert.Equal("I am literally friendly!", friendly.Hello());

Expand Down
33 changes: 33 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
1 change: 1 addition & 0 deletions packages/jsii-calc/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ export * as module2617 from './module2617';
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';
}
}
254 changes: 253 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,12 @@
"line": 2
}
},
"jsii-calc.module2700": {
"locationInModule": {
"filename": "lib/index.ts",
"line": 19
}
},
"jsii-calc.nodirect": {
"locationInModule": {
"filename": "lib/index.ts",
Expand Down Expand Up @@ -13234,6 +13240,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": {
Expand Down Expand Up @@ -14710,6 +14816,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 @@ -15466,5 +15718,5 @@
}
},
"version": "3.20.120",
"fingerprint": "NN/7JA2G5+NzNqrZC0MowhGB3DS5OlSmsNIF8KMR7Kg="
"fingerprint": "0UwgWtQxMTELv9VFLdPFtg1Zv/tSVnARqKIjCAhYAgg="
}
13 changes: 13 additions & 0 deletions packages/jsii-pacmak/lib/naming-util.ts
Original file line number Diff line number Diff line change
@@ -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);
}
4 changes: 3 additions & 1 deletion packages/jsii-pacmak/lib/targets/dotnet/nameutils.ts
Original file line number Diff line number Diff line change
@@ -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)) {
Expand Down Expand Up @@ -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 */
Expand Down
Loading

0 comments on commit 25528fb

Please sign in to comment.