golangci-lint-action icon indicating copy to clipboard operation
golangci-lint-action copied to clipboard

github action log output doesn't print the affected file name & line

Open dprotaso opened this issue 3 years ago • 39 comments

Please include the following information:

Version of golangci-lint
      - id: golangci_configuration
        uses: andstor/file-existence-action@v1
        with:
          files: .golangci.yaml
      - name: Go Lint
        if: steps.golangci_configuration.outputs.files_exists == 'true'
        uses: golangci/golangci-lint-action@v2
        with:
          version: v1.30
Config file
run:
  timeout: 5m

  build-tags:
    - e2e

  skip-dirs:
    - pkg/client

linters:
  enable:
    - asciicheck
    - golint
    - gosec
    - prealloc
    - stylecheck
    - unconvert
    - unparam
  disable:
    - errcheck

issues:
  exclude-rules:
    - path: test # Excludes /test, *_test.go etc.
      linters:
        - gosec
        - unparam
Go environment
$ go version && go env
1.15
Verbose output of running

https://github.com/knative/networking/pull/233/checks?check_run_id=1275974348

I had to view the 'files changed' page to see the lints

dprotaso avatar Oct 19 '20 15:10 dprotaso

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

boring-cyborg[bot] avatar Oct 19 '20 15:10 boring-cyborg[bot]

The above run seems not available now. I have created one dummy run to check here. The output is as per expectation.

run golangci-lint
  Running [/home/runner/golangci-lint-1.31.0-linux-amd64/golangci-lint run --out-format=github-actions] in [] ...
  Error: File is not `gofmt`-ed with `-s` (gofmt)
  Error: File is not `goimports`-ed with -local github.com/golangci/golangci-lint (goimports)
  
  Error: issues found
  Ran golangci-lint in 58975ms

Underlying golangci-lint-action is using github annotations for more user friendly, so where the error is shown (e.g. Files Changes, etc) is up to github annotation, but not golangci-lint. Related to https://github.com/golangci/golangci-lint-action/issues/5.

I am closing this issue. Feel free to re-open if there is anything else that I could have missed.

sayboras avatar Oct 24 '20 23:10 sayboras

so where the error is shown (e.g. Files Changes, etc) is up to github annotation, but not golangci-lint.

I guess to clarify I was just hoping the file names would be displayed inline with the run golangci-lint steps logs in addition to the github annotations

ie. I marked the location in the logs with /// - when I run the action locally I see them appear

run golangci-lint
  Running [/home/runner/golangci-lint-1.31.0-linux-amd64/golangci-lint run --out-format=github-actions] in [] ...  
  /// output file name and line
  Error: File is not `gofmt`-ed with `-s` (gofmt)
  /// output file name and line
  Error: File is not `goimports`-ed with -local github.com/golangci/golangci-lint (goimports)
  
  Error: issues found
  Ran golangci-lint in 58975ms

dprotaso avatar Oct 27 '20 02:10 dprotaso

@dprotaso Thanks for your clarification. As golangci-lint is showing file name + line number, I transfered this issue to golangci-lint-action repo.

$ golangci-lint run ./...        
pkg/golinters/unparam.go:7: File is not `gofmt`-ed with `-s` (gofmt)

pkg/golinters/unparam.go:14: File is not `goimports`-ed with -local github.com/golangci/golangci-lint (goimports)

sayboras avatar Oct 27 '20 03:10 sayboras

Can somebody confirm that it is on our side and it is technically possible to adjust the way how github annotations are represented in logs?

AFAIK annotation format already contains file name + line number, but github is not showing this info in logs for some reason.

I've tried to investigate this problem, but I can't find complete documentation for github annotations and how to manipulate them, so I'll be grateful for any help.

ernado avatar Nov 02 '20 15:11 ernado

Because file name and line number is written in :: syntax, github is not show this information in github actions logs.

I confirmed that.

on:
  push
jobs:
  test:
    runs-on: ubuntu-20.04
    steps:
      # `golangci-lint --output-format=github-actions` error message line format
      - run: echo '::error file=path/to/source.go,line=367,col=16::error message (lint tool name)'
      # improve format to output line number and file name to github actions log
      - run: echo '::error file=path/to/source.go,line=367,col=16::path/to/source.go:367:16:error message (lint tool name)'

github-actions-annotation

github-actions-log

link

  • https://github.com/actions/toolkit/blob/main/docs/commands.md

takehito avatar Dec 03 '20 22:12 takehito

Great, so fix should be trivial?

Upd: oh, seems like we can only fix that via duplicating link to file

ernado avatar Dec 03 '20 22:12 ernado

yes. we can also move file line and col variables out of :: syntax and add probrem matcher file.

https://github.com/actions/toolkit/blob/main/docs/problem-matchers.md

takehito avatar Dec 03 '20 23:12 takehito

Is there any news about this? i am currently trying to implement this in a github repo and not seeing which file and line an error belongs to in a direct way (having to check the full text based logs) is pretty annoying.

ss89 avatar Feb 07 '21 16:02 ss89

Kindly ping. I have this issue too - annotation links are not working.

r10r avatar Feb 15 '21 16:02 r10r

Sure would be nice to see the affected filenames in the output on the GitHub website. It's not even visible when I click "View raw logs".

But I can repro the results by running golangci locally:

docker run --rm -v $(pwd):/app -w /app golangci/golangci-lint:v1.37.1 golangci-lint run -v

... and see the filenames that way. Not great, but that unblocks me 👍

aimichal avatar Feb 22 '21 20:02 aimichal

I changed the printer for github to do a format similar to eslint compact.

$ golangci-lint run --out-format=github-actions ./...
::error file=sample.go,line=12,col=10::Error return value of `retError` is not checked (errcheck) ([	retError()])
::error file=sample.go,line=13,col=11::Error return value of `retError2` is not checked (errcheck) ([	retError2()])
::error file=sample.go,line=16,col=9::Error return value of `h.Write` is not checked (errcheck) ([	h.Write([]byte(data))])

$ golangci-lint run --out-format=github-actions ./...
sample.go: line 12, col 10, error - Error return value of `retError` is not checked (errcheck)
sample.go: line 13, col 11, error - Error return value of `retError2` is not checked (errcheck)
sample.go: line 16, col 9, error - Error return value of `h.Write` is not checked (errcheck)

Now, once I can test that the output format will work in general instead of just on the sample.go file, we can use a problem matcher file that's almost the same as the example here? https://github.com/actions/toolkit/blob/main/docs/problem-matchers.md#single-line-matchers

pkuca avatar Feb 24 '21 02:02 pkuca

I believe that showing file/line info in build log is absolutely a must. In cases when already existing project adds golangci-lint-action (microsoft/hcsshim#975 as an example), none of reported errors are in diff. Instead, they are in files that already exist. The same will happen for repos that update to a newer version of golangci-lint.

slonopotamus avatar Mar 17 '21 19:03 slonopotamus

As well as if one to run the linter as a workflow_dispatch against a certain branch (not as a pull request run), then annotations are useless, as there are no files to browse to check them.

So having line numbers in the action run log is very much awaited

hellt avatar Mar 19 '21 21:03 hellt

As well as if one to run the linter as a workflow_dispatch against a certain branch (not as a pull request run), then annotations are useless, as there are no files to browse to check them.

So having line numbers in the action run log is very much awaited

There are a few things to unpack here: The action specifically states that it is designed to be run in a push or pull request context. Running in other contexts, where "there are no files to browse" is not an officially supported use-case as it stands (however, this check is not performed, so you can use this action effectively anywhere).

That being said: While not "officially" supported (in-fact, the code attempts to prevent it, but seems to be bugged at the moment), I believe you can get the results you are looking for by setting args: --out-format=tab, which will result in output such as below:

  utils/ui_utils.go:137:19                                     gomnd       mnd: Magic number: 20, in <argument> detected
  utils/ui_utils.go:147                                        lll         line is 124 characters
  utils/ui_utils.go:149                                        lll         line is 106 characters
  utils/ui_utils.go:152                                        lll         line is 106 characters

If you still want the annotation feature on push and pull request contexts, you would need a .yml configured to trigger on push/pull and one triggered for for any other workflows (with the output set to tab).

thebeline avatar Apr 01 '21 13:04 thebeline

The action specifically states that it is designed to be run in a push or pull request context.

This doesn't guarantee that files with violations were changed by that push or pull request. Trivial counter-example: initial pull request that adds golangci-lint-action to the repo.

slonopotamus avatar Apr 01 '21 14:04 slonopotamus

The action specifically states that it is designed to be run in a push or pull request context.

This doesn't guarantee that files with violations were changed by that push or pull request. Trivial counter-example: initial pull request that adds golangci-lint-action to the repo.

I can confirm that in the event of an initial pull request that adds this to the repo, if configured with only-new-issues: true, it will only run on a patch of the modified files in the PR.

All of that being said, you are not wrong, the Action as it is currently implemented can be run in any context, however the output is only tailored for two specific contexts. I am working on a PR to resolve this.

That being said, while my above suggestion of tab would likely resolve your issue in your specific use-case, I apologize for just submitting a PR that fixes the bug that allowed setting out-format.

For what it is worth, I am also going to be submitting a PR that will then address your use-case (as well as some others), so please keep an eye out and support it if it behaves as you desire.

thebeline avatar Apr 01 '21 14:04 thebeline

it will only run on a patch of the modified files in the PR.

I should clarify what I am confirming here:

  • If not configured with only-new-issues: true, it will, at all times, report errors in all files it is configured to lint.
  • If is configured with only-new-issues: true, I can confirm that it will not report issues with lines not also modified in the whole of the Pull Request.
  • If the action is triggered in any other situation other than a Pull Request, it will report issues in ALL FILES, regardless of the setting of only-new-issues

That final situation seems like what you are referring to.

Although the code explicitly reflects this is desired behavior, I would kind of agree that this seems like a bug; as I would expect that patch data would also be available for the push context as well (the files changed in the Push), and the linter would be run on that data.

Frankly, while researching that last bit I was a little surprised, as well; as I had expected it to run against a patch in the push context as well. So, color me surprised.

thebeline avatar Apr 01 '21 14:04 thebeline

It is also worth mentioning that some checks, such as gofmt, apply to entire files. Those are currently not shown in the "Files changed" tab, making errors particularly tedious to track.

image image

antoineco avatar Apr 16 '21 11:04 antoineco

Any progress? This issue has been bothering me lately, it kept printing lint issues without file names and line numbers on Windows, which drove me crazy cuz I can't locate those problematic files and fix those.

https://github.com/panjf2000/gnet/runs/2621134533?check_suite_focus=true

panjf2000 avatar May 19 '21 14:05 panjf2000

Friendly bump.

wemgl avatar Jul 12 '21 14:07 wemgl

Friendly bump...

gaby avatar Oct 20 '21 13:10 gaby

Friendly bump

lcd1232 avatar Nov 11 '21 08:11 lcd1232

Workaround

    - uses: golangci/golangci-lint-action@v2
      with:
        args: "--out-${NO_FUTURE}format colored-line-number"

Explainer

I tried to override output format with

    - uses: golangci/golangci-lint-action@v2
      with:
        args: --out-format colored-line-number

Spplying multiple --out-format arguments to the linter works locally, unfortunately the github action has future proofing protection in place that says

Failed to run: Error: please, don't change out-format for golangci-lint: it can be broken in a future, Error: please, don't change out-format for golangci-lint: it can be broken in a future
    at /home/runner/work/_actions/golangci/golangci-lint-action/v2/dist/run/index.js:6831:19
    at Generator.next (<anonymous>)
    at /home/runner/work/_actions/golangci/golangci-lint-action/v2/dist/run/index.js:6713:71
    at new Promise (<anonymous>)
    at module.exports.__awaiter (/home/runner/work/_actions/golangci/golangci-lint-action/v2/dist/run/index.js:6709:12)
    at runLint (/home/runner/work/_actions/golangci/golangci-lint-action/v2/dist/run/index.js:6816:12)
    at /home/runner/work/_actions/golangci/golangci-lint-action/v2/dist/run/index.js:6885:57
    at Object.<anonymous> (/home/runner/work/_actions/golangci/golangci-lint-action/v2/dist/run/index.js:42546:28)
    at Generator.next (<anonymous>)
    at /home/runner/work/_actions/golangci/golangci-lint-action/v2/dist/run/index.js:42349:71

So in order to make the output useful in present you have to fool the future-proof protection like:

    - uses: golangci/golangci-lint-action@v2
      with:
        args: "--out-${NO_FUTURE}format colored-line-number"

isimluk avatar Nov 28 '21 14:11 isimluk

the action creates the comments, open the pull request changes page and you will see all issues. also the action does not allow overriding the format. If you want to see the list of issues in the jobs log, I would recommend you to add one more step:

    - name: errors
        run: golangci-lint run
        if: ${{ failure() }}

don't worry about the running time, the second run will reuse the cache. In my case the action takes around 10 minutes, but the additional step takes 10 seconds

SVilgelm avatar Nov 28 '21 18:11 SVilgelm

@thebeline @SVilgelm sorry if this is something obvious that I've missed but in our case the annotations are actually on the wrong areas ... what is the specific technical reason that this action can't just print the lines to the output? Developers are used to looking at the output of failed actions and they know how to parse the output of golangci-lint. If they don't want to take the extra click to go look at the GitHub annotations, why should they have to? What is stopping us from both providing the annotations and the simple log output?

stevekuznetsov avatar Mar 07 '22 16:03 stevekuznetsov

@isimluk Thank you for the fix, using

    - uses: golangci/golangci-lint-action@v2
      with:
        args: "--out-${NO_FUTURE}format colored-line-number"

I can now see the line and file names 👍🏻

gaby avatar Apr 01 '22 12:04 gaby

If the solution is simply to pass the --out-format twice (which is technically what happens when the workaround described in https://github.com/golangci/golangci-lint-action/issues/119#issuecomment-981090648 is implemented), what prevents this format from becoming the default? Everybody gets the best of both worlds, right?

image

image

antoineco avatar Apr 12 '22 16:04 antoineco

The workaround args: "--out-${NO_FUTURE}format colored-line-number" fails when run on windows:

  Running [D:\a\_temp\99ad0a77-8d39-4bd4-a4a4-65b1acdcc359\golangci-lint-1.46.2-windows-amd64\golangci-lint run --out-format=github-actions --out-${NO_FUTURE}format colored-line-number] in [] ...
  level=error msg="Can't get config for command line: can't parse args: unknown flag: --out-${NO_FUTURE}format"
  
  Error: golangci-lint exit with code 3
  Ran golangci-lint in 17314ms

stevenh avatar Jun 12 '22 15:06 stevenh

@stevenh you might want to use a variable syntax that PowerShell can interpret instead: https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_environment_variables?view=powershell-7.2#using-the-variable-syntax

--out-${Env:NO_FUTURE}format

antoineco avatar Jun 12 '22 16:06 antoineco