Extremely different results local vs GH Action

[bradleygore]

Thanks for this great GH Action! I'm excited to start using it!

Description

However, I'm running into an issue in that I get wildly different lint output from GH Action than I do local, on exact same version of golangci-lint.

golangci-lint version

Locally: golangci-lint has version 1.47.2 built from 61673b34 on 2022-07-21T10:53:45Z

GH Action Logs:

2022-07-28T19:52:26.1292087Z Finding needed golangci-lint version...
2022-07-28T19:52:26.1305224Z Installing golangci-lint v1.47.2...
2022-07-28T19:52:26.1306144Z Downloading https://github.com/golangci/golangci-lint/releases/download/v1.47.2/golangci-lint-1.47.2-linux-amd64.tar.gz ...
2022-07-28T19:52:26.5816036Z [command]/usr/bin/tar xz --overwrite --warning=no-unknown-keyword --overwrite -C /home/runner -f /home/runner/work/_temp/ac4d2d69-ffe7-43e5-99ab-875f201d5d97
2022-07-28T19:52:26.8164745Z Installed golangci-lint into /home/runner/golangci-lint-1.47.2-linux-amd64/golangci-lint in 686ms

Repository Structure

/
  .github/workflows/ ...
  cmd/ ...
  internal/ ...
  .golangci.yml
  go.mod
  go.sum
  README.md

As you can see, .golangci.yml file is at the root of the directory.

Inside of cmd dir I have Makefile which is where I run the linter from like golangci-lint -v ../..., and I can see in the output that it picks up my config file due to verbose output in both local run and in GH Action run:

Local: INFO [config_reader] Used config file ../.golangci.yml GH Action: 2022-07-28T19:53:22.1838177Z level=info msg="[config_reader] Used config file .golangci.yml"

I'm getting many lint issues in the GH Action that I do not get locally, so I'm curious what could be the culprit.

Locally, I'm running on macOS Monterey v 12.4 and also run the same in a Jenkins environment using Linux (trying to get away from Jenkins to just use GH Actions). I get the same output on both of those, which is no issues:

INFO Execution took 4.172623868s                  
Linter complete

But in GH Action I get this:

2022-07-28T19:53:22.2013321Z level=info msg="Execution took 47.16372068s"
2022-07-28T19:53:22.2013514Z 
2022-07-28T19:53:22.2014177Z ##[error]issues found
2022-07-28T19:53:22.2015031Z Ran golangci-lint in 48678ms

---

Config Files

Workflow Yaml

name: golangci-lint
on:
  push:
    branches: [master]
  pull_request:
    branches: [master]
permissions:
  contents: read
  # Optional: allow read access to pull request. Use with `only-new-issues` option.
  # pull-requests: read
jobs:
  golangci:
    name: lint
    runs-on: ubuntu-latest
    steps:
      - uses: actions/setup-go@v3
        with:
          go-version: 1.18
      - uses: actions/checkout@v3
      - name: golangci-lint
        uses: golangci/golangci-lint-action@v3
        with:
          # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
          version: v1.47.2

          # Optional: working directory, useful for monorepos
          # working-directory: somedir

          # Optional: golangci-lint command line arguments.
          args: -v

          # Optional: show only new issues if it's a pull request. The default value is `false`.
          # only-new-issues: true

          # Optional: if set to true then the all caching functionality will be complete disabled,
          #           takes precedence over all other caching options.
          # skip-cache: true

          # Optional: if set to true then the action don't cache or restore ~/go/pkg.
          # skip-pkg-cache: true

          # Optional: if set to true then the action don't cache or restore ~/.cache/go-build.
          # skip-build-cache: true

.golangci.yml

# This file contains all available configuration options
# with their default values.

# options for analysis running
run:
  go: '1.18'
  # default concurrency is a available CPU number
  concurrency: 4

  # timeout for analysis, e.g. 30s, 5m, default is 1m
  timeout: 10m

  # exit code when at least one issue was found, default is 1
  issues-exit-code: 1

  # include test files or not, default is true
  tests: true

  # list of build tags, all linters use it. Default is empty list.
  build-tags:
    # - mytag

  # which dirs to skip: they won't be analyzed;
  # can use regexp here: generated.*, regexp is applied on full path;
  # default value is empty list, but next dirs are always skipped independently
  # from this option's value:
  #     vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
  skip-dirs:
    - db/tests
    - internal/testutils
    - internal/builders/check/testchecks
    - /mocks/

  # which files to skip: they will be analyzed, but issues from them
  # won't be reported. Default value is empty list, but there is
  # no need to include all autogenerated files, we confidently recognize
  # autogenerated files. If it's not please let us know.
  skip-files:
    # - ".*\\.my\\.go$"
    # - lib/bad.go

  # by default isn't set. If set we pass it to "go list -mod={option}". From "go help modules":
  # If invoked with -mod=readonly, the go command is disallowed from the implicit
  # automatic updating of go.mod described above. Instead, it fails when any changes
  # to go.mod are needed. This setting is most useful to check that go.mod does
  # not need updates, such as in a continuous integration and testing system.
  # If invoked with -mod=vendor, the go command assumes that the vendor
  # directory holds the correct copies of dependencies and ignores
  # the dependency descriptions in go.mod.
  modules-download-mode: readonly #|release|vendor

# output configuration options
output:
  # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number"
  format: colored-line-number

  # print lines of code with issue, default is true
  print-issued-lines: true

  # print linter name in the end of issue text, default is true
  print-linter-name: true

  # Make issues output unique by line.
  # Default: true
  uniq-by-line: false

  # Sort results by: filepath, line and column.
  sort-results: true

# all available settings of specific linters
linters-settings:
  depguard:
    list-type: blacklist
    include-go-root: false
    packages:
      # - github.com/davecgh/go-spew/spew
  dupl:
    # tokens count to trigger issue, 150 by default
    threshold: 100
  errcheck:
    # report about not checking of errors in type assetions: `a := b.(MyStruct)`;
    # default is false: such cases aren't reported by default.
    check-type-assertions: false

    # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
    # default is false: such cases aren't reported by default.
    check-blank: false

    # path to a file containing a list of functions to exclude from checking
    # see https://github.com/kisielk/errcheck#excluding-functions for details
    exclude: ./.errcheck_excludes.txt
  gocritic:
    # Which checks should be enabled; can't be combined with 'disabled-checks';
    # See https://go-critic.github.io/overview#checks-overview
    # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run`
    # By default list of stable checks is used.
    enabled-checks:

    # Which checks should be disabled; can't be combined with 'enabled-checks'; default is empty
    disabled-checks:
      - regexpMust

    # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint` run to see all tags and checks.
    # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags".
    enabled-tags:
      - performance

    settings: # settings passed to gocritic
      captLocal: # must be valid enabled check name
        paramsOnly: true
      rangeValCopy:
        sizeThreshold: 32
  gofmt:
    # simplify code: gofmt with `-s` option, true by default
    simplify: true
  goimports:
    # put imports beginning with prefix after 3rd-party packages;
    # it's a comma-separated list of prefixes
    local-prefixes: pos-api
  gocognit:
  gocyclo:
    # minimal code complexity to report, 30 by default (but we recommend 10-20)
    min-complexity: 15
  gomnd:
    settings:
      mnd:
        ignored-numbers: 0,1,2,3,4,5,10,24,32,64,100,1_000
  govet:
    # report about shadowed variables
    check-shadowing: true

    # settings per analyzer
    settings:
      printf: # analyzer name, run `go tool vet help` to see all analyzers
        funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf

    disable-all: true
    enable:
      - assign
      - atomic
      - bools
      - buildtag
      - composites
      - copylocks
      - errorsas
      - nilfunc
      - printf
      - stdmethods
      - structtag
      - tests
      - unreachable
      - unusedresult
  lll:
    # max line length, lines longer will be reported. Default is 120.
    # '\t' is counted as 1 character by default, and can be changed with the tab-width option
    line-length: 130
    # tab width in spaces. Default to 1.
    tab-width: 1
  maligned:
    # print struct with more effective memory layout or not, false by default
    suggest-new: true
  misspell:
    # Correct spellings using locale preferences for US or UK.
    # Default is to use a neutral variety of English.
    # Setting locale to US will correct the British spelling of 'colour' to 'color'.
    locale: US
    ignore-words:
      - cheque
  nakedret:
    # make an issue if func has more lines of code than this setting and it has naked returns; default is 30
    max-func-lines: 30
  prealloc:
    # XXX: we don't recommend using this linter before doing performance profiling.
    # For most programs usage of prealloc will be a premature optimization.

    # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them.
    # True by default.
    simple: true
    range-loops: true # Report preallocation suggestions on range loops, true by default
    for-loops: false # Report preallocation suggestions on for loops, false by default
  revive:
    # see https://github.com/mgechev/revive#available-rules for details.
    ignore-generated-header: true
    severity: warning
    errorCode: 0
    warningCode: 0
    rules:
      - name: add-constant
        severity: warning
        arguments:
          - maxLitCount: "5"
            # literal strings we commonly use in error context, path separators, etc..
            allowStrs: '"",",","err","error","required","id","/","0","*"'
            # numbers commonly used as literals
            allowInts: "0,1,2,3,4,5,10,24,32,64,100,1_000"
            allowFloats: "0.0,0.,1.0,1.,2.0,2."
      - name: blank-imports
        severity: warning
      - name: context-as-argument
      - name: context-keys-type
      - name: duplicated-imports
      - name: error-naming
      # - name: error-strings #BDG: This was enabled for months, but it suddenly started working on 3/2/2022.. come to find out we have TONS of error messages starting with capital... disabling for now(ever?)
      - name: error-return
      - name: exported
      - name: if-return
      # - name: get-return // BDG: We have a lot of API endpoint handlers named like getFoos but write to response vs return... maybe later can figure that out
      - name: identical-branches
      - name: indent-error-flow
      - name: import-shadowing
      - name: package-comments
      - name: range-val-in-closure
      - name: range-val-address
      - name: redefines-builtin-id
      - name: struct-tag
      - name: unconditional-recursion
      - name: unnecessary-stmt
      - name: unreachable-code
      - name: unused-parameter
      - name: unused-receiver
  tagliatelle:
    # check the struck tag name case
    case:
      # use the struct field name to check the name of the struct tag
      use-field-name: false # if we set this to true, then if your go struct field is DNREmailAddress it will error if json not "dnrEmailAddress" - don't think we want that strict..
      rules:
        # any struct tag type can be used.
        # support string case: `camel`, `pascal`, `kebab`, `snake`, `goCamel`, `goPascal`, `goKebab`, `goSnake`, `upper`, `lower`
        json: camel
        yaml: camel
        xml: camel
        db: snake
  unused:
    # treat code as a program (not a library) and report unused exported identifiers; default is false.
    # XXX: if you enable this setting, unused will report a lot of false-positives in text editors:
    # if it's called for subdir of a project it can't find funcs usages. All text editor integrations
    # with golangci-lint call it on a directory with the changed file.
    check-exported: false
linters:
  disable-all: true
  enable:
    - deadcode
    - durationcheck
    - errcheck
    # - forcetypeassert //BDG: This one is pretty strict and we have a lot of what I consider "safe" unchecked assertions.. not sure yet..
    - gocognit
    - gocyclo
    - gofmt
    - goimports
    - gomnd
    - gosec
    - gosimple
    - govet
    - ifshort
    - ineffassign
    - lll
    - makezero
    - misspell
    - predeclared
    - promlinter
    - sqlclosecheck
    - staticcheck
    - structcheck
    - tagliatelle
    - typecheck
    - unused
    - varcheck
    - wastedassign
  fast: false

issues:
  # List of regexps of issue texts to exclude, empty list by default.
  # But independently from this option we use default exclude patterns,
  # it can be disabled by `exclude-use-default: false`. To list all
  # excluded by default patterns execute `golangci-lint run --help`
  exclude:
    # https://github.com/golangci/golangci-lint/issues/138
    - "File is not `goimports`-ed with -local pos-api"
    # We are using swaggo/swag CLI for generating API docs, and it doesn't auto-ignore unexported fields unless you add `json:"-"` to them
    - "has json tag but is not exported"
    # Bridge permissions at this point all have an activity of "manage", but made the function take it as param as that will likely not stay the case
    - "always receives `ActivityManage`"

  # Excluding configuration per-path, per-linter, per-text and per-source
  exclude-rules:
    # Exclude some linters from running on tests files.
    - path: _test\.go
      linters:
        - dupl
        - errcheck
        - gocyclo
        - gosec
        - lll
        - misspell
        - promlinter # no need for prometheus-related checks in test files...
        - revive
        - tagliatelle

    # Exclude some staticcheck messages
    - linters:
        - staticcheck
      # https://staticcheck.io/docs/checks
      text: "SA9003:" # Empty body in an if or else branch

    # Exclude lll issues for long lines with go:generate
    - linters:
        - lll
      source: "^//go:generate "

    # Exclude lll issues for long lines with URLs
    - linters:
      - lll
      source: "http?s://"

    # Exclude lll issues for long lines with swagger comments
    - linters:
      - lll
      source: "@(Description|Param|Success)"
    # Exclude lll issues for long lines for GlobalRepository repoFactory
    - linters:
      - lll
      source: "return r.repoFactory"

    # Exclude lll issues for long lines for struct fields with tags
    - linters:
      - lll
      source: "`(json|yaml|validate|db|swaggertype|swaggerignore):"

    # Exclude lll issues for long lines having //nolint: directives
    - linters:
      - lll
      source: "//nolint:"

    # Exclude lll issues for long lines having //revive: directives
    - linters:
      - lll
      source: "//revive:"

  # Independently from option `exclude` we use default exclude patterns,
  # it can be disabled by this option. To list all
  # excluded by default patterns execute `golangci-lint run --help`.
  # Default value for this option is true.
  exclude-use-default: true

  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
  max-issues-per-linter: 75

  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
  max-same-issues: 500

  # Show only new issues: if there are unstaged changes or untracked files,
  # only those changes are analyzed, else only changes in HEAD~ are analyzed.
  # It's a super-useful option for integration of golangci-lint into existing
  # large codebase. It's not practical to fix all existing issues at the moment
  # of integration: much better don't allow issues in new code.
  # Default is false.
  new: false

  # Show only new issues created after git revision `REV`
  #new-from-rev: REV

  # Show only new issues created in git patch with set file path.
  #new-from-patch: path/to/patch/file

Any thoughts or suggestions to try?

[bradleygore]

I also tried running on this agent: macos-12 and same output 😢

[bradleygore]

One thing I'm seeing is that it seems to not like dot imports - which I heavily use in tests. So, like where it errors about 2022-07-28T20:42:26.8920580Z ##[error]undeclared name: `MockConnection` (typecheck) the MockConnection is a mock generated by mockgen and pkg is imported as . "pos-api/internal/websocket/mocks

Could there be something causing the linter to handle this setup different in the GH Action vs locally or even in Jenkins CI env?

[maratori]

@bradleygore What happens if change command in Makefile to cd .. && golangci-lint run?

[bradleygore]

Hi @maratori - thanks for the reply! If I just cd to root and do golangci-lint run -v --timeout 3m I get the same output as I previously did locally. I think it has something to do with the dot imports when running in GH Action, based on the output errors.

[bradleygore]

@maratori - I decided to take a step back and walk before running and just try to go build the project in the GH Action. Turns out, there was a private dependency I had forgotten about and had to set up 😲 Now the linter action is working perfectly after I addressed this - so it was totally my problem 😅 Sorry to bother - have a great weekend 😄