RE: comments on draft with no action

["Larry Golding (Myriad Consulting Inc)" <v-lgold@microsoft.com>]

Hi Jim,

With the exception of "location relationships" which we're still considering, the items you list below are the ones we decided not to take. I sent you an email with responses on all your items, including rationales for the ones we did not take. I've attached it here to share it with everybody. I suppose I should fill out #366 with the rest of the responses.

Thanks,
Larry

-----Original Message-----
From: James Kupsch <kupsch@cs.wisc.edu> 
Sent: Monday, April 15, 2019 1:43 PM
To: Larry Golding (Myriad Consulting Inc) <v-lgold@microsoft.com>; sarif@lists.oasis-open.org
Subject: comments on draft with no action

Larry,

There are a few additional comments that were not listed in issue #366. 
I just want to make sure that these were not missed (one of them is in issue #375), but I do not see tickets or comments for the others.

There should have been changes or comments in the docx I uploaded earlier.

Jim




------------------------------------------------------------
p.38 3.11.3 Plain text messages
I think the language about sentences should removed:

1) many of the existing tools in the SWAMP do not produce messages that are complete sentences, so this is hard to enforce for converters
2) I think there are languages that do not have sentences, and
3) programmatically defining a sentence is hard for english language
messages:  'The question is "How is Mr. X!?"' ends on the double quote character.
4) in general this algorithm needs to handle different languages which is likely difficult.

A better solution would be to have an abbrText and abbrMarkdown type of messages for single line, short (for some definition of short such as 50 characters).  If not present display the first number of character followed by an elipse that fit the display area.
----

------------------------------------------------------------
p.57 3.15.2 location property
No need to to restrict uriBaseId usage.  Change "its uri property SHALL be an absolute URI using the sarif scheme (Â3.10.3), and its uriBaseId property SHALL be absent." -> "the resolved URI SHALL be an absolute URI using the sarif scheme (Â3.10.3)."
----

------------------------------------------------------------
p.66 3.18.6 name property
It would be nice to have an optional abbrName property that is the tool name restricted to 15 characters for use by viewers and reports.  This allows a narrow column that can display the tool's name.  I have seen viewers where the default column name for the tool and rule is too narrow (by default usually) to display this information.
----

------------------------------------------------------------
p.67 3.18.15 product property
How does product relate to name and fullName?
----

------------------------------------------------------------
p.68 3.18.18 fullDescription property
Remove first sentence paragraph and use shortDescription as sentence is ill-defined.
----

------------------------------------------------------------
p.103 3.25.14 fingerprints property
Why the restriction "A direct SARIF producer SHOULD NOT populate".  It seems like a direct produce MAY produce these fingerprints, and very well produce better fingerprints than other mechanisms.
----

------------------------------------------------------------
p.105 3.25.20 relatedLocations (Issue #375) A related location should say what is related to with a relatedTo property that is a URI (a sarif scheme uri relative to thisResult) this is necessary as a tool may produce a relatedLocation that has its own related location for instance error an error in foo.h may have a related location to say it was included from bar.h which was included in baz.c. 
Also since there may also be multiple locations, they may each have unique related locations.
----

------------------------------------------------------------
p.146 3.44.3 id property
For viewers, it would be really useful to have a short (20-30) character human readable id (abbrName property) that can be used to display in columnar tables or UI sidebars that can be localized and is not necessarily stable.

Id's are not supposed to be generated by converters, but result management system is supposed to use them for comparison.
----

------------------------------------------------------------
p.149 3.44.10 fullDescription
Remove discussion about first sentence.  Require shortDescription instead.
----

--- Begin Message ---

• From: "Larry Golding (Myriad Consulting Inc)" <v-lgold@microsoft.com>
• To: James Kupsch <kupsch@cs.wisc.edu>
• Date: Sat, 13 Apr 2019 22:18:19 +0000

Jim,

 

Thanks for your incredibly detailed reading of the spec and for the feedback. I addressed the easy half of the issues â the purely copy-edit items â myself, and discussed the remainder with Michael.

 

Hereâs what we came to. There is more detail in this issue comment: https://github.com/oasis-tcs/sarif-spec/issues/366#issuecomment-481827464.

 

We took the following changes that you proposed:

 

1. We enabled client UX to display descriptions for originalUriBaseIds symbols by adding a description property of type message to the artifactLocation object. (We also added artifact.description.)
2. We replaced toolComponent.artifactIndices with a property locations of type artifactLocation[] to avoid the problem that toolComponent.artifactIndices would force theRun.artifacts into existence.
3. We changed the requirement on reportingDescriptorRelationship.kinds from SHALL to MAY, and provide a default of [ "relevant" ].
4. We recommended (normative SHOULD) that date/time properties other than release dates should have precision of at least whole seconds.
5. We removed the requirements that a plain text message consist of a single paragraph and that it have no line breaks.
6. We clarified that a toolComponentâs guid property is a stable identifier for the tool and add a requirement (normative SHALL) that it not vary with tool version.
7. We removed the advice against converters synthesizing ruleId. More generallyâ
8. We modified Appendix D (Production of SARIF by converters) to remove the advice against synthesizing properties.
   
   In this case in particular, your argument persuaded us to change our long-standing opinion on a topic.
9. We modified Appendix H to no longer suggests distinct uriBaseIds as the way to distinguish instance of multiply generated files; rather, we use distinct indices into the artifacts array as you suggested.
   
   We did not use the "modified" role, which means something else, but we can talk about defining roles for generated files if you like.

The following suggestion was rendered moot by the changes we accepted:

 

10. Appendix D no longer uses the lack of a converter version number to justify the now-removed advice against synthesizing properties, so the text that incorrectly stated that a converter doesnât have a version number has been removed.
    
    We could discuss whether the general guidance for version numbers on tool components should be changed. As the spec stands, all the versioning properties MAY be present. Iâd be open to tightening that, for instance, âAt least one of version or semanticVersion SHOULD be present.â

 

Here are explanations on some of your other feedback items:

 

11. With regard to your question about the distinction between toolComponent.productSuite, product, and name, think âMicrosoft Office/Excel/Excel Charting Toolâ.
12. With regard to your suggestion to provide a short (20-30 character) human readable name for a reportingDescriptor that is localizable and not necessarily stable, we do have such a property: reportingDescriptor.name. See Â3.46.7 in the provisional draft (Â3.44.7 in the e-ballot-3 draft). We donât provide explicit guidance on its length, but the example given ("SpecifyMarshalingForPInvokeStringArguments") conveys the idea.
13. With regard to your suggestion in Â3.15.2 (externalPropertyFileReference.location) that we not restrict uriBaseId, this section does not actually introduce a new restriction. Â3.4.4 (artifactLocation.uriBaseId property) already says:
    
    If the uri property contains an absolute URI, the uriBaseId property SHALL be absent.
    
    Although strictly speaking thereâs no need to repeat that in Â3.15.2, I think itâs helpful, so I added these words:
    
    If location is present, its uri property SHALL be an absolute URI using the sarif scheme (Â3.10.3), and so (by Â3.4.4) its uriBaseId property SHALL be absent.
14. With regard to your question about why we advise direct producers against populating result.fingerprints, itâs because we have another slot reserved for fingerprints computed by a tool: result.fingerprintContributions. The idea is that we allow a tool to supply its notion of a result fingerprint. But the actual fingerprint value(s) used by the engineering system are up to the engineering system. So we reserve result.fingerprints for the engineering system.

For the following suggestion, we have two proposals:

 

15. âAdd more languagesâ. We donât want to be in the business of proposing an exhaustive list, or even of presenting a list that spans many pages. Here are two options:
    
    a) In the Appendix, add a statement to the effect that this list is just a start, and we expect the community to develop it over time.
    
    b) Remove the Appendix.

 

We decided not to take the following suggestions:

 

16. With regard to the suggestion to introduce toolComponent.abbreviatedName, we prefer to add guidance to the effect that the tool name should be narrow because you never know where itâs going to be displayed, and if the name is long, at least make sure that its leading portion is sufficiently informative to survive the name being truncated.
17. With regard to the guidance around toolComponent.shortDescription and fullDescription: We understand that converters canât always follow the guidance; converters are always at the mercy of the native output format. And a tool author who writes in a language with no sentences, or who writes a message whose first sentence canât be readily identified, should follow the guidance and supply both short and full descriptions.
18. Likewise for toolComponent.shortDescription and fullDescription.
19. Likewise for the guidance in Â3.11.3 about plain text messages (we did take the suggestion to remove the restriction to a single paragraph).

 

Finally, we are still considering the ârelated locationsâ design. We are open to taking it if it can be taken without breaking objects that are already in common use. (With regard to #2 above, we were willing to take a breaking change because weâre confident nobody is using the feature yet).

 

In summary, thank you so much for all the great feedback. SARIF is much improved from it, and from your earlier contributions.

 

Best regards,

Larry

 

--- End Message ---