diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js index d3548aa6..20824769 100644 --- a/conf/rulesets/solhint-all.js +++ b/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', diff --git a/docs/rules.md b/docs/rules.md index 2506059c..153509b9 100644 --- a/docs/rules.md +++ b/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 diff --git a/docs/rules/gas-consumption/gas-length-in-loops.md b/docs/rules/gas-consumption/gas-length-in-loops.md new file mode 100644 index 00000000..aca08a0e --- /dev/null +++ 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/lib/rules/gas-consumption/gas-length-in-loops.js b/lib/rules/gas-consumption/gas-length-in-loops.js new file mode 100644 index 00000000..6fa95207 --- /dev/null +++ b/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 diff --git a/lib/rules/gas-consumption/index.js b/lib/rules/gas-consumption/index.js index d29e54b0..956fce25 100644 --- 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 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), diff --git a/test/rules/gas-consumption/gas-length-in-loops.js b/test/rules/gas-consumption/gas-length-in-loops.js new file mode 100644 index 00000000..37fe451f --- /dev/null +++ b/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) + }) +})