Merge pull request #531 from protofire/autofix-avoid-suicide

added: autofix avoid-suicide

docs/rules/best-practises/no-console.md

@@ -28,6 +28,8 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 }
 ```
 
+### Notes
+- Solhint allows this rule to automatically fix the code with `--fix` option
 
 ## Examples
 ### 👎 Examples of **incorrect** code for this rule

docs/rules/best-practises/payable-fallback.md

@@ -26,6 +26,8 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 }
 ```
 
+### Notes
+- Solhint allows this rule to automatically fix the code with `--fix` option
 
 ## Examples
 ### 👍 Examples of **correct** code for this rule

docs/rules/naming/private-vars-leading-underscore.md

@@ -34,6 +34,7 @@ This rule accepts an array of options:
 - This rule skips external and public functions
 - This rule skips external and public state variables
 - See [here](https://docs.soliditylang.org/en/latest/style-guide.html#underscore-prefix-for-non-external-functions-and-variables) for further information
+- Solhint allows this rule to automatically fix the code with `--fix` option
 
 ## Examples
 ### 👍 Examples of **correct** code for this rule

e2e/08-autofix/avoid-suicide/.solhint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "avoid-suicide": "error"
+  }
+}

e2e/08-autofix/avoid-suicide/Foo1.sol

@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.0;
+
+import '@openzeppelin/contracts/ownership/Suicide.sol';
+import { feature as suicide} from '@openzeppelin/contracts/ownership/Suicide.sol';
+
+uint256 constant suicide = 1;
+
+enum MyEnum {
+    suicide,
+    B
+}
+
+struct OneNiceStruct {
+    uint256 suicide;
+    uint256 b;
+}
+
+error Unauthorized();
+
+function freeFunction() pure returns (uint256) {
+    suicide();
+    return 1;
+}
+
+contract Generic {
+    struct AnotherNiceStruct {
+        uint256 suicide;
+        uint256 c;
+    }
+
+    address constant owner = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48";
+
+    function seekAndestroy1() external {
+        suicide();
+    }    
+
+    function seekAndestroy2() external {
+        suicide(owner);
+    }
+
+    function seekAndestroy3() external {
+        selfdestruct();
+    }
+
+    function seekAndestroy4() external {
+        selfdestruct(owner);
+    }
+}

e2e/08-autofix/avoid-suicide/Foo1AfterFix.sol

@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.0;
+
+import '@openzeppelin/contracts/ownership/Suicide.sol';
+import { feature as suicide} from '@openzeppelin/contracts/ownership/Suicide.sol';
+
+uint256 constant suicide = 1;
+
+enum MyEnum {
+    suicide,
+    B
+}
+
+struct OneNiceStruct {
+    uint256 suicide;
+    uint256 b;
+}
+
+error Unauthorized();
+
+function freeFunction() pure returns (uint256) {
+    selfdestruct();
+    return 1;
+}
+
+contract Generic {
+    struct AnotherNiceStruct {
+        uint256 suicide;
+        uint256 c;
+    }
+
+    address constant owner = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48";
+
+    function seekAndestroy1() external {
+        selfdestruct();
+    }    
+
+    function seekAndestroy2() external {
+        selfdestruct(owner);
+    }
+
+    function seekAndestroy3() external {
+        selfdestruct();
+    }
+
+    function seekAndestroy4() external {
+        selfdestruct(owner);
+    }
+}

e2e/08-autofix/avoid-suicide/Foo1BeforeFix.sol

@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.0;
+
+import '@openzeppelin/contracts/ownership/Suicide.sol';
+import { feature as suicide} from '@openzeppelin/contracts/ownership/Suicide.sol';
+
+uint256 constant suicide = 1;
+
+enum MyEnum {
+    suicide,
+    B
+}
+
+struct OneNiceStruct {
+    uint256 suicide;
+    uint256 b;
+}
+
+error Unauthorized();
+
+function freeFunction() pure returns (uint256) {
+    suicide();
+    return 1;
+}
+
+contract Generic {
+    struct AnotherNiceStruct {
+        uint256 suicide;
+        uint256 c;
+    }
+
+    address constant owner = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48";
+
+    function seekAndestroy1() external {
+        suicide();
+    }    
+
+    function seekAndestroy2() external {
+        suicide(owner);
+    }
+
+    function seekAndestroy3() external {
+        selfdestruct();
+    }
+
+    function seekAndestroy4() external {
+        selfdestruct(owner);
+    }
+}

e2e/autofix-test.js

@@ -60,7 +60,7 @@ describe('e2e', function () {
 
     describe('autofix command line options', () => {
       before(function () {
-        params = retrieveParams('commands/')
+        params = retrieveParams('_commands/')
         currentConfig = `${params.path}${params.subpath}.solhint.json`
         currentFile = `${params.path}${params.subpath}Foo1.sol`
         beforeFixFile = `${params.path}${params.subpath}Foo1BeforeFix.sol`
@@ -167,7 +167,7 @@ describe('e2e', function () {
         })
 
         it('should execute and compare Foo1 with template AFTER FIX and they should match (2)', () => {
-          ({ code, stdout } = shell.exec(
+          ;({ code, stdout } = shell.exec(
             `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt`
           ))
 
@@ -213,7 +213,7 @@ describe('e2e', function () {
         })
 
         it('should execute and compare Foo1 with template AFTER FIX and they should match (3)', () => {
-          ({ code, stdout } = shell.exec(
+          ;({ code, stdout } = shell.exec(
             `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt`
           ))
 
@@ -259,7 +259,7 @@ describe('e2e', function () {
         })
 
         it('should execute and compare Foo1 with template AFTER FIX and they should match (4)', () => {
-          ({ code, stdout } = shell.exec(
+          ;({ code, stdout } = shell.exec(
             `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt`
           ))
 
@@ -305,7 +305,7 @@ describe('e2e', function () {
         })
 
         it('should execute and compare Foo1 with template AFTER FIX and they should match (5)', () => {
-          ({ code, stdout } = shell.exec(
+          ;({ code, stdout } = shell.exec(
             `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt`
           ))
 
@@ -352,7 +352,7 @@ describe('e2e', function () {
         })
 
         it('should execute and compare Foo1 with template AFTER FIX and they should match (6)', () => {
-          ({ code, stdout } = shell.exec(
+          ;({ code, stdout } = shell.exec(
             `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt`
           ))
 
@@ -392,7 +392,7 @@ describe('e2e', function () {
         })
 
         it('should execute and compare Foo1 with template AFTER FIX and they should match (6)', () => {
-          ({ code, stdout } = shell.exec(
+          ;({ code, stdout } = shell.exec(
             `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt`
           ))
 
@@ -417,6 +417,52 @@ describe('e2e', function () {
       })
     })
   })
+
+  describe('autofix rule: avoid-suicide', () => {
+    before(function () {
+      params = retrieveParams('avoid-suicide/')
+      currentConfig = `${params.path}${params.subpath}.solhint.json`
+      currentFile = `${params.path}${params.subpath}Foo1.sol`
+      beforeFixFile = `${params.path}${params.subpath}Foo1BeforeFix.sol`
+      afterFixFile = `${params.path}${params.subpath}Foo1AfterFix.sol`
+    })
+    after(function () {
+      if (!E2E) {
+        copyFile(beforeFixFile, currentFile)
+      }
+    })
+
+    describe('--fix with noPrompt', () => {
+      it('should compare Foo1 file with template BEFORE FIX file and they should match (7)', () => {
+        result = compareTextFiles(currentFile, beforeFixFile)
+        expect(result).to.be.true
+      })
+
+      it('should execute and compare Foo1 with template AFTER FIX and they should match (7)', () => {
+        ;({ code, stdout } = shell.exec(
+          `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt`
+        ))
+
+        result = compareTextFiles(currentFile, afterFixFile)
+        expect(result).to.be.true
+      })
+
+      it('should execute and exit with code 1 (7)', () => {
+        expect(code).to.equal(1)
+      })
+
+      it('should get the right report (7)', () => {
+        const reportLines = stdout.split('\n')
+        const finalLine = '3 problems (3 errors, 0 warnings)'
+        expect(reportLines[reportLines.length - 3]).to.contain(finalLine)
+      })
+    })
+
+    it('should check FOO1 does not change after test (7)', () => {
+      result = compareTextFiles(currentFile, beforeFixFile)
+      expect(result).to.be.true
+    })
+  })
 })
 
 // FALTA LA PRUEBA DEL STORE TO FILE

lib/rules/best-practises/no-console.js

@@ -23,6 +23,11 @@ const meta = {
         },
       ],
     },
+    notes: [
+      {
+        note: 'Solhint allows this rule to automatically fix the code with `--fix` option',
+      },
+    ],
   },
 
   isDefault: true,

lib/rules/best-practises/payable-fallback.js

@@ -22,6 +22,11 @@ const meta = {
         },
       ],
     },
+    notes: [
+      {
+        note: 'Solhint allows this rule to automatically fix the code with `--fix` option',
+      },
+    ],
   },
 
   isDefault: false,

lib/rules/naming/private-vars-leading-underscore.js

@@ -78,6 +78,9 @@ const meta = {
       {
         note: 'See [here](https://docs.soliditylang.org/en/latest/style-guide.html#underscore-prefix-for-non-external-functions-and-variables) for further information',
       },
+      {
+        note: 'Solhint allows this rule to automatically fix the code with `--fix` option',
+      },
     ],
   },
 

lib/rules/security/avoid-suicide.js

@@ -12,7 +12,7 @@ const meta = {
   isDefault: false,
   recommended: true,
   defaultSetup: 'warn',
-
+  fixable: true,
   schema: null,
 }
 
@@ -21,10 +21,24 @@ class AvoidSuicideChecker extends BaseChecker {
     super(reporter, ruleId, meta)
   }
 
-  Identifier(node) {
-    if (node.name === 'suicide') {
-      this.error(node, 'Use "selfdestruct" instead of deprecated "suicide"')
+  FunctionCall(node) {
+    if (node.expression.type === 'Identifier' && node.expression.name === 'suicide') {
+      this.error(
+        node,
+        'Use "selfdestruct" instead of deprecated "suicide"',
+        this.fixStatement(node)
+      )
+    }
+  }
+
+  fixStatement(node) {
+    let stringToPut = 'selfdestruct()'
+
+    if (node.arguments.length > 0) {
+      stringToPut = 'selfdestruct(' + node.arguments[0].name + ')'
     }
+
+    return (fixer) => fixer.replaceTextRange(node.range, stringToPut)
   }
 }