Skip to content

Commit

Permalink
fix(pacmak): illegal static overrides in java & c#
Browse files Browse the repository at this point in the history
Stop emitting "overrides" in the respecting language idiomatic way for
static methods and properties (while ES6 supports those, this is not
true of Java and C#).

In Java, this is mostly getting rid of `@Overrides` on static
declarations. C# *allows* (but does not require) explicitly hiding the
parent declaration using the `new` keyword. This was introduced as it
provides additional safeguards against our generating incorrect code.

Fixes #2358
  • Loading branch information
RomainMuller committed Dec 21, 2020
1 parent e20ca97 commit db6376d
Show file tree
Hide file tree
Showing 11 changed files with 479 additions and 11 deletions.
28 changes: 28 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2840,3 +2840,31 @@ export namespace LevelOne {
readonly value: boolean;
}
}

/**
* Static methods that override parent class are technically overrides (the
* inheritance of statics is part of the ES6 specification), but certain other
* languages such as Java do not share this feature.
*/
export class StaticHelloParent {
public static get property(): number {
return 1337;
}

public static method(): void {
/* A static method */
}
}
export class StaticHelloChild extends StaticHelloParent {
public static get property(): number {
return 42;
}

public static method(): void {
/* An ES6 Override */
}

private constructor() {
super();
}
}
97 changes: 96 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -11422,6 +11422,101 @@
}
]
},
"jsii-calc.StaticHelloChild": {
"assembly": "jsii-calc",
"base": "jsii-calc.StaticHelloParent",
"docs": {
"stability": "stable"
},
"fqn": "jsii-calc.StaticHelloChild",
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2858
},
"methods": [
{
"docs": {
"stability": "stable"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2863
},
"name": "method",
"overrides": "jsii-calc.StaticHelloParent",
"static": true
}
],
"name": "StaticHelloChild",
"properties": [
{
"docs": {
"stability": "stable"
},
"immutable": true,
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2859
},
"name": "property",
"overrides": "jsii-calc.StaticHelloParent",
"static": true,
"type": {
"primitive": "number"
}
}
]
},
"jsii-calc.StaticHelloParent": {
"assembly": "jsii-calc",
"docs": {
"stability": "stable",
"summary": "Static methods that override parent class are technically overrides (the inheritance of statics is part of the ES6 specification), but certain other languages such as Java do not share this feature."
},
"fqn": "jsii-calc.StaticHelloParent",
"initializer": {
"docs": {
"stability": "stable"
}
},
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2849
},
"methods": [
{
"docs": {
"stability": "stable"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2854
},
"name": "method",
"static": true
}
],
"name": "StaticHelloParent",
"properties": [
{
"docs": {
"stability": "stable"
},
"immutable": true,
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2850
},
"name": "property",
"static": true,
"type": {
"primitive": "number"
}
}
]
},
"jsii-calc.Statics": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -14345,5 +14440,5 @@
}
},
"version": "0.0.0",
"fingerprint": "XfCnzPEGbEJR6hhBX8zWyFzWJP2wH9ztW2ZcW6Wdb+4="
"fingerprint": "lMjqpro90RI1jZrcELFmEzcSJQCxkEhm77olVYW0fc8="
}
13 changes: 6 additions & 7 deletions packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ export class DotNetGenerator extends Generator {
const returnType = method.returns
? this.typeresolver.toDotNetType(method.returns.type)
: 'void';
let staticKeyWord = '';
const staticKeyWord = method.static ? 'static ' : '';
let overrideKeyWord = '';
let virtualKeyWord = '';

Expand All @@ -496,12 +496,11 @@ export class DotNetGenerator extends Generator {
overrides = true;
}
}
if (method.static) {
staticKeyWord = 'static ';
} else if (overrides) {
if (overrides) {
// Add the override key word if the method is emitted for a proxy or data type or is defined on an ancestor
overrideKeyWord = 'override ';
overrideKeyWord = method.static ? 'new ' : 'override ';
} else if (
!method.static &&
(method.abstract || !definedOnAncestor) &&
!emitForProxyOrDatatype
) {
Expand Down Expand Up @@ -833,7 +832,7 @@ export class DotNetGenerator extends Generator {
);
if (implementedInBase || datatype || proxy) {
// Override if the property is in a datatype or proxy class or declared in a parent class
isOverrideKeyWord = 'override ';
isOverrideKeyWord = prop.static ? 'new ' : 'override ';
} else if (prop.abstract) {
// Abstract members get decorated as such
isAbstractKeyword = 'abstract ';
Expand All @@ -845,7 +844,7 @@ export class DotNetGenerator extends Generator {

const propTypeFQN = this.typeresolver.toDotNetType(prop.type);
const isOptional = prop.optional ? '?' : '';
const statement = `${access} ${isAbstractKeyword}${isVirtualKeyWord}${isOverrideKeyWord}${staticKeyWord}${propTypeFQN}${isOptional} ${propName}`;
const statement = `${access} ${isAbstractKeyword}${isVirtualKeyWord}${staticKeyWord}${isOverrideKeyWord}${propTypeFQN}${isOptional} ${propName}`;
this.code.openBlock(statement);

// Emit getters
Expand Down
6 changes: 3 additions & 3 deletions packages/jsii-pacmak/lib/targets/java.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1274,7 +1274,7 @@ class JavaGenerator extends Generator {
if (includeGetter) {
this.code.line();
this.addJavaDocs(prop);
if (overrides) {
if (overrides && !prop.static) {
this.code.line('@Override');
}
this.emitStabilityAnnotations(prop);
Expand Down Expand Up @@ -1307,7 +1307,7 @@ class JavaGenerator extends Generator {
for (const type of setterTypes) {
this.code.line();
this.addJavaDocs(prop);
if (overrides) {
if (overrides && !prop.static) {
this.code.line('@Override');
}
this.emitStabilityAnnotations(prop);
Expand Down Expand Up @@ -1364,7 +1364,7 @@ class JavaGenerator extends Generator {
this.code.line();
this.addJavaDocs(method);
this.emitStabilityAnnotations(method);
if (overrides) {
if (overrides && !method.static) {
this.code.line('@Override');
}
if (method.abstract && !defaultImpl) {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit db6376d

Please sign in to comment.