diff --git a/docs/rules.md b/docs/rules.md index 7c79c301..67faacb6 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -57,18 +57,18 @@ title: "Rule Index of Solhint" ## Gas Consumption Rules -| Rule Id | Error | Recommended | Deprecated | -| ----------------------------------------------------------------------------- | ------------------------------------------------------------------------------------ | ------------ | ---------- | -| [gas-calldata-parameters](./rules/gas-consumption/gas-calldata-parameters.md) | Suggest calldata keyword on function arguments when read only | | | -| [gas-custom-errors](./rules/gas-consumption/gas-custom-errors.md) | Enforces the use of Custom Errors over Require and Revert statements | $~~~~~~~~$✔️ | | -| [gas-increment-by-one](./rules/gas-consumption/gas-increment-by-one.md) | Suggest increments by one, like this ++i instead of other type | | | -| [gas-indexed-events](./rules/gas-consumption/gas-indexed-events.md) | Suggest indexed arguments on events for uint, bool and address | | | -| [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 | | | -| [gas-multitoken1155](./rules/gas-consumption/gas-multitoken1155.md) | ERC1155 is a cheaper non-fungible token than ERC721 | | | -| [gas-named-return-values](./rules/gas-consumption/gas-named-return-values.md) | Enforce the return values of a function to be named | | | -| [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 | | | +| Rule Id | Error | Recommended | Deprecated | +| ----------------------------------------------------------------------------- | --------------------------------------------------------------------------------------- | ------------ | ---------- | +| [gas-calldata-parameters](./rules/gas-consumption/gas-calldata-parameters.md) | Suggest calldata keyword on function arguments when read only | | | +| [gas-custom-errors](./rules/gas-consumption/gas-custom-errors.md) | Enforces the use of Custom Errors over Require with strings error and Revert statements | $~~~~~~~~$✔️ | | +| [gas-increment-by-one](./rules/gas-consumption/gas-increment-by-one.md) | Suggest increments by one, like this ++i instead of other type | | | +| [gas-indexed-events](./rules/gas-consumption/gas-indexed-events.md) | Suggest indexed arguments on events for uint, bool and address | | | +| [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 | | | +| [gas-multitoken1155](./rules/gas-consumption/gas-multitoken1155.md) | ERC1155 is a cheaper non-fungible token than ERC721 | | | +| [gas-named-return-values](./rules/gas-consumption/gas-named-return-values.md) | Enforce the return values of a function to be named | | | +| [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 | | | ## Miscellaneous diff --git a/docs/rules/gas-consumption/gas-custom-errors.md b/docs/rules/gas-consumption/gas-custom-errors.md index c8e6c906..1beac5c5 100644 --- a/docs/rules/gas-consumption/gas-custom-errors.md +++ b/docs/rules/gas-consumption/gas-custom-errors.md @@ -12,7 +12,7 @@ title: "gas-custom-errors | Solhint" ## Description -Enforces the use of Custom Errors over Require and Revert statements +Enforces the use of Custom Errors over Require with strings error and Revert statements ## Options This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Defaults to warn. @@ -44,6 +44,24 @@ revert CustomErrorFunction(); revert CustomErrorFunction({ msg: "Insufficient Balance" }); ``` +#### Use of Require with Custom Error with arguments + +```solidity +require(success, CustomErrorFunction({ msg: "Insufficient Balance" }); +``` + +#### Use of Require with function call and Custom Error + +```solidity +require(isAuthorized(account), CustomErrorFunction(); +``` + +#### Use of Require with binary comparison and Custom Error + +```solidity +require(a > b, CustomErrorFunction(); +``` + ### 👎 Examples of **incorrect** code for this rule #### Use of require statement diff --git a/lib/rules/gas-consumption/gas-custom-errors.js b/lib/rules/gas-consumption/gas-custom-errors.js index f6ed21e2..577de5ab 100644 --- a/lib/rules/gas-consumption/gas-custom-errors.js +++ b/lib/rules/gas-consumption/gas-custom-errors.js @@ -9,7 +9,8 @@ const meta = { type: 'gas-consumption', docs: { - description: 'Enforces the use of Custom Errors over Require and Revert statements', + description: + 'Enforces the use of Custom Errors over Require with strings error and Revert statements', category: 'Gas Consumption Rules', options: [ { @@ -32,6 +33,18 @@ const meta = { description: 'Use of Custom Errors with arguments', code: 'revert CustomErrorFunction({ msg: "Insufficient Balance" });', }, + { + description: 'Use of Require with Custom Error with arguments', + code: 'require(success, CustomErrorFunction({ msg: "Insufficient Balance" });', + }, + { + description: 'Use of Require with function call and Custom Error', + code: 'require(isAuthorized(account), CustomErrorFunction();', + }, + { + description: 'Use of Require with binary comparison and Custom Error', + code: 'require(a > b, CustomErrorFunction();', + }, ], bad: [ { @@ -75,9 +88,9 @@ class GasCustomErrorsChecker extends BaseChecker { FunctionCall(node) { let errorStr = '' - if (this.isVersionGreater(node)) { - if (node.expression.name === 'require') { + // 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' } else if ( node.expression.name === 'revert' && diff --git a/test/rules/gas-consumption/gas-custom-errors.js b/test/rules/gas-consumption/gas-custom-errors.js index 0256207a..e2fd0281 100644 --- a/test/rules/gas-consumption/gas-custom-errors.js +++ b/test/rules/gas-consumption/gas-custom-errors.js @@ -17,6 +17,70 @@ function replaceSolidityVersion(code, newVersion) { } describe('Linter - gas-custom-errors', () => { + 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') + + const report = linter.processStr(code, { + rules: { 'gas-custom-errors': 'error' }, + }) + + assertNoWarnings(report) + assertNoErrors(report) + }) + + it('should NOT raise error for require with function call and comparison along with custom error', () => { + let code = funcWith(`require(isAok(a) > b, CustomErrorEmitted(param1));`) + code = replaceSolidityVersion(code, '^0.8.4') + + const report = linter.processStr(code, { + rules: { 'gas-custom-errors': 'error' }, + }) + + assertNoWarnings(report) + assertNoErrors(report) + }) + + it('should NOT raise error for require with boolean check and custom error call', () => { + let code = funcWith(`require(success, CustomErrorEmitted(param1, param2));`) + code = replaceSolidityVersion(code, '^0.8.4') + + const report = linter.processStr(code, { + rules: { 'gas-custom-errors': 'error' }, + }) + + assertNoWarnings(report) + assertNoErrors(report) + }) + + it('should NOT raise error for require with function call and custom error call', () => { + let code = funcWith( + `require(isSuccess(param1) == true && value > 10, CustomErrorEmitted(param1, param2));` + ) + code = replaceSolidityVersion(code, '^0.8.4') + + const report = linter.processStr(code, { + rules: { 'gas-custom-errors': 'error' }, + }) + + assertNoWarnings(report) + assertNoErrors(report) + }) + + it('should NOT raise error for require and mapping access with boolean value check and custom error call', () => { + let code = funcWith( + `require(users[msg.sender].isRegistered, CustomErrorEmitted(param1, param2));` + ) + code = replaceSolidityVersion(code, '^0.8.4') + + const report = linter.processStr(code, { + rules: { 'gas-custom-errors': 'error' }, + }) + + assertNoWarnings(report) + assertNoErrors(report) + }) + it('should raise error for revert()', () => { let code = funcWith(`revert();`) code = replaceSolidityVersion(code, '^0.8.4') @@ -30,7 +94,7 @@ describe('Linter - gas-custom-errors', () => { }) it('should raise error for revert([string])', () => { - let code = funcWith(`revert("Insufficent funds");`) + let code = funcWith(`revert("Insufficient funds");`) code = replaceSolidityVersion(code, '0.8.4') const report = linter.processStr(code, { @@ -54,7 +118,7 @@ describe('Linter - gas-custom-errors', () => { }) it('should NOT raise error for revert ErrorFunction() with arguments', () => { - let code = funcWith(`revert ErrorFunction({ msg: "Insufficent funds msg" });`) + let code = funcWith(`revert ErrorFunction({ msg: "Insufficient funds msg" });`) code = replaceSolidityVersion(code, '^0.8.5') const report = linter.processStr(code, { @@ -134,7 +198,7 @@ describe('Linter - gas-custom-errors', () => { }) it('should NOT raise error for lower versions 0.4.4', () => { - const code = funcWith(`revert("Insufficent funds");`) + const code = funcWith(`revert("Insufficient funds");`) const report = linter.processStr(code, { rules: { 'gas-custom-errors': 'error' },