Skip to content
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

Errcheck not excluding functions as expected when used by Golangci-lint #656

Closed
leosunmo opened this issue Aug 31, 2019 · 12 comments
Closed
Labels
bug Something isn't working dependencies Relates to an upstream dependency false positive An error is reported when one does not exist stale No recent correspondence or work activity

Comments

@leosunmo
Copy link

I am having trouble getting Errcheck to not check methods that are supposedly excluded by default.
Specifically the Hash.Write method explicitly excluded here: https://github.com/kisielk/errcheck/blob/master/internal/errcheck/errcheck.go#L159

If I run Errcheck by itself it does not throw an error in this line, but with Golangci-lint it does, even if I add an explicit exclude file. I don't know what I'm missing here, so any suggestions or troubleshooting steps welcome!

Version:

$ golangci-lint --version
golangci-lint has version 1.17.1 built from 4ba2155 on 2019-06-10T09:06:49Z

Golangci-lint Config file:

$ cat golangci-lint.yaml
linters-settings:
  errcheck:
    # 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-exclude.txt

Errcheck exlude config:

$ cat errcheck-exclude.txt 
(hash.Hash).Write

Go version:
go version go1.12.5 linux/amd64

Golangci-lint output:

$ golangci-lint run -E golint -c golangci-lint.yaml -v
INFO [config_reader] Used config file golangci-lint.yaml 
INFO [lintersdb] Active 11 linters: [deadcode errcheck golint gosimple govet ineffassign staticcheck structcheck typecheck unused varcheck] 
INFO [lintersdb] Optimized sublinters [staticcheck gosimple unused] into metalinter megacheck 
INFO [loader] Go packages loading at mode load types and syntax took 1.538698963s 
INFO [loader] SSA repr building timing: packages building 25.196461ms, total 207.794664ms 
INFO [runner] worker.7 took 1.344549ms with stages: errcheck: 1.340164ms 
INFO [runner] worker.2 took 1.403045ms with stages: varcheck: 1.398897ms 
INFO [runner] worker.1 took 955ns                 
INFO [runner] worker.3 took 1.57871ms with stages: deadcode: 836.488µs, structcheck: 717.989µs, typecheck: 14.465µs 
main.go:362:9: Error return value of `h.Write` is not checked (errcheck)
        h.Write([]byte(key))
               ^
INFO [runner] worker.6 took 93.006833ms with stages: ineffassign: 93.000101ms 
INFO [runner] worker.8 took 372.121254ms with stages: govet: 372.110864ms 
INFO [runner] worker.5 took 433.001196ms with stages: golint: 432.992387ms 
INFO [runner] worker.4 took 1.578426975s with stages: megacheck: 1.578419329s 
INFO [runner] Workers idle times: #1: 1.576949046s, #2: 1.576962288s, #3: 1.576773932s, #5: 1.145364012s, #6: 1.48528822s, #7: 1.577010948s, #8: 1.206269976s 
INFO [runner] Issues before processing: 3, after processing: 1 
INFO [runner] processing took 999.692µs with stages: exclude: 461.177µs, autogenerated_exclude: 164.488µs, identifier_marker: 159.149µs, skip_dirs: 84.189µs, source_code: 58.289µs, nolint: 24.863µs, cgo: 15.244µs, path_prettifier: 13.987µs, max_same_issues: 5.004µs, path_shortener: 3.639µs, filename_unadjuster: 3.276µs, uniq_by_line: 1.788µs, max_from_linter: 1.678µs, diff: 943ns, max_per_file_from_linter: 932ns, skip_files: 553ns, exclude-rules: 493ns 
INFO File cache stats: 1 entries of total size 14.6KiB 
INFO Memory: 33 samples, avg is 629.1MB, max is 1081.6MB 
INFO Execution took 3.36854003s                   
@leosunmo
Copy link
Author

I cloned Errcheck and commented out this line and it gives me the same error as golangci-lint does.

$ ~/go/bin/errcheck # Downloaded errcheck gives me no errors
$ ~/tmp/errcheck/errcheck # My modified errcheck
main.go:362:9:	h.Write([]byte(key)) # Same error golangci-lint throws.

Might be that golangci-lint just needs to upgrade its version of Errcheck.

@tpounds tpounds added bug Something isn't working false positive An error is reported when one does not exist labels Sep 24, 2019
@tpounds tpounds added the dependencies Relates to an upstream dependency label Oct 6, 2019
@stale
Copy link

stale bot commented Apr 3, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale No recent correspondence or work activity label Apr 3, 2020
@stale stale bot closed this as completed Apr 17, 2020
@Zeplar
Copy link

Zeplar commented Apr 9, 2021

I'm on golangci-lint v1.39.0 and this appears to still be a problem. As in the original post, errcheck alone respects the excludes file but golangci-lint does not.

@ldez
Copy link
Member

ldez commented Apr 9, 2021

@Zeplar could you provide an example? Thank you.

@na4ma4
Copy link

na4ma4 commented Aug 3, 2021

still an issue in v1.41.1, I didn't test pulling in an updated errcheck and rebuilding golangci-lint though.

linters-settings/errcheck/exclude and a file works, but using exclude-functions does not.

https://github.com/na4ma4/golangci-lint-errcheck-test

with .golangci.yml using:

linters:
  enable:
    - errcheck

issues:
  exclude-use-default: false

linters-settings:
  errcheck:
    # exclude: errcheck-exclude.txt
    exclude-functions:
    - (io.Closer).Close

golangci-lint still fails on resp.Body.Close() even though the exclude-functions covers (io.Closer).Close.

$ artifacts/bin/v1.41.1/golangci-lint run --sort-results --max-same-issues 0 --max-issues-per-linter 0  ./... | tee "artifacts/lint/golangci-lint"

cmd/errcheck-test/main.go:23:23: Error return value of `resp.Body.Close` is not checked (errcheck)
    defer resp.Body.Close()
                         ^

Using errcheck directly works as expected though.

$ cat "errcheck-exclude.txt"
(io.Closer).Close
$ artifacts/bin/errcheck -verbose ./...
checking github.com/na4ma4/golangci-lint-errcheck-test/cmd/errcheck-test
cmd/errcheck-test/main.go:23:23:	(io.Closer).Close	defer resp.Body.Close()
$ artifacts/bin/errcheck -verbose -exclude errcheck-exclude.txt ./...
checking github.com/na4ma4/golangci-lint-errcheck-test/cmd/errcheck-test

this is suppressed by the default exclude:

# EXC0001 errcheck: Almost all programs ignore errors on these functions and in most cases it's ok
- Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked

@ldez
Copy link
Member

ldez commented Aug 3, 2021

exclude-functions options will be a part of the next release v1.42.0, it's not available in v1.41.1

@bunyk
Copy link

bunyk commented Oct 8, 2021

I seem to be doing something wrong. I created a folder with two files:

package okhandler

import "net/http"

func OkHandler(w http.ResponseWriter, r *http.Request) {
	w.WriteHeader(http.StatusOK)
	w.Write([]byte("OK"))
}

And .golangci.yml:

linters:
  enable:
    - errcheck
  errcheck:
    exclude-functions:
      - (http.ResponseWriter).Write

I run golangci-lint run okhandler.go and still get "Error return value of w.Write is not checked"

$ golangci-lint --version
golangci-lint has version v1.42.0 built from (unknown, mod sum: "h1:hqf1zo6zY3GKGjjBk3ttdH22tGwF6ZRpk6j6xyJmE8I=") on (unknown)

@bunyk

This comment has been minimized.

@bunyk

This comment has been minimized.

@ldez

This comment has been minimized.

@ldez
Copy link
Member

ldez commented Oct 8, 2021

I found the root cause, it's a wrong configuration: you have to set the right package net/http instead of http:

linters:
  enable:
    - errcheck

linters-settings:
  errcheck:
    exclude-functions:
      - (net/http.ResponseWriter).Write

@msamoylov

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Relates to an upstream dependency false positive An error is reported when one does not exist stale No recent correspondence or work activity
Projects
None yet
Development

No branches or pull requests

7 participants