Rule proposal: restrict-constructor-expressions #10895

karlhorky · 2025-02-27T10:34:36Z

Before You File a Proposal Please Confirm You Have Done The Following...

I have searched for related issues and found none that match my proposal.
I have searched the current rule list and found no rules that match my proposal.
I have read the FAQ and my problem is not listed.

My proposal is suitable for this project

My proposal specifically checks TypeScript syntax, or it proposes a check that requires type information to be accurate.
My proposal is not a "formatting rule"; meaning it does not just enforce how code is formatted (whitespace, brace placement, etc).
I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Description

Similar to restrict-template-expressions, the new rule restrict-constructor-expressions restrict the types of expressions passed to constructors such as Number() and String() to a subset which result in non-surprising return values

TypeScript types for constructors have a permissive any type for the parameters:

Fail Cases

Number({stdout: '123'}) // NaN
Number(['2']) // 2
String({result: 123}) // '[object Object]'

// I'm guessing there are many more...

Pass Cases

Number({stdout: '123'}.stdout) // 123
Number(['2'][0]) // 2
String({result: 123}.result) // '123'

Additional Info

No response

The text was updated successfully, but these errors were encountered:

kirkwaiblinger · 2025-02-27T15:10:43Z

Related: #10862, #4314

bradzacher · 2025-02-27T19:02:04Z

I can't search right now but this came up not long ago and I vaguely remember the team was broadly aligned that such a rule was niche and not broadly applicable enough to need its own rule.

The String case can be handled by no-base-to-string
The Boolean case we generally ignore as an intentional escape hatch for rules (eg strict-boolean-expressions ignores it)
The Number case is the "looser" form of Number.parseInt/Float and (generally) isn't something people should use and should be banned via no-restricted-globals or similar.
The Symbol case doesn't need restricting.

Josh-Cena · 2025-02-27T19:08:30Z

I disagree that Number should be banned, but I agree that this rule doesn't seem all that useful in general. It seems that the motivation is ultimately "ban arbitrary coercion to number", but I doubt that happens a lot.

karlhorky · 2025-02-27T19:12:55Z

Our use case was this code:

const postgresUid = Number(await execa`id -u postgres`);

This was originally working, a few execa versions ago. But in one of the breaking changes, they started returning an object with .stdout property.

Fixed code was:

const postgresUid = Number((await execa`id -u postgres`).stdout);

It was surprising to me that no types or linting were preventing this Number(obj) pattern - seems like somethi 8000 ng that could happen to people a lot, similar to the String() case.

Josh-Cena · 2025-02-27T19:21:09Z

I suppose you could do it with no-deprecated and some declaration merging to deprecate Number(any) and require Number(string) instead. Haven't tried though.

bradzacher · 2025-02-27T22:16:22Z

I'd personally just ban Number and force people to use parseInt/parseFloat which iirc have better types.

karlhorky · 2025-02-28T08:12:23Z

I suppose you could do it with no-deprecated and some declaration merging to deprecate Number(any) and require Number(string) instead. Haven't tried though.

Yeah, I guess I will come up with my own home-grown solution to lint / type-check for Number(nonString) and maybe also String(nonSerializable).

Too bad that typescript-eslint won't add a no-base-to-string-symmetrical rule for Number(), I think these problems are probably not uncommon.

I'd personally just ban Number and force people to use parseInt/parseFloat which iirc have better types.

I'm also of the opinion that Number() is ok and is widely documented and recommended. So I probably won't switch our teaching to parseInt / parseFloat.

bradzacher · 2025-02-28T08:36:22Z

8000

I'm also of the opinion that Number() is ok and is widely documented and recommended. So I probably won't switch our teaching to parseInt / parseFloat

I mean the type definition of Number.parseInt/Float is strict and only accepts string.

It seems weird that you want a lint rule to ensure correct usage of Number when you could just ban it and instead use parseInt/Float and have correctness enforced by TS at typecheck time.

bradzacher · 2025-02-28T08:43:16Z

Too bad that typescript-eslint won't add a no-base-to-string-symmetrical rule for Number()

There isn't a symmetry here though.

toString is a method that's declared on every type and value and it is implicitly called in countless scenarios.

OTOH there is no toNumber method that is implicitly called - the only way to convert a value to a number is Number(), parseInt/Float, to use it in a binary operation with a math operator, or to use a unary +/-. Of those cases parseInt/Float is strict and only accepts string, the binary case is handled by TS itself.
The unary case is unhandled but is so so so niche that it's not worth thinking about, and the Number case can be replaced with parseInt/Float for safety.

It just doesn't seem broadly valuable to build a rule for this, IMO?

kirkwaiblinger · 2025-02-28T14:28:41Z

@karlhorky FYI if you do end up doing syntactic bans of these functions I would suggest allowing the argument to be a satisfies expression, that way you can demand a good type at each of the usage sites. Just a thought.

karlhorky · 2025-02-28T15:37:29Z

It just doesn't seem broadly valuable to build a rule for this, IMO?

I guess I disagree on that:

Number() is widely documented and used - also, it has nice symmetry with String() and Boolean()
There is no rule guiding away from Number() and towards parseInt(), parseFloat()
Even if there is other syntax that allows for safer usage, I think it's good to also make Number(), String(), etc. safe, for the existing usage

I think these types of "safety" rules (including also the rules for string interpolation) can be argued against with buried "best practices" comments on issues, but unless these best practices are exposed to users, users will keep getting bitten by this. My gut feel says most users won't know that it's possible to guard against this, so won't ask / search for this.

Developing a rule would expose these best practices and offer high value right away to a wide range of codebases.

Josh-Cena · 2025-02-28T16:36:47Z

A few points:

I recommend Number() over other parsing functions, because its parsing rules are what you expect: exactly how you parse number literals. Both parseInt and parseFloat "do their own thing" and it's not always easy to remember what it accepts or does not.
There in fact does exist a generic number coercion method: valueOf(). A rule that bans implicitly calling the base valueOf() method would also be desirable. Alternatively, you could well claim that Number() indeed calls toString(): because valueOf() returns an object, it gets ignored, so the string coercion result is parsed to a number instead, which happens to be invalid. This is documented in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number#number_coercion

So now I think about it, I think we should have a no-base-to-string equivalent for number coercion that catches +x and Number().

JoshuaKGoldberg · 2025-03-31T13:48:11Z

What is the resolution here? Just having a no-base-to-number-esque rule?

bradzacher · 2025-03-31T22:50:11Z

I'm a -1 because this is my 2c for codebases that care about it:

{
  "rules": {
    "no-restricted-syntax": [
      "error",
      {
        "selector": ":matches(CallExpression, NewExpression) > Identifier[name = 'Number'].callee",
        "message": "Don't use the number constructor, use parseInt/parseFloat"
      },
      {
        "selector": "UnaryExpression[operator = '+']",
        "message": "Don't use implicit number coercion, use parseInt/parseFloat"
      }
    ]
  }
}

playground

But I ofc won't block anyone progressing on this.

JoshuaKGoldberg · 2025-04-07T12:46:49Z

Coming back to this, it's a really interesting discussion and we see the potential use. But Number()/+/etc. practices are much less common (and less defined) than String()/etc. It's hard to justify spending the space+time to put the rule in core.

Marking as evaluating community engagement to see if other folks want to chime in. If anybody can post links to other discussions asking for this, real-world bugs/similar this would have been helpful in, or other supporting evidence, please do!

karlhorky added enhancement: new plugin rule New rule request for eslint-plugin package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Feb 27, 2025

ProchaLu mentioned this issue Feb 27, 2025

Fix NaN issue in Execa by extracting .stdout upleveled/preflight#629

Merged

JoshuaKGoldberg added evaluating community engagement we're looking for community engagement on this issue to show that this problem is widely important and removed triage Waiting for team members to take a look labels Apr 7, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rule proposal: restrict-constructor-expressions #10895

Rule proposal: restrict-constructor-expressions #10895

Rule proposal: restrict-constructor-expressions #10895

Rule proposal: restrict-constructor-expressions #10895

Comments

Before You File a Proposal Please Confirm You Have Done The Following...

My proposal is suitable for this project

Description

Fail Cases

Pass Cases

Additional Info