Allow reserved words to be used for AS class field names

[krisbitney]

This PR allows reserved words to be used for both type property names and function argument names. Reserved words will no longer be prefixed with "m_". Instead, users will be expected to wrap reserved words in quotation marks.

Usage

schema.graphql

type Query {
  method1(
    const: Arg1!
  ): Result!
}

type Arg1 {
  const: String!
}

type Result {
  const: String!
}

index.ts

import {
  Input_method1,
  Result,
} from "./w3";

export function method1(input: Input_method1): Result {
  return {
    "const": "result: " + input.const.const,
  };
}

Closes issue #721

[dOrgJelli]

Could you get tests to pass @krisbitney? We should make sure the AssemblyScript compiler doesn't complain about these keywords-as-type-properties by adding a few more tests for commonly used keywords: const, let, function, module, export, etc.

I'm good with this PR if we can prove that it indeed works with asc.

[krisbitney]

Could you get tests to pass @krisbitney? We should make sure the AssemblyScript compiler doesn't complain about these keywords-as-type-properties by adding a few more tests for commonly used keywords: const, let, function, module, export, etc.

I'm good with this PR if we can prove that it indeed works with asc.

After looking at it, I think it should work but will require a change. It doesn't work with object literal syntax. Instead, we will need to add constructors to generated classes and use that syntax. I think users will still be able to use object literal syntax in cases where reserved words are not used for field names of the return value. What do you think? @dOrgJelli @Niraj-Kamdar

// doesn't compile
return {
 const: "foo"
};

// compiles
return new Result("foo");

[krisbitney]

After looking into it further, I don't think it's possible to instantiate a class from an object literal when you declare a constructor. I thought it would be possible because I did some experimentation and I didn't get a compiler error, but it seems it doesn't work more generally.

I don't think this bug can be fixed. I think we need to accept the m_ prefix on class property names that are reserved words. The alternative is to lose object literal syntax altogether, which I think is a much greater inconvenience than the prefix.

[Niraj-Kamdar]

How about,

return {
"const": "foo",
}

Does this compile with asc?

[krisbitney]

How about,

return {
"const": "foo",
}

Does this compile with asc?

that's a really good question. I'll find out.

[krisbitney]

How about,

return {
"const": "foo",
}

Does this compile with asc?

Yes, this works. This PR will allow reserved words to be used for both property names and function arguments.

[krisbitney]

Don't we still need to handleKeywords for module methods.

Ex:

type Query {
  type(token: String!): TokenType!
}

For above schema following as module file won't compile since type is a keyword.

export function type(token: string): TokenType {
  ...
}

This would work if we're using class for modules.

Sure thing. I'll add this feature for type and method names. This isn't something we had before. We were only allowing reserved words for property and argument names.

[krisbitney]

After looking into this more, I think allowing reserved words to be used for type and method names is going to be a bit more complicated. Doing so will require changing the TypeInfo, since our Array and Map type definitions don't currently distinguish between object and non-object types.

For example, an Int32[] would contain items of type i32 in AssemblyScript. A naive implementation would prepend m_ to the type when generating the AssemblyScript code because "i32" is a reserved word.

I propose we handle type and method names in another issue and merge the fix for property and argument names.

[Niraj-Kamdar]

@krisbitney Can we get this merge and open issue for the fix for reserved words as a method name?

[krisbitney]

After looking into this more, I think allowing reserved words to be used for type and method names is going to be a bit more complicated. Doing so will require changing the TypeInfo, since our Array and Map type definitions don't currently distinguish between object and non-object types.

For example, an Int32[] would contain items of type i32 in AssemblyScript. A naive implementation would prepend m_ to the type when generating the AssemblyScript code because "i32" is a reserved word.

Opened an issue here: https://github.com/polywrap/monorepo/issues/842

[dOrgJelli]

I'm still a bit confused by this... why don't we just solve all naming cases (class, method, argument, property) by adding something like m_ in AssemblyScript? Ideally we should be able to handle these all in a similar way.

It's also worth noting that you can still use the reserved word when invoking the function with the PolywrapClient because it uses strings for method + argument names.

[krisbitney]

I think this PR should address all cases in a unified way:

• methods
• arguments
• type names

I propose the following: schema.graphql

type Module {
  const(float: Int32!): ArrayBuffer!
}

type ArrayBuffer {
  i32: Int32!
}

index.ts

import {
  Args__const,
  _ArrayBuffer
} from "./wrap";

export function _const(args: Args__const): string {
  const obj: _ArrayBuffer = {
    _i32: args._float
  };

  return obj;
}

This sounds good to me.

[Niraj-Kamdar]

I didn't like the appending reserved words with m_ but I really like the _ append approach.

But I have a question: Should users be allowed to create a _ prefixed field in the schema like: _foo?

[krisbitney]

I didn't like the appending reserved words with m_ but I really like the _ append approach.

But I have a question: Should users be allowed to create a _ prefixed field in the schema like: _foo?

What if we prohibit the simultaneous existence of a reserved word and the same word with the _ prefix in the same namespace?

# const would be prefixed to _const in both the type and field names
type const {
  const: Int32!
}

# const would be prefixed to _const in just the type name
type const {
  _const: Int32!
}

# this would throw an exception and codegen would fail
type const {
  const: Int32!
  _const: Int32!
}

#  codegen would succeed because foo is not a reserved word
type bar {
  foo: Int32!
  _foo: Int32!
}

[krisbitney]

replaced by https://github.com/polywrap/monorepo/pull/1171