Skip to content

Commit aeb0255

Browse files
dbale-altorosdbale-altoros
authored
Merge pull request #528 from protofire/autofix-payable
autofix payable-fallback added
2 parents a281da1 + 1582fe4 commit aeb0255

File tree

8 files changed

+268
-26
lines changed

8 files changed

+268
-26
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"rules": {
3+
"payable-fallback": "error"
4+
}
5+
}

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
pragma solidity ^0.8.0;
3+
4+
contract Generic {
5+
6+
constructor() {}
7+
8+
function anyFunction() {}
9+
10+
//// fallback with receive
11+
receive() public {}
12+
13+
receive() external onlyOwner {}
14+
15+
receive() external virtual {
16+
uint256 anUintToFillSpace;
17+
}
18+
19+
//// fallback with no name
20+
function() public {}
21+
22+
function() {
23+
uint256 anUintToFillSpace;
24+
}
25+
26+
function() external onlyOwner {}
27+
28+
function() virtual {
29+
uint256 anUintToFillSpace;
30+
}
31+
32+
33+
//// fallback explicit
34+
fallback() {}
35+
36+
fallback() {
37+
uint256 anUintToFillSpace;
38+
}
39+
40+
fallback() external onlyOwner{
41+
uint256 anUintToFillSpace;
42+
}
43+
44+
fallback() virtual {}
45+
46+
47+
fallback() external payable {}
48+
function() external payable {}
49+
receive() public virtual payable {}
50+
}

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
pragma solidity ^0.8.0;
3+
4+
contract Generic {
5+
6+
constructor() {}
7+
8+
function anyFunction() {}
9+
10+
//// fallback with receive
11+
receive() payable public {}
12+
13+
receive() payable external onlyOwner {}
14+
15+
receive() payable external virtual {
16+
uint256 anUintToFillSpace;
17+
}
18+
19+
//// fallback with no name
20+
function() payable public {}
21+
22+
function() payable {
23+
uint256 anUintToFillSpace;
24+
}
25+
26+
function() payable external onlyOwner {}
27+
28+
function() payable virtual {
29+
uint256 anUintToFillSpace;
30+
}
31+
32+
33+
//// fallback explicit
34+
fallback() payable {}
35+
36+
fallback() payable {
37+
uint256 anUintToFillSpace;
38+
}
39+
40+
fallback() payable external onlyOwner{
41+
uint256 anUintToFillSpace;
42+
}
43+
44+
fallback() payable virtual {}
45+
46+
47+
fallback() external payable {}
48+
function() external payable {}
49+
receive() public virtual payable {}
50+
}

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
pragma solidity ^0.8.0;
3+
4+
contract Generic {
5+
6+
constructor() {}
7+
8+
function anyFunction() {}
9+
10+
//// fallback with receive
11+
receive() public {}
12+
13+
receive() external onlyOwner {}
14+
15+
receive() external virtual {
16+
uint256 anUintToFillSpace;
17+
}
18+
19+
//// fallback with no name
20+
function() public {}
21+
22+
function() {
23+
uint256 anUintToFillSpace;
24+
}
25+
26+
function() external onlyOwner {}
27+
28+
function() virtual {
29+
uint256 anUintToFillSpace;
30+
}
31+
32+
33+
//// fallback explicit
34+
fallback() {}
35+
36+
fallback() {
37+
uint256 anUintToFillSpace;
38+
}
39+
40+
fallback() external onlyOwner{
41+
uint256 anUintToFillSpace;
42+
}
43+
44+
fallback() virtual {}
45+
46+
47+
fallback() external payable {}
48+
function() external payable {}
49+
receive() public virtual payable {}
50+
}

‎e2e/autofix-test.js

Lines changed: 82 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ function retrieveParams(subpath) {
2525
function compareTextFiles(file1Path, file2Path) {
2626
const file1Content = fs.readFileSync(file1Path, 'utf-8')
2727
const file2Content = fs.readFileSync(file2Path, 'utf-8')
28-
2928
return file1Content === file2Content
3029
}
3130

@@ -51,6 +50,8 @@ function useFixture(dir) {
5150

5251
describe('e2e', function () {
5352
let result = false
53+
let code
54+
let stdout
5455

5556
describe('autofix tests', () => {
5657
if (E2E) {
@@ -160,28 +161,32 @@ describe('e2e', function () {
160161
}
161162
})
162163

163-
it('should compare Foo1 file with template BEFORE FIX file and they should match 2', () => {
164+
it('should compare Foo1 file with template BEFORE FIX file and they should match (2)', () => {
164165
result = compareTextFiles(currentFile, beforeFixFile)
165166
expect(result).to.be.true
166167
})
167168

168-
it('should compare Foo1 file with template AFTER FIX file and they should match 2', () => {
169-
const { code, stdout } = shell.exec(
169+
it('should execute and compare Foo1 with template AFTER FIX and they should match (2)', () => {
170+
({ code, stdout } = shell.exec(
170171
`${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt`
171-
)
172+
))
173+
174+
result = compareTextFiles(currentFile, afterFixFile)
175+
expect(result).to.be.true
176+
})
172177

178+
it('should execute and exit with code 1 (2)', () => {
173179
expect(code).to.equal(1)
180+
})
174181

182+
it('should get the right report (2)', () => {
175183
const reportLines = stdout.split('\n')
176184
const finalLine = '5 problems (5 errors, 0 warnings)'
177185
expect(reportLines[reportLines.length - 3]).to.contain(finalLine)
178-
179-
result = compareTextFiles(currentFile, afterFixFile)
180-
expect(result).to.be.true
181186
})
182187
})
183188

184-
it('should check FOO1 does not change after test 2', () => {
189+
it('should check FOO1 does not change after test (2)', () => {
185190
result = compareTextFiles(currentFile, beforeFixFile)
186191
expect(result).to.be.true
187192
})
@@ -202,28 +207,32 @@ describe('e2e', function () {
202207
}
203208
})
204209

205-
it('should compare Foo1 file with template BEFORE FIX file and they should match 3', () => {
210+
it('should compare Foo1 file with template BEFORE FIX file and they should match (3)', () => {
206211
result = compareTextFiles(currentFile, beforeFixFile)
207212
expect(result).to.be.true
208213
})
209214

210-
it('should compare Foo1 file with template AFTER FIX file and they should match 3', () => {
211-
const { code, stdout } = shell.exec(
215+
it('should execute and compare Foo1 with template AFTER FIX and they should match (3)', () => {
216+
({ code, stdout } = shell.exec(
212217
`${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt`
213-
)
218+
))
214219

220+
result = compareTextFiles(currentFile, afterFixFile)
221+
expect(result).to.be.true
222+
})
223+
224+
it('should execute and exit with code 1 (3)', () => {
215225
expect(code).to.equal(1)
226+
})
216227

228+
it('should get the right report (3)', () => {
217229
const reportLines = stdout.split('\n')
218230
const finalLine = '9 problems (9 errors, 0 warnings)'
219231
expect(reportLines[reportLines.length - 3]).to.contain(finalLine)
220-
221-
result = compareTextFiles(currentFile, afterFixFile)
222-
expect(result).to.be.true
223232
})
224233
})
225234

226-
it('should check FOO1 does not change after test 3', () => {
235+
it('should check FOO1 does not change after test (3)', () => {
227236
result = compareTextFiles(currentFile, beforeFixFile)
228237
expect(result).to.be.true
229238
})
@@ -244,33 +253,83 @@ describe('e2e', function () {
244253
}
245254
})
246255

247-
it('should compare Foo1 file with template BEFORE FIX file and they should match 4', () => {
256+
it('should compare Foo1 file with template BEFORE FIX file and they should match (4)', () => {
248257
result = compareTextFiles(currentFile, beforeFixFile)
249258
expect(result).to.be.true
250259
})
251260

252-
it('should compare Foo1 file with template AFTER FIX file and they should match 4', () => {
253-
const { code, stdout } = shell.exec(
261+
it('should execute and compare Foo1 with template AFTER FIX and they should match (4)', () => {
262+
({ code, stdout } = shell.exec(
254263
`${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt`
255-
)
264+
))
256265

266+
result = compareTextFiles(currentFile, afterFixFile)
267+
expect(result).to.be.true
268+
})
269+
270+
it('should execute and exit with code 1 (4)', () => {
257271
expect(code).to.equal(1)
272+
})
258273

274+
it('should get the right report (4)', () => {
259275
const reportLines = stdout.split('\n')
260276
const finalLine = '19 problems (19 errors, 0 warnings)'
261277
expect(reportLines[reportLines.length - 3]).to.contain(finalLine)
278+
})
279+
})
280+
281+
it('should check FOO1 does not change after test (4)', () => {
282+
result = compareTextFiles(currentFile, beforeFixFile)
283+
expect(result).to.be.true
284+
})
285+
})
286+
287+
describe('autofix rule: payable-fallback', () => {
288+
before(function () {
289+
params = retrieveParams('payable-fallback/')
290+
currentConfig = `${params.path}${params.subpath}.solhint.json`
291+
currentFile = `${params.path}${params.subpath}Foo1.sol`
292+
beforeFixFile = `${params.path}${params.subpath}Foo1BeforeFix.sol`
293+
afterFixFile = `${params.path}${params.subpath}Foo1AfterFix.sol`
294+
})
295+
describe('--fix with noPrompt', () => {
296+
after(function () {
297+
if (!E2E) {
298+
copyFile(beforeFixFile, currentFile)
299+
}
300+
})
301+
302+
it('should compare Foo1 file with template BEFORE FIX file and they should match (5)', () => {
303+
result = compareTextFiles(currentFile, beforeFixFile)
304+
expect(result).to.be.true
305+
})
306+
307+
it('should execute and compare Foo1 with template AFTER FIX and they should match (5)', () => {
308+
({ code, stdout } = shell.exec(
309+
`${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt`
310+
))
262311

263312
result = compareTextFiles(currentFile, afterFixFile)
264313
expect(result).to.be.true
265314
})
315+
316+
it('should execute and exit with code 1 (5)', () => {
317+
expect(code).to.equal(1)
318+
})
319+
320+
it('should get the right report (5)', () => {
321+
const reportLines = stdout.split('\n')
322+
const finalLine = '11 problems (11 errors, 0 warnings)'
323+
expect(reportLines[reportLines.length - 3]).to.contain(finalLine)
324+
})
266325
})
267-
it('should check FOO1 does not change after test 4', () => {
326+
327+
it('should check FOO1 does not change after test (5)', () => {
268328
result = compareTextFiles(currentFile, beforeFixFile)
269329
expect(result).to.be.true
270330
})
271331
})
272332
})
273333
})
274334

275-
// FALTA LA COMPARACION DEL FIX CON EL TEMPLATE FIX
276335
// FALTA LA PRUEBA DEL STORE TO FILE

‎lib/common/ast-types.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
function isFallbackFunction(node) {
2-
return isFunctionDefinition(node) && node.isFallback
2+
return isFunctionDefinition(node) && (node.isFallback || node.isReceiveEther)
33
}
44

55
function isReceiveFunction(node) {

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const meta = {
2727
isDefault: false,
2828
recommended: true,
2929
defaultSetup: 'warn',
30-
30+
fixable: true,
3131
schema: null,
3232
}
3333

@@ -39,10 +39,27 @@ class PayableFallbackChecker extends BaseChecker {
3939
FunctionDefinition(node) {
4040
if (isFallbackFunction(node)) {
4141
if (node.stateMutability !== 'payable') {
42-
this.warn(node, 'When fallback is not payable you will not be able to receive ether')
42+
this.warn(
43+
node,
44+
'Fallback should be external and payable to accept native currency',
45+
this.fixStatement(node)
46+
)
4347
}
4448
}
4549
}
50+
51+
fixStatement(node) {
52+
const range = node.range
53+
const stringToPut = ' payable '
54+
55+
if (node.isReceiveEther) {
56+
range[0] += 9
57+
} else {
58+
range[0] += 10
59+
}
60+
61+
return (fixer) => fixer.insertTextBeforeRange(range, stringToPut)
62+
}
4663
}
4764

4865
module.exports = PayableFallbackChecker

0 commit comments

Comments
 (0)