microsoft / TypeScript

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

Flag boolean in comparison operator operands as errors #15506

Open mhegazy opened 7 years ago

mhegazy commented 7 years ago

From https://github.com/Microsoft/TypeScript/issues/15444#issuecomment-298055832

var x: boolean, y: boolean;
if (x > y) {  // suspicious!
}
dmichon-msft commented 7 years ago

Well, that is the most concise way of writing:

let x: boolean, y: boolean;
if (x && !y) {
}
RyanCavanaugh commented 7 years ago

Accepting PRs: Disallow non-any/number/string operands on either side of <, >, etc.. Should be an easy change

Jessidhia commented 7 years ago

Should {} also be comparable?

At least I have used it in the past to indicate a "comparable" that was not any (i.e. couldn't do anything with it other than compare; using any would allow property access and other shenanigans).

DanielRosenwasser commented 7 years ago

Should {} also be comparable?

Could you elaborate on the scenario? What was the run-time value supposed to be?

Jessidhia commented 7 years ago

In this specific case, it was a function that received a function that produces an ordered list of sorting keys; where the value of the sorting keys are anything comparable. As the function doesn't need the keys to be anything in particular (besides comparable), they were typed as {} (instead of any). At the time that included both numbers and Date objects.

Speaking of which, are Date objects comparable because of spec magic, or because of Symbol.toPrimitive 🤔

RyanCavanaugh commented 7 years ago

Speaking of which, are Date objects comparable because of spec magic

Extreme spec magic; see https://github.com/Microsoft/TypeScript/issues/2361#issuecomment-82509802

FabianLauer commented 7 years ago

I've just taken a stab at this issue. The conformance tests for comparison operators work as expected. However, a few tests still fail, such as:

// asyncFunctionDeclaration5_es5.ts

    async function foo(await): Promise<void> {
                       ~~~~~
!!! error TS1138: Parameter declaration expected.
                       ~~~~~
!!! error TS2304: Cannot find name 'await'.
                            ~
!!! error TS1005: ';' expected.
                             ~
!!! error TS1128: Declaration or statement expected.
                               ~~~~~~~~~~~~~~~
                                       ~~~~
!!! error TS2532: Object is possibly 'undefined'.
                                           ~
!!! error TS1109: Expression expected.
    }
    ~
!!! error TS2365: Operator '>' cannot be applied to types 'boolean' and '{}'.

... or ...

// parserTypeAssertionInObjectCreationExpression1.ts

    new <T>Foo()
    ~~~~~~~~~~~~
!!! error TS2365: Operator '>' cannot be applied to types 'boolean' and 'any'.
        ~
!!! error TS1109: Expression expected.
         ~
!!! error TS2304: Cannot find name 'T'.
           ~~~
!!! error TS2304: Cannot find name 'Foo'.

Am I right to assume that this is an issue with (graceful) parsing? If so, should I deal with this in a new issue?

I've not opened a PR yet, BTW. Can do though if you prefer to. Any guidance is much appreciated 🙂

mhegazy commented 7 years ago

Am I right to assume that this is an issue with (graceful) parsing? If so, should I deal with this in a new issue?

yes.

gcnew commented 7 years ago

Maybe late to the party, but I think Date should be comparable as well. It's true that the arithmetic operators yield strange results, but AFAIK the relational ones do not and are frequently used in JS sources.

staeke commented 6 years ago

@gcnew Came here from issue #17119

As the comments stand now, I don't think this issue is a clear duplicate, and if we're keeping addressing that issue in this thread, then at least I want to make it clear. As the reporter @adius said, it would be nice if typescript produced a warning for below (d1, d2 are Dates)

d1 === d2
d1 == d2

As @gcnew pointed out <, >, <=, >= work fine

Richard-Lynch commented 5 years ago

Working on this, I’ll follow up with a PR via Bloomberg in the by Monday

HersiYussuf commented 4 months ago

I would love to work on this if no one hasn't already or working on it now with a bit a of help!!

HersiYussuf commented 4 months ago

Working on this, I’ll follow up with a PR via Bloomberg in the by Monday

I want to work with on it, can I ?