Merge pull request #473 from protofire/develop

MERGE Develop into Master
protofire · Aug 11, 2023 · 48258ca · 48258ca
2 parents 3444491 + b67d523
commit 48258ca
Show file tree

Hide file tree

Showing 55 changed files with 1,678 additions and 401 deletions.
diff --git a/.gitignore b/.gitignore
@@ -9,4 +9,4 @@ convertLib.sol
 antlr4.jar
 /docs/.sass-cache/
 _temp/
-.solhint.json
+
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,43 @@
+## [3.6.1] - 2023-08-11
+
+### BREAKING CHANGE
+- RULE: `not-rely-on-time` was REMOVED from RECOMMENDED ruleset<br>
+This was long overdue.<br> 
+Beware!! If you are relying on this rule and it is not explicitly configured (meaning there's only `solhint:recommended` option).<br>
+You should add this rule manually:
+```json
+  {
+    "extends": "solhint:recommended",
+    "rules": { 
+        "not-rely-on-time": "warn", 
+        "compiler-version": "off" 
+    },
+  }
+```
+If not explicitly added, this rule will not be executed.
+
+### SPECIAL ATTENTION
+- RULE: `compiler-version` default was updated from ^0.5.2 to ^0.8.0
+
+### Added
+- New Rule: Enforces the use of Custom Errors over Require and Revert statements [#475](https://github.com/protofire/solhint/pull/475)
+- New Rule: Enforces the test_ prefix on a file for Foundry users [#476](https://github.com/protofire/solhint/pull/476)
+- New Rule: Enforces the naming of function return values [#478](https://github.com/protofire/solhint/pull/478)
+- `Notes` option on docs to add more information of each rule. See `foundry-test-functions`. [#476](https://github.com/protofire/solhint/pull/476)
+
+### Fixed
+- `func-named-parameters` - false positives on builtin functions [#472](https://github.com/protofire/solhint/pull/472)
+- `ordering` - treat initializer weight same as constructor [#474](https://github.com/protofire/solhint/pull/474)
+- `check-send-result` - false positive on `erc777.send()`` function [#477](https://github.com/protofire/solhint/pull/477)
+- `explicit-types` - default value is now taking into account when no value is specified in config [#481](https://github.com/protofire/solhint/pull/481)
+- `compiler-version` - default value is now taking into account when no value is specified in config [#483](https://github.com/protofire/solhint/pull/483)
+
+### Updates
+- Rule: `check-send-result` added config clarification in the new `Notes` section [#482](https://github.com/protofire/solhint/pull/482)
+- Rule: `compiler-version` default was updated from ^0.5.2 to ^0.8.0 [#483](https://github.com/protofire/solhint/pull/483)
+
+
+
 ## [3.5.1] - 2023-08-04
 ### Updated
 - Support `ignoreConstructors` option for `no-empty-blocks` [#418](https://github.com/protofire/solhint/pull/418)
@@ -30,7 +70,6 @@
 
 
 
-<br><br>
 ## [3.4.1] - 2023-03-06
 ### Updated
 - Updated solidity parser to 0.16.0 [#420](https://github.com/protofire/solhint/pull/420)
@@ -46,10 +85,6 @@
 - Fix named-parameters-mapping to not enforce on nested mappings [#421](https://github.com/protofire/solhint/pull/421)
 
 
-
-
-
-<br><br>
 ## [3.4.0] - 2023-02-17
 ### Updated
 - Solhint dependencies to support newer versions [#380](https://github.com/protofire/solhint/pull/380)
@@ -73,10 +108,6 @@
 - False positive on no-unused-vars for payable arguments without name [#399](https://github.com/protofire/solhint/pull/399)
 
 
-
-
-
-<br><br>
 ## [3.3.8] - 2023-01-17
 ### Fixed Docs and Typos
 - [#292](https://github.com/protofire/solhint/pull/292)

diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js
@@ -6,6 +6,7 @@
 module.exports = Object.freeze({
   rules: {
     'code-complexity': ['warn', 7],
+    'custom-errors': 'warn',
     'explicit-types': ['warn', 'explicit'],
     'function-max-lines': ['warn', 50],
     'max-line-length': ['error', 120],
@@ -28,8 +29,9 @@ module.exports = Object.freeze({
     'const-name-snakecase': 'warn',
     'contract-name-camelcase': 'warn',
     'event-name-camelcase': 'warn',
+    'foundry-test-functions': ['off', ['setUp']],
     'func-name-mixedcase': 'warn',
-    'func-named-parameters': ['warn', 2],
+    'func-named-parameters': ['warn', 4],
     'func-param-name-mixedcase': 'warn',
     'immutable-vars-naming': [
       'warn',
@@ -39,6 +41,7 @@ module.exports = Object.freeze({
     ],
     'modifier-name-mixedcase': 'warn',
     'named-parameters-mapping': 'off',
+    'named-return-values': 'warn',
     'private-vars-leading-underscore': [
       'warn',
       {
@@ -57,7 +60,7 @@ module.exports = Object.freeze({
     'avoid-throw': 'warn',
     'avoid-tx-origin': 'warn',
     'check-send-result': 'warn',
-    'compiler-version': ['error', '^0.5.8'],
+    'compiler-version': ['error', '^0.8.0'],
     'func-visibility': [
       'warn',
       {

diff --git a/conf/rulesets/solhint-recommended.js b/conf/rulesets/solhint-recommended.js
@@ -5,6 +5,7 @@
 
 module.exports = Object.freeze({
   rules: {
+    'custom-errors': 'warn',
     'explicit-types': ['warn', 'explicit'],
     'max-states-count': ['warn', 15],
     'no-console': 'error',
@@ -24,7 +25,6 @@ module.exports = Object.freeze({
     'contract-name-camelcase': 'warn',
     'event-name-camelcase': 'warn',
     'func-name-mixedcase': 'warn',
-    'func-named-parameters': ['warn', 2],
     'immutable-vars-naming': [
       'warn',
       {
@@ -42,7 +42,7 @@ module.exports = Object.freeze({
     'avoid-throw': 'warn',
     'avoid-tx-origin': 'warn',
     'check-send-result': 'warn',
-    'compiler-version': ['error', '^0.5.8'],
+    'compiler-version': ['error', '^0.8.0'],
     'func-visibility': [
       'warn',
       {
@@ -53,7 +53,6 @@ module.exports = Object.freeze({
     'no-complex-fallback': 'warn',
     'no-inline-assembly': 'warn',
     'not-rely-on-block-hash': 'warn',
-    'not-rely-on-time': 'warn',
     reentrancy: 'warn',
     'state-visibility': 'warn',
   },

diff --git a/docs/rules.md b/docs/rules.md
@@ -9,6 +9,7 @@ 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.                                                                              |              |            |
@@ -33,24 +34,26 @@ title:       "Rule Index of Solhint"
 
 ## Style Guide Rules
 
-| Rule Id                                                                              | Error                                                                                                   | Recommended  | Deprecated  |
-| ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------- | ------------ | ----------- |
-| [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 name must be in CamelCase.                                                                     | $~~~~~~~~$✔️ |             |
-| [event-name-camelcase](./rules/naming/event-name-camelcase.md)                       | Event name must be in CamelCase.                                                                        | $~~~~~~~~$✔️ |             |
-| [func-name-mixedcase](./rules/naming/func-name-mixedcase.md)                         | Function name must be in mixedCase.                                                                     | $~~~~~~~~$✔️ |             |
-| [func-named-parameters](./rules/naming/func-named-parameters.md)                     | Enforce function calls with Named Parameters when containing more than the configured qty               | $~~~~~~~~$✔️ |             |
-| [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.              | $~~~~~~~~$✔️ |             |
-| [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) | Private and internal names must start with a single underscore.                                         |              |             |
-| [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)              | $~~~~~~~~$✔️ |             |
-| [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.                                                                       | $~~~~~~~~$✔️ |             |
-| [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.                                                 | $~~~~~~~~$✔️ |             |
+| Rule Id                                                                              | Error                                                                                                         | Recommended  | Deprecated  |
+| ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------- | ------------ | ----------- |
+| [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 name must be in CamelCase.                                                                           | $~~~~~~~~$✔️ |             |
+| [event-name-camelcase](./rules/naming/event-name-camelcase.md)                       | Event name must be in CamelCase.                                                                              | $~~~~~~~~$✔️ |             |
+| [foundry-test-functions](./rules/naming/foundry-test-functions.md)                   | Enforce naming convention on functions for Foundry test cases                                                 |              |             |
+| [func-name-mixedcase](./rules/naming/func-name-mixedcase.md)                         | Function name must be in mixedCase.                                                                           | $~~~~~~~~$✔️ |             |
+| [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.                    | $~~~~~~~~$✔️ |             |
+| [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) | Private and internal names must start with a single underscore.                                               |              |             |
+| [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)                    | $~~~~~~~~$✔️ |             |
+| [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.                                                                             | $~~~~~~~~$✔️ |             |
+| [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.                                                       | $~~~~~~~~$✔️ |             |
         
 
 ## Security Rules
@@ -71,7 +74,7 @@ title:       "Rule Index of Solhint"
 | [no-complex-fallback](./rules/security/no-complex-fallback.md)         | Fallback function must be simple.                                        | $~~~~~~~~$✔️ |             |
 | [no-inline-assembly](./rules/security/no-inline-assembly.md)           | Avoid to use inline assembly. It is acceptable only in rare cases.       | $~~~~~~~~$✔️ |             |
 | [not-rely-on-block-hash](./rules/security/not-rely-on-block-hash.md)   | Do not rely on "block.blockhash". Miners can influence its value.        | $~~~~~~~~$✔️ |             |
-| [not-rely-on-time](./rules/security/not-rely-on-time.md)               | Avoid making time-based decisions in your business logic.                | $~~~~~~~~$✔️ |             |
+| [not-rely-on-time](./rules/security/not-rely-on-time.md)               | Avoid making time-based decisions in your business logic.                |              |             |
 | [reentrancy](./rules/security/reentrancy.md)                           | Possible reentrancy vulnerabilities. Avoid state changes after transfer. | $~~~~~~~~$✔️ |             |
 | [state-visibility](./rules/security/state-visibility.md)               | Explicitly mark visibility of state.                                     | $~~~~~~~~$✔️ |             |
         

diff --git a/docs/rules/best-practises/custom-errors.md b/docs/rules/best-practises/custom-errors.md
@@ -0,0 +1,71 @@
+---
+warning:     "This is a dynamically generated file. Do not edit manually."
+layout:      "default"
+title:       "custom-errors | Solhint"
+---
+
+# custom-errors
+![Recommended Badge](https://img.shields.io/badge/-Recommended-brightgreen)
+![Category Badge](https://img.shields.io/badge/-Best%20Practise%20Rules-informational)
+![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow)
+> The {"extends": "solhint:recommended"} property in a configuration file enables this rule.
+
+
+## Description
+Enforces the use of Custom Errors over Require and Revert statements
+
+## Options
+This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn.
+
+### Example Config
+```json
+{
+  "rules": {
+    "custom-errors": "warn"
+  }
+}
+```
+
+
+## Examples
+### 👍 Examples of **correct** code for this rule
+
+#### Use of Custom Errors
+
+```solidity
+revert CustomErrorFunction();
+```
+
+#### Use of Custom Errors with arguments
+
+```solidity
+revert CustomErrorFunction({ msg: "Insufficient Balance" });
+```
+
+### 👎 Examples of **incorrect** code for this rule
+
+#### Use of require statement
+
+```solidity
+require(userBalance >= availableAmount, "Insufficient Balance");
+```
+
+#### Use of plain revert statement
+
+```solidity
+revert();
+```
+
+#### Use of revert statement with message
+
+```solidity
+revert("Insufficient Balance");
+```
+
+## Version
+This rule is introduced in the latest version.
+
+## Resources
+- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/best-practises/custom-errors.js)
+- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/best-practises/custom-errors.md)
+- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/best-practises/custom-errors.js)
diff --git a/docs/rules/best-practises/explicit-types.md b/docs/rules/best-practises/explicit-types.md
@@ -63,7 +63,7 @@ uint256 public variableName
 ```
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 3.5.1](https://github.com/protofire/solhint/tree/v3.5.1)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/best-practises/explicit-types.js)

diff --git a/docs/rules/best-practises/no-unused-import.md b/docs/rules/best-practises/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 3.5.1](https://github.com/protofire/solhint/tree/v3.5.1)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/best-practises/no-unused-import.js)