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(pacmak): allow opt-out of runtime type checking generation #3710

Merged
merged 5 commits into from
Aug 18, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
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 @@ -20,28 +20,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 @@ -645,6 +652,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