-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Support type variables in MaD typings #10205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a really hard figuring out how each of the predicates work, but I think it makes sense now.
(I did a whole lot of quick-evals using your test-case).
But there's one part that turned out empty in the quick-eval, and that part I still don't get.
pragma[nomagic] | ||
private predicate typeStepModel(string package, string type, AccessPath basePath, AccessPath output) { | ||
summaryModel(package, type, basePath, "", output, "type") | ||
} | ||
|
||
pragma[nomagic] | ||
private predicate typeStep(API::Node pred, API::Node succ) { | ||
exists(string package, string type, AccessPath basePath, AccessPath output | | ||
typeStepModel(package, type, basePath, output) and | ||
pred = getNodeFromPath(package, type, basePath) and | ||
succ = getNodeFromSubPath(pred, output) | ||
) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really get these, and they also don't seem to be used in your tests?
A comment in getNodeFromPath()
says that typeStep()
is a receiver step, but I don't see where that comes from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, there really should have been tests for this. Added some tests, which also revealed a bug in getNodeFromSubPath
which I fixed.
A comment in getNodeFromPath() says that typeStep() is a receiver step, but I don't see where that comes from?
I renamed it from "receiver step" to "type step" and I missed that comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to rebase to fix a conflict, but could you take a look at the commits called:
- JS: add partially failing test
- Also apply type step in getNodeFromSubPath
- Fix typo: receiver step -> type step
This adds some features related to generics, to support generating MaD typings from
.d.ts
files and similar.Type variables
To track types through generics, we encode all the paths leading from some type to all uses of its type variables.
For example for this type,
we'd generate the following rows in a table "type variable" table of form
name;path
:Suppose we want to record how to obtain an instance of
ImportantObject
frommylib
, given this declarationwe'd generate this type-definition row:
The rule is that the access path token
TypeVar[name]
can be expanded to any of the paths given by aname;path
row with a matching name. Type variables are really just non-terminals in a context-free grammar, but since they have a 1:1 relationship with type variables, I decided to name them after this use-case.Type-variable rows are thus similar to type-definitions in that the offer a way to reuse access paths, except they can occur anywhere in an access path, whereas type definitions can only occur at the beginning. Also, type-definitions are associated with a set of API nodes, whereas type variables are just intermediate steps and not themselves associated with API nodes (you can ask for all uses of a given type, but not for all uses of a given type variable).
Why
this
types are no longer type variablesthis
types were previously encoded as an extra type variable, but this caused a lot ofTypeVar
tokens to occur and became really hard to understand. To demonstrate, supposeNode<T>
from above had a method returningthis
:This meant
TypeVar[Node.T]
must be expanded to take into account that any number offoo()
calls may occur initially:In practice these
this
types show up in a lot of places, like every time a type extendingEventEmitter
was used.Type steps
Instead of the above, a
this
type gives rise to a "type step", which is a kind of step between API nodes, which we use for propagating type information. It could also be used as a value-preserving flow step, but for now we just propagate type information.The
Foo
type above would give rise to this summary row:meaning that whenever we see a call to
x.foo()
where thex
might have typeFoo
, we add a type step fromx
tox.foo()
.