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
488 stars 39 forks source link

[Bug/ enhancement] oas-schema-check , valid schema is set as invalid , when leveraging required + allOf #468

Open LasneF opened 3 months ago

LasneF commented 3 months ago

given this object , vaccum raise an error about pom not defined

.\component-descriptor.yml:2617:7 | error | required field pom is not defined in properties | oas-schema-check

accoding to JSonSchema the scenario is valid , even if to me looks bad design

still Vacuum need to clearly differenciate was is invalid from structural point of view vs what is opinonated rules

=> not sure if it is a bug in the json schema structure validator, if it is a design , and opinionated rules

validated against https://github.com/hyperjump-io/json-schema & https://json-everything.net/json-schema/

{
    "type": "object",
    "required": [
        "tata" , "pom"
    ],
    "properties": {
        "tata": {
            "type": "number"
        }
    },
    "allOf": [
        {
            "type": "object",
            "properties": {
                "pom": {
                    "type": "string"
                }
            }
        }
    ]
}
daveshanley commented 3 months ago

This is an upgrade to the rule, it's not a bug - this rule is manual, and it's not checking that deep I don't think when performing a required check.

LasneF commented 3 months ago

notice that to me the rules is accurate and to me (opinionned but not factual) it s a good practice , and to me this looks bad design. but that can be not the case for others

as it is a valid pattern ; it should be a dedidated rules.

=> Need to put in place a sharp issue naming convention : each case should have its own unique identifier should be easy to put in place straight

=> given the fact that we have unique identifier for each case , adding a tag mechanism could be usefull like here schema , good practice etc , that you can display to categorize the issue . can be then leverage in a ui for filtering / sorting purpose

nb: do not change the rules just tag it as good practice , and not

i would propose naming : required-properties-missing , tag schema, good practice , and proper documentation show casing what is good , what is definitly wrong , was is passable like the use case propose