Parameter with required and example fails valid-example rule

[lag-of-death]

Please check if the PR fulfills these requirements

• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce?

Bug fix.

What is the current behavior?

#223

Does this PR introduce a breaking change?

No.

[philsturgeon]

Is this required or is this an older change that is left in here?

This should not need to be done. If required: true is there it means the rule is operating on a parameter object, which with the jsonpath rules you've set up, it shouldnt ever be.

The only way a schema object will have a required: true in it, is because the user has invalid API specifications, and that'll get caught earlier.

[lag-of-death]

hi @phil :)
so oas3-schema rule lints a spec with:

     parameters:
        - name: petId
          in: path
          required: true
          description: The id of the pet to retrieve
          schema:
            type: string
            example: "2323"
          example: "2323"

as OK. So, I'm assuming this is valid OAS3. Maybe it isn't?
Now:

1. valid-openapi-example will match schema and schema.example
2. valid-example will match parameters[0] and parameters[0].example

The problem is with 2, there is required: true on the object, that is later passed to

spectral/src/functions/schema.ts

Line 61 in 6590414

if (!ajv.validate(schemaObj, targetVal) && ajv.errors) {

, and that's why I'm using withoutRequiredIfNotArray. Maybe oas3-schema should not lint this case as OK ?

[philsturgeon]

this is entirely valid OAS3 yeah.

1. valid-schema-example should match example against the schema object
2. valid-openapi-example should match example against the current ($.) object

[lag-of-death]

@phil, done, withoutRequiredIfNotArray is removed now as well

[philsturgeon]

This is pretty close but OpenAPI v2 and v3 are tricky beasts. Let's do some TDD and I'll write up the tests for you to code into.

[philsturgeon]

Looks like we've split this one rule into two but have left them both in the old file? The file names match the rules so we should have:

• src/rulesets/oas3/tests/valid-openapi-example.ts
• src/rulesets/oas3/tests/valid-schema-example.ts

[lag-of-death]

If not in src/rulesets/oas3/ruleset.json, then where I should put these 2 new rules? Thought oas3/ruleset.json is the place as the two rules belong to oas3 validators, no?

[philsturgeon]

The valid-example.ts is when there was one rule. You are splitting it into two rules. So make two files, the ones above, and tadaaaa you’re done. No new ruleset, that’s a total misuse of ruleset a.

1 rule becomes 2 rules.

[philsturgeon]

I've never seen a rule specific ruleset before. What is in the intention? Is this for testing?

[lag-of-death]

It's to move out this rule from oas3 rules. My understanding is that spectral can also validate jsons that are not oas2/oas3. We could also move this rule to some other not oas2/oas3 ruleset, but haven't found one (is there one existing?)

[philsturgeon]

functions are generic and can be used anywhere. We have schemaPath as a generic function, and this is the basis of the valid example rules.

The rules are different for oas2 and oas3 because the specification structures are different.

So we reuse the schemaPath function, but do not reuse the valid example stuff because we cannot.

[lag-of-death]

And that's why I have moved valid-example out of ruleset.json for oas3

[philsturgeon]

Please remove this new file.

[lag-of-death]

@philsturgeon, if I remove it, where do I put valid-example? I'm kinda confused. I still have to test valid-example rule..

[lag-of-death]

@philsturgeon, I can move already existing tests from src/rulesets/oas3/__tests__/valid-example.ts to src/rulesets/oas2/__tests__/valid-example.ts, since valid-example rule didn't change for oas2. So this way src/rulesets/oas3/__tests__/valid-example.ts and src/rulesets/oas3/valid-example-ruleset.json are removed. Would that be OK?

[philsturgeon]

So after seeing the talk about how many different types of param/header/schema example there is, maybe you could use that to create some tests which highlight each type of parameter and property example we looked at, then make sure this change works with each of those?

The ruleset you added is not the way to go about DRYing things, although I do understand the urge. Mixing up OAS2 and OAS3 is also not the way to go, as the two specs are different.

Seeing as you have split the valid-example rule in two, and the describe tests split in two, I was expecting you to just split the valid-example file in two.

instead of having describe('valid-schema-example', () => { inside src/rulesets/oas3/__tests__/valid-example.ts the convention is to match the test file with whatever you are describing, so if you are gonna have two different rules have two different test files.

Is that all starting to make sense?

[lag-of-death]

Hi @phil. This is WIP, but I wanted to show you what's been done so far. Please mind that external examples (.externalValue) were not considered.

[philsturgeon]

I don’t understand what this file is, could you give it a quick explanation?

[lag-of-death]

Sure. @philsturgeon, this file is used by valid-type-example rule:

spectral/src/rulesets/oas3/ruleset.json

Line 2432 in 06fcb9e

"function": "exampleType"

This rule takes care of the cases where we have:

type: some_type
example: some example

So where the shape of an example is defined by .type, not by schema.

[philsturgeon]

Work is now being done in #472