Increase the test coverage of the rules to above 90% for branches

lib/rules/rule-indentation.js

@@ -16,7 +16,7 @@ function whitespaceToDescription(whitespace) {
     } else {
         let hasSpace = whitespace.indexOf(' ') !== -1;
         let hasTab = whitespace.indexOf('\t') !== -1;
-        let type = hasSpace ? (hasTab ? 'mixed' : 'space') : (hasTab ? 'space' : 'unknown');
+        let type = hasSpace ? (hasTab ? 'mixed' : 'space') : (hasTab ? 'tab' : 'unknown');
         let count = hasSpace !== hasTab ? whitespace.length : 0;
         return { type, count };
     }
@@ -42,7 +42,7 @@ module.exports = {
                     let expected = configToDescription(indentation, state.level);
                     let actual = whitespaceToDescription(state.whitespace);
                     if (expected.type !== actual.type || expected.count !== actual.count) {
-                        let expectedDescription = `${expected.count === 0 ? '' : expected.count + ' '}${expected.type}${expected.count > 1 ? 's' : ''}`;
+                        let expectedDescription = `${expected.count} ${expected.type}${expected.count > 1 ? 's' : ''}`;
                         let actualDescription = `${actual.count === 0 ? '' : actual.count + ' '}${actual.type}${actual.count > 1 ? 's' : ''}`;
                         errors.push(`Expected ${expectedDescription}, found ${actualDescription}`);
                     }

lib/rules/runner.js

@@ -8,7 +8,7 @@ function defaults(rules) {
 }
 
 function parseOption(value) {
-    if (value === 'off') {
+    if (value === 'off' || value === 'false') {
         return false;
     } else if (value.match(/^[a-zA-Z]+$/)) {
         return value;
@@ -42,9 +42,8 @@ function runRulesForNode(node, rules, results, optionsStack, state) {
                     options = null;
                 } else {
                     node.text.split(' ').slice(1).forEach(part => {
-                        part.split(':', 2).forEach(([key, value]) => {
-                            options[key] = parseOption(value);
-                        });
+                        let [key, value] = part.split(':', 2);
+                        options[key] = parseOption(value);
                     });
                 }
                 optionsStack[0] = options;

test/rules/rule-if-is-evil.spec.js

@@ -11,26 +11,44 @@ function testConfig(name, mode, contents, expectedErrors) {
 const TEST_CONFIGS = [
     testConfig('simple if with return', 'mostly', `
 location / {
-    error_page 418 = @other;
-    recursive_error_pages on;
-
     if ($something) {
         return 418;
     }
-}
-
-location @other {
-    # some other configuration
-}
-    `, []),
+    return 200;
+}`, []),
+    testConfig('simple if with return but with always', 'always', `
+location / {
+    if ($something) {
+        return 418;
+    }
+    return 200;
+}`, [
+        {
+            rule: 'if-is-evil',
+            text: 'if is evil and not allowed',
+            type: 'error',
+            pos: {
+                start: { column: 5, line: 3, offset: 18 },
+                end: { column: 6, line: 5, offset: 61 }
+            },
+        }
+    ]),
+    testConfig('simple if with return with always with comment', 'always', `
+location / {
+    #nginxlinter if-is-evil:mostly
+    if ($something) {
+        return 418;
+    }
+    return 200;
+}`, []),
     testConfig('if-is-evil.conf file', 'mostly', fs.readFileSync(__dirname + '/../examples/if-is-evil.conf', 'utf8'), [
         {
             rule: 'if-is-evil',
             type: 'error',
-            text: 'A \'rewrite\' within an \'if\' must use the \'last\' flag, found \'break\'',
+            text: 'if is evil and not allowed',
             pos: {
-                start: { column: 39, line: 39, offset: 863 },
-                end: { column: 44, line: 39, offset: 868 }
+                start: { column: 13, line: 19, offset: 353 },
+                end: { column: 14, line: 21, offset: 424 }
             }
         },
         {

test/rules/rule-indentation.spec.js

@@ -12,16 +12,59 @@ const TEST_CONFIGS = [
 location / {
     error_page 418 = @other;
     recursive_error_pages on;
-
+    
+    # if is normally evil, but I know what I'm doing
     if ($something) {
         return 418;
     }
-}
-
-location @other {
-    # some other configuration
-}
-    `, []),
+}`, []),
+    testConfig('2 spaces', 2, 'location / {\n  # OK\n  return 200;\n}', []),
+    testConfig('4 spaces instead of 2', 2, 'location / {\n    return 200;\n}', [
+        {
+            rule: 'indentation',
+            text: 'Expected 2 spaces, found 4 spaces',
+            type: 'error',
+            pos: {
+                start: { column: 5, line: 2, offset: 17 },
+                end: { column: 16, line: 2, offset: 28 }
+            }
+        }
+    ]),
+    testConfig('4 spaces', 4, 'location / {\n    # OK\n    return 200;\n}', []),
+    testConfig('2 spaces instead of 4', 4, 'location / {\n  return 200;\n}', [
+        {
+            rule: 'indentation',
+            text: 'Expected 4 spaces, found 2 spaces',
+            type: 'error',
+            pos: {
+                start: { column: 3, line: 2, offset: 15 },
+                end: { column: 14, line: 2, offset: 26 }
+            }
+        }
+    ]),
+    testConfig('tabs', 'tab', 'location / {\n\t# OK\n\treturn 200;\n}', []),
+    testConfig('2 spaces instead of tabs', 'tab', 'location / {\n  return 200;\n}', [
+        {
+            rule: 'indentation',
+            text: 'Expected 1 tab, found 2 spaces',
+            type: 'error',
+            pos: {
+                start: { column: 3, line: 2, offset: 15 },
+                end: { column: 14, line: 2, offset: 26 }
+            }
+        }
+    ]),
+    testConfig('2 spaces instead of tabs', 'tab', 'location / {\n \treturn 200;\n}', [
+        {
+            rule: 'indentation',
+            text: 'Expected 1 tab, found mixed',
+            type: 'error',
+            pos: {
+                start: { column: 3, line: 2, offset: 15 },
+                end: { column: 14, line: 2, offset: 26 }
+            }
+        }
+    ])
 ];
 
 describe('rules/indentation', () => {