Merge pull request #4810 from trufflesuite/new-tracer

Display "new" before contract name in stacktraces for constructors

packages/debug-utils/index.js

@@ -34,59 +34,59 @@ const verbosePanicTable = {
 };
 
 const commandReference = {
-  o: "step over",
-  i: "step into",
-  u: "step out",
-  n: "step next",
+  "o": "step over",
+  "i": "step into",
+  "u": "step out",
+  "n": "step next",
   ";": "step instruction (include number to step multiple)",
-  p: "print instruction & state (`p [mem|cal|sto]*`; see docs for more)",
-  l: "print additional source context (`l [+<lines-ahead>] [-<lines-back>]`)",
-  h: "print this help",
-  v: "print variables and values (`v [bui|glo|con|loc]*`)",
+  "p": "print instruction & state (`p [mem|cal|sto]*`; see docs for more)",
+  "l": "print additional source context (`l [+<lines-ahead>] [-<lines-back>]`)",
+  "h": "print this help",
+  "v": "print variables and values (`v [bui|glo|con|loc]*`)",
   ":": "evaluate expression - see `v`",
   "+": "add watch expression (`+:<expr>`)",
   "-": "remove watch expression (-:<expr>)",
   "?": "list existing watch expressions and breakpoints",
-  b: "add breakpoint (`b [[<source-file>:]<line-number>]`; see docs for more)",
-  B: "remove breakpoint (similar to adding, or `B all` to remove all)",
-  c: "continue until breakpoint",
-  q: "quit",
-  r: "reset",
-  t: "load new transaction",
-  T: "unload transaction",
-  s: "print stacktrace",
-  g: "turn on generated sources",
-  G: "turn off generated sources except via `;`",
-  y: "(if at end) reset & continue to final error",
-  Y: "reset & continue to previous error"
+  "b": "add breakpoint (`b [[<source-file>:]<line-number>]`; see docs for more)",
+  "B": "remove breakpoint (similar to adding, or `B all` to remove all)",
+  "c": "continue until breakpoint",
+  "q": "quit",
+  "r": "reset",
+  "t": "load new transaction",
+  "T": "unload transaction",
+  "s": "print stacktrace",
+  "g": "turn on generated sources",
+  "G": "turn off generated sources except via `;`",
+  "y": "(if at end) reset & continue to final error",
+  "Y": "reset & continue to previous error"
 };
 
 const shortCommandReference = {
-  o: "step over",
-  i: "step into",
-  u: "step out",
-  n: "step next",
+  "o": "step over",
+  "i": "step into",
+  "u": "step out",
+  "n": "step next",
   ";": "step instruction",
-  p: "print state",
-  l: "print context",
-  h: "print help",
-  v: "print variables",
+  "p": "print state",
+  "l": "print context",
+  "h": "print help",
+  "v": "print variables",
   ":": "evaluate",
   "+": "add watch",
   "-": "remove watch",
   "?": "list watches & breakpoints",
-  b: "add breakpoint",
-  B: "remove breakpoint",
-  c: "continue",
-  q: "quit",
-  r: "reset",
-  t: "load",
-  T: "unload",
-  s: "stacktrace",
-  g: "turn on generated sources",
-  G: "turn off generated sources",
-  y: "reset & go to final error",
-  Y: "reset & go to previous error"
+  "b": "add breakpoint",
+  "B": "remove breakpoint",
+  "c": "continue",
+  "q": "quit",
+  "r": "reset",
+  "t": "load",
+  "T": "unload",
+  "s": "stacktrace",
+  "g": "turn on generated sources",
+  "G": "turn off generated sources",
+  "y": "reset & go to final error",
+  "Y": "reset & go to previous error"
 };
 
 const truffleColors = {
@@ -107,69 +107,69 @@ const DEFAULT_TAB_WIDTH = 8;
 
 const trufflePalette = {
   /* base (chromafi special, not hljs) */
-  base: chalk,
-  lineNumbers: chalk,
-  trailingSpace: chalk,
+  "base": chalk,
+  "lineNumbers": chalk,
+  "trailingSpace": chalk,
   /* classes hljs-solidity actually uses */
-  keyword: truffleColors.mint,
-  number: truffleColors.red,
-  string: truffleColors.green,
-  params: truffleColors.pink,
-  builtIn: truffleColors.watermelon,
-  built_in: truffleColors.watermelon, //just to be sure
-  literal: truffleColors.watermelon,
-  function: truffleColors.orange,
-  title: truffleColors.orange,
-  class: truffleColors.orange,
-  comment: truffleColors.comment,
-  doctag: truffleColors.comment,
-  operator: truffleColors.blue,
-  punctuation: truffleColors.purple,
+  "keyword": truffleColors.mint,
+  "number": truffleColors.red,
+  "string": truffleColors.green,
+  "params": truffleColors.pink,
+  "builtIn": truffleColors.watermelon,
+  "built_in": truffleColors.watermelon, //just to be sure
+  "literal": truffleColors.watermelon,
+  "function": truffleColors.orange,
+  "title": truffleColors.orange,
+  "class": truffleColors.orange,
+  "comment": truffleColors.comment,
+  "doctag": truffleColors.comment,
+  "operator": truffleColors.blue,
+  "punctuation": truffleColors.purple,
   /* classes it might soon use! */
-  meta: truffleColors.pink,
-  metaString: truffleColors.green,
+  "meta": truffleColors.pink,
+  "metaString": truffleColors.green,
   "meta-string": truffleColors.green, //similar
   /* classes it doesn't currently use but notionally could */
-  type: truffleColors.orange,
-  symbol: truffleColors.orange,
-  metaKeyword: truffleColors.mint,
+  "type": truffleColors.orange,
+  "symbol": truffleColors.orange,
+  "metaKeyword": truffleColors.mint,
   "meta-keyword": truffleColors.mint, //again, to be sure
-  property: chalk, //not putting any highlighting here for now
+  "property": chalk, //not putting any highlighting here for now
   /* classes that don't make sense for Solidity */
-  regexp: chalk, //solidity does not have regexps
-  subst: chalk, //or string interpolation
-  name: chalk, //or s-expressions
-  builtInName: chalk, //or s-expressions, again
+  "regexp": chalk, //solidity does not have regexps
+  "subst": chalk, //or string interpolation
+  "name": chalk, //or s-expressions
+  "builtInName": chalk, //or s-expressions, again
   "builtin-name": chalk, //just to be sure
   /* classes for config, markup, CSS, templates, diffs (not programming) */
-  section: chalk,
-  tag: chalk,
-  attr: chalk,
-  attribute: chalk,
-  variable: chalk,
-  bullet: chalk,
-  code: chalk,
-  emphasis: chalk,
-  strong: chalk,
-  formula: chalk,
-  link: chalk,
-  quote: chalk,
-  selectorAttr: chalk, //lotta redundancy follows
+  "section": chalk,
+  "tag": chalk,
+  "attr": chalk,
+  "attribute": chalk,
+  "variable": chalk,
+  "bullet": chalk,
+  "code": chalk,
+  "emphasis": chalk,
+  "strong": chalk,
+  "formula": chalk,
+  "link": chalk,
+  "quote": chalk,
+  "selectorAttr": chalk, //lotta redundancy follows
   "selector-attr": chalk,
-  selectorClass: chalk,
+  "selectorClass": chalk,
   "selector-class": chalk,
-  selectorId: chalk,
+  "selectorId": chalk,
   "selector-id": chalk,
-  selectorPseudo: chalk,
+  "selectorPseudo": chalk,
   "selector-pseudo": chalk,
-  selectorTag: chalk,
+  "selectorTag": chalk,
   "selector-tag": chalk,
-  templateTag: chalk,
+  "templateTag": chalk,
   "template-tag": chalk,
-  templateVariable: chalk,
+  "templateVariable": chalk,
   "template-variable": chalk,
-  addition: chalk,
-  deletion: chalk
+  "addition": chalk,
+  "deletion": chalk
 };
 
 var DebugUtils = {
@@ -736,12 +736,24 @@ var DebugUtils = {
     //reverse
     stacktrace = stacktrace.slice().reverse(); //reverse is in-place so clone first
     let lines = stacktrace.map(
-      ({ functionName, contractName, address, location, type }) => {
+      ({
+        functionName,
+        contractName,
+        address,
+        location,
+        isConstructor,
+        type
+      }) => {
         let name;
         if (contractName && functionName) {
           name = `${contractName}.${functionName}`;
         } else if (contractName) {
-          name = contractName;
+          name =
+            type === "external" && isConstructor
+              ? `new ${contractName}`
+              : contractName;
+          //for internal calls, it doesn't really make sense
+          //to write "new Contract"
         } else if (functionName) {
           name = functionName;
         } else {

packages/debugger/lib/stacktrace/reducers.js

@@ -15,6 +15,7 @@ function callstack(state = [], action) {
         type: "internal",
         calledFromLocation: location,
         address: state[state.length - 1].address,
+        isConstructor: state[state.length - 1].isConstructor,
         functionName:
           functionNode &&
           (functionNode.nodeType === "FunctionDefinition" ||
@@ -47,6 +48,7 @@ function callstack(state = [], action) {
         calledFromLocation: action.location,
         functionName: undefined,
         contractName: action.context.contractName,
+        isConstructor: action.context.isConstructor,
         combineWithNextInternal: action.combineWithNextInternal
         //sourceIsInternal doesn't really apply here, so let's just omit it
       };

packages/debugger/lib/stacktrace/selectors/index.js

@@ -33,6 +33,7 @@ function generateReport(rawStack, location, status, message) {
         //note: since the next frame is internal, it will have the
         //same address as this, so we don't have to specify which
         //one to take the address from
+        //(same with isConstructor)
       };
       callstack.push(combinedFrame);
       i++; //!! SKIP THE NEXT FRAME!
@@ -49,10 +50,11 @@ function generateReport(rawStack, location, status, message) {
   locations.push(location);
   debug("locations: %O", locations);
   const names = callstack.map(
-    ({ functionName, contractName, address, type }) => ({
+    ({ functionName, contractName, address, isConstructor, type }) => ({
       functionName,
       contractName,
       address,
+      isConstructor,
       type
     })
   );

packages/debugger/test/stacktrace.js

@@ -216,6 +216,7 @@ describe("Stack tracing", function () {
     assert(contractNames.every(name => name === "StacktraceTest"));
     let addresses = report.map(({ address }) => address);
     assert(addresses.every(address => address === instance.address));
+    assert(report.every(({ isConstructor }) => !isConstructor));
     let status = report[report.length - 1].status;
     assert.isFalse(status);
     let location = report[report.length - 1].location;
@@ -262,6 +263,7 @@ describe("Stack tracing", function () {
     assert(contractNames.every(name => name === "StacktraceTest"));
     let addresses = report.map(({ address }) => address);
     assert(addresses.every(address => address === instance.address));
+    assert(report.every(({ isConstructor }) => !isConstructor));
     let status = report[report.length - 1].status;
     assert.isUndefined(status);
     let location = report[report.length - 1].location;
@@ -332,6 +334,7 @@ describe("Stack tracing", function () {
     assert(contractNames.every(name => name === "StacktraceTest"));
     let addresses = report.map(({ address }) => address);
     assert(addresses.every(address => address === instance.address));
+    assert(report.every(({ isConstructor }) => !isConstructor));
     let status = report[report.length - 1].status;
     assert.isFalse(status);
     let location = report[report.length - 2].location; //note, -2 because of undefined on top
@@ -383,6 +386,7 @@ describe("Stack tracing", function () {
     assert(contractNames.slice(0, -2).every(name => name === "StacktraceTest"));
     let addresses = report.map(({ address }) => address);
     assert(addresses.every(address => address === instance.address));
+    assert(report.every(({ isConstructor }) => !isConstructor));
     let status = report[report.length - 1].status;
     assert.isFalse(status);
     let location = report[report.length - 3].location; //note, -3 because of panic & undefined on top
@@ -441,6 +445,7 @@ describe("Stack tracing", function () {
     );
     addresses.pop();
     assert(addresses.every(address => address === instance.address));
+    assert(report.every(({ isConstructor }) => !isConstructor));
     let status = report[report.length - 1].status;
     assert.isTrue(status);
     let location = report[report.length - 1].location;
@@ -505,6 +510,12 @@ describe("Stack tracing", function () {
     );
     addresses = addresses.slice(0, -2); //cut off top two frames that we just checked
     assert(addresses.every(address => address === instance.address));
+    let isConstructorArray = report.map(({ isConstructor }) => isConstructor);
+    assert.isTrue(isConstructorArray[isConstructorArray.length - 1]);
+    isConstructorArray.pop(); //top frame
+    assert.isTrue(isConstructorArray[isConstructorArray.length - 1]);
+    isConstructorArray.pop(); //second-to-top frame
+    assert(isConstructorArray.every(isConstructor => !isConstructor));
     let status = report[report.length - 1].status;
     assert.isFalse(status);
     let location = report[report.length - 1].location;
@@ -563,6 +574,7 @@ describe("Stack tracing", function () {
     assert(contractNames.every(name => name === "StacktraceTest"));
     let addresses = report.map(({ address }) => address);
     assert(addresses.every(address => address === instance.address));
+    assert(report.every(({ isConstructor }) => !isConstructor));
     let status = report[report.length - 1].status;
     assert.isFalse(status);
     let location = report[report.length - 1].location;
@@ -580,7 +592,7 @@ describe("Stack tracing", function () {
     assert.isTrue(report[0].custom);
   });
 
-  it("Generates correct stack trace after an internal call in a constructor", async function () {
+  it("Generates correct stack trace after an internal call in a library", async function () {
     this.timeout(12000);
     const instance = await abstractions.StacktraceTest.deployed();
     const library = await abstractions.Library.deployed();
@@ -636,6 +648,7 @@ describe("Stack tracing", function () {
       addresses.every(address => address === instance.address),
       "unexpected remaining addresses"
     );
+    assert(report.every(({ isConstructor }) => !isConstructor));
     let status = report[report.length - 1].status;
     assert.isFalse(status);
     let location = report[report.length - 1].location;