Repository: protofire/solhint
Pull request #555 (2024-03-06 15:10:14)

fix: custom errors checks from 0.8.4 forward


Patch:
diff --git a/docs/rules/gas-consumption/gas-custom-errors.md b/docs/rules/gas-consumption/gas-custom-errors.md
index 55f1fdf..145c599 100644
--- a/docs/rules/gas-consumption/gas-custom-errors.md
+++ b/docs/rules/gas-consumption/gas-custom-errors.md
@@ -26,6 +26,8 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
 }
 ```
 
+### Notes
+- This rules applies to Solidity version 0.8.4 and higher
 
 ## Examples
 ### 👍 Examples of **correct** code for this rule
diff --git a/lib/rules/gas-consumption/gas-custom-errors.js b/lib/rules/gas-consumption/gas-custom-errors.js
index e67ad3b..842b22e 100644
--- a/lib/rules/gas-consumption/gas-custom-errors.js
+++ b/lib/rules/gas-consumption/gas-custom-errors.js
@@ -2,7 +2,7 @@ const BaseChecker = require('../base-checker')
 const { severityDescription } = require('../../doc/utils')
 
 const DEFAULT_SEVERITY = 'warn'
-
+const BASE_VERSION = [0, 8, 4]
 const ruleId = 'gas-custom-errors'
 const meta = {
   type: 'gas-consumption',
@@ -16,6 +16,11 @@ const meta = {
         default: DEFAULT_SEVERITY,
       },
     ],
+    notes: [
+      {
+        note: 'This rules applies to Solidity version 0.8.4 and higher',
+      },
+    ],
     examples: {
       good: [
         {
@@ -54,21 +59,60 @@ const meta = {
 class GasCustomErrorsChecker extends BaseChecker {
   constructor(reporter) {
     super(reporter, ruleId, meta)
+    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
+  }
+
+  PragmaDirective(node) {
+    if (node.type === 'PragmaDirective' && node.name === 'solidity') {
+      this.solidityVersion = node.value
+    }
   }
 
   FunctionCall(node) {
     let errorStr = ''
-    if (node.expression.name === 'require') {
-      errorStr = 'require'
-    } else if (
-      node.expression.name === 'revert' &&
-      (node.arguments.length === 0 || node.arguments[0].type === 'StringLiteral')
-    ) {
-      errorStr = 'revert'
-    }
 
-    if (errorStr !== '') {
-      this.error(node, `GC: Use Custom Errors instead of ${errorStr} statements`)
+    if (this.isVersionGreater(node)) {
+      if (node.expression.name === 'require') {
+        errorStr = 'require'
+      } else if (
+        node.expression.name === 'revert' &&
+        (node.arguments.length === 0 || node.arguments[0].type === 'StringLiteral')
+      ) {
+        errorStr = 'revert'
+      }
+
+      if (errorStr !== '') {
+        this.error(node, `GC: Use Custom Errors instead of ${errorStr} statements`)
+      }
     }
   }
 }


Test patch:
diff --git a/test/rules/gas-consumption/gas-custom-errors.js b/test/rules/gas-consumption/gas-custom-errors.js
index 89d06b8..ff4874f 100644
--- a/test/rules/gas-consumption/gas-custom-errors.js
+++ b/test/rules/gas-consumption/gas-custom-errors.js
@@ -9,9 +9,18 @@ const {
 const linter = require('../../../lib/index')
 const { funcWith } = require('../../common/contract-builder')
 
+function replaceSolidityVersion(code, newVersion) {
+  // Regular expression to match the version number in the pragma directive
+  const versionRegex = /pragma solidity \d+\.\d+\.\d+;/
+  // Replace the matched version with the newVersion
+  return code.replace(versionRegex, `pragma solidity ${newVersion};`)
+}
+
 describe('Linter - gas-custom-errors', () => {
   it('should raise error for revert()', () => {
-    const code = funcWith(`revert();`)
+    let code = funcWith(`revert();`)
+    code = replaceSolidityVersion(code, '^0.8.4')
+
     const report = linter.processStr(code, {
       rules: { 'gas-custom-errors': 'error' },
     })
@@ -21,7 +30,9 @@ describe('Linter - gas-custom-errors', () => {
   })
 
   it('should raise error for revert([string])', () => {
-    const code = funcWith(`revert("Insufficent funds");`)
+    let code = funcWith(`revert("Insufficent funds");`)
+    code = replaceSolidityVersion(code, '0.8.4')
+
     const report = linter.processStr(code, {
       rules: { 'gas-custom-errors': 'error' },
     })
@@ -31,7 +42,9 @@ describe('Linter - gas-custom-errors', () => {
   })
 
   it('should NOT raise error for revert ErrorFunction()', () => {
-    const code = funcWith(`revert ErrorFunction();`)
+    let code = funcWith(`revert ErrorFunction();`)
+    code = replaceSolidityVersion(code, '^0.8.22')
+
     const report = linter.processStr(code, {
       rules: { 'gas-custom-errors': 'error' },
     })
@@ -41,7 +54,9 @@ describe('Linter - gas-custom-errors', () => {
   })
 
   it('should NOT raise error for revert ErrorFunction() with arguments', () => {
-    const code = funcWith(`revert ErrorFunction({ msg: "Insufficent funds msg" });`)
+    let code = funcWith(`revert ErrorFunction({ msg: "Insufficent funds msg" });`)
+    code = replaceSolidityVersion(code, '^0.8.5')
+
     const report = linter.processStr(code, {
       rules: { 'gas-custom-errors': 'error' },
     })
@@ -51,9 +66,11 @@ describe('Linter - gas-custom-errors', () => {
   })
 
   it('should raise error for require', () => {
-    const code = funcWith(`require(!has(role, account), "Roles: account already has role");
+    let code = funcWith(`require(!has(role, account), "Roles: account already has role");
         role.bearer[account] = true;role.bearer[account] = true;
     `)
+    code = replaceSolidityVersion(code, '0.8.21')
+
     const report = linter.processStr(code, {
       rules: { 'gas-custom-errors': 'error' },
     })
@@ -63,9 +80,11 @@ describe('Linter - gas-custom-errors', () => {
   })
 
   it('should NOT raise error for regular function call', () => {
-    const code = funcWith(`callOtherFunction();
+    let code = funcWith(`callOtherFunction();
         role.bearer[account] = true;role.bearer[account] = true;
     `)
+    code = replaceSolidityVersion(code, '^0.9.0')
+
     const report = linter.processStr(code, {
       rules: { 'gas-custom-errors': 'error' },
     })
@@ -74,10 +93,12 @@ describe('Linter - gas-custom-errors', () => {
   })
 
   it('should raise error for require, revert message and not for revert CustomError() for [recommended] config', () => {
-    const code = funcWith(`require(!has(role, account), "Roles: account already has role");
+    let code = funcWith(`require(!has(role, account), "Roles: account already has role");
         revert("RevertMessage");
         revert CustomError();
     `)
+    code = replaceSolidityVersion(code, '0.8.20')
+
     const report = linter.processStr(code, {
       extends: 'solhint:recommended',
       rules: { 'compiler-version': 'off' },
@@ -88,7 +109,7 @@ describe('Linter - gas-custom-errors', () => {
     assert.equal(report.reports[1].message, 'GC: Use Custom Errors instead of revert statements')
   })
 
-  it('should NOT raise error for default config', () => {
+  it('should NOT raise error for default config because rule is not part of default', () => {
     const code = funcWith(`require(!has(role, account), "Roles: account already has role");
         revert("RevertMessage");
         revert CustomError();
@@ -100,4 +121,37 @@ describe('Linter - gas-custom-errors', () => {
     assertWarnsCount(report, 0)
     assertErrorCount(report, 0)
   })
+
+  it('should NOT raise error for lower versions 0.8.3', () => {
+    let code = funcWith(`revert();`)
+    code = replaceSolidityVersion(code, '0.8.3')
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-custom-errors': 'error' },
+    })
+
+    assertErrorCount(report, 0)
+  })
+
+  it('should NOT raise error for lower versions 0.4.4', () => {
+    const code = funcWith(`revert("Insufficent funds");`)
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-custom-errors': 'error' },
+    })
+
+    assertErrorCount(report, 0)
+  })
+
+  it('should NOT raise error for lower versions 0.8.3', () => {
+    let code = funcWith(`revert();`)
+    code = replaceSolidityVersion(code, '0.8')
+
+    const report = linter.processStr(code, {
+      rules: { 'gas-custom-errors': 'error' },
+    })
+
+    assertErrorCount(report, 1)
+    assertErrorMessage(report, 'GC: Invalid Solidity version')
+  })
 })