fedora-copr / vcs-diff-lint-action

Differential code linting GitHub action
2 stars 1 forks source link

The sarif output should somehow let users know about the linter #13

Open praiskup opened 1 year ago

praiskup commented 1 year ago

Experimenting with the SARIF upload, it seems that it is not obvious what tool generates the error message: Screenshot_20230131_093911

The output from vcs-diff-lint now supports at least MYPY_ERROR, MYPY_WARNING, PYLINT_WARNING types.

praiskup commented 1 year ago

@jmacku fyi

praiskup commented 1 year ago

Nah, I meant @jamacku, sorry.

jamacku commented 1 year ago

This has to be dealt with in vcs-diff-lint itself. In Differential ShellCheck, we are using the following syntax suggested by @kdudka:

# The sed part ensures that cstools will recognize the output as being produced by ShellCheck and not GCC.
local output
output=$(shellcheck "${shellcheck_args[@]}" 2> /dev/null | sed -e 's|$| <--[shellcheck]|')

But this wouldn't be the complete solution. Ideally, we should generate SARIF so that each tool has its own section.

/cc @lzaoral

For reference, SARIF with defects detected by vcs-diff-lint:

{
    "$schema": "https://json.schemastore.org/sarif-2.1.0.json",
    "version": "2.1.0",
    "inlineExternalProperties": [
        {
            "externalizedProperties": {
                "tool": "vcs-diff-lint",
                "tool-url": "https://github.com/fedora-copr/vcs-diff-lint#readme"
            }
        }
    ],
    "runs": [
        {
            "tool": {
                "driver": {
                    "name": "vcs-diff-lint",
                    "informationUri": "https://github.com/fedora-copr/vcs-diff-lint#readme"
                }
            },
            "results": [
                {
                    "ruleId": "PYLINT_WARNING: W0402[deprecated-module]",
                    "locations": [
                        {
                            "id": 0,
                            "physicalLocation": {
                                "artifactLocation": {
                                    "uri": "mock/py/mock.py"
                                },
                                "region": {
                                    "startLine": 60
                                }
                            }
                        }
                    ],
                    "message": {
                        "text": "Deprecated module 'pipes'"
                    },
                    "codeFlows": [
                        {
                            "threadFlows": [
                                {
                                    "locations": [
                                        {
                                            "location": {
                                                "id": 0,
                                                "physicalLocation": {
                                                    "artifactLocation": {
                                                        "uri": "mock/py/mock.py"
                                                    },
                                                    "region": {
                                                        "startLine": 60
                                                    }
                                                },
                                                "message": {
                                                    "text": "Deprecated module 'pipes'"
                                                }
                                            },
                                            "nestingLevel": 0,
                                            "kinds": [
                                                "W0402[deprecated-module]"
                                            ]
                                        }
                                    ]
                                }
                            ]
                        }
                    ]
                },
                {
                    "ruleId": "PYLINT_WARNING: W0611[unused-import]",
                    "locations": [
                        {
                            "id": 0,
                            "physicalLocation": {
                                "artifactLocation": {
                                    "uri": "mock/py/mock.py"
                                },
                                "region": {
                                    "startLine": 60
                                }
                            }
                        }
                    ],
                    "message": {
                        "text": "Unused import pipes"
                    },
                    "codeFlows": [
                        {
                            "threadFlows": [
                                {
                                    "locations": [
                                        {
                                            "location": {
                                                "id": 0,
                                                "physicalLocation": {
                                                    "artifactLocation": {
                                                        "uri": "mock/py/mock.py"
                                                    },
                                                    "region": {
                                                        "startLine": 60
                                                    }
                                                },
                                                "message": {
                                                    "text": "Unused import pipes"
                                                }
                                            },
                                            "nestingLevel": 0,
                                            "kinds": [
                                                "W0611[unused-import]"
                                            ]
                                        }
                                    ]
                                }
                            ]
                        }
                    ]
                }
            ]
        }
    ]
}
praiskup commented 1 year ago

Aha, thank you!

So there are no other useful metadata fields we could use to affect the web-UI report appearance. Obviously GitHub knows the ruleId string (that would be useful in web-UI) though there might be good reason why it is not displayed. Not sure if we can do something about this.

jamacku commented 1 year ago

The ruleId is visible once you click on Show more details in the report, but this link is visible only for members of the repository.

There is opened PR on csdiff that is adding links to documentation, but I don't know if it will be possible to use it with linters supported by vcs-diff-lint. But it's again visible only to members.