feat(linter) eslint plugin unicorn: new-for-builtins (#1159)

This commit is contained in:
Cameron 2023-11-06 01:55:28 +00:00 committed by GitHub
parent c7598b22ff
commit 013cfe9c44
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 591 additions and 0 deletions

View file

@ -141,6 +141,7 @@ mod unicorn {
pub mod catch_error_name;
pub mod error_message;
pub mod filename_case;
pub mod new_for_builtins;
pub mod no_console_spaces;
pub mod no_empty_file;
pub mod no_instanceof_array;
@ -266,6 +267,7 @@ oxc_macros::declare_all_lint_rules! {
unicorn::catch_error_name,
unicorn::error_message,
unicorn::filename_case,
unicorn::new_for_builtins,
unicorn::no_console_spaces,
unicorn::no_empty_file,
unicorn::no_instanceof_array,

View file

@ -0,0 +1,298 @@
use oxc_ast::{ast::Expression, AstKind};
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::{self, Error},
};
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;
use oxc_syntax::operator::BinaryOperator;
use phf::phf_set;
use crate::{context::LintContext, rule::Rule, AstNode};
#[derive(Debug, Error, Diagnostic)]
enum NewForBuiltinsDiagnostic {
#[error("eslint-plugin-unicorn(new-for-builtins): Use `new {1}()` instead of `{1}()`")]
#[diagnostic(severity(warning))]
Enforce(#[label] Span, String),
#[error("eslint-plugin-unicorn(new-for-builtins): Use `{1}()` instead of `new {1}()`")]
#[diagnostic(severity(warning))]
Disallow(#[label] Span, String),
}
#[derive(Debug, Default, Clone)]
pub struct NewForBuiltins;
declare_oxc_lint!(
/// ### What it does
///
/// Enforces the use of `new` for following builtins: `Object`, `Array`, `ArrayBuffer`, `BigInt64Array`, `BigUint64Array`, `DataView`, `Date`, `Error`, `Float32Array`, `Float64Array`, `Function`, `Int8Array`, `Int16Array`, `Int32Array`, `Map`, `WeakMap`, `Set`, `WeakSet`, `Promise`, `RegExp`, `Uint8Array`, `Uint16Array`, `Uint32Array`, `Uint8ClampedArray`, `SharedArrayBuffer`, `Proxy`, `WeakRef`, `FinalizationRegistry`.
///
/// Disallows the use of `new` for following builtins: `String`, `Number`, `Boolean`, `Symbol`, `BigInt`.
///
/// These should not use `new` as that would create object wrappers for the primitive values, which is not what you want. However, without `new` they can be useful for coercing a value to that type.
///
/// ### Why is this bad?
///
/// They work the same, but `new` should be preferred for consistency with other constructors.
///
///
/// ### Example
/// ```javascript
/// // bad
/// const foo = new String('hello world');
/// const bar = Array(1, 2, 3);
///
/// // good
/// const foo = String('hello world');
/// const bar = new Array(1, 2, 3);
/// ```
NewForBuiltins,
pedantic
);
impl Rule for NewForBuiltins {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
match node.kind() {
AstKind::NewExpression(new_expr) => {
let callee = new_expr.callee.without_parenthesized();
let Some(builtin_name) = is_expr_global_builtin(callee, ctx) else { return };
if DISALLOW_NEW_FOR_BUILTINS.contains(builtin_name) {
ctx.diagnostic(NewForBuiltinsDiagnostic::Disallow(
new_expr.span,
builtin_name.to_string(),
));
}
}
AstKind::CallExpression(call_expr) => {
let Some(builtin_name) =
is_expr_global_builtin(call_expr.callee.without_parenthesized(), ctx)
else {
return;
};
if ENFORCE_NEW_FOR_BUILTINS.contains(builtin_name) {
if builtin_name == "Object" {
if let Some(parent) = ctx.nodes().parent_node(node.id()) {
if let AstKind::BinaryExpression(bin_expr) = parent.kind() {
if bin_expr.operator == BinaryOperator::StrictEquality
|| bin_expr.operator == BinaryOperator::StrictInequality
{
return;
}
}
}
}
ctx.diagnostic(NewForBuiltinsDiagnostic::Enforce(
call_expr.span,
builtin_name.to_string(),
));
}
}
_ => {}
}
}
}
fn is_expr_global_builtin<'a, 'b>(
expr: &'b Expression<'a>,
ctx: &'b LintContext<'a>,
) -> Option<&'b str> {
match expr {
Expression::Identifier(ident) => {
if !ctx.semantic().is_reference_to_global_variable(ident) {
return None;
}
Some(ident.name.as_str())
}
Expression::MemberExpression(member_expr) => {
let Expression::Identifier(ident) = member_expr.object() else { return None };
if !GLOBAL_OBJECT_NAMES.contains(ident.name.as_str()) {
return None;
}
return member_expr.static_property_name();
}
_ => None,
}
}
const ENFORCE_NEW_FOR_BUILTINS: phf::Set<&'static str> = phf_set! {
"Int8Array",
"Uint8Array",
"Uint8ClampedArray",
"Int16Array",
"Uint16Array",
"Int32Array",
"Uint32Array",
"Float32Array",
"Float64Array",
"BigInt64Array",
"BigUint64Array",
"Object",
"Array",
"ArrayBuffer",
"DataView",
"Date",
"Error",
"Function",
"Map",
"WeakMap",
"Set",
"WeakSet",
"Promise",
"RegExp",
"SharedArrayBuffer",
"Proxy",
"WeakRef",
"FinalizationRegistry",
};
const DISALLOW_NEW_FOR_BUILTINS: phf::Set<&'static str> = phf_set! {
"BigInt",
"Boolean",
"Number",
"Symbol",
"String",
};
const GLOBAL_OBJECT_NAMES: phf::Set<&'static str> = phf_set! {
"global",
"globalThis",
"self",
"window",
};
#[test]
fn test() {
use crate::tester::Tester;
let pass = vec![
r#"const foo = new Object()"#,
r#"const foo = new Array()"#,
r#"const foo = new ArrayBuffer()"#,
r#"const foo = new BigInt64Array()"#,
r#"const foo = new BigUint64Array()"#,
r#"const foo = new DataView()"#,
r#"const foo = new Date()"#,
r#"const foo = new Error()"#,
r#"const foo = new Float32Array()"#,
r#"const foo = new Float64Array()"#,
r#"const foo = new Function()"#,
r#"const foo = new Int8Array()"#,
r#"const foo = new Int16Array()"#,
r#"const foo = new Int32Array()"#,
r#"const foo = new Map()"#,
r#"const foo = new Map([['foo', 'bar'], ['unicorn', 'rainbow']])"#,
r#"const foo = new WeakMap()"#,
r#"const foo = new Set()"#,
r#"const foo = new WeakSet()"#,
r#"const foo = new Promise()"#,
r#"const foo = new RegExp()"#,
r#"const foo = new UInt8Array()"#,
r#"const foo = new UInt16Array()"#,
r#"const foo = new UInt32Array()"#,
r#"const foo = new Uint8ClampedArray()"#,
r#"const foo = BigInt()"#,
r#"const foo = Boolean()"#,
r#"const foo = Number()"#,
r#"const foo = String()"#,
r#"const foo = Symbol()"#,
r#"
import { Map } from 'immutable';
const m = Map();
"#,
r#"
const {Map} = require('immutable');
const foo = Map();
"#,
r#"
const {String} = require('guitar');
const lowE = new String();
"#,
r#"
import {String} from 'guitar';
const lowE = new String();
"#,
r#"new Foo();Bar();"#,
r#"Foo();new Bar();"#,
r#"const isObject = v => Object(v) === v;"#,
r#"const isObject = v => globalThis.Object(v) === v;"#,
r#"(x) !== Object(x)"#,
];
let fail = vec![
r#"const object = (Object)();"#,
r#"const symbol = new (Symbol)("");"#,
r#"const symbol = new /* comment */ Symbol("");"#,
r#"const symbol = new Symbol;"#,
r#"new globalThis.String()"#,
r#"new global.String()"#,
r#"new self.String()"#,
r#"new window.String()"#,
r#"globalThis.Array()"#,
r#"global.Array()"#,
r#"self.Array()"#,
r#"window.Array()"#,
r#"globalThis.Array()"#,
r#"const foo = Object()"#,
r#"const foo = Array()"#,
r#"const foo = ArrayBuffer()"#,
r#"const foo = BigInt64Array()"#,
r#"const foo = BigUint64Array()"#,
r#"const foo = DataView()"#,
r#"const foo = Date()"#,
r#"const foo = Error()"#,
r#"const foo = Error('Foo bar')"#,
r#"const foo = Float32Array()"#,
r#"const foo = Float64Array()"#,
r#"const foo = Function()"#,
r#"const foo = Int8Array()"#,
r#"const foo = Int16Array()"#,
r#"const foo = Int32Array()"#,
r#"const foo = (( Map ))()"#,
r#"const foo = Map([['foo', 'bar'], ['unicorn', 'rainbow']])"#,
r#"const foo = WeakMap()"#,
r#"const foo = Set()"#,
r#"const foo = WeakSet()"#,
r#"const foo = Promise()"#,
r#"const foo = RegExp()"#,
r#"const foo = Uint8Array()"#,
r#"const foo = Uint16Array()"#,
r#"const foo = Uint32Array()"#,
r#"const foo = Uint8ClampedArray()"#,
r#"const foo = new BigInt(123)"#,
r#"const foo = new Boolean()"#,
r#"const foo = new Number()"#,
r#"const foo = new Number('123')"#,
r#"const foo = new String()"#,
r#"const foo = new Symbol()"#,
r#"
function varCheck() {
{
var WeakMap = function() {};
}
// This should not reported
return WeakMap()
}
function constCheck() {
{
const Array = function() {};
}
return Array()
}
function letCheck() {
{
let Map = function() {};
}
return Map()
}
"#,
];
Tester::new_without_config(NewForBuiltins::NAME, pass, fail).test_and_snapshot();
}

View file

@ -0,0 +1,291 @@
---
source: crates/oxc_linter/src/tester.rs
expression: new_for_builtins
---
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Object()` instead of `Object()`
╭─[new_for_builtins.tsx:1:1]
1 │ const object = (Object)();
· ──────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `Symbol()` instead of `new Symbol()`
╭─[new_for_builtins.tsx:1:1]
1 │ const symbol = new (Symbol)("");
· ────────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `Symbol()` instead of `new Symbol()`
╭─[new_for_builtins.tsx:1:1]
1 │ const symbol = new /* comment */ Symbol("");
· ────────────────────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `Symbol()` instead of `new Symbol()`
╭─[new_for_builtins.tsx:1:1]
1 │ const symbol = new Symbol;
· ──────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `String()` instead of `new String()`
╭─[new_for_builtins.tsx:1:1]
1 │ new globalThis.String()
· ───────────────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `String()` instead of `new String()`
╭─[new_for_builtins.tsx:1:1]
1 │ new global.String()
· ───────────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `String()` instead of `new String()`
╭─[new_for_builtins.tsx:1:1]
1 │ new self.String()
· ─────────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `String()` instead of `new String()`
╭─[new_for_builtins.tsx:1:1]
1 │ new window.String()
· ───────────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Array()` instead of `Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ globalThis.Array()
· ──────────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Array()` instead of `Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ global.Array()
· ──────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Array()` instead of `Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ self.Array()
· ────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Array()` instead of `Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ window.Array()
· ──────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Array()` instead of `Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ globalThis.Array()
· ──────────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Object()` instead of `Object()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Object()
· ────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Array()` instead of `Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Array()
· ───────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new ArrayBuffer()` instead of `ArrayBuffer()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = ArrayBuffer()
· ─────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new BigInt64Array()` instead of `BigInt64Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = BigInt64Array()
· ───────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new BigUint64Array()` instead of `BigUint64Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = BigUint64Array()
· ────────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new DataView()` instead of `DataView()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = DataView()
· ──────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Date()` instead of `Date()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Date()
· ──────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Error()` instead of `Error()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Error()
· ───────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Error()` instead of `Error()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Error('Foo bar')
· ────────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Float32Array()` instead of `Float32Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Float32Array()
· ──────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Float64Array()` instead of `Float64Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Float64Array()
· ──────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Function()` instead of `Function()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Function()
· ──────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Int8Array()` instead of `Int8Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Int8Array()
· ───────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Int16Array()` instead of `Int16Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Int16Array()
· ────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Int32Array()` instead of `Int32Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Int32Array()
· ────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Map()` instead of `Map()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = (( Map ))()
· ───────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Map()` instead of `Map()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Map([['foo', 'bar'], ['unicorn', 'rainbow']])
· ─────────────────────────────────────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new WeakMap()` instead of `WeakMap()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = WeakMap()
· ─────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Set()` instead of `Set()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Set()
· ─────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new WeakSet()` instead of `WeakSet()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = WeakSet()
· ─────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Promise()` instead of `Promise()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Promise()
· ─────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new RegExp()` instead of `RegExp()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = RegExp()
· ────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Uint8Array()` instead of `Uint8Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Uint8Array()
· ────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Uint16Array()` instead of `Uint16Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Uint16Array()
· ─────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Uint32Array()` instead of `Uint32Array()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Uint32Array()
· ─────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Uint8ClampedArray()` instead of `Uint8ClampedArray()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = Uint8ClampedArray()
· ───────────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `BigInt()` instead of `new BigInt()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = new BigInt(123)
· ───────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `Boolean()` instead of `new Boolean()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = new Boolean()
· ─────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `Number()` instead of `new Number()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = new Number()
· ────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `Number()` instead of `new Number()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = new Number('123')
· ─────────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `String()` instead of `new String()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = new String()
· ────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `Symbol()` instead of `new Symbol()`
╭─[new_for_builtins.tsx:1:1]
1 │ const foo = new Symbol()
· ────────────
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Array()` instead of `Array()`
╭─[new_for_builtins.tsx:12:1]
12 │ }
13 │ return Array()
· ───────
14 │ }
╰────
⚠ eslint-plugin-unicorn(new-for-builtins): Use `new Map()` instead of `Map()`
╭─[new_for_builtins.tsx:18:1]
18 │ }
19 │ return Map()
· ─────
20 │ }
╰────