Merge pull request #563 from protofire/develop

Merge into master: v4.5.0
protofire · Mar 15, 2024 · 37f5082 · 37f5082
2 parents 2738047 + 6ae3f4b
commit 37f5082
Show file tree

Hide file tree

Showing 71 changed files with 4,874 additions and 1,139 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,10 +1,54 @@
+
+## [4.5.0] - 2024-03-15
+
+### Updated
+- Update dependencies in package json [#548](https://github.com/protofire/solhint/pull/548)
+- Custom errors rules checks from 0.8.4 forward before warning [#555](https://github.com/protofire/solhint/pull/555)
+- Parser support up to Solidity 0.8.22 (*)
+
+(*) Note: Solhint can work with latest Solidity versions. 
+          If new grammar/syntax is added or changed, it could give some false positives or miss a rule. 
+          But overall functionality will work as expected.
+
+
+### Added
+- New Rule: Interface starts with `i` [#557](https://github.com/protofire/solhint/pull/557)
+
+#### Gas Consumption Rules
+- New Rule: [GC] Mutlitoken1155 rule [#541](https://github.com/protofire/solhint/pull/541)
+- New Rule: [GC] Small strings check [#542](https://github.com/protofire/solhint/pull/542)
+- New Rule: [GC] Indexed events [#543](https://github.com/protofire/solhint/pull/543)
+- New Rule: [GC] Calldata parameters [#544](https://github.com/protofire/solhint/pull/544)
+- New Rule: [GC] Increment by one [#545](https://github.com/protofire/solhint/pull/545)
+- New Rule: [GC] Struct packing [#546](https://github.com/protofire/solhint/pull/546)
+- New Rule: [GC] Name Return Values [#552](https://github.com/protofire/solhint/pull/552)
+- New Rule: [GC] Custom Errors #553 [#555](https://github.com/protofire/solhint/pull/553)
+- New Rule: [GC] Dot Length in Loops [#559](https://github.com/protofire/solhint/pull/559)
+- New Rule: [GC] Gas Strict Inequalities [#560](https://github.com/protofire/solhint/pull/560)
+
+
+### Fixed
+- `explicit-types` logic improved and bug free [#551](https://github.com/protofire/solhint/pull/551)
+- `payable fallback`  Improved behavior [#561](https://github.com/protofire/solhint/pull/561)
+
+
+
+### BREAKING CHANGES
+- `named-return-values` rule was renamed to gas-named-return-values and now it is part of Gas Consumption ruleset [#552](https://github.com/protofire/solhint/pull/552)
+- `custom-errors` rule was renamed to gas-custom-errors and now it is part of Gas Consumption ruleset [#553](https://github.com/protofire/solhint/pull/553)
+- Return error 0 when executed correctly [#554](https://github.com/protofire/solhint/pull/554)
+- Default severity modified to `WARN` instead of `OFF` for 
+  `foundry-test-functions` and `named-parameters-mapping` rules [#556](https://github.com/protofire/solhint/pull/556)
+
+
+
+
 ## [4.1.1] - 2024-01-08
 
 ### Fixed
 - Fix changelog typos
 
 
-
 ## [4.1.0] - 2024-01-08
 
 ### Updated

diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js
@@ -6,9 +6,9 @@
 module.exports = Object.freeze({
   rules: {
     'code-complexity': ['warn', 7],
-    'custom-errors': 'warn',
     'explicit-types': ['warn', 'explicit'],
     'function-max-lines': ['warn', 50],
+    'interface-starts-with-i': 'warn',
     'max-line-length': ['error', 120],
     'max-states-count': ['warn', 15],
     'no-console': 'error',
@@ -25,12 +25,22 @@ module.exports = Object.freeze({
       },
     ],
     'constructor-syntax': 'warn',
+    'gas-calldata-parameters': 'warn',
+    'gas-custom-errors': 'warn',
+    'gas-increment-by-one': 'warn',
+    'gas-indexed-events': 'warn',
+    'gas-length-in-loops': 'warn',
+    'gas-multitoken1155': 'warn',
+    'gas-named-return-values': 'warn',
+    'gas-small-strings': 'warn',
+    'gas-strict-inequalities': 'warn',
+    'gas-struct-packing': 'warn',
     'comprehensive-interface': 'warn',
     quotes: ['error', 'double'],
     'const-name-snakecase': 'warn',
     'contract-name-camelcase': 'warn',
     'event-name-camelcase': 'warn',
-    'foundry-test-functions': ['off', ['setUp']],
+    'foundry-test-functions': ['warn', ['setUp']],
     'func-name-mixedcase': 'warn',
     'func-named-parameters': ['warn', 4],
     'func-param-name-mixedcase': 'warn',
@@ -41,8 +51,7 @@ module.exports = Object.freeze({
       },
     ],
     'modifier-name-mixedcase': 'warn',
-    'named-parameters-mapping': 'off',
-    'named-return-values': 'warn',
+    'named-parameters-mapping': 'warn',
     'private-vars-leading-underscore': [
       'warn',
       {

diff --git a/conf/rulesets/solhint-recommended.js b/conf/rulesets/solhint-recommended.js
@@ -5,7 +5,6 @@
 
 module.exports = Object.freeze({
   rules: {
-    'custom-errors': 'warn',
     'explicit-types': ['warn', 'explicit'],
     'max-states-count': ['warn', 15],
     'no-console': 'error',
@@ -21,6 +20,7 @@ module.exports = Object.freeze({
         maxLength: 32,
       },
     ],
+    'gas-custom-errors': 'warn',
     quotes: ['error', 'double'],
     'const-name-snakecase': 'warn',
     'contract-name-camelcase': 'warn',

diff --git a/docs/rules.md b/docs/rules.md
@@ -9,7 +9,6 @@ title:       "Rule Index of Solhint"
 | Rule Id                                                                  | Error                                                                                                                                      | Recommended  | Deprecated |
 | ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------ | ------------ | ---------- |
 | [code-complexity](./rules/best-practises/code-complexity.md)             | Function has cyclomatic complexity "current" but allowed no more than maxcompl.                                                            |              |            |
-| [custom-errors](./rules/best-practises/custom-errors.md)                 | Enforces the use of Custom Errors over Require and Revert statements                                                                       | $~~~~~~~~$✔️ |            |
 | [explicit-types](./rules/best-practises/explicit-types.md)               | Forbid or enforce explicit types (like uint256) that have an alias (like uint).                                                            | $~~~~~~~~$✔️ |            |
 | [function-max-lines](./rules/best-practises/function-max-lines.md)       | Function body contains "count" lines but allowed no more than maxlines.                                                                    |              |            |
 | [max-line-length](./rules/best-practises/max-line-length.md)             | Line length must be no more than maxlen.                                                                                                   |              |            |
@@ -20,23 +19,16 @@ title:       "Rule Index of Solhint"
 | [no-unused-import](./rules/best-practises/no-unused-import.md)           | Imported object name is not being used by the contract.                                                                                    | $~~~~~~~~$✔️ |            |
 | [no-unused-vars](./rules/best-practises/no-unused-vars.md)               | Variable "name" is unused.                                                                                                                 | $~~~~~~~~$✔️ |            |
 | [one-contract-per-file](./rules/best-practises/one-contract-per-file.md) | Enforces the use of ONE Contract per file see [here](https://docs.soliditylang.org/en/v0.8.21/style-guide.html#contract-and-library-names) | $~~~~~~~~$✔️ |            |
-| [payable-fallback](./rules/best-practises/payable-fallback.md)           | When fallback is not payable you will not be able to receive ethers.                                                                       | $~~~~~~~~$✔️ |            |
+| [payable-fallback](./rules/best-practises/payable-fallback.md)           | When fallback is not payable and there is no receive function you will not be able to receive currency.                                    | $~~~~~~~~$✔️ |            |
 | [reason-string](./rules/best-practises/reason-string.md)                 | Require or revert statement must have a reason string and check that each reason string is at most N characters long.                      | $~~~~~~~~$✔️ |            |
 | [constructor-syntax](./rules/best-practises/constructor-syntax.md)       | Constructors should use the new constructor keyword.                                                                                       |              |            |
         
 
-## Miscellaneous
-
-| Rule Id                                                                     | Error                                                                                                                                  | Recommended  | Deprecated |
-| --------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------- | ------------ | ---------- |
-| [comprehensive-interface](./rules/miscellaneous/comprehensive-interface.md) | Check that all public or external functions are override. This is iseful to make sure that the whole API is extracted in an interface. |              |            |
-| [quotes](./rules/miscellaneous/quotes.md)                                   | Enforces the use of double or simple quotes as configured for string literals. Values must be 'single' or 'double'.                    | $~~~~~~~~$✔️ |            |
-        
-
 ## Style Guide Rules
 
 | Rule Id                                                                              | Error                                                                                                         | Recommended  | Deprecated  |
 | ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------- | ------------ | ----------- |
+| [interface-starts-with-i](./rules/naming/interface-starts-with-i.md)                 | Solidity Interfaces names should start with an `I`                                                            |              |             |
 | [const-name-snakecase](./rules/naming/const-name-snakecase.md)                       | Constant name must be in capitalized SNAKE_CASE. (Does not check IMMUTABLES, use immutable-vars-naming)       | $~~~~~~~~$✔️ |             |
 | [contract-name-camelcase](./rules/naming/contract-name-camelcase.md)                 | Contract, Structs and Enums should be in CamelCase.                                                           | $~~~~~~~~$✔️ |             |
 | [event-name-camelcase](./rules/naming/event-name-camelcase.md)                       | Event name must be in CamelCase.                                                                              | $~~~~~~~~$✔️ |             |
@@ -47,7 +39,6 @@ title:       "Rule Index of Solhint"
 | [immutable-vars-naming](./rules/naming/immutable-vars-naming.md)                     | Check Immutable variables. Capitalized SNAKE_CASE or mixedCase depending on configuration.                    | $~~~~~~~~$✔️ |             |
 | [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.                                      |              |             |
-| [named-return-values](./rules/naming/named-return-values.md)                         | Enforce the return values of a function to be named                                                           |              |             |
 | [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           |              |             |
 | [use-forbidden-name](./rules/naming/use-forbidden-name.md)                           | Avoid to use letters 'I', 'l', 'O' as identifiers.                                                            | $~~~~~~~~$✔️ |             |
 | [var-name-mixedcase](./rules/naming/var-name-mixedcase.md)                           | Variable name must be in mixedCase. (Does not check IMMUTABLES, use immutable-vars-naming)                    | $~~~~~~~~$✔️ |             |
@@ -57,6 +48,30 @@ title:       "Rule Index of Solhint"
 | [visibility-modifier-order](./rules/order/visibility-modifier-order.md)              | Visibility modifier must be first in list of modifiers.                                                       | $~~~~~~~~$✔️ |             |
         
 
+## Gas Consumption Rules
+
+| Rule Id                                                                       | Error                                                                                | Recommended  | Deprecated |
+| ----------------------------------------------------------------------------- | ------------------------------------------------------------------------------------ | ------------ | ---------- |
+| [gas-calldata-parameters](./rules/gas-consumption/gas-calldata-parameters.md) | Suggest calldata keyword on function arguments when read only                        |              |            |
+| [gas-custom-errors](./rules/gas-consumption/gas-custom-errors.md)             | Enforces the use of Custom Errors over Require and Revert statements                 | $~~~~~~~~$✔️ |            |
+| [gas-increment-by-one](./rules/gas-consumption/gas-increment-by-one.md)       | Suggest incrementation by one like this ++i instead of other type                    |              |            |
+| [gas-indexed-events](./rules/gas-consumption/gas-indexed-events.md)           | Suggest indexed arguments on events for uint, bool and address                       |              |            |
+| [gas-length-in-loops](./rules/gas-consumption/gas-length-in-loops.md)         | Suggest replacing object.length in a loop condition to avoid calculation on each lap |              |            |
+| [gas-multitoken1155](./rules/gas-consumption/gas-multitoken1155.md)           | ERC1155 is a cheaper non-fungible token than ERC721                                  |              |            |
+| [gas-named-return-values](./rules/gas-consumption/gas-named-return-values.md) | Enforce the return values of a function to be named                                  |              |            |
+| [gas-small-strings](./rules/gas-consumption/gas-small-strings.md)             | Keep strings smaller than 32 bytes                                                   |              |            |
+| [gas-strict-inequalities](./rules/gas-consumption/gas-strict-inequalities.md) | Suggest Strict Inequalities over non Strict ones                                     |              |            |
+| [gas-struct-packing](./rules/gas-consumption/gas-struct-packing.md)           | Suggest to re-arrange struct packing order when it is inefficient                    |              |            |
+        
+
+## Miscellaneous
+
+| Rule Id                                                                     | Error                                                                                                                                  | Recommended  | Deprecated |
+| --------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------- | ------------ | ---------- |
+| [comprehensive-interface](./rules/miscellaneous/comprehensive-interface.md) | Check that all public or external functions are override. This is iseful to make sure that the whole API is extracted in an interface. |              |            |
+| [quotes](./rules/miscellaneous/quotes.md)                                   | Enforces the use of double or simple quotes as configured for string literals. Values must be 'single' or 'double'.                    | $~~~~~~~~$✔️ |            |
+        
+
 ## Security Rules
 
 | Rule Id                                                                | Error                                                                    | Recommended  | Deprecated  |

diff --git a/docs/rules/best-practises/payable-fallback.md b/docs/rules/best-practises/payable-fallback.md
@@ -12,7 +12,7 @@ title:       "payable-fallback | Solhint"
 
 
 ## Description
-When fallback is not payable you will not be able to receive ethers.
+When fallback is not payable and there is no receive function you will not be able to receive currency.
 
 ## Options
 This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn.
@@ -28,36 +28,35 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 
 ### Notes
 - Solhint allows this rule to automatically fix the code with `--fix` option
+- Instead of having a fallback function to receive native currency it is recommended to code a receive() function [[here]](https://docs.soliditylang.org/en/v0.8.24/contracts.html#fallback-function)
 
 ## Examples
 ### 👍 Examples of **correct** code for this rule
 
 #### Fallback is payable
 
 ```solidity
+function() public payable {}
+```
+
+#### Fallback is payable
 
-      pragma solidity 0.4.4;
-        
-        
-      contract A {
-        function () public payable {}
-      }
-    
+```solidity
+fallback() external payable {}
 ```
 
 ### 👎 Examples of **incorrect** code for this rule
 
 #### Fallback is not payable
 
 ```solidity
+function() {} function g() payable {}
+```
+
+#### Fallback is not payable
 
-      pragma solidity 0.4.4;
-        
-        
-      contract A {
-        function () public {}
-      }
-    
+```solidity
+fallback() {} function g() payable {}
 ```
 
 ## Version

diff --git a/docs/rules/gas-consumption/gas-calldata-parameters.md b/docs/rules/gas-consumption/gas-calldata-parameters.md
@@ -0,0 +1,41 @@
+---
+warning:     "This is a dynamically generated file. Do not edit manually."
+layout:      "default"
+title:       "gas-calldata-parameters | Solhint"
+---
+
+# gas-calldata-parameters
+![Category Badge](https://img.shields.io/badge/-Gas%20Consumption%20Rules-informational)
+![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow)
+
+## Description
+Suggest calldata keyword on function arguments when read only
+
+## Options
+This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn.
+
+### Example Config
+```json
+{
+  "rules": {
+    "gas-calldata-parameters": "warn"
+  }
+}
+```
+
+### Notes
+- Only applies for external functions when receiving arguments with [memory] keyword
+- This rule makes a soft check to see if argument is readOnly to make the suggestion. Check it manually before changing it.
+- [source 1](https://coinsbench.com/comprehensive-guide-tips-and-tricks-for-gas-optimization-in-solidity-5380db734404) of the rule initiative (see Calldata vs Memory)
+- [source 2](https://www.rareskills.io/post/gas-optimization?postId=c9db474a-ff97-4fa3-a51d-fe13ccb8fe3b#viewer-6acr7) of the rule initiative
+
+## 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/gas-consumption/gas-calldata-parameters.js)
+- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/gas-consumption/gas-calldata-parameters.md)
+- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/gas-consumption/gas-calldata-parameters.js)