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

Flag boolean in comparison operator operands as errors #15506

Open
mhegazy opened this issue May 1, 2017 · 13 comments · May be fixed by #56666
Open

Flag boolean in comparison operator operands as errors #15506

mhegazy opened this issue May 1, 2017 · 13 comments · May be fixed by #56666
Labels
Good First Issue Well scoped, documented and has the green light Help Wanted You can do this Suggestion An idea for TypeScript
Milestone

Comments

@mhegazy
Copy link
Contributor

mhegazy commented May 1, 2017

From #15444 (comment)

var x: boolean, y: boolean;
if (x > y) {  // suspicious!
}
@dmichon-msft
Copy link
Contributor

Well, that is the most concise way of writing:

let x: boolean, y: boolean;
if (x && !y) {
}

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this and removed In Discussion Not yet reached consensus labels May 8, 2017
@RyanCavanaugh RyanCavanaugh added this to the Community milestone May 8, 2017
@RyanCavanaugh RyanCavanaugh added the Good First Issue Well scoped, documented and has the green light label May 8, 2017
@RyanCavanaugh
Copy link
Member

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

@Jessidhia
Copy link

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
Copy link
Member

Should {} also be comparable?

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

@Jessidhia
Copy link

Jessidhia commented May 10, 2017

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
Copy link
Member

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

Extreme spec magic; see #2361 (comment)

@FabianLauer
Copy link

FabianLauer commented Jun 12, 2017

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
Copy link
Contributor Author

mhegazy commented Jun 12, 2017

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
Copy link
Contributor

gcnew commented Jun 14, 2017

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
Copy link

staeke commented Feb 16, 2018

@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
Copy link

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

@HersiYussuf
Copy link

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
Copy link

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 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Well scoped, documented and has the green light Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants