feat: add expect-should-assertion rule (#12)

README.md

@@ -1,6 +1,6 @@
 # eslint-plugin-shouldjs
 
-Rules that apply to testing with the [should-js](https://shouldjs.github.io/) library.
+Rules that apply to testing with the [should.js](https://shouldjs.github.io/) library.
 
 ## Installation
 
@@ -30,7 +30,7 @@ Add `@jaredmcateer/shouldjs` to the plugins section of your `.eslintrc` configur
 
 ### Settings
 
-By default the only allowed variable name for should is `should`, this can be changed by providing an array to `shouldVarNames` in the eslint settings.
+By default the only allowed variable name for Should.js is `should`, this can be changed by providing an array to `shouldVarNames` in the eslint settings.
 
 ```json
 {
@@ -65,6 +65,7 @@ Alternative you can use the recommended settings
 
 - [should-var-names](lib/rules/should-var-name/should-var-name.md)
 - [no-property-assertion](lib/rules/no-property-assertions/no-property-assertion.md)
+- [expect-should-assertion](lib/rules/expect-should-assertion/expect-should-assertions.md)
 
 ## Acknowledgements
 

lib/configs/recommended.ts

@@ -1,4 +1,5 @@
 import { ConfigSettings } from ".";
+import { EXPECT_SHOULD_ASSERTION } from "../rules/expect-should-assertion/expect-should-assertions";
 import { NO_PROPERTY_ASSERTIONS } from "../rules/no-property-assertions/no-property-assertions";
 import { SHOULD_VAR_NAME } from "../rules/should-var-name/should-var-name";
 
@@ -8,6 +9,7 @@ export const recommended = {
   rules: {
     [`@jaredmcateer/shouldjs/${NO_PROPERTY_ASSERTIONS}`]: ["error"],
     [`@jaredmcateer/shouldjs/${SHOULD_VAR_NAME}`]: ["error"],
+    [`@jaredmcateer/shouldjs/${EXPECT_SHOULD_ASSERTION}`]: ["error"],
   },
 
   settings: {

lib/rules/expect-should-assertion/expect-should-assertions.md

@@ -0,0 +1,27 @@
+# expect-should-assertion
+
+The `"extends": "@jaredmcateer/shouldjs:recommended"` property in a configuration file enables this rule.
+
+Disallows should function variable to be called without chaining an assertion.
+
+Should.js does not assert solely by calling the function variable.
+
+You can configure the `shouldVarNames` in the `settings` property of the eslint config to limit which variable names will be checked.
+
+## Rule Details
+
+Examples of **incorrect** code for this rule:
+
+```js
+should(foo);
+```
+
+Examples of **correct** code for this rule:
+
+```js
+should(foo).be.truthy();
+```
+
+## When Not To Use It
+
+It is not recommended to turn this rule off.

lib/rules/expect-should-assertion/expect-should-assertions.test.ts

@@ -0,0 +1,25 @@
+import { ESLintUtils } from "@typescript-eslint/utils";
+import {
+  ADD_ASSERTION_MESSAGE,
+  expectShouldAssertion,
+  EXPECT_SHOULD_ASSERTION,
+} from "./expect-should-assertions";
+
+const ruleTester = new ESLintUtils.RuleTester({
+  parser: "@typescript-eslint/parser",
+});
+
+ruleTester.run(EXPECT_SHOULD_ASSERTION, expectShouldAssertion, {
+  valid: [
+    { code: "should(foo).be.true();" },
+    { code: "should(foo);", settings: { shouldVarNames: ["expect"] } },
+  ],
+  invalid: [
+    { code: "should(foo);", errors: [{ messageId: ADD_ASSERTION_MESSAGE }] },
+    {
+      code: "expect();",
+      settings: { shouldVarNames: ["expect"] },
+      errors: [{ messageId: ADD_ASSERTION_MESSAGE }],
+    },
+  ],
+});

lib/rules/expect-should-assertion/expect-should-assertions.ts

@@ -0,0 +1,52 @@
+import { AST_NODE_TYPES } from "@typescript-eslint/utils";
+import { ConfigSettings } from "../../configs";
+import { createRule } from "../../utils/create-rule";
+
+export const EXPECT_SHOULD_ASSERTION = "expect-should-assertion";
+export const ADD_ASSERTION_MESSAGE = "missingAssertionError";
+
+type MessageIds = typeof ADD_ASSERTION_MESSAGE;
+
+export const expectShouldAssertion = createRule<[], MessageIds>({
+  name: EXPECT_SHOULD_ASSERTION,
+  meta: {
+    docs: {
+      description: "Should.js function calls should be chained with an assertion.",
+      recommended: "error",
+      requiresTypeChecking: false,
+    },
+    messages: {
+      [ADD_ASSERTION_MESSAGE]: "Should.js function calls should be chained with an assertion.",
+    },
+    schema: [],
+    hasSuggestions: false,
+    type: "problem",
+  },
+  defaultOptions: [],
+  create(context) {
+    const validVarNames = (context.settings as ConfigSettings).shouldVarNames || ["should"];
+
+    return {
+      /**
+       * When encountering an CallExpression with expected name (i.e., should),
+       * Check the parents if it is an ExpressionStatement then report an error.
+       */
+      CallExpression(node) {
+        if (node?.callee?.type !== AST_NODE_TYPES.Identifier) return;
+
+        // If CallExpression isn't an expected variable name then stop here
+        const name = node.callee.name;
+        if (!validVarNames?.find((varName) => varName === name)) return;
+
+        // If we've hit an Expression statement it means that the Should.js
+        // function var is being called without an assertion.
+        if (node.parent?.type === AST_NODE_TYPES.ExpressionStatement) {
+          context.report({
+            messageId: ADD_ASSERTION_MESSAGE,
+            node,
+          });
+        }
+      },
+    };
+  },
+});

lib/rules/index.ts

@@ -1,3 +1,7 @@
+import {
+  expectShouldAssertion,
+  EXPECT_SHOULD_ASSERTION,
+} from "./expect-should-assertion/expect-should-assertions";
 import {
   noPropertyAssertions,
   NO_PROPERTY_ASSERTIONS,
@@ -7,4 +11,5 @@ import { shouldVarName, SHOULD_VAR_NAME } from "./should-var-name/should-var-nam
 export const rules = {
   [NO_PROPERTY_ASSERTIONS]: noPropertyAssertions,
   [SHOULD_VAR_NAME]: shouldVarName,
+  [EXPECT_SHOULD_ASSERTION]: expectShouldAssertion,
 };

lib/rules/no-property-assertions/no-property-assertion.md

@@ -2,9 +2,9 @@
 
 The `"extends": "@jaredmcateer/shouldjs:recommended"` property in a configuration file enables this rule.
 
-Disallows ending a should-js chain with a property instead of a method.
+Disallows ending a Should.js chain with a property instead of a method.
 
-Should-js does not use properties for assertions but it is a common gotcha with certain assertions.
+Should.js does not use properties for assertions but it is a common gotcha with certain assertions.
 
 When checking by the CallExpression you can configure the `shouldVarNames` in the `settings` property of the eslint config to limit which variable names will be checked.
 

lib/rules/no-property-assertions/no-property-assertion.test.ts

@@ -14,6 +14,8 @@ ruleTester.run(NO_PROPERTY_ASSERTIONS, noPropertyAssertions, {
     { code: "foo.should.be.true();" },
     { code: "should(foo).be.false();" },
     { code: "foo.should.not.have.been.eql(bar);" },
+    // This is an error but handled by another rule
+    { code: "should(false);" },
     { code: "myCustomVar(foo).should.be.eql(bar);", settings: { shouldVarNames: ["myCustomVar"] } },
   ],
   invalid: [

lib/rules/no-property-assertions/no-property-assertions.ts

@@ -14,12 +14,12 @@ export const noPropertyAssertions = createRule<[], MessageIds>({
   name: NO_PROPERTY_ASSERTIONS,
   meta: {
     docs: {
-      description: "Should-js assertions should be methods.",
+      description: "Should.js assertions should be methods.",
       recommended: "error",
       requiresTypeChecking: false,
     },
     messages: {
-      [PROPERTY_ASSERTION_ERROR]: "Should-js assertions should be methods.",
+      [PROPERTY_ASSERTION_ERROR]: "Should.js assertions should be methods.",
     },
     schema: [],
     hasSuggestions: false,
@@ -58,6 +58,13 @@ export const noPropertyAssertions = createRule<[], MessageIds>({
         const name = node.callee.name;
         if (!validVarNames?.find((varName) => varName === name)) return;
 
+        if (node.parent?.type === AST_NODE_TYPES.ExpressionStatement) {
+          // If we've hit an Expression statement it means that the Should.js
+          // function var is being called without an assertion, this is handled
+          // by the expect-should-assertion rule.
+          return;
+        }
+
         // CallExpression matches function variable name, begin traversing AST to check.
         checkChain(node.parent, context);
       },

lib/rules/should-var-name/should-var-name.md

@@ -2,7 +2,7 @@
 
 The `"extends": "@jaredmcateer/shouldjs:recommended"` property in a configuration file enables this rule.
 
-Disallows assigning a variable for should that isn't part of the approved list. This is set by the `shouldVarNames` array in settings property of the eslint config.
+Disallows assigning a variable for Should.js that isn't part of the approved list. This is set by the `shouldVarNames` array in settings property of the eslint config.
 
 ## Rule Details
 
@@ -32,4 +32,4 @@ import should from "should";
 
 ## When Not To Use It
 
-It is not recommended to turn this rule off. Other rules in the plugin rely on the variable name when using the should function to decide whether to lint by assigning it an unknown value those rules will not be run.
+It is not recommended to turn this rule off. Other rules in the plugin rely on the variable name when using the Should.js function to decide whether to lint by assigning it an unknown value those rules will not be run.

lib/rules/should-var-name/should-var-name.ts

@@ -14,12 +14,12 @@ export const shouldVarName = createRule<[], MessageIds>({
   name: SHOULD_VAR_NAME,
   meta: {
     docs: {
-      description: "Only allows the configured variable name for should-js variable name.",
+      description: "Only allows the configured variable name for Should.js variable name.",
       recommended: "error",
       requiresTypeChecking: false,
     },
     messages: {
-      [INVALID_VAR_NAME]: "Invalid variable name for should-js.",
+      [INVALID_VAR_NAME]: "Invalid variable name for Should.js.",
       [SUGGEST_FUNCTION_VAR_RENAME]: "Rename variable to: {{name}}",
     },
     schema: [],
@@ -60,7 +60,7 @@ export const shouldVarName = createRule<[], MessageIds>({
       CallExpression(node) {
         if (node?.callee?.type !== AST_NODE_TYPES.Identifier) return;
 
-        // If CallExpression isn't requiring should then stop here
+        // If CallExpression isn't requiring Should.js then stop here
         const isRequire = node.callee.name === "require";
         const isOneArg = isRequire && node.arguments.length === 1;
         const packageArg = isOneArg && node.arguments[0];
@@ -87,7 +87,7 @@ export const shouldVarName = createRule<[], MessageIds>({
       ImportDeclaration(node) {
         if (node?.source?.type !== AST_NODE_TYPES.Literal) return;
 
-        // If ImportDeclaration isn't requiring should then stop here
+        // If ImportDeclaration isn't requiring Should.js then stop here
         if (node.source.value !== "should") return;
         const defaultSpecifier = node.specifiers.find(
           (s) => s.type === AST_NODE_TYPES.ImportDefaultSpecifier

package.json

@@ -1,14 +1,15 @@
 {
   "name": "@jaredmcateer/eslint-plugin-shouldjs",
   "version": "1.0.0",
-  "description": "A collection of Should-js rules",
+  "description": "A collection of Should.js rules",
   "keywords": [
     "eslint",
     "eslintplugin",
     "eslint-plugin",
     "should",
     "shouldjs",
-    "should-js"
+    "should-js",
+    "should.js"
   ],
   "author": "Jared McAteer <jared.mcateer+npm@gmail.com>",
   "main": "dist/index.js",