yaml: implement basic Swagger/OpenAPI subparser #3245

segoon · 2021-12-31T12:52:22Z

The parser is able to parse both Swagger 2.0 and Openapi 3.0.x.
The code is partly based on ansible playbook parser. It was tested
using the following vim's TagBar parser declaration:

let g:tagbar_type_yaml = {
                    \  'ctagstype': 'openapi',
                    \  'kinds': [
                    \          'p:path',
                    \          'd:schema',
                    \          'P:parameter',
                    \          'R:response',
                    \          ]
                    \ }

masatake · 2021-12-31T16:26:59Z

parsers/openapi.c

+*
+*   This source code is released for free distribution under the terms of the
+*   GNU General Public License version 2 or (at your option) any later version.
+*


Could you add references for the target language or input like:

ctags/parsers/gdscript.c

Line 12 in 8215492

* GDScript language reference:

git push --force is welcome.

codecov · 2021-12-31T16:27:11Z

Codecov Report

Merging #3245 (370796b) into master (8215492) will increase coverage by 0.32%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3245      +/-   ##
==========================================
+ Coverage   84.98%   85.31%   +0.32%     
==========================================
  Files         207      208       +1     
  Lines       49324    49525     +201     
==========================================
+ Hits        41916    42250     +334     
+ Misses       7408     7275     -133

Impacted Files	Coverage Δ
parsers/openapi.c	`100.00% <100.00%> (ø)`
parsers/rst.c	`92.98% <0.00%> (-6.33%)`	⬇️
main/lregex.c	`82.07% <0.00%> (-1.13%)`	⬇️
main/field.c	`92.73% <0.00%> (-0.29%)`	⬇️
optlib/markdown.c
parsers/markdown.c	`100.00% <0.00%> (ø)`
parsers/html.c	`99.25% <0.00%> (+<0.01%)`	⬆️
parsers/ada.c	`87.21% <0.00%> (+0.06%)`	⬆️
parsers/rust.c	`92.16% <0.00%> (+0.11%)`	⬆️
parsers/basic.c	`99.03% <0.00%> (+0.55%)`	⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8215492...370796b. Read the comment docs.

masatake · 2021-12-31T16:35:26Z

Writing a subparser on the YAML parser is an incredible challenge!
Let's jump into the wild area.

masatake · 2021-12-31T16:36:43Z

Is there any difference between the targets, Swagger 2.0 and Openapi 3.0.x?

The parser is able to parse both Swagger 2.0 and Openapi 3.0.x. The code is partly based on ansible playbook parser. It was tested using the following vim's TagBar parser declaration: let g:tagbar_type_yaml = { \ 'ctagstype': 'openapi', \ 'kinds': [ \ 'p:path', \ 'd:schema', \ 'P:parameter', \ 'R:response', \ ] \ }

segoon · 2021-12-31T18:36:59Z

parsers/openapi.c

+	KEY_PATHS,
+};
+
+static const enum openapiKeys responses3Keys[] = {


Is there any difference between the targets, Swagger 2.0 and Openapi 3.0.x?

They are 2 different revisions of the protocol (Swagger == OpenAPI 2.0). They are mostly the same except for several details (not interesting for ctags) and several sections have a distinct path in yaml (e.g. "responses" in swagger and "components.responses" for openapi). I tried to express it in XXXKeys names with 2 (swagger) or 3 (openapi). pathKeys is the same for both.

I see. Thank you.

masatake · 2022-01-02T15:09:40Z

Units/parser-openapi.r/swagger.d/input.yaml

+swagger: '2.0'
+info:
+    description: test
+    title: test


How about tagging the title?

If it makes sense to tag all user-defined entities, then ok.
But then more fields must be tagged:

info itself

servers children

webhooks children

security children

tags children

I don't think the following entities worth adding tags (too minor IMHO):

openapi

jsonSchemaDialect

externalDocs

https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#openapi-object

What do you think?

Also the items of "components": https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#fixed-fields-6

The way I wrote is wrong.
In 97915e7, your parser extracts "title".
However, what I expected is your parser extract "test" in "title: test".

masatake · 2022-01-02T15:11:01Z

Units/parser-openapi.r/openapi.d/input.yaml

+    version: '1.0'
+
+servers:
+  - url: http://example.com


How about tagging the URLs under servers?

masatake · 2022-01-02T15:11:18Z

Units/parser-openapi.r/openapi.d/input.yaml

@@ -0,0 +1,55 @@
+openapi: 3.0.0
+info:
+    title: test


How about tagging the title?

masatake · 2022-01-02T15:19:55Z

parsers/openapi.c

+		{ DEPTYPE_SUBPARSER, "Yaml", &openapiSubparser },
+	};
+
+	parserDefinition* const def = parserNew ("OpenApi");


It seems that OpenAPI may be better than OpenApi.

ok, no problem

masatake · 2022-01-02T15:25:41Z

Though I named the yaml parser "Yaml", I think "OpenAPI" is better than "OpenApi" as the name of your parser.

(The review is in progress.)

masatake · 2022-01-05T02:50:15Z

parsers/openapi.c

+	def->kindTable	= OpenApiKinds;
+	def->kindCount = ARRAY_SIZE (OpenApiKinds);
+	def->parser	= findOpenApiTags;
+	def->useCork = CORK_NIL;


CORK_NIL is not suitable value for useCork field.
The useCork field is initialized with 0 in parserNew. So you don't have to give a value to the field as far as your parser doesn't need cork indexes. Let's remove this line.

masatake · 2022-01-05T02:51:25Z

parsers/openapi.c

+	return def;
+}
+
+/* vi:set tabstop=4 shiftwidth=4: */


Is .editorconfig not enough?

it was a copy-pasted line from ansible parser, removed

Oh, sorry, I wonder where it came from because I don't use vim:-).

masatake · 2022-01-05T03:21:37Z

parsers/openapi.c

+											 yaml_token_t *token)
+{
+	int i;
+	for (i = 0; i < ARRAY_SIZE(tagSources); i++) {


Will you maintain this parser？
If the answer is "yes", you can choose your favorite C coding style.
If the answer is "no", I would like you to follow the ctags' coding style like:

for (i = 0; i < ARRAY_SIZE(tagSources); i++) { ...

See the position of {.

masatake · 2022-01-05T03:25:55Z

parsers/openapi.c

+					openapi->type_stack,
+					ts->keys,
+					ts->countKeys
+					)) {


Impressive technique. I must rewrite the ansibleplaybook.c with this technique.

masatake · 2022-01-05T03:28:36Z

parsers/openapi.c

+
+static void inputEnd(subparser *s)
+{
+	Assert (((struct sOpenApiSubparser*)s)->type_stack == NULL);


Even if a broken input is given to the parser, we can expect the type_stack becomes empty at the end of parsing?

If we cannot expect it, we should clear the stack here instead of doing "Assert".

masatake · 2022-01-05T19:16:21Z

parsers/openapi.c

+
+static enum openapiKeys parseKey(yaml_token_t *token)
+{
+  if (scalarNeq(token, 10, "components"))


For mapping from "string" to a key, you can use functions defined in main/keyword.h.
I write this here is not for performance. With the function, the code of this parser can be more declarative.

masatake · 2022-01-05T19:33:10Z

This parser is really nice. The technique used in this parser excites me.

In past years, I have been thinking about YPATH, something like XPath but for YAML.
However, I cannot find the time and the way how to implement such a thing.
This parser illustrates the way.
Though the code in this pull request is for OpenAPI. However, most part of the code can be moved to yaml.c/yaml.h.
It will make developing a subparser on the YAML parser much easier.

I'm reading openapi spec slowly. What should be tagged or should not be tagged is not so obvious to me.
It will take more time.

masatake · 2022-01-05T21:16:59Z

@segoon, thank you for updating.
I'm not good at English. So I converted what I wrote here as review comments to C language. See #3254.
It seems that the many parts of #3254 are already included in the updating.

masatake · 2022-01-05T23:12:31Z

handleKey and handleValue do the same with different tables.
How about merging them like:

diff --git a/parsers/openapi.c b/parsers/openapi.c
index 9e26aecdf..493f32ed1 100644
--- a/parsers/openapi.c
+++ b/parsers/openapi.c
@@ -447,13 +447,15 @@ const struct tagSource tagValueSources[] = {
    },
 };
 
-static void handleKey(struct sOpenApiSubparser *openapi,
-                                            yaml_token_t *token)
+static void handleToken(struct sOpenApiSubparser *openapi,
+                       yaml_token_t *token,
+                       const struct tagSource  tss[],
+                       size_t ts_count)
 {
    int i;
-   for (i = 0; i < ARRAY_SIZE(tagSources); i++)
+   for (i = 0; i < ts_count; i++)
    {
-       const struct tagSource* ts = &tagSources[i];
+       const struct tagSource* ts = &tss[i];
 
        if (stateStackMatch(
                    openapi->type_stack,
@@ -473,31 +475,17 @@ static void handleKey(struct sOpenApiSubparser *openapi,
    }
 }
 
+static void handleKey(struct sOpenApiSubparser *openapi,
+                                            yaml_token_t *token)
+{
+   handleToken (openapi, token, tagSources, ARRAY_SIZE(tagSources));
+}
+
 
 static void handleValue(struct sOpenApiSubparser *openapi,
                        yaml_token_t *token)
 {
-   int i;
-   for (i = 0; i < ARRAY_SIZE(tagValueSources); i++)
-   {
-       const struct tagSource* ts = &tagValueSources[i];
-
-       if (stateStackMatch(
-                   openapi->type_stack,
-                   ts->keys,
-                   ts->countKeys
-           ))
-       {
-           // printf("match value! i=%d\n", i);
-
-           tagEntryInfo tag;
-           initTagEntry (&tag, (char *)token->data.scalar.value, ts->kind);
-           attachYamlPosition (&tag, token, false);
-
-           makeTagEntry (&tag);
-           break;
-       }
-   }
+   handleToken (openapi, token, tagValueSources, ARRAY_SIZE(tagValueSources));
 }
 
 static void openapiPlayStateMachine (struct sOpenApiSubparser *openapi,

masatake · 2022-01-05T23:15:42Z

I am sorry for not providing the details all at once.
How do you think about #3254 ?
If you are o.k., I will merge #3254.
We can polish the parser after merging it.

segoon · 2022-01-06T07:29:54Z

I'm OK with that.

yaml: implement basic Swagger/OpenAPI subparser

masatake · 2022-01-06T13:48:35Z

#3254 is merged. Thank you.
Could you make a new pull request for the improvements?

segoon · 2022-01-06T16:38:51Z

here: #3258

masatake reviewed Dec 31, 2021

View reviewed changes

segoon force-pushed the feature/parser-openapi branch from bb573fd to 36a38b0 Compare December 31, 2021 18:17

segoon commented Dec 31, 2021

View reviewed changes

masatake reviewed Jan 2, 2022

View reviewed changes

masatake reviewed Jan 5, 2022

View reviewed changes

segoon added 14 commits January 5, 2022 22:52

rename

9f2d7e7

remove vim footer

ec41b0d

do not use cork

ca51085

tabs, {}, rm the last line

c4160ee

handle tags, externalDocs, security, servers

b94e5c7

fix security, servers values

1d0e1ec

prepare tests

876afe2

update test

d475cb2

handle new keys

4098994

fix tests

97915e7

fixes

e44cbe0

update test data

60d16fc

code style

ddf8f57

format

f4bc443

rm Assert

370796b

masatake added a commit that referenced this pull request Jan 6, 2022

Merge pull request #3245 from segoon/feature/parser-openapi

f3da80e

yaml: implement basic Swagger/OpenAPI subparser

masatake closed this Jan 6, 2022

masatake mentioned this pull request Jan 7, 2022

Can't get yaml parsing working #1322

Closed

yaml: implement basic Swagger/OpenAPI subparser #3245

yaml: implement basic Swagger/OpenAPI subparser #3245

Conversation

segoon commented Dec 31, 2021

Choose a reason for hiding this comment

codecov bot commented Dec 31, 2021 • edited Loading

Codecov Report

masatake commented Dec 31, 2021

masatake commented Dec 31, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

segoon Jan 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masatake commented Jan 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masatake commented Jan 5, 2022

masatake commented Jan 5, 2022

masatake commented Jan 5, 2022

masatake commented Jan 5, 2022

segoon commented Jan 6, 2022

masatake commented Jan 6, 2022

segoon commented Jan 6, 2022

codecov bot commented Dec 31, 2021 •

edited

Loading

segoon Jan 2, 2022 •

edited

Loading