Prevent oas-linter from loading own ruleset

[pderaaij]

Checklist

• Linting issue

I am having an attempt at letting speccy making use of oas-linter in stead of the copied in-app linter. The split in loading and applying rules is awesome, but currently it always has the supplied ruleset of oas-linter loaded.

This is due the fact it is explicitly in the file, see: https://github.com/Mermade/oas-kit/blob/master/packages/oas-linter/index.js#L241

I am not sure if speccy shouldn't be using the default ruleset, but in general it would be nice to either prevent loading of the ruleset provided by oas-linter or just clear the present rules and applying new ones later.

Curious to your thoughts on this @MikeRalphson.

[MikeRalphson]

Maybe I should move the rules loading out to the validator...

[MikeRalphson]

Maybe I should move the rules loading out to the validator...

Ok, that won't work as the loading code is needed for ruleset chaining (the require property), but having a loadDefaultRules function exposed by the linter and called by the validator if the default linter is being used looks like it works well.

Will be included in the v3.0.0 release of oas-linter / oas-validator.

[pderaaij]

Thanks for making the change! Any idea when you would like to release this version 3?

[MikeRalphson]

Certainly aiming for this week. Could be as early as this afternoon. What are the issues with the validate function you mention in wework/speccy#248 ? Anything I can help with?

[pderaaij]

Uncertain yet, have to look into it. Plan for tonight.

[MikeRalphson]

OK, will hold off, unless an RC release would be helpful?

[pderaaij]

Not necessary, will be fine! Keep you posted tonight if I figured out what's wrong.

Might be something like: 71b0f4a

[MikeRalphson]

Might be something like: 71b0f4a

Yep, there's possibly more like that lurking in there from the change to use call-me-maybe.

[pderaaij]

@MikeRalphson I found my struggle, but uncertain what the best solution is.

The pain is caused by the assertion on line 1342: https://github.com/Mermade/oas-kit/blob/master/packages/oas-validator/index.js#L1342.

It the length of warnings is higher then 0 it will throw an AssertionError as the assertion is failing. This is interpreted by speccy as an invalid specification schema instead of it containing linting errors as shown below:

validator.validate(spec, buildValidatorOptions(skip, verbose), (err, _options) => {
        const { context, warnings } = _options || err.options;

        if (err) {
            console.error(colors.red + 'Specification schema is invalid.' + colors.reset);
            console.error(formatSchemaError(err, context));
            process.exit(1);
        }

        if (warnings.length) {
            console.error(colors.red + 'Specification contains lint errors: ' + warnings.length + colors.reset);
            console.warn(formatLintResults(warnings))
            process.exit(1);
        }

        if (!cmd.quiet) {
            console.log(colors.green + 'Specification is valid, with 0 lint errors' + colors.reset)
        }
        process.exit(0);
    });

It feels a bit weird that the validator states that specification is valid, but does throw an error on validating. Linting should only be adding warnings to the option object.

So I leaning towards removing the assertion, but would love to have your thoughts.

[MikeRalphson]

Without any final assertion, the validator will declare the result of the validation (as opposed to the document itself) as having succeeded. The user has requested that the document be linted, and thus the linter returning warnings is treated as a failure.

Is that convincing?

[pderaaij]

Yes, that makes sense. I might got on the wrong foot as we are calling it warnings. Doesn't come across critical...

I'll make the change in speccy. Thanks for thinking along!