Skip to content
This repository has been archived by the owner on Aug 18, 2021. It is now read-only.

Flow generic class members #123

Closed
leebyron opened this issue Jun 8, 2015 · 7 comments · Fixed by #126
Closed

Flow generic class members #123

leebyron opened this issue Jun 8, 2015 · 7 comments · Fixed by #126
Labels

Comments

@leebyron
Copy link
Contributor

leebyron commented Jun 8, 2015

babel-eslint currently produces "unused" errors for valid flow-typed javascript when using generics with class members:

function identity<T>(value: T): T {
  return value;
}
console.log(identity('foo'));

class Box<T> {
  value: T;
  constructor(value: T) {
    this.value = T;
  }
}
var box = new Box();
console.log(box.value);

Most uses of the generic seem to be fine, but typing the class members fails lint.

   7:9  error  "T" is not defined             no-undef
@hzoo
Copy link
Member

hzoo commented Jun 8, 2015

I guess that last patch didn't cut it haha.

Kind of unrelated but is the feature called "generic types?" #109 (comment) used "polymorphic types" and that's what the flow docs were saying http://flowtype.org/docs/functions.html#polymorphic-functions. Generics makes sense too but I was just wondering.

@hzoo hzoo added the bug label Jun 8, 2015
@leebyron
Copy link
Contributor Author

leebyron commented Jun 8, 2015

Ah yeah, Flow uses "polymorphic types" to describe this. The concepts are more or less interchangeable in this context though :)

@hzoo
Copy link
Member

hzoo commented Jun 8, 2015

Ah so the reason is because I'm just ignoring/not visiting the those types.
Seems like 3 types to account for: built-in types like number, boolean, any, mixed, etc, user defined/imported types like x: Foo, and now these polymorphic ones.

Looks like we'll need to create a variable in the scope whenever there is a polymorphic type to fix this no-undef and then visit all types as normal to prevent the related no-unused-vars issue.

@leebyron
Copy link
Contributor Author

leebyron commented Jun 8, 2015

That sounds like a great way to handle this. function and class are the places polymorphic types can be introduced, you could just create a new scope for each and supply the polymorphic types in that scope

@sebmck
Copy link
Contributor

sebmck commented Jun 8, 2015

Might be worth looking into shimming escope with Babel's scope tracker.

@hzoo
Copy link
Member

hzoo commented Jun 8, 2015

@leebyron Kind of doing something like that now for type definitions https://github.com/babel/babel-eslint/blob/master/index.js#L249-L263
as well as in comprehensions

  function createScopeVariable (node, name) {
    this.currentScope().variableScope.__define(name,
      new Definition(
        "Variable",
        name,
        node,
        null,
        null,
        null
      )
    );
  }

  referencer.prototype.TypeAlias = function(node) {
    createScopeVariable.call(this, node, node.id);
   ...
}

@sebmck Ok but will probably need some guidance 😄 Looks like it's https://github.com/babel/babel/tree/master/src/babel/traversal/scope?

@sebmck
Copy link
Contributor

sebmck commented Jun 8, 2015

@hzoo

Ok but will probably need some guidance 😄

Was just an idea 😄 It's likely non-trivial (escope has a bizzare format and architecture) so don't feel pressured.

hzoo added a commit to hzoo/babel-eslint that referenced this issue Jun 9, 2015
leebyron added a commit to leebyron/babel-eslint that referenced this issue Jun 9, 2015
This adds a special escope Scope when function, class, and typealias definitions include typeParameters which includes those parameters for reference, but is write-through to it's parent when encountering new variables. This is a little weird, but seems as good as we can while monkey-patching escope.

This also fixes some tests which had invalid flow but were written to expect no errors.

Fixes babel#123
leebyron added a commit to leebyron/babel-eslint that referenced this issue Jun 9, 2015
This adds a special escope Scope when function, class, and typealias definitions include typeParameters which includes those parameters for reference, but is write-through to it's parent when encountering new variables. This is a little weird, but seems as good as we can while monkey-patching escope.

This also fixes some tests which had invalid flow but were written to expect no errors.

Fixes babel#123
@hzoo hzoo closed this as completed in #126 Jun 9, 2015
hzoo added a commit that referenced this issue Jun 9, 2015
Fixes flow type scoping issues - fixes #123
nicolo-ribaudo pushed a commit to babel/babel that referenced this issue Nov 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants