Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial Spectral ruleset #18

Merged
merged 3 commits into from
Mar 2, 2023
Merged

Initial Spectral ruleset #18

merged 3 commits into from
Mar 2, 2023

Conversation

gregsdennis
Copy link
Member

(from https://github.com/orgs/json-schema-org/discussions/323)

This is a ruleset for Spectral that covers

I've been using the Spectral VSCode extension to write them. There are a couple gotchas when using it, though:

  • It expects the .spectral.yml file to be at the root of the workspace, so you'll need to open the /spectral folder instead of the repo root.
  • You'll need to configure the extension to ignore **/*.spectral.yml files so that it doesn't try to validate the rule files themselves.

The approach I'm using in designing these rules is to get VSCode to highlight the problem keyword specifically. For example, highlighting if (or its value) if there isn't also a then or else. This provides a better dev experience by showing where the problem is rather than just telling them.

For many of these rules I found simpler ways to express the rule, but those ways seemed to highlight more of the JSON than was necessary (e.g. highlighting the entire subschema that contains the if doesn't tell you that the if is the problem).

We will be looking for a way to properly publish these, but for now this gets them off of my local computer.

I'll also include a readme in the spectral folder.

@jdesrosiers
Copy link
Member

I'm not really sure how to review this. It's a lot and I don't think any of us a familiar enough with Spectral to understand what's going on. Is it possible to write tests for these rules? That would increase confidence without having to know all the spectral details.

@gregsdennis
Copy link
Member Author

gregsdennis commented Feb 16, 2023

Is it possible to write tests for these rules?

Maaaybe? It'd have to be a runner that runs different scenarios through the CLI and checks the console output for the intended result. Even then, I'd probably want a way to test the same scenarios across the eventually-multple tools we have here. Like have a test suite (could be a folder in this repo) and a runner for each tool.

Is that something you'd like to see in place before merging this?

@jdesrosiers
Copy link
Member

Is that something you'd like to see in place before merging this?

Not necessarily. It's something I think I would need to effectively review it, but it doesn't necessarily need thorough review at this point. I'd be ok with just merging it as a starting point.

@gregsdennis
Copy link
Member Author

gregsdennis commented Feb 22, 2023

I've been trying to conceive of how to add general linting tests, but I'm coming up empty on how to actually specify what problems should be reported and how. Each tool is going to have its own output (and what data that output reports).

Spectral, for example, reports the rule name and desciription along with a "pointer" in array form, and the way I've been writing them, they tend to (but not always) point to the problem keyword.

A tool may report the problem at the keyword or at the containing schema, and tests might need to account for both. E.g. "if requires then or else" could be reported by pointing to the if or by pointing to the subschema that contains if. A (bad?) tool might also just say what the problem is without indicating where it is.

I think the other option is to have a test suite for each tool, but that's doesn't really let you compare tools. We'd want to keep them in sync to do that, and that's a manual job.

I'll keep noodling on this. For now, to not block this, I'll create another issue for tests.

@gregsdennis gregsdennis merged commit a3126d3 into main Mar 2, 2023
@gregsdennis gregsdennis deleted the gregsdennis/spectral-ruleset branch March 2, 2023 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants