fix(block-scoping): deconflicts blocks in switch-statement and parent…

[vitorveiga]

… function scope

In the following snippet, the n variable is declared twice, one const n = 12 in function body and, another const n = 12 inside switch statement

function foo() {
    const n = 1;
    for (let i = 0; i < n; i++) {
        const o = n;
        switch (n) {
            case 1:
                const n = 12;
                console.log(n);
                break;
        }
    }
}
// OUTPUT: 12

Buble transpiles to

function foo() {
  var n = 1;
  for (var i = 0; i < n; i++) {
    var o = n$1;
    switch (n$1) {
      case 1:
        var n$1 = 12;
        console.log(n$1);
        break;
    }
  }
}
// NO OUTPUT TO CONSOLE

n variable is wrongly replace in const o = n and switch(n).

[vitorveiga]

Seems to fail in 262 tests...
language/statements/try/scope-catch-block-lex-open.js

var probeParam, probeBlock;
let x = 'outside';

try {
  throw [];
} catch ([_ = probeParam = function() { return x; }]) {
  probeBlock = function() { return x; };
  let x = 'inside';
}

assert.sameValue(probeParam(), 'outside');
assert.sameValue(probeBlock(), 'inside');

Checking that...

[vitorveiga]

Node:6 and node:4 don't use npm ci to install their dependencies, thus will not respect package-lock versions. Seems that magic-string recent releases a newer version, which alters source-map generation process.

Fixing magic-string version at package.json level (using same version as in package-lock), resolves that issue in the node:6 and node:4.

However, another testing fail appear in built-ins/RegExp/property-escapes/character-class.js for node:6 and node:4

[mourner]

Let's keep the PR open — the change is definitely very welcome. I haven't been able to figure the test failure yet though, and it seems to happen in other PRs too like those from @luiscubal. @adrianheine maybe you would have some pointers on how to fix the failures?

[nathancahill]

I know it's been talked about before, but Node 8 is reaching the end of Maintenance this month. Node 4 and Node 6 have been many years past EOL. I've worked on a few PRs to keep Buble going, and it's a bit depressing to have changes that work perfectly on Node 8 and above but break on Node 4. Possibly because other libraries have dropped support for Node 4?

I'm a huge proponent of keeping as wide a range of compatibility, but it's something to consider, especially with the maintenance status of the project.

[mourner]

@nathancahill yes, I believe now's the time to make the plunge. Before it was more of a hypothetical consideration, but now it severely affects maintainability of the project.

[vitorveiga]

Hello, I recently closed this PR because i found some one bug that not occurs in buble master

In the following code

 class e {
   run() {
      if(this instanceof e) {
        for(let e = 0; e < 1; e++) console.log('here');
      }
   }
 }
 
 new e().run();

it wrongly transpiles to

 class e {
   run() {
      if(this instanceof e) {
        for(var e = 0; e < 1; e++) console.log('here');
      }
   }
 }
 
 new e().run();

variable e inside for statement should have been renamed to e$1.

Adding this test case to the PR.