Skip to content

Commit

Permalink
autofix payable-fallback added
Browse files Browse the repository at this point in the history
  • Loading branch information
dbale-altoros committed Dec 20, 2023
1 parent 80bb793 commit a29462c
Show file tree
Hide file tree
Showing 8 changed files with 227 additions and 4 deletions.
5 changes: 5 additions & 0 deletions e2e/08-autofix/payable-fallback/.solhint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"payable-fallback": "warn"
}
}
50 changes: 50 additions & 0 deletions e2e/08-autofix/payable-fallback/Foo1.sol
Original file line number Diff line number Diff line change
@@ -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 {}
}
50 changes: 50 additions & 0 deletions e2e/08-autofix/payable-fallback/Foo1AfterFix.sol
Original file line number Diff line number Diff line change
@@ -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 {}
}
50 changes: 50 additions & 0 deletions e2e/08-autofix/payable-fallback/Foo1BeforeFix.sol
Original file line number Diff line number Diff line change
@@ -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 {}
}
42 changes: 41 additions & 1 deletion e2e/autofix-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion lib/common/ast-types.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
function isFallbackFunction(node) {
return isFunctionDefinition(node) && node.isFallback
return isFunctionDefinition(node) && (node.isFallback || node.isReceiveEther)
}

function isReceiveFunction(node) {
Expand Down
21 changes: 19 additions & 2 deletions lib/rules/best-practises/payable-fallback.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const meta = {
isDefault: false,
recommended: true,
defaultSetup: 'warn',

fixable: true,
schema: null,
}

Expand All @@ -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
11 changes: 11 additions & 0 deletions test/rules/best-practises/payable-fallback.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
})

0 comments on commit a29462c

Please sign in to comment.