From ccffdcb18f57c9319cb4e9ec110cf7d370404d28 Mon Sep 17 00:00:00 2001 From: "Hans Bergren (0xCLARITY)" Date: Thu, 23 May 2024 10:01:10 -0700 Subject: [PATCH 01/28] Exclude abi.encodeX calls from func-named-parameters --- lib/rules/naming/func-named-parameters.js | 28 ++++++++++++++++++----- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/lib/rules/naming/func-named-parameters.js b/lib/rules/naming/func-named-parameters.js index 170ea4ba..2836d654 100644 --- a/lib/rules/naming/func-named-parameters.js +++ b/lib/rules/naming/func-named-parameters.js @@ -36,6 +36,10 @@ const meta = { description: 'Function call with four NAMED parameters', code: 'functionName({ sender: _senderAddress, amount: 1e18, token: _tokenAddress, receiver: _receiverAddress })', }, + { + description: 'abi.encodeX call with four UNNAMED parameters', + code: 'abi.encodePacked(_senderAddress, 1e18, _tokenAddress, _receiverAddress )', + }, ], bad: [ { @@ -67,12 +71,24 @@ class FunctionNamedParametersChecker extends BaseChecker { const qtyNamed = node.names.length const qtyArgs = node.arguments.length - if (qtyArgs !== 0) { - if (qtyNamed === 0 && qtyArgs > this.maxUnnamedArguments) { - this.error( - node, - `Named parameters missing. MIN unnamed argumenst is ${this.maxUnnamedArguments}` - ) + if (!this.isAbiCall(node)) { + if (qtyArgs !== 0) { + if (qtyNamed === 0 && qtyArgs > this.maxUnnamedArguments) { + this.error( + node, + `Named parameters missing. MIN unnamed argumenst is ${this.maxUnnamedArguments}` + ) + } + } + } + } + + isAbiCall(node) { + if (node.expression.type == 'MemberAccess') { + if (node.expression.expression.type == 'Identifier') { + if (node.expression.expression.name == 'abi') { + return true + } } } } From 3b74499af1f2e78cd521e671a43c3b0343a3ddaa Mon Sep 17 00:00:00 2001 From: "Hans Bergren (0xCLARITY)" Date: Tue, 28 May 2024 09:38:50 -0700 Subject: [PATCH 02/28] Fix linting --- lib/rules/naming/func-named-parameters.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rules/naming/func-named-parameters.js b/lib/rules/naming/func-named-parameters.js index 2836d654..2fd22c69 100644 --- a/lib/rules/naming/func-named-parameters.js +++ b/lib/rules/naming/func-named-parameters.js @@ -84,9 +84,9 @@ class FunctionNamedParametersChecker extends BaseChecker { } isAbiCall(node) { - if (node.expression.type == 'MemberAccess') { - if (node.expression.expression.type == 'Identifier') { - if (node.expression.expression.name == 'abi') { + if (node.expression.type === 'MemberAccess') { + if (node.expression.expression.type === 'Identifier') { + if (node.expression.expression.name === 'abi') { return true } } From 2b2a0a45c56361e3f6e3a4f7ebdbb2eda444511d Mon Sep 17 00:00:00 2001 From: dropbigfish Date: Thu, 13 Jun 2024 11:59:08 +0800 Subject: [PATCH 03/28] chore: fix some comments Signed-off-by: dropbigfish --- docs/rules/naming/named-parameters-mapping.md | 2 +- e2e/test.js | 2 +- lib/rules/naming/func-named-parameters.js | 2 +- lib/rules/naming/named-parameters-mapping.js | 2 +- test/rules/naming/func-named-parameters.js | 6 +++--- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/rules/naming/named-parameters-mapping.md b/docs/rules/naming/named-parameters-mapping.md index 1f3d1a81..fe793ea6 100644 --- a/docs/rules/naming/named-parameters-mapping.md +++ b/docs/rules/naming/named-parameters-mapping.md @@ -39,7 +39,7 @@ mapping(string name => uint256 balance) public users; mapping(address owner => mapping(address token => uint256 balance)) public tokenBalances; ``` -#### Main key of mapping is enforced. On nested mappings other naming are not neccesary +#### Main key of mapping is enforced. On nested mappings other naming are not necessary ```solidity mapping(address owner => mapping(address => uint256)) public tokenBalances; diff --git a/e2e/test.js b/e2e/test.js index 2d77e757..59c2bfd7 100644 --- a/e2e/test.js +++ b/e2e/test.js @@ -96,7 +96,7 @@ describe('e2e', function () { const PATH = '03-no-empty-blocks' const { PREFIX, SUFFIX } = prepareContext(PATH) - it('No contracts to lint should fail with appropiate message', function () { + it('No contracts to lint should fail with appropriate message', function () { const { code, stderr } = shell.exec(`${NODE}solhint Foo1.sol ${SUFFIX}`) expect(code).to.equal(EXIT_CODES.BAD_OPTIONS) diff --git a/lib/rules/naming/func-named-parameters.js b/lib/rules/naming/func-named-parameters.js index 170ea4ba..8d57f757 100644 --- a/lib/rules/naming/func-named-parameters.js +++ b/lib/rules/naming/func-named-parameters.js @@ -71,7 +71,7 @@ class FunctionNamedParametersChecker extends BaseChecker { if (qtyNamed === 0 && qtyArgs > this.maxUnnamedArguments) { this.error( node, - `Named parameters missing. MIN unnamed argumenst is ${this.maxUnnamedArguments}` + `Named parameters missing. MIN unnamed arguments is ${this.maxUnnamedArguments}` ) } } diff --git a/lib/rules/naming/named-parameters-mapping.js b/lib/rules/naming/named-parameters-mapping.js index c170fa62..a798abc8 100644 --- a/lib/rules/naming/named-parameters-mapping.js +++ b/lib/rules/naming/named-parameters-mapping.js @@ -20,7 +20,7 @@ const meta = { }, { description: - 'Main key of mapping is enforced. On nested mappings other naming are not neccesary', + 'Main key of mapping is enforced. On nested mappings other naming are not necessary', code: 'mapping(address owner => mapping(address => uint256)) public tokenBalances;', }, { diff --git a/test/rules/naming/func-named-parameters.js b/test/rules/naming/func-named-parameters.js index c1102d2d..97389e69 100644 --- a/test/rules/naming/func-named-parameters.js +++ b/test/rules/naming/func-named-parameters.js @@ -27,7 +27,7 @@ describe('Linter - func-named-parameters', () => { assertErrorCount(report, 1) assertErrorMessage( report, - `Named parameters missing. MIN unnamed argumenst is ${ + `Named parameters missing. MIN unnamed arguments is ${ minUnnamed < DEFAULT_MIN_UNNAMED_ARGUMENTS ? DEFAULT_MIN_UNNAMED_ARGUMENTS : minUnnamed }` ) @@ -90,7 +90,7 @@ describe('Linter - func-named-parameters', () => { assertErrorMessage( report, - `Named parameters missing. MIN unnamed argumenst is ${DEFAULT_MIN_UNNAMED_ARGUMENTS}` + `Named parameters missing. MIN unnamed arguments is ${DEFAULT_MIN_UNNAMED_ARGUMENTS}` ) }) @@ -106,7 +106,7 @@ describe('Linter - func-named-parameters', () => { assertErrorMessage( report, - `Named parameters missing. MIN unnamed argumenst is ${DEFAULT_MIN_UNNAMED_ARGUMENTS}` + `Named parameters missing. MIN unnamed arguments is ${DEFAULT_MIN_UNNAMED_ARGUMENTS}` ) }) }) From b9530be9f4accc068005f22c3892591a189b0981 Mon Sep 17 00:00:00 2001 From: "Hans Bergren (0xCLARITY)" Date: Fri, 28 Jun 2024 11:46:17 -0700 Subject: [PATCH 04/28] Fix typo in error msg --- lib/rules/naming/func-named-parameters.js | 2 +- test/rules/naming/func-named-parameters.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/rules/naming/func-named-parameters.js b/lib/rules/naming/func-named-parameters.js index 2fd22c69..3f821f84 100644 --- a/lib/rules/naming/func-named-parameters.js +++ b/lib/rules/naming/func-named-parameters.js @@ -76,7 +76,7 @@ class FunctionNamedParametersChecker extends BaseChecker { if (qtyNamed === 0 && qtyArgs > this.maxUnnamedArguments) { this.error( node, - `Named parameters missing. MIN unnamed argumenst is ${this.maxUnnamedArguments}` + `Named parameters missing. MIN unnamed arguments is ${this.maxUnnamedArguments}` ) } } diff --git a/test/rules/naming/func-named-parameters.js b/test/rules/naming/func-named-parameters.js index c1102d2d..97389e69 100644 --- a/test/rules/naming/func-named-parameters.js +++ b/test/rules/naming/func-named-parameters.js @@ -27,7 +27,7 @@ describe('Linter - func-named-parameters', () => { assertErrorCount(report, 1) assertErrorMessage( report, - `Named parameters missing. MIN unnamed argumenst is ${ + `Named parameters missing. MIN unnamed arguments is ${ minUnnamed < DEFAULT_MIN_UNNAMED_ARGUMENTS ? DEFAULT_MIN_UNNAMED_ARGUMENTS : minUnnamed }` ) @@ -90,7 +90,7 @@ describe('Linter - func-named-parameters', () => { assertErrorMessage( report, - `Named parameters missing. MIN unnamed argumenst is ${DEFAULT_MIN_UNNAMED_ARGUMENTS}` + `Named parameters missing. MIN unnamed arguments is ${DEFAULT_MIN_UNNAMED_ARGUMENTS}` ) }) @@ -106,7 +106,7 @@ describe('Linter - func-named-parameters', () => { assertErrorMessage( report, - `Named parameters missing. MIN unnamed argumenst is ${DEFAULT_MIN_UNNAMED_ARGUMENTS}` + `Named parameters missing. MIN unnamed arguments is ${DEFAULT_MIN_UNNAMED_ARGUMENTS}` ) }) }) From 6c338401cb7c7ec0c99a84615d046315743ec7ad Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 1 Jul 2024 15:37:59 -0300 Subject: [PATCH 05/28] add: import-order rule --- conf/rulesets/solhint-all.js | 1 + lib/rules/naming/import-order.js | 233 +++++++++++++++++++++++++++++++ lib/rules/naming/index.js | 2 + 3 files changed, 236 insertions(+) create mode 100644 lib/rules/naming/import-order.js diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js index b10c1e1a..71c6a849 100644 --- a/conf/rulesets/solhint-all.js +++ b/conf/rulesets/solhint-all.js @@ -50,6 +50,7 @@ module.exports = Object.freeze({ immutablesAsConstants: true, }, ], + 'import-order': ['warn', 4], 'modifier-name-mixedcase': 'warn', 'named-parameters-mapping': 'warn', 'private-vars-leading-underscore': [ diff --git a/lib/rules/naming/import-order.js b/lib/rules/naming/import-order.js new file mode 100644 index 00000000..7abb7dfc --- /dev/null +++ b/lib/rules/naming/import-order.js @@ -0,0 +1,233 @@ +const BaseChecker = require('../base-checker') +const { severityDescription } = require('../../doc/utils') + +const FOLDER_LEVELS_SUPPORT = 10 +const DEFAULT_SEVERITY = 'warn' + +const ruleId = 'import-order' +const meta = { + type: 'naming', + + docs: { + description: `Order the imports of the contract to follow a certain hierarchy`, + category: 'Style Guide Rules', + options: [ + { + description: severityDescription, + default: DEFAULT_SEVERITY, + }, + ], + notes: [ + { + note: 'Order by hierarchy of directories first, e.g. ../../ comes before ../, which comes before ./, which comes before ./foo', + }, + { + note: 'Order alphabetically for each file at the same level, e.g. ./bar comes before ./foo', + }, + { + note: 'Rule support up to 10 folder levels "../../../../../../../../../../"', + }, + ], + }, + + isDefault: false, + recommended: false, + defaultSetup: 'warn', + fixable: true, + schema: null, +} + +class ImportOrderChecker extends BaseChecker { + constructor(reporter) { + super(reporter, ruleId, meta) + } + + SourceUnit(node) { + // get all the imports into one object + this.fromContractImports = node.children + .filter((child) => child.type === 'ImportDirective') + .map((child) => ({ + range: child.range, + path: child.path, + fullSentence: child.symbolAliases + ? this.getFullSentence(child.symbolAliases) + '"' + child.path + '";' + : `import "${child.path}";`, + })) + + // copy the object into another one + this.orderedImports = [...this.fromContractImports] + + // order the object to get the ordered and the contract order + this.orderImports(this.orderedImports) + } + + 'SourceUnit:exit'(node) { + // when finish analyzing check if ordered import array is equal to the import contract order + const areEqual = areArraysEqual(this.fromContractImports, this.orderedImports) + + // if are equal do nothing, if not enter the if + if (!areEqual) { + // Find the lowest starting range to start the replacement + let currentStart = Math.min(...this.fromContractImports.map((imp) => imp.range[0])) + // Prepare replacements changing the range + const replacements = this.orderedImports.map((orderedImport) => { + const newText = orderedImport.fullSentence + const rangeEnd = currentStart + newText.length + + const replacement = { + range: [currentStart, rangeEnd], + newText, + } + + currentStart = rangeEnd + + return replacement + }) + + // get the name of the contract only to report the error + let name = '' + const loopQty = replacements.length - 1 + // loop through imports to report and correct if requested + for (let i = loopQty; i >= 0; i--) { + name = this.fromContractImports[i].path.split('/') + name = name[name.length - 1] + this.error( + node, + `Wrong Import Order for {${name}}`, + // replace from bottom to top all the imports in the right order + // flag the first one, which will be the last import + this.fixStatement(replacements[i], i === loopQty) + ) + } + } + } + + fixStatement(replacement, isLast) { + // the last import should replace all the chars up to the end + // this is for imports path longer than the one it was before + if (isLast) { + const lastRangeEnd = this.fromContractImports[this.fromContractImports.length - 1].range[1] + return (fixer) => + fixer.replaceTextRange([replacement.range[0], lastRangeEnd], replacement.newText) + } + return (fixer) => fixer.replaceTextRange(replacement.range, replacement.newText + '\n') + } + + orderImports(imports) { + // it supports up to ten folder of hierarchy + // ===>> ../../../../../../../../../../ + // generate that hierarchy and put @ first + const generateDirHierarchy = (levels) => { + const hierarchy = ['@'] + for (let i = levels; i > 0; i--) { + hierarchy.push('../'.repeat(i)) + } + hierarchy.push('./') + return hierarchy + } + + const dirHierarchy = generateDirHierarchy(FOLDER_LEVELS_SUPPORT) + + // get dir level to compare and order + const getDirLevel = (path) => { + for (let i = 0; i < dirHierarchy.length; i++) { + if (path.startsWith(dirHierarchy[i])) { + return i + } + } + return dirHierarchy.length // Default level if no match + } + + // count the folders to put first if there's an only file at the same level + const getSubDirCount = (path) => { + // Count number of slashes in the path + return (path.match(/\//g) || []).length + } + + // compare paths alphabetically + const comparePathsAlphabetically = (pathA, pathB) => { + const partsA = pathA.split('/') + const partsB = pathB.split('/') + + for (let i = 0; i < Math.min(partsA.length, partsB.length); i++) { + const comparison = partsA[i].localeCompare(partsB[i]) + if (comparison !== 0) { + return comparison + } + } + + return 0 // Equal parts up to the length of the shorter path + } + + // check if there's a single file and put it at last + const isSingleFile = (path) => { + const parts = path.split('/') + return parts.length === 2 && parts[1].includes('.') + } + + return imports.sort((a, b) => { + const dirLevelA = getDirLevel(a.path) + const dirLevelB = getDirLevel(b.path) + + if (dirLevelA !== dirLevelB) { + return dirLevelA - dirLevelB + } + + const singleFileA = isSingleFile(a.path) + const singleFileB = isSingleFile(b.path) + + if (singleFileA !== singleFileB) { + return singleFileA - singleFileB // Place single file imports last at each level + } + + const alphabeticComparison = comparePathsAlphabetically(a.path, b.path) + if (alphabeticComparison !== 0) { + return alphabeticComparison + } + + const subDirCountA = getSubDirCount(a.path) + const subDirCountB = getSubDirCount(b.path) + + return subDirCountB - subDirCountA // More subdirectories come first + }) + } + + // function to get the full sentence, because the AST only get the path and alias separated + getFullSentence(node) { + const sentenceStart = 'import {' + const sentenceEnd = '} from ' + let imported = '' + + if (node.length === 1) { + imported = node[0][0] + } else { + for (let i = 0; i < node.length; i++) { + if (i === node.length - 1) imported += node[i][0] + else imported += node[i][0] + ', ' + } + } + + return sentenceStart + imported + sentenceEnd + } +} + +// function to compare is both arrays are equal +function areArraysEqual(arr1, arr2) { + if (arr1.length !== arr2.length) { + return false + } + + for (let i = 0; i < arr1.length; i++) { + if ( + arr1[i].path !== arr2[i].path || + arr1[i].range[0] !== arr2[i].range[0] || + arr1[i].range[1] !== arr2[i].range[1] + ) { + return false + } + } + + return true +} + +module.exports = ImportOrderChecker diff --git a/lib/rules/naming/index.js b/lib/rules/naming/index.js index 43ead75b..84448d79 100644 --- a/lib/rules/naming/index.js +++ b/lib/rules/naming/index.js @@ -11,6 +11,7 @@ const NamedParametersMappingChecker = require('./named-parameters-mapping') const ImmutableVarsNamingChecker = require('./immutable-vars-naming') const FunctionNamedParametersChecker = require('./func-named-parameters') const FoundryTestFunctionsChecker = require('./foundry-test-functions') +const ImportOrderChecker = require('./import-order') module.exports = function checkers(reporter, config) { return [ @@ -27,5 +28,6 @@ module.exports = function checkers(reporter, config) { new ImmutableVarsNamingChecker(reporter, config), new FunctionNamedParametersChecker(reporter, config), new FoundryTestFunctionsChecker(reporter, config), + new ImportOrderChecker(reporter, config), ] } From 0ae84e34bececf3bc3d5dd0cd216e98fc91648a7 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 1 Jul 2024 15:38:47 -0300 Subject: [PATCH 06/28] add: import-order rule --- conf/rulesets/solhint-all.js | 2 +- docs/rules.md | 1 + docs/rules/naming/import-order.md | 40 +++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 docs/rules/naming/import-order.md diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js index 71c6a849..060d3a4e 100644 --- a/conf/rulesets/solhint-all.js +++ b/conf/rulesets/solhint-all.js @@ -50,7 +50,7 @@ module.exports = Object.freeze({ immutablesAsConstants: true, }, ], - 'import-order': ['warn', 4], + 'import-order': 'warn', 'modifier-name-mixedcase': 'warn', 'named-parameters-mapping': 'warn', 'private-vars-leading-underscore': [ diff --git a/docs/rules.md b/docs/rules.md index 61bc0c8b..3af1e879 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -37,6 +37,7 @@ title: "Rule Index of Solhint" | [func-named-parameters](./rules/naming/func-named-parameters.md) | Enforce named parameters for function calls with 4 or more arguments. This rule may have some false positives | | | | [func-param-name-mixedcase](./rules/naming/func-param-name-mixedcase.md) | Function param name must be in mixedCase. | | | | [immutable-vars-naming](./rules/naming/immutable-vars-naming.md) | Check Immutable variables. Capitalized SNAKE_CASE or mixedCase depending on configuration. | $~~~~~~~~$✔️ | | +| [import-order](./rules/naming/import-order.md) | Order the imports of the contract to follow a certain hierarchy | | | | [modifier-name-mixedcase](./rules/naming/modifier-name-mixedcase.md) | Modifier name must be in mixedCase. | | | | [named-parameters-mapping](./rules/naming/named-parameters-mapping.md) | Solidity v0.8.18 introduced named parameters on the mappings definition. | | | | [private-vars-leading-underscore](./rules/naming/private-vars-leading-underscore.md) | Non-external functions and state variables should start with a single underscore. Others, shouldn't | | | diff --git a/docs/rules/naming/import-order.md b/docs/rules/naming/import-order.md new file mode 100644 index 00000000..3bf54c00 --- /dev/null +++ b/docs/rules/naming/import-order.md @@ -0,0 +1,40 @@ +--- +warning: "This is a dynamically generated file. Do not edit manually." +layout: "default" +title: "import-order | Solhint" +--- + +# import-order +![Category Badge](https://img.shields.io/badge/-Style%20Guide%20Rules-informational) +![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow) + +## Description +Order the imports of the contract to follow a certain hierarchy + +## Options +This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn. + +### Example Config +```json +{ + "rules": { + "import-order": "warn" + } +} +``` + +### Notes +- Order by hierarchy of directories first, e.g. ../../ comes before ../, which comes before ./, which comes before ./foo +- Order alphabetically for each file at the same level, e.g. ./bar comes before ./foo +- Rule support up to 10 folder levels "../../../../../../../../../../" + +## Examples +This rule does not have examples. + +## Version +This rule is introduced in the latest version. + +## Resources +- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/naming/import-order.js) +- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/naming/import-order.md) +- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/naming/import-order.js) From 93b02f4d6efda9c35a9588a795456a05a4e545a2 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 1 Jul 2024 15:57:59 -0300 Subject: [PATCH 07/28] autofix e2e tests --- e2e/08-autofix/imports-order/.solhint.json | 5 + e2e/08-autofix/imports-order/Foo1.sol | 17 ++++ e2e/08-autofix/imports-order/Foo1AfterFix.sol | 17 ++++ .../imports-order/Foo1BeforeFix.sol | 17 ++++ e2e/08-autofix/imports-order/Foo2.sol | 19 ++++ e2e/08-autofix/imports-order/Foo2AfterFix.sol | 19 ++++ .../imports-order/Foo2BeforeFix.sol | 19 ++++ e2e/autofix-test.js | 99 ++++++++++++++++++- .../{import-order.js => imports-order.js} | 6 +- lib/rules/naming/index.js | 4 +- 10 files changed, 214 insertions(+), 8 deletions(-) create mode 100644 e2e/08-autofix/imports-order/.solhint.json create mode 100644 e2e/08-autofix/imports-order/Foo1.sol create mode 100644 e2e/08-autofix/imports-order/Foo1AfterFix.sol create mode 100644 e2e/08-autofix/imports-order/Foo1BeforeFix.sol create mode 100644 e2e/08-autofix/imports-order/Foo2.sol create mode 100644 e2e/08-autofix/imports-order/Foo2AfterFix.sol create mode 100644 e2e/08-autofix/imports-order/Foo2BeforeFix.sol rename lib/rules/naming/{import-order.js => imports-order.js} (98%) diff --git a/e2e/08-autofix/imports-order/.solhint.json b/e2e/08-autofix/imports-order/.solhint.json new file mode 100644 index 00000000..b17afc51 --- /dev/null +++ b/e2e/08-autofix/imports-order/.solhint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "imports-order": "error" + } +} diff --git a/e2e/08-autofix/imports-order/Foo1.sol b/e2e/08-autofix/imports-order/Foo1.sol new file mode 100644 index 00000000..24e01492 --- /dev/null +++ b/e2e/08-autofix/imports-order/Foo1.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +import {Foo1} from './Foo1.sol'; +import '../token/interfaces/IXTokenWrapper2.sol'; +import {IXTokenFactory2} from '../../atoken/interfaces/IXTokenFactory2.sol'; +import {Initializable} from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; +import '../token/interfaces/IXTokenWrapper.sol'; +import {IXTokenFactory, holaquetal} from '../../token/interfaces/IXTokenFactory.sol'; +import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; +import {Afool1} from './Afool1.sol'; +import {IXTokenWrapper2} from '../token/interfaces/IXTokenWrapper2.sol'; +import {ReentrancyGuardUpgradeable2} from '@Apenzeppelin/contracts-upgradeable/ReentrancyGuardUpgradeable2.sol'; + +contract ImportsOrder { + constructor() {} +} diff --git a/e2e/08-autofix/imports-order/Foo1AfterFix.sol b/e2e/08-autofix/imports-order/Foo1AfterFix.sol new file mode 100644 index 00000000..a725462b --- /dev/null +++ b/e2e/08-autofix/imports-order/Foo1AfterFix.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +import {ReentrancyGuardUpgradeable2} from '@Apenzeppelin/contracts-upgradeable/ReentrancyGuardUpgradeable2.sol'; +import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; +import {IXTokenFactory2} from '../../atoken/interfaces/IXTokenFactory2.sol'; +import {IXTokenFactory, holaquetal} from '../../token/interfaces/IXTokenFactory.sol'; +import '../token/interfaces/IXTokenWrapper.sol'; +import {IXTokenWrapper2} from '../token/interfaces/IXTokenWrapper2.sol'; +import '../token/interfaces/IXTokenWrapper3.sol'; +import {Initializable} from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; +import {Afool1} from './Afool1.sol'; +import {Foo1} from './Foo1.sol'; + +contract ImportsOrder { + constructor() {} +} diff --git a/e2e/08-autofix/imports-order/Foo1BeforeFix.sol b/e2e/08-autofix/imports-order/Foo1BeforeFix.sol new file mode 100644 index 00000000..24e01492 --- /dev/null +++ b/e2e/08-autofix/imports-order/Foo1BeforeFix.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +import {Foo1} from './Foo1.sol'; +import '../token/interfaces/IXTokenWrapper2.sol'; +import {IXTokenFactory2} from '../../atoken/interfaces/IXTokenFactory2.sol'; +import {Initializable} from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; +import '../token/interfaces/IXTokenWrapper.sol'; +import {IXTokenFactory, holaquetal} from '../../token/interfaces/IXTokenFactory.sol'; +import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; +import {Afool1} from './Afool1.sol'; +import {IXTokenWrapper2} from '../token/interfaces/IXTokenWrapper2.sol'; +import {ReentrancyGuardUpgradeable2} from '@Apenzeppelin/contracts-upgradeable/ReentrancyGuardUpgradeable2.sol'; + +contract ImportsOrder { + constructor() {} +} diff --git a/e2e/08-autofix/imports-order/Foo2.sol b/e2e/08-autofix/imports-order/Foo2.sol new file mode 100644 index 00000000..5912b628 --- /dev/null +++ b/e2e/08-autofix/imports-order/Foo2.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +import {Foo1} from './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOne.sol'; +import '../../../../../token/interfaces/IXTokenWrapper3.sol'; +import {IXTokenFactory2} from '../../atoken/interfaces/IXTokenFactory2.sol'; +import {Initializable} from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; +import '../../../../token/interfaces/FakeContract1.sol'; +import '../token/interfaces/IXTokenWrapper.sol'; +import {IXTokenFactory, holaquetal} from '../../token/interfaces/IXTokenFactory.sol'; +import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; +import {FakeContract2} from '../../../token/interfaces/FakeContract2.sol'; +import {Afool1} from './Afool1.sol'; +import {IXTokenWrapper2} from '../token/interfaces/IXTokenWrapper2.sol'; +import {ReentrancyGuardUpgradeable2} from '@Apenzeppelin/ReentrancyGuardUpgradeable2.sol'; + +contract ImportsOrder2 { + constructor() {} +} diff --git a/e2e/08-autofix/imports-order/Foo2AfterFix.sol b/e2e/08-autofix/imports-order/Foo2AfterFix.sol new file mode 100644 index 00000000..8b5e0961 --- /dev/null +++ b/e2e/08-autofix/imports-order/Foo2AfterFix.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +import {ReentrancyGuardUpgradeable2} from '@Apenzeppelin/ReentrancyGuardUpgradeable2.sol'; +import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; +import '../../../../../token/interfaces/IXTokenWrapper3.sol'; +import '../../../../token/interfaces/FakeContract1.sol'; +import {FakeContract2} from '../../../token/interfaces/FakeContract2.sol'; +import {IXTokenFactory2} from '../../atoken/interfaces/IXTokenFactory2.sol'; +import {IXTokenFactory, holaquetal} from '../../token/interfaces/IXTokenFactory.sol'; +import '../token/interfaces/IXTokenWrapper.sol'; +import {IXTokenWrapper2} from '../token/interfaces/IXTokenWrapper2.sol'; +import {Initializable} from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; +import {Afool1} from './Afool1.sol'; +import './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOne.sol'; + +contract ImportsOrder2 { + constructor() {} +} diff --git a/e2e/08-autofix/imports-order/Foo2BeforeFix.sol b/e2e/08-autofix/imports-order/Foo2BeforeFix.sol new file mode 100644 index 00000000..5912b628 --- /dev/null +++ b/e2e/08-autofix/imports-order/Foo2BeforeFix.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +import {Foo1} from './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOne.sol'; +import '../../../../../token/interfaces/IXTokenWrapper3.sol'; +import {IXTokenFactory2} from '../../atoken/interfaces/IXTokenFactory2.sol'; +import {Initializable} from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; +import '../../../../token/interfaces/FakeContract1.sol'; +import '../token/interfaces/IXTokenWrapper.sol'; +import {IXTokenFactory, holaquetal} from '../../token/interfaces/IXTokenFactory.sol'; +import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; +import {FakeContract2} from '../../../token/interfaces/FakeContract2.sol'; +import {Afool1} from './Afool1.sol'; +import {IXTokenWrapper2} from '../token/interfaces/IXTokenWrapper2.sol'; +import {ReentrancyGuardUpgradeable2} from '@Apenzeppelin/ReentrancyGuardUpgradeable2.sol'; + +contract ImportsOrder2 { + constructor() {} +} diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index 3edaaa29..9295f69d 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -458,7 +458,7 @@ describe('e2e', function () { expect(reportLines[reportLines.length - 3]).to.contain(finalLine) }) }) - + it('should check FOO1 does not change after test (7)', () => { result = compareTextFiles(currentFile, beforeFixFile) expect(result).to.be.true @@ -504,7 +504,7 @@ describe('e2e', function () { 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 @@ -550,12 +550,105 @@ describe('e2e', function () { 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 }) }) + + describe.only('autofix rule: imports-order P1', () => { + describe('autofix rule: imports-order Foo1', () => { + before(function () { + params = retrieveParams('imports-order/') + 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 (9)', () => { + result = compareTextFiles(currentFile, beforeFixFile) + expect(result).to.be.true + }) + + it('should execute and compare Foo1 with template AFTER FIX and they should match (9)', () => { + ;({ 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 (9)', () => { + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) + }) + + it('should get the right report (9)', () => { + const reportLines = stdout.split('\n') + const finalLine = '10 problems (10 errors, 0 warnings)' + expect(reportLines[reportLines.length - 3]).to.contain(finalLine) + }) + }) + + it('should check FOO1 does not change after test (9)', () => { + result = compareTextFiles(currentFile, beforeFixFile) + expect(result).to.be.true + }) + }) + describe('autofix rule: imports-order Foo2', () => { + before(function () { + params = retrieveParams('imports-order/') + currentConfig = `${params.path}${params.subpath}.solhint.json` + currentFile = `${params.path}${params.subpath}Foo2.sol` + beforeFixFile = `${params.path}${params.subpath}Foo2BeforeFix.sol` + afterFixFile = `${params.path}${params.subpath}Foo2AfterFix.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 (10)', () => { + result = compareTextFiles(currentFile, beforeFixFile) + expect(result).to.be.true + }) + + it('should execute and compare Foo1 with template AFTER FIX and they should match (10)', () => { + ;({ 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 (10)', () => { + expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) + }) + + it('should get the right report (10)', () => { + const reportLines = stdout.split('\n') + const finalLine = '12 problems (12 errors, 0 warnings)' + expect(reportLines[reportLines.length - 3]).to.contain(finalLine) + }) + }) + + it('should check FOO1 does not change after test (10)', () => { + result = compareTextFiles(currentFile, beforeFixFile) + expect(result).to.be.true + }) + }) + }) }) // FALTA LA PRUEBA DEL STORE TO FILE diff --git a/lib/rules/naming/import-order.js b/lib/rules/naming/imports-order.js similarity index 98% rename from lib/rules/naming/import-order.js rename to lib/rules/naming/imports-order.js index 7abb7dfc..5dd9cdfe 100644 --- a/lib/rules/naming/import-order.js +++ b/lib/rules/naming/imports-order.js @@ -4,7 +4,7 @@ const { severityDescription } = require('../../doc/utils') const FOLDER_LEVELS_SUPPORT = 10 const DEFAULT_SEVERITY = 'warn' -const ruleId = 'import-order' +const ruleId = 'imports-order' const meta = { type: 'naming', @@ -37,7 +37,7 @@ const meta = { schema: null, } -class ImportOrderChecker extends BaseChecker { +class ImportsOrderChecker extends BaseChecker { constructor(reporter) { super(reporter, ruleId, meta) } @@ -230,4 +230,4 @@ function areArraysEqual(arr1, arr2) { return true } -module.exports = ImportOrderChecker +module.exports = ImportsOrderChecker diff --git a/lib/rules/naming/index.js b/lib/rules/naming/index.js index 84448d79..2949ad35 100644 --- a/lib/rules/naming/index.js +++ b/lib/rules/naming/index.js @@ -11,7 +11,7 @@ const NamedParametersMappingChecker = require('./named-parameters-mapping') const ImmutableVarsNamingChecker = require('./immutable-vars-naming') const FunctionNamedParametersChecker = require('./func-named-parameters') const FoundryTestFunctionsChecker = require('./foundry-test-functions') -const ImportOrderChecker = require('./import-order') +const ImportsOrderChecker = require('./imports-order') module.exports = function checkers(reporter, config) { return [ @@ -28,6 +28,6 @@ module.exports = function checkers(reporter, config) { new ImmutableVarsNamingChecker(reporter, config), new FunctionNamedParametersChecker(reporter, config), new FoundryTestFunctionsChecker(reporter, config), - new ImportOrderChecker(reporter, config), + new ImportsOrderChecker(reporter, config), ] } From 6fc5b55526b2411b8c1330a96a54c87f09877db0 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 1 Jul 2024 15:58:16 -0300 Subject: [PATCH 08/28] autofix e2e tests --- conf/rulesets/solhint-all.js | 2 +- docs/rules.md | 2 +- docs/rules/naming/imports-order.md | 40 ++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 docs/rules/naming/imports-order.md diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js index 060d3a4e..06b9fc5a 100644 --- a/conf/rulesets/solhint-all.js +++ b/conf/rulesets/solhint-all.js @@ -50,7 +50,7 @@ module.exports = Object.freeze({ immutablesAsConstants: true, }, ], - 'import-order': 'warn', + 'imports-order': 'warn', 'modifier-name-mixedcase': 'warn', 'named-parameters-mapping': 'warn', 'private-vars-leading-underscore': [ diff --git a/docs/rules.md b/docs/rules.md index 3af1e879..1541ec02 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -37,7 +37,7 @@ title: "Rule Index of Solhint" | [func-named-parameters](./rules/naming/func-named-parameters.md) | Enforce named parameters for function calls with 4 or more arguments. This rule may have some false positives | | | | [func-param-name-mixedcase](./rules/naming/func-param-name-mixedcase.md) | Function param name must be in mixedCase. | | | | [immutable-vars-naming](./rules/naming/immutable-vars-naming.md) | Check Immutable variables. Capitalized SNAKE_CASE or mixedCase depending on configuration. | $~~~~~~~~$✔️ | | -| [import-order](./rules/naming/import-order.md) | Order the imports of the contract to follow a certain hierarchy | | | +| [imports-order](./rules/naming/imports-order.md) | Order the imports of the contract to follow a certain hierarchy | | | | [modifier-name-mixedcase](./rules/naming/modifier-name-mixedcase.md) | Modifier name must be in mixedCase. | | | | [named-parameters-mapping](./rules/naming/named-parameters-mapping.md) | Solidity v0.8.18 introduced named parameters on the mappings definition. | | | | [private-vars-leading-underscore](./rules/naming/private-vars-leading-underscore.md) | Non-external functions and state variables should start with a single underscore. Others, shouldn't | | | diff --git a/docs/rules/naming/imports-order.md b/docs/rules/naming/imports-order.md new file mode 100644 index 00000000..05f04128 --- /dev/null +++ b/docs/rules/naming/imports-order.md @@ -0,0 +1,40 @@ +--- +warning: "This is a dynamically generated file. Do not edit manually." +layout: "default" +title: "imports-order | Solhint" +--- + +# imports-order +![Category Badge](https://img.shields.io/badge/-Style%20Guide%20Rules-informational) +![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow) + +## Description +Order the imports of the contract to follow a certain hierarchy + +## Options +This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn. + +### Example Config +```json +{ + "rules": { + "imports-order": "warn" + } +} +``` + +### Notes +- Order by hierarchy of directories first, e.g. ../../ comes before ../, which comes before ./, which comes before ./foo +- Order alphabetically for each file at the same level, e.g. ./bar comes before ./foo +- Rule support up to 10 folder levels "../../../../../../../../../../" + +## Examples +This rule does not have examples. + +## Version +This rule is introduced in the latest version. + +## Resources +- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/naming/imports-order.js) +- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/naming/imports-order.md) +- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/naming/imports-order.js) From 00325ef6571a0972ee5faf7f42756a9595abda63 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 1 Jul 2024 16:04:55 -0300 Subject: [PATCH 09/28] autofix e2e tests2 --- e2e/autofix-test.js | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index 9295f69d..5dfa1f67 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -526,12 +526,12 @@ describe('e2e', function () { }) describe('--fix with noPrompt', () => { - it('should compare Foo1 file with template BEFORE FIX file and they should match (8)', () => { + it('should compare Foo1 file with template BEFORE FIX file and they should match (9)', () => { result = compareTextFiles(currentFile, beforeFixFile) expect(result).to.be.true }) - it('should execute and compare Foo1 with template AFTER FIX and they should match (8)', () => { + it('should execute and compare Foo1 with template AFTER FIX and they should match (9)', () => { ;({ code, stdout } = shell.exec( `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt` )) @@ -540,24 +540,24 @@ describe('e2e', function () { expect(result).to.be.true }) - it('should execute and exit with code 1 (8)', () => { + it('should execute and exit with code 1 (9)', () => { expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) }) - it('should get the right report (8)', () => { + it('should get the right report (9)', () => { 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)', () => { + it('should check FOO1 does not change after test (9)', () => { result = compareTextFiles(currentFile, beforeFixFile) expect(result).to.be.true }) }) - describe.only('autofix rule: imports-order P1', () => { + describe('autofix rule: imports-order P1', () => { describe('autofix rule: imports-order Foo1', () => { before(function () { params = retrieveParams('imports-order/') @@ -573,12 +573,12 @@ describe('e2e', function () { }) describe('--fix with noPrompt', () => { - it('should compare Foo1 file with template BEFORE FIX file and they should match (9)', () => { + it('should compare Foo1 file with template BEFORE FIX file and they should match (10)', () => { result = compareTextFiles(currentFile, beforeFixFile) expect(result).to.be.true }) - it('should execute and compare Foo1 with template AFTER FIX and they should match (9)', () => { + it('should execute and compare Foo1 with template AFTER FIX and they should match (10)', () => { ;({ code, stdout } = shell.exec( `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt` )) @@ -587,18 +587,18 @@ describe('e2e', function () { expect(result).to.be.true }) - it('should execute and exit with code 1 (9)', () => { + it('should execute and exit with code 1 (10)', () => { expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) }) - it('should get the right report (9)', () => { + it('should get the right report (10)', () => { const reportLines = stdout.split('\n') const finalLine = '10 problems (10 errors, 0 warnings)' expect(reportLines[reportLines.length - 3]).to.contain(finalLine) }) }) - it('should check FOO1 does not change after test (9)', () => { + it('should check FOO1 does not change after test (10)', () => { result = compareTextFiles(currentFile, beforeFixFile) expect(result).to.be.true }) @@ -618,12 +618,12 @@ describe('e2e', function () { }) describe('--fix with noPrompt', () => { - it('should compare Foo1 file with template BEFORE FIX file and they should match (10)', () => { + it('should compare Foo1 file with template BEFORE FIX file and they should match (11)', () => { result = compareTextFiles(currentFile, beforeFixFile) expect(result).to.be.true }) - it('should execute and compare Foo1 with template AFTER FIX and they should match (10)', () => { + it('should execute and compare Foo1 with template AFTER FIX and they should match (11)', () => { ;({ code, stdout } = shell.exec( `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt` )) @@ -632,18 +632,18 @@ describe('e2e', function () { expect(result).to.be.true }) - it('should execute and exit with code 1 (10)', () => { + it('should execute and exit with code 1 (11)', () => { expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) }) - it('should get the right report (10)', () => { + it('should get the right report (11)', () => { const reportLines = stdout.split('\n') const finalLine = '12 problems (12 errors, 0 warnings)' expect(reportLines[reportLines.length - 3]).to.contain(finalLine) }) }) - it('should check FOO1 does not change after test (10)', () => { + it('should check FOO1 does not change after test (11)', () => { result = compareTextFiles(currentFile, beforeFixFile) expect(result).to.be.true }) From 79ecd108a3deeb8e4cadf7adaf5a56dbb80d2d6d Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 1 Jul 2024 16:13:05 -0300 Subject: [PATCH 10/28] autofix e2e tests3 --- e2e/autofix-test.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index 5dfa1f67..611e11aa 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -582,7 +582,13 @@ describe('e2e', function () { ;({ code, stdout } = shell.exec( `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt` )) - + console.log('================================================================================='); + console.log('================================================================================='); + console.log('================================================================================='); + console.log('currentFile :>> ', currentFile); + console.log('================================================================================='); + console.log('================================================================================='); + console.log('================================================================================='); result = compareTextFiles(currentFile, afterFixFile) expect(result).to.be.true }) @@ -603,7 +609,7 @@ describe('e2e', function () { expect(result).to.be.true }) }) - describe('autofix rule: imports-order Foo2', () => { + xdescribe('autofix rule: imports-order Foo2', () => { before(function () { params = retrieveParams('imports-order/') currentConfig = `${params.path}${params.subpath}.solhint.json` From 35adc1bcae1f8faaf2ba2b85906fc0de3da6705a Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 1 Jul 2024 16:16:37 -0300 Subject: [PATCH 11/28] autofix e2e tests4 --- e2e/autofix-test.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index 611e11aa..de448175 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -26,6 +26,13 @@ function retrieveParams(subpath) { function compareTextFiles(file1Path, file2Path) { const file1Content = fs.readFileSync(file1Path, 'utf-8') const file2Content = fs.readFileSync(file2Path, 'utf-8') + + console.log('=================================================================================') + console.log('=================================================================================') + console.log('=================================================================================') + console.log('file2Content: ', file2Content) + console.log('=================================================================================') + console.log('=================================================================================') return file1Content === file2Content } @@ -582,13 +589,6 @@ describe('e2e', function () { ;({ code, stdout } = shell.exec( `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt` )) - console.log('================================================================================='); - console.log('================================================================================='); - console.log('================================================================================='); - console.log('currentFile :>> ', currentFile); - console.log('================================================================================='); - console.log('================================================================================='); - console.log('================================================================================='); result = compareTextFiles(currentFile, afterFixFile) expect(result).to.be.true }) From f7d1991ff8b9d6f54cf0c0d0baebac21d101193d Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 1 Jul 2024 16:19:54 -0300 Subject: [PATCH 12/28] autofix e2e tests5 --- e2e/autofix-test.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index de448175..2f80e0fc 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -27,6 +27,10 @@ function compareTextFiles(file1Path, file2Path) { const file1Content = fs.readFileSync(file1Path, 'utf-8') const file2Content = fs.readFileSync(file2Path, 'utf-8') + console.log('=================================================================================') + console.log('=================================================================================') + console.log('file2Content: ', file1Content) + console.log('=================================================================================') console.log('=================================================================================') console.log('=================================================================================') console.log('=================================================================================') From f51ea6ab5d1743bff3d1e042a24785731f4beeee Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 1 Jul 2024 16:24:39 -0300 Subject: [PATCH 13/28] autofix e2e tests5 --- e2e/08-autofix/imports-order/Foo1.sol | 2 +- e2e/08-autofix/imports-order/Foo1BeforeFix.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e/08-autofix/imports-order/Foo1.sol b/e2e/08-autofix/imports-order/Foo1.sol index 24e01492..948dfc1d 100644 --- a/e2e/08-autofix/imports-order/Foo1.sol +++ b/e2e/08-autofix/imports-order/Foo1.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; import {Foo1} from './Foo1.sol'; -import '../token/interfaces/IXTokenWrapper2.sol'; +import '../token/interfaces/IXTokenWrapper3.sol'; import {IXTokenFactory2} from '../../atoken/interfaces/IXTokenFactory2.sol'; import {Initializable} from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; import '../token/interfaces/IXTokenWrapper.sol'; diff --git a/e2e/08-autofix/imports-order/Foo1BeforeFix.sol b/e2e/08-autofix/imports-order/Foo1BeforeFix.sol index 24e01492..948dfc1d 100644 --- a/e2e/08-autofix/imports-order/Foo1BeforeFix.sol +++ b/e2e/08-autofix/imports-order/Foo1BeforeFix.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; import {Foo1} from './Foo1.sol'; -import '../token/interfaces/IXTokenWrapper2.sol'; +import '../token/interfaces/IXTokenWrapper3.sol'; import {IXTokenFactory2} from '../../atoken/interfaces/IXTokenFactory2.sol'; import {Initializable} from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; import '../token/interfaces/IXTokenWrapper.sol'; From 83393976cea7da3c3ce9362f59e88c3abe88f146 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 1 Jul 2024 16:29:12 -0300 Subject: [PATCH 14/28] autofix e2e tests7 --- lib/rules/naming/imports-order.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rules/naming/imports-order.js b/lib/rules/naming/imports-order.js index 5dd9cdfe..af72e7cc 100644 --- a/lib/rules/naming/imports-order.js +++ b/lib/rules/naming/imports-order.js @@ -50,8 +50,8 @@ class ImportsOrderChecker extends BaseChecker { range: child.range, path: child.path, fullSentence: child.symbolAliases - ? this.getFullSentence(child.symbolAliases) + '"' + child.path + '";' - : `import "${child.path}";`, + ? this.getFullSentence(child.symbolAliases) + "'" + child.path + "';" + : `import '${child.path}';`, })) // copy the object into another one From 25a6fcb6c321c4ef10cd7a099db962720b4988ca Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 1 Jul 2024 16:33:56 -0300 Subject: [PATCH 15/28] autofix e2e tests8 --- e2e/08-autofix/imports-order/Foo2.sol | 2 +- e2e/08-autofix/imports-order/Foo2AfterFix.sol | 2 +- e2e/08-autofix/imports-order/Foo2BeforeFix.sol | 2 +- e2e/autofix-test.js | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/e2e/08-autofix/imports-order/Foo2.sol b/e2e/08-autofix/imports-order/Foo2.sol index 5912b628..32bb9ee2 100644 --- a/e2e/08-autofix/imports-order/Foo2.sol +++ b/e2e/08-autofix/imports-order/Foo2.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.0; -import {Foo1} from './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOne.sol'; +import './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOnelooooooooooong.sol'; import '../../../../../token/interfaces/IXTokenWrapper3.sol'; import {IXTokenFactory2} from '../../atoken/interfaces/IXTokenFactory2.sol'; import {Initializable} from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; diff --git a/e2e/08-autofix/imports-order/Foo2AfterFix.sol b/e2e/08-autofix/imports-order/Foo2AfterFix.sol index 8b5e0961..7b47ba53 100644 --- a/e2e/08-autofix/imports-order/Foo2AfterFix.sol +++ b/e2e/08-autofix/imports-order/Foo2AfterFix.sol @@ -12,7 +12,7 @@ import '../token/interfaces/IXTokenWrapper.sol'; import {IXTokenWrapper2} from '../token/interfaces/IXTokenWrapper2.sol'; import {Initializable} from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; import {Afool1} from './Afool1.sol'; -import './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOne.sol'; +import './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOnelooooooooooong.sol'; contract ImportsOrder2 { constructor() {} diff --git a/e2e/08-autofix/imports-order/Foo2BeforeFix.sol b/e2e/08-autofix/imports-order/Foo2BeforeFix.sol index 5912b628..32bb9ee2 100644 --- a/e2e/08-autofix/imports-order/Foo2BeforeFix.sol +++ b/e2e/08-autofix/imports-order/Foo2BeforeFix.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.0; -import {Foo1} from './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOne.sol'; +import './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOnelooooooooooong.sol'; import '../../../../../token/interfaces/IXTokenWrapper3.sol'; import {IXTokenFactory2} from '../../atoken/interfaces/IXTokenFactory2.sol'; import {Initializable} from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index 2f80e0fc..b556f9c2 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -29,7 +29,7 @@ function compareTextFiles(file1Path, file2Path) { console.log('=================================================================================') console.log('=================================================================================') - console.log('file2Content: ', file1Content) + console.log('file1Content: ', file1Content) console.log('=================================================================================') console.log('=================================================================================') console.log('=================================================================================') From e95e2bebc0ff26c98c77ba74d4ebc80e93075b3a Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 1 Jul 2024 16:35:51 -0300 Subject: [PATCH 16/28] autofix e2e tests9 --- e2e/autofix-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index b556f9c2..5d32730c 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -613,7 +613,7 @@ describe('e2e', function () { expect(result).to.be.true }) }) - xdescribe('autofix rule: imports-order Foo2', () => { + describe('autofix rule: imports-order Foo2', () => { before(function () { params = retrieveParams('imports-order/') currentConfig = `${params.path}${params.subpath}.solhint.json` From 824270ba54a138b549742a17dae0a9128b695e87 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Tue, 16 Jul 2024 18:04:12 -0300 Subject: [PATCH 17/28] autofix e2e tests10 --- .prettierrc | 3 +- e2e/08-autofix/imports-order/Foo1.sol | 24 ++- e2e/08-autofix/imports-order/Foo1AfterFix.sol | 24 ++- .../imports-order/Foo1BeforeFix.sol | 24 ++- e2e/08-autofix/imports-order/Foo2.sol | 2 +- e2e/08-autofix/imports-order/Foo2AfterFix.sol | 2 +- .../imports-order/Foo2BeforeFix.sol | 2 +- e2e/autofix-test.js | 92 +++++----- lib/rules/naming/imports-order.js | 159 ++++++++---------- 9 files changed, 165 insertions(+), 167 deletions(-) diff --git a/.prettierrc b/.prettierrc index 75a894a2..c8e7be8d 100644 --- a/.prettierrc +++ b/.prettierrc @@ -1,5 +1,6 @@ { "semi": false, "singleQuote": true, - "printWidth": 100 + "printWidth": 100, + "bracketSpacing": true } diff --git a/e2e/08-autofix/imports-order/Foo1.sol b/e2e/08-autofix/imports-order/Foo1.sol index 948dfc1d..d79c9999 100644 --- a/e2e/08-autofix/imports-order/Foo1.sol +++ b/e2e/08-autofix/imports-order/Foo1.sol @@ -1,16 +1,22 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.0; -import {Foo1} from './Foo1.sol'; -import '../token/interfaces/IXTokenWrapper3.sol'; -import {IXTokenFactory2} from '../../atoken/interfaces/IXTokenFactory2.sol'; -import {Initializable} from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; -import '../token/interfaces/IXTokenWrapper.sol'; -import {IXTokenFactory, holaquetal} from '../../token/interfaces/IXTokenFactory.sol'; +import './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOnelooooooooooong.sol'; +import { Unauthorized, add as func, Point } from './Foo.sol'; +import 'https://github.com/owner/repo/blob/branch/path/to/Contract.sol'; +import { Initializable } from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; +import './../../../../token/interfaces/FakeContract1.sol'; +import { FakeContract3 } from './../../../token/interfaces/FakeContract3.sol'; +import './../../../../token/interfaces/AFakeContract1.sol'; +import './../token/interfaces/IXTokenWrapper.sol'; +import { IXTokenFactory, holaquetal } from './../../token/interfaces/IXTokenFactory.sol'; import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; -import {Afool1} from './Afool1.sol'; -import {IXTokenWrapper2} from '../token/interfaces/IXTokenWrapper2.sol'; -import {ReentrancyGuardUpgradeable2} from '@Apenzeppelin/contracts-upgradeable/ReentrancyGuardUpgradeable2.sol'; +import { FakeContract2 } from './../../../token/interfaces/FakeContract2.sol'; +import 'http://github.com/owner/repo/blob/branch/path/to/Contract2.sol'; +import { Afool1 } from './Afool1.sol'; +import './Ownable.sol'; +import { IXTokenWrapper2 } from './../token/interfaces/IXTokenWrapper2.sol'; +import { ReentrancyGuardUpgradeable2 } from '@apenzeppelin/ReentrancyGuardUpgradeable2.sol'; contract ImportsOrder { constructor() {} diff --git a/e2e/08-autofix/imports-order/Foo1AfterFix.sol b/e2e/08-autofix/imports-order/Foo1AfterFix.sol index a725462b..50cd30fa 100644 --- a/e2e/08-autofix/imports-order/Foo1AfterFix.sol +++ b/e2e/08-autofix/imports-order/Foo1AfterFix.sol @@ -1,16 +1,22 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.0; -import {ReentrancyGuardUpgradeable2} from '@Apenzeppelin/contracts-upgradeable/ReentrancyGuardUpgradeable2.sol'; +import './../../../../token/interfaces/AFakeContract1.sol'; +import './../../../../token/interfaces/FakeContract1.sol'; +import { FakeContract2 } from './../../../token/interfaces/FakeContract2.sol'; +import { FakeContract3 } from './../../../token/interfaces/FakeContract3.sol'; +import { IXTokenFactory, holaquetal } from './../../token/interfaces/IXTokenFactory.sol'; +import './../token/interfaces/IXTokenWrapper.sol'; +import { IXTokenWrapper2 } from './../token/interfaces/IXTokenWrapper2.sol'; +import { Afool1 } from './Afool1.sol'; +import { Unauthorized, add as func, Point } from './Foo.sol'; +import { Initializable } from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; +import './Ownable.sol'; +import './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOnelooooooooooong.sol'; +import { ReentrancyGuardUpgradeable2 } from '@apenzeppelin/ReentrancyGuardUpgradeable2.sol'; import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; -import {IXTokenFactory2} from '../../atoken/interfaces/IXTokenFactory2.sol'; -import {IXTokenFactory, holaquetal} from '../../token/interfaces/IXTokenFactory.sol'; -import '../token/interfaces/IXTokenWrapper.sol'; -import {IXTokenWrapper2} from '../token/interfaces/IXTokenWrapper2.sol'; -import '../token/interfaces/IXTokenWrapper3.sol'; -import {Initializable} from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; -import {Afool1} from './Afool1.sol'; -import {Foo1} from './Foo1.sol'; +import 'http://github.com/owner/repo/blob/branch/path/to/Contract2.sol'; +import 'https://github.com/owner/repo/blob/branch/path/to/Contract.sol'; contract ImportsOrder { constructor() {} diff --git a/e2e/08-autofix/imports-order/Foo1BeforeFix.sol b/e2e/08-autofix/imports-order/Foo1BeforeFix.sol index 948dfc1d..d79c9999 100644 --- a/e2e/08-autofix/imports-order/Foo1BeforeFix.sol +++ b/e2e/08-autofix/imports-order/Foo1BeforeFix.sol @@ -1,16 +1,22 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.0; -import {Foo1} from './Foo1.sol'; -import '../token/interfaces/IXTokenWrapper3.sol'; -import {IXTokenFactory2} from '../../atoken/interfaces/IXTokenFactory2.sol'; -import {Initializable} from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; -import '../token/interfaces/IXTokenWrapper.sol'; -import {IXTokenFactory, holaquetal} from '../../token/interfaces/IXTokenFactory.sol'; +import './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOnelooooooooooong.sol'; +import { Unauthorized, add as func, Point } from './Foo.sol'; +import 'https://github.com/owner/repo/blob/branch/path/to/Contract.sol'; +import { Initializable } from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; +import './../../../../token/interfaces/FakeContract1.sol'; +import { FakeContract3 } from './../../../token/interfaces/FakeContract3.sol'; +import './../../../../token/interfaces/AFakeContract1.sol'; +import './../token/interfaces/IXTokenWrapper.sol'; +import { IXTokenFactory, holaquetal } from './../../token/interfaces/IXTokenFactory.sol'; import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; -import {Afool1} from './Afool1.sol'; -import {IXTokenWrapper2} from '../token/interfaces/IXTokenWrapper2.sol'; -import {ReentrancyGuardUpgradeable2} from '@Apenzeppelin/contracts-upgradeable/ReentrancyGuardUpgradeable2.sol'; +import { FakeContract2 } from './../../../token/interfaces/FakeContract2.sol'; +import 'http://github.com/owner/repo/blob/branch/path/to/Contract2.sol'; +import { Afool1 } from './Afool1.sol'; +import './Ownable.sol'; +import { IXTokenWrapper2 } from './../token/interfaces/IXTokenWrapper2.sol'; +import { ReentrancyGuardUpgradeable2 } from '@apenzeppelin/ReentrancyGuardUpgradeable2.sol'; contract ImportsOrder { constructor() {} diff --git a/e2e/08-autofix/imports-order/Foo2.sol b/e2e/08-autofix/imports-order/Foo2.sol index 32bb9ee2..4959ef29 100644 --- a/e2e/08-autofix/imports-order/Foo2.sol +++ b/e2e/08-autofix/imports-order/Foo2.sol @@ -12,7 +12,7 @@ import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable. import {FakeContract2} from '../../../token/interfaces/FakeContract2.sol'; import {Afool1} from './Afool1.sol'; import {IXTokenWrapper2} from '../token/interfaces/IXTokenWrapper2.sol'; -import {ReentrancyGuardUpgradeable2} from '@Apenzeppelin/ReentrancyGuardUpgradeable2.sol'; +import {ReentrancyGuardUpgradeable2} from '@apenzeppelin/ReentrancyGuardUpgradeable2.sol'; contract ImportsOrder2 { constructor() {} diff --git a/e2e/08-autofix/imports-order/Foo2AfterFix.sol b/e2e/08-autofix/imports-order/Foo2AfterFix.sol index 7b47ba53..8e9a3120 100644 --- a/e2e/08-autofix/imports-order/Foo2AfterFix.sol +++ b/e2e/08-autofix/imports-order/Foo2AfterFix.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.0; -import {ReentrancyGuardUpgradeable2} from '@Apenzeppelin/ReentrancyGuardUpgradeable2.sol'; +import {ReentrancyGuardUpgradeable2} from '@apenzeppelin/ReentrancyGuardUpgradeable2.sol'; import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; import '../../../../../token/interfaces/IXTokenWrapper3.sol'; import '../../../../token/interfaces/FakeContract1.sol'; diff --git a/e2e/08-autofix/imports-order/Foo2BeforeFix.sol b/e2e/08-autofix/imports-order/Foo2BeforeFix.sol index 32bb9ee2..4959ef29 100644 --- a/e2e/08-autofix/imports-order/Foo2BeforeFix.sol +++ b/e2e/08-autofix/imports-order/Foo2BeforeFix.sol @@ -12,7 +12,7 @@ import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable. import {FakeContract2} from '../../../token/interfaces/FakeContract2.sol'; import {Afool1} from './Afool1.sol'; import {IXTokenWrapper2} from '../token/interfaces/IXTokenWrapper2.sol'; -import {ReentrancyGuardUpgradeable2} from '@Apenzeppelin/ReentrancyGuardUpgradeable2.sol'; +import {ReentrancyGuardUpgradeable2} from '@apenzeppelin/ReentrancyGuardUpgradeable2.sol'; contract ImportsOrder2 { constructor() {} diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index 5d32730c..20d01033 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -568,7 +568,7 @@ describe('e2e', function () { }) }) - describe('autofix rule: imports-order P1', () => { + describe('autofix rule: imports-order', () => { describe('autofix rule: imports-order Foo1', () => { before(function () { params = retrieveParams('imports-order/') @@ -613,51 +613,51 @@ describe('e2e', function () { expect(result).to.be.true }) }) - describe('autofix rule: imports-order Foo2', () => { - before(function () { - params = retrieveParams('imports-order/') - currentConfig = `${params.path}${params.subpath}.solhint.json` - currentFile = `${params.path}${params.subpath}Foo2.sol` - beforeFixFile = `${params.path}${params.subpath}Foo2BeforeFix.sol` - afterFixFile = `${params.path}${params.subpath}Foo2AfterFix.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 (11)', () => { - result = compareTextFiles(currentFile, beforeFixFile) - expect(result).to.be.true - }) - - it('should execute and compare Foo1 with template AFTER FIX and they should match (11)', () => { - ;({ 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 (11)', () => { - expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) - }) - - it('should get the right report (11)', () => { - const reportLines = stdout.split('\n') - const finalLine = '12 problems (12 errors, 0 warnings)' - expect(reportLines[reportLines.length - 3]).to.contain(finalLine) - }) - }) - - it('should check FOO1 does not change after test (11)', () => { - result = compareTextFiles(currentFile, beforeFixFile) - expect(result).to.be.true - }) - }) + // describe('autofix rule: imports-order Foo2', () => { + // before(function () { + // params = retrieveParams('imports-order/') + // currentConfig = `${params.path}${params.subpath}.solhint.json` + // currentFile = `${params.path}${params.subpath}Foo2.sol` + // beforeFixFile = `${params.path}${params.subpath}Foo2BeforeFix.sol` + // afterFixFile = `${params.path}${params.subpath}Foo2AfterFix.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 (11)', () => { + // result = compareTextFiles(currentFile, beforeFixFile) + // expect(result).to.be.true + // }) + + // it('should execute and compare Foo1 with template AFTER FIX and they should match (11)', () => { + // ;({ 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 (11)', () => { + // expect(code).to.equal(EXIT_CODES.REPORTED_ERRORS) + // }) + + // it('should get the right report (11)', () => { + // const reportLines = stdout.split('\n') + // const finalLine = '12 problems (12 errors, 0 warnings)' + // expect(reportLines[reportLines.length - 3]).to.contain(finalLine) + // }) + // }) + + // it('should check FOO1 does not change after test (11)', () => { + // result = compareTextFiles(currentFile, beforeFixFile) + // expect(result).to.be.true + // }) + // }) }) }) diff --git a/lib/rules/naming/imports-order.js b/lib/rules/naming/imports-order.js index af72e7cc..b71107a8 100644 --- a/lib/rules/naming/imports-order.js +++ b/lib/rules/naming/imports-order.js @@ -1,7 +1,7 @@ const BaseChecker = require('../base-checker') const { severityDescription } = require('../../doc/utils') -const FOLDER_LEVELS_SUPPORT = 10 +// const FOLDER_LEVELS_SUPPORT = 8 const DEFAULT_SEVERITY = 'warn' const ruleId = 'imports-order' @@ -25,7 +25,10 @@ const meta = { note: 'Order alphabetically for each file at the same level, e.g. ./bar comes before ./foo', }, { - note: 'Rule support up to 10 folder levels "../../../../../../../../../../"', + note: 'Rule supports up to 8 folder levels "../../../../../../../../"', + }, + { + note: 'Rule does NOT support this kind of import "import * as Alias from "./filename.sol"', }, ], }, @@ -40,31 +43,42 @@ const meta = { class ImportsOrderChecker extends BaseChecker { constructor(reporter) { super(reporter, ruleId, meta) + this.orderedImports = [] // This will hold the sorted imports } SourceUnit(node) { + // console.log('node.children :>> ', node.children) // get all the imports into one object this.fromContractImports = node.children .filter((child) => child.type === 'ImportDirective') - .map((child) => ({ - range: child.range, - path: child.path, - fullSentence: child.symbolAliases - ? this.getFullSentence(child.symbolAliases) + "'" + child.path + "';" - : `import '${child.path}';`, - })) + .map((child) => { + const normalizedPath = this.normalizePath(child.path) + const result = { + range: child.range, + path: normalizedPath, + fullSentence: child.symbolAliases + ? `${this.getFullSentence(child.symbolAliases)}'${normalizedPath}';` + : `import '${normalizedPath}';`, + } + return result + }) - // copy the object into another one - this.orderedImports = [...this.fromContractImports] + // Create a deep copy of fromContractImports for sorting + this.orderedImports = JSON.parse(JSON.stringify(this.fromContractImports)) // order the object to get the ordered and the contract order - this.orderImports(this.orderedImports) + this.orderedImports = this.sortImports(this.orderedImports) + + // console.log('NO Order: \n') + // this.fromContractImports.forEach((importItem) => console.log(importItem.fullSentence)) + // console.log('\n\nOrdered: \n') + // this.orderedImports.forEach((importItem) => console.log(importItem.fullSentence)) } 'SourceUnit:exit'(node) { // when finish analyzing check if ordered import array is equal to the import contract order const areEqual = areArraysEqual(this.fromContractImports, this.orderedImports) - + console.log('\n\n\n\nareEqual :>> ', areEqual) // if are equal do nothing, if not enter the if if (!areEqual) { // Find the lowest starting range to start the replacement @@ -113,101 +127,66 @@ class ImportsOrderChecker extends BaseChecker { return (fixer) => fixer.replaceTextRange(replacement.range, replacement.newText + '\n') } - orderImports(imports) { - // it supports up to ten folder of hierarchy - // ===>> ../../../../../../../../../../ - // generate that hierarchy and put @ first - const generateDirHierarchy = (levels) => { - const hierarchy = ['@'] - for (let i = levels; i > 0; i--) { - hierarchy.push('../'.repeat(i)) + sortImports(unorderedImports) { + // Helper function to determine the hierarchical level of a path + function getHierarchyLevel(path) { + const protocolOrder = { + '@': 1, + 'http://': 2, + 'https://': 3, } - hierarchy.push('./') - return hierarchy - } - - const dirHierarchy = generateDirHierarchy(FOLDER_LEVELS_SUPPORT) - // get dir level to compare and order - const getDirLevel = (path) => { - for (let i = 0; i < dirHierarchy.length; i++) { - if (path.startsWith(dirHierarchy[i])) { - return i + // Check for protocol-specific paths and assign them their respective order levels + for (const protocol in protocolOrder) { + if (path.startsWith(protocol)) { + return protocolOrder[protocol] } } - return dirHierarchy.length // Default level if no match - } - - // count the folders to put first if there's an only file at the same level - const getSubDirCount = (path) => { - // Count number of slashes in the path - return (path.match(/\//g) || []).length - } - - // compare paths alphabetically - const comparePathsAlphabetically = (pathA, pathB) => { - const partsA = pathA.split('/') - const partsB = pathB.split('/') - for (let i = 0; i < Math.min(partsA.length, partsB.length); i++) { - const comparison = partsA[i].localeCompare(partsB[i]) - if (comparison !== 0) { - return comparison - } + // Relative path handling + if (path.startsWith('./')) { + // Count the number of '../' sequences to determine depth + const depth = path.split('/').filter((part) => part === '..').length + // Return a negative value to ensure that lesser depth has higher precedence (closer to 0) + return -depth } - return 0 // Equal parts up to the length of the shorter path - } - - // check if there's a single file and put it at last - const isSingleFile = (path) => { - const parts = path.split('/') - return parts.length === 2 && parts[1].includes('.') + // Default catch-all for unhandled cases + return Infinity } - return imports.sort((a, b) => { - const dirLevelA = getDirLevel(a.path) - const dirLevelB = getDirLevel(b.path) - - if (dirLevelA !== dirLevelB) { - return dirLevelA - dirLevelB - } - - const singleFileA = isSingleFile(a.path) - const singleFileB = isSingleFile(b.path) + // Sort imports based on the hierarchy level and then alphabetically + const orderedImports = unorderedImports.sort((a, b) => { + const levelA = getHierarchyLevel(a.path) + const levelB = getHierarchyLevel(b.path) - if (singleFileA !== singleFileB) { - return singleFileA - singleFileB // Place single file imports last at each level + if (levelA !== levelB) { + return levelA - levelB } - const alphabeticComparison = comparePathsAlphabetically(a.path, b.path) - if (alphabeticComparison !== 0) { - return alphabeticComparison - } + // For same level, sort alphabetically by the entire path, case-insensitive + return a.path.localeCompare(b.path, undefined, { sensitivity: 'base' }) + }) - const subDirCountA = getSubDirCount(a.path) - const subDirCountB = getSubDirCount(b.path) + return orderedImports + } - return subDirCountB - subDirCountA // More subdirectories come first + // Map through each subarray to convert it to the desired format + getFullSentence(elements) { + const importParts = elements.map(([name, alias]) => { + // Check if there is an alias; if so, format it as 'name as alias', otherwise just return the name + return alias ? `${name} as ${alias}` : name }) + + // Join all the parts with a comma and space, and format it into the final import string + return `import { ${importParts.join(', ')} } from ` } - // function to get the full sentence, because the AST only get the path and alias separated - getFullSentence(node) { - const sentenceStart = 'import {' - const sentenceEnd = '} from ' - let imported = '' - - if (node.length === 1) { - imported = node[0][0] - } else { - for (let i = 0; i < node.length; i++) { - if (i === node.length - 1) imported += node[i][0] - else imported += node[i][0] + ', ' - } + normalizePath(path) { + if (path.startsWith('../')) { + return `./${path}` } - - return sentenceStart + imported + sentenceEnd + return path } } From e25b721b5cef6309a8ba54387ba07f359f2093a2 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Tue, 16 Jul 2024 18:04:32 -0300 Subject: [PATCH 18/28] autofix e2e tests10 --- docs/rules/naming/imports-order.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/rules/naming/imports-order.md b/docs/rules/naming/imports-order.md index 05f04128..fbd1669e 100644 --- a/docs/rules/naming/imports-order.md +++ b/docs/rules/naming/imports-order.md @@ -26,7 +26,8 @@ This rule accepts a string option of rule severity. Must be one of "error", "war ### Notes - Order by hierarchy of directories first, e.g. ../../ comes before ../, which comes before ./, which comes before ./foo - Order alphabetically for each file at the same level, e.g. ./bar comes before ./foo -- Rule support up to 10 folder levels "../../../../../../../../../../" +- Rule supports up to 8 folder levels "../../../../../../../../" +- Rule does NOT support this kind of import "import * as Alias from "./filename.sol" ## Examples This rule does not have examples. From 649dee8deea2e2ccf0af7a1d01edf022eb6073eb Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Wed, 17 Jul 2024 09:14:50 -0300 Subject: [PATCH 19/28] autofix e2e test11 --- e2e/08-autofix/imports-order/Foo1.sol | 12 +++++++----- e2e/08-autofix/imports-order/Foo1AfterFix.sol | 14 ++++++++------ e2e/08-autofix/imports-order/Foo1BeforeFix.sol | 12 +++++++----- lib/rules/naming/imports-order.js | 2 +- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/e2e/08-autofix/imports-order/Foo1.sol b/e2e/08-autofix/imports-order/Foo1.sol index d79c9999..d833c02e 100644 --- a/e2e/08-autofix/imports-order/Foo1.sol +++ b/e2e/08-autofix/imports-order/Foo1.sol @@ -5,17 +5,19 @@ import './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOneloo import { Unauthorized, add as func, Point } from './Foo.sol'; import 'https://github.com/owner/repo/blob/branch/path/to/Contract.sol'; import { Initializable } from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; -import './../../../../token/interfaces/FakeContract1.sol'; -import { FakeContract3 } from './../../../token/interfaces/FakeContract3.sol'; +import '../../../../token/interfaces/FakeContract1.sol'; +import { FakeContract3 } from '../../../token/interfaces/FakeContract3.sol'; import './../../../../token/interfaces/AFakeContract1.sol'; import './../token/interfaces/IXTokenWrapper.sol'; -import { IXTokenFactory, holaquetal } from './../../token/interfaces/IXTokenFactory.sol'; +import { IXTokenFactory, holaquetal } from '../../token/interfaces/IXTokenFactory.sol'; +import './../../bpath/otherfolder/otherfolder/aContract.sol'; import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; -import { FakeContract2 } from './../../../token/interfaces/FakeContract2.sol'; +import { FakeContract2 } from '../../../token/interfaces/FakeContract2.sol'; +import './../../apath/zContract.sol'; import 'http://github.com/owner/repo/blob/branch/path/to/Contract2.sol'; import { Afool1 } from './Afool1.sol'; import './Ownable.sol'; -import { IXTokenWrapper2 } from './../token/interfaces/IXTokenWrapper2.sol'; +import { IXTokenWrapper2 } from '../token/interfaces/IXTokenWrapper2.sol'; import { ReentrancyGuardUpgradeable2 } from '@apenzeppelin/ReentrancyGuardUpgradeable2.sol'; contract ImportsOrder { diff --git a/e2e/08-autofix/imports-order/Foo1AfterFix.sol b/e2e/08-autofix/imports-order/Foo1AfterFix.sol index 50cd30fa..281f064b 100644 --- a/e2e/08-autofix/imports-order/Foo1AfterFix.sol +++ b/e2e/08-autofix/imports-order/Foo1AfterFix.sol @@ -2,14 +2,16 @@ pragma solidity ^0.8.0; import './../../../../token/interfaces/AFakeContract1.sol'; -import './../../../../token/interfaces/FakeContract1.sol'; -import { FakeContract2 } from './../../../token/interfaces/FakeContract2.sol'; -import { FakeContract3 } from './../../../token/interfaces/FakeContract3.sol'; -import { IXTokenFactory, holaquetal } from './../../token/interfaces/IXTokenFactory.sol'; +import '../../../../token/interfaces/FakeContract1.sol'; +import { FakeContract2 } from '../../../token/interfaces/FakeContract2.sol'; +import { FakeContract3 } from '../../../token/interfaces/FakeContract3.sol'; +import './../../apath/zContract.sol'; +import './../../bpath/otherfolder/otherfolder/aContract.sol'; +import { holaquetal, IXTokenFactory } from '../../token/interfaces/IXTokenFactory.sol'; import './../token/interfaces/IXTokenWrapper.sol'; -import { IXTokenWrapper2 } from './../token/interfaces/IXTokenWrapper2.sol'; +import { IXTokenWrapper2 } from '../token/interfaces/IXTokenWrapper2.sol'; import { Afool1 } from './Afool1.sol'; -import { Unauthorized, add as func, Point } from './Foo.sol'; +import { add as func, Point, Unauthorized } from './Foo.sol'; import { Initializable } from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; import './Ownable.sol'; import './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOnelooooooooooong.sol'; diff --git a/e2e/08-autofix/imports-order/Foo1BeforeFix.sol b/e2e/08-autofix/imports-order/Foo1BeforeFix.sol index d79c9999..d833c02e 100644 --- a/e2e/08-autofix/imports-order/Foo1BeforeFix.sol +++ b/e2e/08-autofix/imports-order/Foo1BeforeFix.sol @@ -5,17 +5,19 @@ import './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOneloo import { Unauthorized, add as func, Point } from './Foo.sol'; import 'https://github.com/owner/repo/blob/branch/path/to/Contract.sol'; import { Initializable } from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; -import './../../../../token/interfaces/FakeContract1.sol'; -import { FakeContract3 } from './../../../token/interfaces/FakeContract3.sol'; +import '../../../../token/interfaces/FakeContract1.sol'; +import { FakeContract3 } from '../../../token/interfaces/FakeContract3.sol'; import './../../../../token/interfaces/AFakeContract1.sol'; import './../token/interfaces/IXTokenWrapper.sol'; -import { IXTokenFactory, holaquetal } from './../../token/interfaces/IXTokenFactory.sol'; +import { IXTokenFactory, holaquetal } from '../../token/interfaces/IXTokenFactory.sol'; +import './../../bpath/otherfolder/otherfolder/aContract.sol'; import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; -import { FakeContract2 } from './../../../token/interfaces/FakeContract2.sol'; +import { FakeContract2 } from '../../../token/interfaces/FakeContract2.sol'; +import './../../apath/zContract.sol'; import 'http://github.com/owner/repo/blob/branch/path/to/Contract2.sol'; import { Afool1 } from './Afool1.sol'; import './Ownable.sol'; -import { IXTokenWrapper2 } from './../token/interfaces/IXTokenWrapper2.sol'; +import { IXTokenWrapper2 } from '../token/interfaces/IXTokenWrapper2.sol'; import { ReentrancyGuardUpgradeable2 } from '@apenzeppelin/ReentrancyGuardUpgradeable2.sol'; contract ImportsOrder { diff --git a/lib/rules/naming/imports-order.js b/lib/rules/naming/imports-order.js index b71107a8..fd7840bc 100644 --- a/lib/rules/naming/imports-order.js +++ b/lib/rules/naming/imports-order.js @@ -78,7 +78,7 @@ class ImportsOrderChecker extends BaseChecker { 'SourceUnit:exit'(node) { // when finish analyzing check if ordered import array is equal to the import contract order const areEqual = areArraysEqual(this.fromContractImports, this.orderedImports) - console.log('\n\n\n\nareEqual :>> ', areEqual) + // if are equal do nothing, if not enter the if if (!areEqual) { // Find the lowest starting range to start the replacement From a0c9cad8e5add8bebfceef0e40330daf6e34a0e8 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Wed, 17 Jul 2024 09:20:34 -0300 Subject: [PATCH 20/28] autofix e2e test11 --- lib/rules/naming/imports-order.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/rules/naming/imports-order.js b/lib/rules/naming/imports-order.js index fd7840bc..5cc8ec85 100644 --- a/lib/rules/naming/imports-order.js +++ b/lib/rules/naming/imports-order.js @@ -1,7 +1,6 @@ const BaseChecker = require('../base-checker') const { severityDescription } = require('../../doc/utils') -// const FOLDER_LEVELS_SUPPORT = 8 const DEFAULT_SEVERITY = 'warn' const ruleId = 'imports-order' @@ -9,7 +8,7 @@ const meta = { type: 'naming', docs: { - description: `Order the imports of the contract to follow a certain hierarchy`, + description: `Order the imports of the contract to follow a certain hierarchy (read "Notes section")`, category: 'Style Guide Rules', options: [ { @@ -19,17 +18,20 @@ const meta = { ], notes: [ { - note: 'Order by hierarchy of directories first, e.g. ../../ comes before ../, which comes before ./, which comes before ./foo', + note: 'Order by hierarchy of directories first, e.g. ./../../ comes before ./../, which comes before ./, which comes before ./foo', }, { - note: 'Order alphabetically for each file at the same level, e.g. ./bar comes before ./foo', + note: 'Paths starting with "@" like "@openzeppelin/" and urls ("http" and "https") will go at last', }, { - note: 'Rule supports up to 8 folder levels "../../../../../../../../"', + note: 'Order alphabetically for each path at the same level, e.g. ./contract/Zbar.sol comes before ./interface/Ifoo.sol', }, { note: 'Rule does NOT support this kind of import "import * as Alias from "./filename.sol"', }, + { + note: 'When "--fix", rule will re-write this notation "../folder/file.sol" or this one "../file.sol" to "./../folder/file.sol" or this one "./../file.sol"', + }, ], }, From 684aa065f99b6d04170800282490c63ec1994b70 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Wed, 17 Jul 2024 09:20:58 -0300 Subject: [PATCH 21/28] autofix e2e test11 --- docs/rules.md | 2 +- docs/rules/naming/imports-order.md | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/rules.md b/docs/rules.md index 1541ec02..8c644b5f 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -37,7 +37,7 @@ title: "Rule Index of Solhint" | [func-named-parameters](./rules/naming/func-named-parameters.md) | Enforce named parameters for function calls with 4 or more arguments. This rule may have some false positives | | | | [func-param-name-mixedcase](./rules/naming/func-param-name-mixedcase.md) | Function param name must be in mixedCase. | | | | [immutable-vars-naming](./rules/naming/immutable-vars-naming.md) | Check Immutable variables. Capitalized SNAKE_CASE or mixedCase depending on configuration. | $~~~~~~~~$✔️ | | -| [imports-order](./rules/naming/imports-order.md) | Order the imports of the contract to follow a certain hierarchy | | | +| [imports-order](./rules/naming/imports-order.md) | Order the imports of the contract to follow a certain hierarchy (read "Notes section") | | | | [modifier-name-mixedcase](./rules/naming/modifier-name-mixedcase.md) | Modifier name must be in mixedCase. | | | | [named-parameters-mapping](./rules/naming/named-parameters-mapping.md) | Solidity v0.8.18 introduced named parameters on the mappings definition. | | | | [private-vars-leading-underscore](./rules/naming/private-vars-leading-underscore.md) | Non-external functions and state variables should start with a single underscore. Others, shouldn't | | | diff --git a/docs/rules/naming/imports-order.md b/docs/rules/naming/imports-order.md index fbd1669e..9a224be5 100644 --- a/docs/rules/naming/imports-order.md +++ b/docs/rules/naming/imports-order.md @@ -9,7 +9,7 @@ title: "imports-order | Solhint" ![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow) ## Description -Order the imports of the contract to follow a certain hierarchy +Order the imports of the contract to follow a certain hierarchy (read "Notes section") ## Options This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn. @@ -24,10 +24,11 @@ This rule accepts a string option of rule severity. Must be one of "error", "war ``` ### Notes -- Order by hierarchy of directories first, e.g. ../../ comes before ../, which comes before ./, which comes before ./foo -- Order alphabetically for each file at the same level, e.g. ./bar comes before ./foo -- Rule supports up to 8 folder levels "../../../../../../../../" +- Order by hierarchy of directories first, e.g. ./../../ comes before ./../, which comes before ./, which comes before ./foo +- Paths starting with "@" like "@openzeppelin/" and urls ("http" and "https") will go at last +- Order alphabetically for each path at the same level, e.g. ./contract/Zbar.sol comes before ./interface/Ifoo.sol - Rule does NOT support this kind of import "import * as Alias from "./filename.sol" +- When "--fix", rule will re-write this notation "../folder/file.sol" or this one "../file.sol" to "./../folder/file.sol" or this one "./../file.sol" ## Examples This rule does not have examples. From a8ac616bf2aa7928fdef53a007f9f8837aea0ca0 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Wed, 17 Jul 2024 09:26:16 -0300 Subject: [PATCH 22/28] autofix e2e test12 --- e2e/08-autofix/imports-order/Foo1.sol | 2 +- e2e/08-autofix/imports-order/Foo1AfterFix.sol | 10 +++++----- e2e/08-autofix/imports-order/Foo1BeforeFix.sol | 2 +- e2e/autofix-test.js | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/e2e/08-autofix/imports-order/Foo1.sol b/e2e/08-autofix/imports-order/Foo1.sol index d833c02e..a15c2fb7 100644 --- a/e2e/08-autofix/imports-order/Foo1.sol +++ b/e2e/08-autofix/imports-order/Foo1.sol @@ -13,7 +13,7 @@ import { IXTokenFactory, holaquetal } from '../../token/interfaces/IXTokenFactor import './../../bpath/otherfolder/otherfolder/aContract.sol'; import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; import { FakeContract2 } from '../../../token/interfaces/FakeContract2.sol'; -import './../../apath/zContract.sol'; +import '../../apath/zContract.sol'; import 'http://github.com/owner/repo/blob/branch/path/to/Contract2.sol'; import { Afool1 } from './Afool1.sol'; import './Ownable.sol'; diff --git a/e2e/08-autofix/imports-order/Foo1AfterFix.sol b/e2e/08-autofix/imports-order/Foo1AfterFix.sol index 281f064b..2bd5c468 100644 --- a/e2e/08-autofix/imports-order/Foo1AfterFix.sol +++ b/e2e/08-autofix/imports-order/Foo1AfterFix.sol @@ -2,14 +2,14 @@ pragma solidity ^0.8.0; import './../../../../token/interfaces/AFakeContract1.sol'; -import '../../../../token/interfaces/FakeContract1.sol'; -import { FakeContract2 } from '../../../token/interfaces/FakeContract2.sol'; -import { FakeContract3 } from '../../../token/interfaces/FakeContract3.sol'; +import './../../../../token/interfaces/FakeContract1.sol'; +import { FakeContract2 } from './../../../token/interfaces/FakeContract2.sol'; +import { FakeContract3 } from './../../../token/interfaces/FakeContract3.sol'; import './../../apath/zContract.sol'; import './../../bpath/otherfolder/otherfolder/aContract.sol'; -import { holaquetal, IXTokenFactory } from '../../token/interfaces/IXTokenFactory.sol'; +import { holaquetal, IXTokenFactory } from './../../token/interfaces/IXTokenFactory.sol'; import './../token/interfaces/IXTokenWrapper.sol'; -import { IXTokenWrapper2 } from '../token/interfaces/IXTokenWrapper2.sol'; +import { IXTokenWrapper2 } from './../token/interfaces/IXTokenWrapper2.sol'; import { Afool1 } from './Afool1.sol'; import { add as func, Point, Unauthorized } from './Foo.sol'; import { Initializable } from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; diff --git a/e2e/08-autofix/imports-order/Foo1BeforeFix.sol b/e2e/08-autofix/imports-order/Foo1BeforeFix.sol index d833c02e..a15c2fb7 100644 --- a/e2e/08-autofix/imports-order/Foo1BeforeFix.sol +++ b/e2e/08-autofix/imports-order/Foo1BeforeFix.sol @@ -13,7 +13,7 @@ import { IXTokenFactory, holaquetal } from '../../token/interfaces/IXTokenFactor import './../../bpath/otherfolder/otherfolder/aContract.sol'; import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; import { FakeContract2 } from '../../../token/interfaces/FakeContract2.sol'; -import './../../apath/zContract.sol'; +import '../../apath/zContract.sol'; import 'http://github.com/owner/repo/blob/branch/path/to/Contract2.sol'; import { Afool1 } from './Afool1.sol'; import './Ownable.sol'; diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index 20d01033..df9f0b0a 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -603,7 +603,7 @@ describe('e2e', function () { it('should get the right report (10)', () => { const reportLines = stdout.split('\n') - const finalLine = '10 problems (10 errors, 0 warnings)' + const finalLine = '18 problems (18 errors, 0 warnings)' expect(reportLines[reportLines.length - 3]).to.contain(finalLine) }) }) From 22aca44b4ef42cc0de9e2f7b5e70e3c0edf9b3d0 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Wed, 17 Jul 2024 09:30:56 -0300 Subject: [PATCH 23/28] autofix e2e test13 --- e2e/08-autofix/imports-order/Foo1AfterFix.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e/08-autofix/imports-order/Foo1AfterFix.sol b/e2e/08-autofix/imports-order/Foo1AfterFix.sol index 2bd5c468..0e7f38c1 100644 --- a/e2e/08-autofix/imports-order/Foo1AfterFix.sol +++ b/e2e/08-autofix/imports-order/Foo1AfterFix.sol @@ -7,11 +7,11 @@ import { FakeContract2 } from './../../../token/interfaces/FakeContract2.sol'; import { FakeContract3 } from './../../../token/interfaces/FakeContract3.sol'; import './../../apath/zContract.sol'; import './../../bpath/otherfolder/otherfolder/aContract.sol'; -import { holaquetal, IXTokenFactory } from './../../token/interfaces/IXTokenFactory.sol'; +import { IXTokenFactory, holaquetal } from './../../token/interfaces/IXTokenFactory.sol'; import './../token/interfaces/IXTokenWrapper.sol'; import { IXTokenWrapper2 } from './../token/interfaces/IXTokenWrapper2.sol'; import { Afool1 } from './Afool1.sol'; -import { add as func, Point, Unauthorized } from './Foo.sol'; +import { Unauthorized, add as func, Point } from './Foo.sol'; import { Initializable } from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; import './Ownable.sol'; import './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOnelooooooooooong.sol'; From f7d7126deafbb2ca7a31733335b6c44a7923ac8b Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Wed, 17 Jul 2024 09:35:21 -0300 Subject: [PATCH 24/28] autofix e2e test14 --- docs/rules/naming/import-order.md | 40 ------------------------------- e2e/autofix-test.js | 20 ++++++++-------- 2 files changed, 10 insertions(+), 50 deletions(-) delete mode 100644 docs/rules/naming/import-order.md diff --git a/docs/rules/naming/import-order.md b/docs/rules/naming/import-order.md deleted file mode 100644 index 3bf54c00..00000000 --- a/docs/rules/naming/import-order.md +++ /dev/null @@ -1,40 +0,0 @@ ---- -warning: "This is a dynamically generated file. Do not edit manually." -layout: "default" -title: "import-order | Solhint" ---- - -# import-order -![Category Badge](https://img.shields.io/badge/-Style%20Guide%20Rules-informational) -![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow) - -## Description -Order the imports of the contract to follow a certain hierarchy - -## Options -This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn. - -### Example Config -```json -{ - "rules": { - "import-order": "warn" - } -} -``` - -### Notes -- Order by hierarchy of directories first, e.g. ../../ comes before ../, which comes before ./, which comes before ./foo -- Order alphabetically for each file at the same level, e.g. ./bar comes before ./foo -- Rule support up to 10 folder levels "../../../../../../../../../../" - -## Examples -This rule does not have examples. - -## Version -This rule is introduced in the latest version. - -## Resources -- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/naming/import-order.js) -- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/naming/import-order.md) -- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/naming/import-order.js) diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index df9f0b0a..0b7812ca 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -27,16 +27,16 @@ function compareTextFiles(file1Path, file2Path) { const file1Content = fs.readFileSync(file1Path, 'utf-8') const file2Content = fs.readFileSync(file2Path, 'utf-8') - console.log('=================================================================================') - console.log('=================================================================================') - console.log('file1Content: ', file1Content) - console.log('=================================================================================') - console.log('=================================================================================') - console.log('=================================================================================') - console.log('=================================================================================') - console.log('file2Content: ', file2Content) - console.log('=================================================================================') - console.log('=================================================================================') + // console.log('=================================================================================') + // console.log('=================================================================================') + // console.log('file1Content: ', file1Content) + // console.log('=================================================================================') + // console.log('=================================================================================') + // console.log('=================================================================================') + // console.log('=================================================================================') + // console.log('file2Content: ', file2Content) + // console.log('=================================================================================') + // console.log('=================================================================================') return file1Content === file2Content } From 0b52aaabfe3a6670e142a403c7de57f1357bbe7b Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 22 Jul 2024 17:29:33 -0300 Subject: [PATCH 25/28] fix: import-order rule --- e2e/08-autofix/imports-order/Foo1AfterFix.sol | 6 +++--- lib/rules/naming/imports-order.js | 12 +++++++----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/e2e/08-autofix/imports-order/Foo1AfterFix.sol b/e2e/08-autofix/imports-order/Foo1AfterFix.sol index 0e7f38c1..e7ae91c9 100644 --- a/e2e/08-autofix/imports-order/Foo1AfterFix.sol +++ b/e2e/08-autofix/imports-order/Foo1AfterFix.sol @@ -1,6 +1,9 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.0; +import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; +import 'http://github.com/owner/repo/blob/branch/path/to/Contract2.sol'; +import 'https://github.com/owner/repo/blob/branch/path/to/Contract.sol'; import './../../../../token/interfaces/AFakeContract1.sol'; import './../../../../token/interfaces/FakeContract1.sol'; import { FakeContract2 } from './../../../token/interfaces/FakeContract2.sol'; @@ -16,9 +19,6 @@ import { Initializable } from './openzeppelin/contracts-upgradeable/proxy/utils/ import './Ownable.sol'; import './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOnelooooooooooong.sol'; import { ReentrancyGuardUpgradeable2 } from '@apenzeppelin/ReentrancyGuardUpgradeable2.sol'; -import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; -import 'http://github.com/owner/repo/blob/branch/path/to/Contract2.sol'; -import 'https://github.com/owner/repo/blob/branch/path/to/Contract.sol'; contract ImportsOrder { constructor() {} diff --git a/lib/rules/naming/imports-order.js b/lib/rules/naming/imports-order.js index 5cc8ec85..6d3a255e 100644 --- a/lib/rules/naming/imports-order.js +++ b/lib/rules/naming/imports-order.js @@ -18,10 +18,10 @@ const meta = { ], notes: [ { - note: 'Order by hierarchy of directories first, e.g. ./../../ comes before ./../, which comes before ./, which comes before ./foo', + note: 'Paths starting with "@" like "@openzeppelin/" and urls ("http" and "https") will go first', }, { - note: 'Paths starting with "@" like "@openzeppelin/" and urls ("http" and "https") will go at last', + note: 'Order by hierarchy of directories first, e.g. ./../../ comes before ./../, which comes before ./, which comes before ./foo', }, { note: 'Order alphabetically for each path at the same level, e.g. ./contract/Zbar.sol comes before ./interface/Ifoo.sol', @@ -71,6 +71,7 @@ class ImportsOrderChecker extends BaseChecker { // order the object to get the ordered and the contract order this.orderedImports = this.sortImports(this.orderedImports) + // console.log('this.orderedImports :>> ', this.fromContractImports) // console.log('NO Order: \n') // this.fromContractImports.forEach((importItem) => console.log(importItem.fullSentence)) // console.log('\n\nOrdered: \n') @@ -132,10 +133,11 @@ class ImportsOrderChecker extends BaseChecker { sortImports(unorderedImports) { // Helper function to determine the hierarchical level of a path function getHierarchyLevel(path) { + // put very large numbers so these comes first in precedence const protocolOrder = { - '@': 1, - 'http://': 2, - 'https://': 3, + '@': -30000, + 'http://': -20000, + 'https://': -10000, } // Check for protocol-specific paths and assign them their respective order levels From b91d312f49b22ed491e9e5ea21352c27c303ba60 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 22 Jul 2024 17:29:50 -0300 Subject: [PATCH 26/28] fix: import-order rule --- docs/rules/naming/imports-order.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/naming/imports-order.md b/docs/rules/naming/imports-order.md index 9a224be5..b5322754 100644 --- a/docs/rules/naming/imports-order.md +++ b/docs/rules/naming/imports-order.md @@ -24,8 +24,8 @@ This rule accepts a string option of rule severity. Must be one of "error", "war ``` ### Notes +- Paths starting with "@" like "@openzeppelin/" and urls ("http" and "https") will go first - Order by hierarchy of directories first, e.g. ./../../ comes before ./../, which comes before ./, which comes before ./foo -- Paths starting with "@" like "@openzeppelin/" and urls ("http" and "https") will go at last - Order alphabetically for each path at the same level, e.g. ./contract/Zbar.sol comes before ./interface/Ifoo.sol - Rule does NOT support this kind of import "import * as Alias from "./filename.sol" - When "--fix", rule will re-write this notation "../folder/file.sol" or this one "../file.sol" to "./../folder/file.sol" or this one "./../file.sol" From bf7893f99721d83d074993562c37420786476cc9 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Mon, 22 Jul 2024 17:34:21 -0300 Subject: [PATCH 27/28] fix: import-order rule --- e2e/08-autofix/imports-order/Foo1AfterFix.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/08-autofix/imports-order/Foo1AfterFix.sol b/e2e/08-autofix/imports-order/Foo1AfterFix.sol index e7ae91c9..e835d7dd 100644 --- a/e2e/08-autofix/imports-order/Foo1AfterFix.sol +++ b/e2e/08-autofix/imports-order/Foo1AfterFix.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.0; +import { ReentrancyGuardUpgradeable2 } from '@apenzeppelin/ReentrancyGuardUpgradeable2.sol'; import '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol'; import 'http://github.com/owner/repo/blob/branch/path/to/Contract2.sol'; import 'https://github.com/owner/repo/blob/branch/path/to/Contract.sol'; @@ -18,7 +19,6 @@ import { Unauthorized, add as func, Point } from './Foo.sol'; import { Initializable } from './openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol'; import './Ownable.sol'; import './ThisIsAVeryLongFileOnPurposeToTestTheFirstPathShorterThanTheLastOnelooooooooooong.sol'; -import { ReentrancyGuardUpgradeable2 } from '@apenzeppelin/ReentrancyGuardUpgradeable2.sol'; contract ImportsOrder { constructor() {} From 7bdc0597665c1e4c60c9df57609ebe01631529e6 Mon Sep 17 00:00:00 2001 From: dbale-altoros Date: Thu, 25 Jul 2024 19:15:04 -0300 Subject: [PATCH 28/28] 5.0.2 pre-release --- CHANGELOG.md | 10 ++++++++++ docker/Dockerfile | 2 +- package.json | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce343433..8e91fe0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +## [5.0.2] - 2024-07-25 +### Fixed +- `func-named-parameters` exclude abi.encodeX from the rule [#583](https://github.com/protofire/solhint/pull/583) (Thanks to [@0xCLARITY](https://github.com/0xCLARITY)) +- Several typos in comments [#586](https://github.com/protofire/solhint/pull/586) (Thanks to [@dropbigfish](https://github.com/dropbigfish)) + +### Added +- New Rule: Imports order [#587](https://github.com/protofire/solhint/pull/587) + +

+ ## [5.0.1] - 2024-05-13 ### BREAKING CHANGES (refer to v5.0.0) Fixed an issue on the returining values where only was evaluating the first report instead of all of them. diff --git a/docker/Dockerfile b/docker/Dockerfile index e5d2f8c0..e7af3f49 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -1,5 +1,5 @@ FROM node:20-alpine LABEL maintainer="diego.bale@protofire.io" -ENV VERSION=5.0.1 +ENV VERSION=5.0.2 RUN npm install -g solhint@"$VERSION" \ No newline at end of file diff --git a/package.json b/package.json index 35199efd..bb17a54e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "solhint", - "version": "5.0.1", + "version": "5.0.2", "description": "Solidity Code Linter", "main": "lib/index.js", "keywords": [