Add a JSON Schema for `.ameba.yml` and a CLI tool for automatically generating it
When added to JSON Schema Store, this will allow autocomplete and documentation information for the .ameba.yml format. Instead of writing one by-hand like was the case for shard.yml, instead I wrote a CLI tool that will automatically generate it and update it as new rules are added and changed.
https://github.com/user-attachments/assets/0e507ba0-e01a-4e04-b185-50945ed7f9f0
That looks great, thanks! ❤️ I'll review it sometime in the upcoming days.
How feasible would it be to make running this script / updating the schema a part of CI? That way ppl don't have to remember to run it for it to stay up to date.
@nobodywasishere That certainly makes sense. It needs to be automated in order to make it hassle-free for the end-users.
That certainly makes sense. It needs to be automated in order to make it hassle-free for the end-users.
Didn't feel right about having a CI do commits, so instead added one that will fail if the user needs to run shards run schema, lmk if this works.
Didn't feel right about having a CI do commits, so instead added one that will fail if the user needs to run
shards run schema, lmk if this works.
I'm gonna insist on having this automated (I see nothing bad in CI doing commits, btw) :) The end user IMO shouldn't be concerned with a need to operate tooling like this - except for the case when we're talking about rules development, then it makes sense, yet still more for local development than for our main repo.
@Sija IMO that requirement is asking too much at once. It brings a completely new concept into the CI game, which isn't even directly related. This feels like a very unnecessary obstacle.
This PR is a complete feature. It makes sure the schema cannot go out of sync. That's a solid base and should be acceptable.
Automating the synchronization is an enhancement for convenience. It should not be a hard requirement. Everything is safe and sound without it. It just replaces a round of potential manual intervention. But it's not a complex process to run the schema generator and commit its changes if CI tells you it needs an update.
Currently, this repo has not a single CI job that performs any automatic commits. So it seems very reasonable not to ask for introducing a completely new CI functionality on top of a feature change.
(I see nothing bad in CI doing commits, btw) :)
Maybe it could be fine, but I see it potentially adding to noise if someone's working on a rule, the commit being added and removed, etc, versus if it's just a simple command the dev needs to run once the rule is done being developed to pass CI.
The end user IMO shouldn't be concerned with a need to operate tooling like this
The end user won't need to run the schema command, it's only meant for developers. The committed version of the schema file on master is all people need to worry about (or the schema file for their version of ameba - if people want to get that granular).
Looks great! One thing, though, that still needs some attention is the size of the resulting json file. I believe it could be cut down by extracting the repetitive parts (common rule properties + severity enum) into their own (sub)schemas.
Good point! Quick and easy de-dups brings it from 3900 to 2100 lines
Needs definitions update and it's GTG 🚀
@straight-shoota Sorry for the late reply! I had to sleep over the topic, and I agree that's too much to ask in such a case. I plan to integrate the schema generator into the CI automation anyway, since I still believe it's the way to go. This might be done in a separate PR, though, so there's no need to conflate these two things. Thanks for bearing with me! 🙏 🐻
I think it may be good to move away from force pushes, makes PRs a lot less clear, and we can just squash when merging to master
@nobodywasishere Yeah, I've been kinda fuzzy on this, since in general I prefer having atomic changes per commit, as I find it easy to browse through the history this way l8r on. In some cases, if the PR commit history is messy, I'd prefer squashing it before merging to avoid noise. As you can see, there's a bit of inner-conflict in terms of using single approach for all of the cases 😅
Next step would be adding it to the Schema Store, is that correct?
Yes, just need to create a PR there
Looking at the Schema Store spec, it seems that there need to be a schema generated for the Ameba v1.6.4 (I reckon by applying the patch from this PR over the v1.6.4 tag) and only this published, since there's no formal v1.7.0 yet. Hmm, shit - but then, there still would be no file to link to, as v.1.6.4 is already tagged...
Looking at this more closely, I think we might want to postpone this until v1.7.0 release...
I don't think that's necessary. We can just have the schema store point to whatever the latest schema is on master. I also don't think these are necessarily versioned.
This should be all that's necessary: https://github.com/SchemaStore/schemastore/pull/3669
Then, the suggestions might end up incorrect due to the possible version difference between master and the locally installed version.
That's completely fine imo. We don't know what version of ameba they're using and master provides the most up-to-date version. More accurate autocompletes n such can be provided by ameba-ls
We don't know what version of ameba they're using [...]
Do we? That's what piqued my interest, since I've seen versioned schemas in there 🤔
I haven't seen any if you can link to one
Do we?
I meant client-side, in vscode or w/e. I don't know how vscode would know which schema to select beyond the most recent, as it doesn't involve ameba binaries at all.
In terms of releasing a v1.6.4 schema, we can just create a new v1.6.4-schema tag w/ this commit and point to that in the schema store. Doesn't need to be in the v1.6.4 tag
Me forgetting that the Version prop exists
I haven't seen any if you can link to one
https://github.com/SchemaStore/schemastore/pull/5122
Ahhh okay this is possible. vtesttree.yaml is an example to follow.
https://github.com/SchemaStore/schemastore/blob/master/CONTRIBUTING.md#how-to-add-a-json-schema-with-multiple-versions
Nice! In such a case, I'd generate the schema for v1.6.4 first and host it in the schemastore repo to see how it works, so if any changes are needed, we'll have it ironed out before v1.7.0, wdyt?
https://github.com/SchemaStore/schemastore/pull/5131
I'd prefer hosting versioned schemas (all except master) in the schemastore repo to avoid keeping lingering branches.
Relevant configuration settings.json, allowing for testing the feature until it's officially supported.
VS Code
"yaml.schemas": {
"https://raw.githubusercontent.com/crystal-ameba/ameba/master/.ameba.yml.schema.json": ".ameba.yml"
}
Zed
"lsp": {
"yaml-language-server": {
"settings": {
"yaml": {
"schemas": {
"https://raw.githubusercontent.com/crystal-ameba/ameba/master/.ameba.yml.schema.json": [
"/.ameba.yml"
]
}
}
}
}
}