Skip to content

Commit

Permalink
Fix(jsii): improve diagnostics when dereference fails (#222)
Browse files Browse the repository at this point in the history
In some cases, when a type could not be referenced, the diagnostic 
message doesn't indicate that (the undefined return value will still 
cause an error, but we also want to know that we couldn't deref the 
type).

Fixes #221
  • Loading branch information
Elad Ben-Israel authored Sep 13, 2018
1 parent e80a889 commit 72927bd
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 28 deletions.
59 changes: 31 additions & 28 deletions packages/jsii/lib/assembler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,21 @@ export class Assembler implements Emitter {
*
* @returns the de-referenced type, if it was found, otherwise ``undefined``.
*/
private _dereference(ref: spec.NamedTypeReference): spec.Type | undefined {
private _dereference(ref: spec.NamedTypeReference, referencingNode: ts.Node): spec.Type | undefined {
const [assm, ] = ref.fqn.split('.');
let type;
if (assm === this.projectInfo.name) {
return this._types[ref.fqn];
type = this._types[ref.fqn];
} else {
const assembly = this.projectInfo.transitiveDependencies.find(dep => dep.name === assm);
return assembly && assembly.types && assembly.types[ref.fqn];
type = assembly && assembly.types && assembly.types[ref.fqn];
}

if (!type) {
this._diagnostic(referencingNode, ts.DiagnosticCategory.Error, `Unable to resolve referenced type '${ref.fqn}'. Missing export?`);
}

return type;
}

private _diagnostic(node: ts.Node | null, category: ts.DiagnosticCategory, messageText: string) {
Expand Down Expand Up @@ -206,7 +213,7 @@ export class Assembler implements Emitter {
return `unknown.${typeName}`;
}
const fqn = `${pkg.name}.${typeName}`;
if (pkg.name !== this.projectInfo.name && !this._dereference({ fqn })) {
if (pkg.name !== this.projectInfo.name && !this._dereference({ fqn }, type.symbol.valueDeclaration)) {
this._diagnostic(type.symbol.valueDeclaration,
ts.DiagnosticCategory.Error,
`Use of foreign type not present in the ${pkg.name}'s assembly: ${fqn}`);
Expand Down Expand Up @@ -314,7 +321,8 @@ export class Assembler implements Emitter {
continue;
}
this._defer(() => {
if (!spec.isClassType(this._dereference(ref))) {
const deref = this._dereference(ref, base.symbol.valueDeclaration);
if (deref && !spec.isClassType(deref)) {
this._diagnostic(base.symbol.valueDeclaration,
ts.DiagnosticCategory.Error,
`Base type of ${jsiiType.fqn} is not a class (${spec.describeTypeReference(ref)})`);
Expand All @@ -340,7 +348,8 @@ export class Assembler implements Emitter {
continue;
}
this._defer(() => {
if (!spec.isInterfaceType(this._dereference(typeRef))) {
const deref = this._dereference(typeRef, expression);
if (deref && !spec.isInterfaceType(deref)) {
this._diagnostic(expression,
ts.DiagnosticCategory.Error,
`Implements clause of ${jsiiType.fqn} uses ${spec.describeTypeReference(typeRef)} as an interface`);
Expand Down Expand Up @@ -393,17 +402,15 @@ export class Assembler implements Emitter {
}
} else if (jsiiType.base) {
this._defer(() => {
const baseType = this._dereference(jsiiType.base!);
if (!baseType) {
this._diagnostic(type.symbol.valueDeclaration,
ts.DiagnosticCategory.Error,
`Unable to resolve type ${jsiiType.base!.fqn} (base type of ${jsiiType.fqn})`);
} else if (spec.isClassType(baseType)) {
jsiiType.initializer = baseType.initializer;
} else {
this._diagnostic(type.symbol.valueDeclaration,
ts.DiagnosticCategory.Error,
`Base type of ${jsiiType.fqn} (${jsiiType.base!.fqn}) is not a class`);
const baseType = this._dereference(jsiiType.base!, type.symbol.valueDeclaration);
if (baseType) {
if (spec.isClassType(baseType)) {
jsiiType.initializer = baseType.initializer;
} else {
this._diagnostic(type.symbol.valueDeclaration,
ts.DiagnosticCategory.Error,
`Base type of ${jsiiType.fqn} (${jsiiType.base!.fqn}) is not a class`);
}
}
});
} else {
Expand Down Expand Up @@ -499,17 +506,13 @@ export class Assembler implements Emitter {
continue;
}
this._defer(() => {
if (!spec.isInterfaceType(this._dereference(ref))) {
const baseType = this._dereference(ref);
if (baseType) {
this._diagnostic(base.symbol.valueDeclaration,
ts.DiagnosticCategory.Error,
`Base type of ${jsiiType.fqn} is not an interface (${baseType.kind} ${spec.describeTypeReference(ref)})`);
} else {
this._diagnostic(base.symbol.valueDeclaration,
ts.DiagnosticCategory.Error,
`Base type of ${jsiiType.fqn} could not be resolved (${spec.describeTypeReference(ref)})`);
}
const baseType = this._dereference(ref, base.symbol.valueDeclaration);
if (baseType && !spec.isInterfaceType(baseType)) {
// tslint:disable:max-line-length
this._diagnostic(base.symbol.valueDeclaration,
ts.DiagnosticCategory.Error,
`Base type of ${jsiiType.fqn} is not an interface (${baseType.kind} ${spec.describeTypeReference(ref)})`);
// tslint:enable:max-line-length
}
});
if (jsiiType.interfaces) {
Expand Down
9 changes: 9 additions & 0 deletions packages/jsii/test/negatives/neg.deref-error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
///!MATCH_ERROR: Unable to resolve referenced type 'Base'. Missing export?

class Base {

}

export class Derived extends Base {

}

0 comments on commit 72927bd

Please sign in to comment.