Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
__tests__/diffcrlf.input.txt -text
37 changes: 34 additions & 3 deletions .github/workflows/check-code.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,15 @@ jobs:
- {linter: 'yamllint', format: 'yamllint', regex: '', levelMap: '', analysisPath: '.'}
- {linter: 'ghalint', format: 'ghalint', regex: '', levelMap: '', analysisPath: '.'}
- {linter: 'sarif', format: 'sarif', regex: '', levelMap: '', analysisPath: '.'}
- {linter: 'sariffix', format: 'sarif', regex: '', levelMap: '', analysisPath: '.'}
- {linter: 'diff', format: 'diff', regex: '', levelMap: '', analysisPath: '.'}
- {linter: 'diffcrlf', format: 'diff', regex: '', levelMap: '', analysisPath: '.'}
- {linter: 'flake8subpath', format: 'flake8', regex: '', levelMap: '', analysisPath: 'A\B'}
- {linter: 'noissues', format: 'flake8', regex: '', levelMap: '', analysisPath: '.', outcome: 'success'}
- {linter: 'noissues', format: 'flake8', regex: '', levelMap: '', analysisPath: '.', fail: 'false', outcome: 'success', name: 'noissues-nofail'}
- {linter: 'pylint', format: 'pylint', regex: '', levelMap: '', analysisPath: '.', fail: 'false', outcome: 'success', name: 'pylint-nofail'}
- {linter: 'pylint', format: 'pylint', regex: '', levelMap: '', analysisPath: '.', failOnlyNew: 'true', outcome: 'success', name: 'pylint-onlynew'}
# yamllint disable-line rule:line-length
- {linter: 'pylint', format: 'pylint', regex: '', levelMap: '', analysisPath: '.', onlyNew: 'true', output: 'noissues', outcome: 'success', name: 'pylint-onlynew'}
- linter: 'custom'
format: ''
# yamllint disable-line rule:line-length
Expand All @@ -109,7 +113,7 @@ jobs:
levelMap: ${{ matrix.run.levelMap }}
analysisPath: ${{ matrix.run.analysisPath }}
fail: ${{ matrix.run.fail || 'true' }}
failOnlyNew: ${{ matrix.run.failOnlyNew || 'false' }}
onlyNew: ${{ matrix.run.onlyNew || 'false' }}
- name: Test Outcome
run: |
if [ "${{ steps.run.outcome }}" != "${{ matrix.run.outcome || 'failure' }}" ];
Expand All @@ -118,7 +122,7 @@ jobs:
exit 1
fi
- name: Create Diff
run: json-diff "__tests__/${{ matrix.run.linter }}.output.json" "sarif.json" | tee diff.txt
run: json-diff "__tests__/${{ matrix.run.output || matrix.run.linter }}.output.json" "sarif.json" | tee diff.txt
- name: Test Output
run: |
if [ -s diff.txt ];
Expand All @@ -129,3 +133,30 @@ jobs:
else
echo "Success"
fi
gha-renamed:
name: GitHub Action (renamed input)
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Run Action
id: run
continue-on-error: true
uses: ./
with:
inputFile: '__tests__/noissues.input.txt'
toolName: 'test'
inputFormat: 'flake8'
failOnlyNew: 'true'
- name: Test Outcome
run: |
if [ "${{ steps.run.outcome }}" != "failure" ];
then
echo "Expected the action to reject the renamed input but it ended with ${{ steps.run.outcome }}"
exit 1
fi
if [ -f sarif.json ];
then
echo "Expected the action to reject the renamed input before writing any output"
exit 1
fi
124 changes: 120 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,24 @@ steps:
- `sarif`: The path to the output SARIF file this action should generate. If not specified, the action will generate a `sarif.json` file in the root of the
repository. If set to an empty string, the action will not write a SARIF file. The SARIF is always generated and printed to the workflow log.

- `comment`: Set to true to comment on the PR with the issues. If set to false or ommitted, the action will not comment on the PR.
- `comment`: Set to true to comment on the PR with the issues. If set to false or ommitted, the action will not comment on the PR. Issues that carry a fix are
commented as [suggested changes](#suggested-changes). An issue is commented on only if every line it spans is part of the pull request's diff, as GitHub
rejects comments anchored outside it. Comments are [reconciled](#comment-reconciliation) between runs rather than recreated.

- `summary`: True by default - generates a markdown summary for the job. If set to false, the action will not generate a markdown summary.

- `fail`: True by default - fails the step if the linter found any issues. If set to false, the action will not fail the step.

- `failOnlyNew`: Set to true to fail only on issues found on lines added in the pull request (requires running on a pull request). If set to false or
omitted, the action will fail on any issue. Ignored if `fail` is set to false.
- `onlyNew`: Set to true to ignore every issue that is not on a line added in the pull request (requires running on a pull request). The issues are filtered
before anything else happens, so the SARIF file, the workflow log, the summary, the comments and the step's success or failure all reflect only the new
issues. If set to false or omitted, the action considers every issue. An issue spanning several lines is considered new if any one of them was added, since
fixing such an issue usually requires changing the lines around it as well.

Note that this also removes the old issues from the SARIF, so uploading it to code scanning resolves their alerts. Leave it unset when the SARIF is uploaded
and the alerts of the whole repository should be kept.

This input was named `failOnlyNew` before it applied to anything but the failure. Since GitHub silently ignores an input an action does not declare, the
action fails with an explanatory error when the old name is passed, rather than letting it look like it is still in effect.

- `toolName`: _(required)_ The `tool name` that will be written in the SARIF output. This is used by both code scanning and auto-pr-commenting to resolve fixed
issues.
Expand All @@ -64,6 +74,8 @@ steps:
root. This is required only when the linter's output contains paths that are relative but not to the repository's root, for which this action will
re-relativize them.

- `message`: The message of the issues found by an input format that does not carry one, currently only `diff`. Defaults to `Not formatted correctly`.

- `githubToken`: Relevant only for "comment" mode. The GitHub token to use to post the comment. If not specified, the action will use the action's token.

#### Natively Supported Linter Output Formats
Expand All @@ -85,7 +97,40 @@ This action supports a bunch of linter output formats, for which no `inputRegex`
- `ghalint`: The format of [ghallint](https://github.com/suzuki-shunsuke/ghalint/cmd/ghalint/) linter's parsable output.

- `SARIF`: A [standard format for static analysis](https://sarifweb.azurewebsites.net/). This is useful if you already have a SARIF file and want to create a summary
for it, or create comments on the PR.
for it, or create comments on the PR. It can carry [suggested changes](#suggested-changes).

- `diff`: The output of `git diff`, which turns [any formatter that rewrites files in place](#formatter-diffs) into a linter reporting suggested changes.

#### Formatter Diffs

The `diff` input format turns the output of `git diff` into issues, which makes any formatter that can rewrite files in place a linter reporting
[suggested changes](#suggested-changes):

```yaml
- run: clang-format -i $(git ls-files '*.cpp')
- run: git diff > clang-format.diff
- uses: bugale/bugalint@v1
with:
inputFile: 'clang-format.diff'
toolName: 'clang-format'
inputFormat: 'diff'
message: 'Not formatted according to .clang-format'
comment: true
```

Every contiguous run of changed lines becomes one issue, rather than every hunk, so the context lines `git diff` prints around each change do not widen the
reported range. Issues are anchored on the lines of the old side of the diff, which are the lines of the committed file that the pull request shows and that
comments can be attached to, while the new side becomes the fix. A run that only adds lines has no line of its own to anchor to, so it is extended to a
neighbouring line, preferring the preceding one, whose content is repeated in the fix. The marker `git diff` prints for a file that does not end with a newline
is ignored, so the last line of such a file is reported like any other. A change of that terminator alone leaves the old and the new lines identical, so the
issue is reported without a fix rather than with a suggestion replacing a line with itself. A run replacing lines with nothing deletes them, while one replacing
them with an empty line blanks them, which is what a formatter stripping the whitespace of a blank line produces.

Note that a formatter that fails without writing anything produces an empty diff, which is indistinguishable from a formatter that found nothing to fix. The
step running the formatter should therefore fail the job by itself.

Unlike the other input formats, a diff is read byte for byte, since a carriage return in it may be content rather than a line terminator. A repository storing
its files with CRLF therefore gets suggestions with CRLF in them, instead of suggestions that silently rewrite the line endings of every line they touch.

#### Input Regex Named Groups

Expand Down Expand Up @@ -113,6 +158,77 @@ The supported named groups are:

- `ecol`: The end column on which the issue was reported.

### Suggested Changes

When an issue carries a fix, the comment posted on the pull request contains it as a
[suggested change](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/incorporating-feedback-in-your-pull-request),
which a reviewer can apply in one click. Fixes are produced by the [`diff` input format](#formatter-diffs), and are read from the first `replacements` entry of
the first `artifactChanges` entry of the result's first `fixes` entry when `inputFormat` is `sarif`:

```json
{
"message": { "text": "Not formatted correctly" },
"locations": [{ "physicalLocation": { "artifactLocation": { "uri": "test.py" }, "region": { "startLine": 3, "endLine": 4 } } }],
"fixes": [
{
"artifactChanges": [
{
"artifactLocation": { "uri": "test.py" },
"replacements": [{ "deletedRegion": { "startLine": 3, "endLine": 4 }, "insertedContent": { "text": "def f():\n return 1" } }]
}
]
}
]
}
```

GitHub replaces whole lines, so a fix is rendered only when its `deletedRegion` covers exactly the lines of the result's own region, which is what the comment
is anchored to. Following the SARIF specification, in which an absent `endColumn` means the end of the text of `endLine`, both of the usual ways of writing
such a region are accepted:

- `{ "startLine": 3, "endLine": 4 }` covers the text of lines 3 to 4 without the line terminator ending line 4, so `insertedContent.text` is the new text of
those lines and must not end with a newline.

- `{ "startLine": 3, "startColumn": 1, "endLine": 5, "endColumn": 1 }` covers the same lines including the line terminator ending line 4, so
`insertedContent.text` must end with a newline. Exactly one is removed when rendering the suggestion.

Any other `deletedRegion`, such as one replacing a part of a line or lines other than the reported ones, cannot be rendered as a suggestion. Such a fix is
ignored, and the issue is commented on without one.

The text itself is never trimmed beyond the single line terminator described above, so an additional trailing newline is rendered as a trailing empty line.
An empty `insertedContent.text` renders as an empty suggestion, which deletes the lines, in both forms. Replacing the lines with a single empty line is
therefore expressible only in the second form, as a text of exactly one newline — in the first form that same replacement is written as an empty text, which
cannot be told apart from a deletion. A producer restricted to the first form should widen the replacement to include a neighbouring line.

The fixes Bugalint writes out always use the second form, so a fix survives being read back from a SARIF file that Bugalint itself generated.

### Comment Reconciliation

Every comment Bugalint posts starts with an invisible HTML comment carrying the `toolName` and a fingerprint of the rest of the comment: the message, the level,
the rule identifiers and the whole suggestion. On every run Bugalint lists the pull request's review comments, and for each issue it would comment on it looks
for one of its own comments anchored on the same file and the same line range and carrying the same fingerprint:

- A comment that matches an issue is left completely alone. It is neither deleted nor posted again, so pushing to a pull request no longer re-notifies every
reviewer about every issue that did not change, and links to such a comment keep working.

- An issue matching no comment gets a new one.

- A comment of the same `toolName` matching no issue is deleted, which is what makes a fixed issue's comment go away.

The line range is compared to the line GitHub currently reports for the comment rather than the one it was created on. GitHub re-anchors a comment as the pull
request is pushed to, so a comment that merely moved is still recognized. The fingerprint itself covers no line number, so it does not change when lines are
added above the issue. It does cover the suggestion, so an issue whose fix changed is a different comment: the old one is deleted and a new one is posted.

A comment that anyone replied to is never deleted, so a discussion is not orphaned when the issue that started it is fixed. Comments of other tools, of other
`toolName`s and of humans are never touched.

Comments posted by versions of Bugalint older than this feature carry a tag with no fingerprint, so they can never match. On the first run after the upgrade
each of them is deleted and its issue is commented on again in the new format, once per pull request.

At most 50 comments carrying a current issue are kept on a pull request. The comments that are kept count against that limit, so consecutive runs cannot
accumulate 50 comments each: when 48 comments are kept, only 2 new ones are posted. Comments kept only because someone replied to them are not counted, as
they are discussions rather than reports.

### Example With Custom Regex

This is an example of how this action can be used to parse the output of a hypothetical custom linter called `mylinter`, which outputs issues in the following
Expand Down
Loading
Loading