Address review feedback

Consensys · Sep 29, 2021 · 6e61211 · 6e61211
1 parent 9fa2191
commit 6e61211
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 27 deletions.
diff --git a/src/misc/pretty_printing.ts b/src/misc/pretty_printing.ts
@@ -4,15 +4,27 @@ export interface PPAble {
     pp(): string;
 }
 
-export type PPIsh = PPAble | ASTNode | string | number | boolean | bigint | null | undefined;
+export type PPIsh =
+    | PPAble
+    | ASTNode
+    | string
+    | number
+    | boolean
+    | bigint
+    | null
+    | undefined
+    | PPIsh[]
+    | Set<PPIsh>
+    | Map<PPIsh, PPIsh>
+    | Iterable<PPIsh>;
 
 export function isPPAble(value: any): value is PPAble {
     return value ? typeof value.pp === "function" : false;
 }
 
 export function pp(value: PPIsh): string {
     if (value instanceof ASTNode) {
-        return value.type + "#" + value.id;
+        return value.type + " #" + value.id;
     }
 
     if (value === undefined) {
@@ -33,6 +45,22 @@ export function pp(value: PPIsh): string {
         return value.pp();
     }
 
+    if (value instanceof Array) {
+        return ppArr(value);
+    }
+
+    if (value instanceof Set) {
+        return ppSet(value);
+    }
+
+    if (value instanceof Map) {
+        return ppMap(value);
+    }
+
+    if (typeof value[Symbol.iterator] === "function") {
+        return ppIter(value);
+    }
+
     throw new Error("Unhandled value in pp(): " + String(value));
 }
 
@@ -55,7 +83,7 @@ export function ppSet(set: Set<PPIsh>, separator = ",", start = "{", end = "}"):
 }
 
 export function ppMap(
-    map: Map<PPIsh, PPAble>,
+    map: Map<PPIsh, PPIsh>,
     separator = ",",
     keyValueSeparator = ":",
     start = "{",

diff --git a/src/misc/utils.ts b/src/misc/utils.ts
@@ -1,4 +1,4 @@
-import { pp, ppArr, PPIsh } from "..";
+import { pp, PPIsh } from "..";
 import { ASTNode } from "../ast/ast_node";
 
 export function forAll<T>(iterable: Iterable<T>, cb: (v: T) => boolean): boolean {
@@ -25,17 +25,10 @@ export function assert(
 
         for (let i = 0; i < details.length; i++) {
             const detail = details[i];
-
-            let part: string;
+            const part = pp(detail);
 
             if (detail instanceof ASTNode) {
-                part = pp(detail);
-
                 nodes.push(detail);
-            } else if (detail instanceof Array) {
-                part = ppArr(detail);
-            } else {
-                part = pp(detail);
             }
 
             message = message.replace(new RegExp("\\{" + i + "\\}", "g"), part);

diff --git a/test/unit/misc/pretty_printing.spec.ts b/test/unit/misc/pretty_printing.spec.ts
@@ -40,14 +40,30 @@ describe("Utility formatting routines", () => {
             ],
             [
                 new ElementaryTypeName(1, "0:0:0", "ElementaryTypeName", "uint8", "uint8"),
-                "ElementaryTypeName#1"
+                "ElementaryTypeName #1"
+            ],
+            [["x", 1, true, null], "[x,1,true,null]"],
+            [new Set(["a", 2, false, null]), "{a,2,false,null}"],
+            [
+                new Map<string, boolean | number>([
+                    ["x", true],
+                    ["y", 100],
+                    ["z", false]
+                ]),
+                "{x:true,y:100,z:false}"
+            ],
+            [
+                (function* gen() {
+                    yield 10;
+                    yield 20;
+                    yield 30;
+                })(),
+                "[10,20,30]"
             ]
         ];
 
         for (const [value, result] of cases) {
-            it(`${
-                typeof value === "bigint" ? value.toString() + "n" : JSON.stringify(value)
-            } results ${result}`, () => {
+            it(`${value} results ${result}`, () => {
                 expect(pp(value)).toEqual(result);
             });
         }
@@ -60,9 +76,7 @@ describe("Utility formatting routines", () => {
         ];
 
         for (const [value, options, result] of cases) {
-            it(`${JSON.stringify(value)} with options ${JSON.stringify(
-                options
-            )} results ${result}`, () => {
+            it(`${value} with options ${JSON.stringify(options)} results ${result}`, () => {
                 expect(ppArr(value, ...options)).toEqual(result);
             });
         }
@@ -103,9 +117,7 @@ describe("Utility formatting routines", () => {
         ];
 
         for (const [value, options, result] of cases) {
-            it(`${JSON.stringify(value)} with options ${JSON.stringify(
-                options
-            )} results ${result}`, () => {
+            it(`${value} with options ${JSON.stringify(options)} results ${result}`, () => {
                 expect(ppSet(value, ...options)).toEqual(result);
             });
         }
@@ -124,9 +136,7 @@ describe("Utility formatting routines", () => {
         ];
 
         for (const [value, options, result] of cases) {
-            it(`${JSON.stringify(value)} with options ${JSON.stringify(
-                options
-            )} results ${result}`, () => {
+            it(`${value} with options ${JSON.stringify(options)} results ${result}`, () => {
                 expect(ppMap(value, ...options)).toEqual(result);
             });
         }

diff --git a/test/unit/misc/utils/assert.spec.ts b/test/unit/misc/utils/assert.spec.ts
@@ -17,13 +17,13 @@ ctx.register(astNode);
 const cases: Array<[boolean, string, Array<string | ASTNode | TypeNode>, string | undefined]> = [
     [true, "Test none", [], undefined],
     [false, "Test string {0}, {1}, {2}", ["x", "y", "z"], "Test string x, y, z"],
-    [false, "Test {0} AST node", [astNode], "Test ElementaryTypeName#1 AST node"],
+    [false, "Test {0} AST node", [astNode], "Test ElementaryTypeName #1 AST node"],
     [false, "Test {0} type node", [typeNode], "Test uint256 type node"],
     [
         false,
         "Test {0} combined {1} parts {2}",
         [astNode, typeNode, "xyz"],
-        "Test ElementaryTypeName#1 combined uint256 parts xyz"
+        "Test ElementaryTypeName #1 combined uint256 parts xyz"
     ],
     [
         false,