autofix payable-fallback added

e2e/08-autofix/payable-fallback/.solhint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "payable-fallback": "error"
+  }
+}

e2e/08-autofix/payable-fallback/Foo1.sol

@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.0;
+
+contract Generic {
+
+    constructor()  {}
+
+    function anyFunction() {}
+
+    //// fallback with receive
+    receive() public {}
+
+    receive() external onlyOwner {}
+
+    receive() external virtual {
+        uint256 anUintToFillSpace;
+    }
+
+    //// fallback with no name
+    function() public {}
+
+    function() {
+        uint256 anUintToFillSpace;
+    }
+
+    function() external onlyOwner {}
+
+    function() virtual {
+        uint256 anUintToFillSpace;
+    }
+
+
+    //// fallback explicit
+    fallback() {}
+
+    fallback() {
+        uint256 anUintToFillSpace;
+    }
+
+    fallback() external onlyOwner{
+        uint256 anUintToFillSpace;
+    }
+
+    fallback() virtual {}
+
+
+    fallback() external payable {}
+    function() external payable {}    
+    receive() public virtual payable {}
+}

e2e/08-autofix/payable-fallback/Foo1AfterFix.sol

@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.0;
+
+contract Generic {
+
+    constructor()  {}
+
+    function anyFunction() {}
+
+    //// fallback with receive
+    receive() payable public {}
+
+    receive() payable external onlyOwner {}
+
+    receive() payable external virtual {
+        uint256 anUintToFillSpace;
+    }
+
+    //// fallback with no name
+    function() payable public {}
+
+    function() payable {
+        uint256 anUintToFillSpace;
+    }
+
+    function() payable external onlyOwner {}
+
+    function() payable virtual {
+        uint256 anUintToFillSpace;
+    }
+
+
+    //// fallback explicit
+    fallback() payable {}
+
+    fallback() payable {
+        uint256 anUintToFillSpace;
+    }
+
+    fallback() payable external onlyOwner{
+        uint256 anUintToFillSpace;
+    }
+
+    fallback() payable virtual {}
+
+
+    fallback() external payable {}
+    function() external payable {}    
+    receive() public virtual payable {}
+}

e2e/08-autofix/payable-fallback/Foo1BeforeFix.sol

@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.0;
+
+contract Generic {
+
+    constructor()  {}
+
+    function anyFunction() {}
+
+    //// fallback with receive
+    receive() public {}
+
+    receive() external onlyOwner {}
+
+    receive() external virtual {
+        uint256 anUintToFillSpace;
+    }
+
+    //// fallback with no name
+    function() public {}
+
+    function() {
+        uint256 anUintToFillSpace;
+    }
+
+    function() external onlyOwner {}
+
+    function() virtual {
+        uint256 anUintToFillSpace;
+    }
+
+
+    //// fallback explicit
+    fallback() {}
+
+    fallback() {
+        uint256 anUintToFillSpace;
+    }
+
+    fallback() external onlyOwner{
+        uint256 anUintToFillSpace;
+    }
+
+    fallback() virtual {}
+
+
+    fallback() external payable {}
+    function() external payable {}    
+    receive() public virtual payable {}
+}

e2e/autofix-test.js

@@ -25,7 +25,6 @@ function retrieveParams(subpath) {
 function compareTextFiles(file1Path, file2Path) {
   const file1Content = fs.readFileSync(file1Path, 'utf-8')
   const file2Content = fs.readFileSync(file2Path, 'utf-8')
-
   return file1Content === file2Content
 }
 
@@ -51,6 +50,8 @@ function useFixture(dir) {
 
 describe('e2e', function () {
   let result = false
+  let code
+  let stdout
 
   describe('autofix tests', () => {
     if (E2E) {
@@ -160,28 +161,32 @@ describe('e2e', function () {
           }
         })
 
-        it('should compare Foo1 file with template BEFORE FIX file and they should match 2', () => {
+        it('should compare Foo1 file with template BEFORE FIX file and they should match (2)', () => {
           result = compareTextFiles(currentFile, beforeFixFile)
           expect(result).to.be.true
         })
 
-        it('should compare Foo1 file with template AFTER FIX file and they should match 2', () => {
-          const { code, stdout } = shell.exec(
+        it('should execute and compare Foo1 with template AFTER FIX and they should match (2)', () => {
+          ({ 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 (2)', () => {
           expect(code).to.equal(1)
+        })
 
+        it('should get the right report (2)', () => {
           const reportLines = stdout.split('\n')
           const finalLine = '5 problems (5 errors, 0 warnings)'
           expect(reportLines[reportLines.length - 3]).to.contain(finalLine)
-
-          result = compareTextFiles(currentFile, afterFixFile)
-          expect(result).to.be.true
         })
       })
 
-      it('should check FOO1 does not change after test 2', () => {
+      it('should check FOO1 does not change after test (2)', () => {
         result = compareTextFiles(currentFile, beforeFixFile)
         expect(result).to.be.true
       })
@@ -202,28 +207,32 @@ describe('e2e', function () {
           }
         })
 
-        it('should compare Foo1 file with template BEFORE FIX file and they should match 3', () => {
+        it('should compare Foo1 file with template BEFORE FIX file and they should match (3)', () => {
           result = compareTextFiles(currentFile, beforeFixFile)
           expect(result).to.be.true
         })
 
-        it('should compare Foo1 file with template AFTER FIX file and they should match 3', () => {
-          const { code, stdout } = shell.exec(
+        it('should execute and compare Foo1 with template AFTER FIX and they should match (3)', () => {
+          ({ 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 (3)', () => {
           expect(code).to.equal(1)
+        })
 
+        it('should get the right report (3)', () => {
           const reportLines = stdout.split('\n')
           const finalLine = '9 problems (9 errors, 0 warnings)'
           expect(reportLines[reportLines.length - 3]).to.contain(finalLine)
-
-          result = compareTextFiles(currentFile, afterFixFile)
-          expect(result).to.be.true
         })
       })
 
-      it('should check FOO1 does not change after test 3', () => {
+      it('should check FOO1 does not change after test (3)', () => {
         result = compareTextFiles(currentFile, beforeFixFile)
         expect(result).to.be.true
       })
@@ -244,33 +253,83 @@ describe('e2e', function () {
           }
         })
 
-        it('should compare Foo1 file with template BEFORE FIX file and they should match 4', () => {
+        it('should compare Foo1 file with template BEFORE FIX file and they should match (4)', () => {
           result = compareTextFiles(currentFile, beforeFixFile)
           expect(result).to.be.true
         })
 
-        it('should compare Foo1 file with template AFTER FIX file and they should match 4', () => {
-          const { code, stdout } = shell.exec(
+        it('should execute and compare Foo1 with template AFTER FIX and they should match (4)', () => {
+          ({ 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 (4)', () => {
           expect(code).to.equal(1)
+        })
 
+        it('should get the right report (4)', () => {
           const reportLines = stdout.split('\n')
           const finalLine = '19 problems (19 errors, 0 warnings)'
           expect(reportLines[reportLines.length - 3]).to.contain(finalLine)
+        })
+      })
+
+      it('should check FOO1 does not change after test (4)', () => {
+        result = compareTextFiles(currentFile, beforeFixFile)
+        expect(result).to.be.true
+      })
+    })
+
+    describe('autofix rule: payable-fallback', () => {
+      before(function () {
+        params = retrieveParams('payable-fallback/')
+        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`
+      })
+      describe('--fix with noPrompt', () => {
+        after(function () {
+          if (!E2E) {
+            copyFile(beforeFixFile, currentFile)
+          }
+        })
+
+        it('should compare Foo1 file with template BEFORE FIX file and they should match (5)', () => {
+          result = compareTextFiles(currentFile, beforeFixFile)
+          expect(result).to.be.true
+        })
+
+        it('should execute and compare Foo1 with template AFTER FIX and they should match (5)', () => {
+          ({ 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 (5)', () => {
+          expect(code).to.equal(1)
+        })
+
+        it('should get the right report (5)', () => {
+          const reportLines = stdout.split('\n')
+          const finalLine = '11 problems (11 errors, 0 warnings)'
+          expect(reportLines[reportLines.length - 3]).to.contain(finalLine)
+        })
       })
-      it('should check FOO1 does not change after test 4', () => {
+
+      it('should check FOO1 does not change after test (5)', () => {
         result = compareTextFiles(currentFile, beforeFixFile)
         expect(result).to.be.true
       })
     })
   })
 })
 
-// FALTA LA COMPARACION DEL FIX CON EL TEMPLATE FIX
 // FALTA LA PRUEBA DEL STORE TO FILE

lib/common/ast-types.js

@@ -1,5 +1,5 @@
 function isFallbackFunction(node) {
-  return isFunctionDefinition(node) && node.isFallback
+  return isFunctionDefinition(node) && (node.isFallback || node.isReceiveEther)
 }
 
 function isReceiveFunction(node) {

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

@@ -27,7 +27,7 @@ const meta = {
   isDefault: false,
   recommended: true,
   defaultSetup: 'warn',
-
+  fixable: true,
   schema: null,
 }
 
@@ -39,10 +39,27 @@ class PayableFallbackChecker extends BaseChecker {
   FunctionDefinition(node) {
     if (isFallbackFunction(node)) {
       if (node.stateMutability !== 'payable') {
-        this.warn(node, 'When fallback is not payable you will not be able to receive ether')
+        this.warn(
+          node,
+          'Fallback should be external and payable to accept native currency',
+          this.fixStatement(node)
+        )
       }
     }
   }
+
+  fixStatement(node) {
+    const range = node.range
+    const stringToPut = ' payable '
+
+    if (node.isReceiveEther) {
+      range[0] += 9
+    } else {
+      range[0] += 10
+    }
+
+    return (fixer) => fixer.insertTextBeforeRange(range, stringToPut)
+  }
 }
 
 module.exports = PayableFallbackChecker