Bug 413585 - initial support for index files for content assist
Summary: initial support for index files for content assist
Status: RESOLVED FIXED
Alias: None
Product: Orion (Archived)
Classification: ECD
Component: JS Tools (show other bugs)
Version: unspecified   Edit
Hardware: PC Mac OS X
: P3 normal (vote)
Target Milestone: 4.0 M1   Edit
Assignee: Mark Macdonald CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy
Depends on:
Blocks:
 
Reported: 2013-07-23 19:35 EDT by Manu Sridharan CLA
Modified: 2013-09-20 15:27 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Manu Sridharan CLA 2013-07-23 19:35:13 EDT
I've added support for content assist index files in a github branch:

https://github.com/msridhar/orion.client/tree/index-files

In this branch, I've made the following major changes:

* Removed types.js.  Previously, it contained hand-written models for key libraries (ecma5, the DOM, and node.js) and also utility methods for manipulating the types inferred by content assist.  Now, the utility methods are in typeUtils.js:

https://github.com/msridhar/orion.client/blob/index-files/bundles/org.eclipse.orion.client.ui/web/plugins/esprima/typeUtils.js

The hand-written models have been serialized as index files.

* Added support for parsing index files and using them during content assist.  The key file is typesFromIndexFile.js:

https://github.com/msridhar/orion.client/blob/index-files/bundles/org.eclipse.orion.client.ui/web/plugins/esprima/typesFromIndexFile.js

The main content assist module, esprimaJsContentAssist.js, was modified to use typesFromIndexFile.js instead of types.js, along with other minor changes.

* Generated index files corresponding to the previous library support that was in types.js.  They are in the indexFiles folder:

https://github.com/msridhar/orion.client/tree/index-files/bundles/org.eclipse.orion.client.ui/web/plugins/esprima/indexFiles

The format is borrowed from that used by Tern, but we don't use any of Tern's index files.

With minor modifications, all extant regression tests pass.

Please let me know if there is feedback or if other changes are desired before merging.
Comment 1 Manu Sridharan CLA 2013-07-24 16:57:03 EDT
I have rebased and cleaned up the branch, so now there are only two commits to merge:

https://github.com/msridhar/orion.client/tree/index-files

I also updated the code so that index files are loaded asynchronously, by changing the content assist code to return a promise.
Comment 2 Mark Macdonald CLA 2013-07-29 17:21:17 EDT
Andrew: you're more familiar with the Esprima content assist code-- any thoughts on this? 

I am gradually scraping my way through the patch, but have very little idea what I'm looking at.
Comment 3 Andrew Eisenberg CLA 2013-07-29 23:31:43 EDT
Hi, I can chime in here.  After a look at the changes, this seems like a good way to go since it will let you piggy-back on any other tern-like index files.  Is the longer-term goal to use this syntax to index files in the current project and use the indexes for cross-file inferencing?

I do see that closure-compiler syntax is not being used in the indexes.  It also looks like much of the serialization and de-serialization code from types.js has been removed. This moves it away from the direction of scripted and means that merging this back into scripted would be more challenging. However, that doesn't really concern Orion.

From the perspective of Orion, I don't see anything wrong with this patch.

+1
Comment 4 Manu Sridharan CLA 2013-07-30 01:28:56 EDT
Hi Andrew, 

Re: cross-file inferencing, yes, I think we'd like these index files to be the basis of such functionality in Orion.  I'm still in discussions with Simon on how that might work, but hopefully we'll get to it sooner now that this basic infrastructure is implemented.

Re: the Closure compiler syntax, I'd prefer to use that, as it would make the index-file parsing code simpler, and that way we'd have one syntax everywhere.  I started with Tern syntax since I could test things out with existing index files that way.  We may still switch to Closure, and maybe have a separate conversion script for Tern files.

A lot of the serialization code is still present in esprimaJsContentAssist.js (I think pretty much all of types.js is still present, either in typeUtils.js or as index files).  There is another file serializer.js now that I used to convert the specifications from types.js into index files; that code might be useful for cross-file inferencing in the future.
Comment 5 Andrew Eisenberg CLA 2013-07-30 14:59:03 EDT
Another, more fundamental question: Since you are going with tern syntax, why not just use tern itself and remove what we have already?
Comment 6 Manu Sridharan CLA 2013-07-30 17:55:09 EDT
I think using Tern is something we should investigate more thoroughly.  I haven't looked at the Tern code in detail, but from the high-level descriptions, it seems that its key advantage over our current approach is tracking of inter-procedural data flow in its type inference.  I'm not 100% convinced that such a flow analysis is worth the cost, given the added complexity required in the implementation.  But, I should collect some real-world numbers to quantify Tern's advantage, and dig into the code more to understand exactly how it works.

In the meantime, I think adding index file support to our current engine makes sense, as it will ease the process of adding better content assist support for other libraries.  Plus, any new index files could probably be reused with minimal effort if we do decide to switch engines.
Comment 7 Mark Macdonald CLA 2013-07-31 16:27:51 EDT
Merged to master. Thanks guys.
http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=7303534


As for Tern, maybe Simon can chip in here, I think he has some opinions on it as well.