diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c285c7e..7ae0b65b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ - Changelog and docs for `no-empty-blocks` rule to clarify its functionality ### Added -- `fixShow` option to show report. `fix` option skips showing report on screen +- `fix` option now shows the report on screen - `save` option to store report on disk with the standard or the specified format - Check for updates on Solhint version to keep users with the last versin available. There's an option to disable this check (`--disc`) - Autofix for `explicit-types` rule [504](https://github.com/protofire/solhint/pull/504) diff --git a/README.md b/README.md index 1963c4c4..27ffa79b 100644 --- a/README.md +++ b/README.md @@ -62,8 +62,7 @@ Options: -c, --config [file_name] file to use as your .solhint.json -q, --quiet report errors only - default: false --ignore-path [file_name] file to use as your .solhintignore - --fix automatically fix problems. Skip report - --fixShow automatically fix problems. Show report + --fix automatically fix problems and show report --noPrompt do not suggest to backup files when any `fix` option is selected --init create configuration file for solhint --disc do not check for solhint updates diff --git a/docs/rules/best-practises/explicit-types.md b/docs/rules/best-practises/explicit-types.md index 18964a4e..2a472762 100644 --- a/docs/rules/best-practises/explicit-types.md +++ b/docs/rules/best-practises/explicit-types.md @@ -33,7 +33,7 @@ This rule accepts an array of options: ``` ### Notes -- Solhint allows this rule to automatically fix the code with `--fix` or `--fixShow` option +- Solhint allows this rule to automatically fix the code with `--fix` option ## Examples ### 👍 Examples of **correct** code for this rule diff --git a/docs/rules/naming/private-vars-leading-underscore.md b/docs/rules/naming/private-vars-leading-underscore.md index 6bcbaee6..bba9e897 100644 --- a/docs/rules/naming/private-vars-leading-underscore.md +++ b/docs/rules/naming/private-vars-leading-underscore.md @@ -9,7 +9,7 @@ title: "private-vars-leading-underscore | Solhint" ![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow) ## Description -Non-external functions and state variables should start with a single underscore. Others, shouldn't. +Non-external functions and state variables should start with a single underscore. Others, shouldn't ## Options This rule accepts an array of options: diff --git a/docs/rules/security/avoid-sha3.md b/docs/rules/security/avoid-sha3.md index 6b009388..7947a087 100644 --- a/docs/rules/security/avoid-sha3.md +++ b/docs/rules/security/avoid-sha3.md @@ -27,7 +27,7 @@ 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` or `--fixShow` option +- Solhint allows this rule to automatically fix the code with `--fix` option ## Examples This rule does not have examples. diff --git a/docs/rules/security/avoid-throw.md b/docs/rules/security/avoid-throw.md index b5a3022d..5b12fcd7 100644 --- a/docs/rules/security/avoid-throw.md +++ b/docs/rules/security/avoid-throw.md @@ -27,7 +27,7 @@ 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` or `--fixShow` option +- Solhint allows this rule to automatically fix the code with `--fix` option ## Examples This rule does not have examples. diff --git a/e2e/08-autofix/commands/.solhint.json b/e2e/08-autofix/commands/.solhint.json new file mode 100644 index 00000000..7bd7c6a2 --- /dev/null +++ b/e2e/08-autofix/commands/.solhint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "explicit-types": ["error", "explicit"] + } +} diff --git a/e2e/08-autofix/commands/Foo1.sol b/e2e/08-autofix/commands/Foo1.sol new file mode 100644 index 00000000..78bcaec7 --- /dev/null +++ b/e2e/08-autofix/commands/Foo1.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.4; + +import {ERC20Burnable} from '@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol'; + +contract Foo1 is ERC20Burnable { + uint public hola; + uint public hola2; + int public constant hola3 = 2; + ufixed hola4; + fixed internal hola5; + + constructor() ERC20('MyToken', 'MTK') {} + + // solhint-disable no-empty-blocks + function payableTrue() public payable {} + + // solhint-disable no-empty-blocks + function payableFalse() public {} + + function zarasa() {} +} diff --git a/e2e/08-autofix/commands/Foo1AfterFix.sol b/e2e/08-autofix/commands/Foo1AfterFix.sol new file mode 100644 index 00000000..ca900d69 --- /dev/null +++ b/e2e/08-autofix/commands/Foo1AfterFix.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.4; + +import {ERC20Burnable} from '@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol'; + +contract Foo1 is ERC20Burnable { + uint256 public hola; + uint256 public hola2; + int256 public constant hola3 = 2; + ufixed128x18 hola4; + fixed128x18 internal hola5; + + constructor() ERC20('MyToken', 'MTK') {} + + // solhint-disable no-empty-blocks + function payableTrue() public payable {} + + // solhint-disable no-empty-blocks + function payableFalse() public {} + + function zarasa() {} +} diff --git a/e2e/08-autofix/commands/Foo1BeforeFix.sol b/e2e/08-autofix/commands/Foo1BeforeFix.sol new file mode 100644 index 00000000..78bcaec7 --- /dev/null +++ b/e2e/08-autofix/commands/Foo1BeforeFix.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.4; + +import {ERC20Burnable} from '@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol'; + +contract Foo1 is ERC20Burnable { + uint public hola; + uint public hola2; + int public constant hola3 = 2; + ufixed hola4; + fixed internal hola5; + + constructor() ERC20('MyToken', 'MTK') {} + + // solhint-disable no-empty-blocks + function payableTrue() public payable {} + + // solhint-disable no-empty-blocks + function payableFalse() public {} + + function zarasa() {} +} diff --git a/e2e/08-autofix/explicit-types/.solhint.json b/e2e/08-autofix/explicit-types/.solhint.json new file mode 100644 index 00000000..7bd7c6a2 --- /dev/null +++ b/e2e/08-autofix/explicit-types/.solhint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "explicit-types": ["error", "explicit"] + } +} diff --git a/e2e/08-autofix/explicit-types/Foo1.sol b/e2e/08-autofix/explicit-types/Foo1.sol new file mode 100644 index 00000000..78bcaec7 --- /dev/null +++ b/e2e/08-autofix/explicit-types/Foo1.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.4; + +import {ERC20Burnable} from '@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol'; + +contract Foo1 is ERC20Burnable { + uint public hola; + uint public hola2; + int public constant hola3 = 2; + ufixed hola4; + fixed internal hola5; + + constructor() ERC20('MyToken', 'MTK') {} + + // solhint-disable no-empty-blocks + function payableTrue() public payable {} + + // solhint-disable no-empty-blocks + function payableFalse() public {} + + function zarasa() {} +} diff --git a/e2e/08-autofix/explicit-types/Foo1AfterFix.sol b/e2e/08-autofix/explicit-types/Foo1AfterFix.sol new file mode 100644 index 00000000..ca900d69 --- /dev/null +++ b/e2e/08-autofix/explicit-types/Foo1AfterFix.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.4; + +import {ERC20Burnable} from '@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol'; + +contract Foo1 is ERC20Burnable { + uint256 public hola; + uint256 public hola2; + int256 public constant hola3 = 2; + ufixed128x18 hola4; + fixed128x18 internal hola5; + + constructor() ERC20('MyToken', 'MTK') {} + + // solhint-disable no-empty-blocks + function payableTrue() public payable {} + + // solhint-disable no-empty-blocks + function payableFalse() public {} + + function zarasa() {} +} diff --git a/e2e/08-autofix/explicit-types/Foo1BeforeFix.sol b/e2e/08-autofix/explicit-types/Foo1BeforeFix.sol new file mode 100644 index 00000000..78bcaec7 --- /dev/null +++ b/e2e/08-autofix/explicit-types/Foo1BeforeFix.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.4; + +import {ERC20Burnable} from '@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol'; + +contract Foo1 is ERC20Burnable { + uint public hola; + uint public hola2; + int public constant hola3 = 2; + ufixed hola4; + fixed internal hola5; + + constructor() ERC20('MyToken', 'MTK') {} + + // solhint-disable no-empty-blocks + function payableTrue() public payable {} + + // solhint-disable no-empty-blocks + function payableFalse() public {} + + function zarasa() {} +} diff --git a/e2e/08-autofix/no-console/.solhint.json b/e2e/08-autofix/no-console/.solhint.json new file mode 100644 index 00000000..2ff50f91 --- /dev/null +++ b/e2e/08-autofix/no-console/.solhint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "no-console": "error" + } +} diff --git a/e2e/08-autofix/no-console/Foo1.sol b/e2e/08-autofix/no-console/Foo1.sol new file mode 100644 index 00000000..93b6d395 --- /dev/null +++ b/e2e/08-autofix/no-console/Foo1.sol @@ -0,0 +1,37 @@ +pragma solidity 0.8.0; + +import 'hardhat/console.sol'; +import 'forge-std/console.sol'; +import 'forge-std/console2.sol'; +import 'forge-std/xxxxx.sol'; + +contract Foo1 { + Console[] public consoleTest; + Console[] public console; + + struct Console { + uint256 one; + uint256 two; + } + + function a() public { + console.log('test'); + // comment + console.logString('test logString'); + uint256 declaration; + console.logBytes12('test logBytes12'); + } + + function b() public { + console2.log('test console 2'); + // comment + console.logString('test logString'); + uint256 declaration; + console.logBytes12('test'); + } + + function c() external { + consoleTest.push(0, 0); + console.push = (1, 1); + } +} diff --git a/e2e/08-autofix/no-console/Foo1AfterFix.sol b/e2e/08-autofix/no-console/Foo1AfterFix.sol new file mode 100644 index 00000000..22075370 --- /dev/null +++ b/e2e/08-autofix/no-console/Foo1AfterFix.sol @@ -0,0 +1,37 @@ +pragma solidity 0.8.0; + + + + +import 'forge-std/xxxxx.sol'; + +contract Foo1 { + Console[] public consoleTest; + Console[] public console; + + struct Console { + uint256 one; + uint256 two; + } + + function a() public { + + // comment + + uint256 declaration; + + } + + function b() public { + + // comment + + uint256 declaration; + + } + + function c() external { + consoleTest.push(0, 0); + console.push = (1, 1); + } +} diff --git a/e2e/08-autofix/no-console/Foo1BeforeFix.sol b/e2e/08-autofix/no-console/Foo1BeforeFix.sol new file mode 100644 index 00000000..93b6d395 --- /dev/null +++ b/e2e/08-autofix/no-console/Foo1BeforeFix.sol @@ -0,0 +1,37 @@ +pragma solidity 0.8.0; + +import 'hardhat/console.sol'; +import 'forge-std/console.sol'; +import 'forge-std/console2.sol'; +import 'forge-std/xxxxx.sol'; + +contract Foo1 { + Console[] public consoleTest; + Console[] public console; + + struct Console { + uint256 one; + uint256 two; + } + + function a() public { + console.log('test'); + // comment + console.logString('test logString'); + uint256 declaration; + console.logBytes12('test logBytes12'); + } + + function b() public { + console2.log('test console 2'); + // comment + console.logString('test logString'); + uint256 declaration; + console.logBytes12('test'); + } + + function c() external { + consoleTest.push(0, 0); + console.push = (1, 1); + } +} diff --git a/e2e/08-autofix/private-vars-underscore/.solhint.json b/e2e/08-autofix/private-vars-underscore/.solhint.json new file mode 100644 index 00000000..47ff676f --- /dev/null +++ b/e2e/08-autofix/private-vars-underscore/.solhint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "private-vars-leading-underscore": ["error",{"strict":false}] + } +} diff --git a/e2e/08-autofix/private-vars-underscore/Foo1.sol b/e2e/08-autofix/private-vars-underscore/Foo1.sol new file mode 100644 index 00000000..f8a07515 --- /dev/null +++ b/e2e/08-autofix/private-vars-underscore/Foo1.sol @@ -0,0 +1,83 @@ +pragma solidity 0.8.0; + +library libraryName { + uint256 internal lzarasa1; + uint256 internal lzarasa2 = 2; + uint256 internal _lzarasa3; + uint256 private lzarasa4; + uint256 private lzarasa5 = 5; + uint256 private _lzarasa6; + + uint256 public _lzarasa7; + uint256 public _lzarasa8 = 8; + uint256 public lzarasa9; + + function fofo1() public {} + function _fofo2() public {} + function _fofo3() internal {} + function fofo4() internal {} +} + +contract Foo1 { + uint256 internal zarasa1; + uint256 internal zarasa2 = 2; + uint256 internal _zarasa3; + uint256 private zarasa4; + uint256 private zarasa5 = 5; + uint256 private _zarasa6; + + uint256 public _zarasa7; + uint256 public _zarasa8 = 8; + uint256 public zarasa9; + + address payable internal constant zarasa10 = '0x0'; + address public constant _zarasa11 = '0x0'; + uint256 public immutable _zarasa12 = 2; + uint256 _zarasa13; + uint256 zarasa14; + + mapping(address => uint256) internal zarMapping1; + mapping(address => uint256) public _zarMapping2; + mapping(address => uint256) internal _zarMapping3; + mapping(address => uint256) public zarMapping4; + + function fooA(uint bar) internal payable onlyOwner returns (uint256 barA) { + uint256 zarasaFunc; + } + + function fooB(uint bar) private onlyOwner returns (uint256 _barB) { + uint256 zarasaFunc; + } + + function fooC(uint bar) private onlyOwner returns (uint256 _barC) { + uint256 zarasaFunc; + } + + function fooD(uint bar) external onlyOwner { + uint256 zarasaFunc; + } + + function fooE(uint bar) public onlyOwner { + uint256 zarasaFunc; + } + + function fooF(uint bar) onlyOwner returns (uint256 _barF) { + uint256 zarasaFunc; + } + + function fooG(uint bar) onlyOwner { + uint256 zarasaFunc; + } + + function _fooH(uint bar) external onlyOwner { + uint256 zarasaFunc; + } + + function _fooI(uint bar) public onlyOwner { + uint256 zarasaFunc; + } + + function _fooJ(uint bar) onlyOwner returns (uint256 barJ) { + uint256 zarasaFunc; + } +} diff --git a/e2e/08-autofix/private-vars-underscore/Foo1AfterFix.sol b/e2e/08-autofix/private-vars-underscore/Foo1AfterFix.sol new file mode 100644 index 00000000..87b80ad6 --- /dev/null +++ b/e2e/08-autofix/private-vars-underscore/Foo1AfterFix.sol @@ -0,0 +1,83 @@ +pragma solidity 0.8.0; + +library libraryName { + uint256 internal _lzarasa1; + uint256 internal _lzarasa2 = 2; + uint256 internal _lzarasa3; + uint256 private _lzarasa4; + uint256 private _lzarasa5 = 5; + uint256 private _lzarasa6; + + uint256 public lzarasa7; + uint256 public lzarasa8 = 8; + uint256 public lzarasa9; + + function fofo1() public {} + function fofo2() public {} + function _fofo3() internal {} + function _fofo4() internal {} +} + +contract Foo1 { + uint256 internal _zarasa1; + uint256 internal _zarasa2 = 2; + uint256 internal _zarasa3; + uint256 private _zarasa4; + uint256 private _zarasa5 = 5; + uint256 private _zarasa6; + + uint256 public zarasa7; + uint256 public zarasa8 = 8; + uint256 public zarasa9; + + address payable internal constant _zarasa10 = '0x0'; + address public constant zarasa11 = '0x0'; + uint256 public immutable zarasa12 = 2; + uint256 _zarasa13; + uint256 _zarasa14; + + mapping(address => uint256) internal _zarMapping1; + mapping(address => uint256) public zarMapping2; + mapping(address => uint256) internal _zarMapping3; + mapping(address => uint256) public zarMapping4; + + function _fooA(uint bar) internal payable onlyOwner returns (uint256 barA) { + uint256 zarasaFunc; + } + + function _fooB(uint bar) private onlyOwner returns (uint256 _barB) { + uint256 zarasaFunc; + } + + function _fooC(uint bar) private onlyOwner returns (uint256 _barC) { + uint256 zarasaFunc; + } + + function fooD(uint bar) external onlyOwner { + uint256 zarasaFunc; + } + + function fooE(uint bar) public onlyOwner { + uint256 zarasaFunc; + } + + function _fooF(uint bar) onlyOwner returns (uint256 _barF) { + uint256 zarasaFunc; + } + + function _fooG(uint bar) onlyOwner { + uint256 zarasaFunc; + } + + function fooH(uint bar) external onlyOwner { + uint256 zarasaFunc; + } + + function fooI(uint bar) public onlyOwner { + uint256 zarasaFunc; + } + + function _fooJ(uint bar) onlyOwner returns (uint256 barJ) { + uint256 zarasaFunc; + } +} diff --git a/e2e/08-autofix/private-vars-underscore/Foo1BeforeFix.sol b/e2e/08-autofix/private-vars-underscore/Foo1BeforeFix.sol new file mode 100644 index 00000000..f8a07515 --- /dev/null +++ b/e2e/08-autofix/private-vars-underscore/Foo1BeforeFix.sol @@ -0,0 +1,83 @@ +pragma solidity 0.8.0; + +library libraryName { + uint256 internal lzarasa1; + uint256 internal lzarasa2 = 2; + uint256 internal _lzarasa3; + uint256 private lzarasa4; + uint256 private lzarasa5 = 5; + uint256 private _lzarasa6; + + uint256 public _lzarasa7; + uint256 public _lzarasa8 = 8; + uint256 public lzarasa9; + + function fofo1() public {} + function _fofo2() public {} + function _fofo3() internal {} + function fofo4() internal {} +} + +contract Foo1 { + uint256 internal zarasa1; + uint256 internal zarasa2 = 2; + uint256 internal _zarasa3; + uint256 private zarasa4; + uint256 private zarasa5 = 5; + uint256 private _zarasa6; + + uint256 public _zarasa7; + uint256 public _zarasa8 = 8; + uint256 public zarasa9; + + address payable internal constant zarasa10 = '0x0'; + address public constant _zarasa11 = '0x0'; + uint256 public immutable _zarasa12 = 2; + uint256 _zarasa13; + uint256 zarasa14; + + mapping(address => uint256) internal zarMapping1; + mapping(address => uint256) public _zarMapping2; + mapping(address => uint256) internal _zarMapping3; + mapping(address => uint256) public zarMapping4; + + function fooA(uint bar) internal payable onlyOwner returns (uint256 barA) { + uint256 zarasaFunc; + } + + function fooB(uint bar) private onlyOwner returns (uint256 _barB) { + uint256 zarasaFunc; + } + + function fooC(uint bar) private onlyOwner returns (uint256 _barC) { + uint256 zarasaFunc; + } + + function fooD(uint bar) external onlyOwner { + uint256 zarasaFunc; + } + + function fooE(uint bar) public onlyOwner { + uint256 zarasaFunc; + } + + function fooF(uint bar) onlyOwner returns (uint256 _barF) { + uint256 zarasaFunc; + } + + function fooG(uint bar) onlyOwner { + uint256 zarasaFunc; + } + + function _fooH(uint bar) external onlyOwner { + uint256 zarasaFunc; + } + + function _fooI(uint bar) public onlyOwner { + uint256 zarasaFunc; + } + + function _fooJ(uint bar) onlyOwner returns (uint256 barJ) { + uint256 zarasaFunc; + } +} diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js new file mode 100644 index 00000000..1f11d184 --- /dev/null +++ b/e2e/autofix-test.js @@ -0,0 +1,276 @@ +const chai = require('chai') +const { expect } = chai +const fs = require('fs-extra') +const os = require('os') +const path = require('path') +const shell = require('shelljs') +const spawnSync = require('spawn-sync') + +const E2E = true + +let params +let currentConfig +let currentFile +let beforeFixFile +let afterFixFile + +function retrieveParams(subpath) { + if (E2E) { + return { command: 'solhint', param1: '', path: '', subpath } + } else { + return { command: 'node', param1: 'solhint', path: 'e2e/08-autofix/', subpath } + } +} + +function compareTextFiles(file1Path, file2Path) { + const file1Content = fs.readFileSync(file1Path, 'utf-8') + const file2Content = fs.readFileSync(file2Path, 'utf-8') + + return file1Content === file2Content +} + +function copyFile(sourcePath, destinationPath) { + shell.cp(sourcePath, destinationPath) +} + +function useFixture(dir) { + beforeEach(`switch to ${dir}`, function () { + const fixturePath = path.join(__dirname, dir) + + const tmpDirContainer = os.tmpdir() + this.testDirPath = path.join(tmpDirContainer, `solhint-tests-${dir}`) + + fs.ensureDirSync(this.testDirPath) + fs.emptyDirSync(this.testDirPath) + + fs.copySync(fixturePath, this.testDirPath) + + shell.cd(this.testDirPath) + }) +} + +describe('e2e', function () { + let result = false + + describe('autofix tests', () => { + if (E2E) { + useFixture('08-autofix') + } + + describe('autofix command line options', () => { + before(function () { + params = retrieveParams('commands/') + 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 without noPrompt', () => { + after(function () { + if (!E2E) { + copyFile(beforeFixFile, currentFile) + } + }) + + it('should terminate with --fix and user choose NOT to continue', () => { + const solhintProcess = spawnSync( + `${params.command}`, + [`${params.param1}`, '-c', currentConfig, currentFile, '--fix', '--disc'], + { + input: 'n\n', // Provide 'n' as input + shell: true, + } + ) + + expect(solhintProcess.status).to.equal(0) + expect(solhintProcess.stdout.toString().includes('Process terminated by user')) + }) + + it('should compare Foo1 file with template beforeFix file and they should match 1a', () => { + result = compareTextFiles(currentFile, beforeFixFile) + expect(result).to.be.true + }) + + it('should fix with --fix and user choose YES to continue', () => { + const solhintProcess = spawnSync( + `${params.command}`, + [`${params.param1}`, '-c', currentConfig, currentFile, '--fix', '--disc'], + { + input: 'y\n', // Provide 'y' as input + shell: true, + } + ) + + expect(solhintProcess.status).to.equal(1) + expect(solhintProcess.stdout.toString().includes('5 problems (5 errors, 0 warnings)')) + }) + }) + it('should check FOO1 does not change after test 1a', () => { + result = compareTextFiles(currentFile, beforeFixFile) + expect(result).to.be.true + }) + + describe('--fix with noPrompt', () => { + after(function () { + if (!E2E) { + copyFile(beforeFixFile, currentFile) + } + }) + + it('should compare Foo1 file with template beforeFix file and they should match 1b', () => { + result = compareTextFiles(currentFile, beforeFixFile) + expect(result).to.be.true + }) + + it('should fix file when noPrompt 1b', () => { + 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 = '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 1b', () => { + result = compareTextFiles(currentFile, beforeFixFile) + expect(result).to.be.true + }) + }) + + describe('autofix rule: explicit-types', () => { + before(function () { + params = retrieveParams('explicit-types/') + 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 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( + `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt` + ) + + expect(code).to.equal(1) + + 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', () => { + result = compareTextFiles(currentFile, beforeFixFile) + expect(result).to.be.true + }) + }) + + describe('autofix rule: no-console', () => { + before(function () { + params = retrieveParams('no-console/') + 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 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( + `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt` + ) + + expect(code).to.equal(1) + + 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', () => { + result = compareTextFiles(currentFile, beforeFixFile) + expect(result).to.be.true + }) + }) + + describe('autofix rule: private-vars-leading-underscore', () => { + 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 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( + `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt` + ) + + expect(code).to.equal(1) + + const reportLines = stdout.split('\n') + const finalLine = '27 problems (27 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 4', () => { + 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/e2e/package.json b/e2e/package.json index 3f817075..282f9edc 100644 --- a/e2e/package.json +++ b/e2e/package.json @@ -4,7 +4,7 @@ "description": "E2E tests for solhint", "main": "index.js", "scripts": { - "test": "mocha test.js formatters-test.js" + "test": "mocha test.js formatters-test.js autofix-test.js" }, "author": "", "license": "MIT", @@ -14,6 +14,7 @@ "fs-extra": "^11.1.0", "get-stream": "^6.0.0", "mocha": "^10.2.0", - "shelljs": "^0.8.5" + "shelljs": "^0.8.5", + "spawn-sync": "^2.0.0" } } diff --git a/lib/rules/best-practises/explicit-types.js b/lib/rules/best-practises/explicit-types.js index 266a38dd..3273b340 100644 --- a/lib/rules/best-practises/explicit-types.js +++ b/lib/rules/best-practises/explicit-types.js @@ -58,7 +58,7 @@ const meta = { }, notes: [ { - note: 'Solhint allows this rule to automatically fix the code with `--fix` or `--fixShow` option', + note: 'Solhint allows this rule to automatically fix the code with `--fix` option', }, ], }, diff --git a/lib/rules/security/avoid-sha3.js b/lib/rules/security/avoid-sha3.js index aa0127c5..a8d5c0bb 100644 --- a/lib/rules/security/avoid-sha3.js +++ b/lib/rules/security/avoid-sha3.js @@ -9,7 +9,7 @@ const meta = { category: 'Security Rules', notes: [ { - note: 'Solhint allows this rule to automatically fix the code with `--fix` or `--fixShow` option', + note: 'Solhint allows this rule to automatically fix the code with `--fix` option', }, ], }, diff --git a/lib/rules/security/avoid-throw.js b/lib/rules/security/avoid-throw.js index 279233ff..72a9f4f1 100644 --- a/lib/rules/security/avoid-throw.js +++ b/lib/rules/security/avoid-throw.js @@ -9,7 +9,7 @@ const meta = { category: 'Security Rules', notes: [ { - note: 'Solhint allows this rule to automatically fix the code with `--fix` or `--fixShow` option', + note: 'Solhint allows this rule to automatically fix the code with `--fix` option', }, ], }, diff --git a/solhint.js b/solhint.js index 5f8d5b4a..520a6e5c 100644 --- a/solhint.js +++ b/solhint.js @@ -1,5 +1,4 @@ #!/usr/bin/env node - const program = require('commander') const _ = require('lodash') const fs = require('fs') @@ -29,7 +28,7 @@ function init() { .option('-q, --quiet', 'report errors only - default: false') .option('--ignore-path [file_name]', 'file to use as your .solhintignore') .option('--fix', 'automatically fix problems. Skips fixes in report') - .option('--fixShow', 'automatically fix problems. Show fixes in report') + // .option('--fixShow', 'automatically fix problems. Show fixes in report') .option('--noPrompt', 'do not suggest to backup files when any `fix` option is selected') .option('--init', 'create configuration file for solhint') .option('--disc', 'do not check for solhint updates') @@ -82,10 +81,12 @@ function askUserToContinue(callback) { } function execMainAction() { - if ((program.opts().fix || program.opts().fixShow) && !program.opts().noPrompt) { + // if ((program.opts().fix || program.opts().fixShow) && !program.opts().noPrompt) { + if (program.opts().fix && !program.opts().noPrompt) { askUserToContinue((userAnswer) => { if (userAnswer !== 'y') { console.log('\nProcess terminated by user') + process.exit(0) } else { // User agreed, continue with the operation. continueExecution() @@ -126,7 +127,8 @@ function executeMainActionLogic() { const reportLists = program.args.filter(_.isString).map(processPath) const reports = _.flatten(reportLists) - if (program.opts().fix || program.opts().fixShow) { + // if (program.opts().fix || program.opts().fixShow) { + if (program.opts().fix) { for (const report of reports) { const inputSrc = fs.readFileSync(report.filePath).toString() @@ -138,18 +140,26 @@ function executeMainActionLogic() { const { fixed, output } = applyFixes(fixes, inputSrc) if (fixed) { - // skip or not the report when fixed - if (program.opts().fix) { - report.reports = report.reports.filter((x) => !x.fix) - } else { - // console.log('report.reports :>> ', report.reports) - report.reports.forEach((report) => { - if (report.fix !== null) { - report.message = `[FIXED] - ${report.message}` - } - }) + // // skip or not the report when fixed + // // This was filtering fixed rules so status code was not 1 + // if (program.opts().fix) { + // report.reports = report.reports.filter((x) => !x.fix) + // } else { + // console.log('report.reports :>> ', report.reports) + report.reports.forEach((report) => { + if (report.fix !== null) { + report.message = `[FIXED] - ${report.message}` + } + }) + // } + + // fs.writeFileSync(report.filePath, output) + try { + fs.writeFileSync(report.filePath, output) + // fs.writeFileSync('no-console/Foo1Modified.sol', output) + } catch (error) { + console.error('An error occurred while writing the file:', error) } - fs.writeFileSync(report.filePath, output) } } }