Skip to content

Commit

Permalink
feat(spec): model submodules in the Assembly schema
Browse files Browse the repository at this point in the history
Adds a `submodules` property on the `Assembly` structure, and carry it
forward in the `dependencyClosure`, so the information can later be used
to improve code generation for languages such as Python where the
submodule structure is modeled as first-class entity that needs to be
propertly dealt with. It can also help with properly adjusting the
submodule names so they look more "native" in target languages, without
facing problems when trying to generate type names in dependent packages.

The forwarding of dependent submodules is not exercized in the current
regression test suite because of a pair of other bugs (#1528, #1557)
that need to be addressed before the generated Python code can
successfully load. The last of those PRs to be merged will incldude the
necessary test coverage.
  • Loading branch information
RomainMuller committed Apr 17, 2020
1 parent e259b95 commit 9c37f08
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 19 deletions.
18 changes: 18 additions & 0 deletions packages/@jsii/spec/lib/assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,23 @@ export interface Assembly extends AssemblyConfiguration, Documentable {
* Shareable configuration of a jsii Assembly.
*/
export interface AssemblyConfiguration {
/**
* Submodules declared in this assembly.
*
* @default none
*/
submodules?: {
[fqn: string]: {
/**
* The specific code generation configuration for this submodule, if it
* differs from it's parent.
*
* @default - the parent submodule or assembly's configuration.
*/
targets?: AssemblyTargets;
};
};

/**
* A map of target name to configuration, which is used when generating
* packages for various languages.
Expand All @@ -158,6 +175,7 @@ export interface AssemblyConfiguration {
targets?: AssemblyTargets;
}


/**
* Versions of the JSII Assembly Specification.
*/
Expand Down
13 changes: 12 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,17 @@
"url": "https://github.com/aws/jsii.git"
},
"schema": "jsii/0.10.0",
"submodules": {
"jsii-calc.DerivedClassHasNoProperties": {},
"jsii-calc.InterfaceInNamespaceIncludesClasses": {},
"jsii-calc.InterfaceInNamespaceOnlyInterface": {},
"jsii-calc.composition": {},
"jsii-calc.submodule": {},
"jsii-calc.submodule.back_references": {},
"jsii-calc.submodule.child": {},
"jsii-calc.submodule.nested_submodule": {},
"jsii-calc.submodule.nested_submodule.deeplyNested": {}
},
"targets": {
"dotnet": {
"iconUrl": "https://sdk-for-net.amazonwebservices.com/images/AWSLogo128x128.png",
Expand Down Expand Up @@ -12442,5 +12453,5 @@
}
},
"version": "0.0.0",
"fingerprint": "XNjL7+hJ5KwFtBCVEz4TZH4c2JtVW/gTPS7UNrw02Cg="
"fingerprint": "4Q+a0fGKWl6wIrpV76N462Ux3HDgzTeZ3NQrjw3x0Mo="
}
99 changes: 81 additions & 18 deletions packages/jsii/lib/assembler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export class Assembler implements Emitter {

/** Map of Symbol to namespace export Symbol */
private readonly _submoduleMap = new Map<ts.Symbol, ts.Symbol>();
private readonly _submodules = new Set<ts.Symbol>();
private readonly _submodules = new Map<ts.Symbol, SubmoduleSpec>();

/**
* @param projectInfo information about the package being assembled
Expand Down Expand Up @@ -153,6 +153,7 @@ export class Assembler implements Emitter {
dependencyClosure: noEmptyDict(toDependencyClosure(this.projectInfo.dependencyClosure)),
bundled: this.projectInfo.bundleDependencies,
types: this._types,
submodules: noEmptyDict(toSubmoduleDeclarations(this._submodules.values())),
targets: this.projectInfo.targets,
metadata: this.projectInfo.metadata,
docs,
Expand Down Expand Up @@ -345,17 +346,13 @@ export class Assembler implements Emitter {
return `unknown.${typeName}`;
}

let submodule = this._submoduleMap.get( type.symbol);
let submoduleNs = submodule?.name;
// Submodules can be in submodules themselves, so we crawl up the tree...
while (submodule != null && this._submoduleMap.has(submodule)) {
submodule = this._submoduleMap.get(submodule)!;
submoduleNs = `${submodule.name}.${submoduleNs}`;
const submodule = this._submoduleMap.get(type.symbol);
if (submodule != null) {
const submoduleNs = this._submodules.get(submodule)!.fqnResolutionPrefix;
return `${submoduleNs}.${typeName}`;
}

const fqn = submoduleNs != null
? `${pkg.name}.${submoduleNs}.${typeName}`
: `${pkg.name}.${typeName}`;
const fqn = `${pkg.name}.${typeName}`;
if (pkg.name !== this.projectInfo.name && !this._dereference({ fqn }, type.symbol.valueDeclaration)) {
this._diagnostic(node,
ts.DiagnosticCategory.Error,
Expand All @@ -376,10 +373,23 @@ export class Assembler implements Emitter {

private _registerNamespaces(symbol: ts.Symbol): void {
const declaration = symbol.valueDeclaration ?? symbol.declarations[0];
if (declaration == null || !ts.isNamespaceExport(declaration)) {
if (declaration == null) {
// Nothing to do here...
return;
}
if (ts.isModuleDeclaration(declaration)) {
const { fqn, fqnResolutionPrefix } = qualifiedNameOf.call(this, symbol, true);
const targets = undefined; // This will be configurable in the future.

this._submodules.set(symbol, { fqn, fqnResolutionPrefix, targets });
this._addToSubmodule(symbol, symbol);
return;
}
if (!ts.isNamespaceExport(declaration)) {
// Nothing to do here...
return;
}

const moduleSpecifier = declaration.parent.moduleSpecifier;
if (moduleSpecifier == null || !ts.isStringLiteral(moduleSpecifier)) {
// There is a grammar error here, so we'll let tsc report this for us.
Expand Down Expand Up @@ -409,9 +419,29 @@ export class Assembler implements Emitter {
this._diagnostic(declaration, ts.DiagnosticCategory.Error,
`Submodule namespaces must be camelCased or snake_cased. Consider renaming to "${Case.camel(symbol.name)}".`);
}
this._submodules.add(symbol);

const { fqn, fqnResolutionPrefix } = qualifiedNameOf.call(this, symbol);
const targets = undefined; // This will be configurable in the future.

this._submodules.set(symbol, { fqn, fqnResolutionPrefix, targets });
this._addToSubmodule(symbol, sourceModule);
}

function qualifiedNameOf(this: Assembler, sym: ts.Symbol, inlineNamespace = false): { fqn: string, fqnResolutionPrefix: string } {
if (this._submoduleMap.has(sym)) {
const parent = this._submodules.get(this._submoduleMap.get(sym)!)!;
const fqn = `${parent.fqn}.${sym.name}`;
return {
fqn,
fqnResolutionPrefix: inlineNamespace ? parent.fqnResolutionPrefix : fqn,
};
}
const fqn = `${this.projectInfo.name}.${sym.name}`;
return {
fqn,
fqnResolutionPrefix: inlineNamespace ? this.projectInfo.name : fqn,
};
}
}

/**
Expand Down Expand Up @@ -479,9 +509,8 @@ export class Assembler implements Emitter {
this._addToSubmodule(ns, symbol);
}
} else if (ts.isModuleDeclaration(decl)) {
this._addToSubmodule(ns, symbol);
this._registerNamespaces(symbol);
} else if (ts.isNamespaceExport(decl)) {
this._submoduleMap.set(symbol, ns);
this._registerNamespaces(symbol);
}
}
Expand Down Expand Up @@ -548,7 +577,7 @@ export class Assembler implements Emitter {

// Let's quickly verify the declaration does not collide with a submodule. Submodules get case-adjusted for each
// target language separately, so names cannot collide with case-variations.
for (const submodule of this._submodules) {
for (const submodule of this._submodules.keys()) {
const candidates = Array.from(new Set([
submodule.name,
Case.camel(submodule.name),
Expand Down Expand Up @@ -1566,6 +1595,25 @@ export class Assembler implements Emitter {
}
}

interface SubmoduleSpec {
/**
* The submodule's fully qualified name.
*/
readonly fqn: string;

/**
* The submodule's fully qualified name prefix to use when resolving type FQNs. This does not
* include "inline namespace" names as those are already represented in the TypeCheckers' view of
* the type names.
*/
readonly fqnResolutionPrefix: string;

/**
* Any customized configuration for the currentl submodule.
*/
readonly targets?: spec.AssemblyTargets;
}

function _fingerprint(assembly: spec.Assembly): spec.Assembly {
delete assembly.fingerprint;
assembly = sortJson(assembly);
Expand Down Expand Up @@ -1782,17 +1830,32 @@ function* intersect<T>(xs: Set<T>, ys: Set<T>) {
}
}

function noEmptyDict<T>(xs: {[key: string]: T}): {[key: string]: T} | undefined {
if (Object.keys(xs).length === 0) { return undefined; }
function noEmptyDict<T>(xs: Record<string, T> | undefined): Record<string, T> | undefined {
if (xs == null || Object.keys(xs).length === 0) { return undefined; }
return xs;
}

function toDependencyClosure(assemblies: readonly spec.Assembly[]): { [name: string]: spec.AssemblyConfiguration } {
const result: { [name: string]: spec.AssemblyTargets } = {};
for (const assembly of assemblies) {
if (!assembly.targets) { continue; }
result[assembly.name] = { targets: assembly.targets };
result[assembly.name] = {
submodules: assembly.submodules,
targets: assembly.targets,
};
}
return result;
}

function toSubmoduleDeclarations(submodules: IterableIterator<SubmoduleSpec>): spec.Assembly['submodules'] {
const result: spec.Assembly['submodules'] = {};

for (const submodule of submodules) {
result[submodule.fqn] = {
targets: submodule.targets,
};
}

return result;
}

Expand Down

0 comments on commit 9c37f08

Please sign in to comment.