[Issue #83] Enhanced chai-vague-errors rule to ban strict comparison …

…in the expects parameter closes #83
microsoft · Aug 11, 2016 · 3f85442 · 3f85442
1 parent d74d383
commit 3f85442
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 4 deletions.
diff --git a/src/chaiVagueErrorsRule.ts b/src/chaiVagueErrorsRule.ts
@@ -6,6 +6,11 @@ import {SyntaxKind} from './utils/SyntaxKind';
 import {ChaiUtils} from './utils/ChaiUtils';
 import {ExtendedMetadata} from './utils/ExtendedMetadata';
 
+const BASE_ERROR: string = 'Found chai call with vague failure message. ';
+const FAILURE_STRING = BASE_ERROR + 'Please add an explicit failure message';
+const FAILURE_STRING_COMPARE_TRUE = BASE_ERROR + 'Move the strict equality comparison from the expect call into the assertion value';
+const FAILURE_STRING_COMPARE_FALSE = BASE_ERROR + 'Move the strict inequality comparison from the expect call into the assertion value. ';
+
 /**
  * Implementation of the chai-vague-errors rule.
  */
@@ -24,8 +29,6 @@ export class Rule extends Lint.Rules.AbstractRule {
         commonWeaknessEnumeration: '398, 710'
     };
 
-    public static FAILURE_STRING = 'Found chai call with vague failure message. Please add an explicit failure message';
-
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
         return this.applyWithWalker(new ChaiVagueErrorsRuleWalker(sourceFile, this.getOptions()));
     }
@@ -35,7 +38,7 @@ class ChaiVagueErrorsRuleWalker extends ErrorTolerantWalker {
     protected visitPropertyAccessExpression(node: ts.PropertyAccessExpression): void {
         if (ChaiUtils.isExpectInvocation(node)) {
             if (/ok|true|false|undefined|null/.test(node.name.getText())) {
-                this.addFailure(this.createFailure(node.getStart(), node.getWidth(), Rule.FAILURE_STRING));
+                this.addFailure(this.createFailure(node.getStart(), node.getWidth(), FAILURE_STRING));
             }
         }
         super.visitPropertyAccessExpression(node);
@@ -47,11 +50,26 @@ class ChaiVagueErrorsRuleWalker extends ErrorTolerantWalker {
                 if (ChaiUtils.isEqualsInvocation(<ts.PropertyAccessExpression>node.expression)) {
                     if (node.arguments.length === 1) {
                         if (/true|false|null|undefined/.test(node.arguments[0].getText())) {
-                            this.addFailure(this.createFailure(node.getStart(), node.getWidth(), Rule.FAILURE_STRING));
+                            this.addFailure(this.createFailure(node.getStart(), node.getWidth(), FAILURE_STRING));
                         }
                     }
                 }
             }
+
+            const actualValue: ts.Node = ChaiUtils.getFirstExpectCallParameter(node);
+            if (actualValue.kind === SyntaxKind.current().BinaryExpression) {
+                const expectedValue: ts.Node = ChaiUtils.getFirstExpectationParameter(node);
+                const binaryExpression: ts.BinaryExpression = <ts.BinaryExpression>actualValue;
+                const operator: string = binaryExpression.operatorToken.getText();
+                const expectingBooleanKeyword: boolean = expectedValue.kind === SyntaxKind.current().TrueKeyword
+                    || expectedValue.kind === SyntaxKind.current().FalseKeyword;
+
+                if (operator === '===' && expectingBooleanKeyword) {
+                    this.addFailure(this.createFailure(node.getStart(), node.getWidth(), FAILURE_STRING_COMPARE_TRUE));
+                } else if (operator === '!==' && expectingBooleanKeyword) {
+                    this.addFailure(this.createFailure(node.getStart(), node.getWidth(), FAILURE_STRING_COMPARE_FALSE));
+                }
+            }
         }
         super.visitCallExpression(node);
     }

diff --git a/src/utils/ChaiUtils.ts b/src/utils/ChaiUtils.ts
@@ -28,6 +28,21 @@ export module ChaiUtils {
         return null;
     }
 
+    export function getFirstExpectCallParameter(node: ts.CallExpression): ts.Node {
+        const expectCall: ts.CallExpression = ChaiUtils.getLeftMostCallExpression(node);
+        if (expectCall.arguments.length > 0) {
+            return expectCall.arguments[0];
+        }
+        return null;
+    }
+
+    export function getFirstExpectationParameter(node: ts.CallExpression): ts.Node {
+        if (node.arguments.length > 0) {
+            return node.arguments[0];
+        }
+        return null;
+    }
+
     export function isEqualsInvocation(propExpression: ts.PropertyAccessExpression): boolean {
         return /equal|equals|eq|eql|eqs/.test(propExpression.name.getText());
     }

diff --git a/tests/ChaiVagueErrorsRuleTests.ts b/tests/ChaiVagueErrorsRuleTests.ts
@@ -275,6 +275,69 @@ describe('chaiVagueErrorsRule', () : void => {
         ]);
     });
 
+    it('should fail on strictly equality in expectation', () : void => {
+        const script : string = `
+            expect(something === undefined).to.equal(true, 'something should not have been set');
+            chai.expect(something === undefined).to.equal(true, 'something should not have been set');
+            expect(something !== undefined).to.equal(false, 'something should not have been set');
+            chai.expect(something !== undefined).to.equal(false, 'something should not have been set');
+        `;
+
+        TestHelper.assertViolations(ruleName, script, [
+            {
+                "failure": "Found chai call with vague failure message. " +
+                    "Move the strict equality comparison from the expect call into the assertion value",
+                "name": "file.ts",
+                "ruleName": "chai-vague-errors",
+                "startPosition": { "character": 13, "line": 2 }
+            },
+            {
+                "failure": "Found chai call with vague failure message. " +
+                    "Move the strict equality comparison from the expect call into the assertion value",
+                "name": "file.ts",
+                "ruleName": "chai-vague-errors",
+                "startPosition": { "character": 13, "line": 3 }
+            },
+            {
+                "failure": "Found chai call with vague failure message. " +
+                    "Move the strict inequality comparison from the expect call into the assertion value. ",
+                "name": "file.ts",
+                "ruleName": "chai-vague-errors",
+                "startPosition": { "character": 13, "line": 4 }
+            },
+            {
+                "failure": "Found chai call with vague failure message. " +
+                    "Move the strict inequality comparison from the expect call into the assertion value. ",
+                "name": "file.ts",
+                "ruleName": "chai-vague-errors",
+                "startPosition": { "character": 13, "line": 5 }
+            }
+        ]);
+    });
+
+    it('should fail on strictly in-equality in expectation', () : void => {
+        const script : string = `
+            expect(something !== undefined).to.equal(true, 'something should not have been set');
+            expect(something === undefined).to.equal(false, 'something should not have been set');
+        `;
+
+        TestHelper.assertViolations(ruleName, script, [
+            {
+                "failure": "Found chai call with vague failure message. " +
+                    "Move the strict inequality comparison from the expect call into the assertion value. ",
+                "name": "file.ts",
+                "ruleName": "chai-vague-errors",
+                "startPosition": { "character": 13, "line": 2 }
+            },
+            {
+                "failure": "Found chai call with vague failure message. " +
+                    "Move the strict equality comparison from the expect call into the assertion value",
+                "name": "file.ts",
+                "ruleName": "chai-vague-errors",
+                "startPosition": { "character": 13, "line": 3 }
+            }
+        ]);
+    });
 });
 /* tslint:enable:quotemark */
 /* tslint:enable:no-multiline-string */