From e505b3ed1d0025f2e6f9f4b07d27698a43eb78ac Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 25 Dec 2023 19:02:33 -0300 Subject: [PATCH 1/5] autofix: contract-name-camelcase --- .../contract-name-camelcase/.solhint.json | 5 ++ .../contract-name-camelcase/Foo1.sol | 61 +++++++++++++++++++ .../contract-name-camelcase/Foo1AfterFix.sol | 61 +++++++++++++++++++ .../contract-name-camelcase/Foo1BeforeFix.sol | 61 +++++++++++++++++++ e2e/autofix-test.js | 48 ++++++++++++++- lib/rules/naming/contract-name-camelcase.js | 52 +++++++++++++--- 6 files changed, 278 insertions(+), 10 deletions(-) create mode 100644 e2e/08-autofix/contract-name-camelcase/.solhint.json create mode 100644 e2e/08-autofix/contract-name-camelcase/Foo1.sol create mode 100644 e2e/08-autofix/contract-name-camelcase/Foo1AfterFix.sol create mode 100644 e2e/08-autofix/contract-name-camelcase/Foo1BeforeFix.sol diff --git a/e2e/08-autofix/contract-name-camelcase/.solhint.json b/e2e/08-autofix/contract-name-camelcase/.solhint.json new file mode 100644 index 00000000..6da8ad19 --- /dev/null +++ b/e2e/08-autofix/contract-name-camelcase/.solhint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "contract-name-camelcase": "error" + } +} diff --git a/e2e/08-autofix/contract-name-camelcase/Foo1.sol b/e2e/08-autofix/contract-name-camelcase/Foo1.sol new file mode 100644 index 00000000..2cf3e9bd --- /dev/null +++ b/e2e/08-autofix/contract-name-camelcase/Foo1.sol @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +struct not_Used_Struct1 { + uint256 three; + uint256 four; +} + +enum __status1_ { + Pending, + Approved, + Rejected + } + +struct NotUsedStruct2 { + uint256 three; + uint256 four; +} + +enum Status2 { + Pending, + Approved, + Rejected + } + +contract __generic { + + struct not_UsedStruct3__ { + uint256 three; + uint256 four; + } + + enum status3 { + Pending, + Approved, + Rejected + } + + struct NotUsedStruct4 { + uint256 three; + uint256 four; + } + + enum Status4 { + Pending, + Approved, + Rejected + } + + constructor() {} + + function function1() pure external { + uint af1 = 0; + af1; + } + + function function2() external pure { + uint b; + b; + } +} diff --git a/e2e/08-autofix/contract-name-camelcase/Foo1AfterFix.sol b/e2e/08-autofix/contract-name-camelcase/Foo1AfterFix.sol new file mode 100644 index 00000000..eb5ab8fe --- /dev/null +++ b/e2e/08-autofix/contract-name-camelcase/Foo1AfterFix.sol @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +struct NotUsedStruct1 { + uint256 three; + uint256 four; +} + +enum Status1 { + Pending, + Approved, + Rejected + } + +struct NotUsedStruct2 { + uint256 three; + uint256 four; +} + +enum Status2 { + Pending, + Approved, + Rejected + } + +contract Generic { + + struct NotUsedStruct3 { + uint256 three; + uint256 four; + } + + enum Status3 { + Pending, + Approved, + Rejected + } + + struct NotUsedStruct4 { + uint256 three; + uint256 four; + } + + enum Status4 { + Pending, + Approved, + Rejected + } + + constructor() {} + + function function1() pure external { + uint af1 = 0; + af1; + } + + function function2() external pure { + uint b; + b; + } +} diff --git a/e2e/08-autofix/contract-name-camelcase/Foo1BeforeFix.sol b/e2e/08-autofix/contract-name-camelcase/Foo1BeforeFix.sol new file mode 100644 index 00000000..2cf3e9bd --- /dev/null +++ b/e2e/08-autofix/contract-name-camelcase/Foo1BeforeFix.sol @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +struct not_Used_Struct1 { + uint256 three; + uint256 four; +} + +enum __status1_ { + Pending, + Approved, + Rejected + } + +struct NotUsedStruct2 { + uint256 three; + uint256 four; +} + +enum Status2 { + Pending, + Approved, + Rejected + } + +contract __generic { + + struct not_UsedStruct3__ { + uint256 three; + uint256 four; + } + + enum status3 { + Pending, + Approved, + Rejected + } + + struct NotUsedStruct4 { + uint256 three; + uint256 four; + } + + enum Status4 { + Pending, + Approved, + Rejected + } + + constructor() {} + + function function1() pure external { + uint af1 = 0; + af1; + } + + function function2() external pure { + uint b; + b; + } +} diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index 9a7227b6..d031bcf6 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -418,7 +418,7 @@ describe('e2e', function () { }) }) - describe('autofix rule: avoid-suicide', () => { + describe('autofix rule: contract-name-camecalse', () => { before(function () { params = retrieveParams('avoid-suicide/') currentConfig = `${params.path}${params.subpath}.solhint.json` @@ -463,6 +463,52 @@ describe('e2e', function () { expect(result).to.be.true }) }) + + describe('autofix rule: avoid-suicide', () => { + before(function () { + params = retrieveParams('contract-name-camecalse/') + 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 (8)', () => { + result = compareTextFiles(currentFile, beforeFixFile) + expect(result).to.be.true + }) + + it('should execute and compare Foo1 with template AFTER FIX and they should match (8)', () => { + ;({ 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 (8)', () => { + expect(code).to.equal(1) + }) + + it('should get the right report (8)', () => { + const reportLines = stdout.split('\n') + const finalLine = '5 problems (5 errors, 0 warnings)' + expect(reportLines[reportLines.length - 3]).to.contain(finalLine) + }) + }) + + it('should check FOO1 does not change after test (8)', () => { + result = compareTextFiles(currentFile, beforeFixFile) + expect(result).to.be.true + }) + }) }) // FALTA LA PRUEBA DEL STORE TO FILE diff --git a/lib/rules/naming/contract-name-camelcase.js b/lib/rules/naming/contract-name-camelcase.js index 2d39a88a..d7fd9de1 100644 --- a/lib/rules/naming/contract-name-camelcase.js +++ b/lib/rules/naming/contract-name-camelcase.js @@ -6,14 +6,22 @@ const meta = { type: 'naming', docs: { - description: 'Contract name must be in CamelCase.', + description: 'Contract, Structs and Enums should be in CamelCase.', category: 'Style Guide Rules', + notes: [ + { + note: 'Solhint allows this rule to automatically fix the code with `--fix` option', + }, + { + note: 'The FIX will only change first letter and remove underscores', + }, + ], }, isDefault: false, recommended: true, defaultSetup: 'warn', - + fixable: true, schema: null, } @@ -23,25 +31,51 @@ class ContractNameCamelcaseChecker extends BaseChecker { } ContractDefinition(node) { - this.validateName(node) + this.validateName(node, 'contract') } EnumDefinition(node) { - this.validateName(node) + this.validateName(node, 'enum') } StructDefinition(node) { - this.validateName(node) + this.validateName(node, 'struct') } - validateName(node) { + validateName(node, type) { if (naming.isNotCamelCase(node.name)) { - this._error(node) + this._error(node, type) } } - _error(node) { - this.error(node, 'Contract name must be in CamelCase') + fixStatement(node, type) { + // Remove leading and trailing underscores + let nameToPut = node.name.replace(/^[_]+|[_]+$/g, '') + + // Replace '-' with space and split the string into an array + let words = nameToPut.replace(/-/g, ' ').split('_') + + // Capitalize the first letter of each word + words = words.map((word) => word.charAt(0).toUpperCase() + word.slice(1)) + + // Join the words back into a single string + nameToPut = words.join('') + + const originalNameLength = node.name.length + const typeLength = type.length + + const rangeStart = node.range[0] + typeLength + 1 + const rangeEnd = node.range[0] + typeLength + originalNameLength + + return (fixer) => fixer.replaceTextRange([rangeStart, rangeEnd], nameToPut) + } + + _error(node, type) { + this.error( + node, + 'Contract, Structs and Enums should be in CamelCase', + this.fixStatement(node, type) + ) } } From d025f06792f3b49aa3fd2922693df0728280819a Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 25 Dec 2023 19:03:20 -0300 Subject: [PATCH 2/5] autofix: contract-name-camelcase --- docs/rules.md | 2 +- docs/rules/naming/contract-name-camelcase.md | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/rules.md b/docs/rules.md index 64185cd1..4bded6c7 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -38,7 +38,7 @@ title: "Rule Index of Solhint" | Rule Id | Error | Recommended | Deprecated | | ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------- | ------------ | ----------- | | [const-name-snakecase](./rules/naming/const-name-snakecase.md) | Constant name must be in capitalized SNAKE_CASE. (Does not check IMMUTABLES, use immutable-vars-naming) | $~~~~~~~~$✔️ | | -| [contract-name-camelcase](./rules/naming/contract-name-camelcase.md) | Contract name must be in CamelCase. | $~~~~~~~~$✔️ | | +| [contract-name-camelcase](./rules/naming/contract-name-camelcase.md) | Contract, Structs and Enums should be in CamelCase. | $~~~~~~~~$✔️ | | | [event-name-camelcase](./rules/naming/event-name-camelcase.md) | Event name must be in CamelCase. | $~~~~~~~~$✔️ | | | [foundry-test-functions](./rules/naming/foundry-test-functions.md) | Enforce naming convention on functions for Foundry test cases | | | | [func-name-mixedcase](./rules/naming/func-name-mixedcase.md) | Function name must be in mixedCase. | $~~~~~~~~$✔️ | | diff --git a/docs/rules/naming/contract-name-camelcase.md b/docs/rules/naming/contract-name-camelcase.md index 5165c2e1..03fca320 100644 --- a/docs/rules/naming/contract-name-camelcase.md +++ b/docs/rules/naming/contract-name-camelcase.md @@ -12,7 +12,7 @@ title: "contract-name-camelcase | Solhint" ## Description -Contract name must be in CamelCase. +Contract, Structs and Enums should be in CamelCase. ## Options This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn. @@ -26,6 +26,9 @@ 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 +- The FIX will only change first letter and remove underscores ## Examples This rule does not have examples. From 1a3ad583991e153d416be35a648b319c2a890dae Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 25 Dec 2023 19:07:20 -0300 Subject: [PATCH 3/5] autofix: contract-name-camelcase --- e2e/autofix-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index d031bcf6..b2c3686a 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -418,7 +418,7 @@ describe('e2e', function () { }) }) - describe('autofix rule: contract-name-camecalse', () => { + describe('autofix rule: contract-name-camelcase', () => { before(function () { params = retrieveParams('avoid-suicide/') currentConfig = `${params.path}${params.subpath}.solhint.json` @@ -466,7 +466,7 @@ describe('e2e', function () { describe('autofix rule: avoid-suicide', () => { before(function () { - params = retrieveParams('contract-name-camecalse/') + params = retrieveParams('contract-name-camelcase/') currentConfig = `${params.path}${params.subpath}.solhint.json` currentFile = `${params.path}${params.subpath}Foo1.sol` beforeFixFile = `${params.path}${params.subpath}Foo1BeforeFix.sol` From e4cce341fbf10c77d52e13b0b4b0f157141c5786 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Tue, 26 Dec 2023 17:53:16 -0300 Subject: [PATCH 4/5] autofix: event name camelcase --- docs/rules/naming/event-name-camelcase.md | 3 ++ .../event-name-camelcase/.solhint.json | 5 ++ e2e/08-autofix/event-name-camelcase/Foo1.sol | 33 ++++++++++++ .../event-name-camelcase/Foo1AfterFix.sol | 33 ++++++++++++ .../event-name-camelcase/Foo1BeforeFix.sol | 33 ++++++++++++ e2e/autofix-test.js | 50 ++++++++++++++++++- lib/rules/naming/event-name-camelcase.js | 34 ++++++++++++- 7 files changed, 187 insertions(+), 4 deletions(-) create mode 100644 e2e/08-autofix/event-name-camelcase/.solhint.json create mode 100644 e2e/08-autofix/event-name-camelcase/Foo1.sol create mode 100644 e2e/08-autofix/event-name-camelcase/Foo1AfterFix.sol create mode 100644 e2e/08-autofix/event-name-camelcase/Foo1BeforeFix.sol diff --git a/docs/rules/naming/event-name-camelcase.md b/docs/rules/naming/event-name-camelcase.md index 70207821..a6c00b63 100644 --- a/docs/rules/naming/event-name-camelcase.md +++ b/docs/rules/naming/event-name-camelcase.md @@ -26,6 +26,9 @@ 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 +- The FIX will only change first letter and remove underscores ## Examples This rule does not have examples. diff --git a/e2e/08-autofix/event-name-camelcase/.solhint.json b/e2e/08-autofix/event-name-camelcase/.solhint.json new file mode 100644 index 00000000..6da8ad19 --- /dev/null +++ b/e2e/08-autofix/event-name-camelcase/.solhint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "contract-name-camelcase": "error" + } +} diff --git a/e2e/08-autofix/event-name-camelcase/Foo1.sol b/e2e/08-autofix/event-name-camelcase/Foo1.sol new file mode 100644 index 00000000..8a1b5cca --- /dev/null +++ b/e2e/08-autofix/event-name-camelcase/Foo1.sol @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +contract Generic { + + event not_UsedEvent1(uint256 indexed amount, address account); + + event __not_UsedEvent___2(uint256 indexed amount, address indexed account); + + event not___UsedEvent3(uint256 amount, address account); + + event notUsedEvent4(uint256 indexed amount, address indexed account); + + event notUsedEvent5____(uint256 indexed amount, address account); + + event __not_UsedEvent6__(uint256 indexed amount, address account); + + event OkEvent1(uint256 indexed amount, address account); + + event OkEventOk2(uint256 indexed amount, address indexed account); + + constructor() {} + + function function1() external pure { + uint af1 = 0; + af1; + } + + function function2() external pure { + uint b; + b; + } +} diff --git a/e2e/08-autofix/event-name-camelcase/Foo1AfterFix.sol b/e2e/08-autofix/event-name-camelcase/Foo1AfterFix.sol new file mode 100644 index 00000000..8d6820c2 --- /dev/null +++ b/e2e/08-autofix/event-name-camelcase/Foo1AfterFix.sol @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +contract Generic { + + event NotUsedEvent1(uint256 indexed amount, address account); + + event NotUsedEvent2(uint256 indexed amount, address indexed account); + + event NotUsedEvent3(uint256 amount, address account); + + event NotUsedEvent4(uint256 indexed amount, address indexed account); + + event NotUsedEvent5(uint256 indexed amount, address account); + + event NotUsedEvent6(uint256 indexed amount, address account); + + event OkEvent1(uint256 indexed amount, address account); + + event OkEventOk2(uint256 indexed amount, address indexed account); + + constructor() {} + + function function1() external pure { + uint af1 = 0; + af1; + } + + function function2() external pure { + uint b; + b; + } +} diff --git a/e2e/08-autofix/event-name-camelcase/Foo1BeforeFix.sol b/e2e/08-autofix/event-name-camelcase/Foo1BeforeFix.sol new file mode 100644 index 00000000..8a1b5cca --- /dev/null +++ b/e2e/08-autofix/event-name-camelcase/Foo1BeforeFix.sol @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +contract Generic { + + event not_UsedEvent1(uint256 indexed amount, address account); + + event __not_UsedEvent___2(uint256 indexed amount, address indexed account); + + event not___UsedEvent3(uint256 amount, address account); + + event notUsedEvent4(uint256 indexed amount, address indexed account); + + event notUsedEvent5____(uint256 indexed amount, address account); + + event __not_UsedEvent6__(uint256 indexed amount, address account); + + event OkEvent1(uint256 indexed amount, address account); + + event OkEventOk2(uint256 indexed amount, address indexed account); + + constructor() {} + + function function1() external pure { + uint af1 = 0; + af1; + } + + function function2() external pure { + uint b; + b; + } +} diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index b2c3686a..ae2c82d9 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -418,7 +418,7 @@ describe('e2e', function () { }) }) - describe('autofix rule: contract-name-camelcase', () => { + describe('autofix rule: avoid-suicide', () => { before(function () { params = retrieveParams('avoid-suicide/') currentConfig = `${params.path}${params.subpath}.solhint.json` @@ -464,7 +464,7 @@ describe('e2e', function () { }) }) - describe('autofix rule: avoid-suicide', () => { + describe('autofix rule: contract-name-camelcase', () => { before(function () { params = retrieveParams('contract-name-camelcase/') currentConfig = `${params.path}${params.subpath}.solhint.json` @@ -509,6 +509,52 @@ describe('e2e', function () { expect(result).to.be.true }) }) + + describe('autofix rule: event-name-camelcase', () => { + before(function () { + params = retrieveParams('event-name-camelcase/') + 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 (8)', () => { + result = compareTextFiles(currentFile, beforeFixFile) + expect(result).to.be.true + }) + + it('should execute and compare Foo1 with template AFTER FIX and they should match (8)', () => { + ;({ 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 (8)', () => { + expect(code).to.equal(1) + }) + + it('should get the right report (8)', () => { + const reportLines = stdout.split('\n') + const finalLine = '6 problems (6 errors, 0 warnings)' + expect(reportLines[reportLines.length - 3]).to.contain(finalLine) + }) + }) + + it('should check FOO1 does not change after test (8)', () => { + result = compareTextFiles(currentFile, beforeFixFile) + expect(result).to.be.true + }) + }) }) // FALTA LA PRUEBA DEL STORE TO FILE diff --git a/lib/rules/naming/event-name-camelcase.js b/lib/rules/naming/event-name-camelcase.js index 5bcbbb1c..aecfca95 100644 --- a/lib/rules/naming/event-name-camelcase.js +++ b/lib/rules/naming/event-name-camelcase.js @@ -8,12 +8,20 @@ const meta = { docs: { description: 'Event name must be in CamelCase.', category: 'Style Guide Rules', + notes: [ + { + note: 'Solhint allows this rule to automatically fix the code with `--fix` option', + }, + { + note: 'The FIX will only change first letter and remove underscores', + }, + ], }, isDefault: false, recommended: true, defaultSetup: 'warn', - + fixable: true, schema: null, } @@ -22,9 +30,31 @@ class EventNameCamelcaseChecker extends BaseChecker { super(reporter, ruleId, meta) } + fixStatement(node) { + // Remove leading and trailing underscores + let nameToPut = node.name.replace(/^[_]+|[_]+$/g, '') + + // Replace '-' with space and split the string into an array + let words = nameToPut.replace(/-/g, ' ').split('_') + + // Capitalize the first letter of each word + words = words.map((word) => word.charAt(0).toUpperCase() + word.slice(1)) + + // Join the words back into a single string + nameToPut = words.join('') + + const originalNameLength = node.name.length + const typeLength = 'event'.length + + const rangeStart = node.range[0] + typeLength + 1 + const rangeEnd = node.range[0] + typeLength + originalNameLength + + return (fixer) => fixer.replaceTextRange([rangeStart, rangeEnd], nameToPut) + } + EventDefinition(node) { if (naming.isNotCamelCase(node.name)) { - this.error(node, 'Event name must be in CamelCase') + this.error(node, 'Event name must be in CamelCase', this.fixStatement(node)) } } } From 8fe415d0be7111b3afa91ea3c26605a385ba22ee Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Tue, 26 Dec 2023 17:57:38 -0300 Subject: [PATCH 5/5] fix: .solhint --- e2e/08-autofix/event-name-camelcase/.solhint.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/08-autofix/event-name-camelcase/.solhint.json b/e2e/08-autofix/event-name-camelcase/.solhint.json index 6da8ad19..7f809bb3 100644 --- a/e2e/08-autofix/event-name-camelcase/.solhint.json +++ b/e2e/08-autofix/event-name-camelcase/.solhint.json @@ -1,5 +1,5 @@ { "rules": { - "contract-name-camelcase": "error" + "event-name-camelcase": "error" } }