Skip to content

Commit

Permalink
feat(pacmak): allow opt-out of runtime type checking generation (#3710)
Browse files Browse the repository at this point in the history
Adds a new CLI flag `--no-runtime-type-checking` to allow library authors to
opt out of generating runtime type checking code if they are not interested
in those.



---

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
  • Loading branch information
RomainMuller committed Aug 18, 2022
1 parent e7fadc0 commit 39923a5
Show file tree
Hide file tree
Showing 18 changed files with 4,664 additions and 54 deletions.
10 changes: 10 additions & 0 deletions packages/jsii-pacmak/bin/jsii-pacmak.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ import { VERSION_DESC } from '../lib/version';
desc: 'generate code only (instead of building and packaging)',
default: false,
})
.option('runtime-type-checking', {
type: 'boolean',
desc: [
'generate runtime type checking code where compile-time type checking is not possible.',
'Disabling this will generate less code, but will produce less helpful error messages when',
'developers pass invalid values to the generated bindings.',
].join(' '),
default: true,
})
.option('fingerprint', {
type: 'boolean',
desc: 'attach a fingerprint to the generated artifacts, and skip generation if outdir contains artifacts that have a matching fingerprint',
Expand Down Expand Up @@ -171,6 +180,7 @@ import { VERSION_DESC } from '../lib/version';
recurse: argv.recurse,
rosettaUnknownSnippets,
rosettaTablet: argv['rosetta-tablet'],
runtimeTypeChecking: argv['runtime-type-checking'],
targets: argv.targets?.map((target) => target as TargetName),
updateNpmIgnoreFiles: argv.npmignore,
validateAssemblies: argv['validate-assemblies'],
Expand Down
27 changes: 17 additions & 10 deletions packages/jsii-pacmak/lib/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,38 +13,44 @@ export interface BuildOptions {
* Whether to fingerprint the produced artifacts.
* @default true
*/
fingerprint?: boolean;
readonly fingerprint?: boolean;

/**
* Whether artifacts should be re-build even if their fingerprints look up-to-date.
* @default false
*/
force?: boolean;
readonly force?: boolean;

/**
* Arguments provided by the user (how they are used is target-dependent)
*/
arguments: { readonly [name: string]: any };
readonly arguments: { readonly [name: string]: any };

/**
* Only generate code, don't build
*/
codeOnly?: boolean;
readonly codeOnly?: boolean;

/**
* Whether or not to clean
*/
clean?: boolean;
readonly clean?: boolean;

/**
* Whether to add an additional subdirectory for the target language
*/
languageSubdirectory?: boolean;
readonly languageSubdirectory?: boolean;

/**
* The Rosetta instance to load examples from
*/
rosetta: Rosetta;
readonly rosetta: Rosetta;

/**
* Whether to generate runtime type checking code in places where compile-time
* type checking is not possible.
*/
readonly runtimeTypeChecking: boolean;
}

/**
Expand Down Expand Up @@ -130,13 +136,14 @@ export class IndependentPackageBuilder implements TargetBuilder {

private makeTarget(module: JsiiModule, options: BuildOptions): Target {
return new this.targetConstructor({
targetName: this.targetName,
packageDir: module.moduleDirectory,
arguments: options.arguments,
assembly: module.assembly,
fingerprint: options.fingerprint,
force: options.force,
arguments: options.arguments,
packageDir: module.moduleDirectory,
rosetta: options.rosetta,
runtimeTypeChecking: options.runtimeTypeChecking,
targetName: this.targetName,
});
}

Expand Down
12 changes: 11 additions & 1 deletion packages/jsii-pacmak/lib/generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ export interface GeneratorOptions {
* If this property is set, the generator will add "Base" to abstract class names
*/
addBasePostfixToAbstractClassNames?: boolean;

/**
* If this property is set, the generator will add runtime type checking code in places
* where compile-time type checking is not possible.
*/
runtimeTypeChecking: boolean;
}

export interface IGenerator {
Expand Down Expand Up @@ -88,7 +94,11 @@ export abstract class Generator implements IGenerator {
protected _reflectAssembly?: reflect.Assembly;
private fingerprint?: string;

public constructor(private readonly options: GeneratorOptions = {}) {}
public constructor(private readonly options: GeneratorOptions) {}

protected get runtimeTypeChecking() {
return this.options.runtimeTypeChecking;
}

protected get assembly(): spec.Assembly {
if (!this._assembly) {
Expand Down
20 changes: 16 additions & 4 deletions packages/jsii-pacmak/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ export async function pacmak({
parallel = true,
recurse = false,
rosettaTablet,
rosettaUnknownSnippets = undefined,
runtimeTypeChecking = true,
targets = Object.values(TargetName),
timers = new Timers(),
rosettaUnknownSnippets = undefined,
updateNpmIgnoreFiles = false,
validateAssemblies = false,
}: PacmakOptions): Promise<void> {
Expand Down Expand Up @@ -126,6 +127,7 @@ export async function pacmak({
force,
perLanguageDirectory,
rosetta,
runtimeTypeChecking,
},
),
)
Expand Down Expand Up @@ -251,6 +253,13 @@ export interface PacmakOptions {
*/
readonly rosettaTablet?: string;

/**
* Whether to inject runtime type checks in places where compile-time type checking is not performed.
*
* @default true
*/
readonly runtimeTypeChecking?: boolean;

/**
* The list of targets for which code should be generated. Unless `forceTarget` is `true`, a given target will only
* be generated for assemblies that have configured it.
Expand Down Expand Up @@ -295,6 +304,7 @@ async function buildTargetsForLanguage(
force,
perLanguageDirectory,
rosetta,
runtimeTypeChecking,
}: {
argv: { readonly [name: string]: any };
clean: boolean;
Expand All @@ -303,6 +313,7 @@ async function buildTargetsForLanguage(
force: boolean;
perLanguageDirectory: boolean;
rosetta: Rosetta;
runtimeTypeChecking: boolean;
},
): Promise<void> {
// ``argv.target`` is guaranteed valid by ``yargs`` through the ``choices`` directive.
Expand All @@ -312,13 +323,14 @@ async function buildTargetsForLanguage(
}

return factory(modules, {
arguments: argv,
clean: clean,
codeOnly: codeOnly,
rosetta,
force: force,
fingerprint: fingerprint,
arguments: argv,
force: force,
languageSubdirectory: perLanguageDirectory,
rosetta,
runtimeTypeChecking,
}).buildModules();
}

Expand Down
11 changes: 8 additions & 3 deletions packages/jsii-pacmak/lib/target.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,18 @@ export abstract class Target {
protected readonly targetName: string;
protected readonly assembly: reflect.Assembly;
protected readonly rosetta: Rosetta;
protected readonly runtimeTypeChecking: boolean;

protected abstract readonly generator: IGenerator;

public constructor(options: TargetOptions) {
this.packageDir = options.packageDir;
this.arguments = options.arguments;
this.assembly = options.assembly;
this.rosetta = options.rosetta;
this.fingerprint = options.fingerprint ?? true;
this.force = options.force ?? false;
this.arguments = options.arguments;
this.packageDir = options.packageDir;
this.rosetta = options.rosetta;
this.runtimeTypeChecking = options.runtimeTypeChecking;
this.targetName = options.targetName;
}

Expand Down Expand Up @@ -211,6 +213,9 @@ export interface TargetOptions {
/** The Rosetta instance */
rosetta: Rosetta;

/** Whether to generate runtime type-checking code */
runtimeTypeChecking: boolean;

/**
* Whether to fingerprint the produced artifacts.
* @default true
Expand Down
9 changes: 5 additions & 4 deletions packages/jsii-pacmak/lib/targets/dotnet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,14 @@ export class DotnetBuilder implements TargetBuilder {
private makeTarget(module: JsiiModule): Dotnet {
return new Dotnet(
{
targetName: this.targetName,
packageDir: module.moduleDirectory,
arguments: this.options.arguments,
assembly: module.assembly,
fingerprint: this.options.fingerprint,
force: this.options.force,
arguments: this.options.arguments,
packageDir: module.moduleDirectory,
rosetta: this.options.rosetta,
runtimeTypeChecking: this.options.runtimeTypeChecking,
targetName: this.targetName,
},
this.modules.map((m) => m.name),
);
Expand Down Expand Up @@ -316,7 +317,7 @@ export default class Dotnet extends Target {

this.generator = new DotNetGenerator(
assembliesCurrentlyBeingCompiled,
options.rosetta,
options,
);
}

Expand Down
20 changes: 16 additions & 4 deletions packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,35 @@ import { DotNetNameUtils } from './nameutils';
* CODE GENERATOR V2
*/
export class DotNetGenerator extends Generator {
private readonly nameutils: DotNetNameUtils = new DotNetNameUtils();

private readonly rosetta: Rosetta;

// Flags that tracks if we have already wrote the first member of the class
private firstMemberWritten = false;

private typeresolver!: DotNetTypeResolver;

private readonly nameutils: DotNetNameUtils = new DotNetNameUtils();

private dotnetRuntimeGenerator!: DotNetRuntimeGenerator;

private dotnetDocGenerator!: DotNetDocGenerator;

public constructor(
private readonly assembliesCurrentlyBeingCompiled: string[],
private readonly rosetta: Rosetta,
options: {
readonly rosetta: Rosetta;
readonly runtimeTypeChecking: boolean;
},
) {
super();
super(options);

// Override the openBlock to get a correct C# looking code block with the curly brace after the line
this.code.openBlock = function (text) {
this.line(text);
this.open('{');
};

this.rosetta = options.rosetta;
}

public async load(
Expand Down Expand Up @@ -646,6 +653,11 @@ export class DotNetGenerator extends Generator {
parameters?: readonly spec.Parameter[],
{ noMangle = false }: { noMangle?: boolean } = {},
): void {
if (!this.runtimeTypeChecking) {
// We were configured not to emit those, so bail out now.
return;
}

const unionParameters = parameters?.filter(({ type }) =>
containsUnionType(type),
);
Expand Down
19 changes: 13 additions & 6 deletions packages/jsii-pacmak/lib/targets/java.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,13 +328,14 @@ export class JavaBuilder implements TargetBuilder {

private makeTarget(module: JsiiModule, options: BuildOptions): Target {
return new Java({
targetName: this.targetName,
packageDir: module.moduleDirectory,
arguments: options.arguments,
assembly: module.assembly,
fingerprint: options.fingerprint,
force: options.force,
arguments: options.arguments,
packageDir: module.moduleDirectory,
rosetta: options.rosetta,
runtimeTypeChecking: options.runtimeTypeChecking,
targetName: this.targetName,
});
}
}
Expand Down Expand Up @@ -421,7 +422,7 @@ export default class Java extends Target {
public constructor(options: TargetOptions) {
super(options);

this.generator = new JavaGenerator(options.rosetta);
this.generator = new JavaGenerator(options);
}

public async build(sourceDir: string, outDir: string): Promise<void> {
Expand Down Expand Up @@ -619,8 +620,14 @@ class JavaGenerator extends Generator {
[name: string]: spec.AssemblyConfiguration;
} = {};

public constructor(private readonly rosetta: Rosetta) {
super({ generateOverloadsForMethodWithOptionals: true });
private readonly rosetta: Rosetta;

public constructor(options: {
readonly rosetta: Rosetta;
readonly runtimeTypeChecking: boolean;
}) {
super({ ...options, generateOverloadsForMethodWithOptionals: true });
this.rosetta = options.rosetta;
}

protected onBeginAssembly(assm: spec.Assembly, fingerprint: boolean) {
Expand Down
5 changes: 5 additions & 0 deletions packages/jsii-pacmak/lib/targets/js.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ export default class JavaScript extends Target {
// ##################

class PackOnly extends Generator {
public constructor() {
// NB: This does not generate code, so runtime type checking is irrelevant
super({ runtimeTypeChecking: false });
}

public async save(outdir: string, tarball: string, _: Legalese) {
// Intentionally ignore the Legalese field here... it's not useful here.
return super.save(outdir, tarball, {});
Expand Down
Loading

0 comments on commit 39923a5

Please sign in to comment.