Fix 'revert' case of reason-string

'revert' was incorrectly being treated the same as 'require' i.e. the reason string as the 2nd parameter. However 'revert' takes the reason string as its first and only parameter
protofire · Sep 6, 2019 · 00b494e · 00b494e
1 parent 313d33a
commit 00b494e
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 5 deletions.
diff --git a/lib/rules/best-practises/reason-string.js b/lib/rules/best-practises/reason-string.js
@@ -80,7 +80,8 @@ class ReasonStringChecker extends BaseChecker {
       const functionName = this.getFunctionName(ctx)
 
       // Throw an error if have no message
-      if (functionParameters.length <= 1) {
+      if ((functionName === 'revert' && functionParameters.length === 0) ||
+        (functionName === 'require' && functionParameters.length <= 1)) {
         this._errorHaveNoMessage(ctx, functionName)
         return
       }
@@ -112,7 +113,9 @@ class ReasonStringChecker extends BaseChecker {
   getFunctionParameters(ctx) {
     const parent = ctx.parentCtx
     const nodes = parent.children[2]
-    const children = nodes.children[0].children
+    const children = nodes.children && nodes.children.length > 0 
+      ? nodes.children[0].children
+      : []
     const parameters = children
       .filter(value => value.getText() !== ',')
       .map(value => value.getText())

diff --git a/test/rules/best-practises/reason-string.js b/test/rules/best-practises/reason-string.js
@@ -23,7 +23,7 @@ describe('Linter - reason-string', () => {
   })
 
   it('should raise reason string is mandatory for revert', () => {
-    const code = funcWith(`revert(!has(role, account));
+    const code = funcWith(`revert();
         role.bearer[account] = true;role.bearer[account] = true;
     `)
 
@@ -51,7 +51,7 @@ describe('Linter - reason-string', () => {
   })
 
   it('should raise reason string maxLength error for revert', () => {
-    const code = funcWith(`revert(!has(role, account), "Roles: account already has role");
+    const code = funcWith(`revert("Roles: account already has role");
         role.bearer[account] = true;role.bearer[account] = true;
     `)
 
@@ -76,7 +76,7 @@ describe('Linter - reason-string', () => {
   })
 
   it('should not raise error for revert', () => {
-    const code = funcWith(`revert(!has(role, account), "Roles: account already has role");
+    const code = funcWith(`revert("Roles: account already has role");
         role.bearer[account] = true;role.bearer[account] = true;
     `)