fix: compiler generates invalid syntax with top-level key `constructor`

[WillsterJohnsonAtZenesis]

When accidentally typing constructor instead of construct as the key name for a qooxdoo class constructor function, the compiler would generate invalid code:

  constructor() {
    // clearly, this won't parse in any engine
    my.alpha.Application.superclass.prototype.function Object() { [native code] }.call(this);
  }

This is following the correct pattern of {classname}.superclass.prototype.{methodname}.call(this), however the methodname portion is coming from FUNCTION_NAMES[keyName]. Any POJO when indexed at constructor (the keyname in this case) will just return the native Object constructor.

This pr modifies the logic to check if the given keyname is a real field of the FUNCTION_NAMES object, not merely if there is any result from taking the index.

  // now we can reach the runtime qx.Class warning against `constructor` as a top level key
  // without being hindered by engine syntax errors
  constructor() {
    my.alpha.Application.superclass.prototype.constructor.call(this);
  }

[derrell]

I don't quite understand what this is doing. Is it simply making a better error message when the programmer accidentally types constructor instead of construct? Or is it allowing constructor as a synonym for construct? I would oppose the latter and greatly approve of the former.

[oetiker]

@WillsterJohnsonAtZenesis please elaborate!

[WillsterJohnsonAtZenesis]

I don't quite understand what this is doing. Is it simply making a better error message when the programmer accidentally types constructor instead of construct? Or is it allowing constructor as a synonym for construct? I would oppose the latter and greatly approve of the former.

What this is doing is changing what the compiler does when it encounters a top-level constructor entry in the class definition.

Currently, the compiler emits completely invalid Javascript. Because it was naively reading the key constructor of the FUNCTION_NAMES object when indexing FUNCTION_NAMES[keyName], what it was receiving back was not undefined (as you would expect for any other key, eg if someone added a top-level foo key) but instead it accessed the constructor function of the native JS Object. This is what is causing the invalid code, as the compiler simply writes the .toString() value of the object constructor function, hence the function Object() { [native code] } in the first code block.

What this PR changes is just to avoid the edge case. Instead of indexing the object FUNCTION_NAMES, it first ensures that it has a real key of that name. If it does, then it will do the intended custom behavior, otherwise it defaults to regular behavior. This allows the code to run without syntax errors, at which point in development qx.Class will notice the illegal key constructor and give the developer an accurate error message about what went wrong.

We're definitely not permitting constructor as an alternative, that would be a pretty bad idea imo.