Skip to content

Commit d75658e

Browse files
authored
feat: prepare support for intersection types (#4898)
This is a preparatory PR for adding support for intersection types (`A & B`) to jsii. That feature might land or might not, but to figure it out we first need to update `@jsii/spec` to allow modeling intersection types to begin with, as other jsii packages live outside this repo so we can't do an atomic commit adding the feature everywhere; we have to release an updated version of the spec first. To this end, we're doing the following: - Add the necessary fields to the spec to express the new feature. - Add a `usedFeatures` field to assemblies to indicate whether or not a "new feature" is being used, so that tools can reject an assembly if it's using a feature they don't support. - Wherever we call Rosetta directly from `jsii-pacmak`, we have to do runtime type assertions to make sure the types conversion is valid. The `usedFeatures` field is chosen rather than a schema version field, to maximize compatibility: we want to gate loading an assembly on whether or not the assembly actually *uses* the new feature, not on whether or not the producing tool *knows about* the new feature. JSON Schema validation would do the same thing (albeit with a worse error message), but for example the kernel doesn't do JSON Schema validation because it is a slow process. Proper support for type intersections is added to `@jsii/kernel` already (because it's trivial), but `jsii-pacmak` only has pro-forma type support; it will fail at runtime. --- 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
1 parent e92b9eb commit d75658e

File tree

12 files changed

+236
-12
lines changed

12 files changed

+236
-12
lines changed

packages/@jsii/kernel/src/kernel.test.ts

Lines changed: 68 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
/* eslint-disable @typescript-eslint/prefer-promise-reject-errors */
2+
import { Assembly, SchemaVersion } from '@jsii/spec';
23
import * as childProcess from 'child_process';
34
import * as fs from 'fs-extra';
45
import * as os from 'os';
@@ -1336,6 +1337,36 @@ defineTest('loading a module twice idepotently succeeds', async (sandbox) => {
13361337
});
13371338
});
13381339

1340+
defineTest.skipIf(!!recordingOutput)(
1341+
'loading an assembly with unsupported features fails',
1342+
async (sandbox) => {
1343+
const testAssembly: Assembly = {
1344+
author: {
1345+
name: 'Me',
1346+
roles: ['owner'],
1347+
},
1348+
description: 'Test',
1349+
fingerprint: 'asdf',
1350+
homepage: 'www.example.com',
1351+
jsiiVersion: '1.20.3',
1352+
license: 'MIT',
1353+
name: 'test',
1354+
repository: { type: 'other', url: 'www.example.com' },
1355+
schema: SchemaVersion.LATEST,
1356+
version: '1.2.3',
1357+
usedFeatures: ['will-never-be-used' as any],
1358+
};
1359+
1360+
await expect(async () =>
1361+
sandbox.load({
1362+
tarball: await makeJsiiTarball(testAssembly),
1363+
name: testAssembly.name,
1364+
version: testAssembly.version,
1365+
}),
1366+
).rejects.toThrow(/will-never-be-used/);
1367+
},
1368+
);
1369+
13391370
defineTest(
13401371
'fails if trying to load two different versions of the same module',
13411372
async (sandbox) => {
@@ -2275,27 +2306,57 @@ async function preparePackage(module: string, useCache = true) {
22752306
}
22762307

22772308
const packageRoot = findPackageRoot(module);
2278-
await new Promise<void>((ok, ko) => {
2279-
const child = childProcess.spawn('npm', ['pack', packageRoot], {
2280-
cwd: staging,
2309+
2310+
await runNpm(['pack', packageRoot], staging);
2311+
2312+
const dir = path.join(staging, (await fs.readdir(staging))[0]);
2313+
cache.set(module, dir);
2314+
return dir;
2315+
}
2316+
2317+
async function makeJsiiTarball(assembly: Assembly) {
2318+
const staging = await fs.mkdtemp(
2319+
path.join(os.tmpdir(), 'jsii-kernel-tests-'),
2320+
);
2321+
2322+
// clean up only if we are not recording, so playback can refer to these
2323+
if (!recordingOutput) {
2324+
process.on('exit', () => fs.removeSync(staging)); // cleanup
2325+
}
2326+
2327+
await fs.writeJson(path.join(staging, 'package.json'), {
2328+
name: assembly.name,
2329+
version: assembly.version,
2330+
});
2331+
2332+
await fs.writeJson(path.join(staging, '.jsii'), assembly);
2333+
2334+
await runNpm(['pack'], staging);
2335+
2336+
const tarballs = (await fs.readdir(staging)).filter((x) => x.endsWith('gz'));
2337+
2338+
return path.join(staging, tarballs[0]);
2339+
}
2340+
2341+
async function runNpm(args: string[], cwd: string): Promise<string> {
2342+
return new Promise<string>((ok, ko) => {
2343+
const child = childProcess.spawn('npm', args, {
2344+
cwd,
22812345
shell: true,
22822346
stdio: ['ignore', 'pipe', 'ignore'],
22832347
});
22842348
const stdout = new Array<Buffer>();
22852349
child.stdout.on('data', (chunk) => stdout.push(Buffer.from(chunk)));
22862350
child.once('close', (code, signal) => {
22872351
if (code === 0) {
2288-
return ok();
2352+
return ok(Buffer.concat(stdout).toString());
22892353
}
22902354
if (code != null) {
22912355
return ko(`'npm pack' exited with code ${code}`);
22922356
}
22932357
return ko(`'npm pack' killed by signal ${signal}`);
22942358
});
22952359
});
2296-
const dir = path.join(staging, (await fs.readdir(staging))[0]);
2297-
cache.set(module, dir);
2298-
return dir;
22992360
}
23002361

23012362
function findPackageRoot(pkg: string) {

packages/@jsii/kernel/src/kernel.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export class RuntimeError extends Error implements JsiiError {
3434
super(message);
3535
}
3636
}
37+
3738
export class Kernel {
3839
/**
3940
* Set to true for verbose debugging.

packages/@jsii/kernel/src/serialization.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -941,6 +941,10 @@ export function serializationType(
941941
);
942942
}
943943

944+
if (spec.isIntersectionTypeReference(typeRef.type)) {
945+
throw new Error(`Intersection types cannot be serialized`);
946+
}
947+
944948
// The next part of the conversion is lookup-dependent
945949
const type = lookup(typeRef.type.fqn);
946950

packages/@jsii/spec/src/assembly-utils.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as path from 'path';
33
import * as zlib from 'zlib';
44

55
import {
6+
ALL_TYPESYSTEM_ENFORCED_FEATURES,
67
Assembly,
78
SPEC_FILE_NAME,
89
SPEC_FILE_NAME_COMPRESSED,
@@ -122,11 +123,13 @@ const failNoReadfileProvided = (filename: string) => {
122123
* @param assemblyBuffer buffer containing SPEC_FILE_NAME contents
123124
* @param readFile a callback to use for reading additional support files
124125
* @param validate whether or not to validate the assembly
126+
* @param supportedFeatures the set of supported features (default: all features enforced by the type system)
125127
*/
126128
export function loadAssemblyFromBuffer(
127129
assemblyBuffer: Buffer,
128130
readFile: (filename: string) => Buffer = failNoReadfileProvided,
129131
validate = true,
132+
supportedFeatures: string[] = ALL_TYPESYSTEM_ENFORCED_FEATURES,
130133
): Assembly {
131134
let contents = JSON.parse(assemblyBuffer.toString('utf-8'));
132135

@@ -135,6 +138,20 @@ export function loadAssemblyFromBuffer(
135138
contents = followRedirect(contents, readFile);
136139
}
137140

141+
// Do feature checking *before* validating using JSONSchema, and do it always.
142+
// - In case validation is enabled, feature checking will produce a
143+
// more useful error message.
144+
// - In case validation is disabled, feature checking is cheap and will catch
145+
// common problems.
146+
const unsupported = ((contents as Assembly).usedFeatures ?? []).filter(
147+
(feat) => !supportedFeatures.includes(feat),
148+
);
149+
if (unsupported.length > 0) {
150+
throw new Error(
151+
`This jsii tool cannot load the given assembly; using unsupported feature(s): ${unsupported.join(', ')}`,
152+
);
153+
}
154+
138155
return validate ? validateAssembly(contents) : contents;
139156
}
140157

@@ -149,9 +166,10 @@ export function loadAssemblyFromBuffer(
149166
export function loadAssemblyFromPath(
150167
directory: string,
151168
validate = true,
169+
supportedFeatures?: string[],
152170
): Assembly {
153171
const assemblyFile = findAssemblyFile(directory);
154-
return loadAssemblyFromFile(assemblyFile, validate);
172+
return loadAssemblyFromFile(assemblyFile, validate, supportedFeatures);
155173
}
156174

157175
/**
@@ -165,13 +183,15 @@ export function loadAssemblyFromPath(
165183
export function loadAssemblyFromFile(
166184
pathToFile: string,
167185
validate = true,
186+
supportedFeatures?: string[],
168187
): Assembly {
169188
const data = fs.readFileSync(pathToFile);
170189
try {
171190
return loadAssemblyFromBuffer(
172191
data,
173192
(filename) => fs.readFileSync(path.resolve(pathToFile, '..', filename)),
174193
validate,
194+
supportedFeatures,
175195
);
176196
} catch (e: any) {
177197
throw new Error(`Error loading assembly from file ${pathToFile}:\n${e}`);

packages/@jsii/spec/src/assembly.ts

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,16 @@ export interface Assembly
1717
ReadMeContainer {
1818
/**
1919
* The version of the spec schema
20+
*
21+
* NOTE: we cannot ever change this value anymore! If we do, new instances of
22+
* assmblies that would have been backwards compatible with the old schema
23+
* still cannot be loaded, because the schema versions don't match exactly (as
24+
* validated by JSON Schema validators on loading).
25+
*
26+
* We *must* always emit this value as 'jsii/0.10.0'.
27+
*
28+
* Instead, see `usedFeatures` for a dynamic way to do feature validation
29+
* without relying on a fixed schema version.
2030
*/
2131
schema: SchemaVersion.LATEST;
2232

@@ -154,6 +164,18 @@ export interface Assembly
154164
* @default none
155165
*/
156166
bin?: { readonly [script: string]: string };
167+
168+
/**
169+
* List of features used in this assembly
170+
*
171+
* If a modern jsii feature is used in the assembly, a descriptive string
172+
* should be added here. This field will be used to selectively reject the
173+
* loading of assemblies that requires features not currently supported by the
174+
* jsii kernel, or downstream jsii tools.
175+
*
176+
* @default - Only original jsii features.
177+
*/
178+
usedFeatures?: JsiiFeature[];
157179
}
158180

159181
/**
@@ -545,7 +567,8 @@ export type TypeReference =
545567
| NamedTypeReference
546568
| PrimitiveTypeReference
547569
| CollectionTypeReference
548-
| UnionTypeReference;
570+
| UnionTypeReference
571+
| IntersectionTypeReference;
549572

550573
/**
551574
* The standard representation of the `any` type (includes optionality marker).
@@ -626,12 +649,36 @@ export interface UnionTypeReference {
626649
types: TypeReference[];
627650
};
628651
}
652+
629653
export function isUnionTypeReference(
630654
ref: TypeReference | undefined,
631655
): ref is UnionTypeReference {
632656
return !!(ref as UnionTypeReference)?.union;
633657
}
634658

659+
/**
660+
* Reference to an intersection type.
661+
*/
662+
export interface IntersectionTypeReference {
663+
/**
664+
* Indicates that this is an intersection type, which means it must satisfy all of the given types.
665+
*/
666+
intersection: {
667+
/**
668+
* All the possible types (including the primary type).
669+
*
670+
* @minItems 2
671+
*/
672+
types: TypeReference[];
673+
};
674+
}
675+
676+
export function isIntersectionTypeReference(
677+
ref: TypeReference | undefined,
678+
): ref is IntersectionTypeReference {
679+
return !!(ref as IntersectionTypeReference)?.intersection;
680+
}
681+
635682
/**
636683
* Methods and properties can be overridden from parent classes or implemented
637684
* from interfaces.
@@ -1028,6 +1075,39 @@ export function describeTypeReference(type?: TypeReference): string {
10281075
throw new Error('Unrecognized type reference');
10291076
}
10301077

1078+
/**
1079+
* Predefined constants for a set of jsii extension features
1080+
*/
1081+
export type JsiiFeature = 'intersection-types';
1082+
1083+
/**
1084+
* For every feature, is it enforced by the type system?
1085+
*
1086+
* Effectively: if a jsii tools links against the most recent version of the
1087+
* spec, is the TypeScript type system going to ensure that they must have
1088+
* support for a given new feature, through exhaustiveness checking?
1089+
*
1090+
* (This map also forces completeness, so we are guaranteed to have a string
1091+
* value for every possible `JsiiFeature` type branch).
1092+
*/
1093+
const IS_FEATURE_TYPESYSTEM_ENFORCED: Record<JsiiFeature, boolean> = {
1094+
'intersection-types': true,
1095+
};
1096+
1097+
/**
1098+
* A list of all jsii extension features
1099+
*/
1100+
export const ALL_FEATURES = Object.keys(IS_FEATURE_TYPESYSTEM_ENFORCED);
1101+
1102+
/**
1103+
* A list of all jsii extension features
1104+
*/
1105+
export const ALL_TYPESYSTEM_ENFORCED_FEATURES = Object.entries(
1106+
IS_FEATURE_TYPESYSTEM_ENFORCED,
1107+
)
1108+
.filter(([_, v]) => v)
1109+
.map(([k, _]) => k);
1110+
10311111
/**
10321112
* Determines whether an entity is deprecated.
10331113
*

packages/jsii-pacmak/lib/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { cwd } from 'process';
66
import * as logging from './logging';
77
import { findJsiiModules, updateAllNpmIgnores } from './npm-modules';
88
import { JsiiModule } from './packaging';
9+
import { assertSpecIsRosettaCompatible } from './rosetta-assembly';
910
import { ALL_BUILDERS, TargetName } from './targets';
1011
import { Timers } from './timer';
1112
import { Toposorted } from './toposort';
@@ -84,6 +85,9 @@ export async function pacmak({
8485
return Promise.all(
8586
modulesToPackageFlat.map(async (m) => {
8687
await m.load(system, validateAssemblies);
88+
89+
assertSpecIsRosettaCompatible(m.assembly.spec);
90+
8791
return rosetta.addAssembly(m.assembly.spec, m.moduleDirectory);
8892
}),
8993
);
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import * as spec from '@jsii/spec';
2+
import { JsiiFeature } from '@jsii/spec';
3+
import { RosettaTabletReader } from 'jsii-rosetta';
4+
5+
/**
6+
* Assert that the given spec is safe to give to Rosetta
7+
*
8+
* We have to do it like this, because Rosetta has its own internal copy of the
9+
* spec and new schema additions may make it technically illegal to assign our
10+
* Assembly instance to Rosetta's Assembly type.
11+
*
12+
* We do runtime validation here to make sure that assignment is safe,
13+
* and then assert it in the type system.
14+
*
15+
* The check should be cheap, this gets called quite a lot.
16+
*/
17+
export function assertSpecIsRosettaCompatible(
18+
x: spec.Assembly,
19+
): asserts x is Parameters<RosettaTabletReader['addAssembly']>[0] {
20+
const rosettaSupportedFeatures: JsiiFeature[] = [];
21+
22+
const unsupported = (x.usedFeatures ?? []).filter(
23+
(f) => !rosettaSupportedFeatures.includes(f),
24+
);
25+
if (unsupported.length > 0) {
26+
throw new Error(
27+
`This assembly uses features Rosetta doesn't support yet: ${unsupported.join(', ')}`,
28+
);
29+
}
30+
}

packages/jsii-pacmak/lib/targets/dotnet/dotnetdocgenerator.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import * as xmlbuilder from 'xmlbuilder';
1111

1212
import { renderSummary } from '../_utils';
1313
import { DotNetNameUtils } from './nameutils';
14+
import { assertSpecIsRosettaCompatible } from '../../rosetta-assembly';
1415

1516
/**
1617
* Generates the Jsii attributes and calls for the .NET runtime
@@ -160,6 +161,7 @@ export class DotNetDocGenerator {
160161
}
161162

162163
private convertExample(example: string, apiLocation: ApiLocation): string {
164+
assertSpecIsRosettaCompatible(this.assembly);
163165
const translated = this.rosetta.translateExample(
164166
apiLocation,
165167
example,
@@ -170,6 +172,7 @@ export class DotNetDocGenerator {
170172
}
171173

172174
private convertSamplesInMarkdown(markdown: string, api: ApiLocation): string {
175+
assertSpecIsRosettaCompatible(this.assembly);
173176
return this.rosetta.translateSnippetsInMarkdown(
174177
api,
175178
markdown,

0 commit comments

Comments
 (0)