palantir / conjure

Strongly typed HTTP/JSON APIs for browsers and microservices
https://palantir.github.io/conjure/
Apache License 2.0
419 stars 66 forks source link

map<any, T> shouldn't be allowed #135

Open iamdanfox opened 5 years ago

iamdanfox commented 5 years ago

What happened?

I was able to write a conjure definition which seems to be non-representable in JSON. Here's a spec-test that passes but I think it shouldn't pass:

  aliasOfMapKeyAnyNotAllowed:
    expected-error: 'TODO'
    conjure:
      types:
        definitions:
          default-package: test.api
          objects:
            MapAnyAliasExample:
              alias: map<any, boolean>
  objectFieldWithMapKeyAnyNotAllowed:
    expected-error: 'TODO'
    conjure:
      types:
        definitions:
          default-package: test.api
          objects:
            MapAnyObjectExample:
              fields:
                bad: map<any, boolean>
  objectWithAliasOfMapKeyAnyNotAllowed:
    expected-error: 'TODO'
    conjure:
      types:
        definitions:
          default-package: test.api
          objects:
            MapAnyAliasExample:
              alias: map<any, boolean>
            MapAnyObjectExample:
              fields:
                bad: MapAnyAliasExample

What did you want to happen?

A nice validation message explaining why this is not allowed.

Also I think the source_files specification should probably describe this limitation: https://github.com/palantir/conjure/blob/develop/docs/spec/source_files.md#containertype

Proposal

In order to implement this, I think we'll need to use the DealiasingTypeVisitor but currently validation happens as objects are constructed, before the entire definition is created.

iamdanfox commented 5 years ago

Similar to #23

maciekf commented 5 years ago

It also used to be handled properly by conjure-java somehow...