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

fix: do not report on shadowed constructors in no-new-wrappers #17447

Merged
merged 2 commits into from Aug 9, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
26 changes: 19 additions & 7 deletions lib/rules/no-new-wrappers.js
Expand Up @@ -5,6 +5,12 @@

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const { getVariableByName } = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand All @@ -28,18 +34,24 @@ module.exports = {
},

create(context) {
const { sourceCode } = context;

return {

NewExpression(node) {
const wrapperObjects = ["String", "Number", "Boolean"];

if (wrapperObjects.includes(node.callee.name)) {
context.report({
node,
messageId: "noConstructor",
data: { fn: node.callee.name }
});
const { name } = node.callee;

if (wrapperObjects.includes(name)) {
const variable = getVariableByName(sourceCode.getScope(node), name);

if (variable && variable.identifiers.length === 0) {
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
context.report({
node,
messageId: "noConstructor",
data: { fn: name }
});
}
}
}
};
Expand Down
49 changes: 48 additions & 1 deletion tests/lib/rules/no-new-wrappers.js
Expand Up @@ -21,7 +21,36 @@ const ruleTester = new RuleTester();
ruleTester.run("no-new-wrappers", rule, {
valid: [
"var a = new Object();",
"var a = String('test'), b = String.fromCharCode(32);"
"var a = String('test'), b = String.fromCharCode(32);",
`
function test(Number) {
return new Number;
}
`,
{
code: `
import String from "./string";
const str = new String(42);
`,
parserOptions: { ecmaVersion: 6, sourceType: "module" }
},
`
if (foo) {
result = new Boolean(bar);
} else {
var Boolean = CustomBoolean;
}
`,
{
code: "new String()",
globals: {
String: "off"
}
},
`
/* global Boolean:off */
assert(new Boolean);
`
],
invalid: [
{
Expand Down Expand Up @@ -53,6 +82,24 @@ ruleTester.run("no-new-wrappers", rule, {
},
type: "NewExpression"
}]
},
{
code: `
const a = new String('bar');
{
const String = CustomString;
const b = new String('foo');
}
`,
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "noConstructor",
data: {
fn: "String"
},
type: "NewExpression",
line: 2
}]
}
]
});