Merge pull request #573 from protofire/fix-gas-indexed-and-custom-errors

Fix gas indexed events and gas custom errors
protofire · Apr 10, 2024 · fe83f46 · fe83f46
2 parents 1344624 + 00e8142
commit fe83f46
Show file tree

Hide file tree

Showing 18 changed files with 101 additions and 44 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## [4.5.3] - 2024-04-10
+### Fixed
+- `gas-custom-errors` improved logic to ranged pragma versions [#568](https://github.com/protofire/solhint/pull/568)
+- `gas-indexed-events`  [#568](https://github.com/protofire/solhint/pull/568)
+
 
 ## [4.5.2] - 2024-03-15
 

diff --git a/docker/Dockerfile b/docker/Dockerfile
@@ -1,5 +1,5 @@
 FROM node:20-alpine
 LABEL maintainer="[email protected]" 
-ENV VERSION=4.5.2
+ENV VERSION=4.5.3
 
 RUN npm install -g solhint@"$VERSION"
diff --git a/docs/rules/gas-consumption/gas-calldata-parameters.md b/docs/rules/gas-consumption/gas-calldata-parameters.md
@@ -33,7 +33,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 This rule does not have examples.
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-calldata-parameters.js)

diff --git a/docs/rules/gas-consumption/gas-custom-errors.md b/docs/rules/gas-consumption/gas-custom-errors.md
@@ -65,7 +65,7 @@ revert("Insufficient Balance");
 ```
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-custom-errors.js)

diff --git a/docs/rules/gas-consumption/gas-increment-by-one.md b/docs/rules/gas-consumption/gas-increment-by-one.md
@@ -32,7 +32,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 This rule does not have examples.
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-increment-by-one.js)

diff --git a/docs/rules/gas-consumption/gas-indexed-events.md b/docs/rules/gas-consumption/gas-indexed-events.md
@@ -30,7 +30,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 This rule does not have examples.
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-indexed-events.js)

diff --git a/docs/rules/gas-consumption/gas-length-in-loops.md b/docs/rules/gas-consumption/gas-length-in-loops.md
@@ -30,7 +30,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 This rule does not have examples.
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-length-in-loops.js)

diff --git a/docs/rules/gas-consumption/gas-multitoken1155.md b/docs/rules/gas-consumption/gas-multitoken1155.md
@@ -30,7 +30,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 This rule does not have examples.
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-multitoken1155.js)

diff --git a/docs/rules/gas-consumption/gas-named-return-values.md b/docs/rules/gas-consumption/gas-named-return-values.md
@@ -42,7 +42,7 @@ function checkBalance(address wallet) external view returns(uint256) {}
 ```
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-named-return-values.js)

diff --git a/docs/rules/gas-consumption/gas-small-strings.md b/docs/rules/gas-consumption/gas-small-strings.md
@@ -30,7 +30,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 This rule does not have examples.
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-small-strings.js)

diff --git a/docs/rules/gas-consumption/gas-strict-inequalities.md b/docs/rules/gas-consumption/gas-strict-inequalities.md
@@ -32,7 +32,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 This rule does not have examples.
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-strict-inequalities.js)

diff --git a/docs/rules/gas-consumption/gas-struct-packing.md b/docs/rules/gas-consumption/gas-struct-packing.md
@@ -32,7 +32,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 This rule does not have examples.
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-struct-packing.js)

diff --git a/docs/rules/naming/interface-starts-with-i.md b/docs/rules/naming/interface-starts-with-i.md
@@ -42,7 +42,7 @@ interface Foo { function foo () external; }
 ```
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/best-practises/interface-starts-with-i.js)

diff --git a/lib/rules/gas-consumption/gas-custom-errors.js b/lib/rules/gas-consumption/gas-custom-errors.js
@@ -1,8 +1,9 @@
+const semver = require('semver')
 const BaseChecker = require('../base-checker')
 const { severityDescription } = require('../../doc/utils')
 
 const DEFAULT_SEVERITY = 'warn'
-const BASE_VERSION = [0, 8, 4]
+const BASE_VERSION = '>=0.8.4'
 const ruleId = 'gas-custom-errors'
 const meta = {
   type: 'gas-consumption',
@@ -62,33 +63,8 @@ class GasCustomErrorsChecker extends BaseChecker {
     this.solidityVersion = 0
   }
 
-  parseVersion(version) {
-    // Remove any leading ^ or ~ and other non-numeric characters before the first digit
-    const cleanVersion = version.replace(/^[^\d]+/, '')
-    return cleanVersion.split('.').map((num) => parseInt(num, 10))
-  }
-
-  isVersionGreater(node) {
-    const currentVersion = this.parseVersion(this.solidityVersion)
-    if (currentVersion.length !== 3) {
-      this.error(node, `GC: Invalid Solidity version`)
-      return
-    }
-
-    for (let i = 0; i < BASE_VERSION.length; i++) {
-      if (currentVersion[i] < BASE_VERSION[i]) {
-        // If any part of the current version is less than the base version, return false
-        return false
-      } else if (currentVersion[i] > BASE_VERSION[i]) {
-        // If any part of the current version is greater, no need to check further, return true
-        return true
-      }
-      // If parts are equal, continue to the next part
-    }
-
-    // Reaching here means all parts compared are equal, or the base version parts have been exhausted,
-    // so the current version is at least equal, return true
-    return true
+  isVersionGreater() {
+    return semver.intersects(this.solidityVersion, BASE_VERSION)
   }
 
   PragmaDirective(node) {

diff --git a/lib/rules/gas-consumption/gas-indexed-events.js b/lib/rules/gas-consumption/gas-indexed-events.js
@@ -40,7 +40,7 @@ class GasIndexedEvents extends BaseChecker {
 
       // compare if the type is one of the possible suggestion types
       for (const type of suggestForTypes) {
-        if (argumentType.startsWith(type)) {
+        if (argumentType.startsWith(type) && !node.parameters[i].isIndexed) {
           // push the position into an array to come back later if there's room for another indexed argument
           positionsOfPossibleSuggestions.push(i)
         }

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "solhint",
-  "version": "4.5.2",
+  "version": "4.5.3",
   "description": "Solidity Code Linter",
   "main": "lib/index.js",
   "keywords": [

diff --git a/test/rules/gas-consumption/gas-custom-errors.js b/test/rules/gas-consumption/gas-custom-errors.js
@@ -143,7 +143,7 @@ describe('Linter - gas-custom-errors', () => {
     assertErrorCount(report, 0)
   })
 
-  it('should NOT raise error for lower versions 0.8.3', () => {
+  it('should raise error for higher versions 0.8', () => {
     let code = funcWith(`revert();`)
     code = replaceSolidityVersion(code, '0.8')
 
@@ -152,6 +152,62 @@ describe('Linter - gas-custom-errors', () => {
     })
 
     assertErrorCount(report, 1)
-    assertErrorMessage(report, 'GC: Invalid Solidity version')
+    assert.equal(report.reports[0].message, 'GC: Use Custom Errors instead of revert statements')
+  })
+
+  it('should raise error for higher versions 0.9', () => {
+    let code = funcWith(`revert();`)
+    code = replaceSolidityVersion(code, '0.9')
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-custom-errors': 'error' },
+    })
+
+    assertErrorCount(report, 1)
+    assert.equal(report.reports[0].message, 'GC: Use Custom Errors instead of revert statements')
+  })
+
+  it('should NOT raise error for lower versions 0.7', () => {
+    let code = funcWith(`revert();`)
+    code = replaceSolidityVersion(code, '0.7')
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-custom-errors': 'error' },
+    })
+
+    assertErrorCount(report, 0)
+  })
+
+  it('should NOT raise error for range versions lower than 0.8.4', () => {
+    let code = funcWith(`revert();`)
+    code = replaceSolidityVersion(code, '>= 0.8.1 <= 0.8.3')
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-custom-errors': 'error' },
+    })
+
+    assertErrorCount(report, 0)
+  })
+
+  it('should raise error for range versions higher than 0.8.4', () => {
+    let code = funcWith(`revert();`)
+    code = replaceSolidityVersion(code, '> 0.8.4 <= 0.9')
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-custom-errors': 'error' },
+    })
+
+    assertErrorCount(report, 1)
+  })
+
+  it('should raise error for range versions containing 0.8.4', () => {
+    let code = funcWith(`revert();`)
+    code = replaceSolidityVersion(code, '> 0.8.1 <= 0.8.6')
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-custom-errors': 'error' },
+    })
+
+    assertErrorCount(report, 1)
   })
 })
diff --git a/test/rules/gas-consumption/gas-indexed-events.js b/test/rules/gas-consumption/gas-indexed-events.js
@@ -79,4 +79,24 @@ describe('Linter - gas-indexed-events', () => {
 
     assert.equal(report.errorCount, 0)
   })
+
+  it('should NOT raise error, all variables are indexed', () => {
+    const code = contractWith('event Increment (address indexed sender, uint256 indexed value);')
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-indexed-events': 'error' },
+    })
+
+    assert.equal(report.errorCount, 0)
+  })
+
+  it('should NOT raise error, all variables are indexed or not for suggestion', () => {
+    const code = contractWith('event Increment (address indexed sender, string whatever);')
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-indexed-events': 'error' },
+    })
+
+    assert.equal(report.errorCount, 0)
+  })
 })