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

Exclusion patterns work on embedded interfaces, not the actual type of the receiver #132

Open
antifuchs opened this Issue Oct 8, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@antifuchs

antifuchs commented Oct 8, 2017

I'm trying to remove one of my least favorite errcheck highlights, defer response.Body.Close(), which conveniently is a Close on a ReadCloser - a thing that doesn't cause errors I ever care about. So I added it to my -exclude file, like so:

(io.ReadCloser).Close

Unfortunately, this code still makes errcheck signal there's an interesting error to be caught:

package playground

import "io"

type foo struct{}

func (f *foo) Read(p []byte) (n int, err error) {
	return 0, nil
}

func (f *foo) Close() error {
	return nil
}

func tryIt() {
	var f io.ReadCloser = &foo{}
	defer f.Close()  // Shouldn't highlight this, but it does.
}

Does -exclude not work for interfaces' methods?

@dominikh

This comment has been minimized.

Show comment
Hide comment
@dominikh

dominikh Oct 8, 2017

Collaborator

The problem here is that the method is really (io.Closer).Close due to the way embedded interfaces work. Excluding that method does work.

Collaborator

dominikh commented Oct 8, 2017

The problem here is that the method is really (io.Closer).Close due to the way embedded interfaces work. Excluding that method does work.

@antifuchs

This comment has been minimized.

Show comment
Hide comment
@antifuchs

antifuchs Oct 8, 2017

Hrm, that's a bit more wide-ranging than I'd like, but probably worth using considering that I would like to cut down on the number of lints that I ignore in idiomatic code. Would it be possible to issue a warning if an excluded function name can never match?

antifuchs commented Oct 8, 2017

Hrm, that's a bit more wide-ranging than I'd like, but probably worth using considering that I would like to cut down on the number of lints that I ignore in idiomatic code. Would it be possible to issue a warning if an excluded function name can never match?

@dominikh

This comment has been minimized.

Show comment
Hide comment
@dominikh

dominikh Oct 8, 2017

Collaborator

I'll leave this issue open in case someone wants to work on improving the exclusion matching to look at the receiver type in a way a human would expect (i.e. io.ReadCloser in this case)

I don't think a warning is a good idea in general. People are likely to share exclusion lists for different packages or projects, and the method sets can vary.

Collaborator

dominikh commented Oct 8, 2017

I'll leave this issue open in case someone wants to work on improving the exclusion matching to look at the receiver type in a way a human would expect (i.e. io.ReadCloser in this case)

I don't think a warning is a good idea in general. People are likely to share exclusion lists for different packages or projects, and the method sets can vary.

@dominikh dominikh changed the title from (io.ReadCloser).Close does not match in exclusion criteria to Exclusion patterns work on embedded interfaces, not the actual type of the receiver Oct 8, 2017

@kisielk kisielk added the help wanted label May 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment