Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 514801

Summary: [Tern] Type information for dependencies are being guessed
Product: [ECD] Orion Reporter: Curtis Windatt <curtis.windatt.public>
Component: JS ToolsAssignee: Michael Rennie <Michael_Rennie>
Status: RESOLVED FIXED QA Contact:
Severity: major    
Priority: P2 CC: Michael_Rennie, steve_northover
Version: 13.0   
Target Milestone: 15.0   
Hardware: PC   
OS: Windows 7   
See Also: https://github.com/eclipse/orion.client/pull/86
Whiteboard:
Bug Depends on:    
Bug Blocks: 513025, 514274    

Description Curtis Windatt CLA 2017-04-05 16:46:49 EDT
There is a major problem with how properties from module dependencies are being calculated (node and requirejs).  Tern does such a great job guessing potential properties that it is difficult to notice.

1)
define('name', ['import'], function(importname) {
	importname. // Returns no proposals
});

2)
define('name', ['import'], function(importname) {
	importname.a;
	importname. // Returns all import property proposals
});

In the above example (1), Tern looks at the props for importname, see there are none and returns nothing.  In (2) Tern still sees nothing for importname, but does see the forward 'a'.  It adds 'a' as a proposal and then looks for existing types that have a property 'a'.  Since import.js has such a property, all properties from it are added as proposals.

This explains some weird behaviour I've seen in the past
a) If you have a requireJS project and have a file with a, b, c as module properties, then you switch to a Node project that can't resolve it's dependencies but call a property 'a', 'b' or 'c', Tern will offer completions from the requireJS project!
b) Once you have used a property from the root messages file (Bug 513025), you can start getting content assist proposals for it, even though there is no dependency chain.
Comment 1 Curtis Windatt CLA 2017-04-06 10:02:02 EDT
I believe the cause of this problem is the performance enhancements we implemented to reduce the number of file requests we performed.  Different modules may depend on the same module using different paths/short names.  Since they resolve to the same file, we only add the one file to Tern server with the long name.

If I add the shortname file to the server with the same contents, it appears to fix the problem (though I need to confirm that it is not guessing anything).
Comment 2 Curtis Windatt CLA 2017-04-06 16:19:23 EDT
(In reply to Curtis Windatt from comment #1)
> If I add the shortname file to the server with the same contents, it appears
> to fix the problem (though I need to confirm that it is not guessing
> anything).

This fixes the trivial requirejs case, but does not fix it for a similar case inside of Orion code, nor does it work for a node require example.
Comment 3 Michael Rennie CLA 2017-04-07 14:24:39 EDT
(In reply to Curtis Windatt from comment #2)
> (In reply to Curtis Windatt from comment #1)
> > If I add the shortname file to the server with the same contents, it appears
> > to fix the problem (though I need to confirm that it is not guessing
> > anything).
> 
> This fixes the trivial requirejs case, but does not fix it for a similar
> case inside of Orion code, nor does it work for a node require example.

I think we also should consider a "bigger fix" here (and it would also fix bug 514803).

Here's what I am thinking:

1. when a file is changed in the client, we currently have a "changed" cache in javaScript plugin that prevents us always sending the file contents to Tern (this avoids re-parsing / re-inferring for Tern commands that happen when the file has not been modified). We should make use of this to start sending an addfile request rather than sending the contents in the request - my thinking here is that we can then have addFile (in Tern) signal listening plugins that a file has changed so we can update any caches without any special tricks (bug 514803).

2. we would also do the same as 1) for the deleteFile support in Tern - so we can properly remove unused files from the respective caches

3. so implementing 1) and 2) would get rid of our caching problems, but now we are left with the multiple logical name to long name problem. I think for this we could probably just make our resolver "better" to make sure that if a request is sent with the long name for a file, we can properly "know" if there are mapped shortnames that also point to that file - or we always resolve getFiles' in Tern to use the long name and we sort out the shortnames in the resolver.

For example a cache of: longNames -> Map(shortNames) 
and then the reverse of: shortName -> longName so we don't have to crawl through the map of longNames to resolve a shortName.
Comment 4 Eclipse Genie CLA 2017-06-22 16:32:16 EDT
GitHub Pull Request 86 created by [mrennie]
https://github.com/eclipse/orion.client/pull/86
Comment 5 Michael Rennie CLA 2017-06-22 16:48:39 EDT
The fix was to revert the changes that made the requirejs plugin use the modules plugin. 

The underlying problem was two-fold: (1) types were being propagated into the wrong scope - which allowed unrelated types to be found for assist, etc, and (2) during resolution the same type would be re-inferred and propagated over and over again - this was the cause of the problem where you would see multiple versions of the same proposal for a type.
Comment 6 Michael Rennie CLA 2017-06-23 12:22:44 EDT
Merged.
Comment 7 Michael Rennie CLA 2017-06-23 12:44:35 EDT
*** Bug 513025 has been marked as a duplicate of this bug. ***