-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Modifying operators must only be allowed for type number (and += for string) #48857
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
Comments
@MartinJohns, what is the confusion here? Can you clarify, so I could try to respond? |
@MartinJohns Your issue #47027 focuses on If this did not become apparent, I totally agree with you that for example let foo: `${string}.png` = "foobar.png";
foo += "/"; should result in a type error. |
Maybe another example, where one could be surprised by the type error: type SomeAs = "a" | "aa" | "aaa";
let aa: SomeAs = "a";
aa += "a"; Although aa = aa + "a"; |
Thanks for the clear and thorough report; however, the original issues are not locked so if you want to continue the discussion, you can do so in those issues. Opening new issues to discuss old ones just creates more triage work and makes it harder for future readers to follow the conversation. |
Some of these issues are already closed as won't fix, and the argument I am trying to make is a bit different, so none of them fit 100%. |
I'm confused to why you open a new issue when you already found an existing open (#47027) which has the label "In Discussion".
The focus on |
Okay, I found this discussion rather fragmented and after realizing that #14745 had been closed as "won't fix", I tried to consolidate it in a new issue with a concrete suggestion that is supposed to tackle all special cases. |
Well, maybe I tried to "solve fragmentation by adding another fragment", like in https://xkcd.com/927/ |
I've changed the title and mentioned that it applies to |
Thank you for already changing the title! I still think that, as explained above, "literal types" are not the only source of the problem, so I'd suggest "constrained types" or even "subtypes of |
And I'll add the "uint" example there, but maybe not today (as it is already 8 pm in my time-zone). |
Literal types can only be |
@MartinJohns He also intends for the rule to apply to e.g. |
Please continue here: #47027 (comment) |
@andrewbranch After following your advice to close this issue and add my examples and suggestions to #47027, I would be delighted to get some reaction there! |
Bug Report
In this report, I'd like to motivate that the type checker should not allow applying modifying operators to custom sub-types at all.
For reference and all kinds of unexpected behavior because this is currently allowed, see #14745, #47027, #30783 and an attempt to fix (part of) this behavior, #28344.
In #14745 (comment), @ahejlsberg was asking for further practical examples that would motivate stricter typing of modifying operators, which this report provides imho.
@fatcerberus #47027 (comment) agrees that any usage of
:=
on something that is not of typestring
is "code smell".🔎 Search Terms
🕗 Version & Regression Information
⏯ Playground Link
Playground link 1: Integer Range Example
Playground link 2: Unsigned Integer Example
💻 Code
Example 1: Integer Range
Example 2: Unsigned Integer
🙁 Actual behavior
Modifying operators like
++
,--
,+=
,-=
,*=
,/=
etc. are allowed for types that specializenumber
(and for+=
,string
), although the result does not necessarily adhere to the constraints given by the sub-type.🙂 Expected behavior
Modifying operators are only allowed for the type
number
(plusstring
for+=
) itself.When using a sub-type of
number
, like a literal type, a union of literal types (like in the "Integer Range" example), or an intersection ofnumber
with any other type (like in the "Unsigned Integer" example), an explicit type conversion or type assertion is needed.Like for other changes that lead to stricter typing, this behavior could be activated through a compiler option.
Rationale:
The built-in numeric operators in ECMAScript,
+
,-
,*
,/
,%
are all defined fornumber
arguments (except for overloaded '+') and always result in anumber
(maybeNaN
, which is also of typenumber
). At compile-time, it is in most cases impossible to check that a sub-type ofnumber
is maintained by any of these operators. Thus, the result is only safely assignable tonumber
(or a more generic type, essentially onlyany
), not to a more specific type.Any modifying operator used with a sub-type of
number
(and+=
used with a sub-type ofstring
) would then be a type error. To resolve this error is not an unacceptable burden for the developer, because modifying operators are just a shorthand for an assignment. When expanding to an assignment, the right-hand-side can easily be complemented by a type assertion (if you are sure the computation result stays within the constraints of the sub-type for semantic reasons the type checker cannot deduce) or a conversion function that ensures the constraints at run-time (see both examples). It could be discussed whether there should be special syntax to add a type assertion to modifying operators that refer to the right-hand side only (note that++foo as uint
is only an assertion for the expression result, not for the right-hand sidefoo + 1
of the implicit assignment), but I don't think this is necessary.The examples show typical use cases, where you want to enforce a restricted set of
number
values, which is implemented through a coercion function at run-time. You want the compiler to check that you never forget to apply the coercion function to anumber
before assuming the result is one of the restricted values. This works very well in TypeScript, but applying modifying operators currently disables the check in a way only a type assertion should.The same argument can be made for
string
and its specializations, like unions of string literals or string template types. Forstring
, the only meaningful modifying operator is+=
. I tried to add this special case in parenthesis throughout this report.The text was updated successfully, but these errors were encountered: