sublimehq / Packages

Syntax highlighting files shipped with Sublime Text and Sublime Merge
https://sublimetext.com
Other
2.95k stars 588 forks source link

JavaScript: object properties #965

Open pdknsk opened 7 years ago

pdknsk commented 7 years ago

This is the reference.

window.onload = function(){};

With possible bugs.

// 1
var name = function(){}
window.onload = name;

// 2
window.onload = window.onclick = function(){};

// 3
window.onload = window.onclick;

// 4
window.onclick = (function() {
    return function() {
        console.log(window);
    }; 
})();

I think 2 definitely is, and 4 likely.

1

Thom1729 commented 7 years ago

What is the bug? Do you want onload to be consistently marked as entity.name.function in these situations?

The problem is that the syntax can't really tell when onload will receive a function value. In fact, this is impossible; even if it were generally possible to tell whether a JavaScript value is a function, to do so is far beyond what Sublime's syntax highlighter could ever do. What the syntax does instead is mark a few simple cases that can be easily identified. For example, there is a special case for a statement like obj.name = function....

In example 1, there is no way for Sublime to know that name is a function. It would have to implement a complete type analyzer that would run every time the buffer changed. Even if this could be done (which it can't with the existing highlighting engine) it would be very bad for performance. The same applies to example 3.

In example 2, in order for Sublime to mark the first onload as a function name, it would have to look ahead over the intervening window.onclick =. This could conceivably be special-cased, but it would be complicated to implement (possibly a source of bugs or other quirks) and it would probably come up infrequently in practice. In my opinion, it isn't worth the hassle and added complexity, but reasonable opinions may differ.

In example 4, the body of the immediately-executed function expression could evaluate to anything at all. As in examples 1 and 3, there is no way for Sublime to tell that the result is a function. The opening (function... provides no clue; this construct is used all the time to produce non-function results.

Note that GitHub's code highlighting produces the same results as Sublime in all of these cases. There are certainly situations in which we can improve upon GitHub's results, but unfortunately I don't think that this is one of them.

keith-hall commented 7 years ago

Maybe this could be done really simply by special casing any window properties that begin with on, as they must (/should) be functions?

pdknsk commented 7 years ago

I think the above proposal makes colliding with global variables beginning with on likely, even though one would usually not reference them with window.variable.

I only filed the bug for 2 really, and knew that 1 and 3 are practically impossible to implement. On 4, that it returns a function is a coincidence here. I only meant for the anonymous function to trigger the highlighting, regardless of what it returns.

I think I actually prefer the special case to be removed, as it makes the highlighting really inconsistent.

FichteFoll commented 7 years ago

I'm mostly with Thom here. 1 and 3 are basically impossible, unless special-cased like Keith proposed, which would be fine solution in isolation. It raises the question how many of these variables exist that only ever expect functions to have assigned to. 4 is put of the question. 2, however, seems like something that could be done without too much effort.