Update: deprecate Node.js & CommonJS rules (#12898)

* Update: deprecate Node.js & CommonJS rules * Upgrade eslint-plugin-node@11 * Update config/replaceBy fields * Fix lint errors * Update .eslintrc.js * Fix tests * Disallow directory imports * Address feedback * Fix typo
eslint · Apr 7, 2020 · e7c1d4b · e7c1d4b
1 parent 95e1c70
commit e7c1d4b
Show file tree

Hide file tree

Showing 30 changed files with 180 additions and 95 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -1,13 +1,44 @@
 "use strict";
 
-const internalFiles = [
-    "**/cli-engine/**/*",
-    "**/init/**/*",
-    "**/linter/**/*",
-    "**/rule-tester/**/*",
-    "**/rules/**/*",
-    "**/source-code/**/*"
-];
+const path = require("path");
+
+const INTERNAL_FILES = {
+    CLI_ENGINE_PATTERN: "lib/cli-engine/**/*",
+    INIT_PATTERN: "lib/init/**/*",
+    LINTER_PATTERN: "lib/linter/**/*",
+    RULE_TESTER_PATTERN: "lib/rule-tester/**/*",
+    RULES_PATTERN: "lib/rules/**/*",
+    SOURCE_CODE_PATTERN: "lib/source-code/**/*"
+};
+
+/**
+ * Resolve an absolute path or glob pattern.
+ * @param {string} pathOrPattern the path or glob pattern.
+ * @returns {string} The resolved path or glob pattern.
+ */
+function resolveAbsolutePath(pathOrPattern) {
+    return path.resolve(__dirname, pathOrPattern);
+}
+
+/**
+ * Create an array of `no-restricted-require` entries for ESLint's core files.
+ * @param {string} [pattern] The glob pattern to create the entries for.
+ * @returns {Object[]} The array of `no-restricted-require` entries.
+ */
+function createInternalFilesPatterns(pattern = null) {
+    return Object.values(INTERNAL_FILES)
+        .filter(p => p !== pattern)
+        .map(p => ({
+            name: [
+
+                // Disallow all children modules.
+                resolveAbsolutePath(p),
+
+                // Allow the main `index.js` module.
+                `!${resolveAbsolutePath(p.replace(/\*\*\/\*$/u, "index.js"))}`
+            ]
+        }));
+}
 
 module.exports = {
     root: true,
@@ -84,106 +115,90 @@ module.exports = {
         {
             files: ["lib/*"],
             rules: {
-                "no-restricted-modules": ["error", {
-                    patterns: [
-                        ...internalFiles
-                    ]
-                }]
+                "node/no-restricted-require": ["error", [
+                    ...createInternalFilesPatterns()
+                ]]
             }
         },
         {
-            files: ["lib/cli-engine/**/*"],
+            files: [INTERNAL_FILES.CLI_ENGINE_PATTERN],
             rules: {
-                "no-restricted-modules": ["error", {
-                    patterns: [
-                        ...internalFiles,
-                        "**/init"
-                    ]
-                }]
+                "node/no-restricted-require": ["error", [
+                    ...createInternalFilesPatterns(INTERNAL_FILES.CLI_ENGINE_PATTERN),
+                    resolveAbsolutePath("lib/init/index.js")
+                ]]
             }
         },
         {
-            files: ["lib/init/**/*"],
+            files: [INTERNAL_FILES.INIT_PATTERN],
             rules: {
-                "no-restricted-modules": ["error", {
-                    patterns: [
-                        ...internalFiles,
-                        "**/rule-tester"
-                    ]
-                }]
+                "node/no-restricted-require": ["error", [
+                    ...createInternalFilesPatterns(INTERNAL_FILES.INIT_PATTERN),
+                    resolveAbsolutePath("lib/rule-tester/index.js")
+                ]]
             }
         },
         {
-            files: ["lib/linter/**/*"],
+            files: [INTERNAL_FILES.LINTER_PATTERN],
             rules: {
-                "no-restricted-modules": ["error", {
-                    patterns: [
-                        ...internalFiles,
-                        "fs",
-                        "**/cli-engine",
-                        "**/init",
-                        "**/rule-tester"
-                    ]
-                }]
+                "node/no-restricted-require": ["error", [
+                    ...createInternalFilesPatterns(INTERNAL_FILES.LINTER_PATTERN),
+                    "fs",
+                    resolveAbsolutePath("lib/cli-engine/index.js"),
+                    resolveAbsolutePath("lib/init/index.js"),
+                    resolveAbsolutePath("lib/rule-tester/index.js")
+                ]]
             }
         },
         {
-            files: ["lib/rules/**/*"],
+            files: [INTERNAL_FILES.RULES_PATTERN],
             rules: {
-                "no-restricted-modules": ["error", {
-                    patterns: [
-                        ...internalFiles,
-                        "fs",
-                        "**/cli-engine",
-                        "**/init",
-                        "**/linter",
-                        "**/rule-tester",
-                        "**/source-code"
-                    ]
-                }]
+                "node/no-restricted-require": ["error", [
+                    ...createInternalFilesPatterns(INTERNAL_FILES.RULES_PATTERN),
+                    "fs",
+                    resolveAbsolutePath("lib/cli-engine/index.js"),
+                    resolveAbsolutePath("lib/init/index.js"),
+                    resolveAbsolutePath("lib/linter/index.js"),
+                    resolveAbsolutePath("lib/rule-tester/index.js"),
+                    resolveAbsolutePath("lib/source-code/index.js")
+                ]]
             }
         },
         {
             files: ["lib/shared/**/*"],
             rules: {
-                "no-restricted-modules": ["error", {
-                    patterns: [
-                        ...internalFiles,
-                        "**/cli-engine",
-                        "**/init",
-                        "**/linter",
-                        "**/rule-tester",
-                        "**/source-code"
-                    ]
-                }]
+                "node/no-restricted-require": ["error", [
+                    ...createInternalFilesPatterns(),
+                    resolveAbsolutePath("lib/cli-engine/index.js"),
+                    resolveAbsolutePath("lib/init/index.js"),
+                    resolveAbsolutePath("lib/linter/index.js"),
+                    resolveAbsolutePath("lib/rule-tester/index.js"),
+                    resolveAbsolutePath("lib/source-code/index.js")
+                ]]
             }
         },
         {
-            files: ["lib/source-code/**/*"],
+            files: [INTERNAL_FILES.SOURCE_CODE_PATTERN],
             rules: {
-                "no-restricted-modules": ["error", {
-                    patterns: [
-                        ...internalFiles,
-                        "fs",
-                        "**/cli-engine",
-                        "**/init",
-                        "**/linter",
-                        "**/rule-tester",
-                        "**/rules"
-                    ]
-                }]
+                "node/no-restricted-require": ["error", [
+                    ...createInternalFilesPatterns(INTERNAL_FILES.SOURCE_CODE_PATTERN),
+                    "fs",
+                    resolveAbsolutePath("lib/cli-engine/index.js"),
+                    resolveAbsolutePath("lib/init/index.js"),
+                    resolveAbsolutePath("lib/linter/index.js"),
+                    resolveAbsolutePath("lib/rule-tester/index.js"),
+                    resolveAbsolutePath("lib/rules/index.js")
+                ]]
             }
         },
         {
-            files: ["lib/rule-tester/**/*"],
+            files: [INTERNAL_FILES.RULE_TESTER_PATTERN],
             rules: {
-                "no-restricted-modules": ["error", {
-                    patterns: [
-                        ...internalFiles,
-                        "**/cli-engine",
-                        "**/init"
-                    ]
-                }]
+                "node/no-restricted-require": ["error", [
+                    ...createInternalFilesPatterns(INTERNAL_FILES.RULE_TESTER_PATTERN),
+                    resolveAbsolutePath("lib/cli-engine/index.js"),
+                    resolveAbsolutePath("lib/init/index.js")
+                ]]
             }
         }
     ]

diff --git a/docs/rules/callback-return.md b/docs/rules/callback-return.md
@@ -1,5 +1,7 @@
 # Enforce Return After Callback (callback-return)
 
+This rule was **deprecated** in ESLint v7.0.0. Please use the corresponding rule in [`eslint-plugin-node`](https://github.com/mysticatea/eslint-plugin-node).
+
 The callback pattern is at the heart of most I/O and event-driven programming
  in JavaScript.
 

diff --git a/docs/rules/global-require.md b/docs/rules/global-require.md
@@ -1,5 +1,7 @@
 # Enforce require() on the top-level module scope (global-require)
 
+This rule was **deprecated** in ESLint v7.0.0. Please use the corresponding rule in [`eslint-plugin-node`](https://github.com/mysticatea/eslint-plugin-node).
+
 In Node.js, module dependencies are included using the `require()` function, such as:
 
 ```js

diff --git a/docs/rules/handle-callback-err.md b/docs/rules/handle-callback-err.md
@@ -1,5 +1,7 @@
 # Enforce Callback Error Handling (handle-callback-err)
 
+This rule was **deprecated** in ESLint v7.0.0. Please use the corresponding rule in [`eslint-plugin-node`](https://github.com/mysticatea/eslint-plugin-node).
+
 In Node.js, a common pattern for dealing with asynchronous behavior is called the callback pattern.
 This pattern expects an `Error` object or `null` as the first argument of the callback.
 Forgetting to handle these errors can lead to some really strange behavior in your application.

diff --git a/docs/rules/no-buffer-constructor.md b/docs/rules/no-buffer-constructor.md
@@ -1,5 +1,7 @@
 # disallow use of the Buffer() constructor (no-buffer-constructor)
 
+This rule was **deprecated** in ESLint v7.0.0. Please use the corresponding rule in [`eslint-plugin-node`](https://github.com/mysticatea/eslint-plugin-node).
+
 In Node.js, the behavior of the `Buffer` constructor is different depending on the type of its argument. Passing an argument from user input to `Buffer()` without validating its type can lead to security vulnerabilities such as remote memory disclosure and denial of service. As a result, the `Buffer` constructor has been deprecated and should not be used. Use the producer methods `Buffer.from`, `Buffer.alloc`, and `Buffer.allocUnsafe` instead.
 
 ## Rule Details

diff --git a/docs/rules/no-mixed-requires.md b/docs/rules/no-mixed-requires.md
@@ -1,5 +1,7 @@
 # disallow `require` calls to be mixed with regular variable declarations (no-mixed-requires)
 
+This rule was **deprecated** in ESLint v7.0.0. Please use the corresponding rule in [`eslint-plugin-node`](https://github.com/mysticatea/eslint-plugin-node).
+
 In the Node.js community it is often customary to separate initializations with calls to `require` modules from other variable declarations, sometimes also grouping them by the type of module. This rule helps you enforce this convention.
 
 ## Rule Details

diff --git a/docs/rules/no-new-require.md b/docs/rules/no-new-require.md
@@ -1,5 +1,7 @@
 # Disallow new require (no-new-require)
 
+This rule was **deprecated** in ESLint v7.0.0. Please use the corresponding rule in [`eslint-plugin-node`](https://github.com/mysticatea/eslint-plugin-node).
+
 The `require` function is used to include modules that exist in separate files, such as:
 
 ```js

diff --git a/docs/rules/no-path-concat.md b/docs/rules/no-path-concat.md
@@ -1,5 +1,7 @@
 # Disallow string concatenation when using `__dirname` and `__filename` (no-path-concat)
 
+This rule was **deprecated** in ESLint v7.0.0. Please use the corresponding rule in [`eslint-plugin-node`](https://github.com/mysticatea/eslint-plugin-node).
+
 In Node.js, the `__dirname` and `__filename` global variables contain the directory path and the file path of the currently executing script file, respectively. Sometimes, developers try to use these variables to create paths to other files, such as:
 
 ```js

diff --git a/docs/rules/no-process-env.md b/docs/rules/no-process-env.md
@@ -1,5 +1,7 @@
 # Disallow process.env (no-process-env)
 
+This rule was **deprecated** in ESLint v7.0.0. Please use the corresponding rule in [`eslint-plugin-node`](https://github.com/mysticatea/eslint-plugin-node).
+
 The `process.env` object in Node.js is used to store deployment/configuration parameters. Littering it through out a project could lead to maintenance issues as it's another kind of global dependency. As such, it could lead to merge conflicts in a multi-user setup and deployment issues in a multi-server setup. Instead, one of the best practices is to define all those parameters in a single configuration/settings file which could be accessed throughout the project.
 
 

diff --git a/docs/rules/no-process-exit.md b/docs/rules/no-process-exit.md
@@ -1,5 +1,7 @@
 # Disallow process.exit() (no-process-exit)
 
+This rule was **deprecated** in ESLint v7.0.0. Please use the corresponding rule in [`eslint-plugin-node`](https://github.com/mysticatea/eslint-plugin-node).
+
 The `process.exit()` method in Node.js is used to immediately stop the Node.js process and exit. This is a dangerous operation because it can occur in any method at any point in time, potentially stopping a Node.js application completely when an error occurs. For example:
 
 ```js

diff --git a/docs/rules/no-restricted-modules.md b/docs/rules/no-restricted-modules.md
@@ -1,5 +1,7 @@
 # Disallow Node.js modules (no-restricted-modules)
 
+This rule was **deprecated** in ESLint v7.0.0. Please use the corresponding rule in [`eslint-plugin-node`](https://github.com/mysticatea/eslint-plugin-node).
+
 A module in Node.js is a simple or complex functionality organized in a JavaScript file which can be reused throughout the Node.js
 application. The keyword `require` is used in Node.js/CommonJS to import modules into an application. This way you can have dynamic loading where the loaded module name isn't predefined /static, or where you conditionally load a module only if it's "truly required".
 

diff --git a/docs/rules/no-sync.md b/docs/rules/no-sync.md
@@ -1,5 +1,7 @@
 # Disallow Synchronous Methods (no-sync)
 
+This rule was **deprecated** in ESLint v7.0.0. Please use the corresponding rule in [`eslint-plugin-node`](https://github.com/mysticatea/eslint-plugin-node).
+
 In Node.js, most I/O is done through asynchronous methods. However, there are often synchronous versions of the asynchronous methods. For example, `fs.exists()` and `fs.existsSync()`. In some contexts, using synchronous operations is okay (if, as with ESLint, you are writing a command line utility). However, in other contexts the use of synchronous operations is considered a bad practice that should be avoided. For example, if you are running a high-travel web server on Node.js, you should consider carefully if you want to allow any synchronous operations that could lock up the server.
 
 ## Rule Details

diff --git a/lib/init/autoconfig.js b/lib/init/autoconfig.js
@@ -301,7 +301,7 @@ class Registry {
                 ruleSetIdx += 1;
 
                 if (cb) {
-                    cb(totalFilesLinting); // eslint-disable-line callback-return
+                    cb(totalFilesLinting); // eslint-disable-line node/callback-return
                 }
             });
 

diff --git a/lib/init/source-code-utils.js b/lib/init/source-code-utils.js
@@ -23,7 +23,7 @@ const { CLIEngine } = require("../cli-engine");
  * TODO1: Expose the API that enumerates target files.
  * TODO2: Extract the creation logic of `SourceCode` from `Linter` class.
  */
-const { getCLIEngineInternalSlots } = require("../cli-engine/cli-engine"); // eslint-disable-line no-restricted-modules
+const { getCLIEngineInternalSlots } = require("../cli-engine/cli-engine"); // eslint-disable-line node/no-restricted-require
 
 const debug = require("debug")("eslint:source-code-utils");
 
@@ -97,7 +97,7 @@ function getSourceCodeOfFiles(patterns, options, callback) {
             sourceCodes[filename] = sourceCode;
         }
         if (callback) {
-            callback(filenames.length); // eslint-disable-line callback-return
+            callback(filenames.length); // eslint-disable-line node/callback-return
         }
     });
 

diff --git a/lib/rules/callback-return.js b/lib/rules/callback-return.js
@@ -10,6 +10,10 @@
 
 module.exports = {
     meta: {
+        deprecated: true,
+
+        replacedBy: ["node/callback-return"],
+
         type: "suggestion",
 
         docs: {

diff --git a/lib/rules/global-require.js b/lib/rules/global-require.js
@@ -48,6 +48,10 @@ function isShadowed(scope, node) {
 
 module.exports = {
     meta: {
+        deprecated: true,
+
+        replacedBy: ["node/global-require"],
+
         type: "suggestion",
 
         docs: {

diff --git a/lib/rules/handle-callback-err.js b/lib/rules/handle-callback-err.js
@@ -11,6 +11,10 @@
 
 module.exports = {
     meta: {
+        deprecated: true,
+
+        replacedBy: ["node/handle-callback-err"],
+
         type: "suggestion",
 
         docs: {

diff --git a/lib/rules/no-buffer-constructor.js b/lib/rules/no-buffer-constructor.js
@@ -10,6 +10,10 @@
 
 module.exports = {
     meta: {
+        deprecated: true,
+
+        replacedBy: ["node/no-deprecated-api"],
+
         type: "problem",
 
         docs: {

diff --git a/lib/rules/no-mixed-requires.js b/lib/rules/no-mixed-requires.js
@@ -11,6 +11,10 @@
 
 module.exports = {
     meta: {
+        deprecated: true,
+
+        replacedBy: ["node/no-mixed-requires"],
+
         type: "suggestion",
 
         docs: {

diff --git a/lib/rules/no-new-require.js b/lib/rules/no-new-require.js
@@ -11,6 +11,10 @@
 
 module.exports = {
     meta: {
+        deprecated: true,
+
+        replacedBy: ["node/no-new-require"],
+
         type: "suggestion",
 
         docs: {