apiaryio / drafter

API Blueprint Parser (C++)
https://apiblueprint.org/
MIT License
301 stars 54 forks source link

Confusing AST for enum examples #734

Open goganchic opened 5 years ago

goganchic commented 5 years ago

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

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?


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

goganchic commented 5 years ago

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

tjanc commented 5 years ago

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:

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

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?