jsontypedef / json-typedef-codegen

A CLI tool that generates code from JSON Typedef schemas
https://jsontypedef.com/docs/tools/jtd-codegen
MIT License
157 stars 31 forks source link

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

Closed mlc closed 3 years ago

mlc commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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