Merge pull request #196 from sc-forks/fix/view-alignment-bug

Preserve CRs of removed modifiers

lib/preprocessor.js

@@ -1,9 +1,10 @@
 const SolExplore = require('sol-explore');
 const SolidityParser = require('solidity-parser-sc');
 
+const crRegex = /[\r\n ]+$/g;
 /**
  * Splices enclosing brackets into `contract` around `expression`;
- * @param  {String} contract solidity code
+ * @param  {String} contract solidity source
  * @param  {Object} node     AST node to bracket
  * @return {String}          contract
  */
@@ -11,6 +12,20 @@ function blockWrap(contract, expression) {
   return contract.slice(0, expression.start) + '{' + contract.slice(expression.start, expression.end) + '}' + contract.slice(expression.end);
 }
 
+
+/**
+ * Captures carriage returns at modifiers we'll remove. These need to be re-injected into the
+ * source to keep line report alignments accurate.
+ * @param  {String} contract solidity source
+ * @param  {Object} modifier AST node
+ * @return {String} whitespace around the modifier
+ */
+function getModifierWhitespace(contract,  modifier){
+  const source = contract.slice(modifier.start, modifier.end);
+  const whitespace = source.match(crRegex) || [];
+  return whitespace.join('');
+}
+
 /**
  * Locates unbracketed singleton statements attached to if, else, for and while statements
  * and brackets them. Instrumenter needs to inject events at these locations and having
@@ -55,7 +70,12 @@ module.exports.run = function r(contract) {
             // We want to remove constant / pure / view from functions
             for (let i = 0; i < node.modifiers.length; i++) {
               if (['pure', 'constant', 'view'].indexOf(node.modifiers[i].name) > -1) {
-                contract = contract.slice(0, node.modifiers[i].start) + contract.slice(node.modifiers[i].end);
+                let whitespace = getModifierWhitespace(contract, node.modifiers[i]);
+
+                contract = contract.slice(0, node.modifiers[i].start) +
+                           whitespace +
+                           contract.slice(node.modifiers[i].end);
+
                 keepRunning = true;
                 this.stopTraversal();
               }

package-lock.json

@@ -2842,13 +2842,6 @@
             }
           }
         },
-        "string_decoder": {
-          "version": "1.0.1",
-          "bundled": true,
-          "requires": {
-            "safe-buffer": "5.0.1"
-          }
-        },
         "string-width": {
           "version": "1.0.2",
           "bundled": true,
@@ -2858,6 +2851,13 @@
             "strip-ansi": "3.0.1"
           }
         },
+        "string_decoder": {
+          "version": "1.0.1",
+          "bundled": true,
+          "requires": {
+            "safe-buffer": "5.0.1"
+          }
+        },
         "stringstream": {
           "version": "0.0.5",
           "bundled": true,
@@ -5681,14 +5681,6 @@
         "xtend": "4.0.1"
       }
     },
-    "string_decoder": {
-      "version": "1.0.3",
-      "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz",
-      "integrity": "sha512-4AH6Z5fzNNBcH+6XDMfA/BTt87skxqJlO0lAh3Dker5zThcAxG6mKz+iGu308UKoPPQ8Dcqx/4JhujzltRa+hQ==",
-      "requires": {
-        "safe-buffer": "5.1.1"
-      }
-    },
     "string-width": {
       "version": "2.1.1",
       "resolved": "https://registry.npmjs.org/string-width/-/string-width-2.1.1.tgz",
@@ -5718,6 +5710,14 @@
         }
       }
     },
+    "string_decoder": {
+      "version": "1.0.3",
+      "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz",
+      "integrity": "sha512-4AH6Z5fzNNBcH+6XDMfA/BTt87skxqJlO0lAh3Dker5zThcAxG6mKz+iGu308UKoPPQ8Dcqx/4JhujzltRa+hQ==",
+      "requires": {
+        "safe-buffer": "5.1.1"
+      }
+    },
     "stringstream": {
       "version": "0.0.5",
       "resolved": "https://registry.npmjs.org/stringstream/-/stringstream-0.0.5.tgz",

test/cli/totallyPure.js

@@ -68,4 +68,10 @@ contract('TotallyPure', accounts => {
     const value = await instance.inheritedConstant();
     assert.equal(value.toNumber(), 5);
   });
+
+  it('calls a view method whose modifiers span lines', async () => {
+    const instance = await TotallyPure.deployed();
+    const value = await instance.multiline(5, 7)
+    assert.equal(value.toNumber(), 99);
+  });
 });

test/sources/cli/TotallyPure.sol

@@ -39,4 +39,14 @@ contract TotallyPure is PureView {
   function usesLibrary() public constant returns (uint){
     return CLibrary.a();
   }
+
+  function multiline(uint x,
+                     uint y)
+                     public
+                     view
+                     returns (uint)
+  {
+    return onehundred;
+  }
+
 }