Merge pull request #560 from protofire/gc-strict-inequalities

dbale-altoros · web-flow · commit 53ab05e0fae1 · 2024-03-11T12:54:12.000-03:00
GC: Gas Strict Inequalities
diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js
@@ -31,13 +31,14 @@ module.exports = Object.freeze({
     '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',
@@ -48,7 +49,7 @@ module.exports = Object.freeze({
       },
     ],
     'modifier-name-mixedcase': 'warn',
-    'named-parameters-mapping': 'off',
+    'named-parameters-mapping': 'warn',
     'private-vars-leading-underscore': [
       'warn',
       {
diff --git a/docs/rules.md b/docs/rules.md
@@ -35,6 +35,7 @@ title:       "Rule Index of Solhint"
 | [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    |              |            |
         
 
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/lib/rules/gas-consumption/gas-strict-inequalities.js b/lib/rules/gas-consumption/gas-strict-inequalities.js
@@ -0,0 +1,46 @@
+const BaseChecker = require('../base-checker')
+
+const ruleId = 'gas-strict-inequalities'
+const meta = {
+  type: 'gas-consumption',
+
+  docs: {
+    description: 'Suggest Strict Inequalities over non Strict ones',
+    category: 'Gas Consumption Rules',
+    notes: [
+      {
+        note: 'Strict inequality does not always saves gas. It is dependent on the context of the surrounding opcodes',
+      },
+      {
+        note: '[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)',
+      },
+      {
+        note: '[source 2](https://www.rareskills.io/post/gas-optimization?postId=c9db474a-ff97-4fa3-a51d-fe13ccb8fe3b#viewer-7b77t) of the rule initiative',
+      },
+    ],
+  },
+
+  isDefault: false,
+  recommended: false,
+  defaultSetup: 'warn',
+
+  schema: null,
+}
+
+class GasStrictInequalities extends BaseChecker {
+  constructor(reporter) {
+    super(reporter, ruleId, meta)
+  }
+
+  BinaryOperation(node) {
+    if (node.operator === '>=' || node.operator === '<=') {
+      this.reportError(node)
+    }
+  }
+
+  reportError(node) {
+    this.error(node, `GC: Non strict inequality found. Try converting to a strict one`)
+  }
+}
+
+module.exports = GasStrictInequalities
diff --git a/lib/rules/gas-consumption/index.js b/lib/rules/gas-consumption/index.js
@@ -4,6 +4,7 @@ const GasIndexedEvents = require('./gas-indexed-events')
 const GasCalldataParameters = require('./gas-calldata-parameters')
 const GasIncrementByOne = require('./gas-increment-by-one')
 const GasStructPacking = require('./gas-struct-packing')
+const GasStrictInequalities = require('./gas-strict-inequalities')
 const GasNamedReturnValuesChecker = require('./gas-named-return-values')
 const GasCustomErrorsChecker = require('./gas-custom-errors')
 
@@ -15,6 +16,7 @@ module.exports = function checkers(reporter, config) {
     new GasCalldataParameters(reporter, config),
     new GasIncrementByOne(reporter, config),
     new GasStructPacking(reporter, config),
+    new GasStrictInequalities(reporter, config),
     new GasNamedReturnValuesChecker(reporter),
     new GasCustomErrorsChecker(reporter),
   ]
diff --git a/test/rules/gas-consumption/gas-named-return-values.js b/test/rules/gas-consumption/gas-named-return-values.js
@@ -84,7 +84,12 @@ describe('Linter - gas-named-return-values', () => {
 
     const report = linter.processStr(code, {
       extends: 'solhint:all',
-      rules: { 'compiler-version': 'off' },
+      rules: {
+        'compiler-version': 'off',
+        'comprehensive-interface': 'off',
+        'foundry-test-functions': 'off',
+        'non-state-vars-leading-underscore': 'off',
+      },
     })
 
     assertWarnsCount(report, 2)
diff --git a/test/rules/gas-consumption/gas-strict-inequalities.js b/test/rules/gas-consumption/gas-strict-inequalities.js
@@ -0,0 +1,94 @@
+const assert = require('assert')
+const linter = require('../../../lib/index')
+const { funcWith } = require('../../common/contract-builder')
+
+const ERROR_MSG = 'GC: Non strict inequality found. Try converting to a strict one'
+
+describe('Linter - gas-strict-inequalities', () => {
+  it('should raise error on non strict equalities 1', () => {
+    const code = funcWith(`
+      uint256 a;
+      uint256 b;
+      uint256 c;
+      uint256 d;
+      
+      if (a >= b) {  } 
+      if (c <= d) {  } 
+      if (c < d) {  } 
+      if (c > d) {  }`)
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-strict-inequalities': 'error' },
+    })
+
+    assert.equal(report.errorCount, 2)
+    assert.equal(report.messages[0].message, ERROR_MSG)
+    assert.equal(report.messages[1].message, ERROR_MSG)
+  })
+
+  it('should raise error on non strict equalities 2', () => {
+    const code = funcWith(`
+      uint256 a;
+      uint256 b;
+      uint256 c;
+      uint256 d;
+      
+      while (a >= b) {
+
+      }
+              
+      if (c < d) {  }`)
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-strict-inequalities': 'error' },
+    })
+
+    assert.equal(report.errorCount, 1)
+    assert.equal(report.messages[0].message, ERROR_MSG)
+    // assert.equal(report.errorCount, 0)
+  })
+
+  it('should raise error on non strict equalities 3', () => {
+    const code = funcWith(`
+      uint256 a;
+      uint256 b;
+      uint256 c;
+      uint256 d;
+      
+      while (a >= b) {
+
+      }
+              
+      if ((c < d) && (a <= b) && (d >= a)) {  }`)
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-strict-inequalities': 'error' },
+    })
+
+    assert.equal(report.errorCount, 3)
+    assert.equal(report.messages[0].message, ERROR_MSG)
+    assert.equal(report.messages[1].message, ERROR_MSG)
+    assert.equal(report.messages[2].message, ERROR_MSG)
+    // assert.equal(report.errorCount, 0)
+  })
+
+  it('should NOT raise error on strict equalities', () => {
+    const code = funcWith(`
+      uint256 a;
+      uint256 b;
+      uint256 c;
+      uint256 d;
+      
+      while (a > b) {
+
+      }
+              
+      if ((c < d) && (a < b) && (d > a)) {  }`)
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-strict-inequalities': 'error' },
+    })
+
+    assert.equal(report.errorCount, 0)
+  })
+})
diff --git a/test/rules/naming/func-named-parameters.js b/test/rules/naming/func-named-parameters.js
@@ -79,7 +79,12 @@ describe('Linter - func-named-parameters', () => {
 
     const report = linter.processStr(code, {
       extends: 'solhint:all',
-      rules: { 'compiler-version': 'off', 'comprehensive-interface': 'off' },
+      rules: {
+        'compiler-version': 'off',
+        'comprehensive-interface': 'off',
+        'foundry-test-functions': 'off',
+        'non-state-vars-leading-underscore': 'off',
+      },
     })
 
     assertWarnsCount(report, 1)
