search

daveshanley / vacuum

vacuum is the worlds fastest OpenAPI 3, OpenAPI 2 / Swagger linter and quality analysis tool. Built in go, it tears through API specs faster than you can think. vacuum is compatible with Spectral rulesets and generates compatible reports.
https://quobix.com/vacuum
MIT License
580 stars 48 forks source link

Add lint command flag to ignore circular references in arrays. #430

Closed eli-l closed 8 months ago

LasneF commented 8 months ago

@eli-l , @daveshanley not sure it deserve a dedicate flags , too munch of flags kill the flags :) why should circular reference should be considered in a special way

eli-l commented 8 months ago

@LasneF Because this flag control the underlying github.com/pb33f/libopenap and is rather suspending the error coming from the lib, not changing the behaviour/configuration of the rule. Other than that you are welcome to suggest alternative implementation.

daveshanley commented 8 months ago

@eli-l , @daveshanley not sure it deserve a dedicate flags , too munch of flags kill the flags :) why should circular reference should be considered in a special way

This is actually a gap that needs filling. @eli-l is correct in that there are two switches available in libopenapi that allow fine grained control over circular references and how the library deals with them.

  1. Anything that is a loop is a circular reference
  2. Anything that loops as part of an array (via items) is an array based loop
  3. Anything that loops as part of a polymorphic type oneOf, anyOf, allOf is a poly based loop.

Depending on your usecase and needs, type 1 might be all you need (which is the default). However, there are real use cases that exist where array based circular refs are OK, and the same with poly. As in, the tools know how to deal with them, and libopenapi should ignore them (but still keep track of them and know not to recurse into them in other use cases).

The ability to decide which level of circular reference sensitivity is available in libopenapi but it's not been exposed as a control via vacuum yet.

Thats what this PR solves.

However, if we're going to add in a switch for ignoring the array based loops, we should also include her sister poly loops as well, adding another flag to control that property would be a well rounded upgrade!

eli-l commented 8 months ago

@daveshanley I opened another pool request #432, however test do not pass with the example taken from the docs

I do not know the reason it does not work the same way as Array flag, despite they feel to be the same. This looks like a buf of libopenapi. Despite of that I can easily add a flag for polymorphic circular reference, however I decided having a fully working yet smaller PR is better than functionality that should work based on docs but do not pass the tests. Not working flag is more evil than not having a flag :D

eli-l commented 8 months ago

I actually took an extra step to check out the libopenapi and I opened another PR there to address the underlying issue: libopenapi PR#241

I would appreciate we merge this PR ASAP, as it is a blocker for my current environment. Meanwhile I will adjust PR#432 to have a flag for the poly-ref. Meanwhile I suggest to merge it only after underlying libopenapi PR#241 is patched due to the reasons I've mentioned above.

daveshanley commented 8 months ago

I merged in the libopenapi PR (thank you for fixing that) so the current release is now at v0.15.2 which contains your fix.

You should be able to bump the version of libopenapi and add the flag now :)

eli-l commented 8 months ago

Closing, these changes are a part of PR#432