Assigning namespaced classes to a variable results in an assertion error

[saulecabrera]

Taking into account the following code:

// bar.ts

export namespace bar {
  export class Bar{}
}

import { bar } from './bar';

export const aliasBar = bar.Bar;

This results in an assertion rather than a diagnostic, I'm aware that the expression above is not legal, and as such, I think this should emit the same diagnostic as if the imported class didn't belong to a namespace:

ERROR AS234: Expression refers to a static element that does not compile to a value at runtime.

Compiler version: 0.18.21

[saulecabrera]

After some investigation, the error is happening at this line https://github.com/AssemblyScript/assemblyscript/blob/master/src/compiler.ts#L9052 it seems that after resolving the expression we are not accounting for the target kind to be a CLASS_PROTOTYPE; I wonder if we should just report the diagnostic instead of the assert or fall through. I'm not sure of the implications of the fall thorough though, mostly because:

1. We'll probably have to resolve the class (which is... counterintuitive?)
2. From what I can see the diagnostic that I'm suggesting only happens right now when compiling an identifier, which is totally different than a property access expression.

Applying the following patch fixes it, but not sure if I'm missing something in terms of the implications of the change:

-    assert(false);
-    return module.unreachable();
+    this.error(
+      DiagnosticCode.Expression_refers_to_a_static_element_that_does_not_compile_to_a_value_at_runtime,
+      expression.range
+    );
+    return this.module.unreachable();

And the diagnostic looks like:

ERROR AS234: Expression refers to a static element that does not compile to a value at runtime.

 export const aliasBar = bar.Bar;
                         ~~~~~~~
 in assembly/index.ts(9,25)

[reconbot]

I don't disagree with the fact that "Assign a class to a variable " is not permitted per docs, but I would enjoy the ergonomics of being able to use an aliased class name when it's in a namespace.

import { bar } from './bar';

const Bar = bar.Bar;

export function foo() {
  new Bar()
}

[dcodeIO]

@saulecabrera Your fix looks good to me :)

[saulecabrera]

I'll open a PR then.

[dcodeIO]

@reconbot I agree that being able to do this is desirable, but my intuition is that if we want to support it, we'll very like want to do it properly by representing classes with a value at runtime. Like, Bar could be exported, be initialized with any expression, be returned, or being passed to a function etc., and that's already too many edge cases for my taste that we cannot solve with a static alias.

Speaking of a static alias, the following should work today, btw:

import { bar } from './bar';

type Bar = bar.Bar;

export function foo() {
  instantiate<Bar>()
}

[dcodeIO]

Concretely, the obvious candidate for a value that can be passed around at runtime is the unique id of the class, of course, so

const Bar = bar.Bar;

could compile down to something like

const Bar = changetype<typeof bar.Bar>(<usize>idof<bar.Bar>());

with the missing building blocks being support for typeof SomeClass describing the shape of the class plus virtual lookup tables (like we already have for instance methods) for static methods in case SomeClass is overloaded, I guess.