Add best-practises examples

lib/rules/best-practises/code-complexity.js

@@ -19,7 +19,21 @@ const meta = {
         description: 'Maximum allowed cyclomatic complexity',
         default: DEFAULT_COMPLEXITY
       }
-    ]
+    ],
+    examples: {
+      good: [
+        {
+          description: 'Low code complexity',
+          code: require('../../../test/fixtures/best-practises/code-complexity-low')
+        }
+      ],
+      bad: [
+        {
+          description: 'High code complexity',
+          code: require('../../../test/fixtures/best-practises/code-complexity-high')
+        }
+      ]
+    }
   },
 
   isDefault: false,

lib/rules/best-practises/max-states-count.js

@@ -22,7 +22,21 @@ const meta = {
         description: 'Maximum allowed number of states declarations',
         default: DEFAULT_MAX_STATES_COUNT
       }
-    ]
+    ],
+    examples: {
+      good: [
+        {
+          description: 'Low code complexity',
+          code: require('../../../test/fixtures/best-practises/number-of-states-low')
+        }
+      ],
+      bad: [
+        {
+          description: 'High code complexity',
+          code: require('../../../test/fixtures/best-practises/number-of-states-high')
+        }
+      ]
+    }
   },
 
   isDefault: false,

lib/rules/best-practises/payable-fallback.js

@@ -6,7 +6,21 @@ const meta = {
 
   docs: {
     description: 'When fallback is not payable you will not be able to receive ethers.',
-    category: 'Best Practise Rules'
+    category: 'Best Practise Rules',
+    examples: {
+      good: [
+        {
+          description: 'Fallback is payable',
+          code: require('../../../test/fixtures/best-practises/fallback-payable')
+        }
+      ],
+      bad: [
+        {
+          description: 'Fallback is not payable',
+          code: require('../../../test/fixtures/best-practises/fallback-not-payable')
+        }
+      ]
+    }
   },
 
   isDefault: false,

lib/rules/best-practises/reason-string.js

@@ -24,7 +24,21 @@ const meta = {
           'A JSON object with a single property "maxLength" specifying the max number of characters per reason string.',
         default: JSON.stringify(DEFAULT_OPTION)
       }
-    ]
+    ],
+    examples: {
+      good: [
+        {
+          description: 'Require with reason string',
+          code: require('../../../test/fixtures/best-practises/require-with-reason')
+        }
+      ],
+      bad: [
+        {
+          description: 'Require without reason string',
+          code: require('../../../test/fixtures/best-practises/require-without-reason')
+        }
+      ]
+    }
   },
 
   isDefault: false,

scripts/generate-rule-docs.js

@@ -80,15 +80,12 @@ ${[
 ].filter(s => s !== '').join("\n")}
 
 ## Description
-
 ${rule.meta.docs.description}
 
 ## Options
-
 ${loadOptions(rule)}
 
 ## Examples
-
 ${loadExamples(rule)}
 
 ## Version
@@ -163,7 +160,8 @@ function loadExamples(rule) {
         return "This rule does not have examples.";
     }
 
-    return [loadCorrectExample(rule), loadIncorrectExample(rule)].filter(s => s !== '').join('\n\n');
+    const examples = [loadCorrectExample(rule), loadIncorrectExample(rule)].filter(s => s !== '').join('\n\n');
+    return examples === '' ? 'This rule does not have examples.' : examples;
 }
 
 function loadIncorrectExample(rule) {
@@ -197,6 +195,7 @@ function getDefaultSeverity(rule) {
 function main() {
     const rules = loadRules()
     rules.forEach(rule => {
+        if (rule.ruleId === 'reason-string')
         console.log(generateRuleDoc(rule))
     })
 }

test/common/contract-builder.js

@@ -1,3 +1,5 @@
+const { times } = require('lodash')
+
 function contractWith(code) {
   return `
       pragma solidity 0.4.4;
@@ -11,9 +13,9 @@ function contractWith(code) {
 
 function funcWith(statements) {
   return contractWith(`
-      function b() public {
-        ${statements}
-      }
+        function b() public {
+          ${statements}
+        }
     `)
 }
 
@@ -30,4 +32,26 @@ contract A {
 `
 }
 
-module.exports = { contractWith, funcWith, multiLine, contractWithPrettier }
+function stateDef(count) {
+  return repeatLines('        uint private a;', count)
+}
+
+function constantDef(count) {
+  return repeatLines('        uint private constant TEST = 1;', count)
+}
+
+function repeatLines(line, count) {
+  return times(count)
+    .map(() => line)
+    .join('\n')
+}
+
+module.exports = {
+  contractWith,
+  funcWith,
+  multiLine,
+  contractWithPrettier,
+  stateDef,
+  constantDef,
+  repeatLines
+}

test/fixtures/best-practises/code-complexity-high.js

@@ -0,0 +1,16 @@
+const { multiLine } = require('./../../common/contract-builder')
+
+module.exports = multiLine(
+  ' if (a > b) {                   ',
+  '   if (b > c) {                 ',
+  '     if (c > d) {               ',
+  '       if (d > e) {             ',
+  '       } else {                 ',
+  '       }                        ',
+  '     }                          ',
+  '   }                            ',
+  ' }                              ',
+  'for (i = 0; i < b; i += 1) { }  ',
+  'do { d++; } while (b > c);       ',
+  'while (d > e) { }               '
+)

test/fixtures/best-practises/code-complexity-low.js

@@ -0,0 +1,13 @@
+const { multiLine } = require('./../../common/contract-builder')
+
+module.exports = multiLine(
+  ' if (a > b) {                   ',
+  '   if (b > c) {                 ',
+  '     if (c > d) {               ',
+  '     }                          ',
+  '   }                            ',
+  ' }                              ',
+  'for (i = 0; i < b; i += 1) { }  ',
+  'do { d++; } while (b > c);       ',
+  'while (d > e) { }               '
+)

test/fixtures/best-practises/fallback-not-payable.js

@@ -0,0 +1,3 @@
+const { contractWith } = require('./../../common/contract-builder')
+
+module.exports = contractWith('function () public {}')

test/fixtures/best-practises/fallback-payable.js

@@ -0,0 +1,3 @@
+const { contractWith } = require('./../../common/contract-builder')
+
+module.exports = contractWith('function () public payable {}')

test/fixtures/best-practises/number-of-states-high.js

@@ -0,0 +1,3 @@
+const { contractWith, stateDef } = require('./../../common/contract-builder')
+
+module.exports = contractWith(stateDef(16))

test/fixtures/best-practises/number-of-states-low.js

@@ -0,0 +1,3 @@
+const { contractWith, stateDef, constantDef } = require('./../../common/contract-builder')
+
+module.exports = contractWith([stateDef(10), constantDef(10)].join('\n'))

test/fixtures/best-practises/require-with-reason.js

@@ -0,0 +1,5 @@
+const { funcWith } = require('./../../common/contract-builder')
+
+module.exports = funcWith(`require(!has(role, account), "Roles: account already has role");
+          role.bearer[account] = true;
+          role.bearer[account] = true;`)

test/fixtures/best-practises/require-without-reason.js

@@ -0,0 +1,5 @@
+const { funcWith } = require('./../../common/contract-builder')
+
+module.exports = funcWith(`require(!has(role, account));
+          role.bearer[account] = true;
+          role.bearer[account] = true;`)

test/rules/best-practises/code-complexity.js

@@ -3,50 +3,25 @@ const { funcWith, multiLine } = require('./../../common/contract-builder')
 const { assertErrorCount, assertErrorMessage, assertNoErrors } = require('./../../common/asserts')
 
 describe('Linter - code-complexity', () => {
-  const MAX_COMPLEXITY_EXCEEDED_CODE = funcWith(
-    multiLine(
-      ' if (a > b) {                   ',
-      '   if (b > c) {                 ',
-      '     if (c > d) {               ',
-      '       if (d > e) {             ',
-      '       } else {                 ',
-      '       }                        ',
-      '     }                          ',
-      '   }                            ',
-      ' }                              ',
-      'for (i = 0; i < b; i += 1) { }  ',
-      'do { d++; } while (b > c);       ',
-      'while (d > e) { }               '
-    )
-  )
-
   it('should raise error when cyclomatic complexity of code is too high', () => {
-    const report = linter.processStr(MAX_COMPLEXITY_EXCEEDED_CODE, {
-      rules: { 'code-complexity': 'error' }
-    })
+    const report = linter.processStr(
+      funcWith(require('../../fixtures/best-practises/code-complexity-high')),
+      {
+        rules: { 'code-complexity': 'error' }
+      }
+    )
 
     assertErrorCount(report, 1)
     assertErrorMessage(report, 'complexity')
   })
 
-  const MAX_COMPLEXITY_CODE = funcWith(
-    multiLine(
-      ' if (a > b) {                   ',
-      '   if (b > c) {                 ',
-      '     if (c > d) {               ',
-      '     }                          ',
-      '   }                            ',
-      ' }                              ',
-      'for (i = 0; i < b; i += 1) { }  ',
-      'do { d++; } while (b > c);       ',
-      'while (d > e) { }               '
-    )
-  )
-
   it('should not raise error when cyclomatic complexity is equal to max default allowed', () => {
-    const report = linter.processStr(MAX_COMPLEXITY_CODE, {
-      rules: { 'code-complexity': 'error' }
-    })
+    const report = linter.processStr(
+      funcWith(require('../../fixtures/best-practises/code-complexity-low')),
+      {
+        rules: { 'code-complexity': 'error' }
+      }
+    )
 
     assertNoErrors(report)
   })

test/rules/best-practises/max-states-count.js

@@ -1,11 +1,10 @@
-const _ = require('lodash')
 const { assertErrorCount, assertNoErrors, assertErrorMessage } = require('./../../common/asserts')
 const linter = require('./../../../lib/index')
-const { contractWith } = require('./../../common/contract-builder')
+const { contractWith, stateDef } = require('./../../common/contract-builder')
 
 describe('Linter - max-states-count', () => {
   it('should raise error when count of states too big', () => {
-    const code = contractWith(stateDef(16))
+    const code = require('../../fixtures/best-practises/number-of-states-high')
 
     const report = linter.processStr(code, {
       rules: { 'max-states-count': ['error', 15] }
@@ -16,7 +15,7 @@ describe('Linter - max-states-count', () => {
   })
 
   it('should not raise error for count of states that lower that max', () => {
-    const code = contractWith([stateDef(10), constantDef(10)].join('\n'))
+    const code = require('../../fixtures/best-practises/number-of-states-low')
 
     const report = linter.processStr(code, {
       rules: { 'max-states-count': 'error' }
@@ -32,18 +31,4 @@ describe('Linter - max-states-count', () => {
 
     assertNoErrors(report)
   })
-
-  function repeatLines(line, count) {
-    return _.times(count)
-      .map(() => line)
-      .join('\n')
-  }
-
-  function stateDef(count) {
-    return repeatLines('uint private a;', count)
-  }
-
-  function constantDef(count) {
-    return repeatLines('uint private constant TEST = 1;', count)
-  }
 })

test/rules/best-practises/payable-fallback.js

@@ -1,10 +1,9 @@
 const { assertNoWarnings, assertErrorMessage, assertWarnsCount } = require('./../../common/asserts')
 const linter = require('./../../../lib/index')
-const { contractWith } = require('./../../common/contract-builder')
 
 describe('Linter - payable-fallback', () => {
   it('should raise warn when fallback is not payable', () => {
-    const code = contractWith('function () public {}')
+    const code = require('../../fixtures/best-practises/fallback-not-payable')
 
     const report = linter.processStr(code, {
       rules: { 'payable-fallback': 'warn' }
@@ -15,7 +14,7 @@ describe('Linter - payable-fallback', () => {
   })
 
   it('should not raise warn when fallback is payable', () => {
-    const code = contractWith('function () public payable {}')
+    const code = require('../../fixtures/best-practises/fallback-payable')
 
     const report = linter.processStr(code, {
       rules: { 'payable-fallback': 'warn' }

test/rules/best-practises/reason-string.js

@@ -11,9 +11,7 @@ const { funcWith } = require('./../../common/contract-builder')
 
 describe('Linter - reason-string', () => {
   it('should raise reason string is mandatory for require', () => {
-    const code = funcWith(`require(!has(role, account));
-        role.bearer[account] = true;role.bearer[account] = true;
-    `)
+    const code = require('../../fixtures/best-practises/require-without-reason')
 
     const report = linter.processStr(code, {
       rules: { 'reason-string': ['warn', { maxLength: 5 }] }
@@ -67,9 +65,7 @@ describe('Linter - reason-string', () => {
   })
 
   it('should not raise warning for require', () => {
-    const code = funcWith(`require(!has(role, account), "Roles: account already has role");
-        role.bearer[account] = true;role.bearer[account] = true;
-    `)
+    const code = require('../../fixtures/best-practises/require-with-reason')
 
     const report = linter.processStr(code, {
       rules: { 'reason-string': ['warn', { maxLength: 31 }] }