microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.94k stars 12.47k forks source link

No warning when using unary +/- operators on functions and objects? #20131

Open jcalz opened 6 years ago

jcalz commented 6 years ago

TypeScript Version: 2.6.1

Code

let func = () => "func";
+func; // no error
-func; // no error

let obj = { foo: "bar" };
+obj; // no error
-obj; // no error

Expected behavior: Unary + or - is only applicable to a number or maybe a string. I would expect the lines listed as // no error to be errors.

Actual behavior: Unary + and - never generates a warning no matter what crazy stuff I try to apply it to.


I was made aware of this behavior by a recent Stack Overflow question. I see from #7019 that unary + and - on strings is supported, and I guess I can understand that as a way to parse a string into a number (for +, anyway. Is - really used like this in practice?). But functions? objects? Seems more likely to be an error than intentional to me. Thoughts?

calebsander commented 6 years ago

It may not make much sense for anyone to ever use + or - on a non-primitive value, but it is not incorrect code. It will execute just fine, converting to a string, and then to a number (usually NaN). There is one use case I could could imagine: +[x], where x is a number or string, evaluates to +x. The + operator is equivalent to calling Number() on the value, and the signature of that function is (value?: any) => number, so + and - can technically be applied to any value.

Because JavaScript tries to make sense of a lot of operations that would cause other languages to throw an error, you could argue that TypeScript should prevent (or warn) you for many valid operations. For example, adding a string to a number? 1 + 2 + '3' similarly seems like a "crazy" application of the binary + operator, but it is perfectly valid. What about x / 0?

jcalz commented 6 years ago

Relevant reading about "not incorrect code" by @RyanCavanaugh:

I think the litmus test isn't usually "is this code invalid JavaScript" but "is this code likely to be a bug" (and whether the false positives you would get by flagging it as an error are preferable to the false negatives you would get by allowing it). JavaScript allows almost everything. TypeScript should warn you when you are doing something allowable-but-probably-wrong.

This issue is asking whether people really use unary + or - on non-string and non-number operands in practice enough to warrant supporting it. Who do we break by disallowing it, and should they be left alone or told to make their code more explicit by using +String(x) or -(x.toString()) or Number(x) when x is a function or an object?

RyanCavanaugh commented 6 years ago

Seems wrong when we can be reasonably confident that the result will be NaN. Operands should be of type number, string, Date, or boolean (am I missing anything?)

sean-vieira commented 6 years ago

As @calebsander points out, the single element tuple when it's value is number | string.

Also, any object that overrides Object.valueOf and returns either a string or a number:

var x = +({valueOf() { return '33'; }})  // 33

or toString and returns something that is a stringified number:

var x = +({toString() { return Math.random() > 0.5 ? '3' : 'abc'; } });  // sometimes 3 sometimes NaN
jcalz commented 6 years ago

If we intentionally disallow the single-element tuple and objects with valueOf() of toString() overrides, how much real-world code does that break? I obviously can't speak for everyone but I'd be happier being alerted for all of these:

var w = +({valueOf: ()=>''}); // 0
var x = +([{toString: ()=>'1'}]); // 1
var y = -([[[[[[[-2]]]]]]]); // 2
var z = +([{valueOf() { return 3; }}]); // 3

so I could either rewrite them or assert the quantity in parentheses as any to force the type check to succeed:

var w = +({valueOf: ()=>''} as any); // 0
var x = +([{toString: ()=>'1'}] as any); // 1
var y = -([[[[[[[-2]]]]]]] as any); // 2
var z = NaN; // not 3 

Hey, did you realize that z was actually NaN instead of 3? Neither did I.

RyanCavanaugh commented 6 years ago

I'm really skeptical that anyone uses - to pluck a coerced number out of a single-element array. Or + for that matter.

RyanCavanaugh commented 6 years ago

We agreed that the current behavior is dumb, but would prefer to group this change into an overall revamp of how the general numeric operators (especially > and <) work. The TL;DR of that is that we need the type system to special-case Date, both because of its idiomatic usage (+someDate is common) and wacky behavior (its preference of coercion to string vs number is different from all other objects)

james-cnz commented 6 years ago

I just came across this recently. I declared some strings across multiple lines, concatenated with +, and in one place accidentally put a + at the end of one line and the start of the next. I was very surprised that it got through type checking.

My feeling is that the unary +/- operators are a special case. I think semantically they really shouldn't be operators at all, but rather part of a numeric literal. I suspect they were only made into operators in the first place to make parsing easier.

gcnew commented 6 years ago

Parsing unary operators as operators is actually harder than treating them as part of the literal. However, if they weren't operators you wouldn't have been able to write let y = -x;.

markusjohnsson commented 6 years ago

How about supporting a declaration that a type supports the unary operator, then if someone designs a type meant to be used like that, it can be. Same could be said for binary operators I guess.

Unary could be associated with the type directly:

type HasUnaryPlus = { +: number; };
const x: HasUnaryPlus = aquireTheThing();
console.log(+x); // OK

Binary operators would need to be declared separate from the types:

type A = ... ; 
declare operator+ (a0: A, a2: A): A;

Could be useful for e.g. moment:

moment(moment() + moment.duration(3, 'days'));

The code makes sense, and runs fine as javascript, but does not validate as typescript.

dgreensp commented 6 years ago

Could you make -x follow the same rules as 0 - x?

It seems there is already a concept in TypeScript of what is "numeric," and I don't think any of the concerns about idiomatic uses of unary + apply to unary -.

I just ran into a bug in my program where I wrote -foo.bar instead of -foo.bar.value. Needless to say, foo.bar is not numeric. If I had written 0 - foo.bar, TypeScript would have flagged an error.

methodbox commented 5 years ago

Should unary operators even be allowed in TS?

parkerault commented 5 years ago

I find it hilarious (in a morbid sort of way) that implicitly converting a function to a string to a number is perfectly fine, but I can't even use an object with a toString method as a computed key.

const wtf = -(() => "func") // you do you, javascript developer!

const stringableFn = () => {};
stringableFn.toString = () => "I'm a key!";

const slowDownThereBuddy = {
  [stringableFn]: 1 // A computed property name must be of type 'string', 'number', 'symbol', or 'any'.ts(2464)
}

This annoys the hell out of me since this is a common pattern with many redux action creators libraries. I have a hard time believing that the former example is considered most likely intentional, and the latter is considered most likely a mistake.

cshaa commented 4 years ago

I just want to point out that +object is a perfectly valid expression when using math libraries with custom numeric types (like fraction.js and decimal.js used by math.js).

let fraction = math.fraction(2, 3)
let approxResult = +fraction

Globally disallowing the unary plus for objects sounds like a very bad idea here.

pbadenski commented 3 years ago

Is there lack of appetite to altering this behavior? Just wondering - I got redirected here from eslint github page: https://github.com/typescript-eslint/typescript-eslint/issues/3200

If it's not coming soon in TS, I'm wondering if it's a good idea to implement an eslint rule for this?