Merge pull request #559 from protofire/gc-length-in-loops

GC: Dot Length in Loops

Author: dbale-altoros

conf/rulesets/solhint-all.js

@@ -28,6 +28,7 @@ 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',

docs/rules.md

@@ -37,6 +37,7 @@ title:       "Rule Index of Solhint"
 | [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

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)

lib/rules/gas-consumption/gas-length-in-loops.js

@@ -0,0 +1,75 @@
+const BaseChecker = require('../base-checker')
+
+let found
+const ruleId = 'gas-length-in-loops'
+const meta = {
+  type: 'gas-consumption',
+
+  docs: {
+    description:
+      'Suggest replacing object.length in a loop condition to avoid calculation on each lap',
+    category: 'Gas Consumption Rules',
+    notes: [
+      {
+        note: '[source 1](https://coinsbench.com/comprehensive-guide-tips-and-tricks-for-gas-optimization-in-solidity-5380db734404) of the rule initiative (see Array Length Caching)',
+      },
+    ],
+  },
+
+  isDefault: false,
+  recommended: false,
+  defaultSetup: 'warn',
+
+  schema: null,
+}
+
+class GasLengthInLoops extends BaseChecker {
+  constructor(reporter) {
+    super(reporter, ruleId, meta)
+  }
+
+  checkConditionForMemberAccessLength(node) {
+    if (found) return // Return early if the condition has already been found
+    if (typeof node === 'object' && node !== null) {
+      if (node.type === 'MemberAccess' && node.memberName === 'length') {
+        found = true // Update the flag if the condition is met
+        return
+      }
+      // Recursively search through all object properties
+      Object.values(node).forEach((value) => this.checkConditionForMemberAccessLength(value))
+    }
+  }
+
+  DoWhileStatement(node) {
+    found = false
+    this.checkConditionForMemberAccessLength(node.condition)
+    if (found) {
+      this.reportError(node)
+    }
+  }
+
+  WhileStatement(node) {
+    found = false
+    this.checkConditionForMemberAccessLength(node.condition)
+    if (found) {
+      this.reportError(node)
+    }
+  }
+
+  ForStatement(node) {
+    found = false
+    this.checkConditionForMemberAccessLength(node.conditionExpression)
+    if (found) {
+      this.reportError(node)
+    }
+  }
+
+  reportError(node) {
+    this.error(
+      node,
+      `GC: Found [ .length ] property in Loop condition. Suggestion: assign it to a variable`
+    )
+  }
+}
+
+module.exports = GasLengthInLoops

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 GasLengthInLoops = require('./gas-length-in-loops')
 const GasStrictInequalities = require('./gas-strict-inequalities')
 const GasNamedReturnValuesChecker = require('./gas-named-return-values')
 const GasCustomErrorsChecker = require('./gas-custom-errors')
@@ -16,6 +17,7 @@ module.exports = function checkers(reporter, config) {
     new GasCalldataParameters(reporter, config),
     new GasIncrementByOne(reporter, config),
     new GasStructPacking(reporter, config),
+    new GasLengthInLoops(reporter, config),
     new GasStrictInequalities(reporter, config),
     new GasNamedReturnValuesChecker(reporter),
     new GasCustomErrorsChecker(reporter),

test/rules/gas-consumption/gas-length-in-loops.js

@@ -0,0 +1,129 @@
+const assert = require('assert')
+const linter = require('../../../lib/index')
+const funcWith = require('../../common/contract-builder').funcWith
+
+const ERROR_MSG =
+  'GC: Found [ .length ] property in Loop condition. Suggestion: assign it to a variable'
+
+describe('Linter - gas-length-in-loops', () => {
+  it('should raise error on ForLoop with .length in condition', () => {
+    const code = funcWith(`
+      for (uint256 length = 0; length > object.length; legnth++) {
+        // code block to be executed
+      }`)
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-length-in-loops': 'error' },
+    })
+
+    assert.equal(report.errorCount, 1)
+    assert.equal(report.messages[0].message, ERROR_MSG)
+  })
+
+  it('should raise error on While with .length in condition', () => {
+    const code = funcWith(`
+      while (condition + 1 && boolIsTrue && arr.length > i) {
+        // code block to be executed
+        arr.length.push(1);
+      }`)
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-length-in-loops': 'error' },
+    })
+
+    assert.equal(report.errorCount, 1)
+    assert.equal(report.messages[0].message, ERROR_MSG)
+  })
+
+  it('should raise error on DoWhile with .length in condition', () => {
+    const code = funcWith(`
+        do {
+          // code block to be executed
+        } while (condition[5].member > 35 && length && arr.length < counter); 
+      `)
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-length-in-loops': 'error' },
+    })
+
+    assert.equal(report.errorCount, 1)
+    assert.equal(report.messages[0].message, ERROR_MSG)
+  })
+
+  it('should raise error on DoWhile and While with .length in condition', () => {
+    const code = funcWith(`
+        for (uint256 length = 0; condition; length++) {
+          // code block to be executed
+        }
+
+        while (condition + 1 && boolIsTrue && arr.length > i) {
+          // code block to be executed
+          arr.length.push(1);
+        }
+
+        do {
+          // code block to be executed
+        } while (condition[5].member > 35 && length && arr.length < counter); 
+      `)
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-length-in-loops': 'error' },
+    })
+
+    assert.equal(report.errorCount, 2)
+    assert.equal(report.messages[0].message, ERROR_MSG)
+    assert.equal(report.messages[1].message, ERROR_MSG)
+  })
+
+  it('should NOT raise error on ForLoop with none .length in condition', () => {
+    const code = funcWith(`
+      for (initialization; condition; iteration) {
+        // code block to be executed
+      }`)
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-length-in-loops': 'error' },
+    })
+
+    assert.equal(report.errorCount, 0)
+  })
+
+  it('should NOT raise error on ForLoop with none .length in condition', () => {
+    const code = funcWith(`
+      for (uint256 length = 0; condition; length++) {
+        // code block to be executed
+      }`)
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-length-in-loops': 'error' },
+    })
+
+    assert.equal(report.errorCount, 0)
+  })
+
+  it('should NOT raise error on While with none .length in condition', () => {
+    const code = funcWith(`
+      while (condition) {
+        // code block to be executed
+      }`)
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-length-in-loops': 'error' },
+    })
+
+    assert.equal(report.errorCount, 0)
+  })
+
+  it('should NOT raise error on DoWhile with none .length in condition', () => {
+    const code = funcWith(`
+      do {
+        // code block to be executed
+      } while (condition[5].member > 35 && length);`)
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-length-in-loops': 'error' },
+    })
+
+    assert.equal(report.errorCount, 0)
+  })
+})