feat: add rule no-object-constructor, deprecate no-new-object (#1…

…7576) * Deprecate `no-new-object` * Add rule `no-object-constructor` * Apply suggestions * mark no-new-object deprecated in v8.50.0 Co-authored-by: Francesco Trotta <github@fasttime.org> --------- Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
eslint · Sep 19, 2023 · 22a5582 · 22a5582
1 parent 48a44a7
commit 22a5582
Show file tree

Hide file tree

Showing 12 changed files with 323 additions and 13 deletions.
diff --git a/docs/src/_data/rules.json b/docs/src/_data/rules.json
@@ -950,13 +950,6 @@
                     "fixable": false,
                     "hasSuggestions": false
                 },
-                {
-                    "name": "no-new-object",
-                    "description": "Disallow `Object` constructors",
-                    "recommended": false,
-                    "fixable": false,
-                    "hasSuggestions": false
-                },
                 {
                     "name": "no-new-wrappers",
                     "description": "Disallow `new` operators with the `String`, `Number`, and `Boolean` objects",
@@ -971,6 +964,13 @@
                     "fixable": false,
                     "hasSuggestions": true
                 },
+                {
+                    "name": "no-object-constructor",
+                    "description": "Disallow calls to the `Object` constructor without an argument",
+                    "recommended": false,
+                    "fixable": false,
+                    "hasSuggestions": true
+                },
                 {
                     "name": "no-octal",
                     "description": "Disallow octal literals",
@@ -1956,6 +1956,12 @@
                     "no-unsafe-negation"
                 ]
             },
+            {
+                "name": "no-new-object",
+                "replacedBy": [
+                    "no-object-constructor"
+                ]
+            },
             {
                 "name": "no-new-require",
                 "replacedBy": []

diff --git a/docs/src/_data/rules_meta.json b/docs/src/_data/rules_meta.json
@@ -1364,7 +1364,11 @@
             "description": "Disallow `Object` constructors",
             "recommended": false,
             "url": "https://eslint.org/docs/latest/rules/no-new-object"
-        }
+        },
+        "deprecated": true,
+        "replacedBy": [
+            "no-object-constructor"
+        ]
     },
     "no-new-require": {
         "deprecated": true,
@@ -1409,6 +1413,15 @@
             "url": "https://eslint.org/docs/latest/rules/no-obj-calls"
         }
     },
+    "no-object-constructor": {
+        "type": "suggestion",
+        "docs": {
+            "description": "Disallow calls to the `Object` constructor without an argument",
+            "recommended": false,
+            "url": "https://eslint.org/docs/latest/rules/no-object-constructor"
+        },
+        "hasSuggestions": true
+    },
     "no-octal": {
         "type": "suggestion",
         "docs": {

diff --git a/docs/src/rules/no-array-constructor.md b/docs/src/rules/no-array-constructor.md
@@ -2,8 +2,8 @@
 title: no-array-constructor
 rule_type: suggestion
 related_rules:
-- no-new-object
 - no-new-wrappers
+- no-object-constructor
 ---
 
 

diff --git a/docs/src/rules/no-new-object.md b/docs/src/rules/no-new-object.md
@@ -6,6 +6,7 @@ related_rules:
 - no-new-wrappers
 ---
 
+This rule was **deprecated** in ESLint v8.50.0 and replaced by the [no-object-constructor](no-object-constructor) rule. The new rule flags more situations where object literal syntax can be used, and it does not report a problem when the `Object` constructor is invoked with an argument.
 
 The `Object` constructor is used to create new generic objects in JavaScript, such as:
 
@@ -25,7 +26,7 @@ While there are no performance differences between the two approaches, the byte
 
 ## Rule Details
 
-This rule disallows `Object` constructors.
+This rule disallows calling the `Object` constructor with `new`.
 
 Examples of **incorrect** code for this rule:
 
@@ -37,6 +38,8 @@ Examples of **incorrect** code for this rule:
 var myObject = new Object();
 
 new Object();
+
+var foo = new Object("foo");
 ```
 
 :::
@@ -54,10 +57,12 @@ var myObject = {};
 
 var Object = function Object() {};
 new Object();
+
+var foo = Object("foo");
 ```
 
 :::
 
 ## When Not To Use It
 
-If you wish to allow the use of the `Object` constructor, you can safely turn this rule off.
+If you wish to allow the use of the `Object` constructor with `new`, you can safely turn this rule off.
diff --git a/docs/src/rules/no-new-wrappers.md b/docs/src/rules/no-new-wrappers.md
@@ -3,7 +3,7 @@ title: no-new-wrappers
 rule_type: suggestion
 related_rules:
 - no-array-constructor
-- no-new-object
+- no-object-constructor
 further_reading:
 - https://www.inkling.com/read/javascript-definitive-guide-david-flanagan-6th/chapter-3/wrapper-objects
 ---

diff --git a/docs/src/rules/no-object-constructor.md b/docs/src/rules/no-object-constructor.md
@@ -0,0 +1,50 @@
+---
+title: no-object-constructor
+rule_type: suggestion
+related_rules:
+- no-array-constructor
+- no-new-wrappers
+---
+
+Use of the `Object` constructor to construct a new empty object is generally discouraged in favor of object literal notation because of conciseness and because the `Object` global may be redefined.
+The exception is when the `Object` constructor is used to intentionally wrap a specified value which is passed as an argument.
+
+## Rule Details
+
+This rule disallows calling the `Object` constructor without an argument.
+
+Examples of **incorrect** code for this rule:
+
+:::incorrect
+
+```js
+/*eslint no-object-constructor: "error"*/
+
+Object();
+
+new Object();
+```
+
+:::
+
+Examples of **correct** code for this rule:
+
+:::correct
+
+```js
+/*eslint no-object-constructor: "error"*/
+
+Object("foo");
+
+const obj = { a: 1, b: 2 };
+
+const isObject = value => value === Object(value);
+
+const createObject = Object => new Object();
+```
+
+:::
+
+## When Not To Use It
+
+If you wish to allow the use of the `Object` constructor, you can safely turn this rule off.
diff --git a/lib/rules/index.js b/lib/rules/index.js
@@ -175,6 +175,7 @@ module.exports = new LazyLoadingRuleMap(Object.entries({
     "no-new-wrappers": () => require("./no-new-wrappers"),
     "no-nonoctal-decimal-escape": () => require("./no-nonoctal-decimal-escape"),
     "no-obj-calls": () => require("./no-obj-calls"),
+    "no-object-constructor": () => require("./no-object-constructor"),
     "no-octal": () => require("./no-octal"),
     "no-octal-escape": () => require("./no-octal-escape"),
     "no-param-reassign": () => require("./no-param-reassign"),

diff --git a/lib/rules/no-new-object.js b/lib/rules/no-new-object.js
@@ -1,6 +1,7 @@
 /**
  * @fileoverview A rule to disallow calls to the Object constructor
  * @author Matt DuVall <http://www.mattduvall.com/>
+ * @deprecated in ESLint v8.50.0
  */
 
 "use strict";
@@ -26,6 +27,12 @@ module.exports = {
             url: "https://eslint.org/docs/latest/rules/no-new-object"
         },
 
+        deprecated: true,
+
+        replacedBy: [
+            "no-object-constructor"
+        ],
+
         schema: [],
 
         messages: {

diff --git a/lib/rules/no-object-constructor.js b/lib/rules/no-object-constructor.js
@@ -0,0 +1,118 @@
+/**
+ * @fileoverview Rule to disallow calls to the `Object` constructor without an argument
+ * @author Francesco Trotta
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const { getVariableByName, isArrowToken } = require("./utils/ast-utils");
+
+//------------------------------------------------------------------------------
+// Helpers
+//------------------------------------------------------------------------------
+
+/**
+ * Tests if a node appears at the beginning of an ancestor ExpressionStatement node.
+ * @param {ASTNode} node The node to check.
+ * @returns {boolean} Whether the node appears at the beginning of an ancestor ExpressionStatement node.
+ */
+function isStartOfExpressionStatement(node) {
+    const start = node.range[0];
+    let ancestor = node;
+
+    while ((ancestor = ancestor.parent) && ancestor.range[0] === start) {
+        if (ancestor.type === "ExpressionStatement") {
+            return true;
+        }
+    }
+    return false;
+}
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+/** @type {import('../shared/types').Rule} */
+module.exports = {
+    meta: {
+        type: "suggestion",
+
+        docs: {
+            description: "Disallow calls to the `Object` constructor without an argument",
+            recommended: false,
+            url: "https://eslint.org/docs/latest/rules/no-object-constructor"
+        },
+
+        hasSuggestions: true,
+
+        schema: [],
+
+        messages: {
+            preferLiteral: "The object literal notation {} is preferable.",
+            useLiteral: "Replace with '{{replacement}}'."
+        }
+    },
+
+    create(context) {
+
+        const sourceCode = context.sourceCode;
+
+        /**
+         * Determines whether or not an object literal that replaces a specified node needs to be enclosed in parentheses.
+         * @param {ASTNode} node The node to be replaced.
+         * @returns {boolean} Whether or not parentheses around the object literal are required.
+         */
+        function needsParentheses(node) {
+            if (isStartOfExpressionStatement(node)) {
+                return true;
+            }
+
+            const prevToken = sourceCode.getTokenBefore(node);
+
+            if (prevToken && isArrowToken(prevToken)) {
+                return true;
+            }
+
+            return false;
+        }
+
+        /**
+         * Reports on nodes where the `Object` constructor is called without arguments.
+         * @param {ASTNode} node The node to evaluate.
+         * @returns {void}
+         */
+        function check(node) {
+            if (node.callee.type !== "Identifier" || node.callee.name !== "Object" || node.arguments.length) {
+                return;
+            }
+
+            const variable = getVariableByName(sourceCode.getScope(node), "Object");
+
+            if (variable && variable.identifiers.length === 0) {
+                const replacement = needsParentheses(node) ? "({})" : "{}";
+
+                context.report({
+                    node,
+                    messageId: "preferLiteral",
+                    suggest: [
+                        {
+                            messageId: "useLiteral",
+                            data: { replacement },
+                            fix: fixer => fixer.replaceText(node, replacement)
+                        }
+                    ]
+                });
+            }
+        }
+
+        return {
+            CallExpression: check,
+            NewExpression: check
+        };
+
+    }
+};
diff --git a/packages/js/src/configs/eslint-all.js b/packages/js/src/configs/eslint-all.js
@@ -152,11 +152,11 @@ module.exports = Object.freeze({
         "no-new": "error",
         "no-new-func": "error",
         "no-new-native-nonconstructor": "error",
-        "no-new-object": "error",
         "no-new-symbol": "error",
         "no-new-wrappers": "error",
         "no-nonoctal-decimal-escape": "error",
         "no-obj-calls": "error",
+        "no-object-constructor": "error",
         "no-octal": "error",
         "no-octal-escape": "error",
         "no-param-reassign": "error",