Merge pull request #620 from protofire/fix/custom-errors-in-requires

fix: custom errors in requires
protofire · Jan 1, 2025 · 95ebbd1 · 95ebbd1
2 parents 2bf186f + 23ffe9b
commit 95ebbd1
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 3 deletions.
diff --git a/lib/rules/gas-consumption/gas-custom-errors.js b/lib/rules/gas-consumption/gas-custom-errors.js
@@ -89,9 +89,25 @@ class GasCustomErrorsChecker extends BaseChecker {
   FunctionCall(node) {
     let errorStr = ''
     if (this.isVersionGreater(node)) {
-      // added second part of conditional to be able to use require with Custom Errors
-      if (node.expression.name === 'require' && node.arguments[1].type !== 'FunctionCall') {
-        errorStr = 'require'
+      if (node.expression.name === 'require') {
+        // If no second argument or second argument is a string, flag it
+        if (node.arguments.length < 2) {
+          // No second argument
+          errorStr = 'require'
+        } else {
+          const secondArg = node.arguments[1]
+          if (secondArg.type === 'StringLiteral') {
+            // e.g. require(cond, "Error message");
+            errorStr = 'require'
+          } else if (secondArg.type !== 'FunctionCall') {
+            // e.g. require(cond, 42) or require(cond, someVar)
+            // Probably not a custom error, so still flag
+            errorStr = 'require'
+          }
+          // else if secondArg.type === 'FunctionCall':
+          // e.g. require(cond, MyCustomError())
+          // We skip, because it’s presumably a custom error
+        }
       } else if (
         node.expression.name === 'revert' &&
         (node.arguments.length === 0 || node.arguments[0].type === 'StringLiteral')

diff --git a/test/rules/gas-consumption/gas-custom-errors.js b/test/rules/gas-consumption/gas-custom-errors.js
@@ -17,6 +17,39 @@ function replaceSolidityVersion(code, newVersion) {
 }
 
 describe('Linter - gas-custom-errors', () => {
+  it('should raise error for require with a non-function-call second argument', () => {
+    // second arg is a numeric literal instead of a function call
+    const code = `
+      pragma solidity 0.8.5;
+      contract A {
+        function test() external {
+          require(msg.sender != address(0), 123);
+        }
+      }
+    `
+    const report = linter.processStr(code, {
+      rules: { 'gas-custom-errors': 'error' },
+    })
+    assertErrorCount(report, 1)
+    assertErrorMessage(report, 'Use Custom Errors instead of require statements')
+  })
+
+  it('should raise error for require with no second argument', () => {
+    const code = `
+      pragma solidity 0.8.5;
+      contract A {
+        function test() external {
+          require(msg.sender != address(0));
+        }
+      }
+    `
+    const report = linter.processStr(code, {
+      rules: { 'gas-custom-errors': 'error' },
+    })
+    assertErrorCount(report, 1)
+    assertErrorMessage(report, 'Use Custom Errors instead of require statements')
+  })
+
   it('should NOT raise error for require with comparison and custom error()', () => {
     let code = funcWith(`require(a > b, CustomErrorEmitted());`)
     code = replaceSolidityVersion(code, '^0.8.4')