New reporter - github action annotation

[halkeye]

https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-a-warning-message

essentially
::error file={name},line={line},endLine={endLine},title={title}::{message}
ex
echo "::error file=app.js,line=1,col=5,endColumn=7::Missing semicolon"

All of the fields pretty optional, but I think it could do file, line and msg pretty easily.

I tried to figure out enough rust to implement this, but failed.

[epage]

I would love for the Action to better integrate with Github. I had originally intended to not bake in such specific policy but that tools could be built on top of the json output to do so.

The other challenge with supporting github output in the Action is actions don't always have permission to post formatted information to the PR and so you need to make sure that we can gracefully fallback to a human reaadable form. If they've resolve that problem, that'd be great!

[Bhupesh-V]

The other challenge with supporting github output in the Action is actions don't always have permission to post formatted information to the PR and so you need to make sure that we can gracefully fallback to a human reaadable form. If they've resolve that problem, that'd be great!

So one way to approach is to write all the typos detected to a file, specified by the user in action inputs.
then people can maybe configure actions like https://github.com/peter-evans/create-issue-from-file to periodically detect for any typos that come in, wdyt?

[halkeye]

Where are you seeing the permission thing? That page to me seems like it would just work if you echo it. I'm going to setup a test repo soon to confirm though

As for fallback, how hard would it be to support multiple reporters?

[halkeye]

oops cat stepped on my phone and closed the issue

[epage]

Where are you seeing the permission thing?

When an Action is using the regular API for commenting on a PR, last I knew certain permissions were needed or else it'd be just error out. I know this happens when a PR is from a fork and the author doesn't have any permissions for the target repo. I think I've seen it even when the user does (that its just a fork issue)

As for fallback, how hard would it be to support multiple reporters?

In most cases, it doesn't make sense to have multiple reporters (from the CLI). It'd only help when one reporter is visually silent (ie this one case).

[halkeye]

My current PR #430 currently reports

https://github.com/halkeye/typos-test-repo/actions/runs/1826126424

I think its a win, but I want to see if i can't do better.

[halkeye]

oh cool, and inline too

[epage]

Here is an example of the permissions issue. It looks like the problem only applies to inline annotations and that general error/warning messages still get surfaced. Again, this is also using the API rather than using the text output which they might be more liberal with.

[epage]

So with the permissions question resolved and with the above sample you ran, we know we don't need to double-report.

While Github knows the file and line, in the log view it isn't showing it which is disappointing because it might help for us to show more but then it might be redundant in other views.

I'm still mixed on baking github-specific policy directly into the tool vs wrapping it in a Python script that ships with the docker container.

Could we start with the Python script and see how things go to decide if we really need direct integration?

[halkeye]

Could we start with the Python script and see how things go to decide if we really need direct integration?

up to you really. I'd like it cause its simple. My intention is to next write github actions log parser for jenkins, so we can use more tools written for github actions inside of jenkins so I'd like the standard format, but can work around it like you said.

Might not need python btw, I did a POC a little while ago for brakeman using jq

cat brakeman-output.json | jq '.ignored_warnings[] | "::error file=\(.file),line=\(.line)::\(.check_name):\(.message)"'

[halkeye]

typos --format json | jq -s '.[] | "::error file=\(.path),line=\(.line_num),col=\(.byte_offset)::\(.typo) should be \"" + (.corrections // [] | join("\" or \"") + "\"")'

it does work, though it's very very ugly.

I think a companion rust based app would be better than a python app, just to keep the image/binaries small.

My vote is still the reporter, you could have the github action config let people specify reporter, long, short, github

[epage]

My concerns with baking this in are:

• Github becomes part of our compatibility story
• Slipperly slope for people wanting us to support any one off output format.

The json output is intended to be our solution to adapt to various needs.

[epage]

The main annoyance with Rust would be in distributing the binary. If our Dockerfile was less hacky, then adding in an extra binary becomes easier though it will increase the first-run time for an image unless we start publishing.

[halkeye]

Github becomes part of our compatibility story

As someone who works on jenkins, I totally hear you, but at the same time. There's no real standard for a json output either. I only know about this format because rubocop has it natively supported.

If our Dockerfile was less hacky

do you want help with that? Is what you want documented at all?

we start publishing.

dockerhub publishing is pretty straight forward, I don't really know rust, but could make a multi stage docker file that builds via cargo, then copies to a clean docker image, that could easily be done via any ci (though docker hub makes multi arch really easy)

[epage]

do you want help with that? Is what you want documented at all?

No, the most we have is #427. Basically, the image is downloading a pre-built binary from gitub instead of building cleanly off of a base image.

[halkeye]

okay now that #427 has at least been sorta address, is a separate tool the suggested solution?
typos_json_to_github?

[epage]

We still need to update the Dockerfile for the action. We only updated the docker file for the command.

[halkeye]

@epage what do you want me to do next? Do you want me to update the action to use a specific version as the base? or make it compile every time like the ... other docker file?

[epage]

Sorry, the notification for this fell through the cracks.

My basic assumption is we'd make the action's dockerfile layered on top of our main docker file. I assume that means we'd have to start publishing with all that comes along with that. If that becomes more hasle than worth, we can just duplicate the other dockerfile.

[arminrosu]

Hello folks,

Thanks for both typos and the discussion around the github reporter.

I ended up implementing it via the JSON formatter as suggested here. Please note you also need the --raw-output flag to make it work. My script looks like this:

./typos \
  --format json \
  $(git diff origin/main... --name-only --diff-filter=AM --) |
  jq --sort-keys --raw-output '.[] | "::warning file=\(.path),line=\(.line_num),col=\(.byte_offset)::\"\(.typo)\" should be \"" + (.corrections // [] | join("\" or \"") + "\".")' |
  while IFS= read -r line; do
    echo "$line"
  done

It then looks like this:

It would be nice if this could be integrated into the typos github action. That could work by simply returning the JSON if a flag is set.

HTH

[halkeye]

https://github.com/halkeye/typos-json-to-github-annotations I think works but I don't think I published it

https://github.com/halkeye/typos-json-to-checkstyle absolutely works we use it in the Jenkins project

[epage]

@halkeye feel free to put up a PR having typos link out to those, even if ts a more experimental status!

[epage]

@arminrosu I'm looking at integrating your logic into the action but jq wasn't working correctly for me. If I dropped the .[] | and went straight into the string, it then worked. Just wanted to double check if I was missing something before merging it. You can see the final result over at #501.

Also, anyone have concern about always reporting the annotations? Or should it be configurable or limited to just pull requests?

[arminrosu]

If I dropped the .[] | and went straight into the string, it then worked.

What's the output of the typos --format json command? For me it's (which jq is fine with, because it supports json streams):

{"type":"typo","path":"./code.ts","line_num":58,"byte_offset":86,"typo":"FEEBACK","corrections":["FEEDBACK"]}
{"type":"typo","path":"./code.ts","line_num":59,"byte_offset":85,"typo":"FEEBACK","corrections":["FEEDBACK"]}
{"type":"binary_file","path":"./document.pdf"}
{"type":"binary_file","path":"./archive.zip"}
{"type":"binary_file","path":"./image.png"}

However removing .[] | from the JQ query, results in this error:

jq: error (at <stdin>:0): Cannot index array with string "corrections"

Filtering out the binary formats (which are the only typos without corrections), obviously didn't fix the error either.

I'm using typos version 1.8.1, jq 1.6 on Mac OS 12.3.1

---

Also, anyone have concern about always reporting the annotations? Or should it be configurable or limited to just pull requests?

If it's configurable, people can turn it off according to their use case. I like sensible defaults.

[epage]

@arminrosu I'm baffled at the difference but the modified version seems to also be working in github.

[arminrosu]

Really odd, but glad it works. Thanks for the changes!