search

fedora-copr / vcs-diff-lint-action

Differential code linting GitHub action
2 stars 2 forks source link

The uploaded SARIF doesn't seem to be always reflected in PRs #19

Open praiskup opened 1 year ago

praiskup commented 1 year ago

E.g. https://github.com/rpm-software-management/mock/pull/1103

  1. Ci is Failed: "Lint Python issues / python-lint-job (pull_request) Failing after 40s" task

  2. Log says

    ## Linting in 'mock' subdirectory ##
# ================
#  Added warnings 
# ================

Error: PYLINT_WARNING:
py/mockbuild/buildroot.py:11: W0611[unused-import]: Unused import shlex

Error: PYLINT_WARNING:
py/mockbuild/buildroot.py:22: W0611[unused-import]: Unused RootError imported from exception

But there are no comments "in-line" about those.

jamacku commented 1 year ago

I have noticed a similar thing with differential-shellcheck. In code with "more" defects, GitHub sometimes doesn't show some alerts in UI.

It is related to the recent introduction of GitHub "differential" scanning. GitHub tries to show only relevant alerts, but its algorithm could be better. They are using SARIF fingerprints.

Please see @kdudka comment: https://github.com/csutils/csdiff/issues/98#issuecomment-1494180067

praiskup commented 1 year ago

So does the hash content actually matter? We never report other than new errors; except that it would be useful to have something meaningful long-term, would generating random hashes just help us to work around the problem?

jamacku commented 1 year ago

The implementation of fingerprints in csdiff might help with this issue. But it's quite challenging to make it right.

Maybe generating some random hashes for fingerprints might be a workaround, but I don't know how relatable.

praiskup commented 5 days ago

Similar problem, it seems that GitHub doesn't show new issues that appear in code that has not been updated. A notable example -- I drop a method call foo() from python file, which makes the from baz import foo statement above useless.

Pylint shouts to us loudly, csdiff correctly detects this as a new issue, vcs-diff-lint propagates this as new issue, but GitHub swallows the report because it appears in part of the source file that has not been modified.

praiskup commented 4 days ago

Example: https://github.com/fedora-copr/vcs-diff-lint/pull/35