From 8f3d8bb2e33fc126a8a15c68ce595e76f0a264d7 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 5 Sep 2025 13:10:16 +0200 Subject: [PATCH 1/3] feat: intersection types This adds support for intersection types to the jsii compiler. There are a number of restrictions to the use of intersection types: - They can only appear in argument position, or in structs that are exclusively used as the input to functions. - Not as return values of functions - Not in structs that are returned by functions - Not as arguments to constructors - Not as property types - They must intersect only (behavioral) interface types. - Not structs - Not primitives - Not class types - All shared members of interfaces that are intersected must have the same types and arguments as each other. --- package.json | 2 +- src/assembler.ts | 368 +++++++++++++++++++++++++-- src/helpers.ts | 87 ++++++- src/jsii-diagnostic.ts | 32 +++ src/project-info.ts | 3 +- src/transforms/deprecated-remover.ts | 16 +- src/type-reference.ts | 55 ++++ src/type-visitor.ts | 54 ++++ test/abstract.test.ts | 7 +- test/compiler-helpers.ts | 16 ++ test/compiler.test.ts | 2 +- test/enums.test.ts | 7 +- test/property-inference.test.ts | 15 +- test/type-intersections.test.ts | 306 ++++++++++++++++++++++ yarn.lock | 9 +- 15 files changed, 924 insertions(+), 55 deletions(-) create mode 100644 src/type-reference.ts create mode 100644 src/type-visitor.ts create mode 100644 test/compiler-helpers.ts create mode 100644 test/type-intersections.test.ts diff --git a/package.json b/package.json index 25e12514..2dafc034 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ }, "dependencies": { "@jsii/check-node": "1.113.0", - "@jsii/spec": "^1.113.0", + "@jsii/spec": "^1.114.1", "case": "^1.6.3", "chalk": "^4", "fast-deep-equal": "^3.1.3", diff --git a/src/assembler.ts b/src/assembler.ts index ed48a78e..25d10f15 100644 --- a/src/assembler.ts +++ b/src/assembler.ts @@ -23,6 +23,8 @@ import { DeprecatedRemover } from './transforms/deprecated-remover'; import { DeprecationWarningsInjector } from './transforms/deprecation-warnings'; import { RuntimeTypeInfoInjector } from './transforms/runtime-info'; import { combinedTransformers } from './transforms/utils'; +import { typeReferenceEqual, typeReferenceToString } from './type-reference'; +import { isBehavioralInterfaceType, visitType, visitTypeReference } from './type-visitor'; import { JsiiError } from './utils'; import { Validator } from './validator'; import { SHORT_VERSION, VERSION } from './version'; @@ -44,6 +46,7 @@ export class Assembler implements Emitter { private readonly mainFile: string; private readonly tscRootDir?: string; private readonly compressAssembly?: boolean; + private readonly usedFeatures = new Set(); private readonly _typeChecker: ts.TypeChecker; @@ -175,6 +178,8 @@ export class Assembler implements Emitter { this.callDeferredsInOrder(); + this.validateTypesAgainstPositions(); + // Skip emitting if any diagnostic message is an error if (this._diagnostics.find((diag) => diag.category === ts.DiagnosticCategory.Error) != null) { LOG.debug('Skipping emit due to errors.'); @@ -220,6 +225,7 @@ export class Assembler implements Emitter { jsiiVersion, bin: this.projectInfo.bin, fingerprint: '', + usedFeatures: this.usedFeatures.size > 0 ? Array.from(this.usedFeatures) : undefined, }; if (this.deprecatedRemover) { @@ -304,12 +310,12 @@ export class Assembler implements Emitter { * * @param fqn FQN of the current type (the type that has a dependency on baseTypes) * @param baseTypes Array of type references to be looked up - * @param referencingNode Node to report a diagnostic on if we fail to look up a t ype + * @param referencingNode Node to report a diagnostic on if we fail to look up a type * @param cb Callback to be invoked with the Types corresponding to the TypeReferences in baseTypes */ private _deferUntilTypesAvailable( fqn: string, - baseTypes: Array, + baseTypes: Array, referencingNode: ts.Node | undefined, cb: (...xs: spec.Type[]) => void, ) { @@ -318,10 +324,20 @@ export class Assembler implements Emitter { cb(); return; } - const baseFqns = baseTypes.map((bt) => (typeof bt === 'string' ? bt : bt.fqn)); + const baseFqns = baseTypes.flatMap((bt) => { + if (typeof bt === 'string') { + return [bt]; + } + if (spec.isNamedTypeReference(bt)) { + return [bt.fqn]; + } + return []; + }); this._defer(fqn, baseFqns, () => { - const resolved = baseFqns.map((x) => this._dereference(x, referencingNode)).filter((x) => x !== undefined); + const resolved = baseFqns + .map((x) => this._dereference(x, referencingNode, 'used-in-syntax')) + .filter((x) => x !== undefined); if (resolved.length > 0) { cb(...(resolved as spec.Type[])); } @@ -354,6 +370,7 @@ export class Assembler implements Emitter { private _dereference( ref: string | spec.NamedTypeReference, referencingNode: ts.Node | undefined, + usedInSyntax: 'used-in-syntax' | 'just-validating', ): spec.Type | undefined { if (typeof ref !== 'string') { ref = ref.fqn; @@ -382,7 +399,7 @@ export class Assembler implements Emitter { } } - if (!type) { + if (!type && usedInSyntax === 'used-in-syntax') { this._diagnostics.push( JsiiDiagnostic.JSII_9002_UNRESOLVEABLE_TYPE.create( referencingNode!, // Cheating here for now, until the referencingNode can be made required @@ -500,7 +517,7 @@ export class Assembler implements Emitter { }); const fqn = (dep && symbolId ? symbolIdIndex(dep)[symbolId] : undefined) ?? fallbackFqn; - if (!fqn || !this._dereference({ fqn }, sym.valueDeclaration)) { + if (!fqn || !this._dereference({ fqn }, sym.valueDeclaration, 'used-in-syntax')) { if (!hasError) { this._diagnostics.push( JsiiDiagnostic.JSII_3002_USE_OF_UNEXPORTED_FOREIGN_TYPE.create( @@ -1295,10 +1312,15 @@ export class Assembler implements Emitter { if (signature) { for (const param of signature.getParameters()) { jsiiType.initializer.parameters = jsiiType.initializer.parameters ?? []; - jsiiType.initializer.parameters.push( - // eslint-disable-next-line no-await-in-loop - this._toParameter(param, ctx.replaceStability(jsiiType.docs?.stability)), - ); + + const jsiiParam = // eslint-disable-next-line no-await-in-loop + this._toParameter(param, ctx.replaceStability(jsiiType.docs?.stability)); + + if (containsIntersection(jsiiParam.type)) { + this._diagnostics.push(JsiiDiagnostic.JSII_1010_INTERSECTION_NOT_IN_CTOR.create(param.declarations?.[0])); + } + + jsiiType.initializer.parameters.push(jsiiParam); jsiiType.initializer.variadic = jsiiType.initializer?.parameters?.some((p) => !!p.variadic) || undefined; jsiiType.initializer.protected = (ts.getCombinedModifierFlags(ctorDeclaration) & ts.ModifierFlags.Protected) !== 0 || undefined; @@ -2151,7 +2173,22 @@ export class Assembler implements Emitter { } if (type.isUnion() && !_isEnumLike(type)) { - return _unionType.call(this); + return _setType.call(this, 'union'); + } + + if (type.isIntersection()) { + this.usedFeatures.add('intersection-types'); + const ret = _setType.call(this, 'intersection'); + const intersectionType = ret.type as spec.IntersectionTypeReference; + + this.validateIntersectionType(intersectionType, declaration); + + return ret; + } + + if ((type.getFlags() & ts.TypeFlags.Never) !== 0) { + this._diagnostics.push(JsiiDiagnostic.JSII_1007_NEVER_TYPE.create(declaration)); + return { type: spec.CANONICAL_ANY }; } if (!type.symbol) { @@ -2284,7 +2321,7 @@ export class Assembler implements Emitter { return undefined; } - function _unionType(this: Assembler): spec.OptionalValue { + function _setType(this: Assembler, field: 'union' | 'intersection'): spec.OptionalValue { const types = new Array(); let optional: boolean | undefined; @@ -2301,8 +2338,150 @@ export class Assembler implements Emitter { types.push(resolvedType); } - return types.length === 1 ? { optional, type: types[0] } : { optional, type: { union: { types } } }; + const returnedType: spec.UnionTypeReference | spec.IntersectionTypeReference = { + [field]: { types }, + } as any; + return types.length === 1 ? { optional, type: types[0] } : { optional, type: returnedType }; + } + } + + /** + * Validate the restrictions on an intersection type reference + * + * - Type only consists of (behavioral) interface types + * - For all types referenced in the intersection, the definitions of all shared + * members must match exactly. + */ + private validateIntersectionType(intersectionType: spec.IntersectionTypeReference, declaration: ts.Node | undefined) { + // Validate that this intersection type only consists of interface types + this._deferUntilTypesAvailable('', intersectionType.intersection.types, declaration, (...elementTypes) => { + if ( + // If we didn't resolve some things, they're probably primitives + elementTypes.length !== intersectionType.intersection.types.length || + // Or if there's something explicitly a primitive + elementTypes.some((t) => !isBehavioralInterfaceType(t)) + ) { + this._diagnostics.push(JsiiDiagnostic.JSII_1008_ONLY_INTERFACE_INTERSECTION.create(declaration)); + } + + // Check that shared members are the same + const interfaces = elementTypes.filter(isBehavioralInterfaceType); + const allProps = union(...interfaces.map((int) => this.allProperties(int))); + const allMethods = union(...interfaces.map((int) => this.allMethods(int))); + + for (let i = 1; i < interfaces.length; i++) { + let int0 = interfaces[0]; + let int1 = interfaces[i]; + + for (const prop of allProps) { + const p1 = int0.properties?.find((p) => p.name === prop); + const p2 = int1.properties?.find((p) => p.name === prop); + const diff = this.comparePropForIntersection(p1, p2); + if (diff) { + this._diagnostics.push( + JsiiDiagnostic.JSII_1011_INTERSECTION_MEMBER_DIFFERENT.create( + declaration, + prop, + int0.name, + diff[0], + int1.name, + diff[1], + ), + ); + } + } + + for (const meth of allMethods) { + const m1 = int0.methods?.find((p) => p.name === meth); + const m2 = int1.methods?.find((p) => p.name === meth); + const diff = this.compareMethodForIntersection(m1, m2); + if (diff) { + this._diagnostics.push( + JsiiDiagnostic.JSII_1011_INTERSECTION_MEMBER_DIFFERENT.create( + declaration, + meth, + int0.name, + diff[0], + int1.name, + diff[1], + ), + ); + } + } + } + }); + } + + private comparePropForIntersection( + a: spec.Property | undefined, + b: spec.Property | undefined, + ): [string, string] | undefined { + if (!a || !b) { + return undefined; + } + + if (!typeReferenceEqual(a.type, b.type)) { + return [typeReferenceToString(a.type), typeReferenceToString(b.type)]; + } + + return ( + cmpDesc(a.static, b.static, (s) => (s ? 'static' : 'instance member')) ?? + cmpDesc(a.optional, b.optional, (o) => (o ? 'optional' : 'required')) ?? + cmpDesc(a.abstract, b.abstract, (s) => (s ? 'abstract' : 'concrete')) + ); + } + + private compareMethodForIntersection( + a: spec.Method | undefined, + b: spec.Method | undefined, + ): [string, string] | undefined { + if (!a || !b) { + return undefined; + } + + if (a.returns && b.returns) { + if (!typeReferenceEqual(a.returns.type, b.returns.type)) { + return [typeReferenceToString(a.returns.type), typeReferenceToString(b.returns.type)]; + } + + const x = cmpDesc(a.returns.optional, b.returns.optional, (o) => + o ? 'return type optional' : 'return type required', + ); + if (x) { + return x; + } + } + + const paramsA = a.parameters ?? []; + const paramsB = b.parameters ?? []; + if (paramsA.length !== paramsB.length) { + return [`${paramsA.length} parameters`, `${paramsB.length} parameters`]; + } + for (let i = 0; i < paramsA.length; i++) { + const p1 = paramsA[i]; + const p2 = paramsB[i]; + + if (!typeReferenceEqual(p1.type, p2.type)) { + return [typeReferenceToString(p1.type), typeReferenceToString(p2.type)]; + } + + const x = + cmpDesc(p1.optional, p2.optional, (o) => (o ? `parameter ${i + 1} optional` : `parameter ${i + 1} required`)) ?? + cmpDesc(p1.variadic, p2.variadic, (v) => + v ? `parameter ${i + 1} variadic` : `parameter ${i + 1} not variadic`, + ); + if (x) { + return x; + } } + + return ( + cmpDesc(a.abstract, b.abstract, (s) => (s ? 'abstract' : 'concrete')) ?? + cmpDesc(a.async, b.async, (s) => (s ? 'async' : 'async')) ?? + cmpDesc(a.protected, b.protected, (p) => (p ? 'protected' : 'public')) ?? + cmpDesc(a.variadic, b.variadic, (v) => (v ? 'variadic' : 'non-variadic')) ?? + cmpDesc(a.static, b.static, (s) => (s ? 'static' : 'instance member')) + ); } private callDeferredsInOrder() { @@ -2311,7 +2490,7 @@ export class Assembler implements Emitter { // All fqns in dependency lists that don't have any pending // deferreds themselves can be executed now, so are removed from // dependency lists. - const pendingFqns = new Set(this._deferred.map((x) => x.fqn)); + const pendingFqns = new Set(this._deferred.map((x) => x.fqn).filter((x) => x)); for (const deferred of this._deferred) { restrictDependenciesTo(deferred, pendingFqns); } @@ -2350,22 +2529,37 @@ export class Assembler implements Emitter { * Return the set of all (inherited) properties of an interface */ private allProperties(root: spec.InterfaceType): Set { - const ret = new Set(); + return this.allInterfacesRecursively(root, (int) => { + return (int.properties ?? []).map((p) => p.name); + }); + } + + /** + * Return the set of all (inherited) methods of an interface + */ + private allMethods(root: spec.InterfaceType): Set { + return this.allInterfacesRecursively(root, (int) => { + return (int.methods ?? []).map((p) => p.name); + }); + } + + private allInterfacesRecursively(root: spec.InterfaceType, cb: (x: spec.InterfaceType) => Iterable): Set { + const ret = new Set(); recurse.call(this, root); return ret; function recurse(this: Assembler, int: spec.InterfaceType) { - for (const property of int.properties ?? []) { - ret.add(property.name); + for (const x of cb(int)) { + ret.add(x); } for (const baseRef of int.interfaces ?? []) { - const base = this._dereference(baseRef, undefined); + const base = this._dereference(baseRef, undefined, 'used-in-syntax'); if (!base) { - throw new Error('Impossible to have unresolvable base in allProperties()'); + throw new Error('Impossible to have unresolvable base in allInterfacesRecursively()'); } if (!spec.isInterfaceType(base)) { - throw new Error('Impossible to have non-interface base in allProperties()'); + throw new Error('Impossible to have non-interface base in allInterfacesRecursively()'); } recurse.call(this, base); @@ -2431,6 +2625,99 @@ export class Assembler implements Emitter { return this.findPackageInfo(parent); } } + + /** + * Validate types against input/output positions in functions and APIs + * + * Currently used to validate that intersection types are only used in input position, + * not output position + */ + private validateTypesAgainstPositions() { + const validatedFqns = { + in: new Set(), + out: new Set(), + }; + + for (const type of this._types.values()) { + if (spec.isClassType(type)) { + for (const ctorParam of type.initializer?.parameters ?? []) { + validateRefFor.call(this, 'in', ctorParam.type, `a constructor parameter of ${type.name}`); + } + } + if (spec.isClassType(type) || isBehavioralInterfaceType(type)) { + for (const property of type.properties ?? []) { + validateRefFor.call(this, 'out', property.type, `type of property ${type.name}.${property.name}`); + } + + for (const method of type.methods ?? []) { + for (const param of method.parameters ?? []) { + validateRefFor.call(this, 'in', param.type, `a parameter of method ${type.name}.${method.name}()`); + } + if (method.returns) { + validateRefFor.call( + this, + 'out', + method.returns.type, + `return type of method ${type.name}.${method.name}()`, + ); + } + } + } + } + + function validateRefFor(this: Assembler, dir: 'in' | 'out', typeRef: spec.TypeReference, reason: string) { + visitTypeReference(typeRef, { + named: (ref) => { + // Named types we'll only validate once for every direction + if (validatedFqns[dir].has(ref.fqn)) { + return; + } + + const type = this._dereference(ref, undefined, 'just-validating'); + if (!type) { + // Maybe this is an unexported type. + return; + } + validateTypeFor.call(this, dir, type, reason); + }, + primitive: () => {}, + collection: (ref) => { + validateRefFor.call(this, dir, ref.collection.elementtype, reason); + }, + union: (ref) => { + for (const t of ref.union.types) { + validateRefFor.call(this, dir, t, reason); + } + }, + intersection: (ref) => { + if (dir === 'out') { + this._diagnostics.push(JsiiDiagnostic.JSII_1009_INTERSECTION_ONLY_INPUT.createDetached(reason)); + } + + for (const t of ref.intersection.types) { + validateRefFor.call(this, dir, t, reason); + } + }, + }); + } + + function validateTypeFor(this: Assembler, dir: 'in' | 'out', type: spec.Type, reason: string) { + visitType(type, { + dataType: (t) => { + // We only need to validate data types, because classes and interfaces + // are done as part of the main loop. + // + // Recurse. + for (const prop of t.properties ?? []) { + validateRefFor.call(this, dir, prop.type, `type of property ${t.name}.${prop.name}, ${reason}`); + } + }, + classType: () => {}, + interfaceType: () => {}, + enumType: () => {}, + }); + } + } } export interface AssemblerOptions { @@ -2691,18 +2978,28 @@ function apply(x: T | undefined, fn: (x: T) => U | undefined): U | undefin } /** - * Return the intersection of two sets + * Return the intersection of N sets */ -function intersection(xs: Set, ys: Set): Set { - const ret = new Set(); - for (const x of xs) { - if (ys.has(x)) { - ret.add(x); +function intersection(...xss: Array>): Set { + if (xss.length === 0) { + return new Set(); + } + const ret = new Set(xss[0]); + for (const x of xss[0]) { + if (!xss.every((xs) => xs.has(x))) { + ret.delete(x); } } return ret; } +/** + * Return the union of N sets + */ +function union(...xss: Array>): Set { + return new Set(xss.flatMap((xs) => Array.from(xs))); +} + /** * Return all members names of a JSII interface type * @@ -2750,6 +3047,12 @@ function* intersect(xs: Set, ys: Set) { } } +function cmpDesc(a: T, b: T, desc: (x: T) => string): [string, string] | undefined { + const desc1 = desc(a); + const desc2 = desc(b); + return desc1 !== desc2 ? [desc1, desc2] : undefined; +} + function noEmptyDict(xs: Record | undefined): Record | undefined { if (xs == null || Object.keys(xs).length === 0) { return undefined; @@ -2957,6 +3260,19 @@ function _findHint(decl: ts.Declaration, hint: string): ts.JSDocTag | undefined return node; } +function containsIntersection(type: spec.TypeReference): boolean { + if (spec.isIntersectionTypeReference(type)) { + return true; + } + if (spec.isUnionTypeReference(type)) { + return type.union.types.some(containsIntersection); + } + if (spec.isCollectionTypeReference(type)) { + return containsIntersection(type.collection.elementtype); + } + return false; +} + /** * A location where a type can be used. */ diff --git a/src/helpers.ts b/src/helpers.ts index fb08c2d4..22fd97be 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -11,7 +11,7 @@ import * as os from 'node:os'; import * as path from 'node:path'; import { PackageJson, loadAssemblyFromPath, writeAssembly } from '@jsii/spec'; import * as spec from '@jsii/spec'; -import { DiagnosticCategory } from 'typescript'; +import { Diagnostic, DiagnosticCategory } from 'typescript'; import { Compiler, CompilerOptions } from './compiler'; import { loadProjectInfo, ProjectInfo } from './project-info'; @@ -25,6 +25,11 @@ export type MultipleSourceFiles = { [name: string]: string; }; +/** + * Assembly features supported by this compiler + */ +export const ASSEMBLY_FEATURES_SUPPORTED: spec.JsiiFeature[] = ['intersection-types']; + /** * Compile a piece of source and return the JSII assembly for it * @@ -39,10 +44,24 @@ export function sourceToAssemblyHelper( source: string | MultipleSourceFiles, options?: TestCompilationOptions | ((obj: PackageJson) => void), ): spec.Assembly { - return compileJsiiForTest(source, options).assembly; + const result = compileJsiiForTest(source, options); + if (result.type !== 'success') { + throw new Error('Compilation failed'); + } + return result.assembly; } +export type HelperCompilationOut = HelperCompilationResult | HelperCompilationFailure; + +/** + * Successful output of a compilation command (for testing) + * + * A better name would have been `HelperCompilationSuccess`, but the name is part of + * the public API surface, so we keep it like this. + */ export interface HelperCompilationResult { + readonly type: 'success'; + /** * The generated assembly */ @@ -62,6 +81,22 @@ export interface HelperCompilationResult { * Whether to compress the assembly file */ readonly compressAssembly: boolean; + + /** + * Diagnostics that occurred during compilation + */ + readonly diagnostics: readonly Diagnostic[]; +} + +export interface HelperCompilationFailure { + readonly type: 'failure'; + + /** + * Diagnostics that occurred during compilation + * + * Contains at least one error. + */ + readonly diagnostics: readonly Diagnostic[]; } /** @@ -74,11 +109,11 @@ export interface HelperCompilationResult { * @param options accepts a callback for historical reasons but really expects to * take an options object. */ -export function compileJsiiForTest( +export function compileJsiiForTest( source: string | { 'index.ts': string; [name: string]: string }, - options?: TestCompilationOptions | ((obj: PackageJson) => void), + options?: O | ((obj: PackageJson) => void), compilerOptions?: Omit, -): HelperCompilationResult { +): ResultOrSuccess { if (typeof source === 'string') { source = { 'index.ts': source }; } @@ -108,14 +143,25 @@ export function compileJsiiForTest( const emitResult = compiler.emit(); const errors = emitResult.diagnostics.filter((d) => d.category === DiagnosticCategory.Error); - for (const error of errors) { - console.error(formatDiagnostic(error, projectInfo.projectRoot)); - // logDiagnostic() doesn't work out of the box, so console.error() it is. + + if (typeof options !== 'object' || !options?.captureDiagnostics) { + for (const error of errors) { + console.error(formatDiagnostic(error, projectInfo.projectRoot)); + // logDiagnostic() doesn't work out of the box, so console.error() it is. + } } + if (errors.length > 0 || emitResult.emitSkipped) { + if (typeof options === 'object' && options?.captureDiagnostics) { + return { + type: 'failure', + diagnostics: emitResult.diagnostics, + } satisfies HelperCompilationFailure; + } throw new JsiiError('There were compiler errors'); } - const assembly = loadAssemblyFromPath(process.cwd(), false); + + const assembly = loadAssemblyFromPath(process.cwd(), false, ASSEMBLY_FEATURES_SUPPORTED); const files: Record = {}; for (const filename of Object.keys(source)) { @@ -141,14 +187,20 @@ export function compileJsiiForTest( } return { + type: 'success', assembly, files, packageJson, compressAssembly: isOptionsObject(options) && options.compressAssembly ? true : false, - } as HelperCompilationResult; - }); + diagnostics: emitResult.diagnostics, + } satisfies HelperCompilationResult; + }) as ResultOrSuccess; } +type ResultOrSuccess = O['captureDiagnostics'] extends true + ? HelperCompilationOut + : HelperCompilationResult; + function inTempDir(block: () => T): T { const origDir = process.cwd(); const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'jsii')); @@ -236,6 +288,13 @@ export interface TestCompilationOptions { * @default false */ readonly compressAssembly?: boolean; + + /** + * Whether or not to print the diagnostics + * + * @default false + */ + readonly captureDiagnostics?: boolean; } function isOptionsObject( @@ -278,7 +337,11 @@ export class TestWorkspace { /** * Add a test-compiled jsii assembly as a dependency */ - public addDependency(dependencyAssembly: HelperCompilationResult) { + public addDependency(dependencyAssembly: HelperCompilationOut) { + if (dependencyAssembly.type !== 'success') { + throw new JsiiError('Cannot add dependency: assembly compilation failed'); + } + if (this.installed.has(dependencyAssembly.assembly.name)) { throw new JsiiError( `A dependency with name '${dependencyAssembly.assembly.name}' was already installed. Give one a different name.`, diff --git a/src/jsii-diagnostic.ts b/src/jsii-diagnostic.ts index be64d94f..2ec840f8 100644 --- a/src/jsii-diagnostic.ts +++ b/src/jsii-diagnostic.ts @@ -294,6 +294,38 @@ export class JsiiDiagnostic implements ts.Diagnostic { name: 'typescript-restriction/generic-type', }); + public static readonly JSII_1007_NEVER_TYPE = Code.error({ + code: 1007, + formatter: () => 'The "never" type is not allowed because it cannot be represented in target languages.', + name: 'typescript-restriction/no-never-type', + }); + + public static readonly JSII_1008_ONLY_INTERFACE_INTERSECTION = Code.error({ + code: 1008, + formatter: () => 'Intersection types must combine exclusively interfaces (IOne & ITwo)', + name: 'typescript-restriction/only-interface-intersection', + }); + + public static readonly JSII_1009_INTERSECTION_ONLY_INPUT = Code.error({ + code: 1009, + formatter: (location: string) => + `Intersection types may only be used as inputs, but ${location} is an intersection type used as output`, + name: 'typescript-restriction/intersection-only-input', + }); + + public static readonly JSII_1010_INTERSECTION_NOT_IN_CTOR = Code.error({ + code: 1010, + formatter: () => `Intersection types cannot be used as constructor arguments`, + name: 'typescript-restriction/intersection-no-ctor', + }); + + public static readonly JSII_1011_INTERSECTION_MEMBER_DIFFERENT = Code.error({ + code: 1011, + formatter: (member: string, type1: string, reason1: string, type2: string, reason2: string) => + `Member ${member} is different between types in a type intersection: ${type1} (${reason1}) and ${type2} (${reason2})`, + name: 'typescript-restriction/intersection-member-different', + }); + public static readonly JSII_1999_UNSUPPORTED = Code.error({ code: 1999, formatter: ({ diff --git a/src/project-info.ts b/src/project-info.ts index 376d44f0..a2d33633 100644 --- a/src/project-info.ts +++ b/src/project-info.ts @@ -7,6 +7,7 @@ import * as semver from 'semver'; import * as ts from 'typescript'; import { findDependencyDirectory } from './common/find-utils'; +import { ASSEMBLY_FEATURES_SUPPORTED } from './helpers'; import { JsiiDiagnostic } from './jsii-diagnostic'; import { TypeScriptConfigValidationRuleSet } from './tsconfig'; import { JsiiError, parsePerson, parseRepository } from './utils'; @@ -392,7 +393,7 @@ class DependencyResolver { return this.cache.get(jsiiFile)!; } - const assembly = loadAssemblyFromFile(jsiiFile); + const assembly = loadAssemblyFromFile(jsiiFile, true, ASSEMBLY_FEATURES_SUPPORTED); // Continue loading any dependencies declared in the asm const resolvedDependencies = assembly.dependencies diff --git a/src/transforms/deprecated-remover.ts b/src/transforms/deprecated-remover.ts index 5d7e98a9..9f3bcd55 100644 --- a/src/transforms/deprecated-remover.ts +++ b/src/transforms/deprecated-remover.ts @@ -5,6 +5,7 @@ import { EnumMember, Initializer, InterfaceType, + IntersectionTypeReference, isClassOrInterfaceType, isClassType, isCollectionTypeReference, @@ -12,11 +13,13 @@ import { isMethod, isNamedTypeReference, isPrimitiveTypeReference, + isUnionTypeReference, Method, Parameter, Property, Stability, TypeReference, + UnionTypeReference, } from '@jsii/spec'; import * as ts from 'typescript'; @@ -305,7 +308,10 @@ export class DeprecatedRemover { if (isCollectionTypeReference(ref)) { return this.tryFindReference(ref.collection.elementtype, fqns); } - return ref.union.types.map((type) => this.tryFindReference(type, fqns)).find((typeRef) => typeRef != null); + + return setTypeMembers(ref) + .map((type) => this.tryFindReference(type, fqns)) + .find((typeRef) => typeRef != null); } private shouldFqnBeStripped(fqn: string) { @@ -770,3 +776,11 @@ class DeprecationRemovalTransformer { ); } } + +function setTypeMembers(x: UnionTypeReference | IntersectionTypeReference) { + if (isUnionTypeReference(x)) { + return x.union.types; + } else { + return x.intersection.types; + } +} diff --git a/src/type-reference.ts b/src/type-reference.ts new file mode 100644 index 00000000..bf1f4564 --- /dev/null +++ b/src/type-reference.ts @@ -0,0 +1,55 @@ +import * as spec from '@jsii/spec'; +import { visitTypeReference } from './type-visitor'; + +/** + * Convert a type reference to a string + */ +export function typeReferenceToString(x: spec.TypeReference): string { + return visitTypeReference(x, { + named: function (ref: spec.NamedTypeReference) { + return ref.fqn; + }, + primitive: function (ref: spec.PrimitiveTypeReference) { + return ref.primitive; + }, + collection: function (ref: spec.CollectionTypeReference) { + return `${ref.collection.kind}<${typeReferenceToString(ref.collection.elementtype)}>`; + }, + union: function (ref: spec.UnionTypeReference) { + return ref.union.types.map(typeReferenceToString).join(' | '); + }, + intersection: function (ref: spec.IntersectionTypeReference) { + return ref.intersection.types.map(typeReferenceToString).join(' & '); + }, + }); +} + +/** + * Return whether the given type references are equal + */ +export function typeReferenceEqual(a: spec.TypeReference, b: spec.TypeReference): boolean { + if (spec.isNamedTypeReference(a) && spec.isNamedTypeReference(b)) { + return a.fqn === b.fqn; + } + if (spec.isPrimitiveTypeReference(a) && spec.isPrimitiveTypeReference(b)) { + return a.primitive === b.primitive; + } + if (spec.isCollectionTypeReference(a) && spec.isCollectionTypeReference(b)) { + return ( + a.collection.kind === b.collection.kind && typeReferenceEqual(a.collection.elementtype, b.collection.elementtype) + ); + } + if (spec.isUnionTypeReference(a) && spec.isUnionTypeReference(b)) { + return ( + a.union.types.length === b.union.types.length && + a.union.types.every((aType, i) => typeReferenceEqual(aType, b.union.types[i])) + ); + } + if (spec.isIntersectionTypeReference(a) && spec.isIntersectionTypeReference(b)) { + return ( + a.intersection.types.length === b.intersection.types.length && + a.intersection.types.every((aType, i) => typeReferenceEqual(aType, b.intersection.types[i])) + ); + } + return false; +} diff --git a/src/type-visitor.ts b/src/type-visitor.ts new file mode 100644 index 00000000..32f824d3 --- /dev/null +++ b/src/type-visitor.ts @@ -0,0 +1,54 @@ +import * as spec from '@jsii/spec'; + +export interface TypeReferenceVisitor { + named(ref: spec.NamedTypeReference): A; + primitive(ref: spec.PrimitiveTypeReference): A; + collection(ref: spec.CollectionTypeReference): A; + union(ref: spec.UnionTypeReference): A; + intersection(ref: spec.IntersectionTypeReference): A; +} + +export function visitTypeReference(typeRef: spec.TypeReference, visitor: TypeReferenceVisitor) { + if (spec.isNamedTypeReference(typeRef)) { + return visitor.named(typeRef); + } else if (spec.isPrimitiveTypeReference(typeRef)) { + return visitor.primitive(typeRef); + } else if (spec.isCollectionTypeReference(typeRef)) { + return visitor.collection(typeRef); + } else if (spec.isUnionTypeReference(typeRef)) { + return visitor.union(typeRef); + } else if (spec.isIntersectionTypeReference(typeRef)) { + return visitor.intersection(typeRef); + } else { + throw new Error(`Unknown type reference: ${JSON.stringify(typeRef)}`); + } +} + +export interface TypeVisitor { + classType(t: spec.ClassType): A; + interfaceType(t: spec.InterfaceType): A; + dataType(t: spec.InterfaceType): A; + enumType(t: spec.EnumType): A; +} + +export function visitType(t: spec.Type, visitor: TypeVisitor) { + if (spec.isClassType(t)) { + return visitor.classType(t); + } else if (spec.isInterfaceType(t) && t.datatype) { + return visitor.dataType(t); + } else if (spec.isInterfaceType(t)) { + return visitor.interfaceType(t); + } else if (spec.isEnumType(t)) { + return visitor.enumType(t); + } else { + throw new Error(`Unknown type: ${JSON.stringify(t)}`); + } +} + +export function isDataType(t: spec.Type): t is spec.InterfaceType { + return spec.isInterfaceType(t) && !!t.datatype; +} + +export function isBehavioralInterfaceType(t: spec.Type): t is spec.InterfaceType { + return spec.isInterfaceType(t) && !t.datatype; +} diff --git a/test/abstract.test.ts b/test/abstract.test.ts index 93d5c88b..8f68f64c 100644 --- a/test/abstract.test.ts +++ b/test/abstract.test.ts @@ -1,15 +1,16 @@ import { PrimitiveType, Type, TypeKind } from '@jsii/spec'; import { sourceToAssemblyHelper } from '../lib'; +import { compileJsiiForErrors } from './compiler-helpers'; // ---------------------------------------------------------------------- test('Abstract member cant be marked async', () => { - expect(() => - sourceToAssemblyHelper(` + expect( + compileJsiiForErrors(` export abstract class AbstractClass { public abstract async abstractMethod(): Promise; } `), - ).toThrow(/There were compiler errors/); + ).toContainEqual(expect.stringMatching(/'async' modifier cannot be used with 'abstract' modifier/)); }); test('Abstract member can have a Promise return type', () => { diff --git a/test/compiler-helpers.ts b/test/compiler-helpers.ts new file mode 100644 index 00000000..dcfc2af2 --- /dev/null +++ b/test/compiler-helpers.ts @@ -0,0 +1,16 @@ +import { Diagnostic, DiagnosticCategory } from 'typescript'; +import { CompilerOptions } from '../src/compiler'; +import { compileJsiiForTest, TestCompilationOptions } from '../src/helpers'; + +export function compileJsiiForErrors>( + source: string | { 'index.ts': string; [name: string]: string }, + options?: O, + compilerOptions?: Omit, +): string[] { + const r = compileJsiiForTest(source, { ...options, captureDiagnostics: true }, compilerOptions); + return errors(r.diagnostics); +} + +export function errors(xs: readonly Diagnostic[]) { + return xs.filter((x) => x.category === DiagnosticCategory.Error).map((x) => `${x.messageText}`); +} diff --git a/test/compiler.test.ts b/test/compiler.test.ts index a92322db..1bab7e1f 100644 --- a/test/compiler.test.ts +++ b/test/compiler.test.ts @@ -116,7 +116,7 @@ describe(Compiler, () => { } finally { rmSync(sourceDir, { force: true, recursive: true }); } - }, 25_000); + }, 60_000); test('rootDir is added to assembly', () => { const outDir = 'jsii-outdir'; diff --git a/test/enums.test.ts b/test/enums.test.ts index 6c38a01f..b70074bf 100644 --- a/test/enums.test.ts +++ b/test/enums.test.ts @@ -1,6 +1,7 @@ import { EnumType } from '@jsii/spec'; import { sourceToAssemblyHelper } from '../lib'; +import { compileJsiiForErrors } from './compiler-helpers'; // ---------------------------------------------------------------------- test('test parsing enum with two members and no values', () => { @@ -56,8 +57,8 @@ test('enums can have a mix of letters and number', () => { }); test('enums with the same assigned value should fail', () => { - expect(() => - sourceToAssemblyHelper(` + expect( + compileJsiiForErrors(` export enum Foo { BAR = 'Bar', BAR_DUPE = 'Bar', @@ -65,5 +66,5 @@ test('enums with the same assigned value should fail', () => { BAZ_DUPE = 'Baz', } `), - ).toThrow(/There were compiler errors/); + ).toContainEqual(expect.stringMatching(/Value 'Bar' is used for multiple enum values: BAR, BAR_DUPE/)); }); diff --git a/test/property-inference.test.ts b/test/property-inference.test.ts index d2358bf7..49694c46 100644 --- a/test/property-inference.test.ts +++ b/test/property-inference.test.ts @@ -1,4 +1,5 @@ import { sourceToAssemblyHelper } from '../lib'; +import { compileJsiiForErrors } from './compiler-helpers'; // ---------------------------------------------------------------------- test('Class properties are inferred from the constructor', () => { @@ -58,21 +59,23 @@ test('Class properties are inferred from the constructor', () => { }); test('Class property can not be inferred if property is potentially undefinied', () => { - expect(() => - sourceToAssemblyHelper(` + expect( + compileJsiiForErrors( + ` class Square { sideLength; - + constructor(sideLength: number) { if (Math.random()) { this.sideLength = sideLength; } } - + get area() { return this.sideLength ** 2; } } - `), - ).toThrow(/There were compiler errors/); + `, + ), + ).toContainEqual("Object is possibly 'undefined'."); }); diff --git a/test/type-intersections.test.ts b/test/type-intersections.test.ts new file mode 100644 index 00000000..a773840d --- /dev/null +++ b/test/type-intersections.test.ts @@ -0,0 +1,306 @@ +import * as spec from '@jsii/spec'; + +import { sourceToAssemblyHelper } from '../lib'; +import { compileJsiiForErrors } from './compiler-helpers'; + +const IFOO_IBAR = ` + export interface IFoo { readonly foo: string; } + export interface IBar { readonly bar: string; } +`; + +// ---------------------------------------------------------------------- + +describe('positive tests', () => { + test.each([false, true])('intersection type as a function parameter (optional=%p)', (optional) => { + const q = optional ? '?' : ''; + + const assembly = sourceToAssemblyHelper(` + ${IFOO_IBAR} + export class Api { + public static fooAndBar(x${q}: IFoo & IBar) { + return (x?.foo ?? '') + (x?.bar ?? ''); + } + } + `); + + expect((assembly.types!['testpkg.Api'] as spec.ClassType).methods?.[0]).toMatchObject({ + name: 'fooAndBar', + parameters: [ + { + name: 'x', + ...(optional ? { optional: true } : undefined), + type: { + intersection: { + types: [{ fqn: 'testpkg.IFoo' }, { fqn: 'testpkg.IBar' }], + }, + }, + }, + ], + }); + }); + + test.each([false, true])('intersection type as a struct member (optional=%p)', (optional) => { + const q = optional ? '?' : ''; + + const assembly = sourceToAssemblyHelper(` + ${IFOO_IBAR} + export interface InputProps { + readonly x${q}: IFoo & IBar; + } + + export class Api { + public static fooAndBar(props: InputProps) { + return (props.x?.foo ?? '') + (props.x?.bar ?? ''); + } + } + `); + + expect((assembly.types!['testpkg.InputProps'] as spec.InterfaceType).properties?.[0]).toMatchObject({ + name: 'x', + ...(optional ? { optional: true } : undefined), + type: { + intersection: { + types: [{ fqn: 'testpkg.IFoo' }, { fqn: 'testpkg.IBar' }], + }, + }, + }); + }); + + test('intersection type as an array element type', () => { + const assembly = sourceToAssemblyHelper(` + ${IFOO_IBAR} + export class Api { + public static fooAndBar(xs: (IFoo & IBar)[]) { + return String(xs); + } + } + `); + + expect((assembly.types!['testpkg.Api'] as spec.ClassType).methods?.[0]).toMatchObject({ + name: 'fooAndBar', + parameters: [ + { + name: 'xs', + type: { + collection: { + kind: 'array', + elementtype: { + intersection: { + types: [{ fqn: 'testpkg.IFoo' }, { fqn: 'testpkg.IBar' }], + }, + }, + }, + }, + }, + ], + }); + }); + + test('intersection type as a map element type', () => { + const assembly = sourceToAssemblyHelper(` + ${IFOO_IBAR} + export class Api { + public static fooAndBar(xs: Record) { + return String(xs); + } + } + `); + + expect((assembly.types!['testpkg.Api'] as spec.ClassType).methods?.[0]).toMatchObject({ + name: 'fooAndBar', + parameters: [ + { + name: 'xs', + type: { + collection: { + kind: 'map', + elementtype: { + intersection: { + types: [{ fqn: 'testpkg.IFoo' }, { fqn: 'testpkg.IBar' }], + }, + }, + }, + }, + }, + ], + }); + }); + + test('the use of intersection types is reflected in a usedFeature', () => { + // WHEN + const assembly = sourceToAssemblyHelper(` + ${IFOO_IBAR} + export class Api { + public static fooAndBar(x: IFoo & IBar) { + return (x?.foo ?? '') + (x?.bar ?? ''); + } + } + `); + + // THEN + expect(assembly.usedFeatures).toContain('intersection-types'); + }); +}); + +describe('intersection type may not be used in output position', () => { + function expectIntersectionTypeError(code: string) { + const errs = compileJsiiForErrors(code); + expect(errs).toContainEqual(expect.stringContaining('Intersection types may only be used as inputs')); + } + + test('direct return', () => { + expectIntersectionTypeError(` + ${IFOO_IBAR} + export class Api { + public static fooAndBar(): IFoo & IBar { + return { foo: 'foo', bar: 'bar' }; + } + } + `); + }); + + test('part of return struct', () => { + expectIntersectionTypeError(` + ${IFOO_IBAR} + export interface OutputProps { + readonly x: IFoo & IBar; + } + + export class Api { + public static fooAndBar(): OutputProps { + return { x: { foo: 'foo', bar: 'bar' } }; + } + } + `); + }); + + test('part of both input and output struct', () => { + expectIntersectionTypeError(` + ${IFOO_IBAR} + export interface OutputProps { + readonly x: IFoo & IBar; + } + + export class Api { + public static fooAndBar(props: OutputProps): OutputProps { + return props; + } + } + `); + }); + + test('transitively part of return struct', () => { + expectIntersectionTypeError(` + ${IFOO_IBAR} + export interface NestedObject { readonly x: IFoo & IBar } + export interface OutputProps { readonly nested: NestedObject } + export class Api { + public static fooAndBar(): OutputProps { + return { nested: { x: { foo: 'foo', bar: 'bar' } } }; + } + } + `); + }); + + test.each([ + ['', ''], + ['readonly', ''], + ['', '?'], + ['readonly', '?'], + ])('readable member of interface %p %p', (ro, q) => { + expectIntersectionTypeError(` + ${IFOO_IBAR} + export interface IObject { + ${ro} member${q}: IFoo & IBar; + } + `); + }); + + test.each([ + ['', ''], + ['readonly', ''], + ['', '?'], + ['readonly', '?'], + ])('readable member of class %p %p', (ro, q) => { + expectIntersectionTypeError(` + ${IFOO_IBAR} + export class Obj { + ${ro} member${q}: IFoo & IBar; + + constructor() { + this.member = { foo: 'foo', bar: 'bar' }; + } + } + `); + }); +}); + +test.each([ + ['IFoo', 'string', 'Intersection types must combine exclusively interfaces'], + ['boolean', 'string', '"never" type is not allowed'], + ['IFoo[]', 'IBar', 'Intersection types must combine exclusively interfaces'], + ['IBar', 'SomeClass', 'Intersection types must combine exclusively interfaces'], +])('intersection type may not combine %s and %s', (lhs, rhs, errorMessage) => { + const errs = compileJsiiForErrors( + ` + ${IFOO_IBAR} + export class SomeClass { } + Array.isArray(SomeClass); + + export class Api { + public static fooAndBar(x: ${lhs} & ${rhs}) { + return String(x); + } + } + `, + ); + + expect(errs).toContainEqual(expect.stringContaining(errorMessage)); +}); + +test('intersection types may not be used as constructor args', () => { + const errs = compileJsiiForErrors( + ` + ${IFOO_IBAR} + export class Api { + public constructor(x: IFoo & IBar) { + Array.isArray(x); + } + } + `, + ); + + expect(errs).toContainEqual(expect.stringContaining('Intersection types cannot be used as constructor arguments')); +}); + +test.each([ + ['readonly bar: string', 'readonly bar: string', true as const], + ['readonly bar: string', 'bar: string', true as const], // read-only & read-write is okay + ['readonly bar?: string', 'readonly bar: string', 'Member bar is different'], + ['bar(): string', 'bar(): string', true as const], + ['bar(): string', 'bar(): string | undefined', 'Member bar is different'], +])('intersection of %p and %p => %p', (left, right, error: string | true) => { + const errs = compileJsiiForErrors( + ` + export interface IFoo { + ${left}; + } + + export interface IBar { + ${right}; + } + + export class Api { + public use(x: IFoo & IBar) { + Array.isArray(x); + } + } + `, + ); + + if (error === true) { + expect(errs).toEqual([]); + } else { + expect(errs).toContainEqual(expect.stringContaining(error)); + } +}); diff --git a/yarn.lock b/yarn.lock index 26fac9ae..2c499ce1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -734,13 +734,20 @@ chalk "^4.1.2" semver "^7.7.2" -"@jsii/spec@^1.105.0", "@jsii/spec@^1.113.0": +"@jsii/spec@^1.105.0": version "1.113.0" resolved "https://registry.yarnpkg.com/@jsii/spec/-/spec-1.113.0.tgz#cc9960a69c285466088c370f8e47d07c5ae73fb4" integrity sha512-OAQNfJHzMmE42ySJpelOFFKCgnh6hxcKLnmgtaYEzsFW9UxH9gc945FFOXff52GbhUcigGElCqJ3MclrbgXoGw== dependencies: ajv "^8.17.1" +"@jsii/spec@^1.114.1": + version "1.114.1" + resolved "https://registry.yarnpkg.com/@jsii/spec/-/spec-1.114.1.tgz#9c064d57f062d913bcfda25b5496bdf4c9c95c46" + integrity sha512-SdjVQaNqLkTUK+2R0/t/MnM/NBvv1vzqxO5sn1nnoFD5Wlih8TFOIjl+Q8npzYmOtN+et3D+BMVYrxmVfq4X0w== + dependencies: + ajv "^8.17.1" + "@napi-rs/wasm-runtime@^0.2.11": version "0.2.12" resolved "https://registry.yarnpkg.com/@napi-rs/wasm-runtime/-/wasm-runtime-0.2.12.tgz#3e78a8b96e6c33a6c517e1894efbd5385a7cb6f2" From 84ad60f5ae9cca4ef519643cec70b5c05bd973d4 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 11 Sep 2025 16:37:59 +0200 Subject: [PATCH 2/3] Review comments, recursion breaker --- src/assembler.ts | 86 +++++++++++++++++------------------------- src/jsii-diagnostic.ts | 5 +-- src/sets.ts | 39 +++++++++++++++++++ 3 files changed, 76 insertions(+), 54 deletions(-) create mode 100644 src/sets.ts diff --git a/src/assembler.ts b/src/assembler.ts index 25d10f15..a777f86d 100644 --- a/src/assembler.ts +++ b/src/assembler.ts @@ -19,6 +19,7 @@ import * as literate from './literate'; import * as bindings from './node-bindings'; import { ProjectInfo } from './project-info'; import { isReservedName } from './reserved-words'; +import { Sets } from './sets'; import { DeprecatedRemover } from './transforms/deprecated-remover'; import { DeprecationWarningsInjector } from './transforms/deprecation-warnings'; import { RuntimeTypeInfoInjector } from './transforms/runtime-info'; @@ -1396,7 +1397,7 @@ export class Assembler implements Emitter { .map((x) => x.name), ); // Intersect - for (const member of intersect(statics, nonStatics)) { + for (const member of Sets.intersect(statics, nonStatics)) { this._diagnostics.push(JsiiDiagnostic.JSII_5013_STATIC_INSTANCE_CONFLICT.create(decl, member, klass)); } @@ -1954,7 +1955,7 @@ export class Assembler implements Emitter { // Liftable datatype, make sure no parameter names match any of the properties in the datatype const propNames = this.allProperties(lastParamType); const paramNames = new Set(parameters.slice(0, parameters.length - 1).map((x) => x.name)); - const sharedNames = intersection(propNames, paramNames); + const sharedNames = Sets.intersection(propNames, paramNames); for (const badName of sharedNames) { this._diagnostics.push(JsiiDiagnostic.JSII_5017_POSITIONAL_KEYWORD_CONFLICT.create(declaration, badName)); @@ -2355,19 +2356,30 @@ export class Assembler implements Emitter { private validateIntersectionType(intersectionType: spec.IntersectionTypeReference, declaration: ts.Node | undefined) { // Validate that this intersection type only consists of interface types this._deferUntilTypesAvailable('', intersectionType.intersection.types, declaration, (...elementTypes) => { - if ( - // If we didn't resolve some things, they're probably primitives - elementTypes.length !== intersectionType.intersection.types.length || - // Or if there's something explicitly a primitive - elementTypes.some((t) => !isBehavioralInterfaceType(t)) - ) { - this._diagnostics.push(JsiiDiagnostic.JSII_1008_ONLY_INTERFACE_INTERSECTION.create(declaration)); + const requestedFqns = new Set(intersectionType.intersection.types.map(typeReferenceToString)); + const resolvedFqns = new Set(elementTypes.map((t) => t.fqn)); + + const unresolved = Sets.diff(requestedFqns, resolvedFqns); + if (unresolved.size > 0) { + this._diagnostics.push( + JsiiDiagnostic.JSII_1008_ONLY_INTERFACE_INTERSECTION.create(declaration, Array.from(unresolved).join(', ')), + ); + } + + const nonBehavioral = elementTypes.filter((t) => !isBehavioralInterfaceType(t)); + if (nonBehavioral.length > 0) { + this._diagnostics.push( + JsiiDiagnostic.JSII_1008_ONLY_INTERFACE_INTERSECTION.create( + declaration, + nonBehavioral.map((t) => t.fqn).join(', '), + ), + ); } // Check that shared members are the same const interfaces = elementTypes.filter(isBehavioralInterfaceType); - const allProps = union(...interfaces.map((int) => this.allProperties(int))); - const allMethods = union(...interfaces.map((int) => this.allMethods(int))); + const allProps = Sets.union(...interfaces.map((int) => this.allProperties(int))); + const allMethods = Sets.union(...interfaces.map((int) => this.allMethods(int))); for (let i = 1; i < interfaces.length; i++) { let int0 = interfaces[0]; @@ -2633,6 +2645,8 @@ export class Assembler implements Emitter { * not output position */ private validateTypesAgainstPositions() { + const self = this; + const validatedFqns = { in: new Set(), out: new Set(), @@ -2665,43 +2679,44 @@ export class Assembler implements Emitter { } } - function validateRefFor(this: Assembler, dir: 'in' | 'out', typeRef: spec.TypeReference, reason: string) { + function validateRefFor(dir: 'in' | 'out', typeRef: spec.TypeReference, reason: string) { visitTypeReference(typeRef, { named: (ref) => { // Named types we'll only validate once for every direction if (validatedFqns[dir].has(ref.fqn)) { return; } + validatedFqns[dir].add(ref.fqn); - const type = this._dereference(ref, undefined, 'just-validating'); + const type = self._dereference(ref, undefined, 'just-validating'); if (!type) { // Maybe this is an unexported type. return; } - validateTypeFor.call(this, dir, type, reason); + validateTypeFor(dir, type, reason); }, primitive: () => {}, collection: (ref) => { - validateRefFor.call(this, dir, ref.collection.elementtype, reason); + validateRefFor(dir, ref.collection.elementtype, reason); }, union: (ref) => { for (const t of ref.union.types) { - validateRefFor.call(this, dir, t, reason); + validateRefFor(dir, t, reason); } }, intersection: (ref) => { if (dir === 'out') { - this._diagnostics.push(JsiiDiagnostic.JSII_1009_INTERSECTION_ONLY_INPUT.createDetached(reason)); + self._diagnostics.push(JsiiDiagnostic.JSII_1009_INTERSECTION_ONLY_INPUT.createDetached(reason)); } for (const t of ref.intersection.types) { - validateRefFor.call(this, dir, t, reason); + validateRefFor(dir, t, reason); } }, }); } - function validateTypeFor(this: Assembler, dir: 'in' | 'out', type: spec.Type, reason: string) { + function validateTypeFor(dir: 'in' | 'out', type: spec.Type, reason: string) { visitType(type, { dataType: (t) => { // We only need to validate data types, because classes and interfaces @@ -2709,7 +2724,7 @@ export class Assembler implements Emitter { // // Recurse. for (const prop of t.properties ?? []) { - validateRefFor.call(this, dir, prop.type, `type of property ${t.name}.${prop.name}, ${reason}`); + validateRefFor(dir, prop.type, `type of property ${t.name}.${prop.name}, ${reason}`); } }, classType: () => {}, @@ -2977,29 +2992,6 @@ function apply(x: T | undefined, fn: (x: T) => U | undefined): U | undefin return x !== undefined ? fn(x) : undefined; } -/** - * Return the intersection of N sets - */ -function intersection(...xss: Array>): Set { - if (xss.length === 0) { - return new Set(); - } - const ret = new Set(xss[0]); - for (const x of xss[0]) { - if (!xss.every((xs) => xs.has(x))) { - ret.delete(x); - } - } - return ret; -} - -/** - * Return the union of N sets - */ -function union(...xss: Array>): Set { - return new Set(xss.flatMap((xs) => Array.from(xs))); -} - /** * Return all members names of a JSII interface type * @@ -3039,14 +3031,6 @@ function getConstructor(type: ts.Type): ts.Symbol | undefined { return type.symbol.members?.get(ts.InternalSymbolName.Constructor); } -function* intersect(xs: Set, ys: Set) { - for (const x of xs) { - if (ys.has(x)) { - yield x; - } - } -} - function cmpDesc(a: T, b: T, desc: (x: T) => string): [string, string] | undefined { const desc1 = desc(a); const desc2 = desc(b); diff --git a/src/jsii-diagnostic.ts b/src/jsii-diagnostic.ts index 2ec840f8..5f02b195 100644 --- a/src/jsii-diagnostic.ts +++ b/src/jsii-diagnostic.ts @@ -302,14 +302,13 @@ export class JsiiDiagnostic implements ts.Diagnostic { public static readonly JSII_1008_ONLY_INTERFACE_INTERSECTION = Code.error({ code: 1008, - formatter: () => 'Intersection types must combine exclusively interfaces (IOne & ITwo)', + formatter: (type: string) => `Found non-interface type in type intersection: ${type}`, name: 'typescript-restriction/only-interface-intersection', }); public static readonly JSII_1009_INTERSECTION_ONLY_INPUT = Code.error({ code: 1009, - formatter: (location: string) => - `Intersection types may only be used as inputs, but ${location} is an intersection type used as output`, + formatter: (location: string) => `Intersection types may only be used as inputs, but ${location} is used as output`, name: 'typescript-restriction/intersection-only-input', }); diff --git a/src/sets.ts b/src/sets.ts new file mode 100644 index 00000000..9377acb0 --- /dev/null +++ b/src/sets.ts @@ -0,0 +1,39 @@ +export abstract class Sets { + /** + * Return the intersection of N sets + */ + public static intersection(...xss: Array>): Set { + if (xss.length === 0) { + return new Set(); + } + const ret = new Set(xss[0]); + for (const x of xss[0]) { + if (!xss.every((xs) => xs.has(x))) { + ret.delete(x); + } + } + return ret; + } + + /** + * Return the union of N sets + */ + public static union(...xss: Array>): Set { + return new Set(xss.flatMap((xs) => Array.from(xs))); + } + + /** + * Return the diff of 2 sets + */ + public static diff(xs: Set, ys: Set) { + return new Set(Array.from(xs).filter((x) => !ys.has(x))); + } + + public static *intersect(xs: Set, ys: Set) { + for (const x of xs) { + if (ys.has(x)) { + yield x; + } + } + } +} From cd03e1eecd824266ef2956c79043d539480e7d9f Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 11 Sep 2025 16:47:30 +0200 Subject: [PATCH 3/3] I changed the error messages eh --- test/compiler.test.ts | 2 +- test/type-intersections.test.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/compiler.test.ts b/test/compiler.test.ts index 1bab7e1f..a92322db 100644 --- a/test/compiler.test.ts +++ b/test/compiler.test.ts @@ -116,7 +116,7 @@ describe(Compiler, () => { } finally { rmSync(sourceDir, { force: true, recursive: true }); } - }, 60_000); + }, 25_000); test('rootDir is added to assembly', () => { const outDir = 'jsii-outdir'; diff --git a/test/type-intersections.test.ts b/test/type-intersections.test.ts index a773840d..942d1f4e 100644 --- a/test/type-intersections.test.ts +++ b/test/type-intersections.test.ts @@ -236,10 +236,10 @@ describe('intersection type may not be used in output position', () => { }); test.each([ - ['IFoo', 'string', 'Intersection types must combine exclusively interfaces'], + ['IFoo', 'string', 'Found non-interface type in type intersection: string'], ['boolean', 'string', '"never" type is not allowed'], - ['IFoo[]', 'IBar', 'Intersection types must combine exclusively interfaces'], - ['IBar', 'SomeClass', 'Intersection types must combine exclusively interfaces'], + ['IFoo[]', 'IBar', 'Found non-interface type in type intersection: array'], + ['IBar', 'SomeClass', 'Found non-interface type in type intersection: testpkg.SomeClass'], ])('intersection type may not combine %s and %s', (lhs, rhs, errorMessage) => { const errs = compileJsiiForErrors( `