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

Commit

Permalink
Fixes flow type scoping issues
Browse files Browse the repository at this point in the history 
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 #123
  • Loading branch information
@leebyron
leebyron committed Jun 9, 2015
1 parent f2800d0 commit 8fc39e9
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 48 deletions.
52 changes: 34 additions & 18 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,37 +162,49 @@ function monkeypatch() {
if (node.typeAnnotation) {
visitTypeAnnotation.call(this, node.typeAnnotation);
} else if (node.type === "Identifier") {
// exception for polymorphic types: <T>, <A>, etc
if (node.name.length === 1 && node.name === node.name.toUpperCase()) {
return;
}
this.visit(node);
} else {
visitTypeAnnotation.call(this, node);
}
}

function nestTypeParamScope(manager, node) {
var parentScope = manager.__currentScope;
var scope = new escope.Scope(manager, "type-parameters", parentScope, node, false);
manager.__nestScope(scope);
for (var j = 0; j < node.typeParameters.params.length; j++) {
var name = node.typeParameters.params[j];
scope.__define(name, new Definition("TypeParameter", name, name));
}
scope.__define = function() {
return parentScope.__define.apply(parentScope, arguments);
}
return scope;
}

// visit decorators that are in: ClassDeclaration / ClassExpression
var visitClass = referencer.prototype.visitClass;
referencer.prototype.visitClass = function(node) {
visitDecorators.call(this, node);
var typeParamScope;
if (node.typeParameters) {
typeParamScope = nestTypeParamScope(this.scopeManager, node);
}
// visit flow type: ClassImplements
if (node.implements) {
for (var i = 0; i < node.implements.length; i++) {
checkIdentifierOrVisit.call(this, node.implements[i]);
}
}
if (node.typeParameters) {
for (var j = 0; j < node.typeParameters.params.length; j++) {
checkIdentifierOrVisit.call(this, node.typeParameters.params[j]);
}
}
if (node.superTypeParameters) {
for (var k = 0; k < node.superTypeParameters.params.length; k++) {
checkIdentifierOrVisit.call(this, node.superTypeParameters.params[k]);
}
}
visitClass.call(this, node);
if (typeParamScope) {
this.close(node);
}
};
// visit decorators that are in: Property / MethodDefinition
var visitProperty = referencer.prototype.visitProperty;
Expand All @@ -207,6 +219,10 @@ function monkeypatch() {
// visit flow type in FunctionDeclaration, FunctionExpression, ArrowFunctionExpression
var visitFunction = referencer.prototype.visitFunction;
referencer.prototype.visitFunction = function(node) {
var typeParamScope;
if (node.typeParameters) {
typeParamScope = nestTypeParamScope(this.scopeManager, node);
}
if (node.returnType) {
checkIdentifierOrVisit.call(this, node.returnType);
}
Expand All @@ -218,12 +234,10 @@ function monkeypatch() {
}
}
}
if (node.typeParameters) {
for (var j = 0; j < node.typeParameters.params.length; j++) {
checkIdentifierOrVisit.call(this, node.typeParameters.params[j]);
}
}
visitFunction.call(this, node);
if (typeParamScope) {
this.close(node);
}
};

// visit flow type in VariableDeclaration
Expand Down Expand Up @@ -261,13 +275,15 @@ function monkeypatch() {

referencer.prototype.TypeAlias = function(node) {
createScopeVariable.call(this, node, node.id);
var typeParamScope;
if (node.typeParameters) {
typeParamScope = nestTypeParamScope(this.scopeManager, node);
}
if (node.right) {
visitTypeAnnotation.call(this, node.right);
}
if (node.typeParameters) {
for (var i = 0; i < node.typeParameters.params.length; i++) {
checkIdentifierOrVisit.call(this, node.typeParameters.params[i]);
}
if (typeParamScope) {
this.close(node);
}
};

Expand Down
97 changes: 67 additions & 30 deletions test/non-regression.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@ describe("verify", function () {
verifyAndAssertMessages([
"import type Foo from 'foo';",
"import type Foo2 from 'foo';",
"function log<Foo, Foo2>() {}",
"log();"
"function log<T1, T2>(a: T1, b: T2) { return a + b; }",
"log<Foo, Foo2>(1, 2);"
].join("\n"),
{ "no-unused-vars": 1, "no-undef": 1 },
[]
Expand Down Expand Up @@ -276,9 +276,8 @@ describe("verify", function () {
it("type alias with type parameters", function () {
verifyAndAssertMessages([
"import type Bar from 'foo';",
"import type Foo2 from 'foo';",
"import type Foo3 from 'foo';",
"type Foo<Foo2> = Bar<Foo3>",
"type Foo<T> = Bar<T, Foo3>",
"var x : Foo = 1; x;"
].join("\n"),
{ "no-unused-vars": 1, "no-undef": 1 },
Expand Down Expand Up @@ -316,6 +315,62 @@ describe("verify", function () {
);
});

it("polymorphpic/generic types for class #123", function () {
verifyAndAssertMessages([
"class Box<T> {",
"value: T;",
"}",
"var box = new Box();",
"console.log(box.value);"
].join("\n"),
{ "no-unused-vars": 1, "no-undef": 1 },
[]
);
});

it("polymorphpic/generic types for function #123", function () {
verifyAndAssertMessages([
"export function identity<T>(value) {",
"var a: T = value; a;",
"}"
].join("\n"),
{ "no-unused-vars": 1, "no-undef": 1 },
[]
);
});

it("polymorphpic/generic types for type alias #123", function () {
verifyAndAssertMessages([
"import Bar from './Bar';",
"type Foo<T> = Bar<T>; var x: Foo = 1; x++"
].join("\n"),
{ "no-unused-vars": 1, "no-undef": 1 },
[]
);
});

it("polymorphpic/generic types - outside of fn scope #123", function () {
verifyAndAssertMessages([
"export function foo<T>(value) {",
"};",
"var b: T = 1; b;"
].join("\n"),
{ "no-unused-vars": 1, "no-undef": 1 },
[ '1:20 T is defined but never used no-unused-vars',
'3:7 "T" is not defined. no-undef' ]
);
});

it("polymorphpic/generic types - extending unknown #123", function () {
verifyAndAssertMessages([
"import Bar from 'bar';",
"export class Foo extends Bar<T> {}",
].join("\n"),
{ "no-unused-vars": 1, "no-undef": 1 },
[ '2:29 "T" is not defined. no-undef' ]
);
});

it("1", function () {
verifyAndAssertMessages(
[
Expand Down Expand Up @@ -413,9 +468,7 @@ describe("verify", function () {
it("9", function () {
verifyAndAssertMessages(
[
"import type Foo from 'foo';",
"import type Foo2 from 'foo';",
"export default function <Foo, Foo2>() {}"
"export default function <T1, T2>(a: T1, b: T2) {}"
].join("\n"),
{ "no-unused-vars": 1, "no-undef": 1 },
[]
Expand All @@ -425,9 +478,7 @@ describe("verify", function () {
it("10", function () {
verifyAndAssertMessages(
[
"import type Foo from 'foo';",
"import type Foo2 from 'foo';",
"var a=function<Foo,Foo2>() {}; a;"
"var a=function<T1,T2>(a: T1, b: T2) {return a + b;}; a;"
].join("\n"),
{ "no-unused-vars": 1, "no-undef": 1 },
[]
Expand All @@ -437,10 +488,7 @@ describe("verify", function () {
it("11", function () {
verifyAndAssertMessages(
[
"import type Foo from 'foo';",
"import type Foo2 from 'foo';",
"import type Foo3 from 'foo';",
"var a={*id<Foo>(x: Foo2): Foo3 { x; }}; a;"
"var a={*id<T>(x: T): T { x; }}; a;"
].join("\n"),
{ "no-unused-vars": 1, "no-undef": 1 },
[]
Expand All @@ -450,10 +498,7 @@ describe("verify", function () {
it("12", function () {
verifyAndAssertMessages(
[
"import type Foo from 'foo';",
"import type Foo2 from 'foo';",
"import type Foo3 from 'foo';",
"var a={async id<Foo>(x: Foo2): Foo3 { x; }}; a;"
"var a={async id<T>(x: T): T { x; }}; a;"
].join("\n"),
{ "no-unused-vars": 1, "no-undef": 1 },
[]
Expand All @@ -463,10 +508,7 @@ describe("verify", function () {
it("13", function () {
verifyAndAssertMessages(
[
"import type Foo from 'foo';",
"import type Foo2 from 'foo';",
"import type Foo3 from 'foo';",
"var a={123<Foo>(x: Foo2): Foo3 { x; }}; a;"
"var a={123<T>(x: T): T { x; }}; a;"
].join("\n"),
{ "no-unused-vars": 1, "no-undef": 1 },
[]
Expand Down Expand Up @@ -597,10 +639,8 @@ describe("verify", function () {
it("24", function () {
verifyAndAssertMessages(
[
"import type Foo from 'foo';",
"import type Foo2 from 'foo';",
"import Baz from 'foo';",
"export default class Bar<Foo> extends Baz<Foo2> { };"
"import type Baz from 'baz';",
"export default class Bar<T> extends Baz<T> { };"
].join("\n"),
{ "no-unused-vars": 1, "no-undef": 1 },
[]
Expand All @@ -610,10 +650,7 @@ describe("verify", function () {
it("25", function () {
verifyAndAssertMessages(
[
"import type Foo from 'foo';",
"import type Foo2 from 'foo';",
"import type Foo3 from 'foo';",
"export default class Bar<Foo> { bar<Foo2>():Foo3 { return 42; }}"
"export default class Bar<T> { bar(): T { return 42; }}"
].join("\n"),
{ "no-unused-vars": 1, "no-undef": 1 },
[]
Expand Down

0 comments on commit 8fc39e9

Please sign in to comment.