facebook / flow

Adds static typing to JavaScript to improve developer productivity and code quality.
https://flow.org/
MIT License
22.1k stars 1.86k forks source link

String indexes aren't treated as possibly undefined values #2140

Closed jamiebuilds closed 8 years ago

jamiebuilds commented 8 years ago

Is there a reason that string indexes aren't treated as possibly undefined values?

const string = 'abc';
const char = string[3];
char.charAt(0);
// Flow: No errors!
// Runtime: TypeError: Cannot read property 'charAt' of undefined
samwgoldman commented 8 years ago

Yeah, Flow doesn't do bounds checking at all. The same is true of arrays. This is a design choice, although I can definitely see both sides.

It would actually be possible to statically determine the index operation on your code example, because we statically know the string value and the index value, but I imagine that is very rare. Still, might be worth exploring early errors where we can show that an index is out of bounds statically.

Overall, we need to weigh the cost/benefit here. Should all string index operations be possibly undefined? Array index operations? Programmers are generally good at writing this kind of code safely, so my feeling is that such a rule would produce false positives more often than not.

jamiebuilds commented 8 years ago

Just wondering about it because it seems like an inconsistency amongst the strictness that Flow usually enforces.

I admit this would be quite annoying:

for (let i = 0; i < str.length; i++) {
  if (str[0]) {
    // ...
  }
}

Even an example like this could be able to statically tell that i cannot be outside the bounds of str (I'm not currently sure of how difficult of a task that is for Flow) however it's not hard to create an example that couldn't be resolved statically.

Would it be worth investing time in the future to be able to try and figure out the bounds of a string index getter? And if it can't be resolved require a check?

const char = str[Math.random() * 10];
char.charAt(0); // Flow: Cannot call `charAt` on a possibly undefined string.
samwgoldman commented 8 years ago

Strictness here is a continuum, and Flow's position really can't be "as strict as humanly possible" due to the dynamic nature of JS. I agree that we sit pretty far on the "strict" side of the spectrum, but you can only go so far before you start to make JS feel like Java.

Regarding bounds checking, the greatest value comes from tricky cases, not trivial ones which are easily detected statically. The tricky cases are pretty far beyond the type system's capabilities, although there is a wealth of interesting research in this area, specifically in dependently typed languages. In general, you need to construct and solve various interesting proofs.

You might be interested to check out Liquid Types, which actually plugs in an automated theorem prover and does make it possible to implement checks like this. Specifically this interactive page is a good way to waste a few hours :) http://goto.ucsd.edu:8090/index.html