CODEWITHSUNDEEP
flowtype
Marcus Junius Brutus
Marcus Junius Brutus

Reputation: 27306

dynamic type tests not working as expected

This is a SSCCE.

I have a map container class where an internal Map is created on demand with the first set method that gets called:

// @flow
'use strict';

class MapContainer {

    map: ?Map<any, any>;

    constructor() {
        this.map=null;
    }

    set(key: any, value: any): ?any {
        if (this.map===null) {
            this.map = new Map();
        }
        let prevValue: ?any;
        if (this.map!=null) { // first check
            prevValue = this.map.get(key);
        }
        if (this.map!=null) { // second check
            this.map.set(key, value);
        }
        return prevValue;
    }
}    
exports.MapContainer = MapContainer;

The above code passes npm run flow with no warnings.

However if I merge the two if (this.map!=null) checks into one:

// @flow
'use strict';

class MapContainer {

    map: ?Map<any, any>;

    constructor() {
        this.map=null;
    }

    set(key: any, value: any): ?any {
        if (this.map===null) {
            this.map = new Map();
        }
        let prevValue: ?any;
        if (this.map!=null) { // merged check
            prevValue = this.map.get(key);
            this.map.set(key, value);
        }
        return prevValue;
    }
}    
exports.MapContainer = MapContainer;

… then running flow fails with the following message:

es6/map-container.js:19
 19:                 this.map.set(key, value);
                 ^^^^^^^^^^^^^^^^^^^^^^^^ call of method `set`. Method cannot be called on possibly null value
 19:                 this.map.set(key, value);
                 ^^^^^^^^ null

es6/map-container.js:19
 19:                 this.map.set(key, value);
                 ^^^^^^^^^^^^^^^^^^^^^^^^ call of method `set`. Method cannot be called on possibly undefined value
 19:                 this.map.set(key, value);
                 ^^^^^^^^ undefined

… which makes no sense at all as the access on line 19:

this.map.set(key,value)

… is still covered by the check:

if (this.map!=null)

What gives?

Upvotes: 2

Views: 217

Answers (1)

vkurchatkin
vkurchatkin

Reputation: 13570

The problem is that call to get method can invalidates refinement. What if get sets this.map to null? Flow has no way of knowing, so it assumes the worst. Here is what you can do:

class MapContainer {

    map: ?Map<any, any>;

    constructor() {
        this.map=null;
    }

    set(key: any, value: any): ?any {     
        if (!this.map) {
            this.map = new Map();
        }

        const map = this.map;

        let prevValue: ?any;
        if (this.map!=null) {
            prevValue = map.get(key);
            map.set(key, value);
        }
        return prevValue;
    }
}

Upvotes: 1

Related Questions