Flow: Incorrect static class function signature check

Created on 10 Feb 2016  路  2Comments  路  Source: facebook/flow

When flow attempts to parse a declaration (library) file that contains a base and derived class that have identically named static functions (for example, when each class has a factory), it incorrectly attempts to enforce the same type signature on both functions. For example:

// decl.js
declare class Base {
    static create(aString:string):Base;
}

declare class Derived extends Base {
    static create(aNumber:number):Derived;
}

If you place that in a flow [lib] folder, it will error with:

lib/decl.js:0
  0: 
     ^ Library type error:
  0: 
     ^ inconsistent use of library definitions
  2:     static create(aString:string):Base;
                               ^^^^^^ string. This type is incompatible with
  6:     static create(aNumber:number):Derived;
                               ^^^^^^ number

In normal inheritance, of course you want derived functions to have the same signature as parent functions in the same tree. But for _static_ functions, there shouldn't be enforced signatures. Among other things, existing libraries sometimes use a "create" factory pattern for each level of the tree.

I'm not saying I approve of this pattern; the constructor is there for a reason, after all. But sometimes we have to deal with existing libraries.

Most helpful comment

I appreciate the time you took to look at this issue.

But from an OO design perspective, I strongly disagree that just because derived classes inherit the static methods, that they therefore must be forced to have identical signatures. The create pattern above may need new parameters in a subtype. That's why a constructor can have different parameters on each level in a hierarchy.

There should at least be a way to flag that it's OK for a particular function to change its signature, to opt-in to a different policy than the one you're prescribing. Just because using constructors for this is probably better from a "pure language" point of view doesn't mean that all legacy code libraries that one might want to use will suddenly conform to that model.

I can't use your suggested workaround, either. If the classes are modeled as non-inheriting, then I can't pass a derived class in a place that requires a base class. Instead I am forced to model ALL of the functions that follow that pattern as accepting (p:...any) and returning any, losing any type safety for all of them. Or I have to just set attributes on every class that takes ANY of these classes (and there are a few dozen in the hierarchy) as a parameter as accepting any, losing even more type safety.

In this particular class hierarchy, they also override other functions than just "create." Sometimes you have a call common to different levels in the hierarchy that is intended only to be called on the most-derived class.

If you want to encourage adoption, then allowing people to use FlowType in ways that you don't consider to be "the one true approach to OO design" is going to be critical. I appreciate that you have an opinion on the ways things _should_ be done. I don't even disagree with your opinion. But one of FlowType's strength is in being able to use it with existing code. Forcing "any" as the only possible workaround seems like a huge handicap.

All 2 comments

Derived classes do inherit static methods from their parent. So for the same reason we need instance objects of the derived class to follow sound subtyping rules for instance objects of the parent, we also need instances of the derived _class_ object to follow sound subtyping rules for instances of the parent _class_ object.

It's certainly true that some libraries have inheritance patterns that don't follow safe subtyping rules (both for static and instance members), but we don't want to compromise all of the assumptions we can make elsewhere given sound subtyping relationships -- so the best bet might be to either more loosely type those libraries or model them via a libdef as distinct (non-inherting) classes.

I appreciate the time you took to look at this issue.

But from an OO design perspective, I strongly disagree that just because derived classes inherit the static methods, that they therefore must be forced to have identical signatures. The create pattern above may need new parameters in a subtype. That's why a constructor can have different parameters on each level in a hierarchy.

There should at least be a way to flag that it's OK for a particular function to change its signature, to opt-in to a different policy than the one you're prescribing. Just because using constructors for this is probably better from a "pure language" point of view doesn't mean that all legacy code libraries that one might want to use will suddenly conform to that model.

I can't use your suggested workaround, either. If the classes are modeled as non-inheriting, then I can't pass a derived class in a place that requires a base class. Instead I am forced to model ALL of the functions that follow that pattern as accepting (p:...any) and returning any, losing any type safety for all of them. Or I have to just set attributes on every class that takes ANY of these classes (and there are a few dozen in the hierarchy) as a parameter as accepting any, losing even more type safety.

In this particular class hierarchy, they also override other functions than just "create." Sometimes you have a call common to different levels in the hierarchy that is intended only to be called on the most-derived class.

If you want to encourage adoption, then allowing people to use FlowType in ways that you don't consider to be "the one true approach to OO design" is going to be critical. I appreciate that you have an opinion on the ways things _should_ be done. I don't even disagree with your opinion. But one of FlowType's strength is in being able to use it with existing code. Forcing "any" as the only possible workaround seems like a huge handicap.

Was this page helpful?
0 / 5 - 0 ratings