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

Failed check on function in if clause #19026

Closed
huhuanming opened this issue Oct 9, 2017 · 7 comments
Closed

Failed check on function in if clause #19026

huhuanming opened this issue Oct 9, 2017 · 7 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@huhuanming
Copy link

TypeScript Version: 2.5.3

Code

if (Math.floor(1.2), 2) {
   // do something
}

Expected behavior:

error TS2695: Left side of comma operator is unused and has no side effects.

Actual behavior:

image

Nothing happened.

But can get error like this.

image

@huhuanming huhuanming changed the title failed Check on Function in if clause Failed check on function in if clause Oct 9, 2017
@kitsonk
Copy link
Contributor

kitsonk commented Oct 9, 2017

There are limits to things like call flow analysis, and this would be one of them. While it might be potentially a logic error, what is a real world use case where this would occur. CFA comes at a cost, and if this is not a common logic error, it is unlikely for it to be added to the compiler.

@huhuanming
Copy link
Author

huhuanming commented Oct 9, 2017

But I really did take a very long time to find this error in real world.

As you can see, I just made a example.

@huhuanming
Copy link
Author

And Why if (1, 2 is error, but if (Math.floor(1.2), 2) can pass.

@kitsonk
Copy link
Contributor

kitsonk commented Oct 9, 2017

A simplified example is always good, but again, that is not very realistic or sensible code. If you provided an example that was more realistic, then it might help identify how TypeScript could help in statically identifying issues.

There are trade-offs with CFA (see #9998). This would require the ability to describe the return types of functions in truthy and un-truthy values to be evaluated at run time to produce the same error. While Math.floor() is a built in function, the way TypeScript deals with it is like any other return, which is that it returns a number and when it does the analysis it cannot determine if that number will be 0 (falsey) or != 0 (truthy).

On the other hand, providing more than one statement to an if () expression is likely a static error that TypeScript could detect... I am not really sure any construct like if (foo, bar) { } would ever be desirable, even if it is valid syntax.

@ghost
Copy link

ghost commented Oct 9, 2017

This is unrelated to if.

Math.floor(1.2), 2; // No error
1, 2; // Error

Note that the error message says "and has no side effects". TypeScript doesn't track what functions have side effects, so this error will only happen for an expression that obviously has no side effects such as null -- never for a function call.
You may be interested in palantir/tslint#3250.

@huhuanming
Copy link
Author

Thx,I got it.
😄

@mhegazy mhegazy added the Working as Intended The behavior described is the intended behavior; this is not a bug label Oct 9, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Oct 23, 2017

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@mhegazy mhegazy closed this as completed Oct 23, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants