Imporve/fix some scopes reported by the JavaScript parser #1568
Conversation
Handle scope more consistently, avoiding dealing with it in many places in a fragile and sometimes incorrect manner. This fixes most scopes in current test cases. Handling of qualified names is still questionable, but mostly follows the current handling to prevent breakage. Better handling of these would require tracking which tag the qualified name actually refers to to be able to choose the correct scope, but that's outside the scope of this patch.
|
For newly added test cases, could you set --sort=no option? (In the future, I will put --sort=no to misc/units script.) |
|
@masatake OK, but for JS I don't know if it makes much sense as the parser emits some tags out of order. |
|
@masatake actually, what's the point in not sorting? I agree it's easier to check when the parser is emitting things in the order they appear in the file, but otherwise I kind of feel like the tests would relying too much on an implementation detail that never mattered. Namely, I don't have a really sensible way of "not sorting" the new test I added and that doesn't pass yet: which order should I put? None is really relevant. |
|
@b4n, how about adding |
|
I will review them in the week end. Thank you. BTW, is this change something to do with |
Handle scope more consistently, avoiding dealing with it in many places in a fragile and sometimes incorrect manner. This fixes most scopes in current test cases. Handling of qualified names is still questionable, but mostly follows the current handling to prevent breakage. Better handling of these would require tracking which tag the qualified name actually refers to to be able to choose the correct scope, but that's outside the scope of this patch.
Fix a corner case of do-while loops where the semicolon is actually not needed, not even implicitly.
Unnamed blocks do not create a new scope, leading to the declarations inside being effectively at the root scope.
|
@masatake thanks! I just squashed the fixups and added |
| dummy1 input.js /^ this.dummy1 = function() {}$/;" m line:18 class:doesnt_extend.Cls | ||
| dummy2 input.js /^ Cls.prototype.dummy2 = function() {$/;" m line:20 class:doesnt_extend.Cls | ||
| extend input.js /^function extend() {$/;" f line:7 | ||
| extended input.js /^ Cls.prototype.extended = function() {$/;" m line:9 class:Cls |
There was a problem hiding this comment.
Very interesting.
"Cls" in the scope field is obviously useful information about "extended".
However, "extend" in which "extended" is assigned is also useful information.
None defines the meaning of "scope" field. I have just recognized that there are two type of scopes:
source code aspect scope and runtime scope. lexical scope and dynamic scope are more suitable terms?
For reading and understanding source code, source code aspect scope information is also useful. The
information can be regenerated with line: and end: fiedls but recording it in ctags is not bad idea.
Some parsers may records runtime scope and the other parsers may records source code aspect scope as the value for scope field. It may be too late to redefine the meaning of scope field. However, I would like to explore this area in the future.
@b4n, you have developed an IDE. I would like to hear your comments.
One of my idea is introducing new scope named sourceScope that is filled only when if source code aspect scope and runtime scope about a tag is not matched.
| b input.js /^var A = {}, b = 2;$/;" v | ||
| c input.js /^var c = 3, d = 4;$/;" v | ||
| d input.js /^var c = 3, d = 4;$/;" v | ||
| mem1 input.js /^A.mem1 = function() {return 42;}, A.mem2 = function() {return 43;}$/;" f function:A |
There was a problem hiding this comment.
Is the kind of A, the scope of mem1, function?
I think resolving the kind for A is not easy. Especially if A is defined in another file.
One of technique I used is introducing "unknown" kind. See 563bf97 as an example.
There was a problem hiding this comment.
The problem is that in JavaScript there's not really anything but objects, a class, a function, no real differences. Actually you can see a fun thing in Firefox's JS console:
>> class foo {}
undefined
>> print(foo)
"function foo() {
}"
Anyway, my point is that the effective high-level kind is very dynamic. In this example, A should be a class merely because it gets methods added, but it really becomes a class. So ideally A should be reported as a class, but when it's first seen and the tag emitted it looks like a naked object, hence the "variable" kind.
| v5 input.js /^, v5 = 3$/;" v | ||
| f1_c1_m1 input.js /^ f1_c1_m1:$/;" m class:f1.f1_c1 | ||
| f1_c1_m2 input.js /^ f1_c1_m2$/;" m class:f1.f1_c1 | ||
| f1_c1 input.js /^ var f1_c1 = {$/;" c class:f1 |
There was a problem hiding this comment.
It seems that f1 is a function.
There was a problem hiding this comment.
Indeed. As mentioned somewhere it's a known limitation of the JS parser, with a FIXME in the code.
|
Have you ever consider to use corkAPI for representing scopes in jscritp.c? For filling a scope field, a parser have to track both scope name and scope kind. In my experince, corkAPI works well in this case. I can take over the rest of job for introducing corkAPI if you are o.k. In such case, I will merge this pull request first. |
|
@b4n, after rebasing the lastet master branch where CTAGS_INLINE macro is introduced, could you consider chery-picking following changes? masatake@690c456 All of them are in found at https://github.com/masatake/ctags/commits/jscript-make-signature-field-sophisticated |
|
@masatake I though about using that new cork API, but it would require quite some changes, especially as many tags are held back (to find the right kind). Maybe though it would be quite handy if we can change the kind of a tag after having prepared it. |
Hum… maybe, I'm not sure. OK, it doesn't report something wrong and a client could special-case it, but OTOH the parser is generally right in the assumption, only a few cases are incorrect, so I'm not sure. |
|
I found we have js-scope-resolution.b test case. LGTM. Thank you. |
|
@b4h, should I merge this? |
|
@masatake You can, yes. |
|
Could you look at#1519? It will change the last argument of initTagEntry. It required a pointer to kindDefintion struct. With the change of #1579, it requires an integer instead. This change may have small impact on the code you are working. |
No description provided.