sbcgua / ajson

Yet another json parser serializer for ABAP
MIT License
49 stars 15 forks source link

Fix cyclic ref #141

Closed sbcgua closed 1 year ago

sbcgua commented 1 year ago

@larshp The exception does not refer to ZIF_AJSON anymore but the error is still there ... looks like some bug. image

larshp commented 1 year ago

hmm, yea, its broken

larshp commented 1 year ago

just disable the rule for now, the interfaces are cyclic anyhow, but will probably work in the merged report

sbcgua commented 1 year ago

@larshp Ok, I picked the fix to the master, but left this PR open. Currently it contains just the setting to enable cyclic check, so it can be restarted and merged later when the issue is fixed

larshp commented 1 year ago

reported cycle fixed with https://github.com/abaplint/abaplint/pull/2822

sbcgua commented 1 year ago

Now it is filter and mapping interfaces, they refer to types defined in zif_ajson (ty_node in particular). Hmm. Honestly speaking, semantically, the types are in a good place. Yet technically they create a cyclic ref. Maybe it makes sense to separate ty_node and Ko to zif_ajson_types or similar.

Any serious impact on AG or other tools you maintain ? @larshp @mbtools @albertmink @jrodriguez-rc

larshp commented 1 year ago

if its all good, design wise, just exclude one of the interfaces from the rule

mbtools commented 1 year ago

Minimal impact on my projects.

jrodriguez-rc commented 1 year ago

No impact from my side

albertmink commented 1 year ago

No, it is fine.

sbcgua commented 1 year ago

Ok, so most of types (except ty_opts) were move to zif_ajson_types. cyclic_ref rule re-enabled.

Let's hope nothing breaks ... merging