@rushstack/eslint-plugin - npm Package Compare versions

Comparing version 0.6.0 to 0.6.1

CHANGELOG.json

@@ -5,2 +5,14 @@ {
     {
+      "version": "0.6.1",
+      "tag": "@rushstack/eslint-plugin_v0.6.1",
+      "date": "Thu, 27 Aug 2020 11:27:06 GMT",
+      "comments": {
+        "patch": [
+          {
+            "comment": "Revise the \"@rushstack/hoist-jest-mock\" rule to allow some common Jest coding practices that are not problematic"
+          }
+        ]
+      }
+    },
+    {
       "version": "0.6.0",
@@ -7,0 +19,0 @@ "tag": "@rushstack/eslint-plugin_v0.6.0",

CHANGELOG.md

 # Change Log - @rushstack/eslint-plugin
 
-This log was last generated on Mon, 24 Aug 2020 07:35:20 GMT and should not be manually modified.
+This log was last generated on Thu, 27 Aug 2020 11:27:06 GMT and should not be manually modified.
 
+## 0.6.1
+Thu, 27 Aug 2020 11:27:06 GMT
+
+### Patches
+
+- Revise the "@rushstack/hoist-jest-mock" rule to allow some common Jest coding practices that are not problematic
+
 ## 0.6.0
@@ -6,0 +13,0 @@ Mon, 24 Aug 2020 07:35:20 GMT

lib/hoist-jest-mock.js

@@ -16,4 +16,4 @@ "use strict";
         messages: {
-            'error-unhoisted-jest-mock': "Jest's module mocking APIs must be called before their associated module is imported. " +
-                ' Move this statement to the top of its code block.'
+            'error-unhoisted-jest-mock': "Jest's module mocking APIs must be called before regular imports. Move this call so that it precedes" +
+                ' the import found on line {{importLine}}.'
         },
@@ -27,3 +27,3 @@ schema: [
         docs: {
-            description: 'Require Jest module mocking APIs to be called before any other statements in their code block.' +
+            description: 'Require Jest module mocking APIs to be called before other modules are imported.' +
                 ' Jest module mocking APIs such as "jest.mock(\'./example\')" must be called before the associated module' +
@@ -40,2 +40,4 @@ ' is imported, otherwise they will have no effect. Transpilers such as ts-jest and babel-jest automatically' +
     create: (context) => {
+        // Returns true for a statement such as "jest.mock()" that needs to precede
+        // module imports (i.e. be "hoisted").
         function isHoistableJestCall(node) {
@@ -62,25 +64,77 @@ if (node === undefined) {
         }
-        function isHoistableJestStatement(node) {
-            switch (node.type) {
-                case experimental_utils_1.AST_NODE_TYPES.ExpressionStatement:
-                    return isHoistableJestCall(node.expression);
+        // Given part of an expression, walk upwards in the tree and find the containing statement
+        function findOuterStatement(node) {
+            let current = node;
+            while (current.parent) {
+                switch (current.parent.type) {
+                    // Statements are always found inside a block:
+                    case experimental_utils_1.AST_NODE_TYPES.Program:
+                    case experimental_utils_1.AST_NODE_TYPES.BlockStatement:
+                    case experimental_utils_1.AST_NODE_TYPES.TSModuleBlock:
+                        return current;
+                }
+                current = current.parent;
             }
-            return false;
+            return node;
         }
+        // This tracks the first require() or import expression that we found in the file.
+        let firstImportNode = undefined;
+        // Avoid reporting more than one error for a given statement.
+        // Example: jest.mock('a').mock('b');
+        const reportedStatements = new Set();
         return {
-            'TSModuleBlock, BlockStatement, Program': (node) => {
-                let encounteredRegularStatements = false;
-                for (const statement of node.body) {
-                    if (isHoistableJestStatement(statement)) {
-                        // Are we still at the start of the block?
-                        if (encounteredRegularStatements) {
-                            context.report({ node: statement, messageId: 'error-unhoisted-jest-mock' });
+            CallExpression: (node) => {
+                if (firstImportNode === undefined) {
+                    // EXAMPLE:  const x = require('x')
+                    if (matchTree_1.matchTree(node, hoistJestMockPatterns.requireCallExpression)) {
+                        firstImportNode = node;
+                    }
+                }
+                if (firstImportNode) {
+                    // EXAMPLE:  jest.mock()
+                    if (isHoistableJestCall(node)) {
+                        const outerStatement = findOuterStatement(node);
+                        if (!reportedStatements.has(outerStatement)) {
+                            reportedStatements.add(outerStatement);
+                            debugger;
+                            context.report({
+                                node,
+                                messageId: 'error-unhoisted-jest-mock',
+                                data: { importLine: firstImportNode.loc.start.line }
+                            });
                         }
                     }
-                    else {
-                        // We encountered a non-hoistable statement, so any further children that we visit
-                        // must also be non-hoistable
-                        encounteredRegularStatements = true;
+                }
+            },
+            ImportExpression: (node) => {
+                if (firstImportNode === undefined) {
+                    // EXAMPLE:  const x = import('x');
+                    if (matchTree_1.matchTree(node, hoistJestMockPatterns.importExpression)) {
+                        firstImportNode = node;
                     }
                 }
+            },
+            ImportDeclaration: (node) => {
+                if (firstImportNode === undefined) {
+                    // EXAMPLE:  import { X } from "Y";
+                    // IGNORE:   import type { X } from "Y";
+                    if (node.importKind !== 'type') {
+                        firstImportNode = node;
+                    }
+                }
+            },
+            ExportDeclaration: (node) => {
+                if (firstImportNode === undefined) {
+                    // EXAMPLE: export * from "Y";
+                    // IGNORE:  export type { Y } from "Y";
+                    if (node['exportKind'] !== 'type') {
+                        firstImportNode = node;
+                    }
+                }
+            },
+            TSImportEqualsDeclaration: (node) => {
+                if (firstImportNode === undefined) {
+                    // EXAMPLE:  import x = require("x");
+                    firstImportNode = node;
+                }
             }
@@ -87,0 +141,0 @@ };

lib/hoist-jest-mock.test.js

@@ -9,7 +9,11 @@ "use strict";
 const ruleTester = new RuleTester({
-    parser: '@typescript-eslint/parser'
+    parser: '@typescript-eslint/parser',
+    parserOptions: {
+        ecmaVersion: 2018,
+        sourceType: 'module'
+    }
 });
 // These are the CODE_WITH_HOISTING cases from ts-jest's hoist-jest.spec.ts
 const INVALID_EXAMPLE_CODE = [
-    /* 001 */ "const foo = 'foo'",
+    /* 001 */ "require('foo')",
     /* 002 */ 'console.log(foo)',
@@ -46,38 +50,6 @@ /* 003 */ 'jest.enableAutomock()',
 ].join('\n');
-const VALID_EXAMPLE_CODE = [
-    /* 001 */ 'jest.enableAutomock()',
-    /* 002 */ 'jest.disableAutomock()',
-    /* 003 */ "jest.mock('./foo')",
-    /* 004 */ "jest.mock('./foo/bar', () => 'bar')",
-    /* 005 */ "jest.unmock('./bar/foo').dontMock('./bar/bar')",
-    /* 006 */ "jest.deepUnmock('./foo')",
-    /* 007 */ "jest.mock('./foo').mock('./bar')",
-    /* 008 */ "const foo = 'foo'",
-    /* 009 */ 'console.log(foo)',
-    /* 010 */ 'const func = () => {',
-    /* 011 */ "  jest.unmock('./foo')",
-    /* 012 */ "  jest.mock('./bar')",
-    /* 013 */ "  jest.mock('./bar/foo', () => 'foo')",
-    /* 014 */ "  jest.unmock('./foo/bar')",
-    /* 015 */ "  jest.unmock('./bar/foo').dontMock('./bar/bar')",
-    /* 016 */ "  jest.deepUnmock('./bar')",
-    /* 017 */ "  jest.mock('./foo').mock('./bar')",
-    /* 018 */ "  const bar = 'bar'",
-    /* 019 */ '  console.log(bar)',
-    /* 020 */ '}',
-    /* 021 */ 'const func2 = () => {',
-    /* 022 */ "  jest.mock('./bar')",
-    /* 023 */ "  jest.unmock('./foo/bar')",
-    /* 024 */ "  jest.mock('./bar/foo', () => 'foo')",
-    /* 025 */ "  jest.unmock('./foo')",
-    /* 026 */ "  jest.unmock('./bar/foo').dontMock('./bar/bar')",
-    /* 027 */ "  jest.deepUnmock('./bar')",
-    /* 038 */ "  jest.mock('./foo').mock('./bar')",
-    /* 029 */ "  const bar = 'bar'",
-    /* 030 */ '  console.log(bar)',
-    /* 031 */ '}'
-].join('\n');
 ruleTester.run('hoist-jest-mock', hoist_jest_mock_1.hoistJestMock, {
     invalid: [
         {
+            // Detect all the Jest APIs detected by ts-jest
             code: INVALID_EXAMPLE_CODE,
@@ -107,6 +79,69 @@ errors: [
             ]
+        },
+        {
+            // A simple failure using realistic code
+            // prettier-ignore
+            code: [
+                "const soundPlayer = require('./SoundPlayer');",
+                "jest.mock('./SoundPlayer');"
+            ].join('\n'),
+            errors: [{ messageId: 'error-unhoisted-jest-mock', line: 2 }]
+        },
+        {
+            // Import syntaxes that should fail
+            code: ["import x from 'y';", 'jest.mock();'].join('\n'),
+            errors: [{ messageId: 'error-unhoisted-jest-mock', line: 2 }]
+        },
+        // {
+        //   // Import syntaxes that should fail
+        //   code: ["export { x } from 'y';", 'jest.mock();'].join('\n'),
+        //   errors: [{ messageId: 'error-unhoisted-jest-mock', line: 2 }]
+        // },
+        {
+            // Import syntaxes that should fail
+            code: ["import * as x from 'y';", 'jest.mock();'].join('\n'),
+            errors: [{ messageId: 'error-unhoisted-jest-mock', line: 2 }]
+        },
+        // {
+        //   // Import syntaxes that should fail
+        //   code: ["export * from 'y';", 'jest.mock();'].join('\n'),
+        //   errors: [{ messageId: 'error-unhoisted-jest-mock', line: 2 }]
+        // },
+        {
+            // Import syntaxes that should fail
+            code: ["import 'y';", 'jest.mock();'].join('\n'),
+            errors: [{ messageId: 'error-unhoisted-jest-mock', line: 2 }]
+        },
+        {
+            // Import syntaxes that should fail
+            code: ["const x = require('package-name');", 'jest.mock();'].join('\n'),
+            errors: [{ messageId: 'error-unhoisted-jest-mock', line: 2 }]
+        },
+        {
+            // Import syntaxes that should fail
+            code: ["const x = import('package-name');", 'jest.mock();'].join('\n'),
+            errors: [{ messageId: 'error-unhoisted-jest-mock', line: 2 }]
+        },
+        {
+            // Import syntaxes that should fail
+            code: ["import x = require('package-name');", 'jest.mock();'].join('\n'),
+            errors: [{ messageId: 'error-unhoisted-jest-mock', line: 2 }]
         }
     ],
-    valid: [{ code: VALID_EXAMPLE_CODE }]
+    valid: [
+        {
+            // A simple success using realistic code
+            code: [
+                'const mockPlaySoundFile = jest.fn();',
+                "jest.mock('./SoundPlayer', () => {",
+                '  return {',
+                '    SoundPlayer: jest.fn().mockImplementation(() => {',
+                '      return { playSoundFile: mockPlaySoundFile };',
+                '    })',
+                '  };',
+                '});'
+            ].join('\n')
+        }
+    ]
 });
 //# sourceMappingURL=hoist-jest-mock.test.js.map

lib/hoistJestMockPatterns.d.ts

@@ -18,2 +18,15 @@ export interface IJestCallExpression {
 };
+export declare const requireCallExpression: {
+    type: string;
+    callee: {
+        type: string;
+        name: string;
+    };
+};
+export declare const importExpression: {
+    type: string;
+    source: {
+        type: string;
+    };
+};
 //# sourceMappingURL=hoistJestMockPatterns.d.ts.map

lib/hoistJestMockPatterns.js

@@ -5,3 +5,3 @@ "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
-exports.jestCallExpression = void 0;
+exports.importExpression = exports.requireCallExpression = exports.jestCallExpression = void 0;
 const matchTree_1 = require("./matchTree");
@@ -46,2 +46,19 @@ // Matches a statement expression like this:
 };
+// Matches require() in a statement expression like this:
+//   const x = require("package-name");
+exports.requireCallExpression = {
+    type: 'CallExpression',
+    callee: {
+        type: 'Identifier',
+        name: 'require'
+    }
+};
+// Matches import in a statement expression like this:
+//   const x = import("package-name");
+exports.importExpression = {
+    type: 'ImportExpression',
+    source: {
+        type: 'Literal'
+    }
+};
 //# sourceMappingURL=hoistJestMockPatterns.js.map

lib/matchTree.d.ts

@@ -10,3 +10,3 @@ export declare type IMatchTreeCaptureSet = {
  */
-export declare function matchTree(root: any, pattern: any, captures: IMatchTreeCaptureSet): boolean;
+export declare function matchTree(root: any, pattern: any, captures?: IMatchTreeCaptureSet): boolean;
 /**
@@ -13,0 +13,0 @@ * Used to build the `pattern` tree for `matchTree()`.  For the given `subtree` of the pattern,

lib/matchTree.js

@@ -101,3 +101,3 @@ "use strict";
  */
-function matchTree(root, pattern, captures) {
+function matchTree(root, pattern, captures = {}) {
     return matchTreeRecursive(root, pattern, captures, 'root');
@@ -104,0 +104,0 @@ }

package.json

 {
 	"name": "@rushstack/eslint-plugin",
-	"version": "0.6.0",
+	"version": "0.6.1",
 	"description": "An ESLint plugin providing supplementary rules for use with the @rushstack/eslint-config package",
@@ -5,0 +5,0 @@ "repository": {

README.md

@@ -29,7 +29,6 @@ # @rushstack/eslint-plugin
 ```ts
-import * as file from './file';
+import * as file from './file'; // import statement
 jest.mock('./file'); // error
 
 test("example", () => {
-  const file2: typeof import('./file2') = require('./file2');
   jest.mock('./file2'); // error
@@ -39,12 +38,21 @@ });
 
+```ts
+require('./file'); // import statement
+jest.mock('./file'); // error
+```
+
 The following patterns are NOT considered problems:
 
 ```ts
-jest.mock('./file'); // okay, because mock() is first
-import * as file from './file';
+jest.mock('./file'); // okay, because mock() precedes the import below
+import * as file from './file'; // import statement
+```
 
-test("example", () => {
-  jest.mock('./file2'); // okay, because mock() is first within the test() code block
-  const file2: typeof import('./file2') = require('./file2');
-});
+```ts
+// These statements are not real "imports" because they import compile-time types
+// without any runtime effects
+import type { X } from './file';
+let y: typeof import('./file');
+
+jest.mock('./file'); // okay
 ```
@@ -51,0 +59,0 @@

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

110476

increased by11.41%

Lines of code

1073

increased by13.91%

Number of lines in readme file

227

increased by3.65%

No dependency changes