diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ed59d9d..e2c6f0ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,19 @@ +## [5.0.5] - 2025-01-16 +### Fixed +- `gas-custom-errors` [#620](https://github.com/protofire/solhint/pull/620) - Support for Custom Errors inside `require` statements +- `compiler-version` [#621](https://github.com/protofire/solhint/pull/621) - Upgraded minimum requirement for the rule +- `reentrancy` [#622](https://github.com/protofire/solhint/pull/622) - Fixed path and typos +- Typos [#623](https://github.com/protofire/solhint/pull/623) - Fixed typos +- Typo [#625](https://github.com/protofire/solhint/pull/625) - Fixed typo + + +### Added +- New Rule: Duplicated Imports [#626](https://github.com/protofire/solhint/pull/626) +- Cute Message on console report to gather community into discord channel + +

+ + ## [5.0.4] - 2024-12-31 ### Fixed - `imports-order` [#595](https://github.com/protofire/solhint/pull/595) - Replaced single quotes with double quotes diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js index 5c0629e6..3b964d3f 100644 --- a/conf/rulesets/solhint-all.js +++ b/conf/rulesets/solhint-all.js @@ -36,6 +36,7 @@ module.exports = Object.freeze({ 'gas-strict-inequalities': 'warn', 'gas-struct-packing': 'warn', 'comprehensive-interface': 'warn', + 'duplicated-imports': 'warn', quotes: ['error', 'double'], 'const-name-snakecase': 'warn', 'contract-name-capwords': 'warn', @@ -50,7 +51,6 @@ module.exports = Object.freeze({ immutablesAsConstants: true, }, ], - 'imports-order': 'warn', 'modifier-name-mixedcase': 'warn', 'named-parameters-mapping': 'warn', 'private-vars-leading-underscore': [ @@ -62,6 +62,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', @@ -71,7 +72,7 @@ module.exports = Object.freeze({ 'avoid-throw': 'warn', 'avoid-tx-origin': 'warn', 'check-send-result': 'warn', - 'compiler-version': ['error', '^0.8.0'], + 'compiler-version': ['error', '^0.8.24'], 'func-visibility': [ 'warn', { diff --git a/conf/rulesets/solhint-recommended.js b/conf/rulesets/solhint-recommended.js index da7f6095..9e689089 100644 --- a/conf/rulesets/solhint-recommended.js +++ b/conf/rulesets/solhint-recommended.js @@ -43,7 +43,7 @@ module.exports = Object.freeze({ 'avoid-throw': 'warn', 'avoid-tx-origin': 'warn', 'check-send-result': 'warn', - 'compiler-version': ['error', '^0.8.0'], + 'compiler-version': ['error', '^0.8.24'], 'func-visibility': [ 'warn', { diff --git a/docker/Dockerfile b/docker/Dockerfile index a520a3f2..c70630eb 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.4 +ENV VERSION=5.0.5 RUN npm install -g solhint@"$VERSION" \ No newline at end of file diff --git a/docs/rules.md b/docs/rules.md index 7b108d8a..603dba4c 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -28,6 +28,7 @@ title: "Rule Index of Solhint" | Rule Id | Error | Recommended | Deprecated | | ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------- | ------------ | ----------- | | [interface-starts-with-i](./rules/naming/interface-starts-with-i.md) | Solidity Interfaces names should start with an `I` | | | +| [duplicated-imports](./rules/miscellaneous/duplicated-imports.md) | Check if an import is done twice in the same file and there is no alias | | | | [const-name-snakecase](./rules/naming/const-name-snakecase.md) | Constant name must be in capitalized SNAKE_CASE. (Does not check IMMUTABLES, use immutable-vars-naming) | $~~~~~~~~$✔️ | | | [contract-name-capwords](./rules/naming/contract-name-capwords.md) | Contract, Structs and Enums should be in CapWords. | $~~~~~~~~$✔️ | | | [event-name-capwords](./rules/naming/event-name-capwords.md) | Event name must be in CapWords. | $~~~~~~~~$✔️ | | @@ -36,7 +37,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 +44,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/miscellaneous/duplicated-imports.md b/docs/rules/miscellaneous/duplicated-imports.md new file mode 100644 index 00000000..6088c1a6 --- /dev/null +++ b/docs/rules/miscellaneous/duplicated-imports.md @@ -0,0 +1,41 @@ +--- +warning: "This is a dynamically generated file. Do not edit manually." +layout: "default" +title: "duplicated-imports | Solhint" +--- + +# duplicated-imports +![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 +Check if an import is done twice in the same file and there is no alias + +## Options +This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Defaults to warn. + +### Example Config +```json +{ + "rules": { + "duplicated-imports": "warn" + } +} +``` + +### Notes +- Rule reports "(inline) duplicated" if the same object is imported more than once in the same import statement +- Rule reports "(globalSamePath) duplicated" if the same object is imported on another import statement from same location +- Rule reports "(globalDiffPath) duplicated" if the same object is imported on another import statement, from other location, but no alias +- Rule does NOT support this kind of import "import * as Alias from "./filename.sol" + +## Examples +This rule does not have examples. + +## Version +This rule is introduced in the latest version. + +## Resources +- [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/miscellaneous/duplicated-imports.js) +- [Document source](https://github.com/protofire/solhint/blob/master/docs/rules/miscellaneous/duplicated-imports.md) +- [Test cases](https://github.com/protofire/solhint/blob/master/test/rules/miscellaneous/duplicated-imports.js) 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/docs/rules/security/compiler-version.md b/docs/rules/security/compiler-version.md index 5bb37df6..6082b579 100644 --- a/docs/rules/security/compiler-version.md +++ b/docs/rules/security/compiler-version.md @@ -20,14 +20,14 @@ This rule accepts an array of options: | Index | Description | Default Value | | ----- | ----------------------------------------------------- | ------------- | | 0 | Rule severity. Must be one of "error", "warn", "off". | error | -| 1 | Semver requirement | ^0.8.0 | +| 1 | Semver requirement | ^0.8.24 | ### Example Config ```json { "rules": { - "compiler-version": ["error","^0.8.0"] + "compiler-version": ["error","^0.8.24"] } } ``` diff --git a/docs/use-in-app.md b/docs/use-in-app.md index 72920295..041b04d9 100644 --- a/docs/use-in-app.md +++ b/docs/use-in-app.md @@ -27,7 +27,7 @@ report.messages.forEach(curError => console.log(curError.message)); **linter.processStr**: - Arguments: - - code {String} - Source code to validation + - code {String} - Source code for validation - config {Object} - Object representation of .solhint.json configuration - Returns: - report {Report} - object that contains list of errors and warnings diff --git a/e2e/03-no-empty-blocks/.solhint.json b/e2e/03-no-empty-blocks/.solhint.json index a64b9a28..6a0c26c9 100644 --- a/e2e/03-no-empty-blocks/.solhint.json +++ b/e2e/03-no-empty-blocks/.solhint.json @@ -1,5 +1,6 @@ { "rules": { - "no-empty-blocks": "error" + "no-empty-blocks": "error", + "compiler-version": "off" } } diff --git a/e2e/04-dotSol-on-path/.solhint.json b/e2e/04-dotSol-on-path/.solhint.json index 1ee2c693..97d9efed 100644 --- a/e2e/04-dotSol-on-path/.solhint.json +++ b/e2e/04-dotSol-on-path/.solhint.json @@ -1,5 +1,6 @@ { "rules": { - "no-empty-blocks": "off" + "no-empty-blocks": "off", + "compiler-version": "off" } } diff --git a/e2e/05-max-warnings/contracts/Foo.sol b/e2e/05-max-warnings/contracts/Foo.sol index 1c045747..e97cea3c 100644 --- a/e2e/05-max-warnings/contracts/Foo.sol +++ b/e2e/05-max-warnings/contracts/Foo.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity >=0.8.24; contract Foo { uint256 public constant test1 = 1; diff --git a/e2e/06-formatters/contracts/Foo2.sol b/e2e/06-formatters/contracts/Foo2.sol index 4a5c2af2..9e0b71cf 100644 --- a/e2e/06-formatters/contracts/Foo2.sol +++ b/e2e/06-formatters/contracts/Foo2.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity >=0.8.24; contract Foo { uint256 public constant test1 = 1; diff --git a/e2e/06-formatters/contracts/Foo3.sol b/e2e/06-formatters/contracts/Foo3.sol index 8c1c1e0a..e9ea7297 100644 --- a/e2e/06-formatters/contracts/Foo3.sol +++ b/e2e/06-formatters/contracts/Foo3.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity >=0.8.24; contract Foo { uint256 public constant TEST1 = 1; diff --git a/e2e/06-formatters/helpers/helpers.js b/e2e/06-formatters/helpers/helpers.js index 8376310f..e23b5682 100644 --- a/e2e/06-formatters/helpers/helpers.js +++ b/e2e/06-formatters/helpers/helpers.js @@ -3,7 +3,7 @@ const foo1Output = [ line: 2, column: 1, severity: 'Error', - message: 'Compiler version >=0.6.0 does not satisfy the ^0.8.0 semver requirement', + message: 'Compiler version >=0.6.0 does not satisfy the ^0.8.24 semver requirement', ruleId: 'compiler-version', fix: null, filePath: 'contracts/Foo.sol', diff --git a/e2e/08-autofix/_commands/Foo1.sol b/e2e/08-autofix/_commands/Foo1.sol index 78bcaec7..f5b6b3c6 100644 --- a/e2e/08-autofix/_commands/Foo1.sol +++ b/e2e/08-autofix/_commands/Foo1.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: Apache-2.0 -pragma solidity ^0.8.4; +pragma solidity ^0.8.24; import {ERC20Burnable} from '@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol'; diff --git a/e2e/08-autofix/_commands/Foo1AfterFix.sol b/e2e/08-autofix/_commands/Foo1AfterFix.sol index ca900d69..b45f2bef 100644 --- a/e2e/08-autofix/_commands/Foo1AfterFix.sol +++ b/e2e/08-autofix/_commands/Foo1AfterFix.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: Apache-2.0 -pragma solidity ^0.8.4; +pragma solidity ^0.8.24; import {ERC20Burnable} from '@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol'; diff --git a/e2e/08-autofix/_commands/Foo1BeforeFix.sol b/e2e/08-autofix/_commands/Foo1BeforeFix.sol index 78bcaec7..f5b6b3c6 100644 --- a/e2e/08-autofix/_commands/Foo1BeforeFix.sol +++ b/e2e/08-autofix/_commands/Foo1BeforeFix.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: Apache-2.0 -pragma solidity ^0.8.4; +pragma solidity ^0.8.24; import {ERC20Burnable} from '@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol'; diff --git a/e2e/08-autofix/avoid-suicide/.solhint.json b/e2e/08-autofix/avoid-suicide/.solhint.json index 7f54d632..99c16ecb 100644 --- a/e2e/08-autofix/avoid-suicide/.solhint.json +++ b/e2e/08-autofix/avoid-suicide/.solhint.json @@ -1,5 +1,6 @@ { "rules": { - "avoid-suicide": "error" + "avoid-suicide": "error", + "compiler-version": "off" } } diff --git a/e2e/08-autofix/contract-name-capwords/.solhint.json b/e2e/08-autofix/contract-name-capwords/.solhint.json index e1789abb..d80e50fc 100644 --- a/e2e/08-autofix/contract-name-capwords/.solhint.json +++ b/e2e/08-autofix/contract-name-capwords/.solhint.json @@ -1,5 +1,6 @@ { "rules": { - "contract-name-capwords": "error" + "contract-name-capwords": "error", + "compiler-version": "off" } } diff --git a/e2e/08-autofix/event-name-capwords/.solhint.json b/e2e/08-autofix/event-name-capwords/.solhint.json index 22cb73d9..9fc6a6b8 100644 --- a/e2e/08-autofix/event-name-capwords/.solhint.json +++ b/e2e/08-autofix/event-name-capwords/.solhint.json @@ -1,5 +1,6 @@ { "rules": { - "event-name-capwords": "error" + "event-name-capwords": "error", + "compiler-version": "off" } } diff --git a/e2e/08-autofix/explicit-types/.solhint.json b/e2e/08-autofix/explicit-types/.solhint.json index 7bd7c6a2..f46aa779 100644 --- a/e2e/08-autofix/explicit-types/.solhint.json +++ b/e2e/08-autofix/explicit-types/.solhint.json @@ -1,5 +1,6 @@ { "rules": { - "explicit-types": ["error", "explicit"] + "explicit-types": ["error", "explicit"], + "compiler-version": "off" } } diff --git a/e2e/08-autofix/imports-order/.solhint.json b/e2e/08-autofix/imports-order/.solhint.json index b17afc51..13709f75 100644 --- a/e2e/08-autofix/imports-order/.solhint.json +++ b/e2e/08-autofix/imports-order/.solhint.json @@ -1,5 +1,6 @@ { "rules": { - "imports-order": "error" + "imports-order": "error", + "compiler-version": "off" } } diff --git a/e2e/08-autofix/no-console/.solhint.json b/e2e/08-autofix/no-console/.solhint.json index 2ff50f91..f5e0eb1e 100644 --- a/e2e/08-autofix/no-console/.solhint.json +++ b/e2e/08-autofix/no-console/.solhint.json @@ -1,5 +1,6 @@ { "rules": { - "no-console": "error" + "no-console": "error", + "compiler-version": "off" } } diff --git a/e2e/08-autofix/payable-fallback/.solhint.json b/e2e/08-autofix/payable-fallback/.solhint.json index aa02d3b1..c6939a02 100644 --- a/e2e/08-autofix/payable-fallback/.solhint.json +++ b/e2e/08-autofix/payable-fallback/.solhint.json @@ -1,5 +1,6 @@ { "rules": { - "payable-fallback": "error" + "payable-fallback": "error", + "compiler-version": "off" } } diff --git a/e2e/08-autofix/private-vars-underscore/.solhint.json b/e2e/08-autofix/private-vars-underscore/.solhint.json index 47ff676f..a4b8eeff 100644 --- a/e2e/08-autofix/private-vars-underscore/.solhint.json +++ b/e2e/08-autofix/private-vars-underscore/.solhint.json @@ -1,5 +1,6 @@ { "rules": { - "private-vars-leading-underscore": ["error",{"strict":false}] + "private-vars-leading-underscore": ["error",{"strict":false}], + "compiler-version": "off" } } diff --git a/e2e/08-autofix/quotes/.doubleQuotes.json b/e2e/08-autofix/quotes/.doubleQuotes.json index bd5e47be..21faf61b 100644 --- a/e2e/08-autofix/quotes/.doubleQuotes.json +++ b/e2e/08-autofix/quotes/.doubleQuotes.json @@ -1,5 +1,6 @@ { "rules": { - "quotes": ["error", "double"] + "quotes": ["error", "double"], + "compiler-version": "off" } } diff --git a/e2e/08-autofix/quotes/.singleQuotes.json b/e2e/08-autofix/quotes/.singleQuotes.json index 1cd619b8..cde73263 100644 --- a/e2e/08-autofix/quotes/.singleQuotes.json +++ b/e2e/08-autofix/quotes/.singleQuotes.json @@ -1,5 +1,6 @@ { "rules": { - "quotes": ["error", "single"] + "quotes": ["error", "single"], + "compiler-version": "off" } } diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js index f57ae859..2c800223 100644 --- a/e2e/autofix-test.js +++ b/e2e/autofix-test.js @@ -145,7 +145,7 @@ describe('e2e', function () { const reportLines = stdout.split('\n') const finalLine = '5 problems (5 errors, 0 warnings)' - expect(reportLines[reportLines.length - 3]).to.contain(finalLine) + expect(reportLines[reportLines.length - 7]).to.contain(finalLine) result = compareTextFiles(currentFile, afterFixFile) expect(result).to.be.true @@ -194,7 +194,7 @@ describe('e2e', function () { it('should get the right report (2)', () => { const reportLines = stdout.split('\n') const finalLine = '27 problems (27 errors, 0 warnings)' - expect(reportLines[reportLines.length - 3]).to.contain(finalLine) + expect(reportLines[reportLines.length - 7]).to.contain(finalLine) }) }) @@ -240,7 +240,7 @@ describe('e2e', function () { it('should get the right report (3)', () => { const reportLines = stdout.split('\n') const finalLine = '9 problems (9 errors, 0 warnings)' - expect(reportLines[reportLines.length - 3]).to.contain(finalLine) + expect(reportLines[reportLines.length - 7]).to.contain(finalLine) }) }) @@ -286,7 +286,7 @@ describe('e2e', function () { it('should get the right report (4)', () => { const reportLines = stdout.split('\n') const finalLine = '19 problems (19 errors, 0 warnings)' - expect(reportLines[reportLines.length - 3]).to.contain(finalLine) + expect(reportLines[reportLines.length - 7]).to.contain(finalLine) }) }) @@ -332,7 +332,7 @@ describe('e2e', function () { it('should get the right report (5)', () => { const reportLines = stdout.split('\n') const finalLine = '11 problems (11 errors, 0 warnings)' - expect(reportLines[reportLines.length - 3]).to.contain(finalLine) + expect(reportLines[reportLines.length - 7]).to.contain(finalLine) }) }) @@ -379,7 +379,7 @@ describe('e2e', function () { it('should get the right report (6)', () => { const reportLines = stdout.split('\n') const finalLine = '8 problems (8 errors, 0 warnings)' - expect(reportLines[reportLines.length - 3]).to.contain(finalLine) + expect(reportLines[reportLines.length - 7]).to.contain(finalLine) }) }) @@ -419,7 +419,7 @@ describe('e2e', function () { it('should get the right report (6)', () => { const reportLines = stdout.split('\n') const finalLine = '8 problems (8 errors, 0 warnings)' - expect(reportLines[reportLines.length - 3]).to.contain(finalLine) + expect(reportLines[reportLines.length - 7]).to.contain(finalLine) }) }) @@ -466,7 +466,7 @@ describe('e2e', function () { it('should get the right report (7)', () => { const reportLines = stdout.split('\n') const finalLine = '3 problems (3 errors, 0 warnings)' - expect(reportLines[reportLines.length - 3]).to.contain(finalLine) + expect(reportLines[reportLines.length - 7]).to.contain(finalLine) }) }) @@ -512,7 +512,7 @@ describe('e2e', function () { it('should get the right report (8)', () => { const reportLines = stdout.split('\n') const finalLine = '5 problems (5 errors, 0 warnings)' - expect(reportLines[reportLines.length - 3]).to.contain(finalLine) + expect(reportLines[reportLines.length - 7]).to.contain(finalLine) }) }) @@ -558,7 +558,7 @@ describe('e2e', function () { 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) + expect(reportLines[reportLines.length - 7]).to.contain(finalLine) }) }) @@ -604,7 +604,7 @@ describe('e2e', function () { it('should get the right report (10)', () => { const reportLines = stdout.split('\n') const finalLine = '18 problems (18 errors, 0 warnings)' - expect(reportLines[reportLines.length - 3]).to.contain(finalLine) + expect(reportLines[reportLines.length - 7]).to.contain(finalLine) }) }) @@ -649,7 +649,7 @@ describe('e2e', function () { // 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) + // expect(reportLines[reportLines.length - 7]).to.contain(finalLine) // }) // }) diff --git a/e2e/formatters-test.js b/e2e/formatters-test.js index 0b955587..f903f240 100644 --- a/e2e/formatters-test.js +++ b/e2e/formatters-test.js @@ -507,7 +507,7 @@ describe('e2e', function () { { level: 'error', message: { - text: 'Compiler version >=0.6.0 does not satisfy the ^0.8.0 semver requirement', + text: 'Compiler version >=0.6.0 does not satisfy the ^0.8.24 semver requirement', }, locations: [ { 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-custom-errors.js b/lib/rules/gas-consumption/gas-custom-errors.js index 577de5ab..5907d862 100644 --- a/lib/rules/gas-consumption/gas-custom-errors.js +++ b/lib/rules/gas-consumption/gas-custom-errors.js @@ -89,9 +89,25 @@ class GasCustomErrorsChecker extends BaseChecker { FunctionCall(node) { let errorStr = '' if (this.isVersionGreater(node)) { - // added second part of conditional to be able to use require with Custom Errors - if (node.expression.name === 'require' && node.arguments[1].type !== 'FunctionCall') { - errorStr = 'require' + if (node.expression.name === 'require') { + // If no second argument or second argument is a string, flag it + if (node.arguments.length < 2) { + // No second argument + errorStr = 'require' + } else { + const secondArg = node.arguments[1] + if (secondArg.type === 'StringLiteral') { + // e.g. require(cond, "Error message"); + errorStr = 'require' + } else if (secondArg.type !== 'FunctionCall') { + // e.g. require(cond, 42) or require(cond, someVar) + // Probably not a custom error, so still flag + errorStr = 'require' + } + // else if secondArg.type === 'FunctionCall': + // e.g. require(cond, MyCustomError()) + // We skip, because it’s presumably a custom error + } } else if ( node.expression.name === 'revert' && (node.arguments.length === 0 || node.arguments[0].type === 'StringLiteral') 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/miscellaneous/duplicated-imports.js b/lib/rules/miscellaneous/duplicated-imports.js new file mode 100644 index 00000000..567f9d9c --- /dev/null +++ b/lib/rules/miscellaneous/duplicated-imports.js @@ -0,0 +1,251 @@ +const path = require('path') +const BaseChecker = require('../base-checker') +const { severityDescription } = require('../../doc/utils') + +const DEFAULT_SEVERITY = 'warn' + +const ruleId = 'duplicated-imports' +const meta = { + type: 'miscellaneous', + + docs: { + description: `Check if an import is done twice in the same file and there is no alias`, + category: 'Style Guide Rules', + options: [ + { + description: severityDescription, + default: DEFAULT_SEVERITY, + }, + ], + notes: [ + { + note: 'Rule reports "(inline) duplicated" if the same object is imported more than once in the same import statement', + }, + { + note: 'Rule reports "(globalSamePath) duplicated" if the same object is imported on another import statement from same location', + }, + { + note: 'Rule reports "(globalDiffPath) duplicated" if the same object is imported on another import statement, from other location, but no alias', + }, + { + note: 'Rule does NOT support this kind of import "import * as Alias from "./filename.sol"', + }, + ], + }, + + isDefault: false, + recommended: false, + defaultSetup: 'warn', + fixable: true, + schema: null, +} + +class DuplicatedImportsChecker extends BaseChecker { + constructor(reporter) { + super(reporter, ruleId, meta) + + this.imports = [] + } + + ImportDirective(node) { + const normalizedPath = this.normalizePath(node.path) + + const importStatement = { + path: '', + objectNames: [], + } + + importStatement.path = normalizedPath + importStatement.objectNames = node.symbolAliases + ? node.symbolAliases + : this.getObjectName(normalizedPath) + + this.imports.push(importStatement) + } + + 'SourceUnit:exit'(node) { + const duplicates = this.findDuplicates(this.imports) + + for (let i = 0; i < duplicates.length; i++) { + this.error(node, `Duplicated Import (${duplicates[i].type}) ${duplicates[i].name}`) + } + } + + getObjectName(normalizedPath) { + // get file name + const fileNameWithExtension = path.basename(normalizedPath) + // Remove extension + const objectName = fileNameWithExtension.replace('.sol', '') + return [[objectName, null]] + } + + normalizePath(path) { + if (path.startsWith('../')) { + return `./${path}` + } + return path + } + + findInlineDuplicates(data) { + const inlineDuplicates = [] + + data.forEach((entry) => { + const path = entry.path + // To track object names + const objectNamesSet = new Set() + + entry.objectNames.forEach(([objectName]) => { + // If object name already been found , it is a duplicated + if (objectNamesSet.has(objectName)) { + inlineDuplicates.push({ + name: objectName, + type: 'inline', + paths: [path], + }) + } else { + // If it is not found before, we add it + objectNamesSet.add(objectName) + } + }) + }) + + return inlineDuplicates + } + + finGlobalDuplicatesSamePath(data) { + const duplicates = [] + + // Loop through data + data.forEach((entry) => { + const path = entry.path + + // Object to track object names on each path + const objectNamesMap = {} + + // Loop through each objectName of current object + entry.objectNames.forEach(([objectName]) => { + if (!objectNamesMap[objectName]) { + objectNamesMap[objectName] = [] + } + objectNamesMap[objectName].push(path) + }) + + // Compare this object with the rest to detect duplicates + data.forEach((otherEntry) => { + if (otherEntry !== entry) { + otherEntry.objectNames.forEach(([objectName]) => { + if ( + objectNamesMap[objectName] && + objectNamesMap[objectName].includes(otherEntry.path) + ) { + // Add path only if it is not present + const existingDuplicate = duplicates.find( + (duplicate) => + duplicate.name === objectName && + duplicate.type === 'global' && + duplicate.paths.includes(entry.path) + ) + + if (!existingDuplicate) { + duplicates.push({ + name: objectName, + type: 'globalSamePath', + paths: [entry.path], // Just add path once, it is always the same + }) + } + } + }) + } + }) + }) + + return duplicates + } + + finGlobalDuplicatesDiffPathNoAlias(data) { + const duplicates = [] + + // Loop through data + data.forEach((entry) => { + // Object to track names on each path + entry.objectNames.forEach(([objectName, alias]) => { + // Only compare if there is no alias + if (!alias) { + // Go through rest of objects to search for duplicates + data.forEach((otherEntry) => { + if (otherEntry !== entry) { + otherEntry.objectNames.forEach(([otherObjectName, otherAlias]) => { + // If object name is the same, has no alias and different path + if ( + objectName === otherObjectName && + !otherAlias && + entry.path !== otherEntry.path + ) { + // Check if the name is already in the duplicated array + const existingDuplicate = duplicates.find( + (duplicate) => + duplicate.name === objectName && + duplicate.type === 'global' && + duplicate.paths.includes(entry.path) + ) + + // Add new object if doesn't exist + if (!existingDuplicate) { + duplicates.push({ + name: objectName, + type: 'globalDiffPath', + paths: [entry.path, otherEntry.path], + }) + } + + // Add path if already exists + if (existingDuplicate && !existingDuplicate.paths.includes(otherEntry.path)) { + existingDuplicate.paths.push(otherEntry.path) + } + } + }) + } + }) + } + }) + }) + + return duplicates + } + + removeDuplicatedObjects(data) { + const uniqueData = data.filter((value, index, self) => { + // Order path arrays to be compared later + const sortedPaths = value.paths.slice().sort() + + return ( + index === + self.findIndex( + (t) => + t.name === value.name && + t.type === value.type && + // Compare ordered arrays of paths + JSON.stringify(t.paths.slice().sort()) === JSON.stringify(sortedPaths) + ) + ) + }) + + return uniqueData + } + + findDuplicates(data) { + /// @TODO THIS LOGIC CAN BE IMPROVED - Not done due lack of time + + const duplicates1 = this.findInlineDuplicates(data) + + const duplicates2 = this.finGlobalDuplicatesSamePath(data) + + const duplicates3 = this.finGlobalDuplicatesDiffPathNoAlias(data) + + const duplicates = this.removeDuplicatedObjects(duplicates1.concat(duplicates2, duplicates3)) + + return duplicates + } +} + +module.exports = DuplicatedImportsChecker diff --git a/lib/rules/miscellaneous/index.js b/lib/rules/miscellaneous/index.js index 680f6913..0edeb854 100644 --- a/lib/rules/miscellaneous/index.js +++ b/lib/rules/miscellaneous/index.js @@ -1,9 +1,11 @@ const QuotesChecker = require('./quotes') const ComprehensiveInterfaceChecker = require('./comprehensive-interface') +const DuplicatedImportsChecker = require('./duplicated-imports') module.exports = function checkers(reporter, config, tokens) { return [ new QuotesChecker(reporter, config, tokens), new ComprehensiveInterfaceChecker(reporter, config, tokens), + new DuplicatedImportsChecker(reporter), ] } 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/lib/rules/security/compiler-version.js b/lib/rules/security/compiler-version.js index 955a53bd..c7e4ef36 100644 --- a/lib/rules/security/compiler-version.js +++ b/lib/rules/security/compiler-version.js @@ -4,7 +4,7 @@ const { severityDescription } = require('../../doc/utils') const ruleId = 'compiler-version' const DEFAULT_SEVERITY = 'error' -const DEFAULT_SEMVER = '^0.8.0' +const DEFAULT_SEMVER = '^0.8.24' const meta = { type: 'security', diff --git a/lib/rules/security/reentrancy.js b/lib/rules/security/reentrancy.js index 32eef177..566f600a 100644 --- a/lib/rules/security/reentrancy.js +++ b/lib/rules/security/reentrancy.js @@ -15,25 +15,25 @@ const meta = { good: [ { description: 'Invulnerable Contract 1', - code: require('../../../test/fixtures/security/reentrancy-invulenarble')[0], + code: require('../../../test/fixtures/security/reentrancy-invulnerable')[0], }, { description: 'Invulnerable Contract 2', - code: require('../../../test/fixtures/security/reentrancy-invulenarble')[1], + code: require('../../../test/fixtures/security/reentrancy-invulnerable')[1], }, { description: 'Invulnerable Contract 3', - code: require('../../../test/fixtures/security/reentrancy-invulenarble')[2], + code: require('../../../test/fixtures/security/reentrancy-invulnerable')[2], }, ], bad: [ { description: 'Vulnerable Contract 1', - code: require('../../../test/fixtures/security/reentrancy-vulenarble')[0], + code: require('../../../test/fixtures/security/reentrancy-vulnerable')[0], }, { description: 'Vulnerable Contract 2', - code: require('../../../test/fixtures/security/reentrancy-vulenarble')[1], + code: require('../../../test/fixtures/security/reentrancy-vulnerable')[1], }, ], }, diff --git a/package-lock.json b/package-lock.json index fef8f0d7..678f151b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "solhint", - "version": "5.0.3", + "version": "5.0.4", "lockfileVersion": 2, "requires": true, "packages": { diff --git a/package.json b/package.json index dcaaa8c2..09b61a40 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "solhint", - "version": "5.0.4", + "version": "5.0.5", "description": "Solidity Code Linter", "main": "lib/index.js", "keywords": [ diff --git a/solhint.js b/solhint.js index bb5b617d..01587e17 100755 --- a/solhint.js +++ b/solhint.js @@ -4,6 +4,7 @@ const _ = require('lodash') const fs = require('fs') const process = require('process') const readline = require('readline') +const chalk = require('chalk') const linter = require('./lib/index') const { loadConfig } = require('./lib/config/config-file') @@ -126,13 +127,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) @@ -305,7 +307,29 @@ function printReports(reports, formatter) { } const fullReport = formatter(reports) + (finalMessage || '') - if (!program.opts().quiet) console.log(fullReport) + + if (!program.opts().quiet) { + console.log(fullReport) + if (fullReport && !program.opts().formatter) { + console.log( + chalk.italic.bgYellow.black.bold( + ' -------------------------------------------------------------------------- ' + ) + ) + + console.log( + chalk.italic.bgYellow.black.bold( + ' ===> Join SOLHINT Community at: https://discord.com/invite/4TYGq3zpjs <=== ' + ) + ) + + console.log( + chalk.italic.bgYellow.black.bold( + ' -------------------------------------------------------------------------- \n' + ) + ) + } + } if (program.opts().save) { writeStringToFile(fullReport) diff --git a/test/fixtures/miscellaneous/duplicated-imports-data.js b/test/fixtures/miscellaneous/duplicated-imports-data.js new file mode 100644 index 00000000..605d438b --- /dev/null +++ b/test/fixtures/miscellaneous/duplicated-imports-data.js @@ -0,0 +1,275 @@ +const { multiLine } = require('../../common/contract-builder') + +const noDuplicates = [ + { + name: 'No Duplicates1', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {Afool} from './Afool.sol';", + "import {Afool2} from './Afool2.sol';", + 'contract Test { }' + ), + }, + { + name: 'No Duplicates2', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {Afool as Bfool} from './Afool.sol';", + 'contract Test { }' + ), + }, + { + name: 'No Duplicates4', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {Afool, Cfool} from './Afool.sol';", + "import {Cfool as Dfool} from './Cfool.sol';", + 'contract Test { }' + ), + }, + { + name: 'No Duplicates6', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import './SimpleLibrary.sol';", + "import {SimpleLibrary as lib} from './SimpleLibrary2.sol';", + 'contract Test { }' + ), + }, + { + name: 'No Duplicates7', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {SimpleLibrary} from './SimpleLibrary.sol';", + 'contract Test { }' + ), + }, + { + name: 'No Duplicates8', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {LibraryA} from './LibraryA.sol';", + "import {LibraryB} from './LibraryB.sol';", + 'contract Test { }' + ), + }, +] + +const duplicates = [ + { + name: 'Duplicates1', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {Afool, Afool as Bfool} from './Afool.sol';", + 'contract Test { }' + ), + qtyDuplicates: 1, + message: ['(inline) Afool'], // Inline duplication + }, + { + name: 'Duplicates2', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {Afool} from './Afool.sol';", + "import {Afool} from './Afool.sol';", + 'contract Test { }' + ), + qtyDuplicates: 1, + message: ['(globalSamePath) Afool'], // Global duplication within the same path + }, + { + name: 'Duplicates3', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {Afool} from './Afool.sol';", + "import {Afool as FoolAlias} from './Afool.sol';", + 'contract Test { }' + ), + qtyDuplicates: 1, + message: ['(globalSamePath) Afool'], // Global duplication with alias + }, + { + name: 'Duplicates4', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import './SimpleLibrary.sol';", + "import {SimpleLibrary} from './folder/SimpleLibrary.sol';", + 'contract Test { }' + ), + qtyDuplicates: 1, + message: ['(globalDiffPath) SimpleLibrary'], // Import with and without name specification + }, + { + name: 'Duplicates5', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {IXTokenFactory} from '../../token/interfaces/IXTokenFactory.sol';", + "import {IXTokenFactory} from '../../token/interfaces/IXTokenFactory2.sol';", + 'contract Test { }' + ), + qtyDuplicates: 1, + message: ['(globalDiffPath) IXTokenFactory'], // Global duplication + }, + { + name: 'Duplicates6', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {IXTokenFactory} from '../../token/interfaces/IXTokenFactory.sol';", + "import {IXTokenFactory, IXTokenFactory as AliasFactory} from '../../token/interfaces/IXTokenFactory.sol';", + 'contract Test { }' + ), + qtyDuplicates: 2, + message: ['(inline) IXTokenFactory', '(globalSamePath) IXTokenFactory'], // Mixed inline and global duplication + }, + { + name: 'Duplicates7', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {SharedLib} from './LibraryA.sol';", + "import {SharedLib} from './LibraryB.sol';", + 'contract Test { }' + ), + qtyDuplicates: 1, + message: ['(globalDiffPath) SharedLib'], // Same object name from different libraries + }, + { + name: 'Duplicates8', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {Token, Token as Tkn} from './LibraryA.sol';", + "import {Token} from './LibraryB.sol';", + 'contract Test { }' + ), + qtyDuplicates: 2, + message: ['(inline) Token', '(globalDiffPath) Token'], // Mixed inline and global duplication + }, + { + name: 'Duplicates9', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import './LibraryA.sol';", + "import {LibraryA} from './LibraryB.sol';", + 'contract Test { }' + ), + qtyDuplicates: 1, + message: ['(globalDiffPath) LibraryA'], // Import with and without name specification different libraries + }, + { + name: 'Duplicates10', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import './LibraryA.sol';", + "import {LibraryA} from './LibraryA.sol';", + 'contract Test { }' + ), + qtyDuplicates: 1, + message: ['(globalSamePath) LibraryA'], // Import with and without name specification same libraries + }, + { + name: 'Duplicates11', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {SharedLib} from '../LibraryA.sol';", + "import {SharedLib} from './nested/LibraryA.sol';", + 'contract Test { }' + ), + qtyDuplicates: 1, + message: ['(globalDiffPath) SharedLib'], // Same object imported from different nested paths + }, + { + name: 'Duplicates12', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import '@openzeppelin/contracts/token/ERC20/ERC20.sol';", + "import {ERC20} from '@openzeppelin/contracts/token/ERC20/ERC20.sol';", + 'contract Test { }' + ), + qtyDuplicates: 1, + message: ['(globalSamePath) ERC20'], // Standard library duplicate + }, + { + name: 'Duplicates13', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import '../token/interfaces/IXTokenWrapper.sol';", + "import {IXTokenWrapper} from '../token/interfaces/IXTokenWrapper.sol';", + 'contract Test { }' + ), + qtyDuplicates: 1, + message: ['(globalSamePath) IXTokenWrapper'], // Import with and without name specification + }, + { + name: 'Duplicates14', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {ReentrancyGuardUpgradeable} from '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol';", + "import {ReentrancyGuardUpgradeable as Rguard} from '@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol';", + 'contract Test { }' + ), + qtyDuplicates: 1, + message: ['(globalSamePath) ReentrancyGuardUpgradeable'], // Reentrancy guard duplication with alias + }, + { + name: 'Duplicates15', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import './LibraryA.sol';", + "import './LibraryA.sol';", + 'contract Test { }' + ), + qtyDuplicates: 1, + message: ['(globalSamePath) LibraryA'], // Same path imported multiple times + }, + { + name: 'Duplicates16', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {Afool, Afool as Fool1, Afool as Fool2, Afool as Fool3} from './Afool.sol';", + "import {Afool} from './Afool.sol';", + 'contract Test { }' + ), + qtyDuplicates: 2, + message: ['(inline) Afool', '(globalSamePath) Afool'], // Global duplication within the same path + }, + { + name: 'Duplicates17', + code: multiLine( + '// SPDX-License-Identifier: Apache-2.0', + 'pragma solidity ^0.8.0;', + "import {IXTokenFactory} from '../../token/interfaces/IXTokenFactory2.sol';", + "import {IXTokenFactory, IXTokenFactory as AliasFactory} from '../../token/interfaces/IXTokenFactory.sol';", + "import '../../token/interfaces/IXTokenFactory.sol';", + 'contract Test { }' + ), + qtyDuplicates: 3, + message: [ + '(inline) IXTokenFactory', + '(globalSamePath) IXTokenFactory', + '(globalDiffPath) IXTokenFactory', + ], // Mixed inline and global duplication + }, +] + +module.exports = { noDuplicates, duplicates } diff --git a/test/fixtures/security/reentrancy-invulenarble.js b/test/fixtures/security/reentrancy-invulnerable.js similarity index 100% rename from test/fixtures/security/reentrancy-invulenarble.js rename to test/fixtures/security/reentrancy-invulnerable.js diff --git a/test/fixtures/security/reentrancy-vulenarble.js b/test/fixtures/security/reentrancy-vulnerable.js similarity index 100% rename from test/fixtures/security/reentrancy-vulenarble.js rename to test/fixtures/security/reentrancy-vulnerable.js diff --git a/test/rules/best-practices/no-global-import.js b/test/rules/best-practices/no-global-import.js index 7a357e18..c163085d 100644 --- a/test/rules/best-practices/no-global-import.js +++ b/test/rules/best-practices/no-global-import.js @@ -28,7 +28,7 @@ describe('Linter - no-global-import', () => { assertErrorMessage(report, 'Specify names to import individually') }) it('should raise warning when using solhint:recommended', () => { - const code = `pragma solidity ^0.8.0; import "./A.sol";` + const code = `pragma solidity ^0.8.24; import "./A.sol";` const report = linter.processStr(code, { extends: 'solhint:recommended', diff --git a/test/rules/best-practices/no-unused-import.js b/test/rules/best-practices/no-unused-import.js index f3666891..32c18cef 100644 --- a/test/rules/best-practices/no-unused-import.js +++ b/test/rules/best-practices/no-unused-import.js @@ -48,7 +48,7 @@ describe('Linter - no-unused-import', () => { }) it('should raise error when using solhint:recommended', () => { - const code = `pragma solidity ^0.8.0; import {A} from "./A.sol";` + const code = `pragma solidity ^0.8.24; import {A} from "./A.sol";` const report = linter.processStr(code, { extends: 'solhint:recommended', diff --git a/test/rules/gas-consumption/gas-custom-errors.js b/test/rules/gas-consumption/gas-custom-errors.js index e2fd0281..bbcde4c4 100644 --- a/test/rules/gas-consumption/gas-custom-errors.js +++ b/test/rules/gas-consumption/gas-custom-errors.js @@ -17,6 +17,39 @@ function replaceSolidityVersion(code, newVersion) { } describe('Linter - gas-custom-errors', () => { + it('should raise error for require with a non-function-call second argument', () => { + // second arg is a numeric literal instead of a function call + const code = ` + pragma solidity 0.8.5; + contract A { + function test() external { + require(msg.sender != address(0), 123); + } + } + ` + const report = linter.processStr(code, { + rules: { 'gas-custom-errors': 'error' }, + }) + assertErrorCount(report, 1) + assertErrorMessage(report, 'Use Custom Errors instead of require statements') + }) + + it('should raise error for require with no second argument', () => { + const code = ` + pragma solidity 0.8.5; + contract A { + function test() external { + require(msg.sender != address(0)); + } + } + ` + const report = linter.processStr(code, { + rules: { 'gas-custom-errors': 'error' }, + }) + assertErrorCount(report, 1) + assertErrorMessage(report, 'Use Custom Errors instead of require statements') + }) + it('should NOT raise error for require with comparison and custom error()', () => { let code = funcWith(`require(a > b, CustomErrorEmitted());`) code = replaceSolidityVersion(code, '^0.8.4') 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) + // }) }) diff --git a/test/rules/miscellaneous/duplicated-imports.js b/test/rules/miscellaneous/duplicated-imports.js new file mode 100644 index 00000000..ca8c9c93 --- /dev/null +++ b/test/rules/miscellaneous/duplicated-imports.js @@ -0,0 +1,35 @@ +const assert = require('assert') +const linter = require('../../../lib/index') +const { assertNoErrors, assertErrorCount } = require('../../common/asserts') +const TEST_CASES = require('../../fixtures/miscellaneous/duplicated-imports-data') + +describe('Linter - duplicated-imports', () => { + for (const importCase of TEST_CASES.duplicates) { + it(`Should report ${importCase.qtyDuplicates} error/s for ${importCase.name}`, () => { + const code = importCase.code + + const report = linter.processStr(code, { + rules: { 'duplicated-imports': 'error' }, + }) + + // assert.equal(report.errorCount, ) + assertErrorCount(report, importCase.qtyDuplicates) + + for (let i = 0; i < report.reports.length; i++) { + assert.ok(report.reports[i].message.includes(importCase.message[i])) + } + }) + } + + for (const importCase of TEST_CASES.noDuplicates) { + it(`Should NOT report errors for ${importCase.name}`, () => { + const code = importCase.code + + const report = linter.processStr(code, { + rules: { 'duplicated-imports': 'error' }, + }) + + assertNoErrors(report) + }) + } +}) diff --git a/test/rules/security/compiler-version.js b/test/rules/security/compiler-version.js index b3c51feb..62578565 100644 --- a/test/rules/security/compiler-version.js +++ b/test/rules/security/compiler-version.js @@ -2,7 +2,7 @@ const assert = require('assert') const { assertNoErrors, assertErrorCount, assertErrorMessage } = require('../../common/asserts') const linter = require('../../../lib/index') -const DEFAULT_SEMVER = '^0.8.0' +const DEFAULT_SEMVER = '^0.8.24' describe('Linter - compiler-version', () => { it('should disable only one compiler error on next line', () => { @@ -123,15 +123,15 @@ describe('Linter - compiler-version', () => { }) it('should not report compiler version error on exact match', () => { - const report = linter.processStr('pragma solidity 0.8.0;', { - rules: { 'compiler-version': ['error', '0.8.0'] }, + const report = linter.processStr('pragma solidity 0.8.24;', { + rules: { 'compiler-version': ['error', '0.8.24'] }, }) assert.equal(report.errorCount, 0) }) it('should not report compiler version error on range match', () => { - const report = linter.processStr('pragma solidity ^0.8.0;', { + const report = linter.processStr('pragma solidity ^0.8.24;', { rules: { 'compiler-version': ['error', DEFAULT_SEMVER] }, }) @@ -139,7 +139,7 @@ describe('Linter - compiler-version', () => { }) it('should not report compiler version error on patch bump', () => { - const report = linter.processStr('pragma solidity 0.8.1;', { + const report = linter.processStr('pragma solidity 0.8.25;', { rules: { 'compiler-version': ['error', DEFAULT_SEMVER] }, }) @@ -147,7 +147,7 @@ describe('Linter - compiler-version', () => { }) it('should not report compiler version error on range match', () => { - const report = linter.processStr('pragma solidity ^0.8.2;', { + const report = linter.processStr('pragma solidity ^0.8.25;', { rules: { 'compiler-version': ['error', DEFAULT_SEMVER] }, }) @@ -207,7 +207,7 @@ describe('Linter - compiler-version', () => { it(`should not report compiler version error using default and correct pragma`, () => { const report = linter.processStr( - `pragma solidity ^0.8.1; + `pragma solidity ^0.8.25; pragma experimental ABIEncoderV2; contract Main { @@ -237,7 +237,7 @@ describe('Linter - compiler-version', () => { it(`should not report compiler version error using >= and default and correct pragma`, () => { const report = linter.processStr( - `pragma solidity >=0.8.0; + `pragma solidity >=0.8.24; contract Main { }`, diff --git a/test/rules/security/reentrancy.js b/test/rules/security/reentrancy.js index f807f49f..2b5182b6 100644 --- a/test/rules/security/reentrancy.js +++ b/test/rules/security/reentrancy.js @@ -2,7 +2,7 @@ const linter = require('../../../lib/index') const { assertWarnsCount, assertErrorMessage, assertNoWarnings } = require('../../common/asserts') describe('Linter - reentrancy', () => { - require('../../fixtures/security/reentrancy-vulenarble').forEach((curCode) => + require('../../fixtures/security/reentrancy-vulnerable').forEach((curCode) => it('should return warn when code contains possible reentrancy', () => { const report = linter.processStr(curCode, { rules: { reentrancy: 'warn' }, @@ -13,7 +13,7 @@ describe('Linter - reentrancy', () => { }) ) - require('../../fixtures/security/reentrancy-invulenarble').forEach((curCode) => + require('../../fixtures/security/reentrancy-invulnerable').forEach((curCode) => it('should not return warn when code do not contains transfer', () => { const report = linter.processStr(curCode, { rules: { reentrancy: 'warn' },