Flow: Index property access on objects as maps should require refinement

[vicapow]

js declare var s: {[string]: number}; s.foo.toFixed(); // No error ?!

[wchargin]

Why should your example error? Isn’t it a by-design concession of Flow
that indexer subscripting is unchecked?

If I have s: {[string]: number}, is the proposal that s["foo"] be a
type error, or that it have type number | void, or something else?

[vicapow]

@wchargin sorry I updated the title to clarify. Is that better? It may be that this isn't / shouldn't be supported but I wanted to create a single ticket to group related issues

[wchargin]

I still don’t understand:

1. It’s declared that s: {[string]: number}.
2. "foo" is a literal of type string
3. s["foo"] should thus be a number
4. number admits toFixed(), so s["foo"].toFixed() is valid
5. s["foo"] is identical to s.foo, so s.foo.toFixed() is valid

With which of the above claims do you disagree, and why? What needs to
be refined?

[vicapow]

1. s["foo"] should be a number.

Should be void | number and so accessing the property should require refinement.

js const s: {[string]: number} = {}; s.foo.toFixed(); // No error ?!

[jamesisaac]

You can just explicitly type it as number | void if you want to force refinement on the indexer. That's what I do if I want to have more safety, despite the increased verbosity:

const s: {[string]: number | void} = {};
s.foo.toFixed(); // Error

if (s.foo) {
  s.foo.toFixed(); // No error
}

I think having it opt-in like this is a reasonable approach. There are plenty of uses of indexers where it's clear enough that the usage isn't going lead to missing indexes, so requiring lots of refinement checks would just be adding noise rather than helping.

[wchargin]

1. s["foo"] should be a number.

Should be void | number and so accessing the property should require
refinement.

Okay, so, just to be clear: this is proposing a huge, wide-reaching
change to Flow. Arrays are just objects indexed by number, so
this change would imply that array access is no longer intentionally
unsound. As @mrkev said in #6823, this opens a can of worms. :-)

I’m not necessarily opposed from a purity perspective; I just want to
make sure that we’re on the same page about the implications of this:
this would easily require thousands of changes for medium-to-large
projects.

[vicapow]

@wchargin I think we're on the same page. This is a top level ticket for related issues being reported. The creation of this ticket should not imply this feature will be implemented.

[villesau]

@vicapow is this what you are talking about: https://github.com/facebook/flow/issues/6980

[zeorin]

For both object properties as well as array access, this could be implemented as a strict mode option. That way it's opt-in and only affects those who care.

[lyleunderwood]

Just got bit by this, and would love to have an explicit option for more soundness here.