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

📎 ImplementuseConsistentNewBuiltin - unicorn/new-for-builtins #2539

Closed
minht11 opened this issue Apr 20, 2024 · 5 comments · Fixed by #2540
Closed

📎 ImplementuseConsistentNewBuiltin - unicorn/new-for-builtins #2539

minht11 opened this issue Apr 20, 2024 · 5 comments · Fixed by #2540

Comments

@minht11
Copy link
Contributor

minht11 commented Apr 20, 2024

Description

Implement unicorn/new-for-builtins

@minht11
Copy link
Contributor Author

minht11 commented Apr 20, 2024

I will try to work on it myself.

@Conaclos
Copy link
Member

Note that new Symbol and new BigInt are already forbidden by noInvalidNewBuiltin.
Thus, this rule should not cover these two cases.

Also, I propose a more consistent name with our existing rules: useConsistentNewBuiltin.

@minht11 minht11 changed the title 📎 ImplementuseNewForBuiltins - unicorn/new-for-builtins 📎 ImplementuseConsistentNewBuiltin - unicorn/new-for-builtins Apr 22, 2024
@ematipico
Copy link
Member

Why "Consistent" @Conaclos ? I think useNewBuiltin is enough.

@Conaclos
Copy link
Member

Conaclos commented Apr 22, 2024

Taking a closer look at the rule, the following constructors cannot be instantiated without new.

  • ArrayBuffer
  • BigInt64Array
  • BigUint64Array
  • DataView
  • FinalizationRegistry
  • Float32Array
  • Float64Array
  • Int16Array
  • Int32Array
  • Int8Array
  • Promise
  • Proxy
  • Set
  • SharedArrayBuffer
  • Uint16Array
  • Uint32Array
  • Uint8Array
  • Uint8ClampedArray
  • WeakMap
  • WeakRef
  • WeakSet

Since we have noInvalidNewBuiltin, we should certainly have something like noInvalidBuiltinInstatiation.
However, I doubt the usefulness of having three rules.
I think we should deprecate noInvalidNewBuiltin and simply handle Symbol and BigInt in useConsistentNewBuiltin.

Alternatively we could deprecate noInvalidNewBuiltin and create two rules: useConsistentBuiltinInstatiation and noInvalidBuiltinInstatiation.

Any opinion?

Why "Consistent" @Conaclos ? I think useNewBuiltin is enough.

Became I was thinking about useConsistentBuiltinInstatiation.
You are right we should certainly use useConsistentBuiltinInstatiation or useNewBuiltin.
Any preference?

@minht11
Copy link
Contributor Author

minht11 commented Apr 22, 2024

I slightly prefer useConsistentBuiltinInstatiation even if it is bit long, since rule as it currently is dissallows use of new keyword in some cases, so it is bit more clear. noInvalidNewBuiltin deprecation also makes sense to me since they effetively do the same thing, but I will wait for confirmation before renaming things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants