Chakra should throw SyntaxError when a variable is already declared inside the "with" scope

[baac]

Hi,

there is an inconsistency when a variable is declared twice as "var" and "let" inside the "with" scope. Chakra should not accept the redeclaration and indicate an error.

OS: Ubuntu 17.10 Chakra: 1.10.0.0-beta

Step to reproduce:

with({}) {
   var a;
   let a;
}

Actual result: Pass without failures

Expected result: SyntaxError: Cannot declare a variable twice

V8 and SpiderMonkey raises an exception as expected.

[liminzhu]

Thanks for filing the issue! This looks like a variation of https://github.com/Microsoft/ChakraCore/issues/5070, so we will track it there.

[dilijev]

I'm not sure this is a duplicate, since that has to do with eval and this has to do with with. @VSadov to confirm.

[VSadov]

I think it is a separate issue. It may look related, but since the other one is in eval (and thus conflict check is deferred), the bugs will most certainly need different fixes.

[liminzhu]

I should've put more context here. I thought the issue was let/var redeclaration in a block scope and has not much to do with and eval?

You won't get an error even with

/*
#### V8 --harmony
SyntaxError: Identifier 'a' has already been declared

#### SpiderMonkey
SyntaxError: redeclaration of let a:

#### Chakra

*/
if (true) {
    let a;
    var a;
}

eval just avoids hoisting and creates a sub-scope for var, but you get error all the same. This behavior is normal,

/* 
#### Chakra
ReferenceError: Let/Const redeclaration
*/
let a;
eval('var a');    

[dilijev]

The issue with let/var redeclaration with eval is here: https://github.com/Microsoft/ChakraCore/issues/3405

[dilijev]

using with (or any other block scope) with order of let and var unimportant, engines are split 50/50 on whether to give the SyntaxError. I suspect we are supposed to.

#### jsvu-ch, jsvu-jsc
DONE

#### jsvu-d8
SyntaxError: Identifier 'a' has already been declared

#### jsvu-sm
SyntaxError: redeclaration of var a:

[dilijev]

If there is no enclosing scope we correctly throw.

## Source
// with({}) {
   var a;
   let a;
// }
print('DONE')

#### jsvu-ch
SyntaxError: Let/Const redeclaration

#### jsvu-d8
SyntaxError: Identifier 'a' has already been declared

#### jsvu-sm
SyntaxError: redeclaration of var a:

#### jsvu-jsc
SyntaxError: Cannot declare a let variable twice: 'a'.

[dilijev]

In that case, this issue is exactly duplicate of https://github.com/Microsoft/ChakraCore/issues/849

[pleath]

Yes, we should be raising an exception, but we fail to do that when the let is declared in a scope other than the one the var is being hoisted into (if that makes sense). We should not hoist the var past the let declaration without throwing redeclaration. @aneeshdk and I have discussed possible fixes.

The eval case is related, though as @VSadov says the fix will be rather different, so it probably makes sense to track it separately. There are 3 eval buckets, 2 of which we handle correctly. (Some of this was pointed out by @dilijev while I was writing.)

We correctly throw here, exactly as if there were no eval:

let x; eval('var x;');

We correctly do not throw here, as strict-mode eval does not leak the var:

'use strict'; let x; eval('var x;');

But we incorrectly do not throw here, again because we allow the var to be hoisted past the let:

{ let x; eval('var x;'); }

[kfarnung]

From #5453 it looks like the same issue exists for functions, make sure to test function redeclaration when fixing this issue:

function test() {
  "use strict";
  function f() {
     return 1;
  }
  {
     function f() {
        return 2;
     }
     function f() {
        return 3;
     }
  }
  return f() === 1;
}
if (!test())
  throw new Error("Test failed");

[igor-simoes]

Hi everyone, I think this is a similar case to test when fixing this issue.

eval('{ var x; {let x;} }');
eval('{ let x; {var x;} }');

#### Chakra, JavaScriptCore
#### V8
SyntaxError: Identifier 'x' has already been declared
#### SpiderMonkey
SyntaxError: redeclaration of let x:

cinfuzz