diff --git a/docs/rules/gas-consumption/gas-custom-errors.md b/docs/rules/gas-consumption/gas-custom-errors.md index 55f1fdff..145c5995 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 e67ad3b9..842b22e3 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`) + } } } } diff --git a/test/rules/gas-consumption/gas-custom-errors.js b/test/rules/gas-consumption/gas-custom-errors.js index 89d06b85..ff4874f5 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') + }) })