Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add rule no-object-constructor, deprecate no-new-object #17576

Merged
merged 4 commits into from Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 13 additions & 7 deletions docs/src/_data/rules.json
Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -1956,6 +1956,12 @@
"no-unsafe-negation"
]
},
{
"name": "no-new-object",
"replacedBy": [
"no-object-constructor"
]
},
{
"name": "no-new-require",
"replacedBy": []
Expand Down
15 changes: 14 additions & 1 deletion docs/src/_data/rules_meta.json
Expand Up @@ -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,
Expand Down Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion docs/src/rules/no-array-constructor.md
Expand Up @@ -2,8 +2,8 @@
title: no-array-constructor
rule_type: suggestion
related_rules:
- no-new-object
- no-new-wrappers
- no-object-constructor
---


Expand Down
9 changes: 7 additions & 2 deletions docs/src/rules/no-new-object.md
Expand Up @@ -6,6 +6,7 @@ related_rules:
- no-new-wrappers
---

This rule was **deprecated** in ESLint v8.XX.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.
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved

The `Object` constructor is used to create new generic objects in JavaScript, such as:

Expand All @@ -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:

Expand All @@ -37,6 +38,8 @@ Examples of **incorrect** code for this rule:
var myObject = new Object();

new Object();

var foo = new Object("foo");
```

:::
Expand All @@ -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.
2 changes: 1 addition & 1 deletion docs/src/rules/no-new-wrappers.md
Expand Up @@ -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
---
Expand Down
50 changes: 50 additions & 0 deletions 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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rule overview is partly borrowed from no-array-constructor. Does everything make sense?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me.

## 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.
1 change: 1 addition & 0 deletions lib/rules/index.js
Expand Up @@ -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"),
Expand Down
7 changes: 7 additions & 0 deletions 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.XX.0
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
*/

"use strict";
Expand All @@ -26,6 +27,12 @@ module.exports = {
url: "https://eslint.org/docs/latest/rules/no-new-object"
},

deprecated: true,

replacedBy: [
"no-object-constructor"
],

schema: [],

messages: {
Expand Down
118 changes: 118 additions & 0 deletions 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
};

}
};
2 changes: 1 addition & 1 deletion packages/js/src/configs/eslint-all.js
Expand Up @@ -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",
Expand Down