-
Notifications
You must be signed in to change notification settings - Fork 12.8k
This condition will always return 'false' since the types have no overlap. #25642
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
Comments
Another case for classes without interfaces
|
//cc @RyanCavanaugh and @DanielRosenwasser for error message details. |
The rule for the type assertion is that the compiler can verify that either of the two types is comparable to the other. In simpler terms, can you assign one to the other either upcast or downcast. if neither is true, then it is likely an error.. e.g. |
On one hand, it looks like a code smell to compare types like this. On the other, if such a check passes, I would expect it to behave like some sort of type guard, where both |
In simple cases yes, looks good. But in some cases it alerts on absolutely correct code. Found this case in complicated code that works with heirechical system using abstract interfaces. All parent - child links is interfaces, and nodes implemets this interfaces. |
The current rule covers a large class of errors with good precision; it will necessarily have some false positives but we have carefully considered this and are satisfied with the current balance of value vs "incorrect" errors. |
@RyanCavanaugh And how do you propose to correct these |
@AntiMoron [EDIT] If |
@AnyhowStep Thank you...I redefined |
@AntiMoron How did you even get that to work? o.0
https://nodejs.org/api/process.html#process_process_env Are you using |
How about giving us the ability to silence an error - in my case I have a build script that generates code that contains constants based on the target platform (browser, android, ios...), and in this case |
|
Derp. Seems my google-fu failed me again. Thank you, Ryan. |
If you're using node, it's process.env.NODE_ENV. It's a string.
process.env is an interface ProcessEnv with please use |
Here's another variant of this problem, in case it's useful. I have code where const x = <T extends SomeType, U extends SomeType> (t: T, u: U) => {
if (t === u) {
...
} else {
...
}
} |
Here's another simplified example: import { Foo, Bar } from './foo'
class Baz implements Foo, Bar {
foo() {}
bar() {}
}
const baz = new Baz()
const foo: Foo = baz
const bar: Bar = baz
if (foo === bar) { // ERROR!
} foo.ts: export interface Foo {
foo(): void
}
export interface Bar {
bar(): void
} The comparison Surprisingly, this error only appears if the interfaces are in a different module. |
What irks me here is that the error message implies that the code is provably incorrect ("This condition will always return 'false'"). It is clearly demonstrated that such a comparison may be perfectly valid. The premise of the error is not factual, it's heuristic, and that should be reflected in the error message. "This condition may always return 'false' since the declared types have no overlap." It might sound like a nitpick, but the phrasing cues the user on how to resolve the issue. It should reduce confusion instead of creating confusion. |
@niezw A correct answer was already provided above. All you're doing is misusing a library function because it happens to circumvent the type system and suppress an error message, when in fact the error message is correctly identifying a problem with the program logic.
|
Cast whatever value to any like this
You can then compare as normal |
@prog-24 The problem isn't that we don't know how to work around the error. It's an issue of whether the error is meaningful and instructive. Blindly casting is asking for trouble. In the above case of |
@snarfblam I think it is one of those compiler things that are really annoying but somehow necessary evil. I had a similar issues that caused me to produce this work around. There are times when yes you can make the necessary changes but if you are sure that it would not cause any issues for instance a simple I think the error message is clear but unhelpful which I think is what you are pointing out. |
Well, no. I think it is helpful but unclear. As I said above, the error is heuristic (the heuristic being that comparing two values whose declared types have no overlap is moderately indicative of a potentially hard-to-diagnose programmer mistake—very helpful), and it could be more clearly stated along the lines of, "This condition may always return 'false' since the declared types have no overlap." Currently the error misstates the problem, making it more difficult to understand the root cause. Typescript errors can be confusing to parse (it's not that unusual to get a single error that is several lines of deeply nested symbol soup). If the error message isn't clear and accurate, it can be extremely difficult to work out. I'm imagining a scenario where a programmer develops a knee-jerk habit of casting to |
This is a very poorly worded "error". For example the following code: |
I got the same error-message with the following code:
On the x===0 condition I got the error, which was strange in my view because I was sure the x is either 0 or 1.
And it solved the problem. |
|
This is just another example of someone introducing a bug into their program to try to fix this error message. There is generally no good reason to use @RyanCavanaugh Is there any chance we can re-open this issue to consider re-wording the error message? I think it should convey:
Perhaps something along the lines of:
|
@NeliHarbuzava Your code is wrong, it currently always returns if (typeof imageFormat !== 'string' || (imageFormat !== 'png' && imageFormat !== 'jpg')) {
// ^^ problem solved
console.warn('Invalid "imageFormat" property. Expected one of: "png", "jpg".');
} |
This demonstrates the actual utility of the error message. |
@tenedor - thanks for the succinct MWE - I'm in the same situation.
One simple workaround is to set |
As far as I can tell, this is still an issue. As @kevincox points out, the simple expression |
See #26592 |
@RyanCavanaugh I guess I don't see how that answers the question. I can see the use of pointing out the potential bug-iness of using |
Yes, and the linked issue says that we'd love a PR to change the language of that error message. |
Ahhh okay, sorry for the confusion. Thank you! |
@RyanCavanaugh could you point me to where this check happens in the code? |
TypeScript Version: 3.0.0-dev.20180712
Error: This condition will always return 'false' since the types 'A' and 'I' have no overlap.
TypeScript Version: 2.9.2
Error: Operator '===' cannot be applied to types 'A' and 'I'
Search Terms:
Operator '===' cannot be applied to types
This condition will always return 'false' since the types have no overlap.
Code
Expected behavior:
no error
Actual behavior:
error
Playground Link: link
Related Issues: no
This case found from real project. Class and interface may be implemented and extended by another class, and in this case this error is pointless.
The text was updated successfully, but these errors were encountered: