Merge branch 'develop' into non-state-vars-leading-underscore

protofire · Mar 11, 2024 · 22633ca · 22633ca
2 parents 8deb451 + 10aa9f8
commit 22633ca
Show file tree

Hide file tree

Showing 22 changed files with 623 additions and 67 deletions.
diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js
@@ -8,6 +8,7 @@ module.exports = Object.freeze({
     'code-complexity': ['warn', 7],
     'explicit-types': ['warn', 'explicit'],
     'function-max-lines': ['warn', 50],
+    'interface-starts-with-i': 'warning',
     'max-line-length': ['error', 120],
     'max-states-count': ['warn', 15],
     'no-console': 'error',
@@ -28,16 +29,18 @@ module.exports = Object.freeze({
     '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',

diff --git a/docs/rules.md b/docs/rules.md
@@ -25,32 +25,11 @@ title:       "Rule Index of Solhint"
 | [non-state-vars-leading-underscore](./rules/naming/non-state-vars-leading-underscore.md) | Variables that are not in contract state should start with underscore. Conversely, variables that can cause an SLOAD/SSTORE should NOT start with an underscore. This makes it evident which operations cause expensive storage access when hunting for gas optimizations |              |            |
         
 
-## 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-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-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'.                    | $~~~~~~~~$✔️ |            |
-        
-
 ## 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.                                                                              | $~~~~~~~~$✔️ |             |
@@ -70,6 +49,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-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    |              |            |
+| [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 |             |            |
+        
+
+## 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/gas-consumption/gas-length-in-loops.md b/docs/rules/gas-consumption/gas-length-in-loops.md
@@ -0,0 +1,38 @@
+---
+warning:     "This is a dynamically generated file. Do not edit manually."
+layout:      "default"
+title:       "gas-length-in-loops | Solhint"
+---
+
+# gas-length-in-loops
+![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 replacing object.length in a loop condition to avoid calculation on each lap
+
+## 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-length-in-loops": "warn"
+  }
+}
+```
+
+### Notes
+- [source 1](https://coinsbench.com/comprehensive-guide-tips-and-tricks-for-gas-optimization-in-solidity-5380db734404) of the rule initiative (see Array Length Caching)
+
+## 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-length-in-loops.js)
+- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/gas-consumption/gas-length-in-loops.md)
+- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/gas-consumption/gas-length-in-loops.js)
diff --git a/docs/rules/gas-consumption/gas-strict-inequalities.md b/docs/rules/gas-consumption/gas-strict-inequalities.md
@@ -0,0 +1,40 @@
+---
+warning:     "This is a dynamically generated file. Do not edit manually."
+layout:      "default"
+title:       "gas-strict-inequalities | Solhint"
+---
+
+# gas-strict-inequalities
+![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 Strict Inequalities over non Strict ones
+
+## 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-strict-inequalities": "warn"
+  }
+}
+```
+
+### Notes
+- Strict inequality does not always saves gas. It is dependent on the context of the surrounding opcodes
+- [source 1](https://coinsbench.com/comprehensive-guide-tips-and-tricks-for-gas-optimization-in-solidity-5380db734404) of the rule initiative (see Less/Greater Than vs Less/Greater Than or Equal To)
+- [source 2](https://www.rareskills.io/post/gas-optimization?postId=c9db474a-ff97-4fa3-a51d-fe13ccb8fe3b#viewer-7b77t) 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-strict-inequalities.js)
+- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/gas-consumption/gas-strict-inequalities.md)
+- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/gas-consumption/gas-strict-inequalities.js)
diff --git a/docs/rules/naming/foundry-test-functions.md b/docs/rules/naming/foundry-test-functions.md
@@ -6,7 +6,7 @@ title:       "foundry-test-functions | Solhint"
 
 # foundry-test-functions
 ![Category Badge](https://img.shields.io/badge/-Style%20Guide%20Rules-informational)
-![Default Severity Badge off](https://img.shields.io/badge/Default%20Severity-off-undefined)
+![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow)
 
 ## Description
 Enforce naming convention on functions for Foundry test cases
@@ -16,14 +16,14 @@ This rule accepts an array of options:
 
 | Index | Description                                           | Default Value |
 | ----- | ----------------------------------------------------- | ------------- |
-| 0     | Rule severity. Must be one of "error", "warn", "off". | off           |
+| 0     | Rule severity. Must be one of "error", "warn", "off". | warn          |
 
 
 ### Example Config
 ```json
 {
   "rules": {
-    "foundry-test-functions": ["off",["setUp"]]
+    "foundry-test-functions": ["warn",["setUp"]]
   }
 }
 ```

diff --git a/docs/rules/naming/interface-starts-with-i.md b/docs/rules/naming/interface-starts-with-i.md
@@ -0,0 +1,50 @@
+---
+warning:     "This is a dynamically generated file. Do not edit manually."
+layout:      "default"
+title:       "interface-starts-with-i | Solhint"
+---
+
+# interface-starts-with-i
+![Category Badge](https://img.shields.io/badge/-Style%20Guide%20Rules-informational)
+![Default Severity Badge warning](https://img.shields.io/badge/Default%20Severity-warning-undefined)
+
+## Description
+Solidity Interfaces names should start with an `I`
+
+## Options
+This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warning.
+
+### Example Config
+```json
+{
+  "rules": {
+    "interface-starts-with-i": "warning"
+  }
+}
+```
+
+
+## Examples
+### 👍 Examples of **correct** code for this rule
+
+#### Interface name starts with I
+
+```solidity
+interface IFoo { function foo () external; }
+```
+
+### 👎 Examples of **incorrect** code for this rule
+
+#### Interface name doesnt start with I
+
+```solidity
+interface Foo { function foo () external; }
+```
+
+## Version
+This rule is introduced in the latest version.
+
+## Resources
+- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/best-practises/interface-starts-with-i.js)
+- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/best-practises/interface-starts-with-i.md)
+- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/best-practises/interface-starts-with-i.js)
diff --git a/docs/rules/naming/named-parameters-mapping.md b/docs/rules/naming/named-parameters-mapping.md
@@ -6,19 +6,19 @@ title:       "named-parameters-mapping | Solhint"
 
 # named-parameters-mapping
 ![Category Badge](https://img.shields.io/badge/-Style%20Guide%20Rules-informational)
-![Default Severity Badge off](https://img.shields.io/badge/Default%20Severity-off-undefined)
+![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow)
 
 ## Description
 Solidity v0.8.18 introduced named parameters on the mappings definition.
 
 ## Options
-This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to off.
+This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn.
 
 ### Example Config
 ```json
 {
   "rules": {
-    "named-parameters-mapping": "off"
+    "named-parameters-mapping": "warn"
   }
 }
 ```

diff --git a/e2e/autofix-test.js b/e2e/autofix-test.js
@@ -103,7 +103,7 @@ describe('e2e', function () {
             }
           )
 
-          expect(solhintProcess.status).to.equal(1)
+          expect(solhintProcess.status).to.equal(0)
           expect(solhintProcess.stdout.toString().includes('5 problems (5 errors, 0 warnings)'))
         })
       })
@@ -129,7 +129,7 @@ describe('e2e', function () {
             `${params.command} ${params.param1} -c ${currentConfig} ${currentFile} --fix --disc --noPrompt`
           )
 
-          expect(code).to.equal(1)
+          expect(code).to.equal(0)
 
           const reportLines = stdout.split('\n')
           const finalLine = '5 problems (5 errors, 0 warnings)'
@@ -175,8 +175,8 @@ describe('e2e', function () {
           expect(result).to.be.true
         })
 
-        it('should execute and exit with code 1 (2)', () => {
-          expect(code).to.equal(1)
+        it('should execute and exit with code 0 (2)', () => {
+          expect(code).to.equal(0)
         })
 
         it('should get the right report (2)', () => {
@@ -222,7 +222,7 @@ describe('e2e', function () {
         })
 
         it('should execute and exit with code 1 (3)', () => {
-          expect(code).to.equal(1)
+          expect(code).to.equal(0)
         })
 
         it('should get the right report (3)', () => {
@@ -268,7 +268,7 @@ describe('e2e', function () {
         })
 
         it('should execute and exit with code 1 (4)', () => {
-          expect(code).to.equal(1)
+          expect(code).to.equal(0)
         })
 
         it('should get the right report (4)', () => {
@@ -314,7 +314,7 @@ describe('e2e', function () {
         })
 
         it('should execute and exit with code 1 (5)', () => {
-          expect(code).to.equal(1)
+          expect(code).to.equal(0)
         })
 
         it('should get the right report (5)', () => {
@@ -361,7 +361,7 @@ describe('e2e', function () {
         })
 
         it('should execute and exit with code 1 (6)', () => {
-          expect(code).to.equal(1)
+          expect(code).to.equal(0)
         })
 
         it('should get the right report (6)', () => {
@@ -401,7 +401,7 @@ describe('e2e', function () {
         })
 
         it('should execute and exit with code 1 (6)', () => {
-          expect(code).to.equal(1)
+          expect(code).to.equal(0)
         })
 
         it('should get the right report (6)', () => {
@@ -448,7 +448,7 @@ describe('e2e', function () {
       })
 
       it('should execute and exit with code 1 (7)', () => {
-        expect(code).to.equal(1)
+        expect(code).to.equal(0)
       })
 
       it('should get the right report (7)', () => {
@@ -494,7 +494,7 @@ describe('e2e', function () {
       })
 
       it('should execute and exit with code 1 (8)', () => {
-        expect(code).to.equal(1)
+        expect(code).to.equal(0)
       })
 
       it('should get the right report (8)', () => {
@@ -540,7 +540,7 @@ describe('e2e', function () {
       })
 
       it('should execute and exit with code 1 (8)', () => {
-        expect(code).to.equal(1)
+        expect(code).to.equal(0)
       })
 
       it('should get the right report (8)', () => {