Skip to content

Commit 638b546

Browse files
authored
fix(jsii-pacmak): underscore arguments conflict in Java (#5011)
The `_` identifier is treated as a keyword since Java 9, and we used to escape it to `__`. The problem is that but it is somewhat likely that people will use `_, __, ___` as multiple indifferent arguments, and now the identifiers will conflict. We have to pick something else. Ideally we would look at the surrounding argument names and pick something guaranteed to be unique, but unfortunately the code isn't quite structured that way so we'll pick something unlikely to collide instead. We escape it to `_under_` instead. Changing from `__` -> `_under_` would be a breaking change if applied to public property names, but most likely this will be used for function parameters (unfortunately the code has been structured in such a way that property and parameter names are strongly tied together, in a way that would take more time to unwind than I care to invest right now), where it doesn't matter. --- 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
1 parent 2970397 commit 638b546

File tree

10 files changed

+193
-21
lines changed

10 files changed

+193
-21
lines changed

packages/jsii-calc/lib/module2530/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,8 @@ export class MyClass {
1414
public foo(_: string) {
1515
return;
1616
}
17+
18+
public multipleUnderscores(_: string, __: string, ___: string) {
19+
return;
20+
}
1721
}

packages/jsii-calc/test/assembly.jsii

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17960,6 +17960,36 @@
1796017960
}
1796117961
}
1796217962
]
17963+
},
17964+
{
17965+
"docs": {
17966+
"stability": "stable"
17967+
},
17968+
"locationInModule": {
17969+
"filename": "lib/module2530/index.ts",
17970+
"line": 18
17971+
},
17972+
"name": "multipleUnderscores",
17973+
"parameters": [
17974+
{
17975+
"name": "_",
17976+
"type": {
17977+
"primitive": "string"
17978+
}
17979+
},
17980+
{
17981+
"name": "__",
17982+
"type": {
17983+
"primitive": "string"
17984+
}
17985+
},
17986+
{
17987+
"name": "___",
17988+
"type": {
17989+
"primitive": "string"
17990+
}
17991+
}
17992+
]
1796317993
}
1796417994
],
1796517995
"name": "MyClass",
@@ -19960,5 +19990,5 @@
1996019990
"intersection-types"
1996119991
],
1996219992
"version": "3.20.120",
19963-
"fingerprint": "vOdExLwYXggdf/TTFym/I+wkv5Ih6GlEJke+UiAjaXo="
19993+
"fingerprint": "lIHvj1O/NYsjHa9x/fPvrEdD8ChVE2BSlsfyzXUQaP4="
1996419994
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,16 @@ export class DotNetNameUtils {
8585
throw new Error(`Invalid parameter name: ${original}`);
8686
}
8787
const name = toCamelCase(original);
88+
if (!name) {
89+
// toCamelCase will return an empty string from a string like `_(_+)`. Confirm that
90+
// that is what is happening, then return the original string.
91+
if (original.match(/^__+$/)) {
92+
return original;
93+
}
94+
throw new Error(
95+
`toCamelCase returns an empty string from: ${JSON.stringify(original)}`,
96+
);
97+
}
8898
return this.escapeParameterName(name);
8999
}
90100

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -608,8 +608,22 @@ class JavaGenerator extends Generator {
608608
}
609609

610610
if (propertyName === '_') {
611-
// Slightly different pattern for this one
612-
return '__';
611+
// Slightly different pattern for this one. We used to generate `__` here
612+
// but it's somewhat likely that people will use `_, __, ___` as multiple
613+
// indifferent arguments, so we pick a different name.
614+
//
615+
// Ideally we would look at the alternative argument names and pick
616+
// something guaranteed to be unique, but unfortunately the code isn't
617+
// quite structured that way so we'll pick something unlikely to collide
618+
// instead.
619+
//
620+
// Changing from `__` -> `_under` would be a breaking change if applied to
621+
// public property names, but most likely this will be used for function
622+
// parameters (unfortunately the code has been structured in such a way
623+
// that property and parameter names are strongly tied together, in a way
624+
// that would take more time to unwind than I care to invest right now),
625+
// where it doesn't matter.
626+
return '_under_';
613627
}
614628

615629
if (JavaGenerator.RESERVED_KEYWORDS.includes(propertyName)) {

packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.js.snap

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/jsii-pacmak/test/generated-code/__snapshots__/target-go.test.js.snap

Lines changed: 45 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/jsii-pacmak/test/generated-code/__snapshots__/target-java.test.js.snap

Lines changed: 18 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.js.snap

Lines changed: 39 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/jsii-reflect/test/__snapshots__/jsii-tree.test.js.snap

Lines changed: 13 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/jsii-reflect/test/__snapshots__/tree.test.js.snap

Lines changed: 11 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)