typecheck incorrectly complains of "operator == not defined on untyped nil"

[efd6]

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

We are seeing the following. This should not be happening since the spec says (my emphasis):

Slice, map, and function values are not comparable. However, as a special case, a slice, map, or function value may be compared to the predeclared identifier nil. Comparison of pointer, channel, and interface values to nil is also allowed and follows from the general rules above.

Check failure on line 213 in x-pack/filebeat/input/httpjson/encoding.go
GitHub Actions / lint (darwin)

invalid operation: cannot compare dst.header == nil (operator == not defined on untyped nil) (typecheck)

This does not happen locally.

The issue looks to be related to https://go.dev/issue/39755.

Version of golangci-lint

v1.47.2

Version of the GitHub Action

Question not clearly defined.

Workflow file

name: golangci-lint
on:
  #push:
  #  branches:
  #    - main
  #    - 8.*
  #    - 7.17
  pull_request:
permissions:
  contents: read
  # Optional: allow read access to pull request. Use with `only-new-issues` option.
  pull-requests: read
jobs:
  golangci:
    strategy:
      matrix:
        include:
          - GOOS: windows
          - GOOS: linux
          - GOOS: darwin
    name: lint
    runs-on: ubuntu-latest
    steps:
      - name: Echo details
        env:
          GOOS: ${{ matrix.GOOS }}
        run: echo Go GOOS=$GOOS

      - uses: actions/checkout@v3

      - uses: actions/setup-go@v3
        with:
          go-version-file: .go-version

      - name: golangci-lint
        env:
          GOOS: ${{ matrix.GOOS }}
          CGO_ENABLED: 1
        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

          # Give the job more time to execute.
          # Regarding `--whole-files`, the linter is supposed to support linting of changed a patch only but,
          # for some reason, it's very unreliable this way - sometimes it does not report any or some
          # issues without linting the whole files, so we have to use `--whole-files`
          # which can lead to some frustration from developers who would like to
          # fix a single line in an existing codebase and the linter would force them
          # into fixing all linting issues in the whole file instead
          args: --timeout=30m --whole-files

          # Optional: if set to true then the action will use pre-installed Go.
          skip-go-installation: true

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

Go version

go1.18.9

Code example or link to a public repository

https://github.com/elastic/beats/pull/34044/files#annotation_7094626396

[joshcarp]

Also still an issue with v1.50.1

[AshwinB-hat]

@ldez I am facing the same issue as well. on v1.50.1. this type of error pops up only on the github runners and not on the local. I can take it up, if you have an idea of where we might need to look at

[efd6]

typecheck really just needs to go away; it does nothing that a build won't do and seems to be increasingly wrong. Here we have a complaint about a declaration of rest that is claims in unused, but is then used in six function calls.

Running this locally does not fail here.

[klm127]

I'm also seeing this happen when comparing the value of a function that returns an interface to nil, which shouldn't be a problem, since interfaces are pointers. Compiles without issue.

[ldez]

typecheck is not a real linter it's just a way to parse/display "compilation" and linters errors (linter reports are not errors).
It cannot be disabled because of that.

Of course, this is just as good as the compiler itself and a lot of compilation issues will not properly show where in the code your error lies.

https://golangci-lint.run/welcome/faq/#why-do-you-have-typecheck-errors

typecheck doesn't have a specific behavior inside the GitHub Action.

If there is a difference between local and CI, it's because some elements (not related to golangci-lint) are missing (ex: if using CGO, ensure all required system libraries are installed.)

[efd6]

And yet in all the cases above the code does compile, and in all cases can be trivially seen to be the case. This should not be closed, it should be fixed.

[ldez]

This issue is open on the GitHub Action repository, this issue is not related to GitHub Action, typecheck doesn't have a specific behavior inside the GitHub Action or a CI.

I ran golangci-lint on your code, excepted the CGO imports problem there is no "false-positive" with typecheck.

I can see that you are using an old version of the GitHub Action (latest version is v5): https://github.com/elastic/beats/blob/c1748f7965c9bfd4d488403bb2a734f6f5627219/.github/workflows/golangci-lint.yml#L42
and an old version of golangci-lint (latest version is v1.57.2): https://github.com/elastic/beats/blob/c1748f7965c9bfd4d488403bb2a734f6f5627219/.github/workflows/golangci-lint.yml#L45

Once again, typecheck is not a real linter, it's just a catch-all reporter for unexpected errors.

So, if you can create a minimal reproducible example or if you have something reproducible, you can open an issue on the golangci-lint repository.

[efd6]

The example shown in #624 (comment) is against a package that has no Cgo dep.

[ldez]

$ git clone git@github.com:elastic/beats.git          
Cloning into 'beats'...
remote: Enumerating objects: 387776, done.
remote: Counting objects: 100% (1362/1362), done.
remote: Compressing objects: 100% (835/835), done.
remote: Total 387776 (delta 801), reused 871 (delta 503), pack-reused 386414
Receiving objects: 100% (387776/387776), 415.04 MiB | 6.01 MiB/s, done.
Resolving deltas: 100% (244063/244063), done.
$ cd beats 
$ golangci-lint run ./x-pack/filebeat/input/cel/ 
WARN [config_reader] The configuration option `linters.staticcheck.go` is deprecated, please use global `run.go`. 
WARN [config_reader] The configuration option `linters.gosimple.go` is deprecated, please use global `run.go`. 
WARN [config_reader] The configuration option `linters.stylecheck.go` is deprecated, please use global `run.go`. 
x-pack/filebeat/input/cel/input_test.go:450:15: Error return value of `io.ReadAll` is not checked (errcheck)
                                io.ReadAll(r.Body)
                                          ^
x-pack/filebeat/input/cel/input_test.go:452:12: Error return value of `w.Write` is not checked (errcheck)
                                w.Write([]byte(text))
                                       ^

[efd6]

Yeah, I know. It doesn't fail locally, only here. This is why it's filed against this repo and not that one. I think this is going around in circles.

[ldez]

It's an environmental problem:

• It can be a difference between the Go version and Go versions supported by your version of golangci-lint.
• It can be a problem with the installed Go tooling.
• It can be a side effect of Cgo.
• It can be an OS problem.
• etc.

The problem is not directly related to golangci-lint or the action, golangci-lint is just influenced by something inside your CI.

So, you are asking me to debug your CI.
I don't know if I want to spend hours looking for the broken part in your CI, you are always aggressive and haughty with me.

If I work on that, I will need to have something that currently doesn't work on your CI as a starting point.
For now, from what I can see, the problem seems to only happen to Darwin.

[ldez]

I think the problem is here: https://github.com/elastic/beats/blob/c1748f7965c9bfd4d488403bb2a734f6f5627219/.github/workflows/golangci-lint.yml#L15-L22

  golangci:
    strategy:
      matrix:
        include:
          - GOOS: windows
          - GOOS: linux
          - GOOS: darwin
    name: lint
    runs-on: ubuntu-latest

You are playing with GOOS on Linux to fake a run of golangci-lint on Windows and Darwin.
This is not supposed to work: golangci-lint is not Go.
You can influence the analysis by playing with the GOOS but golangci-lint is compiled for a specific OS, so some pieces, related to the other OS, can be missing.

In the end, you have issues not properly showing where in the code your error lies because the problem is somewhere else.

[efd6]

Thank you for the analysis, that is helpful. I apologise if I appear aggressive; having non-engagement for two years and then a close without discussion is frustrating.

[ldez]

To be honest, I didn't respond sooner because, since your message about "differences in opinion about approaches to linting at scale", I was afraid of your reaction.

I put a lot of effort into improving typecheck report quality and typecheck documentation, and I also improved the behavior of the action.
So I thought it was the right moment to close this issue.

I tried to provide all information when I closed the issue, but it was not enough, I'm sorry, I will improve that.

[efd6]

Thank you. I will try harder too.