Added related locations property (#491)

[sthagen]

Implemented #491

[sthagen]

I will have to add the corresponding change to the schema before this can be merged. Nothing exciting though, as the compliance level is SHOULD and thus the key will not be added to the required properties array of the object. ETA for the change is before the TC meeting today at 15:00 UTC.

And the diff compared to the sarif schema currently on branch main is / will be:

--- sarif-2-2.schema.json       2024-07-11 08:09:56.735303142 +0000
+++ sarif-2-2.new-schema.json   2024-07-11 08:24:03.748233908 +0000
@@ -1432,6 +1432,16 @@
           "description": "The runtime exception, if any, relevant to this notification.",
           "$ref": "#/$defs/exception"
         },
+        "relatedLocations": {
+          "description": "A set of locations relevant to this notification.",
+          "type": "array",
+          "minItems": 0,
+          "uniqueItems": true,
+          "default": [],
+          "items": {
+            "$ref": "#/$defs/location"
+          }
+        },
         "descriptor": {
           "description": "A reference used to locate the descriptor relevant to this notification.",
           "$ref": "#/$defs/reportingDescriptorReference"

• [x] Added related locations property to notification object

[sthagen]

@KalleOlaviNiemitalo after looking at our schema file again: When the default of an array value is the empty value and the key to that value is not in the required array of the "holding" object, my understanding is, that a consumer seeking information on that key SHALL use the empty array regardless if it was present in the consumed JSON instance. ... and the producer may or may not insert such noisy nothingness.

[sthagen]

The TC has discussed the compliance level during the meeting on 2024-07-11 along the consistency and tool provider perspective and came to the conclusion to reduce the requirement level for the notification object field relatedLocation to MAY as already is set on the same property of the result object.

Details are available in the meeting minutes draft PR or after merge in section 3.3.3 Additional discussion of the meeting minutes.

[sthagen]

@KalleOlaviNiemitalo I do not see a contradiction in the complicated second part (an introductory "SHOULD" sentence and two list items naming "cases" decorated with SHALL and SHALL NOT. But, I shortly considered introducing the sentence that currently states:

The relatedLocations property SHOULD allow [...]

instead like that:

If present, the relatedLocations property SHOULD allow [...]

As that is what is meant. I will wait a day or two but then intend to merge as agreed in the TC.

Maybe we can consider going over the specification and systematically identify similar situations and write more clearly.

[sthagen]

The motion to approve and merge this pull request succeeded during the 2024-07-11 TC meeting. Cf. 3.3.3 Additional discussion after merge of the minutes PR, until then https://github.com/oasis-tcs/sarif-spec/blob/meeting-2024-07-11/meeting_minutes/240711_SARIF_TC_89.md#332-additional-discussion.

[KalleOlaviNiemitalo]

I meant, I'd like this paragraph to use the "relevant to understanding the result" wording:

https://github.com/oasis-tcs/sarif-spec/blob/7fde9368c1ce946a1fd8fea5ffd032f5e5e49681/sarif-2.2/prose/edit/src/file-format-58-notification-object.md?plain=1#L106

instead of saying that the condition "applies" to the related locations. That way, it would be consistent with the complicated second part that says there may be locations to which the condition does not apply, and also consistent with how the relatedLocations property of the result object is described.

[sthagen]

@KalleOlaviNiemitalo I will create a subsequent issue for the TC to discuss (→ #649).