ternjs / tern

A JavaScript code analyzer for deep, cross-editor language support
https://ternjs.net/
MIT License
4.25k stars 378 forks source link

missing completions or scope issue #184

Open gilligan opened 11 years ago

gilligan commented 11 years ago

Following test scenario: Use the http://ternjs.net/doc/demo.html and add http://backbonejs.org/backbone.js as a new file. Then add another file with the following contents:

Test = Backbone.View.extend({
  foo: function () {
    this.something = 42;
  },
  bar: function () {
  }
});

var t = new Test();
t.foo();
t.something = 1;

Invoking completions on the newly created Test works fine and includes (apart from all the properties from Backbone.View) the functions foo, bar and the something property.

Inside the scope of either foo() or bar() however tern does not find any properties at all when invoking completion on "this."

marijnh commented 11 years ago

Yes, you'll need the new Test call in order for Tern to realize that this is a class. For 'simple' classes defined with a regular constructor and some prototype assignment, Tern contains some code that detects them and connects their 'this' together, but in this case it can't recognize that it's dealing with a class, and the shared this only gets introduced when an actual new expression is seen.

I don't think there's a lot that can be done about this problem.

gilligan commented 11 years ago

I see, that is too bad since in any backbone project most of the code is laid out like the snippet above.

I wonder if one could introduce a special rule that looks for extend() blocks and in that case bind 'this' accordingly. Even if so I guess it wouldn't find its way in the tern code base since it's rather specific. Nonetheless I wish I could somehow get some sort of semi-solution since this reduces the usefulness of tern quite a bit for me :-(

marijnh commented 11 years ago

Try with the attached patch. It uses a different technique for categorizing property functions as methods, which should also work in your case. At least it works in the new test case (see patch) which tries to emulate that case.

gilligan commented 11 years ago

Thanks i will test this properly shortly.

At first glance it seems like if I do a completion on "this" inside of function "foo" (referring to the test case above) it will only find properties of "this" that are being accessed within the scope of the function "foo" itself.

gilligan commented 11 years ago

This does indeed work fine when I try it with the online demo. It does however not work in vim. Doesn't make much sense does it ? I'm on HEAD (6e44bf0) just like the demo.

In vim the only completions I will get on "this" are properties added to this inside the respective scope. Should I file a bug in tern for vim ?

sqs commented 11 years ago

I believe this is because the online demo coincidentally includes underscore.js. If I loadEagerly underscore.js locally, I get the same completions as in the online demo.

I have a rough integration test that shows this working at https://github.com/sourcegraph/tern-integration-tests/blob/master/groups/backbone-sample-app/view.js, and see https://travis-ci.org/sourcegraph/tern-integration-tests/jobs/8831947 for proof (the test failure there is of a different test case). Note that this tern-integration-tests repo depends on https://github.com/marijnh/tern/pull/189.

gilligan commented 11 years ago

I have always had underscore in my loadEagerly section. Still, no dice. I'm a bit at loss what's going on here. @clausreinke do you have any ideas on this ? suggestions ?

gilligan commented 11 years ago

I should have used exactly the same test code right away - sorry. The code above works, but it does not work in most parts of my production code in my current project. I am trying to distill what is actually confusing tern but still haven't quite figured it out yet.

gilligan commented 11 years ago

Sorry for spamming on this issue but it is driving me nuts. I just couldn't figure out why in simple examples everything would work but in real life production code it wouldn't. Now I just realised that there is some screw up with number of lines ..

Completions on 'this' inside the foo function in this code will work fine :

Huh = Backbone.View.extend({

  foo: function () {
  }
});

However if you add like 250 line comment lines you won't get any completions. Neither inside the foo function scope nor on anything else after the extend() block.

Huh = Backbone.View.extend({
/*
*
* 
 ... lots of more comment lines ..
 *
 */
  foo: function () {
    this.rend
  }
});

Tried that in vim and with the online demo .. o__O" Someone tell me my brain is just stuck please.

marijnh commented 11 years ago

Could you put the actual file that fails for you in a gist or paste service somewhere? I just tried to reproduce what you did from your description, but things still work fine.

gilligan commented 11 years ago

I tested it again with the online demo: It is rather weird. Here is the code: https://gist.github.com/gilligan/5960234 and here is the backbone library http://backbonejs.org/backbone.js

I get completions (on 'this.') on the very first attempt but never again after that. When I remove the comment block it works reliably each time.

marijnh commented 11 years ago

Thanks for the test case! I'm not sure how it differed from mine (this is pretty much exactly what I remember typing in yesterday), but it does reproduce the issue.

The problem is that for big buffers, only a part is sent to the server per request (except for the first request, which is why you saw it work the first try). The error-tolerant parser can make something out of most fragments, but in this case it really gets very confused, causing the analyzed code to be pretty much nonsense, and thus preventing meaningful analysis.

There are several other problems with the way file fragments are currently created. I'll look into creating a new algorithm for that.

marijnh commented 11 years ago

Please try again with attached commit. The problem was not so much in the way the file fragment was created, but in the way it was interpreted. I've added special code handling function defined as properties of huge object literals, since that's a common pattern.

gilligan commented 11 years ago

Sorry I am really slow responding to this. I did some tests in the online demo and that appears to work. I just noticed that it's broken in the vim plugin though.

var Test = Backbone.View.extend({
    foo: function() {
    }
});

If I try to complete anything on 'this.' in the foo scope I will get a 'Pattern not found' error in the command line and no completions. This has to be a different problem though since its working with the online demo. Should I create an issue for the vim plugin ?

marijnh commented 11 years ago

I've never seen a "Pattern not found" error -- not sure where that might be coming from.

gilligan commented 11 years ago

@clausreinke ? any ideas what's going ?

clausreinke commented 11 years ago

"Pattern not found" here just means that no matching completions were found, I think. Were those 4 lines the whole test case (no plugins, .tern-project, or such)?

gilligan commented 11 years ago

I created a tiny test project for this : https://github.com/gilligan/tern-vim-test

And of course it works just fine... The problem remains in my real world, big javascript app at work but i'm aparently having troubles extracting the core issue into a small test case. I will try some more and let you know.

gilligan commented 11 years ago

Well it seems like i figured it out now. The problem lies in using backbone-0.9.2 : https://github.com/gilligan/tern-vim-test/tree/backbone-092 (which i happen to use at work).

When I use backbone 0.92 the completion works exactly once and on the next calls 'this.' won't yield any results (Except for any previous occurrences of 'this' properties that already appear in the current scope).

marijnh commented 11 years ago

Eventually it would be nice to have an accurate .json definition file for backbone, so that we don't have to load all that code anymore, and have reliable doc links and such for all the types.