Apply fixes and report remaining issues

lib/apply-fixes.js

@@ -0,0 +1,31 @@
+function applyFixes(fixes, inputSrc) {
+  if (fixes.length === 0) {
+    return {
+      fixed: false
+    }
+  }
+
+  let output = ''
+
+  let i = 0
+  let fixIndex = 0
+
+  while (i < inputSrc.length) {
+    if (fixIndex < fixes.length) {
+      output += inputSrc.slice(i, fixes[fixIndex].range[0])
+      output += fixes[fixIndex].text
+      i = fixes[fixIndex].range[1]
+      fixIndex += 1
+    } else {
+      output += inputSrc.slice(i)
+      break
+    }
+  }
+
+  return {
+    fixed: true,
+    output
+  }
+}
+
+module.exports = applyFixes

lib/rule-fixer.js

@@ -1,3 +1,5 @@
+// taken from eslint (https://github.com/eslint/eslint)
+
 /**
  * @fileoverview An object that creates fix commands for rules.
  * @author Nicholas C. Zakas

lib/rules/security/avoid-throw.js

@@ -23,9 +23,9 @@ class AvoidThrowChecker extends BaseChecker {
   }
 
   ThrowStatement(node) {
-    this.error(node, '"throw" is deprecated, avoid to use it', fixer => {
-      return [fixer.replaceTextRange(node.range, 'revert()')]
-    })
+    this.error(node, '"throw" is deprecated, avoid to use it', fixer =>
+      fixer.replaceTextRange(node.range, 'revert()')
+    )
   }
 }
 

solhint.js

@@ -8,8 +8,9 @@ const process = require('process')
 const linter = require('./lib/index')
 const { applyExtends, loadConfig } = require('./lib/config/config-file')
 const { validate } = require('./lib/config/config-validator')
-const packageJson = require('./package.json')
+const applyFixes = require('./lib/apply-fixes')
 const ruleFixer = require('./lib/rule-fixer')
+const packageJson = require('./package.json')
 
 function init() {
   const version = packageJson.version
@@ -44,12 +45,6 @@ function init() {
   }
 }
 
-function applyFix(report) {
-  if (typeof report.fix === 'function') {
-    report.fix(ruleFixer)
-  }
-}
-
 function execMainAction() {
   let formatterFn
 
@@ -67,13 +62,21 @@ function execMainAction() {
   const warningsNumberExceeded = program.maxWarnings >= 0 && warningsCount > program.maxWarnings
 
   if (program.fix) {
-    const fixableRules = reports[0].reports.filter(r => r.fix !== null)
-
-    // filter the list of reports, to report those who are not fixable.
-    reports[0].reports = reports[0].reports.filter(r => r.fix === null)
-
-    // fixes those fixable rules
-    fixableRules.forEach(applyFix)
+    for (const report of reports) {
+      const inputSrc = fs.readFileSync(report.filePath).toString()
+
+      const fixes = _(report.reports)
+        .filter(x => x.fix)
+        .map(x => x.fix(ruleFixer))
+        .sort((a, b) => a.range[0] - b.range[0])
+        .value()
+
+      const { fixed, output } = applyFixes(fixes, inputSrc)
+      if (fixed) {
+        report.reports = report.reports.filter(x => !x.fix)
+        fs.writeFileSync(report.filePath, output)
+      }
+    }
   }
 
   if (program.quiet) {
@@ -172,10 +175,6 @@ function processPath(path) {
   return linter.processPath(path, readConfig())
 }
 
-function processReports(reports, formatter) {
-  printReports(reports, formatter)
-}
-
 function printReports(reports, formatter) {
   console.log(formatter(reports))
   return reports

test/common/apply-fixes.js

@@ -0,0 +1,78 @@
+const assert = require('assert')
+
+const applyFixes = require('../../lib/apply-fixes')
+
+describe('applyFixes', () => {
+  it('should work when there are no reports', () => {
+    const inputSrc = 'contract Foo {}'
+    const fixes = []
+
+    const { fixed } = applyFixes(fixes, inputSrc)
+
+    assert.equal(fixed, false)
+  })
+
+  it('should work for a single replace', () => {
+    const inputSrc = `
+contract Foo {
+  function foo() {
+    throw;
+  }
+}`.trim()
+
+    const fixes = [
+      {
+        range: [38, 43],
+        text: 'revert()'
+      }
+    ]
+
+    const { fixed, output } = applyFixes(fixes, inputSrc)
+
+    assert.equal(fixed, true)
+    assert.equal(
+      output,
+      `
+contract Foo {
+  function foo() {
+    revert();
+  }
+}`.trim()
+    )
+  })
+
+  it('should work for two fixes', () => {
+    const inputSrc = `
+contract Foo {
+  function foo() {
+    throw;
+    throw;
+  }
+}`.trim()
+
+    const fixes = [
+      {
+        range: [38, 43],
+        text: 'revert()'
+      },
+      {
+        range: [49, 54],
+        text: 'revert()'
+      }
+    ]
+
+    const { fixed, output } = applyFixes(fixes, inputSrc)
+
+    assert.equal(fixed, true)
+    assert.equal(
+      output,
+      `
+contract Foo {
+  function foo() {
+    revert();
+    revert();
+  }
+}`.trim()
+    )
+  })
+})

test/rules/security/mark-callable-contracts.js

@@ -101,6 +101,21 @@ describe('Linter - mark-callable-contracts', () => {
       rules: { 'mark-callable-contracts': 'warn' }
     })
 
+    assert.equal(report.warningCount, 0)
+  })
+  it('should not return error for an event defined after function', () => {
+    const code = contractWith(`
+      function b() public {
+        emit UpdatedToken();
+      }
+
+      event UpdatedToken();
+    `)
+
+    const report = linter.processStr(code, {
+      rules: { 'mark-callable-contracts': 'warn' }
+    })
+
     assert.equal(report.warningCount, 0)
   })
 })