Merge pull request #158 from kevsul/fix-reason-string-revert

Fix 'revert' case of reason-string
protofire · Sep 9, 2019 · 8745034 · 8745034
2 parents 313d33a + f8807bb
commit 8745034
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,10 @@ 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 +115,7 @@ 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;
     `)