Setting to allow zero-values for structs

[mitar]

In my code I often have return foobar{}, errors.New("some error") to return an empty struct when there is an error. Currently linter complains about that.

I can go around that by doing return *new(foobar), errors.New("some error") but that is too verbose for me.

So I wonder, could there be a setting to allow foobar{} and not complain about it? So if you initialize without any field, then nothing happens. If you add any field, then linter requires to add all of them.

[mitar]

Another example I have in my code is unsafe.Sizeof(unix.RawSockaddrUnix{}) when calling raw syscalls.

[navijation]

@mitar I think the return foobar{}, errors.New("some error") case is already handled here. Do you have a reproduction?

That said, I agree with the general utility of ignoring empty struct literals in general. It comes in handy in many situations. e.g. I have a pattern of namespacing constructors using SomeStruct{}.New() instead of NewSomeStruct which exhaustruct complains about. Usually, people are not "forgetting" to fill out fields when they write down empty struct literals. There's almost always a legitimate reason.

[JakeCapra]

@navijation For me, the returnContainsNonNilError check throws things off. Sometimes I return a bool to indicate ok. And other times I return a struct that implements the error interface. In both these situations, the linter complains.

[mitar]

Do you have a reproduction?

I found this example in one of my libraries.

[navijation]

It seems the linter requires the function to return something with exact type error, instead of anything that implements error.

I'm inclined to believe that the linter should never complain about empty struct literals in return values for the reason @JakeCapra mentioned. That said, one workaround would be to add a named return value e.g. out T and always return out. I find that nicer than repeating the type name.

[JakeCapra]

@navijation Any progress or update on this?

[xobotyi]

Not yet - there is extra hard task on supporting comment directives on structures rather on field tags (tags will remain)
This one is kinds simple but yet a bit annoying - it requires to perform manual duck-typing that structu implements error interface. as for now it only allows zero-values for returns with exact error type.

As i am pretty tight on time now - anyone willing is free to come up with PR.
Basically new code have to be added to https://github.com/GaijinEntertainment/go-exhaustruct/blob/master/analyzer/analyzer.go#L197 in order to detect error interface compliance

[navijation]

@xobotyi there is a semantic ambiguity here for this "return nil of type which implements error":

1. (less common) some structs implement the error interface via value receiver, so there is no way to return a non-nil error
2. (more common) a nil struct can still implement the error interface; returning (error)(nil) is not the same as (*MyError)(nil)

Namely, for case 2, consider this example

package main

import "fmt"

type SomeError struct{}

func (*SomeError) Error() string {
	return "There was an error"
}

func returnsSomeError() *SomeError {
	return nil
}

func main() {
	var err error = returnsSomeError()
	fmt.Printf("err is nil: %t\n", err == nil)
}

A bigger problem is that the zero value of a struct has many valid use case where developers might not want to subject it to exhaustive enforcement. Perhaps a better solution is to ignore enforcement on one of the following categorizations (maybe based on an analyzer config):

1. all empty struct literals
2. empty struct literals returned from a function
3. empty struct literals returned from a function that returns multiple return values

I'm not sure which makes the most sense; I'm inclined to believe either 1 or 2.

[ruslanSorokin]

Hi, there is a PR #102 which I think is pretty related to what you mentioned here earlier. It does not address cases where you return custom error from function whose return type is the custom error itself, but it does eliminate false positives when you return a custom error on the position of the error interface.

[xobotyi]

@navijation imo the way it shoul be done - the one it is done already, but with support for detection of custom error types.

Therefore it can be detmined as allow empty structures in case return statement contain non-zero error

[xobotyi]

Closing as #102 being merged.
Feel free to reopen in case made solution does not fit your purpose.

[mitar]

Thanks!

[JakeCapra]

@xobotyi Any idea when this will be released?

[xobotyi]

I will release minor version in the evening, but have no idea about release plan of golangci-lint