From 6ddc8d11eed500bda5bd051a5d93d1b1de21840b Mon Sep 17 00:00:00 2001 From: rocky Date: Fri, 14 Jun 2019 01:03:06 -0400 Subject: [PATCH 1/4] [WIP] Add routines for finding AST nodes Functions from remix-lib: - sourceLocationFromAstNode - nodesAtPosition - getLinebreakPositions - findNodeAtSourceLocation Add walkFull that traverses more of the AST nodes. This is non-legacy only. --- .gitignore | 2 + remix-astwalker/src/astWalker.ts | 46 +++++++++++ remix-astwalker/src/index.ts | 1 + remix-astwalker/src/sourceMappings.ts | 105 ++++++++++++++++++++++++ remix-astwalker/src/types.ts | 19 +++-- remix-astwalker/tests/sourceMappings.ts | 26 ++++++ 6 files changed, 194 insertions(+), 5 deletions(-) create mode 100644 remix-astwalker/src/sourceMappings.ts create mode 100644 remix-astwalker/tests/sourceMappings.ts diff --git a/.gitignore b/.gitignore index 6c5184c087..440e22a34c 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,5 @@ package-lock.json TODO soljson.js lerna-debug.log +*~ +/tmp diff --git a/remix-astwalker/src/astWalker.ts b/remix-astwalker/src/astWalker.ts index a20a03040a..b8eba9d983 100644 --- a/remix-astwalker/src/astWalker.ts +++ b/remix-astwalker/src/astWalker.ts @@ -98,6 +98,52 @@ export class AstWalker extends EventEmitter { } } + walkFullInternal(ast: AstNode, callback?: Function | Object) { + function isObject(obj: any): boolean { + return obj != null && obj.constructor.name === "Object" + } + function isAstNode(node: Object): boolean { + return ( + isObject(node) && + 'id' in node && + 'nodeType' in node && + 'src' in node + ); + } + + if (isAstNode(ast) && this.manageCallback(ast, callback)) { + // console.log(`XXX id ${ast.id}, nodeType: ${ast.nodeType}, src: ${ast.src}`); + for (let k of Object.keys(ast)) { + const astItem = ast[k]; + if (Array.isArray(astItem)) { + for (let child of astItem) { + if (child) { + this.walkFullInternal(child, callback); + } + } + } else { + this.walkFullInternal(astItem, callback); + } + } + } + } + + // Normalizes parameter callback and calls walkFullInternal + walkFull(ast: AstNode, callback?: Function | Object) { + if (callback) { + if (callback instanceof Function) { + callback = Object({ "*": callback }); + } + if (!("*" in callback)) { + callback["*"] = function() { + return true; + }; + } + } + return this.walkFullInternal(ast, callback); + } + + walkAstList(sourcesList: Node, cb?: Function) { if (cb) { if (sourcesList.ast) { diff --git a/remix-astwalker/src/index.ts b/remix-astwalker/src/index.ts index 929bce2d90..02f371ee18 100644 --- a/remix-astwalker/src/index.ts +++ b/remix-astwalker/src/index.ts @@ -1,2 +1,3 @@ export * from './types' export * from './astWalker' +export * from './sourceMappings' diff --git a/remix-astwalker/src/sourceMappings.ts b/remix-astwalker/src/sourceMappings.ts new file mode 100644 index 0000000000..294c54cb9f --- /dev/null +++ b/remix-astwalker/src/sourceMappings.ts @@ -0,0 +1,105 @@ +import { AstWalker } from './astWalker'; +import { AstNode, Location } from "./index"; + +/** + * Break out fields of an AST's "src" attribute string (s:l:f) + * into its "start", "length", and "file index" components. + * + * @param {AstNode} astNode - the object to convert. + */ +export function sourceLocationFromAstNode(astNode: AstNode): Location | null { + if (astNode.src) { + var split = astNode.src.split(':') + return { + start: parseInt(split[0], 10), + length: parseInt(split[1], 10), + file: parseInt(split[2], 10) + } + } + return null; +} + +/** + * Routines for retrieving AST object(s) using some criteria, usually + * includng "src' information. + */ +export class SourceMappings { + + readonly source: string; + readonly lineBreaks: Array; + + constructor(source: string) { + this.source = source; + this.lineBreaks = this.getLinebreakPositions(); + }; + + /** + * get a list of nodes that are at the given @arg offset + * + * @param {String} astNodeType - type of node to return or null + * @param {Int} position - character offset + * @return {Object} ast object given by the compiler + */ + nodesAtPosition(astNodeType: string | null, position: number, ast: AstNode): Array { + const astWalker = new AstWalker() + const callback = {} + let found: Array = []; + + /* FIXME: Looking at AST walker code, + I don't understand a need to return a boolean. */ + callback['*'] = function(node: AstNode): boolean { + let nodeLocation = sourceLocationFromAstNode(node); + if (nodeLocation && + nodeLocation.start <= position && + nodeLocation.start + nodeLocation.length >= position) { + if (!astNodeType || astNodeType === node.name) { + found.push(node) + } + } + return true; + } + astWalker.walk(ast, callback); + return found; + } + + /** + * Retrieve line/column position of each source char + * + * @param {String} source - contract source code + * @return {Array} returns an array containing offset of line breaks + */ + getLinebreakPositions(source: string = this.source): Array { + let ret: Array = []; + for (var pos = source.indexOf('\n'); pos >= 0; pos = source.indexOf('\n', pos + 1)) { + ret.push(pos) + } + return ret; + } + + findNodeAtSourceLocation(astNodeType: string, sourceLocation: Location, ast: AstNode | null) { + const astWalker = new AstWalker() + const callback = {}; + let found = null; + /* FIXME: Looking at AST walker code, + I don't understand a need to return a boolean. */ + callback['*'] = function(node: AstNode) { + let nodeLocation = sourceLocationFromAstNode(node); + if (nodeLocation && + nodeLocation.start <= sourceLocation.start && + nodeLocation.start + nodeLocation.length >= sourceLocation.start + sourceLocation.length) { + if (astNodeType === node.nodeType) { + found = node; + } + } + return true; + } + + astWalker.walkFull(ast, callback); + return found; + } +} + +module.exports = { + SourceMappings, + sourceLocationFromAstNode +}; diff --git a/remix-astwalker/src/types.ts b/remix-astwalker/src/types.ts index e36b78c772..038f95d02d 100644 --- a/remix-astwalker/src/types.ts +++ b/remix-astwalker/src/types.ts @@ -1,3 +1,9 @@ +export interface Location { + start: number; + length: number; + file: number; // Would it be clearer to call this a file index? +} + export interface Node { ast?: AstNode; legacyAST?: AstNodeLegacy; @@ -6,12 +12,15 @@ export interface Node { } export interface AstNode { + /* The following fields are essential, and indicates an that object + is an AST node. */ + id: number; // This is unique across all nodes in an AST tree + nodeType: string; + src: string; + absolutePath?: string; exportedSymbols?: Object; - id: number; - nodeType: string; nodes?: Array; - src: string; literals?: Array; file?: string; scope?: number; @@ -22,9 +31,9 @@ export interface AstNode { export interface AstNodeLegacy { id: number; - name: string; + name: string; // This corresponds to nodeType in current AST src: string; - children?: Array; + children?: Array; // This corresponds to nodes in current AST attributes?: AstNodeAtt; } diff --git a/remix-astwalker/tests/sourceMappings.ts b/remix-astwalker/tests/sourceMappings.ts new file mode 100644 index 0000000000..aa1baa0b9b --- /dev/null +++ b/remix-astwalker/tests/sourceMappings.ts @@ -0,0 +1,26 @@ +import tape from "tape"; +import { SourceMappings, sourceLocationFromAstNode } from "../src/sourceMappings"; +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.deepEqual(srcMappings.lineBreaks, + [15, 26, 27, 38, 39, 81, 87, 103, 119, 135, 141, 142, 186, 192, 193, 199]); + st.end(); + }); + t.test("SourceMappings fns", (st: tape.Test) => { + st.plan(2); + const ast = node.ast; + st.deepEqual(sourceLocationFromAstNode(ast.nodes[0]), + { start: 0, length: 31, file: 0 }); + const loc = { start: 267, length: 20, file: 0 }; + const rr = srcMappings.findNodeAtSourceLocation('ExpressionStatement', loc, ast); + st.ok(rr); + st.end(); + }); +}); From 13b96ada6ca6fe89c11e80512bf186a278116a80 Mon Sep 17 00:00:00 2001 From: rocky Date: Fri, 14 Jun 2019 15:29:46 -0400 Subject: [PATCH 2/4] Simplify. Also try to address yann300's concerns --- remix-astwalker/src/astWalker.ts | 47 ++-- remix-astwalker/src/sourceMappings.ts | 38 ++- remix-astwalker/src/types.ts | 6 +- remix-astwalker/tests/resources/newAST.ts | 292 +++++++++++++++++++++- 4 files changed, 331 insertions(+), 52 deletions(-) diff --git a/remix-astwalker/src/astWalker.ts b/remix-astwalker/src/astWalker.ts index b8eba9d983..3b86c59692 100644 --- a/remix-astwalker/src/astWalker.ts +++ b/remix-astwalker/src/astWalker.ts @@ -22,6 +22,10 @@ export class AstWalker extends EventEmitter { node: AstNodeLegacy | AstNode, callback: Object | Function ): any { + // FIXME: we shouldn't be doing this callback determination type on each AST node, + // since the callback function is set once per walk. + // Better would be to store the right one as a variable and + // return that. if (node) { if ((node).name in callback) { return callback[(node).name](node); @@ -98,22 +102,26 @@ export class AstWalker extends EventEmitter { } } - walkFullInternal(ast: AstNode, callback?: Function | Object) { - function isObject(obj: any): boolean { - return obj != null && obj.constructor.name === "Object" - } - function isAstNode(node: Object): boolean { - return ( - isObject(node) && - 'id' in node && - 'nodeType' in node && - 'src' in node - ); - } + 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 (isAstNode(ast) && this.manageCallback(ast, callback)) { + if (this.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; const astItem = ast[k]; if (Array.isArray(astItem)) { for (let child of astItem) { @@ -129,17 +137,8 @@ export class AstWalker extends EventEmitter { } // Normalizes parameter callback and calls walkFullInternal - walkFull(ast: AstNode, callback?: Function | Object) { - if (callback) { - if (callback instanceof Function) { - callback = Object({ "*": callback }); - } - if (!("*" in callback)) { - callback["*"] = function() { - return true; - }; - } - } + walkFull(ast: AstNode, callback: any) { + if (!this.isAstNode(ast)) throw new TypeError("first argument should an ast"); return this.walkFullInternal(ast, callback); } diff --git a/remix-astwalker/src/sourceMappings.ts b/remix-astwalker/src/sourceMappings.ts index 294c54cb9f..cbe4c0422f 100644 --- a/remix-astwalker/src/sourceMappings.ts +++ b/remix-astwalker/src/sourceMappings.ts @@ -30,52 +30,42 @@ export class SourceMappings { constructor(source: string) { this.source = source; - this.lineBreaks = this.getLinebreakPositions(); + + // Create a list of line offsets which will be used to map between + // character offset and line/column positions. + let lineBreaks: Array = []; + for (var pos = source.indexOf('\n'); pos >= 0; pos = source.indexOf('\n', pos + 1)) { + lineBreaks.push(pos) + } + this.lineBreaks = lineBreaks; }; /** - * get a list of nodes that are at the given @arg offset + * get a list of nodes that are at the given @arg position * * @param {String} astNodeType - type of node to return or null * @param {Int} position - character offset * @return {Object} ast object given by the compiler */ - nodesAtPosition(astNodeType: string | null, position: number, ast: AstNode): Array { + nodesAtPosition(astNodeType: string | null, position: Location, ast: AstNode): Array { const astWalker = new AstWalker() - const callback = {} let found: Array = []; - /* FIXME: Looking at AST walker code, - I don't understand a need to return a boolean. */ - callback['*'] = function(node: AstNode): boolean { + const callback = function(node: AstNode): boolean { let nodeLocation = sourceLocationFromAstNode(node); if (nodeLocation && - nodeLocation.start <= position && - nodeLocation.start + nodeLocation.length >= position) { + nodeLocation.start == position.start && + nodeLocation.length == position.length) { if (!astNodeType || astNodeType === node.name) { found.push(node) } } return true; } - astWalker.walk(ast, callback); + astWalker.walkFull(ast, callback); return found; } - /** - * Retrieve line/column position of each source char - * - * @param {String} source - contract source code - * @return {Array} returns an array containing offset of line breaks - */ - getLinebreakPositions(source: string = this.source): Array { - let ret: Array = []; - for (var pos = source.indexOf('\n'); pos >= 0; pos = source.indexOf('\n', pos + 1)) { - ret.push(pos) - } - return ret; - } - findNodeAtSourceLocation(astNodeType: string, sourceLocation: Location, ast: AstNode | null) { const astWalker = new AstWalker() const callback = {}; diff --git a/remix-astwalker/src/types.ts b/remix-astwalker/src/types.ts index 038f95d02d..440ba4d9bb 100644 --- a/remix-astwalker/src/types.ts +++ b/remix-astwalker/src/types.ts @@ -30,10 +30,10 @@ export interface AstNode { } export interface AstNodeLegacy { - id: number; - name: string; // This corresponds to nodeType in current AST + id: number; // This is unique across all nodes in an AST tree + name: string; // This corresponds to "nodeType" in ASTNode src: string; - children?: Array; // This corresponds to nodes in current AST + children?: Array; // This corresponds to "nodes" in ASTNode attributes?: AstNodeAtt; } diff --git a/remix-astwalker/tests/resources/newAST.ts b/remix-astwalker/tests/resources/newAST.ts index 328aa4654b..229c485a6d 100644 --- a/remix-astwalker/tests/resources/newAST.ts +++ b/remix-astwalker/tests/resources/newAST.ts @@ -1,7 +1,297 @@ import { Node } from '../../src/' let node: Node; -node = { "ast": { "absolutePath": "greeter.sol", "exportedSymbols": { "Greeter": [25] }, "id": 26, "nodeType": "SourceUnit", "nodes": [{ "id": 1, "literals": ["solidity", ">=", "0.5", ".0", "<", "0.6", ".0"], "nodeType": "PragmaDirective", "src": "0:31:0" }, { "absolutePath": "mortal.sol", "file": "mortal.sol", "id": 2, "nodeType": "ImportDirective", "scope": 26, "sourceUnit": 53, "src": "32:20:0", "symbolAliases": [], "unitAlias": "" }, { "baseContracts": [{ "arguments": null, "baseName": { "contractScope": null, "id": 3, "name": "Mortal", "nodeType": "UserDefinedTypeName", "referencedDeclaration": 52, "src": "74:6:0", "typeDescriptions": { "typeIdentifier": "t_contract$_Mortal_$52", "typeString": "contract Mortal" } }, "id": 4, "nodeType": "InheritanceSpecifier", "src": "74:6:0" }], "contractDependencies": [52], "contractKind": "contract", "documentation": null, "fullyImplemented": true, "id": 25, "linearizedBaseContracts": [25, 52], "name": "Greeter", "nodeType": "ContractDefinition", "nodes": [{ "constant": false, "id": 6, "name": "greeting", "nodeType": "VariableDeclaration", "scope": 25, "src": "141:15:0", "stateVariable": true, "storageLocation": "default", "typeDescriptions": { "typeIdentifier": "t_string_storage", "typeString": "string" }, "typeName": { "id": 5, "name": "string", "nodeType": "ElementaryTypeName", "src": "141:6:0", "typeDescriptions": { "typeIdentifier": "t_string_storage_ptr", "typeString": "string" } }, "value": null, "visibility": "internal" }, { "body": { "id": 15, "nodeType": "Block", "src": "257:37:0", "statements": [{ "expression": { "argumentTypes": null, "id": 13, "isConstant": false, "isLValue": false, "isPure": false, "lValueRequested": false, "leftHandSide": { "argumentTypes": null, "id": 11, "name": "greeting", "nodeType": "Identifier", "overloadedDeclarations": [], "referencedDeclaration": 6, "src": "267:8:0", "typeDescriptions": { "typeIdentifier": "t_string_storage", "typeString": "string storage ref" } }, "nodeType": "Assignment", "operator": "=", "rightHandSide": { "argumentTypes": null, "id": 12, "name": "_greeting", "nodeType": "Identifier", "overloadedDeclarations": [], "referencedDeclaration": 8, "src": "278:9:0", "typeDescriptions": { "typeIdentifier": "t_string_memory_ptr", "typeString": "string memory" } }, "src": "267:20:0", "typeDescriptions": { "typeIdentifier": "t_string_storage", "typeString": "string storage ref" } }, "id": 14, "nodeType": "ExpressionStatement", "src": "267:20:0" }] }, "documentation": null, "id": 16, "implemented": true, "kind": "constructor", "modifiers": [], "name": "", "nodeType": "FunctionDefinition", "parameters": { "id": 9, "nodeType": "ParameterList", "parameters": [{ "constant": false, "id": 8, "name": "_greeting", "nodeType": "VariableDeclaration", "scope": 16, "src": "225:23:0", "stateVariable": false, "storageLocation": "memory", "typeDescriptions": { "typeIdentifier": "t_string_memory_ptr", "typeString": "string" }, "typeName": { "id": 7, "name": "string", "nodeType": "ElementaryTypeName", "src": "225:6:0", "typeDescriptions": { "typeIdentifier": "t_string_storage_ptr", "typeString": "string" } }, "value": null, "visibility": "internal" }], "src": "224:25:0" }, "returnParameters": { "id": 10, "nodeType": "ParameterList", "parameters": [], "src": "257:0:0" }, "scope": 25, "src": "213:81:0", "stateMutability": "nonpayable", "superFunction": null, "visibility": "public" }, { "body": { "id": 23, "nodeType": "Block", "src": "377:32:0", "statements": [{ "expression": { "argumentTypes": null, "id": 21, "name": "greeting", "nodeType": "Identifier", "overloadedDeclarations": [], "referencedDeclaration": 6, "src": "394:8:0", "typeDescriptions": { "typeIdentifier": "t_string_storage", "typeString": "string storage ref" } }, "functionReturnParameters": 20, "id": 22, "nodeType": "Return", "src": "387:15:0" }] }, "documentation": null, "id": 24, "implemented": true, "kind": "function", "modifiers": [], "name": "greet", "nodeType": "FunctionDefinition", "parameters": { "id": 17, "nodeType": "ParameterList", "parameters": [], "src": "338:2:0" }, "returnParameters": { "id": 20, "nodeType": "ParameterList", "parameters": [{ "constant": false, "id": 19, "name": "", "nodeType": "VariableDeclaration", "scope": 24, "src": "362:13:0", "stateVariable": false, "storageLocation": "memory", "typeDescriptions": { "typeIdentifier": "t_string_memory_ptr", "typeString": "string" }, "typeName": { "id": 18, "name": "string", "nodeType": "ElementaryTypeName", "src": "362:6:0", "typeDescriptions": { "typeIdentifier": "t_string_storage_ptr", "typeString": "string" } }, "value": null, "visibility": "internal" }], "src": "361:15:0" }, "scope": 25, "src": "324:85:0", "stateMutability": "view", "superFunction": null, "visibility": "public" }], "scope": 26, "src": "54:357:0" }], "src": "0:412:0" }, "id": 0 } +node = { + "ast": + { + "absolutePath": "greeter.sol", + "exportedSymbols": { + "Greeter": [ + 25 + ] + }, + "id": 26, + "nodeType": "SourceUnit", + "nodes": [ + { + "id": 1, + "literals": [ + "solidity", + ">=", + "0.5", + ".0", + "<", + "0.6", + ".0" + ], + "nodeType": "PragmaDirective", + "src": "0:31:0" + }, + { + "absolutePath": "mortal.sol", + "file": "mortal.sol", + "id": 2, + "nodeType": "ImportDirective", + "scope": 26, + "sourceUnit": 53, + "src": "32:20:0", + "symbolAliases": [], + "unitAlias": "" + }, + { + "baseContracts": [ + { + "arguments": null, + "baseName": { + "contractScope": null, + "id": 3, + "name": "Mortal", + "nodeType": "UserDefinedTypeName", + "referencedDeclaration": 52, + "src": "74:6:0", + "typeDescriptions": { + "typeIdentifier": "t_contract$_Mortal_$52", + "typeString": "contract Mortal" + } + }, + "id": 4, + "nodeType": "InheritanceSpecifier", + "src": "74:6:0" + } + ], + "contractDependencies": [ + 52 + ], + "contractKind": "contract", + "documentation": null, + "fullyImplemented": true, + "id": 25, + "linearizedBaseContracts": [ + 25, + 52 + ], + "name": "Greeter", + "nodeType": "ContractDefinition", + "nodes": [ + { + "constant": false, + "id": 6, + "name": "greeting", + "nodeType": "VariableDeclaration", + "scope": 25, + "src": "141:15:0", + "stateVariable": true, + "storageLocation": "default", + "typeDescriptions": { + "typeIdentifier": "t_string_storage", + "typeString": "string" + }, + "typeName": { + "id": 5, + "name": "string", + "nodeType": "ElementaryTypeName", + "src": "141:6:0", + "typeDescriptions": { + "typeIdentifier": "t_string_storage_ptr", + "typeString": "string" + } + }, + "value": null, + "visibility": "internal" + }, + { + "body": { + "id": 15, + "nodeType": "Block", + "src": "257:37:0", + "statements": [ + { + "expression": { + "argumentTypes": null, + "id": 13, + "isConstant": false, + "isLValue": false, + "isPure": false, + "lValueRequested": false, + "leftHandSide": { + "argumentTypes": null, + "id": 11, + "name": "greeting", + "nodeType": "Identifier", + "overloadedDeclarations": [], + "referencedDeclaration": 6, + "src": "267:8:0", + "typeDescriptions": { + "typeIdentifier": "t_string_storage", + "typeString": "string storage ref" + } + }, + "nodeType": "Assignment", + "operator": "=", + "rightHandSide": { + "argumentTypes": null, + "id": 12, + "name": "_greeting", + "nodeType": "Identifier", + "overloadedDeclarations": [], + "referencedDeclaration": 8, + "src": "278:9:0", + "typeDescriptions": { + "typeIdentifier": "t_string_memory_ptr", + "typeString": "string memory" + } + }, + "src": "267:20:0", + "typeDescriptions": { + "typeIdentifier": "t_string_storage", + "typeString": "string storage ref" + } + }, + "id": 14, + "nodeType": "ExpressionStatement", + "src": "267:20:0" + } + ] + }, + "documentation": null, + "id": 16, + "implemented": true, + "kind": "constructor", + "modifiers": [], + "name": "", + "nodeType": "FunctionDefinition", + "parameters": { + "id": 9, + "nodeType": "ParameterList", + "parameters": [ + { + "constant": false, + "id": 8, + "name": "_greeting", + "nodeType": "VariableDeclaration", + "scope": 16, + "src": "225:23:0", + "stateVariable": false, + "storageLocation": "memory", + "typeDescriptions": { + "typeIdentifier": "t_string_memory_ptr", + "typeString": "string" + }, + "typeName": { + "id": 7, + "name": "string", + "nodeType": "ElementaryTypeName", + "src": "225:6:0", + "typeDescriptions": { + "typeIdentifier": "t_string_storage_ptr", + "typeString": "string" + } + }, + "value": null, + "visibility": "internal" + } + ], + "src": "224:25:0" + }, + "returnParameters": { + "id": 10, + "nodeType": "ParameterList", + "parameters": [], + "src": "257:0:0" + }, + "scope": 25, + "src": "213:81:0", + "stateMutability": "nonpayable", + "superFunction": null, + "visibility": "public" + }, + { + "body": { + "id": 23, + "nodeType": "Block", + "src": "377:32:0", + "statements": [ + { + "expression": { + "argumentTypes": null, + "id": 21, + "name": "greeting", + "nodeType": "Identifier", + "overloadedDeclarations": [], + "referencedDeclaration": 6, + "src": "394:8:0", + "typeDescriptions": { + "typeIdentifier": "t_string_storage", + "typeString": "string storage ref" + } + }, + "functionReturnParameters": 20, + "id": 22, + "nodeType": "Return", + "src": "387:15:0" + } + ] + }, + "documentation": null, + "id": 24, + "implemented": true, + "kind": "function", + "modifiers": [], + "name": "greet", + "nodeType": "FunctionDefinition", + "parameters": { + "id": 17, + "nodeType": "ParameterList", + "parameters": [], + "src": "338:2:0" + }, + "returnParameters": { + "id": 20, + "nodeType": "ParameterList", + "parameters": [ + { + "constant": false, + "id": 19, + "name": "", + "nodeType": "VariableDeclaration", + "scope": 24, + "src": "362:13:0", + "stateVariable": false, + "storageLocation": "memory", + "typeDescriptions": { + "typeIdentifier": "t_string_memory_ptr", + "typeString": "string" + }, + "typeName": { + "id": 18, + "name": "string", + "nodeType": "ElementaryTypeName", + "src": "362:6:0", + "typeDescriptions": { + "typeIdentifier": "t_string_storage_ptr", + "typeString": "string" + } + }, + "value": null, + "visibility": "internal" + } + ], + "src": "361:15:0" + }, + "scope": 25, + "src": "324:85:0", + "stateMutability": "view", + "superFunction": null, + "visibility": "public" + } + ], + "scope": 26, + "src": "54:357:0" + } + ], + "src": "0:412:0" + } +} node.source = `contract test { From 2f4f8cea0dd2854f51a55d0cd13c38e73495b09e Mon Sep 17 00:00:00 2001 From: rocky Date: Fri, 14 Jun 2019 19:53:23 -0400 Subject: [PATCH 3/4] findNodeAtSourceLocation bugfixes --- remix-astwalker/src/sourceMappings.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/remix-astwalker/src/sourceMappings.ts b/remix-astwalker/src/sourceMappings.ts index cbe4c0422f..ca1b845269 100644 --- a/remix-astwalker/src/sourceMappings.ts +++ b/remix-astwalker/src/sourceMappings.ts @@ -66,18 +66,17 @@ export class SourceMappings { return found; } - findNodeAtSourceLocation(astNodeType: string, sourceLocation: Location, ast: AstNode | null) { + findNodeAtSourceLocation(astNodeType: string | undefined, sourceLocation: Location, ast: AstNode | null): AstNode | null { const astWalker = new AstWalker() - const callback = {}; let found = null; /* FIXME: Looking at AST walker code, I don't understand a need to return a boolean. */ - callback['*'] = function(node: AstNode) { + const callback = function(node: AstNode) { let nodeLocation = sourceLocationFromAstNode(node); if (nodeLocation && - nodeLocation.start <= sourceLocation.start && - nodeLocation.start + nodeLocation.length >= sourceLocation.start + sourceLocation.length) { - if (astNodeType === node.nodeType) { + nodeLocation.start == sourceLocation.start && + nodeLocation.length == sourceLocation.length) { + if (astNodeType == undefined || astNodeType === node.nodeType) { found = node; } } From 4eac942db5a97eb740673ae86232045ca1a53d9d Mon Sep 17 00:00:00 2001 From: rocky Date: Sun, 16 Jun 2019 15:45:38 -0400 Subject: [PATCH 4/4] More tests, fix bugs found in tests. --- remix-astwalker/src/astWalker.ts | 35 ++++++++++++----------- remix-astwalker/src/sourceMappings.ts | 17 ++++++----- remix-astwalker/tests/newTests.ts | 33 ++++++++++++++++++--- remix-astwalker/tests/sourceMappings.ts | 38 ++++++++++++++++++------- 4 files changed, 84 insertions(+), 39 deletions(-) diff --git a/remix-astwalker/src/astWalker.ts b/remix-astwalker/src/astWalker.ts index 3b86c59692..ffb105d2f9 100644 --- a/remix-astwalker/src/astWalker.ts +++ b/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); } diff --git a/remix-astwalker/src/sourceMappings.ts b/remix-astwalker/src/sourceMappings.ts index ca1b845269..058b2a66ed 100644 --- a/remix-astwalker/src/sourceMappings.ts +++ b/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 { 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 -}; diff --git a/remix-astwalker/tests/newTests.ts b/remix-astwalker/tests/newTests.ts index 56fe6f9ac7..a494d22087 100644 --- a/remix-astwalker/tests/newTests.ts +++ b/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(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) { diff --git a/remix-astwalker/tests/sourceMappings.ts b/remix-astwalker/tests/sourceMappings.ts index aa1baa0b9b..808d2cfd0b 100644 --- a/remix-astwalker/tests/sourceMappings.ts +++ b/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(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(); }); });