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

Suggestion: stricter operators #7989

Open
evmar opened this issue Apr 10, 2016 · 14 comments
Open

Suggestion: stricter operators #7989

evmar opened this issue Apr 10, 2016 · 14 comments
Labels
Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this Suggestion An idea for TypeScript
Milestone

Comments

@evmar
Copy link
Contributor

evmar commented Apr 10, 2016

Currently operators like "+" are defined such that they match their semantics in JS. The below are all allowed by the compiler and produce the shown values, even with --strictNullChecks on.

  • 2 + 'a' => "2a"
  • null + 'a' => "nulla" (!)
  • 2 - null => 2

I propose letting users opt in (maybe via some --strictOperators) to strict operator behavior. Concretely I think this means:

  • restrict +, and += to just number and string, e.g. for the former only declare

    function +(a: number, b: number): number;
    function +(a: string, b: string): string;
  • restrict - and -= to just number

(any should continue to work as normal, of course.)

Relevant spec section:
https://github.com/Microsoft/TypeScript/blob/master/doc/spec.md#419-binary-operators

See also "Expression operators" in the strictNullTypes change: #7140
and in particular this rationale: #7140 (comment)

This would fall under of "stricter" TypeScript, #274 .

@basarat
Copy link
Contributor

basarat commented Apr 10, 2016

number + string is actually quite common in JavaScript land and will probably not happen as it moves the convenience - type safety slider too much towards safety.

null + string should be disabled as a part of strictNullChecks.

Note:

A number of wat things are disabled in TypeScript e.g. [] + [] (valid JavaScript, produces "") is an error in TypeScript. Similarly "hello" + 1 is allowed (like I mentioned) but "hello" - 1 is an error 🌹

@zpdDG4gta8XKpMCd
Copy link

related #7746

@myitcv
Copy link

myitcv commented Apr 10, 2016

@basarat I really struggle when the argument "because that's how Javascript does it" is applied. Particularly in cases like this where I think the cognitive load on the developer is increased by decisions to stick to the Javascript way. Not because it's unclear how + behaves, rather that is creates cognitive dissonance with the rest of the type system:

let s: string;
let n: number;

s = n;            // ERROR: number is not assignable to string
n = s;            // ERROR: string is not assignable to number

let res = s + n;  // OK: really?

I understand that + is defined by the spec to work on combinations of number and string and so has well-defined behaviour, but given the example above I think it's more confusing than useful for it to have been defined in this way (the Javascript way). Particularly when explicit conversion is so simple and more readable.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 11, 2016

@myitcv how is different from #7746?

@myitcv
Copy link

myitcv commented Apr 11, 2016

@mhegazy because as I understand it, there is no coercion when it comes to +; it's simply specified to operate on various combinations of types.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 11, 2016

@Aleksey-Bykov do you agree that this suggestion encompasses the one in #7746?

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Jun 7, 2016
@zpdDG4gta8XKpMCd
Copy link

i agree, thank you for considering

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". and removed In Discussion Not yet reached consensus labels Jun 9, 2016
@RyanCavanaugh
Copy link
Member

Approved behavior change: under --strictNullChecks it should be an error to use a possibly-null/possibly-undefined operand in a +, -, /, *, |, &, ^, or ** expression. One exception is that string + nullable is still OK since that's very common for producing debugging strings.

@mhegazy mhegazy added this to the Community milestone Jun 9, 2016
@evmar
Copy link
Contributor Author

evmar commented Jun 9, 2016

Thanks for looking at this! That behavior change would be most welcome!

Do you have any thoughts on the other bits of the request (in particular string + number)?

Also, I guess

`${string}${nullable}`

is still legal for debugging purposes even if string + nullable were made illegal.

@RyanCavanaugh
Copy link
Member

We don't want the string template syntax to be different from the basic concat rules in terms of type system behavior; one is just sugar for the other and it'd be weird to have different rules.

@TimvdLippe
Copy link
Contributor

I think this is a duplicate of #12795 which was fixed in #13483

@evmar
Copy link
Contributor Author

evmar commented Feb 14, 2017

The null part, yes. I am still a little disappointed that type checking is basically disabled if the expression involves a string, e.g. this is legal and there's no opting out:

let x = {a: 3};
let y = x + 'a';

@OliverJAsh
Copy link
Contributor

OliverJAsh commented Sep 25, 2017

For tslint users, we have https://palantir.github.io/tslint/rules/restrict-plus-operands/

Edit: this won't help for template strings, at least not yet: palantir/tslint#3670

@Kingwl
Copy link
Contributor

Kingwl commented Apr 11, 2018

seems already fixed

@RyanCavanaugh RyanCavanaugh modified the milestones: Community, Backlog Mar 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

9 participants