Skip to content

jtd-codegen generates invalid typescript if the empty string is allowed as an enum value #16

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mlc opened this issue Mar 22, 2021 · 3 comments · Fixed by #17
Closed

Comments

@mlc
Copy link

mlc commented Mar 22, 2021

Given the following input JTD:

{
  "properties": {
    "a": {"type":  "string"},
    "b": {"enum":  ["", "x", "y"]}
  }
}

jtd-codegen with the --typescript-out flag generates the following output:

// Code generated by jtd-codegen for TypeScript v0.2.0

export enum SimpleB {
   = "",
  X = "x",
  Y = "y",
}

export interface Simple {
  a: string;
  b: SimpleB;
}

However, the line = "" with nothing before the equals sign is rejected by the typescript compiler as a syntax error.

jtd-codegen should either reject the input if I'm misunderstanding something and this is not valid JTD, or it should emit typescript which does not cause the typescript compiler to complain.

@ucarion
Copy link
Contributor

ucarion commented Mar 23, 2021

Hi, sorry for the delayed response. To answer your final concern: this is a bug in JTD-Codegen. JTD-Codegen should always output either working code or else give you a clear error as to why working code can't be generated. I am going to keep this ticket open until we fix this. I can't give you a precise estimate on how long this will take to fix, but I do think this can be fixed reasonably easily. What follows is a bit of notes from me looking into the root cause.

I've been looking into this, and it seems this problem permeates in a few different cases. You can also get broken output with this schema:

{
  "definitions": {
      "": { "type": "string" },
      "$foo": { "type": "string" },
      "_foo": { "type": "string" },
      "0foo": { "type": "string" },
      "foo0bar": { "type": "string" },
      "foo bar": { "type": "string" },
      "foo\nbar": { "type": "string" },
      "foo\uFDFDbar": { "type": "string" }
  }
}

Which generates:

// Code generated by jtd-codegen for TypeScript v0.2.0

export type Root = any;

export type  = string;

export type Foo = string;

export type 0foo = string;

export type Foo0 = string;

export type FooBar = string;

export type FooBar0 = string;

export type Foo0bar = string;

export type FooBar1 = string;

And this schema breaks every target except for TypeScript:

{
    "properties": {
        "": { "type": "string" },
        "$foo": { "type": "string" },
        "_foo": { "type": "string" },
        "0foo": { "type": "string" },
        "foo0bar": { "type": "string" },
        "foo bar": { "type": "string" },
        "foo\nbar": { "type": "string" },
        "foo\uFDFDbar": { "type": "string" }
    }
}

For instance, in Ruby:

module XXX
  class Root
    attr_accessor :
    attr_accessor :foo
    attr_accessor :0foo
    attr_accessor :foo0
    attr_accessor :foo_bar
    attr_accessor :foo_bar0
    attr_accessor :foo0bar
    attr_accessor :foo_bar1
  # ...
  end
end

All of these cases should be made to work for all targets, except for the specific case of empty string and Java+Jackson (because of limitations in Jackson's ability to distinguish @JsonProperty from @JsonProperty("")). Even in that case, jtd-codegen should return an error, rather than silently produce non-working code.

Thankfully, the fix can be done at the "core" of JTD-Codegen. The AST phase of jtd-codegen, which assigns desired names for all classes/structs/etc. it emits, should be updated to never assign names that are usually invalid in most languages -- in other words, it should never give out names that don't match [a-zA-Z][a-zA-Z0-9]*, and instead fall back to some default dummy name (e.g. Default or something) if the desired name is irreparably bad, such as if it's "", as in your example.

Thank you for opening this ticket, and sorry you're hitting this limitation. I'm grateful for you taking the time to report this issue, and I'm going to work on fixing it when time permits.

@mlc
Copy link
Author

mlc commented Mar 23, 2021

Thanks for the detailed response, @ucarion! We were able to work around the issue in our application (by modifying the schema), but it's good to hear that the "right" fix will be forthcoming as well!

@ucarion
Copy link
Contributor

ucarion commented Apr 2, 2021

The fix for this should be incorporated into v0.4.1 of jtd-codegen. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants