-
Notifications
You must be signed in to change notification settings - Fork 49
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
Clarify that most keys in SARIF are case-sensitive #665
Comments
Well, JSON keys are case sensitive. The SARIF 2.1.0 schema at https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/schemas/sarif-schema-2.1.0.json states on the result/ruleId property: "result": {
"description": "A result produced by an analysis tool.",
"additionalProperties": false,
"type": "object",
"properties": {
"ruleId": {
"description": "The stable, unique identifier of the rule, if any, to which this result is relevant.",
"type": "string"
}, If a key (a property of an object in the JSON schema defining SARIF) is spelled out there and required, than any other spelling shall invalidate the SARIF instance file. But, only result/message is a required property ... The problem with JSON Schema for modeling comes when properties are optional (in the schema) and no strict attribute is set on the schema. Then any "misCaSEd" key defined may become an added property unrelated to SARIF required data and the instance file may still be valid SARIF. But, the consumers may do as Windows filesystem API and "fake" spellings you request. Does this help? |
Giving insufficient guidance seems problematic.
Not particularly. It would be much better if the spec said "for defined optional things, using items in the wrong case should not result in them being treated as if they had the correct case". For what it's worth, I'm fairly confident that GitHub wouldn't actually accept this input and treat it as if it were written But, because there's nothing in the spec saying "don't do this", I can't actually file a bug saying "see, the spec says don't do this". Instead, I have to use the weaker "hey, your implementation doesn't like this, please fix your docs to match your implementation." I do understand that implementations are free to do whatever they please, but in general, implementations will give deference to declared statements, and will do whatever they feel like for undeclared bits, so declaring more is helpful to the ecosystem -- or stated simpler: ambiguity leads to compatibility nightmares. |
So the tool is compliant but the docs are not? The spec states all reserved keys in the case they shall be used. Documentation authors describing tools are free to create as many errors as the developers writing the code. How can a spec help fix spelling errors in documentation of tools? so, my final answer here is (worth repeating):
|
I think literally having |
Still, the documentation author SHALL document what the tool does - that is independent of any underlying spec the tool implements.
I repeat: The documentation author is only wrong, when the description deviates from the tool. The questions the spec shall help answer is a different one, namely the compliance of the tool with the spec. The underlying JSON Schema validation delivers already the case sensitivity. The problem of a non-strict schema (we need to offer "bags" where anything can go in, to be useful for the ecosystem) is that you never know if a key is a typo or intended. Maybe we can add some magic sentence (with a SHOULD compliance level at maximum) in SARIF version 2.2. Similar to best practice in SQL to not trick column versioning per different case in same word columns ... until people need that degree of freedom. |
That sounds promising to me. |
My hobby is dealing w/ typos... It's a very big adventure each and every time (especially determining if something is intentional). |
GitHub's documentation about SARIF incorrectly used
ruleID
in a couple of places:ruleId
is written asruleID
in a couple of places github/docs#35709In wanting to file that ticket, I tried to find a reference in the official specification to clearly indicate that they were wrong, but I couldn't find one.
https://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html doesn't seem to mention that keys are generally case-sensitive.
There is a note in
https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/sarif-v2.1.0-errata01-os-complete.html#_Toc141790874 that indicates that something related to hashes may be (it's actually fairly hard to tell that it's just talking about the properties of the hash bag, but that's a separate thing).
The text was updated successfully, but these errors were encountered: