Skip to content

.golangci.yaml doesn't seem to be picked up on Windows #583

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
3 tasks done
itaranto opened this issue Oct 15, 2022 · 6 comments
Closed
3 tasks done

.golangci.yaml doesn't seem to be picked up on Windows #583

itaranto opened this issue Oct 15, 2022 · 6 comments
Labels
question Further information is requested

Comments

@itaranto
Copy link

itaranto commented Oct 15, 2022

Welcome

  • Yes, I understand that the GitHub action repository is not the repository of golangci-lint itself.
  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've included all information below (version, config, etc).

Description of the problem

The following job fails only on Windows due to already ignored linter errors.

Version of golangci-lint

v1.49.0

Version of the GitHub Action

v3.2.0

Workflow file

name: CI

on: [push, pull_request]

jobs:
  test:
    name: Test
    runs-on: ${{ matrix.os }}
    timeout-minutes: 5
    strategy:
      matrix:
        os: [ubuntu-latest, macos-latest, windows-latest]
    steps:
      - name: Checkout code
        uses: actions/checkout@v3
      - name: Setup Go
        uses: actions/setup-go@v3
        with:
          go-version: 1.x
      - name: Lint
        uses: golangci/golangci-lint-action@v3
        with:
          version: v1.49
      - name: Test
        run: make test

Go version

v1.19.2

Code example or link to a public repository

https://github.com/itaranto/efm-langserver/blob/replace-golint-with-revive/.github/workflows/ci.yml
mattn/efm-langserver#231

@ldez ldez added the question Further information is requested label Oct 17, 2022
@itaranto
Copy link
Author

itaranto commented Oct 21, 2022

Any hint on how can I debug this?

@bombsimon
Copy link
Member

If you want to see what config is picked up, use the -v flag, it will show where it looks for files on the first line:

$ /tmp/golangci-lint.exe run -v
level=info msg="[config_reader] Config search paths: [./ ...]"
level=info msg="[config_reader] Used config file .golangci.yaml"

However, I don't think your problem is that it doesn't pick up the config. In your config file you specify local prefix github.com/mattn/efm-langserver. That's also what is used in your linked pipeline:

run golangci-lint
  Running [D:\a\_temp\80fc8004-77d1-4be1-8caa-521a7cd510c4\golangci-lint-1.49.0-windows-amd64\golangci-lint run --out-format=github-actions] in [] ...
  Error: File is not `goimports`-ed with -local github.com/mattn/efm-langserver (goimports)
  Error: File is not `goimports`-ed with -local github.com/mattn/efm-langserver (goimports)
  Error: File is not `goimports`-ed with -local github.com/mattn/efm-langserver (goimports)
  Warning: var-naming: don't use leading k in Go names; var kPrev should be prev (revive)
  
  Error: issues found
  Ran golangci-lint in [29](https://github.com/itaranto/efm-langserver/actions/runs/3285346400/jobs/5412344592#step:4:31)543ms

If it wasn't picked up properly I don't see how it would add -local github.com/mattn/efm-langserver.

I tried cloning your repo and running golangci-lint locally on Windows and I get the same issue but it sure does pick up the config:

~/git/efm-langserver (replace-golint-with-revive)
$ /tmp/golangci-lint.exe run -v --print-issued-lines=false
level=info msg="[config_reader] Config search paths: [./ C:\\Users\\simon\\git\\efm-langserver C:\\Users\\simon\\git C:\\Users\\simon C:\\Users C:\\]"
level=info msg="[config_reader] Used config file .golangci.yaml"
[...]
main.go:1: File is not `goimports`-ed with -local github.com/mattn/efm-langserver (goimports)
langserver\config.go:1: File is not `goimports`-ed with -local github.com/mattn/efm-langserver (goimports)
langserver\diff.go:1: File is not `goimports`-ed with -local github.com/mattn/efm-langserver (goimports)

If I manually run goimports on a file that fails and write to it I get a diff:

$ goimports -local github.com/mattn/efm-langserver -w main.go
$ git status --porcelain | sed s/^...//
main.go

However, I'm not very familiar with Windows and how git/Go handles line endings (I installed Go on Windows first time for this). I don't really see any change here and my output just shows:

$ git diff main.go
warning: LF will be replaced by CRLF in main.go.
The file will have its original line endings in your working directory

I tried to just use the --fix flag for golangci-lint, got the diff on a bunch of files, but then doing git add just removes them all as modified from my stage (without needing to commit). Running golangci-lint after this gives no error, although I assume this time it would be a problem with unix line endings instead.

@itaranto
Copy link
Author

itaranto commented Oct 25, 2022

f you want to see what config is picked up, use the -v flag, it will show where it looks for files on the first line:

Yes, I completely forgot about the --verbose flag.

Running golangci-lint after this gives no error, although I assume this time it would be a problem with unix line endings instead.

I remember adding *.go text eol=lf to the gitattributes file like the golangci-lint-action's documentation suggests but the pipeline failed anyway.

Just to check, I added it again and I notice it throws less errors with this setting. What I mean is that before adding *.go text eol=lf I see:

  Error: File is not `goimports`-ed with -local github.com/mattn/efm-langserver (goimports)
  Error: File is not `goimports`-ed with -local github.com/mattn/efm-langserver (goimports)
  Error: File is not `goimports`-ed with -local github.com/mattn/efm-langserver (goimports)
  Warning: var-naming: don't use leading k in Go names; var kPrev should be prev (revive)

And after adding said attribute I only see:

Error: File is not `goimports`-ed with -local github.com/mattn/efm-langserver (goimports)
Warning: var-naming: don't use leading k in Go names; var kPrev should be prev (revive)

@itaranto
Copy link
Author

itaranto commented Oct 25, 2022

OK, I've managed to fix all the issues, I've found some interesting things...

  1. *.go text eol=lf is necessary for gofmt/goimports because it doesn't handle CLRF terminators, see this issue.

  2. That extra goimports error I saw was very weird, so I decided to run gofmt instead. gofmt started reporting a new error on my machine (Linux), this error was about a file which wasn't "simplified" (formatted with the -s flag) which is enabled by default in golangci-lint. So then I replaced back gofmt with goimports and the 3rd issue was gone, this is very strange since goimports doesn't have the -s flag, and also it was only failing on Windows.

  3. The revive error magically disappeared when I moved from golangci-lint v1.49.0 to v1.50.1 (I previously didn't use v1.50.0 because it had issues).

@itaranto
Copy link
Author

Should I close this? Basically the thing that worries me is point number 2.

@bombsimon
Copy link
Member

Since I think the original issue here is resolved it would be great to close this and if/when you have a repro for 2 open a new issue. I don't think I've seen this exact issue before but I have seen false positives for goimports, example: golangci/golangci-lint#1462

This should not matter but worth noting is that golangci-lint maintain hard forks of gofmt and goimports: https://github.com/golangci/gofmt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants