Skip to content
Merged
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
14 changes: 2 additions & 12 deletions cargo-annotation/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,8 @@ runs:
using: "composite"
steps:
- name: cargo
run: |
CMD="cargo"

if [[ "${{ inputs.cargo-command }}" != "" ]]; then
CMD="$CMD ${{ inputs.cargo-command }}"
fi

if [[ "${{ inputs.with-annotation }}" == "true" ]]; then
CMD="$CMD --message-format=json"
fi

eval "$CMD | ${{ github.action_path }}/annotation.sh"
run: python $GITHUB_ACTION_PATH/annotation.py
shell: bash
env:
INPUT_CARGO_COMMAND: ${{ inputs.cargo-command }}
WITH_ANNOTATION: ${{ inputs.with-annotation }}
87 changes: 87 additions & 0 deletions cargo-annotation/annotation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
#!/usr/bin/env python3

import os
import subprocess
import json
import sys
import shlex
from typing import Any


def emit_annotation_for_clippy(results: list[dict[str, Any]], with_annotation: bool):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def emit_annotation_for_clippy(results: list[dict[str, Any]], with_annotation: bool):
def parse_cargo_output(results: list[dict[str, Any]], emit_annotation: bool) -> bool:

total_count = 0
limit = 10

severenty_map = {
"help": "notice",
"note": "notice",
"warning": "warning",
"error": "error",
}
for item in results:
if total_count >= limit:
break

message = item.get("message")
if not message:
continue

spans = message.get("spans") or []
primary_span = next((span for span in spans if span.get("is_primary")), None)
if not primary_span:
continue

level = severenty_map.get(message.get("level"), "error")
title = message.get("message", "")
rendered_message = message.get("rendered", "")

file_name = (primary_span["file_name"],)
line_start = (primary_span["line_start"],)
line_end = (primary_span["line_end"],)
column_start = (primary_span["column_start"],)
column_end = (primary_span["column_end"],)

line_info = f"line={line_start},endLine={line_end},title={title}"

column_info = ""
if (
line_start == line_end
and column_end is not None
and column_start is not None
):
column_info = f"col={column_start},endColumn={column_end},"
if with_annotation:
print(
f"::{level} file={file_name},{column_info}{line_info}::{rendered_message}"
)
total_count += 1

return 1 if total_count > 0 else 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return 1 if total_count > 0 else 0
return total_count > 0



def main():
cmd = ["cargo"]

cargo_command = os.getenv("INPUT_CARGO_COMMAND", "").strip()
with_annotation = (
os.getenv("INPUT_WITH_ANNOTATION", "true").strip().lower() == "true"
)

if cargo_command:
args = shlex.split(cargo_command)
cmd.extend(args)

if "--message-format=json" not in cmd:
cmd.append("--message-format=json")

proc = subprocess.run(cmd, capture_output=with_annotation)
Copy link
Member

@mukilan mukilan Aug 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the idea is to parse the output of cargo to determine the exit status, then shouldn't we always capture the output, regardless of with_annotation? With the current logic, when with_annotation=False, we will not fail the build even when clippy has some output.

clippy_results = [
json.loads(line) for line in proc.stdout.splitlines() if line.strip()
]

result = emit_annotation_for_clippy(clippy_results, with_annotation)
sys.exit(result)
Comment on lines +82 to +83
Copy link
Member

@mrobinson mrobinson Aug 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can simplify this a bit, by only parsing annotations when with_annotation is true and then returning the original return code:

Suggested change
result = emit_annotation_for_clippy(clippy_results, with_annotation)
sys.exit(result)
if with_annotation:
emit_annotation_for_clippy(clippy_results)
sys.exit(proc.returncode)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this as @mukilan said here. So unless we agreed on using explictly -D warnings internally here, I can go with this solution, what do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, clippy will not return a failure code unless -D warning is passed, but then that will cause clippy to error just after the first warning. It is more useful in the with_annotation=true case to not rely on the exit code (we can show up to 10 warnings), so to me it makes sense to use that same logic even when GH annotation is turned off.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Thank you!



if __name__ == "__main__":
main()
21 changes: 0 additions & 21 deletions cargo-annotation/annotation.sh

This file was deleted.