Skip to content

Missing compiler warning on if-clause #5109

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

Closed
HJoost opened this issue Oct 5, 2015 · 8 comments
Closed

Missing compiler warning on if-clause #5109

HJoost opened this issue Oct 5, 2015 · 8 comments
Labels
Fixed A PR has been merged for this issue Good First Issue Well scoped, documented and has the green light Help Wanted You can do this Suggestion An idea for TypeScript

Comments

@HJoost
Copy link

HJoost commented Oct 5, 2015

Hi,

last week I made a mistake while typing an if-clause:

if(x === false);
{...}

When I debugged it, the code inside the brackets got hit every time. For many code runs I did not see the wrong semicolon after the if-statement.

For me, a compiler warning about the misplaced semicolon would be great, like the C# compiler does it.

Helmut

@DanielRosenwasser
Copy link
Member

While I am dubious of any actual use case for that sort of code, a good workaround for now is to use TSLint: https://github.com/palantir/tslint

The "curly" tslint rule would have caught this for you.

@HJoost
Copy link
Author

HJoost commented Oct 5, 2015

Thanks for your reply.

Indeed this type of code should not be used, it was just a mistake and I wondered why the if path got hit independent of the value of x... (I think you how easy it is to overread such a little mistake while debugging ;-) ).

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Out of Scope This idea sits outside of the TypeScript language design constraints labels Oct 5, 2015
@RyanCavanaugh
Copy link
Member

I'm not aware of any C-syntax languages that flag this as an error. +1 for TSLint.

I remember spending a long time in college debugging this infinite loop 😝

int x = 0;
while (x < 10); {
  x++;
}

@HJoost
Copy link
Author

HJoost commented Oct 6, 2015

c semikolon

This is the way how it looks in my Studio with C#, sadly it doesn't in typescript :-( (German for: "Potentially wrong or empty assignment")

@mhegazy
Copy link
Contributor

mhegazy commented Oct 6, 2015

i do think there is room for making this an error. it is clearly wrong, or at best very bad pattern to follow, in the case of the if statement, and can be detected in cases of while/for cases if we limit it to expressions with side-effects (let's say only assignments, and call expressions).

@DanielRosenwasser
Copy link
Member

i do think there is room for making this an error.

Yes, I somewhat agree; unlike a while loop, I don't see how this could ever be useful.

@danquirk danquirk reopened this Oct 7, 2015
@danquirk danquirk added In Discussion Not yet reached consensus and removed Out of Scope This idea sits outside of the TypeScript language design constraints labels Oct 7, 2015
@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this and removed In Discussion Not yet reached consensus labels Oct 19, 2015
@RyanCavanaugh RyanCavanaugh added this to the Community milestone Oct 19, 2015
@RyanCavanaugh
Copy link
Member

Accepting PRs. The only rule we want is that the "true" arm of an if cannot be the EmptyStatement production.

@mhegazy mhegazy added the Good First Issue Well scoped, documented and has the green light label Oct 19, 2015
@mhegazy mhegazy modified the milestones: TypeScript 1.8, Community Oct 28, 2015
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Oct 28, 2015
@mhegazy mhegazy closed this as completed Oct 28, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Oct 28, 2015

thanks @MartyIX!

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed A PR has been merged for this issue 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

No branches or pull requests

5 participants