Subject: RE: [sarif] RE: I don't think we need an "attachment" object any more
To emphasize: the only thing this proposal does is get rid of an object that does nothing useful except carrying a message, and putting that message it what is IMO a more appropriate place (on the file object).
Merging threads. Michael wrote:
So attachments used to have regions which could be used to annotate a region in the attachment. If simplify to a file location, don’t we lose it?
The same file in the table could be annotated in separate ways, so we can use a unique file location to for this.
The answer is:
No. Attachments have never had regions. An attachment is (and always has been) nothing but a fileLocation (uri + uriBaseId) + a description.
The thing that has a region is a physicalLocation. So for example, result.conversionProvenance is an array of physicalLocation objects, so you can see the region(s) of the analysis tool log file(s) that were converted into the SARIF result object.
You might be confusing attachment with annotation.
This change does not affect annotations. Annotations don’t live on the attachment object, they live on the location object:
… which is indeed a little twisted, but anyway, it’s not related to attachments.
To your request for a more comprehensive view of the result of my proposal:
attachments:fileLocation # Instead of attachment.
attachments:fileLocation # The only other place attachment object was used.
roles:string # Since we just put this here...
description:message # ... it seems natural for this to be here as well.
What has happened to the annotations that are associated with an attachment? Can you provide a more comprehensive view of the final type definitions you have in mind?
I’m sorry, I was so excited about this idea that I was inaccurate. conversionProvenance is of type physicalLocation (and physicalLocation = fileLocation + region), not of type attachment. And it’s the invocation object, not the run object, that has an attachments property.
But I still think replacing the attachment object with the file.description property is a good idea!
Also, I would add a new permittted value analysisToolLogFile in file.roles.
As you know, both run and result have a property attachments of type attachment. result also has conversionProvenance of type attachment. An attachment is nothing but a fileLocation plus a description. The spec says this about description:
An attachment object SHOULD contain a property named description whose value is a message object (§3.9) describing the role played by the attachment.
The “role”, huh? Be we just added a roles property to the file object! So perhaps the file object is also the natural place for description. Then we don’t need the attachment object, and instead we have:
I stumbled on this as I was starting to write the words for Issue #134, “conversion.analysisToolLogFileLocation should be an array”, which made me look at result.conversionProvenance and wonder why it was an attachment.
If we agree, I’d like to do this in the same change draft as the one I’m writing for #134, since they both touch result.conversionProvenance.