feat: new `no-new-native-nonconstructor` rule #16368

sosukesuzuki · 2022-09-30T18:30:12Z

Prerequisites checklist

I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[x] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Fixes #16322

Adds new rule no-new-nonconstructor.

Is there anything you'd like reviewers to focus on?

question: Should we deprecate no-new-symbol in this PR? or in other PR?

netlify · 2022-09-30T18:30:36Z

✅ Deploy Preview for docs-eslint ready!

Name	Link
🔨 Latest commit	`c9f11ed`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/63674a108a0aff000869538a
😎 Deploy Preview	https://deploy-preview-16368--docs-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

sosukesuzuki · 2022-09-30T18:32:43Z

lib/rules/no-new-nonconstructor.js

@@ -0,0 +1,64 @@
+/**
+ * @fileoverview Rule to disallow use of the new operator with global no constructor functions
+ * @author Sosuke Suzuki


Just a question, most of the code for this rule is the same as no-new-symbol. Who should we mention as a an author?

Yes, that would be nice.

sosukesuzuki · 2022-09-30T18:38:05Z

lib/rules/no-new-nonconstructor.js

+
+        docs: {
+            description: "Disallow `new` operators with global no constructor functions",
+            recommended: false,


Should be recommended?

No. We can't set to recommended until we do a major release. We can switch it during the next major release.

ljharb · 2022-09-30T20:45:18Z

The naming of this rule doesn't just imply inclusion of primitive constructors that throw, but also all HTML element constructors, as well as %TypedArray% (altho that one's hard to lint for) - but also any function that's not newable, which includes arrows, async, generators, async generators, concise object methods, and class methods.

Is that broad scope intended?

sosukesuzuki · 2022-10-01T05:50:58Z

Is that broad scope intended?

No. Only built-in functions such as Symbol and BigInt are intended. We need a better name. Like no-new-builtin-nonconstructor?

ljharb · 2022-10-01T05:52:44Z

In the future, when there's other constructors you can't new, would they be included by implication? Is it just primitive constructors? What if there's a future Decimal, or Record or Tuple, that have the same semantics?

If not, it seems like a no-new-bigint rule would be better. If so, then it's important to define exactly what the criteria is, and the name should be informed by that.

mdjermanovic · 2022-10-03T07:24:26Z

The criteria is: ES global variable that is a function (noncallable globals are covered by the no-obj-calls rule) and starts with an uppercase letter ( = looks like a constructor) but can't be used with new. At the moment, that would be Symbol and BigInt. In the future, new ES globals that satisfy the criteria would be added. It wouldn't be restricted to globals that relate to primitive types.

ljharb · 2022-10-03T16:33:07Z

and "can't be used with new" means that you get an exception when you invoke it with new, i gather?

what about HTMLElement and all the other HTML element global constructors that can't be used with new?

mdjermanovic · 2022-10-03T16:39:23Z

and "can't be used with new" means that you get an exception when you invoke it with new, i gather?

Yes, exactly.

what about HTMLElement and all the other HTML element global constructors that can't be used with new?

This rule checks only globals from the ECMAScript specification, currently Symbol and BigInt, similar to the no-obj-calls rule which checks only Math, JSON, Reflect and Atomics.

sosukesuzuki · 2022-10-07T01:11:07Z

I don't think no-new-noconstructor is a bad name. It is similar to no-obj-calls.

I think it is sufficient to specify the criteria in the documentation.

nzakas · 2022-10-25T19:16:37Z

We could go with no-new-native-nonconstructor instead?

docs/src/rules/no-new-native-nonconstructor.md

lib/rules/no-new-native-nonconstructor.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

mdjermanovic · 2022-10-27T07:00:42Z

docs/src/_data/further_reading_links.json

@@ -698,5 +698,19 @@
        "logo": "https://eslint.org/apple-touch-icon.png",
        "title": "Interesting bugs caught by no-constant-binary-expression - ESLint - Pluggable JavaScript Linter",
        "description": "A pluggable and configurable linter tool for identifying and reporting on patterns in JavaScript. Maintain your code quality with ease."
+    },
+    "https://tc39.es/ecma262/#sec-symbol-object": {


Suggested change

"https://tc39.es/ecma262/#sec-symbol-object": {

"https://tc39.es/ecma262/#sec-symbol-constructor": {

This should fix the docs site build.

docs/src/_data/further_reading_links.json

docs/src/rules/no-new-native-nonconstructor.md

lib/rules/no-new-native-nonconstructor.js

mdjermanovic

LGTM, thanks!

I'll leave this open for a few days in case someone else wants to review it before merging.

nzakas

The code looks good. I just left some suggestions to improve the documentation.

docs/src/rules/no-new-native-nonconstructor.md

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

nzakas

LGTM. Thanks!

nzakas · 2022-11-01T17:19:07Z

@mdjermanovic because this is meant to replace no-new-symbol, so we also want to deprecate no-new-symbol or otherwise indicate the relationship between these two rules somehow?

mdjermanovic · 2022-11-02T20:22:50Z

We should deprecate no-new-symbol, but since it's in eslint:recommended then maybe better to deprecate it in the next major version?

snitin315

LGTM, thanks!

snitin315 · 2022-11-04T03:10:19Z

We should deprecate no-new-symbol, but since it's in eslint:recommended then maybe better to deprecate it in the next major version?

I feel we can deprecate in minor versions, and in the major version remove no-new-symbol in favor of no-new-native-nonconstructor. Deprecations are meant to warn users about a change beforehand.

btmills

LGTM, thanks!

I just resolved a merge conflict in the further reading links. Another entry had been added since this branch was last updated to main, so I just needed to add an extra }, line. I'll merge this once CI passes.

@snitin315 in most cases we could deprecate a rule in a minor release, but we're unable to add no-new-native-nonconstructor to eslint:recommended until the next major release, so resolving the deprecation warning would require users to manually override eslint:recommended even if they previously had no customizations. I will, however, call out the replacement in the release notes.

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.26.0` -> `8.27.0`](https://renovatebot.com/diffs/npm/eslint/8.26.0/8.27.0) | --- ### Release Notes <details> <summary>eslint/eslint</summary> ### [`v8.27.0`](https://github.com/eslint/eslint/releases/tag/v8.27.0) [Compare Source](eslint/eslint@v8.26.0...v8.27.0) #### Features - [`f14587c`](eslint/eslint@f14587c) feat: new `no-new-native-nonconstructor` rule ([#16368](eslint/eslint#16368)) (Sosuke Suzuki) - [`978799b`](eslint/eslint@978799b) feat: add new rule `no-empty-static-block` ([#16325](eslint/eslint#16325)) (Sosuke Suzuki) - [`69216ee`](eslint/eslint@69216ee) feat: no-empty suggest to add comment in empty BlockStatement ([#16470](eslint/eslint#16470)) (Nitin Kumar) - [`319f0a5`](eslint/eslint@319f0a5) feat: use `context.languageOptions.ecmaVersion` in core rules ([#16458](eslint/eslint#16458)) (Milos Djermanovic) #### Bug Fixes - [`c3ce521`](eslint/eslint@c3ce521) fix: Ensure unmatched glob patterns throw an error ([#16462](eslint/eslint#16462)) (Nicholas C. Zakas) - [`886a038`](eslint/eslint@886a038) fix: handle files with unspecified path in `getRulesMetaForResults` ([#16437](eslint/eslint#16437)) (Francesco Trotta) #### Documentation - [`ce93b42`](eslint/eslint@ce93b42) docs: Stylelint property-no-unknown ([#16497](eslint/eslint#16497)) (Nick Schonning) - [`d2cecb4`](eslint/eslint@d2cecb4) docs: Stylelint declaration-block-no-shorthand-property-overrides ([#16498](eslint/eslint#16498)) (Nick Schonning) - [`0a92805`](eslint/eslint@0a92805) docs: stylelint color-hex-case ([#16496](eslint/eslint#16496)) (Nick Schonning) - [`74a5af4`](eslint/eslint@74a5af4) docs: fix stylelint error ([#16491](eslint/eslint#16491)) (Milos Djermanovic) - [`324db1a`](eslint/eslint@324db1a) docs: explicit stylelint color related rules ([#16465](eslint/eslint#16465)) (Nick Schonning) - [`94dc4f1`](eslint/eslint@94dc4f1) docs: use Stylelint for HTML files ([#16468](eslint/eslint#16468)) (Nick Schonning) - [`cc6128d`](eslint/eslint@cc6128d) docs: enable stylelint declaration-block-no-duplicate-properties ([#16466](eslint/eslint#16466)) (Nick Schonning) - [`d03a8bf`](eslint/eslint@d03a8bf) docs: Add heading to justification explanation ([#16430](eslint/eslint#16430)) (Maritaria) - [`8a15968`](eslint/eslint@8a15968) docs: add Stylelint configuration and cleanup ([#16379](eslint/eslint#16379)) (Nick Schonning) - [`9b0a469`](eslint/eslint@9b0a469) docs: note commit messages don't support scope ([#16435](eslint/eslint#16435)) (Andy Edwards) - [`1581405`](eslint/eslint@1581405) docs: improve context.getScope() docs ([#16417](eslint/eslint#16417)) (Ben Perlmutter) - [`b797149`](eslint/eslint@b797149) docs: update formatters template ([#16454](eslint/eslint#16454)) (Milos Djermanovic) - [`5ac4de9`](eslint/eslint@5ac4de9) docs: fix link to formatters on the Core Concepts page ([#16455](eslint/eslint#16455)) (Vladislav) - [`33313ef`](eslint/eslint@33313ef) docs: core-concepts: fix link to semi rule ([#16453](eslint/eslint#16453)) (coderaiser) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1628 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

eslint-github-bot bot added triage An ESLint team member will look at this issue soon feature This change adds a new feature to ESLint labels Sep 30, 2022

sosukesuzuki commented Sep 30, 2022

View reviewed changes

sosukesuzuki marked this pull request as ready for review September 30, 2022 18:49

sosukesuzuki added 8 commits October 26, 2022 11:39

feat: add no-new-noconstructor rule

9ccc5c6

docs: add docs for no-new-noconstructor

6cde95a

fix: pass constructor name to message

9263431

docs: add related_rules

ab55170

chore: add new rule

48ef5e5

fix: rename noconstructor -> nonconstructor

1467efe

fix: set recommended: false

e1138b4

fix: rename no-new-nonconstructor -> no-new-native-nonconstructor

b79aac4

sosukesuzuki force-pushed the add-no-new-noconstructor branch from e72bc1d to b79aac4 Compare October 26, 2022 02:45

sosukesuzuki changed the title ~~feat: new no-new-nonconstructor rule~~ feat: new no-new-native-nonconstructor rule Oct 26, 2022

mdjermanovic added rule Relates to ESLint's core rules accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Oct 26, 2022

mdjermanovic requested changes Oct 26, 2022

View reviewed changes

mdjermanovic reviewed Oct 26, 2022

View reviewed changes

lib/rules/no-new-native-nonconstructor.js Outdated Show resolved Hide resolved

docs: fix correct example

0349065

sosukesuzuki force-pushed the add-no-new-noconstructor branch from 218905d to 0349065 Compare October 27, 2022 06:34

Update docs/src/rules/no-new-native-nonconstructor.md

f15fd20

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

mdjermanovic reviewed Oct 27, 2022

View reviewed changes

docs: update links

5eed2d6

mdjermanovic requested changes Oct 29, 2022

View reviewed changes

sosukesuzuki added 2 commits October 30, 2022 11:58

docs: address reviews

410b348

fix: no constructor -> non-constructor

aa6f701

mdjermanovic approved these changes Oct 30, 2022

View reviewed changes

nzakas requested changes Oct 31, 2022

View reviewed changes

Apply suggestions from code review

14489ad

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

nzakas approved these changes Nov 1, 2022

View reviewed changes

nzakas added the contributor pool label Nov 1, 2022

snitin315 approved these changes Nov 4, 2022

View reviewed changes

Merge branch 'main' into add-no-new-noconstructor

c9f11ed

btmills approved these changes Nov 6, 2022

View reviewed changes

btmills merged commit f14587c into eslint:main Nov 6, 2022

sosukesuzuki deleted the add-no-new-noconstructor branch November 6, 2022 06:50

freaktechnik mentioned this pull request Nov 7, 2022

make sure new eslint rules are used freaktechnik/eslint-configs#530

Closed

snitin315 mentioned this pull request Nov 7, 2022

Rule Change: overlapping "no-new-symbol" and "no-new-native-nonconstructor" #16513

Closed

1 task

eslint-github-bot bot locked and limited conversation to collaborators May 6, 2023

eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label May 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: new `no-new-native-nonconstructor` rule #16368

feat: new `no-new-native-nonconstructor` rule #16368

sosukesuzuki commented Sep 30, 2022

netlify bot commented Sep 30, 2022 •

edited

Loading

sosukesuzuki Sep 30, 2022

nzakas Oct 25, 2022

sosukesuzuki Sep 30, 2022

nzakas Oct 25, 2022

ljharb commented Sep 30, 2022

sosukesuzuki commented Oct 1, 2022

ljharb commented Oct 1, 2022

mdjermanovic commented Oct 3, 2022

ljharb commented Oct 3, 2022

mdjermanovic commented Oct 3, 2022

sosukesuzuki commented Oct 7, 2022

nzakas commented Oct 25, 2022

mdjermanovic Oct 27, 2022

mdjermanovic left a comment

nzakas left a comment

nzakas left a comment

nzakas commented Nov 1, 2022

mdjermanovic commented Nov 2, 2022

snitin315 left a comment

snitin315 commented Nov 4, 2022

btmills left a comment •

edited

Loading

	"https://tc39.es/ecma262/#sec-symbol-object": {
	"https://tc39.es/ecma262/#sec-symbol-constructor": {

feat: new no-new-native-nonconstructor rule #16368

feat: new no-new-native-nonconstructor rule #16368

Conversation

sosukesuzuki commented Sep 30, 2022

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

netlify bot commented Sep 30, 2022 • edited Loading

✅ Deploy Preview for docs-eslint ready!

sosukesuzuki Sep 30, 2022

Choose a reason for hiding this comment

nzakas Oct 25, 2022

Choose a reason for hiding this comment

sosukesuzuki Sep 30, 2022

Choose a reason for hiding this comment

nzakas Oct 25, 2022

Choose a reason for hiding this comment

ljharb commented Sep 30, 2022

sosukesuzuki commented Oct 1, 2022

ljharb commented Oct 1, 2022

mdjermanovic commented Oct 3, 2022

ljharb commented Oct 3, 2022

mdjermanovic commented Oct 3, 2022

sosukesuzuki commented Oct 7, 2022

nzakas commented Oct 25, 2022

mdjermanovic Oct 27, 2022

Choose a reason for hiding this comment

mdjermanovic left a comment

Choose a reason for hiding this comment

nzakas left a comment

Choose a reason for hiding this comment

nzakas left a comment

Choose a reason for hiding this comment

nzakas commented Nov 1, 2022

mdjermanovic commented Nov 2, 2022

snitin315 left a comment

Choose a reason for hiding this comment

snitin315 commented Nov 4, 2022

btmills left a comment • edited Loading

Choose a reason for hiding this comment

feat: new `no-new-native-nonconstructor` rule #16368

feat: new `no-new-native-nonconstructor` rule #16368

netlify bot commented Sep 30, 2022 •

edited

Loading

btmills left a comment •

edited

Loading