Repository: protofire/solhint
Pull request #531 (2023-12-21 11:24:34)

added: autofix avoid-suicide


Patch:
diff --git a/docs/rules/best-practises/no-console.md b/docs/rules/best-practises/no-console.md
index da6676e..84f6549 100644
--- a/docs/rules/best-practises/no-console.md
+++ b/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
diff --git a/docs/rules/best-practises/payable-fallback.md b/docs/rules/best-practises/payable-fallback.md
index 4350d04..be0f61f 100644
--- a/docs/rules/best-practises/payable-fallback.md
+++ b/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
diff --git a/docs/rules/naming/private-vars-leading-underscore.md b/docs/rules/naming/private-vars-leading-underscore.md
index d3dea4b..a638d31 100644
--- a/docs/rules/naming/private-vars-leading-underscore.md
+++ b/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
diff --git a/lib/rules/best-practises/no-console.js b/lib/rules/best-practises/no-console.js
index c6156fb..1a8a0fa 100644
--- a/lib/rules/best-practises/no-console.js
+++ b/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,
diff --git a/lib/rules/best-practises/payable-fallback.js b/lib/rules/best-practises/payable-fallback.js
index 723925a..473c7c6 100644
--- a/lib/rules/best-practises/payable-fallback.js
+++ b/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,
diff --git a/lib/rules/naming/private-vars-leading-underscore.js b/lib/rules/naming/private-vars-leading-underscore.js
index f172cab..e630e20 100644
--- a/lib/rules/naming/private-vars-leading-underscore.js
+++ b/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',
+      },
     ],
   },
 
diff --git a/lib/rules/security/avoid-suicide.js b/lib/rules/security/avoid-suicide.js
index 4efabb9..2fdb5ed 100644
--- a/lib/rules/security/avoid-suicide.js
+++ b/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)
   }
 }
 


Test patch:
diff --git a/e2e/08-autofix/commands/.solhint.json b/e2e/08-autofix/_commands/.solhint.json
similarity index 100%
rename from e2e/08-autofix/commands/.solhint.json
rename to e2e/08-autofix/_commands/.solhint.json
diff --git a/e2e/08-autofix/commands/Foo1.sol b/e2e/08-autofix/_commands/Foo1.sol
similarity index 100%
rename from e2e/08-autofix/commands/Foo1.sol
rename to e2e/08-autofix/_commands/Foo1.sol
diff --git a/e2e/08-autofix/commands/Foo1AfterFix.sol b/e2e/08-autofix/_commands/Foo1AfterFix.sol
similarity index 100%
rename from e2e/08-autofix/commands/Foo1AfterFix.sol
rename to e2e/08-autofix/_commands/Foo1AfterFix.sol
diff --git a/e2e/08-autofix/commands/Foo1BeforeFix.sol b/e2e/08-autofix/_commands/Foo1BeforeFix.sol
similarity index 100%
rename from e2e/08-autofix/commands/Foo1BeforeFix.sol
rename to e2e/08-autofix/_commands/Foo1BeforeFix.sol
diff --git a/e2e/08-autofix/avoid-suicide/.solhint.json b/e2e/08-autofix/avoid-suicide/.solhint.json
new file mode 100644
index 0000000..7f54d63
--- /dev/null
+++ b/e2e/08-autofix/avoid-suicide/.solhint.json
@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "avoid-suicide": "error"
+  }
+}
diff --git a/e2e/08-autofix/avoid-suicide/Foo1.sol b/e2e/08-autofix/avoid-suicide/Foo1.sol
new file mode 100644
index 0000000..cf6b1ad
--- /dev/null
+++ b/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);
+    }
+}
diff --git a/e2e/08-autofix/avoid-suicide/Foo1AfterFix.sol b/e2e/08-autofix/avoid-suicide/Foo1AfterFix.sol
new file mode 100644
index 0000000..9ccba15
--- /dev/null
+++ b/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);
+    }
+}
diff --git a/e2e/08-autofix/avoid-suicide/Foo1BeforeFix.sol b/e2e/08-autofix/avoid-suicide/Foo1BeforeFix.sol
new file mode 100644
index 0000000..cf6b1ad
--- /dev/null
+++ b/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);
+    }
+}
diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js
index b0bc60f..9a7227b 100644
--- a/e2e/autofix-test.js
+++ b/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
diff --git a/test/rules/security/avoid-suicide.js b/test/rules/security/avoid-suicide.js
index 3b84e3e..895af02 100644
--- a/test/rules/security/avoid-suicide.js
+++ b/test/rules/security/avoid-suicide.js
@@ -1,6 +1,7 @@
 const assert = require('assert')
 const linter = require('../../../lib/index')
 const funcWith = require('../../common/contract-builder').funcWith
+const contractWith = require('../../common/contract-builder').contractWith
 
 describe('Linter - avoid-suicide', () => {
   const DEPRECATION_ERRORS = ['suicide();']
@@ -18,7 +19,7 @@ describe('Linter - avoid-suicide', () => {
     })
   )
 
-  const ALMOST_DEPRECATION_ERRORS = ['suicides();']
+  const ALMOST_DEPRECATION_ERRORS = ['suicides();', 'selfdestruct();']
 
   ALMOST_DEPRECATION_ERRORS.forEach((curText) =>
     it(`should not return error when doing ${curText}`, () => {
@@ -31,4 +32,14 @@ describe('Linter - avoid-suicide', () => {
       assert.equal(report.errorCount, 0)
     })
   )
+
+  it(`should not return error when doing for struct name suicide`, () => {
+    const code = contractWith('struct AnotherNiceStruct { uint256 suicide; uint256 c; }')
+
+    const report = linter.processStr(code, {
+      rules: { 'avoid-suicide': 'error' },
+    })
+
+    assert.equal(report.errorCount, 0)
+  })
 })