Issue-207 allow second parameter of expect to satisfy documentation r…

…ule (#460) (first PR... hopefully did it right). Hopefully this fixes this issue: Fixes #207
microsoft · Jul 20, 2018 · f3a6adc · f3a6adc
1 parent aaf6946
commit f3a6adc
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 10 deletions.
diff --git a/src/chaiVagueErrorsRule.ts b/src/chaiVagueErrorsRule.ts
@@ -1,9 +1,9 @@
 import * as ts from 'typescript';
 import * as Lint from 'tslint';
 
-import {ErrorTolerantWalker} from './utils/ErrorTolerantWalker';
-import {ChaiUtils} from './utils/ChaiUtils';
-import {ExtendedMetadata} from './utils/ExtendedMetadata';
+import { ErrorTolerantWalker } from './utils/ErrorTolerantWalker';
+import { ChaiUtils } from './utils/ChaiUtils';
+import { ExtendedMetadata } from './utils/ExtendedMetadata';
 
 const BASE_ERROR: string = 'Found chai call with vague failure message. ';
 const FAILURE_STRING: string = BASE_ERROR + 'Please add an explicit failure message';
@@ -41,7 +41,10 @@ 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.addFailureAt(node.getStart(), node.getWidth(), FAILURE_STRING);
+                const expectInvocation: ts.CallExpression = ChaiUtils.getExpectInvocation(node);
+                if (!expectInvocation || expectInvocation.arguments.length !== 2) {
+                    this.addFailureAt(node.getStart(), node.getWidth(), FAILURE_STRING);
+                }
             }
         }
         super.visitPropertyAccessExpression(node);

diff --git a/src/tests/ChaiVagueErrorsRuleTests.ts b/src/tests/ChaiVagueErrorsRuleTests.ts
@@ -8,10 +8,21 @@ describe('chaiVagueErrorsRule', () : void => {
 
     it('should pass on xxx', () : void => {
         const script : string = `
-            expect(something).to.equal(true, 'message');;
-            expect(something).to.be.equal(false, 'message');;
-            expect(something).to.not.equal(null, 'message');;
-            expect(something).to.not.be.equal(undefined, 'message');;
+            expect(something).to.equal(true, 'message');
+            expect(something).to.be.equal(false, 'message');
+            expect(something).to.not.equal(null, 'message');
+            expect(something).to.not.be.equal(undefined, 'message');
+        `;
+
+        TestHelper.assertViolations(ruleName, script, [ ]);
+    });
+
+    it('should pass on xxx fluent', () : void => {
+        const script : string = `
+            expect(something, 'message').to.be.true;
+            expect(something, 'message').to.be.false;
+            expect(something, 'message').to.not.be.null;
+            expect(something, 'message').to.not.be.undefined;
         `;
 
         TestHelper.assertViolations(ruleName, script, [ ]);
@@ -269,7 +280,7 @@ describe('chaiVagueErrorsRule', () : void => {
         ]);
     });
 
-    it('should fail on strictly equality in expectation', () : void => {
+    it('should fail on strict 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');

diff --git a/src/utils/ChaiUtils.ts b/src/utils/ChaiUtils.ts
@@ -10,16 +10,34 @@ export module ChaiUtils {
         if (callExpression == null) {
             return false;
         }
+
         return /.*\.?expect/.test(callExpression.expression.getText());
     }
 
-    export function getLeftMostCallExpression(node: ts.PropertyAccessExpression | ts.CallExpression): ts.CallExpression {
+    export function getExpectInvocation(node: ts.PropertyAccessExpression | ts.CallExpression): ts.CallExpression {
+        const callExpression: ts.CallExpression = getLeftMostCallExpression(node, false);
+        if (callExpression == null) {
+            return null;
+        }
+
+        if (/.*\.?expect/.test(callExpression.expression.getText())) {
+            return callExpression;
+        } else {
+            return null;
+        }
+    }
+
+    export function getLeftMostCallExpression(
+        node: ts.PropertyAccessExpression | ts.CallExpression,
+        checkParent: boolean = false): ts.CallExpression {
         let leftSide: ts.Node = node.expression;
         while (leftSide != null) {
             if (leftSide.kind === ts.SyntaxKind.CallExpression) {
                 return <ts.CallExpression>leftSide;
             } else if (leftSide.kind === (ts.SyntaxKind.PropertyAccessExpression)) {
                 leftSide = (<ts.PropertyAccessExpression>leftSide).expression;
+            } else if (checkParent && leftSide.parent.kind === ts.SyntaxKind.CallExpression) {
+                return <ts.CallExpression>leftSide.parent;
             } else {
                 return null; // cannot walk any further left in the property expression
             }