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

feat: allow per-submodule naming configuration #1690

Merged
merged 5 commits into from
May 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
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
24 changes: 19 additions & 5 deletions docs/specifications/2-type-system.md
Original file line number Diff line number Diff line change
Expand Up @@ -325,11 +325,13 @@ qualified name of the namespace from the package's root (if a package `foo`
defines a namespace `ns1`, which itself contains `ns2`, the submodule for `ns2`
will be named `foo.ns1.ns2`).

*Submodules* may use different [code-generation configuration](#code-generation)
than their parent submodule or package.
*Submodules* are delcared in the *jsii* assembly under the `submodules` key.
This is also where specific [configuration](#submodule-configuration)
is registered, if different from the parent submodule or package.

> :construction: *Submodule*-level code-generation configuration is not yet
> implemented.
*Submodules* are hierarchical, and their fully qualified name is representative
of the relationship. For example the `assm.foo.bar` submodule is considered to
be nested under the `assm.foo` submodule.

### Restrictions

Expand Down Expand Up @@ -364,13 +366,25 @@ documented using a markdown document located at `./module/README.md`.

> :construction: The `./module/README.md` file support is not yet implemented.
### Code Generation
### Submodule Configuration

In languages where this is relevant (e.g: **Python**), *submodules* are rendered
as native *submodules*. In languages where a namespace system exists (**Java**
uses *packages*, **C#** uses *namespaces*, ...), *submodules* are rendered using
that.

By default, *submodule* names are rendered appropriately in the target language
(this typically involves adjusting the case of *submodule* name fragments to the
idiomatic form in the language). In certain cases however, a developer can
choose to use a different configuration by defining the *submodule* using the
namespaced-export syntax (`export * as namespace from './module-name';`) ny
placing a `.jsiirc.json` file next to the entry point of the namespaced module.
For example, if `./module-name`'s entry point is `foo/bar/module-name/index.ts`,
the *submodule* configuration resides in `foo/bar/module-name/.jsiirc.json`.

Since *submodules* are hierarchical, the configuration of a given *submodule*
defines the default configuration of *submodules* nested under it.

## Code Generation

In order to generate code in various programming languages, [`jsii-pacmak`]
Expand Down
2 changes: 1 addition & 1 deletion packages/@jsii/python-runtime/tests/test_compliance.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
ClassWithSelfKwarg,
)
from scope.jsii_calc_lib import IFriendly, EnumFromScopedModule, Number
from scope.jsii_calc_lib.submodule import IReflectable, ReflectableEntry
from scope.jsii_calc_lib.custom_submodule_name import IReflectable, ReflectableEntry


# Note: The names of these test functions have been chosen to map as closely to the
Expand Down
13 changes: 13 additions & 0 deletions packages/@scope/jsii-calc-lib/lib/submodule/.jsiirc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"targets": {
"dotnet": {
"namespace": "Amazon.JSII.Tests.CustomSubmoduleName"
},
"java": {
"package": "software.amazon.jsii.tests.calculator.custom_submodule_name"
},
"python": {
"module": "scope.jsii_calc_lib.custom_submodule_name"
}
}
}
13 changes: 12 additions & 1 deletion packages/@scope/jsii-calc-lib/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,17 @@
"locationInModule": {
"filename": "lib/index.ts",
"line": 112
},
"targets": {
"dotnet": {
"namespace": "Amazon.JSII.Tests.CustomSubmoduleName"
},
"java": {
"package": "software.amazon.jsii.tests.calculator.custom_submodule_name"
},
"python": {
"module": "scope.jsii_calc_lib.custom_submodule_name"
}
}
}
},
Expand Down Expand Up @@ -657,5 +668,5 @@
}
},
"version": "0.0.0",
"fingerprint": "+7cEXNA9wAn/mht3AcwDtIFLvsfZoQhQGBQ4J1pPCO4="
"fingerprint": "w6/8Kda6/JitSbpMK3LGPK5JGRxJ50gflR5S4sUEHKk="
}
13 changes: 12 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,17 @@
"locationInModule": {
"filename": "lib/index.ts",
"line": 112
},
"targets": {
"dotnet": {
"namespace": "Amazon.JSII.Tests.CustomSubmoduleName"
},
"java": {
"package": "software.amazon.jsii.tests.calculator.custom_submodule_name"
},
"python": {
"module": "scope.jsii_calc_lib.custom_submodule_name"
}
}
}
},
Expand Down Expand Up @@ -12989,5 +13000,5 @@
}
},
"version": "0.0.0",
"fingerprint": "h3YiCcvHWOpzGACLQCBkRKp0glz4LOkpVZLrH4mtUvE="
"fingerprint": "/DWCMji07IcZLGXOoefpKRPZyV9IHOIAR19tb1MHHWU="
}
6 changes: 2 additions & 4 deletions packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as clone from 'clone';
import { toPascalCase } from 'codemaker';
import * as fs from 'fs-extra';
import * as reflect from 'jsii-reflect';
import * as spec from '@jsii/spec';
Expand Down Expand Up @@ -463,16 +462,15 @@ export class DotNetGenerator extends Generator {
}

private namespaceFor(assm: spec.Assembly, type: spec.Type): string {
const parts = [assm.targets!.dotnet!.namespace];
let ns = type.namespace;
while (ns != null && assm.types?.[`${assm.name}.${ns}`] != null) {
const nesting = assm.types[`${assm.name}.${ns}`];
ns = nesting.namespace;
}
if (ns != null) {
parts.push(...ns.split('.').map((n) => toPascalCase(n)));
return this.typeresolver.resolveNamespace(assm, assm.name, ns);
}
return parts.join('.');
return assm.targets!.dotnet!.namespace;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the 'non-nullability' of these fields not be represented in the assembly spec because of the presence of the dotnet target isn't guaranteed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Problem is the spec doesn't actually represent per-language configurations... so they're opaque stuff entirely.

}

private emitMethod(
Expand Down
34 changes: 29 additions & 5 deletions packages/jsii-pacmak/lib/targets/dotnet/dotnettyperesolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ export class DotNetTypeResolver {

const dotnetNamespace = depMod.targets?.dotnet?.namespace;
if (!dotnetNamespace) {
throw new Error('The module does not have a dotnet.namespace setting');
throw new Error(
`The assembly ${mod} does not have a dotnet.namespace setting`,
);
}
if (type.namespace) {
// If the type is declared in an additional namespace.
Expand All @@ -65,10 +67,8 @@ export class DotNetTypeResolver {
const actualNamespace = this.toDotNetType(this.findType(namespaceFqn));
return `${actualNamespace}.${typeName}`;
}
return `${dotnetNamespace}.${type.namespace
.split('.')
.map((s) => toPascalCase(s))
.join('.')}.${typeName}`;
const ns = this.resolveNamespace(depMod, mod, type.namespace);
return `${ns}.${typeName}`;
}
// When undefined, the type is located at the root of the assembly
return `${dotnetNamespace}.${typeName}`;
Expand Down Expand Up @@ -137,6 +137,30 @@ export class DotNetTypeResolver {
throw new Error(`Invalid type reference: ${JSON.stringify(typeref)}`);
}

public resolveNamespace(
assm: spec.AssemblyConfiguration,
assmName: string,
ns: string,
): string {
let resolved = assm.targets?.dotnet?.namespace;
if (!resolved) {
throw new Error(
`Assembly ${assmName} does not have targets.dotnet.namespace configured!`,
);
}
const segments = ns.split('.');
for (let i = 0; i < segments.length; i++) {
const submoduleName = `${assmName}.${segments.slice(0, i + 1).join('.')}`;
const submodule = assm.submodules?.[submoduleName];
if (submodule && submodule.targets?.dotnet?.namespace) {
resolved = submodule.targets.dotnet.namespace;
} else {
resolved = `${resolved}.${toPascalCase(segments[i])}`;
}
}
return resolved;
}

/**
* Translates a primitive in jsii to a native .NET primitive
*/
Expand Down
52 changes: 38 additions & 14 deletions packages/jsii-pacmak/lib/targets/java.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2613,13 +2613,8 @@ class JavaGenerator extends Generator {
name: string | undefined,
assmName: string = (assm as spec.Assembly).name,
): string {
const javaPackage = assm.targets?.java?.package;
if (!javaPackage) {
throw new Error(
`The module ${assmName} does not have a java.package setting`,
);
}
return `${javaPackage}${name ? `.${name}` : ''}`;
const { javaPackage, tail } = resolvePackageName(assmName, assm, name);
return `${javaPackage}${tail ? `.${tail}` : ''}`;
}

private toNativeName(assm: spec.Assembly): { packageName: string };
Expand Down Expand Up @@ -2650,13 +2645,8 @@ class JavaGenerator extends Generator {
typeName = `${nestingType.name}.${typeName}`;
}

const packageName =
ns != null
? `${javaPackage}.${ns
.split('.')
.map((s) => toSnakeCase(s))
.join('.')}`
: javaPackage;
const packageName = resolvePackageName(assm.name, assm, ns).javaPackage;

return { packageName, typeName };
}

Expand Down Expand Up @@ -2798,3 +2788,37 @@ function computeOverrides<T extends { param: spec.Parameter }>(
},
};
}

function resolvePackageName(
assmName: string,
config: spec.AssemblyConfiguration,
ns: string | undefined,
): { javaPackage: string; tail?: string } {
const rootPackage: string = config.targets?.java?.package;
if (!rootPackage) {
throw new Error(
`Assembly ${assmName} does not have a targets.java.package configured!`,
);
}
if (!ns) {
return { javaPackage: rootPackage };
}

const segments = ns.split('.');
let javaPackage = rootPackage;
for (let i = 0; i < segments.length; i++) {
const submoduleName = `${assmName}.${segments.slice(0, i + 1).join('.')}`;
const submodule = config.submodules?.[submoduleName];
if (submodule == null) {
// We have reached a type name at this stage, so the rest is "tail".
return { javaPackage, tail: segments.slice(i).join('.') };
}
const override = submodule.targets?.java?.package;
if (override) {
javaPackage = override;
} else {
javaPackage = `${javaPackage}.${toSnakeCase(segments[i])}`;
}
}
return { javaPackage };
}
22 changes: 6 additions & 16 deletions packages/jsii-pacmak/lib/targets/python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
toTypeName,
PythonImports,
mergePythonImports,
toPackageName,
} from './python/type-name';
import { die, toPythonIdentifier } from './python/util';

Expand Down Expand Up @@ -1984,22 +1985,11 @@ class PythonGenerator extends Generator {
}

protected onBeginNamespace(ns: string) {
const module = new PythonModule(
[
this.assembly.targets!.python!.module,
...ns
.split('.')
.slice(1)
.map(toPythonIdentifier)
.map((s) => toSnakeCase(s)),
].join('.'),
ns,
{
assembly: this.assembly,
assemblyFilename: this.getAssemblyFileName(),
package: this.package,
},
);
const module = new PythonModule(toPackageName(ns, this.assembly), ns, {
assembly: this.assembly,
assemblyFilename: this.getAssemblyFileName(),
package: this.package,
});

this.package.addModule(module);
this.types.set(ns, module);
Expand Down
10 changes: 10 additions & 0 deletions packages/jsii-pacmak/lib/targets/python/type-name.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,16 @@ export function toTypeName(ref?: OptionalValue | TypeReference): TypeName {
return optional ? new Optional(result) : result;
}

/**
* Obtains the Python package name for a given submodule FQN.
*
* @param fqn the submodule FQN for which a package name is needed.
* @param rootAssm the assembly this FQN belongs to.
*/
export function toPackageName(fqn: string, rootAssm: Assembly): string {
return getPackageName(fqn, rootAssm).packageName;
}

export function mergePythonImports(
...pythonImports: readonly PythonImports[]
): PythonImports {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,17 @@
"locationInModule": {
"filename": "lib/index.ts",
"line": 112
},
"targets": {
"dotnet": {
"namespace": "Amazon.JSII.Tests.CustomSubmoduleName"
},
"java": {
"package": "software.amazon.jsii.tests.calculator.custom_submodule_name"
},
"python": {
"module": "scope.jsii_calc_lib.custom_submodule_name"
}
}
}
},
Expand Down Expand Up @@ -657,5 +668,5 @@
}
},
"version": "0.0.0",
"fingerprint": "+7cEXNA9wAn/mht3AcwDtIFLvsfZoQhQGBQ4J1pPCO4="
"fingerprint": "w6/8Kda6/JitSbpMK3LGPK5JGRxJ50gflR5S4sUEHKk="
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

#pragma warning disable CS0672,CS0809,CS1591

namespace Amazon.JSII.Tests.CalculatorNamespace.LibNamespace.Submodule
namespace Amazon.JSII.Tests.CustomSubmoduleName
{
/// <remarks>
/// <strong>Stability</strong>: Deprecated
Expand All @@ -16,7 +16,7 @@ public interface IReflectable
/// </remarks>
[JsiiProperty(name: "entries", typeJson: "{\"collection\":{\"elementtype\":{\"fqn\":\"@scope/jsii-calc-lib.submodule.ReflectableEntry\"},\"kind\":\"array\"}}")]
[System.Obsolete()]
Amazon.JSII.Tests.CalculatorNamespace.LibNamespace.Submodule.IReflectableEntry[] Entries
Amazon.JSII.Tests.CustomSubmoduleName.IReflectableEntry[] Entries
{
get;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

#pragma warning disable CS0672,CS0809,CS1591

namespace Amazon.JSII.Tests.CalculatorNamespace.LibNamespace.Submodule
namespace Amazon.JSII.Tests.CustomSubmoduleName
{
/// <remarks>
/// <strong>Stability</strong>: Deprecated
Expand Down
Loading