From a29462cf14ea8fd4991aeb800dae7e48ad2d16ec Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Wed, 20 Dec 2023 10:31:35 -0300 Subject: [PATCH] autofix payable-fallback added --- e2e/08-autofix/payable-fallback/.solhint.json | 5 ++ e2e/08-autofix/payable-fallback/Foo1.sol | 50 +++++++++++++++++++ .../payable-fallback/Foo1AfterFix.sol | 50 +++++++++++++++++++ .../payable-fallback/Foo1BeforeFix.sol | 50 +++++++++++++++++++ e2e/autofix-test.js | 42 +++++++++++++++- lib/common/ast-types.js | 2 +- lib/rules/best-practises/payable-fallback.js | 21 +++++++- test/rules/best-practises/payable-fallback.js | 11 ++++ 8 files changed, 227 insertions(+), 4 deletions(-) create mode 100644 e2e/08-autofix/payable-fallback/.solhint.json create mode 100644 e2e/08-autofix/payable-fallback/Foo1.sol create mode 100644 e2e/08-autofix/payable-fallback/Foo1AfterFix.sol create mode 100644 e2e/08-autofix/payable-fallback/Foo1BeforeFix.sol diff --git a/e2e/08-autofix/payable-fallback/.solhint.json b/e2e/08-autofix/payable-fallback/.solhint.json new file mode 100644 index 00000000..3dd72e68 --- /dev/null +++ b/e2e/08-autofix/payable-fallback/.solhint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "payable-fallback": "warn" + } +} diff --git a/e2e/08-autofix/payable-fallback/Foo1.sol b/e2e/08-autofix/payable-fallback/Foo1.sol new file mode 100644 index 00000000..eb1cfdaf --- /dev/null +++ b/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 {} +} diff --git a/e2e/08-autofix/payable-fallback/Foo1AfterFix.sol b/e2e/08-autofix/payable-fallback/Foo1AfterFix.sol new file mode 100644 index 00000000..95d55746 --- /dev/null +++ b/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 {} + + 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() external virtual payable {} +} diff --git a/e2e/08-autofix/payable-fallback/Foo1BeforeFix.sol b/e2e/08-autofix/payable-fallback/Foo1BeforeFix.sol new file mode 100644 index 00000000..60b2f9c4 --- /dev/null +++ b/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 {} +} diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index 5579f83e..d24e2508 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -269,8 +269,48 @@ describe('e2e', function () { expect(result).to.be.true }) }) + + describe('autofix rule: payable-fallback', () => { + before(function () { + params = retrieveParams('private-vars-underscore/') + 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 compare Foo1 file with template AFTER FIX file and they should match 5', () => { + const { code, stdout } = shell.exec( + `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt` + ) + + expect(code).to.equal(1) + + const reportLines = stdout.split('\n') + const finalLine = '11 problems (0 errors, 11 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 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 diff --git a/lib/common/ast-types.js b/lib/common/ast-types.js index 0bdae5ac..d2fbe4d4 100644 --- a/lib/common/ast-types.js +++ b/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) { diff --git a/lib/rules/best-practises/payable-fallback.js b/lib/rules/best-practises/payable-fallback.js index 79921a92..723925ad 100644 --- a/lib/rules/best-practises/payable-fallback.js +++ b/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 diff --git a/test/rules/best-practises/payable-fallback.js b/test/rules/best-practises/payable-fallback.js index 734c551b..281b7768 100644 --- a/test/rules/best-practises/payable-fallback.js +++ b/test/rules/best-practises/payable-fallback.js @@ -33,4 +33,15 @@ describe('Linter - payable-fallback', () => { assertNoWarnings(report) }) + + it('should raise for other fallback types when are not payable', () => { + const code = contractWith('fallback() external {} receive() onlyOwner {}') + + const report = linter.processStr(code, { + rules: { 'payable-fallback': 'warn' }, + }) + + assertWarnsCount(report, 2) + assertErrorMessage(report, 'payable') + }) })