fix: add diagnose for non-declarative statements in namespace

[JesseCodeBones]

Fixes #2764.

• I've read the contributing guidelines
• I've added my name and email to the NOTICE file

[HerrCai0907]

LGTM

[HerrCai0907]

AS and TS has difference behavior for this case.
TS will throw diagnose Block-scoped variable 'a' used before its declaration but AS will not.

But I think it is different topic with this PR. It is a generic issue both in namespace and function.
Should we introduce variable scope analysis to handle these cases.
@dcodeIO @CountBleck WDYT.

let a: number = 0;

namespace NS {
  a = 100;
  export function foo(): void {
    trace("a", 1, a);
  }
  let a = 200;
}

export function _start(): void {
  trace("aaaaa", 1, a);
  NS.foo();
}

[JesseCodeBones]

AS and TS has difference behavior for this case. TS will throw diagnose Block-scoped variable 'a' used before its declaration but AS will not.

@dcodeIO @CountBleck WDYT.

let a: number = 0;

namespace NS {
  a = 100;
  export function foo(): void {
    trace("a", 1, a);
  }
  let a = 200;
}

export function _start(): void {
  trace("aaaaa", 1, a);
  NS.foo();
}

AS and TS has difference behavior for this case. TS will throw diagnose Block-scoped variable 'a' used before its declaration but AS will not.

But I think it is different topic with this PR. It is a generic issue both in namespace and function. Should we introduce variable scope analysis to handle these cases. @dcodeIO @CountBleck WDYT.

let a: number = 0;

namespace NS {
  a = 100;
  export function foo(): void {
    trace("a", 1, a);
  }
  let a = 200;
}

export function _start(): void {
  trace("aaaaa", 1, a);
  NS.foo();
}

This issue could happened in any scope: top level, function body, namespace and etc.
More information:

1. ts pass and invalid cases
   pass case:

a = 42;
var a = 0;

invalid case:

a = 42;
let a = 0;

invalid case:

a = 42;
const b = ()=>{
  var a = 0;
}

2. asc test case and its compile output

let a:i32 = 1;
function foo():void{
  a = 42;
  let a: i32 = 0;
}
foo();

(module
 (type $none_=>_none (func))
 (global $namespace/a (mut i32) (i32.const 1))
 (global $~lib/memory/__data_end i32 (i32.const 8))
 (global $~lib/memory/__stack_pointer (mut i32) (i32.const 32776))
 (global $~lib/memory/__heap_base i32 (i32.const 32776))
 (memory $0 0)
 (table $0 1 1 funcref)
 (elem $0 (i32.const 1))
 (export "memory" (memory $0))
 (start $~start)
 (func $namespace/foo
  (local $a i32)
  i32.const 42
  global.set $namespace/a
  i32.const 0
  local.set $a
 )
 (func $start:namespace
  call $namespace/foo
 )
 (func $~start
  call $start:namespace
 )
)

[CountBleck]

 (func $namespace/foo
  (local $a i32)
  i32.const 42
  global.set $namespace/a
  i32.const 0
  local.set $a
 )

Okay, this one thing is unacceptable. There should be a temporal deadzone above that let a: i32 = 0;, meaning a compile-time error (as @HerrCai0907 mentioned) should be emitted. AS should not compile that code, and never compile it to that. The meaning of a shouldn't change halfway into the function.

[CountBleck]

But I think it is different topic with this PR. It is a generic issue both in namespace and function.

Sure, but this PR shouldn't be merged (yet) if it allows weird stuff as shown above.

[HerrCai0907]

Sure, but this PR shouldn't be merged (yet) if it allows weird stuff as shown above.

I think we can diagnose "not implement" as temporary solution.

Totally ignoring statement in namespace is also weird stuff.

[CountBleck]

Totally ignoring statement in namespace is also weird stuff.

Yeah, that // TODO: else? is a good place to raise an error.

[JesseCodeBones]

In typescript, it has check file process before enter the compile phase.
the logic can be found in function isBlockScopedNameDeclaredBeforeUse of https://github.com/microsoft/TypeScript/blob/main/src/compiler/checker.ts.
In assembly script, we do not have the check phase, so it is hard to predict that there is a declaration expression after this assignment expression.
could we have the same check functionality for Diagnose purpose?

[HerrCai0907]

Since AS cannot fully support namesapce as TS, maybe diagnose "not implement" as temporary solution.

[JesseCodeBones]

Since AS cannot fully support namesapce as TS, maybe diagnose "not implement" as temporary solution.

Hi Congcong,
finished the diagnose message for parser, please help to review again.
Regards