More tests, fix bugs found in tests.

remix-astwalker/src/astWalker.ts

@@ -4,6 +4,21 @@ import { AstNodeLegacy, Node, AstNode } from "./index";
 export declare interface AstWalker {
   new(): EventEmitter;
 }
+
+const isObject = function(obj: any): boolean {
+  return obj != null && obj.constructor.name === "Object"
+}
+
+export function isAstNode(node: Object): boolean {
+  return (
+    isObject(node) &&
+    'id' in node &&
+    'nodeType' in node &&
+    'src' in node
+  )
+}
+
+
 /**
  * Crawl the given AST through the function walk(ast, callback)
  */
@@ -102,26 +117,14 @@ export class AstWalker extends EventEmitter {
     }
   }
 
-  isObject(obj: any): boolean {
-    return obj != null && obj.constructor.name === "Object"
-  }
-
-  isAstNode(node: Object): boolean {
-    return (
-      this.isObject(node) &&
-      'id' in node &&
-      'nodeType' in node &&
-      'src' in node
-    )
-  }
-
   walkFullInternal(ast: AstNode, callback: Function) {
 
-    if (this.isAstNode(ast)) {
+    if (isAstNode(ast)) {
       // console.log(`XXX id ${ast.id}, nodeType: ${ast.nodeType}, src: ${ast.src}`);
       callback(ast);
       for (let k of Object.keys(ast)) {
-        if (k in ['id', 'src', 'nodeType']) continue;
+        // Possible optimization:
+        // if (k in ['id', 'src', 'nodeType']) continue;
         const astItem = ast[k];
         if (Array.isArray(astItem)) {
           for (let child of astItem) {
@@ -138,7 +141,7 @@ export class AstWalker extends EventEmitter {
 
   // Normalizes parameter callback and calls walkFullInternal
   walkFull(ast: AstNode, callback: any) {
-    if (!this.isAstNode(ast)) throw new TypeError("first argument should an ast");
+    if (!isAstNode(ast)) throw new TypeError("first argument should be an ast");
     return this.walkFullInternal(ast, callback);
   }
 

remix-astwalker/src/sourceMappings.ts

@@ -1,5 +1,9 @@
-import { AstWalker } from './astWalker';
-import { AstNode, Location } from "./index";
+import { isAstNode, AstWalker } from './astWalker';
+import { AstNode, Location } from "./types";
+
+export declare interface SourceMappings {
+  new(): SourceMappings;
+}
 
 /**
  * Break out fields of an AST's "src" attribute string (s:l:f)
@@ -8,7 +12,7 @@ import { AstNode, Location } from "./index";
  * @param {AstNode} astNode  - the object to convert.
  */
 export function sourceLocationFromAstNode(astNode: AstNode): Location | null {
-  if (astNode.src) {
+  if (isAstNode(astNode) && astNode.src) {
     var split = astNode.src.split(':')
     return <Location>{
       start: parseInt(split[0], 10),
@@ -56,7 +60,7 @@ export class SourceMappings {
       if (nodeLocation &&
         nodeLocation.start == position.start &&
         nodeLocation.length == position.length) {
-        if (!astNodeType || astNodeType === node.name) {
+        if (!astNodeType || astNodeType === node.nodeType) {
           found.push(node)
         }
       }
@@ -87,8 +91,3 @@ export class SourceMappings {
     return found;
   }
 }
-
-module.exports = {
-  SourceMappings,
-  sourceLocationFromAstNode
-};

remix-astwalker/tests/newTests.ts

@@ -1,12 +1,13 @@
 import tape from "tape";
-import { AstWalker, AstNode } from "../src";
+import { AstWalker, AstNode, isAstNode } from "../src";
 import node from "./resources/newAST";
+import legacyNode from "./resources/legacyAST";
 
 tape("New ASTWalker", (t: tape.Test) => {
-  t.test("ASTWalker.walk && .walkAST", (st: tape.Test) => {
+  // New Ast Object
+  const astWalker = new AstWalker();
+  t.test("ASTWalker.walk && .walkastList", (st: tape.Test) => {
     st.plan(24);
-    // New Ast Object
-    const astWalker = new AstWalker();
     // EventListener
     astWalker.on("node", node => {
       if (node.nodeType === "ContractDefinition") {
@@ -51,6 +52,30 @@ tape("New ASTWalker", (t: tape.Test) => {
     });
     st.end();
   });
+  t.test("ASTWalkFull", (st: tape.Test) => {
+    const astNodeCount = 26;
+    st.plan(2 + astNodeCount);
+    let count: number = 0;
+    astWalker.walkFull(node.ast, (node: AstNode) => {
+      st.ok(isAstNode(node), "passed an ast node");
+      count += 1;
+    });
+    st.equal(count, astNodeCount, "traverses all AST nodes");
+    count = 0;
+    let badCall = function() {
+      /* Typescript will keep us from calling walkFull with a legacyAST.
+	 However, for non-typescript uses, we add this test which casts
+	 to an AST to check that there is a run-time check in walkFull.
+      */
+      astWalker.walkFull(<AstNode>legacyNode, (node: AstNode) => {
+        count += 1;
+      });
+    }
+    t.throws(badCall, /first argument should be an ast/,
+      "passing legacyAST fails");
+    st.equal(count, 0, "traverses no AST nodes");
+    st.end();
+  });
 });
 
 function checkProgramDirective(st: tape.Test, node: AstNode) {

remix-astwalker/tests/sourceMappings.ts

@@ -1,26 +1,44 @@
 import tape from "tape";
-import { SourceMappings, sourceLocationFromAstNode } from "../src/sourceMappings";
+import { AstNode, isAstNode, SourceMappings, sourceLocationFromAstNode } from "../src";
 import node from "./resources/newAST";
 
 tape("SourceMappings", (t: tape.Test) => {
   const source = node.source;
   const srcMappings = new SourceMappings(source);
   t.test("SourceMappings constructor", (st: tape.Test) => {
-    st.plan(2);
-
-    st.equal(source, srcMappings.source);
+    st.plan(2)
+    st.equal(srcMappings.source, source, "sourceMappings object has source-code string");
     st.deepEqual(srcMappings.lineBreaks,
-      [15, 26, 27, 38, 39, 81, 87, 103, 119, 135, 141, 142, 186, 192, 193, 199]);
+      [15, 26, 27, 38, 39, 81, 87, 103, 119, 135, 141, 142, 186, 192, 193, 199],
+      "sourceMappings has line-break offsets");
     st.end();
   });
-  t.test("SourceMappings fns", (st: tape.Test) => {
-    st.plan(2);
+  t.test("SourceMappings functions", (st: tape.Test) => {
+    // st.plan(2)
     const ast = node.ast;
     st.deepEqual(sourceLocationFromAstNode(ast.nodes[0]),
-      { start: 0, length: 31, file: 0 });
+      { start: 0, length: 31, file: 0 },
+      "sourceLocationFromAstNode extracts a location");
+
+    /* Typescript will keep us from calling sourceLocationFromAstNode
+       with the wrong type. However, for non-typescript uses, we add
+       this test which casts to an AST to check that there is a
+       run-time check in walkFull.
+    */
+    st.notOk(sourceLocationFromAstNode(<AstNode>null),
+      "sourceLocationFromAstNode rejects an invalid astNode");
     const loc = { start: 267, length: 20, file: 0 };
-    const rr = srcMappings.findNodeAtSourceLocation('ExpressionStatement', loc, ast);
-    st.ok(rr);
+    let astNode = srcMappings.findNodeAtSourceLocation('ExpressionStatement', loc, ast);
+    st.ok(isAstNode(astNode), "findsNodeAtSourceLocation finds something");
+    astNode = srcMappings.findNodeAtSourceLocation('NotARealThingToFind', loc, ast);
+    st.notOk(isAstNode(astNode),
+      "findsNodeAtSourceLocation fails to find something when it should");
+    let astNodes = srcMappings.nodesAtPosition(null, loc, ast);
+    st.equal(astNodes.length, 2, "nodesAtPosition should find more than one astNode");
+    st.ok(isAstNode(astNodes[0]), "nodesAtPosition returns only AST nodes");
+    // console.log(astNodes[0]);
+    astNodes = srcMappings.nodesAtPosition("ExpressionStatement", loc, ast);
+    st.equal(astNodes.length, 1, "nodesAtPosition filtered to a single nodeType");
     st.end();
   });
 });