use snapshot-based testing for jsii negatives

Makes it easier to review the actual, complete, user experience (including error locality).
aws · Aug 7, 2020 · 55240a8 · 55240a8
1 parent 17a551e
commit 55240a8
Show file tree

Hide file tree

Showing 72 changed files with 778 additions and 308 deletions.
diff --git a/packages/jsii/lib/assembler.ts b/packages/jsii/lib/assembler.ts
@@ -619,7 +619,7 @@ export class Assembler implements Emitter {
         symbol.name !== Case.snake(symbol.name)
       ) {
         this._diagnostic(
-          declaration,
+          declaration.name,
           ts.DiagnosticCategory.Error,
           `Submodule namespaces must be camelCased or snake_cased. Consider renaming to "${Case.camel(
             symbol.name,
@@ -704,25 +704,38 @@ export class Assembler implements Emitter {
         if (currNs.name !== ns.name) {
           const currNsDecl = currNs.valueDeclaration ?? currNs.declarations[0];
           const nsDecl = ns.valueDeclaration ?? ns.declarations[0];
+
+          const refs = [
+            [currNs.name, currNsDecl] as const,
+            [ns.name, nsDecl] as const,
+          ].sort(([l], [r]) => l.localeCompare(r));
+
           this._diagnostic(
-            symbol.valueDeclaration,
+            (symbol.valueDeclaration as { name?: ts.Node }).name ??
+              symbol.valueDeclaration,
             ts.DiagnosticCategory.Error,
-            `Symbol is re-exported under two distinct submodules (${currNs.name} and ${ns.name})`,
+            `Symbol is re-exported under two distinct submodules (${refs
+              .map(([name]) => name)
+              .join(' and ')})`,
             [
               {
                 category: ts.DiagnosticCategory.Warning,
-                file: currNsDecl.getSourceFile(),
-                length: currNsDecl.getStart() - currNsDecl.getEnd(),
-                messageText: `Symbol is exported under the "${currNs.name}" submodule`,
-                start: currNsDecl.getStart(),
+                file: refs[0][1].getSourceFile(),
+                length:
+                  refs[0][1].getEnd() -
+                  refs[0][1].getStart(refs[0][1].getSourceFile()),
+                messageText: `Symbol is exported under the "${refs[0][0]}" submodule`,
+                start: refs[0][1].getStart(refs[0][1].getSourceFile()),
                 code: JSII_DIAGNOSTICS_CODE,
               },
               {
                 category: ts.DiagnosticCategory.Warning,
-                file: nsDecl.getSourceFile(),
-                length: nsDecl.getStart() - nsDecl.getEnd(),
-                messageText: `Symbol is exported under the "${ns.name}" submodule`,
-                start: nsDecl.getStart(),
+                file: refs[1][1].getSourceFile(),
+                length:
+                  refs[1][1].getEnd() -
+                  refs[1][1].getStart(refs[1][1].getSourceFile()),
+                messageText: `Symbol is exported under the "${refs[1][0]}" submodule`,
+                start: refs[1][1].getStart(refs[1][1].getSourceFile()),
                 code: JSII_DIAGNOSTICS_CODE,
               },
             ],
@@ -920,8 +933,10 @@ export class Assembler implements Emitter {
       if (colliding != null) {
         const submoduleDecl =
           submodule.valueDeclaration ?? submodule.declarations[0];
+        const submoduleDeclName =
+          (submoduleDecl as { name?: ts.Node }).name ?? submoduleDecl;
         this._diagnostic(
-          node,
+          (node as { name?: ts.Node }).name ?? node,
           ts.DiagnosticCategory.Error,
           `Submodule "${submodule.name}" conflicts with "${
             jsiiType.name
@@ -930,10 +945,10 @@ export class Assembler implements Emitter {
             {
               category: ts.DiagnosticCategory.Warning,
               code: JSII_DIAGNOSTICS_CODE,
-              file: submoduleDecl.getSourceFile(),
-              length: submoduleDecl.getEnd() - submoduleDecl.getStart(),
+              file: submoduleDeclName.getSourceFile(),
+              length: submoduleDeclName.getEnd() - submoduleDeclName.getStart(),
               messageText: 'This is the conflicting submodule declaration.',
-              start: submoduleDecl.getStart(),
+              start: submoduleDeclName.getStart(),
             },
           ],
         );
@@ -1290,7 +1305,7 @@ export class Assembler implements Emitter {
           continue;
         }
 
-        if (this._isPrivateOrInternal(member, memberDecl)) {
+        if (this._isPrivateOrInternal(member, memberDecl as ts.ClassElement)) {
           continue;
         }
 
@@ -1526,7 +1541,7 @@ export class Assembler implements Emitter {
    */
   private _isPrivateOrInternal(
     symbol: ts.Symbol,
-    validateDeclaration?: ts.Declaration,
+    validateDeclaration?: ts.Declaration & { name?: ts.Node },
   ): boolean {
     const hasInternalJsDocTag = _hasInternalJsDocTag(symbol);
     const hasUnderscorePrefix =
@@ -1549,7 +1564,7 @@ export class Assembler implements Emitter {
     if (validateDeclaration) {
       if (!hasUnderscorePrefix) {
         this._diagnostic(
-          validateDeclaration,
+          validateDeclaration.name ?? validateDeclaration,
           ts.DiagnosticCategory.Error,
           `${colors.cyan(
             symbol.name,
@@ -1559,7 +1574,7 @@ export class Assembler implements Emitter {
 
       if (!hasInternalJsDocTag) {
         this._diagnostic(
-          validateDeclaration,
+          validateDeclaration.name ?? validateDeclaration,
           ts.DiagnosticCategory.Error,
           `${colors.cyan(
             symbol.name,
@@ -1723,7 +1738,12 @@ export class Assembler implements Emitter {
           continue;
         }
 
-        if (this._isPrivateOrInternal(member, member.valueDeclaration)) {
+        if (
+          this._isPrivateOrInternal(
+            member,
+            member.valueDeclaration as ts.PropertyDeclaration,
+          )
+        ) {
           continue;
         }
 
@@ -1782,7 +1802,9 @@ export class Assembler implements Emitter {
         // If it's not a datatype the name must start with an "I".
         if (!jsiiType.datatype && !interfaceName) {
           this._diagnostic(
-            type.symbol.declarations[0],
+            (type.symbol.valueDeclaration as ts.InterfaceDeclaration).name ??
+              type.symbol.valueDeclaration ??
+              type.symbol.declarations[0],
             ts.DiagnosticCategory.Error,
             `Interface contains behavior: name should be "I${jsiiType.name}"`,
           );