diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js index 0c51314d..730cdd17 100644 --- a/conf/rulesets/solhint-all.js +++ b/conf/rulesets/solhint-all.js @@ -50,7 +50,6 @@ module.exports = Object.freeze({ immutablesAsConstants: true, }, ], - 'imports-order': 'warn', 'modifier-name-mixedcase': 'warn', 'named-parameters-mapping': 'warn', 'private-vars-leading-underscore': [ @@ -62,6 +61,7 @@ module.exports = Object.freeze({ 'use-forbidden-name': 'warn', 'var-name-mixedcase': 'warn', 'imports-on-top': 'warn', + 'imports-order': 'warn', ordering: 'warn', 'visibility-modifier-order': 'warn', 'avoid-call-value': 'warn', diff --git a/docs/rules.md b/docs/rules.md index 7b108d8a..1ebe8de5 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -36,7 +36,6 @@ 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 (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 | | | @@ -44,6 +43,7 @@ title: "Rule Index of Solhint" | [var-name-mixedcase](./rules/naming/var-name-mixedcase.md) | Variable names must be in mixedCase. (Does not check IMMUTABLES, use immutable-vars-naming) | $~~~~~~~~$✔️ | | | [func-order](./rules/order/func-order.md) | Function order is incorrect. | | $~~~~~~~$✔️ | | [imports-on-top](./rules/order/imports-on-top.md) | Import statements must be on top. | $~~~~~~~~$✔️ | | +| [imports-order](./rules/naming/imports-order.md) | Order the imports of the contract to follow a certain hierarchy (read "Notes section") | | | | [ordering](./rules/order/ordering.md) | Check order of elements in file and inside each contract, according to the style guide. | | | | [visibility-modifier-order](./rules/order/visibility-modifier-order.md) | Visibility modifier must be first in list of modifiers. | $~~~~~~~~$✔️ | | diff --git a/docs/rules/best-practices/code-complexity.md b/docs/rules/best-practices/code-complexity.md index a24a049e..324c5a66 100644 --- a/docs/rules/best-practices/code-complexity.md +++ b/docs/rules/best-practices/code-complexity.md @@ -67,7 +67,7 @@ while (d > e) { } ``` ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/best-practices/code-complexity.js) diff --git a/docs/rules/best-practices/explicit-types.md b/docs/rules/best-practices/explicit-types.md index abf0b10d..9d685d28 100644 --- a/docs/rules/best-practices/explicit-types.md +++ b/docs/rules/best-practices/explicit-types.md @@ -77,7 +77,7 @@ uint public variableName = uint256(5) ``` ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/best-practices/explicit-types.js) diff --git a/docs/rules/best-practices/function-max-lines.md b/docs/rules/best-practices/function-max-lines.md index 209d1f9d..5294ea07 100644 --- a/docs/rules/best-practices/function-max-lines.md +++ b/docs/rules/best-practices/function-max-lines.md @@ -34,7 +34,7 @@ This rule accepts an array of options: This rule does not have examples. ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/best-practices/function-max-lines.js) diff --git a/docs/rules/best-practices/max-line-length.md b/docs/rules/best-practices/max-line-length.md index d0c02777..bfe50408 100644 --- a/docs/rules/best-practices/max-line-length.md +++ b/docs/rules/best-practices/max-line-length.md @@ -36,7 +36,7 @@ This rule accepts an array of options: This rule does not have examples. ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/best-practices/max-line-length.js) diff --git a/docs/rules/best-practices/max-states-count.md b/docs/rules/best-practices/max-states-count.md index 6306d5e8..29297f0c 100644 --- a/docs/rules/best-practices/max-states-count.md +++ b/docs/rules/best-practices/max-states-count.md @@ -99,7 +99,7 @@ This rule accepts an array of options: ``` ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/best-practices/max-states-count.js) diff --git a/docs/rules/best-practices/no-console.md b/docs/rules/best-practices/no-console.md index c6765893..86949fb6 100644 --- a/docs/rules/best-practices/no-console.md +++ b/docs/rules/best-practices/no-console.md @@ -53,7 +53,7 @@ import "forge-std/consoleN.sol" ``` ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/best-practices/no-console.js) diff --git a/docs/rules/best-practices/no-empty-blocks.md b/docs/rules/best-practices/no-empty-blocks.md index bc54fe80..e0f0670c 100644 --- a/docs/rules/best-practices/no-empty-blocks.md +++ b/docs/rules/best-practices/no-empty-blocks.md @@ -65,7 +65,7 @@ constructor () { } ``` ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/best-practices/no-empty-blocks.js) diff --git a/docs/rules/best-practices/no-global-import.md b/docs/rules/best-practices/no-global-import.md index 2ebbd01c..7ac4f910 100644 --- a/docs/rules/best-practices/no-global-import.md +++ b/docs/rules/best-practices/no-global-import.md @@ -63,7 +63,7 @@ import "foo.sol" ``` ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/best-practices/no-global-import.js) diff --git a/docs/rules/best-practices/no-unused-import.md b/docs/rules/best-practices/no-unused-import.md index ff253f1d..2d31bb24 100644 --- a/docs/rules/best-practices/no-unused-import.md +++ b/docs/rules/best-practices/no-unused-import.md @@ -51,7 +51,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war ``` ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/best-practices/no-unused-import.js) diff --git a/docs/rules/best-practices/no-unused-vars.md b/docs/rules/best-practices/no-unused-vars.md index c34045cc..719f9836 100644 --- a/docs/rules/best-practices/no-unused-vars.md +++ b/docs/rules/best-practices/no-unused-vars.md @@ -31,7 +31,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war This rule does not have examples. ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/best-practices/no-unused-vars.js) diff --git a/docs/rules/best-practices/one-contract-per-file.md b/docs/rules/best-practices/one-contract-per-file.md index 2a59f567..21a1e472 100644 --- a/docs/rules/best-practices/one-contract-per-file.md +++ b/docs/rules/best-practices/one-contract-per-file.md @@ -31,7 +31,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war This rule does not have examples. ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/best-practices/one-contract-per-file.js) diff --git a/docs/rules/best-practices/payable-fallback.md b/docs/rules/best-practices/payable-fallback.md index 3108b87c..9405d870 100644 --- a/docs/rules/best-practices/payable-fallback.md +++ b/docs/rules/best-practices/payable-fallback.md @@ -60,7 +60,7 @@ fallback() {} function g() payable {} ``` ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/best-practices/payable-fallback.js) diff --git a/docs/rules/best-practices/reason-string.md b/docs/rules/best-practices/reason-string.md index 3b41906f..89d1b960 100644 --- a/docs/rules/best-practices/reason-string.md +++ b/docs/rules/best-practices/reason-string.md @@ -77,7 +77,7 @@ This rule accepts an array of options: ``` ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/best-practices/reason-string.js) diff --git a/docs/rules/gas-consumption/gas-struct-packing.md b/docs/rules/gas-consumption/gas-struct-packing.md index 313edfc1..9d199fc7 100644 --- a/docs/rules/gas-consumption/gas-struct-packing.md +++ b/docs/rules/gas-consumption/gas-struct-packing.md @@ -25,6 +25,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war ### Notes - This rule assumes all UserDefinedTypeName take a new slot. (beware of Enums inside Structs) +- Simple cases like a struct with three addresses might be reported as false positive. (needs to be fixed) - [source 1](https://coinsbench.com/comprehensive-guide-tips-and-tricks-for-gas-optimization-in-solidity-5380db734404) of the rule initiative (Variable Packing) - [source 2](https://www.rareskills.io/post/gas-optimization?postId=c9db474a-ff97-4fa3-a51d-fe13ccb8fe3b#viewer-f8m1r) of the rule initiative diff --git a/docs/rules/naming/contract-name-capwords.md b/docs/rules/naming/contract-name-capwords.md index acdbf7b7..81b02011 100644 --- a/docs/rules/naming/contract-name-capwords.md +++ b/docs/rules/naming/contract-name-capwords.md @@ -34,7 +34,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war This rule does not have examples. ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/naming/contract-name-capwords.js) diff --git a/docs/rules/naming/event-name-capwords.md b/docs/rules/naming/event-name-capwords.md index 8eb29be1..64e668ea 100644 --- a/docs/rules/naming/event-name-capwords.md +++ b/docs/rules/naming/event-name-capwords.md @@ -34,7 +34,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war This rule does not have examples. ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/naming/event-name-capwords.js) diff --git a/docs/rules/naming/imports-order.md b/docs/rules/naming/imports-order.md index 7c668483..c20876cd 100644 --- a/docs/rules/naming/imports-order.md +++ b/docs/rules/naming/imports-order.md @@ -35,9 +35,9 @@ This rule accepts a string option of rule severity. Must be one of "error", "war This rule does not have examples. ## Version -This rule was introduced in [Solhint 5.0.2](https://github.com/protofire/solhint/blob/v5.0.2) +This rule is introduced in the latest version. ## Resources -- [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/naming/imports-order.js) -- [Document source](https://github.com/protofire/solhint/blob/master/docs/rules/naming/imports-order.md) -- [Test cases](https://github.com/protofire/solhint/blob/master/test/rules/naming/imports-order.js) +- [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/order/imports-order.js) +- [Document source](https://github.com/protofire/solhint/blob/master/docs/rules/order/imports-order.md) +- [Test cases](https://github.com/protofire/solhint/blob/master/test/rules/order/imports-order.js) diff --git a/docs/rules/naming/interface-starts-with-i.md b/docs/rules/naming/interface-starts-with-i.md index 41d163c7..b6bfc539 100644 --- a/docs/rules/naming/interface-starts-with-i.md +++ b/docs/rules/naming/interface-starts-with-i.md @@ -42,7 +42,7 @@ interface Foo { function foo () external; } ``` ## Version -This rule is introduced in the latest version. +This rule was introduced in [Solhint 5.0.4](https://github.com/protofire/solhint/blob/v5.0.4) ## Resources - [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/best-practices/interface-starts-with-i.js) diff --git a/e2e/test.js b/e2e/test.js index 59c2bfd7..598fe1c2 100644 --- a/e2e/test.js +++ b/e2e/test.js @@ -46,7 +46,7 @@ describe('e2e', function () { const { code, stderr } = shell.exec(`${NODE}solhint Foo.sol -c ./noconfig/.solhint.json`) expect(code).to.equal(EXIT_CODES.BAD_OPTIONS) - expect(stderr).to.include('couldnt be found') + expect(stderr).to.include("couldn't be found") }) it('should create an initial config with --init', function () { diff --git a/lib/rules/gas-consumption/gas-struct-packing.js b/lib/rules/gas-consumption/gas-struct-packing.js index f8811d54..3b4a1d1e 100644 --- a/lib/rules/gas-consumption/gas-struct-packing.js +++ b/lib/rules/gas-consumption/gas-struct-packing.js @@ -12,6 +12,9 @@ const meta = { { note: 'This rule assumes all UserDefinedTypeName take a new slot. (beware of Enums inside Structs) ', }, + { + note: 'Simple cases like a struct with three addresses might be reported as false positive. (needs to be fixed)', + }, { note: '[source 1](https://coinsbench.com/comprehensive-guide-tips-and-tricks-for-gas-optimization-in-solidity-5380db734404) of the rule initiative (Variable Packing)', }, diff --git a/lib/rules/naming/index.js b/lib/rules/naming/index.js index 9ca600c9..e9335b9b 100644 --- a/lib/rules/naming/index.js +++ b/lib/rules/naming/index.js @@ -11,7 +11,6 @@ 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 ImportsOrderChecker = require('./imports-order') module.exports = function checkers(reporter, config) { return [ @@ -28,6 +27,5 @@ module.exports = function checkers(reporter, config) { new ImmutableVarsNamingChecker(reporter, config), new FunctionNamedParametersChecker(reporter, config), new FoundryTestFunctionsChecker(reporter, config), - new ImportsOrderChecker(reporter, config), ] } diff --git a/lib/rules/naming/imports-order.js b/lib/rules/order/imports-order.js similarity index 100% rename from lib/rules/naming/imports-order.js rename to lib/rules/order/imports-order.js diff --git a/lib/rules/order/index.js b/lib/rules/order/index.js index 2d516e1a..e7cde89d 100644 --- a/lib/rules/order/index.js +++ b/lib/rules/order/index.js @@ -2,6 +2,7 @@ const FuncOrderChecker = require('./func-order') const ImportsOnTopChecker = require('./imports-on-top') const VisibilityModifierOrderChecker = require('./visibility-modifier-order') const OrderingChecker = require('./ordering') +const ImportsOrderChecker = require('./imports-order') module.exports = function order(reporter, tokens) { return [ @@ -9,5 +10,6 @@ module.exports = function order(reporter, tokens) { new ImportsOnTopChecker(reporter), new VisibilityModifierOrderChecker(reporter, tokens), new OrderingChecker(reporter), + new ImportsOrderChecker(reporter), ] } diff --git a/solhint.js b/solhint.js index bb5b617d..6c172666 100755 --- a/solhint.js +++ b/solhint.js @@ -126,13 +126,14 @@ function executeMainActionLogic() { const customConfig = program.opts().config if (customConfig && !fs.existsSync(customConfig)) { - console.error(`Config file "${customConfig}" couldnt be found.`) + console.error(`Config file "${customConfig}" couldn't be found.`) process.exit(EXIT_CODES.BAD_OPTIONS) } let reports try { const reportLists = program.args.filter(_.isString).map(processPath) + // console.log('reportLists :>> ', reportLists) reports = _.flatten(reportLists) } catch (e) { console.error(e) diff --git a/test/rules/gas-consumption/gas-struct-packing.js b/test/rules/gas-consumption/gas-struct-packing.js index 975119e8..4363777a 100644 --- a/test/rules/gas-consumption/gas-struct-packing.js +++ b/test/rules/gas-consumption/gas-struct-packing.js @@ -2,12 +2,14 @@ const assert = require('assert') const linter = require('../../../lib/index') const TEST_CASES = require('../../fixtures/gas-consumption/gas-struct-packing-data') +// const { contractWith, multiLine } = require('../../common/contract-builder') + const replaceErrorMsg = (variableName) => { const errMsg = `GC: For [ ${variableName} ] struct, packing seems inefficient. Try rearranging to achieve 32bytes slots` return errMsg } -describe('Linter - gas-struct-packingone', () => { +describe('Linter - gas-struct-packing', () => { for (const contract of TEST_CASES.contractStructsInefficient) { it(`should raise error for ${contract.name}`, () => { const code = contract.code @@ -30,4 +32,24 @@ describe('Linter - gas-struct-packingone', () => { assert.equal(report.errorCount, 0) }) } + + // it(`should NOT raise error for `, () => { + // const code = contractWith( + // multiLine( + // // // Large Types Followed by Small Types + // 'struct MyStruct2 {', + // ' address addr1;', + // ' address addr2;', + // ' address addr3;', + // '}' + // ) + // ) + // const report = linter.processStr(code, { + // rules: { 'gas-struct-packing': 'error' }, + // }) + + // console.log(report) + + // // assert.equal(report.errorCount, 0) + // }) })