writer-json: do not needlessly duplicate messages in SARIF

[lzaoral]

... when we have only one.

[lzaoral]

Note that we will lose also these nodes. However, they are not documented in GitHub's SARIF manual [1] so I guess it's harmless?

[1] https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#location-object

[kdudka]

This is fine as long as the output of csgrep --mode=sarif | csgrep --mode=json stays unchanged. For defects with more than one event, the kinds.warning field is important for csgrep to preserve data. But such defects should be unaffected by this change.

[kdudka]

Please write this as 1U < def.events.size(). The source code of csdiff always puts lower operand to left for consistency.

[jamacku]

Hmm, it looks like GitHub doesn't care if "codeFlows" message is missing. It duplicates it anyway.

But SARIF generated by csdiff looks correctly

{
    "$schema": "https://json.schemastore.org/sarif-2.1.0.json",
    "version": "2.1.0",
    "runs": [
        {
            "tool": {
                "driver": {
                    "name": "ShellCheck",
                    "version": "2.6.0.20220823.123456.g064f632.pr_77",
                    "informationUri": "https://github.com/csutils/csdiff"
                }
            },
            "results": [
                {
                    "ruleId": "SHELLCHECK_WARNING: note[SC2086]",
                    "locations": [
                        {
                            "id": 0,
                            "physicalLocation": {
                                "artifactLocation": {
                                    "uri": "src/index.sh"
                                },
                                "region": {
                                    "startLine": 9,
                                    "startColumn": 6
                                }
                            }
                        }
                    ],
                    "message": {
                        "text": "Double quote to prevent globbing and word splitting."
                    }
                },
                {
                    "ruleId": "SHELLCHECK_WARNING: warning[SC1090]",
                    "locations": [
                        {
                            "id": 0,
                            "physicalLocation": {
                                "artifactLocation": {
                                    "uri": "src/index.sh"
                                },
                                "region": {
                                    "startLine": 11,
                                    "startColumn": 3
                                }
                            }
                        }
                    ],
                    "message": {
                        "text": "ShellCheck can't follow non-constant source. Use a directive to specify location."
                    }
                },
                {
                    "ruleId": "SHELLCHECK_WARNING: note[SC2086]",
                    "locations": [
                        {
                            "id": 0,
                            "physicalLocation": {
                                "artifactLocation": {
                                    "uri": "src/index.sh"
                                },
                                "region": {
                                    "startLine": 11,
                                    "startColumn": 3
                                }
                            }
                        }
                    ],
                    "message": {
                        "text": "Double quote to prevent globbing and word splitting."
                    }
                },
                {
                    "ruleId": "SHELLCHECK_WARNING: note[SC2250]",
                    "locations": [
                        {
                            "id": 0,
                            "physicalLocation": {
                                "artifactLocation": {
                                    "uri": "src/index.sh"
                                },
                                "region": {
                                    "startLine": 120,
                                    "startColumn": 13
                                }
                            }
                        }
                    ],
                    "message": {
                        "text": "Prefer putting braces around variable references even when not strictly required."
                    }
                },
                {
                    "ruleId": "SHELLCHECK_WARNING: note[SC2250]",
                    "locations": [
                        {
                            "id": 0,
                            "physicalLocation": {
                                "artifactLocation": {
                                    "uri": "src/index.sh"
                                },
                                "region": {
                                    "startLine": 122,
                                    "startColumn": 6
                                }
                            }
                        }
                    ],
                    "message": {
                        "text": "Prefer putting braces around variable references even when not strictly required."
                    }
                }
            ]
        }
    ]
}

• Test csdiff 2 jamacku/differential-shellcheck#28

[jamacku]

LGTM

[lzaoral]

I'll mark this as a WIP since the messages are still duplicated in the visualised output.

[lzaoral]

@jamacku, aren't the duplicities actually absent with #68? Maybe the second line is always sourced from the rule metadata and duplicated only when it's missing.

[lzaoral]

That theory is debunked by redhat-plumbers-in-action/differential-shellcheck#117, so the search continues...

[lzaoral]

Hmm, this PR seems to be stuck since it is out of date with actual contents of https://github.com/lzaoral/csdiff/tree/dont-duplicate-messages and I've already tried pushing few times.

[lzaoral]

@jamacku Could you please create a issue to track the problem depicted in #77 (review), if it is still present? Thank you!

[lzaoral]

After discussion with @jamacku and @kdudka, we have decided to close this PR because it does not introduce any visible improvements for Differential ShellCheck.