Skip to content
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

Confusing AST for enum examples #734

Open
goganchic opened this issue Jul 19, 2019 · 4 comments
Open

Confusing AST for enum examples #734

goganchic opened this issue Jul 19, 2019 · 4 comments

Comments

@goganchic
Copy link
Contributor

Suppose such doc:

# My API

# Data Structures

# Plan

+ kind (string)
    + Sample: free

Drafter generates such AST for samples section

                        samples:
                          element: "array"
                          content:
                            -
                              element: "string"
                              content: "free"

It's logically correct: samples for string element is an array of strings.

Suppose such doc:

# My API

# Data Structures

# Plan

+ kind (enum)
    + free
    + premium
    + Sample: free

Drafter generates such AST for samples section:

...
                        samples:
                          element: "array"
                          content:
                            -
                              element: "enum"
                              content:
                                element: "string"
                                attributes:
                                  typeAttributes:
                                    element: "array"
                                    content:
                                      -
                                        element: "string"
                                        content: "fixed"
                                content: "free"

I think element: "enum" is incorrect in this case. Samples for such enum should be strings from the set "free", "premium", but not a new enum type which contains single string element. So in my opinion correct sample should be something like:

                        samples:
                          element: "array"
                          content:
                            -
                              element: "string"
                              content: "free"
@tjanc
Copy link
Contributor

tjanc commented Jul 23, 2019

I'm interested, in what way is this behavior a regression/bug? What is it breaking?

Admittedly, this transformation is confusing, but I don't understand how it's incorrect. Does the emitted API Elements fail to represent the data structure given to drafter in MSON?

  • The sample is an enum, which is of the type of what we are providing a sample for.
  • The sample enum's content is not just any string value (i.e. without fixed), but the one and only free value (i.e. with fixed).

Also note that the output of drafter, API Elements, is by no means an Abstract Syntax Tree (AST) for API Blueprint.

@goganchic
Copy link
Contributor Author

@tjanc I don't mean that it's a bug or regression. Actually you are right, it's confusing behaviour.

@goganchic goganchic changed the title Wrong AST for enum examples Confusing AST for enum examples Jul 24, 2019
@tjanc
Copy link
Contributor

tjanc commented Jul 24, 2019

We were toying with the idea of saying that samples/default are interpreted as fixed in API Elements by default. If I recall correctly, we dropped that mainly because of three reasons:

  • it adds a point of contextual interpretation (you need to know whether the element is a sample to interpret it), adding confusion
  • fixed behaves differently on array and object compared to other types, more confusion
  • perhaps in the future we want to have second order types in APIE; a sample then would be something more than a value; that's why always fixing every sample to a single value might close some doors on us

That said, I don't really see a reason why we shouldn't put the string directly into samples (without a wrapping enum). My reasoning there was probably to have the type of the sample match the type of the element one to one, but that might be an unnecessary requirement.

@pksunkara What's your take on this?

@goganchic
Copy link
Contributor Author

it adds a point of contextual interpretation (you need to know whether the element is a sample to interpret it), adding confusion

Actually I can't understand your point. Do you mean that such AST could be interpreted without any context?

                              element: "enum"
                              content:
                                element: "string"
                                attributes:
                                  typeAttributes:
                                    element: "array"
                                    content:
                                      -
                                        element: "string"
                                        content: "fixed"
                                content: "free"

If I'm not mistaken this part is the same for enum with one element and enum example, so you need a bit of context to understand what is it.

fixed behaves differently on array and object compared to other types, more confusion

Could you please explain your point a bit more?

perhaps in the future we want to have second order types in APIE

What is second order types in this context?

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

No branches or pull requests

2 participants