Write tests to ensure variable references and definitions tracking is in order

[mozfreddyb]

The work in #167 to trace variables back through the code starts disallowing the code if one definition or reference looks unsafe, even if a later assignment would cause this to be overwritten by an allowed value (e.g., a literal).

It might be possible to have tests that make sure those are in order so as that might unlock better variable tracing.

There is an obvious issue with uncertainty about data flow that is not visible in static analysis. Not sure what happens with code like

let safeStuff = 'a safe literal';
safeStuff = unsafeEvil;
if (todayIsFullMoon) { safeStuff = `another safe literal`; }
el.innerHTML = safeStuff;

.. what I mean to say is that code-order does not necessarily guarantee execution.

Talking to myself here, but this means that we'll probably have to remain conservative and always abort after just one unsafe re-definition or reference.

[mozfreddyb]

I think it would be better not to take order into consideration at all, given deynamic behavior and close this as WONTFIX. Thoughts, @rpl?

[rpl]

:+1: yeah, I would also feel way less tense of "potentially marking as safe something that isn't", figuring out the order at rutime may be quite tricky and error prone.

e.g. even if we would require that the variable initialization and all assignments before the assignment to .innerHTML to be safe, I'm wondering if something like this would still trick the rule:

let safeStuff = 'a safe literal';
someFunction();
el.innerHTL = safeStuff;

function someFunction() {
   safeStuff = unsafeEvil;
}

In this case the unsafe assignment is technically late in the code snippet if we just consider line and column of the unsafe expression, but due to the hoisting the function is actually defined and can be executed before the el.innerHTL = safeStuff; expression will be executed.

As a side note: It may be a good idea to describe the behavior to expect when the rule is tracking back the variable initialization and assignments as part of the docs of the plugin (e.g. I think that docs/rules/fixing-violations.md may be a reasonable place).

it may not 100% avoid new issues to be still filed about this behavior but we would have a doc page to point the reporter to when we will wontfix the issue (and in some cases a developer may even be looking to the docs and not file the issue at all).