Merge pull request #135 from sc-forks/totally-pure

Fix pure fix

lib/app.js

@@ -89,6 +89,7 @@ class App {
           ? fs.writeFileSync(`${this.coverageDir}/truffle-config.js`, trufflejs)
           : fs.writeFileSync(`${this.coverageDir}/truffle.js`, trufflejs);
       }
+
     } catch (err) {
       const msg = ('There was a problem generating the coverage environment: ');
       this.cleanUp(msg + err);
@@ -112,11 +113,9 @@ class App {
       shell.ls(`${this.coverageDir}/contracts/**/*.sol`).forEach(file => {
         if (!this.skipFiles.includes(file)) {
           this.log('Instrumenting ', file);
-          currentFile = file;
 
-          const contractPath = isWin
-            ? path.resolve(file).split('\\').join('/')
-            : path.resolve(file);
+          currentFile = file;
+          const contractPath = this.platformNeutralPath(file);
           const working = this.workingDir.substring(1);
           const canonicalPath = contractPath.split('/coverageEnv').join(working);
           const contract = fs.readFileSync(contractPath).toString();
@@ -131,6 +130,7 @@ class App {
       const msg = `There was a problem instrumenting ${currentFile}: `;
       this.cleanUp(msg + err);
     }
+    this.postProcessPure(this.coverageDir);
   }
 
   /**
@@ -223,7 +223,7 @@ class App {
           // Generate Istanbul report
           try {
             this.coverage.generate(this.events, `${this.workingDir}/contracts`);
-            const relativeMapping = App.makeKeysRelative(this.coverage.coverage, this.workingDir);
+            const relativeMapping = this.makeKeysRelative(this.coverage.coverage, this.workingDir);
             const json = JSON.stringify(relativeMapping);
             fs.writeFileSync('./coverage.json', json);
 
@@ -247,7 +247,13 @@ class App {
 
   // ------------------------------------------ Utils ----------------------------------------------
 
-  static makeKeysRelative(map, root) {
+  /**
+   * Relativizes path keys so that istanbul report can be read on Windows
+   * @param  {Object} map  coverage map generated by coverageMap
+   * @param  {[type]} root working directory
+   * @return {[type]}      map with relativized keys
+   */
+  makeKeysRelative(map, root) {
     const newCoverage = {};
     Object.keys(map).forEach(pathKey => {
       newCoverage[path.relative(root, pathKey)] = map[pathKey];
@@ -255,6 +261,35 @@ class App {
     return newCoverage;
   }
 
+  /**
+   * Conver absolute paths from Windows, if necessary
+   * @param  {String} file path
+   * @return {[type]}      normalized path
+   */
+  platformNeutralPath(file) {
+    return (isWin)
+      ? path.resolve(file).split('\\').join('/')
+      : path.resolve(file);
+  }
+
+  /**
+   * Replaces all occurences of `pure` and `view` modifiers in all .sols
+   * in the coverageEnv before the `contracts` folder is instrumented.
+   * @param  {String} env 'coverageEnv' presumably
+   */
+  postProcessPure(env) {
+    shell.ls(`${env}/**/*.sol`).forEach(file => {
+      const pureRe = /\spure\s/gi;
+      const viewRe = /\sview\s/gi;
+
+      const contractPath = this.platformNeutralPath(file);
+      let contract = fs.readFileSync(contractPath).toString();
+      contract = contract.replace(pureRe, ' ');
+      contract = contract.replace(viewRe, ' ');
+      fs.writeFileSync(contractPath, contract);
+    })
+  }
+
   /**
    * Allows config to turn logging off (for CI)
    * @param {Boolean} isSilent

lib/preprocessor.js

@@ -25,9 +25,6 @@ function blockWrap(contract, expression) {
 module.exports.run = function r(contract) {
   let keepRunning = true;
 
-  contract = contract.replace(/ pure /, ' ');
-  contract = contract.replace(/ view /, ' ');
-
   while (keepRunning) {
     const ast = SolidityParser.parse(contract);
     keepRunning = false;

test/app.js

@@ -246,7 +246,7 @@ describe('app', () => {
     assert(pathExists('./coverage.json') === false, 'should start without: coverage.json');
 
     // Run script (exits 0);
-    mock.install('PureView.sol', 'pureview.js', config);
+    mock.install('TotallyPure.sol', 'totallyPure.js', config);
     shell.exec(script);
     assert(shell.error() === null, 'script should not error');
 
@@ -258,8 +258,8 @@ describe('app', () => {
     // This test is tightly bound to the function names in Simple.sol
     const produced = JSON.parse(fs.readFileSync('./coverage.json', 'utf8'));
     const path = Object.keys(produced)[0];
-    assert(produced[path].fnMap['1'].name === 'isPure', 'coverage.json should map "isPure"');
-    assert(produced[path].fnMap['2'].name === 'isView', 'coverage.json should map "isView"');
+    assert(produced[path].fnMap['1'].name === 'usesThem', 'coverage.json should map "usesThem"');
+    assert(produced[path].fnMap['2'].name === 'isPure', 'coverage.json should map "getX"');
     collectGarbage();
   })
 

test/cli/totallyPure.js

@@ -0,0 +1,32 @@
+/* eslint-env node, mocha */
+/* global artifacts, contract, assert */
+
+const TotallyPure = artifacts.require('./TotallyPure.sol');
+
+contract('TotallyPure', accounts => {
+
+  it('calls imported,  inherited pure/view functions within its own function', async function(){
+    const instance = await TotallyPure.deployed();
+    await instance.usesThem();
+  });
+
+  it('calls an imported, inherited pure function', async function(){
+    const instance = await TotallyPure.deployed();
+    const value = await instance.isPure(4,5);
+  });
+
+  it('calls an importend, inherited view function', async function(){
+    const instance = await TotallyPure.deployed();
+    const value = await instance.isView();
+  })
+
+  it('overrides an imported, inherited abstract pure function', async function(){
+    const instance = await TotallyPure.deployed();
+    const value = await instance.bePure(4,5);
+  })
+
+  it('overrides an imported, inherited abstract view function', async function(){
+    const instance = await TotallyPure.deployed();
+    const value = await instance.beView();
+  });
+});

test/sources/cli/PureView.sol

@@ -5,11 +5,7 @@ contract PureView {
   // Make sure we aren't corrupting anything with the replace
   uint notpureview = 5;
 
-  function isPure(uint a, uint b) pure returns (uint){
-    return a * b;
-  }
-
-  function isView() view returns (uint){
-    return notpureview;
-  }
+  // Abstract functions to inherit from an uninstrumented, imported file.
+  function bePure(uint a, uint b) pure returns (uint);
+  function beView() view returns (uint);
 }

test/sources/cli/TotallyPure.sol

@@ -0,0 +1,28 @@
+pragma experimental "v0.5.0";
+
+import "./../assets/PureView.sol";
+
+contract TotallyPure is PureView {
+  uint onehundred = 99;
+
+  function usesThem() {
+    uint y = isPure(1,2);
+    uint z = isView();
+  }
+
+  function isPure(uint a, uint b) pure returns (uint){
+    return a * b;
+  }
+
+  function isView() view returns (uint){
+    return notpureview;
+  }
+
+  function bePure(uint a, uint b) pure returns (uint) {
+    return a + b;
+  }
+
+  function beView() view returns (uint){
+    return onehundred;
+  }
+}

test/util/mockTruffle.js

@@ -39,7 +39,7 @@ module.exports.install = function install(contract, test, config, _trufflejs, _t
   `module.exports = {
     networks: {
       development: {
-        host: "localhost", 
+        host: "localhost",
         port: 8545,
         network_id: "*"
       }
@@ -68,6 +68,8 @@ module.exports.install = function install(contract, test, config, _trufflejs, _t
   fs.writeFileSync(`./mock/${trufflejsName}`, trufflejs);
   fs.writeFileSync('./mock/assets/asset.js', asset);
   fs.writeFileSync('./.solcover.js', configjs);
+
+  shell.cp(`./test/sources/cli/PureView.sol`, `./mock/assets/PureView.sol`);
   shell.cp(`./test/cli/${test}`, `./mock/test/${test}`);
 };
 
@@ -113,7 +115,7 @@ module.exports.installInheritanceTest = function installInheritanceTest(config)
   const trufflejs = `module.exports = {
                     networks: {
                       development: {
-                        host: "localhost", 
+                        host: "localhost",
                         port: 8545,
                         network_id: "*"
                       }}};`