Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(jsii): unable to use nested types from dependencies #1866

Merged
merged 13 commits into from
Aug 12, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 20 additions & 9 deletions packages/@scope/jsii-calc-lib/lib/submodule/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,26 @@ export interface ReflectableEntry {
}

export class Reflector {
public constructor() { }

public asMap(reflectable: IReflectable): Record<string, unknown> {
return reflectable.entries.reduce(
(mapping, entry) => {
mapping[entry.key] = entry.value;
return mapping;
},
{} as Record<string, unknown>,
);
return reflectable.entries.reduce((mapping, entry) => {
mapping[entry.key] = entry.value;
return mapping;
}, {} as Record<string, unknown>);
}
}

/**
* This class is here to show we can use nested classes across module boundaries.
*/
export class NestingClass {
private constructor() {}
}
// eslint-disable-next-line @typescript-eslint/no-namespace
export namespace NestingClass {
/**
* This class is here to show we can use nested classes across module boundaries.
*/
export class NestedClass {
public readonly property: string = 'property';
}
}
59 changes: 53 additions & 6 deletions packages/@scope/jsii-calc-lib/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,57 @@
}
]
},
"@scope/jsii-calc-lib.submodule.NestingClass": {
"assembly": "@scope/jsii-calc-lib",
"docs": {
"stability": "deprecated",
"summary": "This class is here to show we can use nested classes across module boundaries."
},
"fqn": "@scope/jsii-calc-lib.submodule.NestingClass",
"kind": "class",
"locationInModule": {
"filename": "lib/submodule/index.ts",
"line": 22
},
"name": "NestingClass",
"namespace": "submodule"
},
"@scope/jsii-calc-lib.submodule.NestingClass.NestedClass": {
"assembly": "@scope/jsii-calc-lib",
"docs": {
"stability": "deprecated",
"summary": "This class is here to show we can use nested classes across module boundaries."
},
"fqn": "@scope/jsii-calc-lib.submodule.NestingClass.NestedClass",
"initializer": {
"docs": {
"stability": "deprecated"
}
},
"kind": "class",
"locationInModule": {
"filename": "lib/submodule/index.ts",
"line": 30
},
"name": "NestedClass",
"namespace": "submodule.NestingClass",
"properties": [
{
"docs": {
"stability": "deprecated"
},
"immutable": true,
"locationInModule": {
"filename": "lib/submodule/index.ts",
"line": 31
},
"name": "property",
"type": {
"primitive": "string"
}
}
]
},
"@scope/jsii-calc-lib.submodule.ReflectableEntry": {
"assembly": "@scope/jsii-calc-lib",
"datatype": true,
Expand Down Expand Up @@ -630,10 +681,6 @@
"initializer": {
"docs": {
"stability": "deprecated"
},
"locationInModule": {
"filename": "lib/submodule/index.ts",
"line": 11
}
},
"kind": "class",
Expand All @@ -648,7 +695,7 @@
},
"locationInModule": {
"filename": "lib/submodule/index.ts",
"line": 13
"line": 11
},
"name": "asMap",
"parameters": [
Expand Down Expand Up @@ -676,5 +723,5 @@
}
},
"version": "0.0.0",
"fingerprint": "fVfpIK7xUajlT1zkHIJ8uYJPvy0gLgEe5BM8afu1mVg="
"fingerprint": "f/4VuNiOkSgTgLR80loQUAzAuzFi+25rmfLcRWKDCrY="
}
1 change: 1 addition & 0 deletions packages/jsii-calc/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export * from './calculator';
export * from './compliance';
export * from './documented';
export * from './erasures';
export * from './nested-class';
export * from './stability';
export * from './submodules';

Expand Down
9 changes: 9 additions & 0 deletions packages/jsii-calc/lib/nested-class.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { submodule } from '@scope/jsii-calc-lib';

export class NestedClassInstance {
public static makeInstance(): submodule.NestingClass.NestedClass {
return new submodule.NestingClass.NestedClass();
}

private constructor() {}
}
35 changes: 33 additions & 2 deletions packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@
"jsii-calc.submodule": {
"locationInModule": {
"filename": "lib/index.ts",
"line": 8
"line": 9
}
},
"jsii-calc.submodule.back_references": {
Expand Down Expand Up @@ -8373,6 +8373,37 @@
}
]
},
"jsii-calc.NestedClassInstance": {
"assembly": "jsii-calc",
"docs": {
"stability": "experimental"
},
"fqn": "jsii-calc.NestedClassInstance",
"kind": "class",
"locationInModule": {
"filename": "lib/nested-class.ts",
"line": 3
},
"methods": [
{
"docs": {
"stability": "experimental"
},
"locationInModule": {
"filename": "lib/nested-class.ts",
"line": 4
},
"name": "makeInstance",
"returns": {
"type": {
"fqn": "@scope/jsii-calc-lib.submodule.NestingClass.NestedClass"
}
},
"static": true
}
],
"name": "NestedClassInstance"
},
"jsii-calc.NestedStruct": {
"assembly": "jsii-calc",
"datatype": true,
Expand Down Expand Up @@ -13722,5 +13753,5 @@
}
},
"version": "0.0.0",
"fingerprint": "AksOoFurRMyuF7gSjHXHbIRIIjq0e0R0CvpHycvyI3U="
"fingerprint": "mLU0hvK9Teaq2GAeeCGFPwAZab85G7lFIH/VCKgAvp4="
}
3 changes: 1 addition & 2 deletions packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,6 @@ export class DotNetGenerator extends Generator {

// Nested classes will be dealt with during calc code generation
const nested = this.isNested(cls);
const inner = nested ? ' static' : '';
const absPrefix = abstract ? ' abstract' : '';

this.openFileIfNeeded(className, namespace, nested);
Expand All @@ -294,7 +293,7 @@ export class DotNetGenerator extends Generator {
this.dotnetRuntimeGenerator.emitAttributesForClass(cls);

this.code.openBlock(
`public${inner}${absPrefix} class ${className}${implementsExpr}`,
`public${absPrefix} class ${className}${implementsExpr}`,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C# does not allow static member classes to declare protected members (our contract includes some, so that was bad). The presence of the static keyword here was a Java-ism, since nested classes in C# (with or without static) are actually semantically related to Java static nested classes (the syntactic sugar that Java has for non-static nested classes does not exist in C#).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have been a separate PR, don'tcha agree?

Copy link
Contributor Author

@RomainMuller RomainMuller Aug 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except that separate PR's test would not have been able to pass without the other changes in this PR (cannot test I can generate a valid nested class if I can't compile it), and vice-versa (cannot check I generate valid code for a nested class if I generate invalid code for a nested class)...

);

// Compute the class parameters
Expand Down
Loading