diff --git a/packages/jsii/lib/assembler.ts b/packages/jsii/lib/assembler.ts index 4a88094027..d46dc342ac 100644 --- a/packages/jsii/lib/assembler.ts +++ b/packages/jsii/lib/assembler.ts @@ -190,7 +190,7 @@ export class Assembler implements Emitter { * * @returns the de-referenced type, if it was found, otherwise ``undefined``. */ - private _dereference(ref: spec.NamedTypeReference, referencingNode: ts.Node): spec.Type | undefined { + private _dereference(ref: spec.NamedTypeReference, referencingNode: ts.Node | null): spec.Type | undefined { const [assm, ] = ref.fqn.split('.'); let type; if (assm === this.projectInfo.name) { @@ -615,11 +615,13 @@ export class Assembler implements Emitter { this._diagnostic(declaration, ts.DiagnosticCategory.Error, `Unable to compute signature for ${type.fqn}#${symbol.name}`); return; } + const parameters = await Promise.all(signature.getParameters().map(p => this._toParameter(p))); + const returnType = signature.getReturnType(); const method: spec.Method = { abstract: _isAbstract(symbol, type), name: symbol.name, - parameters: await Promise.all(signature.getParameters().map(p => this._toParameter(p))), + parameters, protected: _isProtected(symbol), returns: _isVoid(returnType) ? undefined : await this._typeReference(returnType, declaration), static: _isStatic(symbol), @@ -628,6 +630,29 @@ export class Assembler implements Emitter { this._visitDocumentation(symbol, method); + // If the last parameter is a datatype, verify that it does not share any field names with + // other function arguments, so that it can be turned into keyword arguments by jsii frontends + // that support such. + const lastParamTypeRef = apply(last(parameters), x => x.type); + const lastParamSymbol = last(signature.getParameters()); + if (lastParamTypeRef && spec.isNamedTypeReference(lastParamTypeRef)) { + this._deferUntilTypesAvailable(symbol.name, [lastParamTypeRef], lastParamSymbol!.declarations[0], (lastParamType) => { + if (!spec.isInterfaceType(lastParamType) || !lastParamType.datatype) { return; } + + // 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); + + if (sharedNames.size > 0) { + this._diagnostic( + declaration, + ts.DiagnosticCategory.Error, + `Name occurs in both function arguments and in datatype properties, rename one: ${Array.from(sharedNames).join(', ')}`); + } + }); + } + type.methods = type.methods || []; type.methods.push(method); } @@ -867,6 +892,31 @@ export class Assembler implements Emitter { def.dependedFqns = def.dependedFqns.filter(fqns.has.bind(fqns)); } } + + /** + * Return the set of all (inherited) properties of an interface + */ + private allProperties(root: spec.InterfaceType): Set { + const self = this; + + const ret = new Set(); + recurse(root); + return ret; + + function recurse(int: spec.InterfaceType) { + for (const property of int.properties || []) { + ret.add(property.name); + } + + for (const baseRef of int.interfaces || []) { + const base = self._dereference(baseRef, null); + if (!base) { throw new Error('Impossible to have unresolvable base in allProperties()'); } + if (!spec.isInterfaceType(base)) { throw new Error('Impossible to have non-interface base in allProperties()'); } + + recurse(base); + } + } + } } /** @@ -1024,6 +1074,33 @@ interface DeferredRecord { cb: () => void; } +/** + * Return the last element from a list + */ +function last(xs: T[]): T | undefined { + return xs.length > 0 ? xs[xs.length - 1] : undefined; +} + +/** + * Apply a function to a value if it's not equal to undefined + */ +function apply(x: T | undefined, fn: (x: T) => U | undefined): U | undefined { + return x !== undefined ? fn(x) : undefined; +} + +/** + * Return the intersection of two sets + */ +function intersection(xs: Set, ys: Set): Set { + const ret = new Set(); + for (const x of xs) { + if (ys.has(x)) { + ret.add(x); + } + } + return ret; +} + /** * Whether or not the given name is conventionally an interface name * @@ -1032,4 +1109,4 @@ interface DeferredRecord { */ function isInterfaceName(name: string) { return name.length >= 2 && name.charAt(0) === 'I' && name.charAt(1).toUpperCase() === name.charAt(1); -} \ No newline at end of file +} diff --git a/packages/jsii/test/negatives/neg.mix-datatype-and-arg-name.ts b/packages/jsii/test/negatives/neg.mix-datatype-and-arg-name.ts new file mode 100644 index 0000000000..243ebd5a69 --- /dev/null +++ b/packages/jsii/test/negatives/neg.mix-datatype-and-arg-name.ts @@ -0,0 +1,15 @@ +///!MATCH_ERROR: Name occurs in both function arguments and in datatype properties, rename one: dontWorry + +export interface Lyrics { + dontWorry: string; + beHappy: string; +} + +export class MyClass { + // Can't have an argument name 'dontWorry' if the last parameter is a datatype + // which also has a field 'dontWorry'--that will give errors if the datatype + // argument fields are lifted to keyword arguments. + public dance(dontWorry: string, lyrics: Lyrics) { + return `${dontWorry}: ${lyrics.beHappy}`; + } +} \ No newline at end of file