unparam icon indicating copy to clipboard operation
unparam copied to clipboard

False positive about result err is always nil

Open lsytj0413 opened this issue 5 years ago • 6 comments

The following code:

package main

import (
        "fmt"
)

func main() {
	modesp := map[int]func(int) (int, int, error){
		1: func(i int) (next int, m int, err error) {
			next = i + 1
			return -1, -1, fmt.Errorf("unexpected char[%v] after %% at %d", next, i)
		},
		2: func(i int) (next int, m int, err error) {
			if i == 1 {
 				next = 1
				m = 1
				return
			}

			return
		},
	}
       _ = modesp
}

returns the following unparam issue:

main.go:13:36: main$2 - result err is always nil

I have update unparam use:

go get -u -v mvdan.cc/unparam

lsytj0413 avatar Sep 11 '19 00:09 lsytj0413

I'm getting something similar for a spy double that I'm using in a test:

	var ackFuncCalled bool
	spyAckFunc := func(bool) error {
		ackFuncCalled = true
		return nil
	}
application/events_worker_test.go:27:27: TestWorkerPullsEvents$1 - result 0 (error) is always nil (unparam)
	spyAckFunc := func(bool) error {
	                         ^

gonzaloserrano avatar Oct 01 '19 10:10 gonzaloserrano

@gonzaloserrano how are you using spyAckFunc?

Thanks for the report @lsytj0413 by the way. I'll take a look when I find time.

mvdan avatar Oct 01 '19 10:10 mvdan

There are some structs collaborating the the ackFunc is passed around but in the end is called like if err := ackFunc(ack); err != nil { where ack is true.

gonzaloserrano avatar Oct 01 '19 12:10 gonzaloserrano

In my case it appears that unparam doesn't like the combination of a map of functions + casting.

Examples:

func blah() {
	_ = map[int]func() ([]byte, error){
		0: func() ([]byte, error) {
			return []byte("0"), nil
		},
		1: func() ([]byte, error) {
			return []byte{}, nil
		},
	}
}

errs with:

internal/server/server.go:91:22: blah$1 - result 1 (error) is always nil (unparam) 0: func() ([]byte, error) {

But....

func blah() {
	_ = []func() ([]byte, error){
		func() ([]byte, error) {
			return []byte("0"), nil
		},
		func() ([]byte, error) {
			return []byte{}, nil
		},
	}
}

And:

func blah() {
	_ = map[int]func() ([]byte, error){
		0: func() ([]byte, error) {
			return []byte{30}, nil
		},
		1: func() ([]byte, error) {
			return []byte{}, nil
		},
	}
}

Both work fine.

Hope these examples help determining/fixing the issue.

LMK if you need more info.

lopezator avatar Jun 25 '21 09:06 lopezator

False positive on a non-nullable return type when there is some other nullable type in the return tuple that is constructed via function that returns pointer but is never nil:

func never_nil() *int {
	a := 1
	return &a
}

// Only fails if the type is a struct
type B struct{}

// False positive
func f() (*int, B) {
	a := never_nil()
	b := B{}
	return a, b
}

// False positive
func f_reordered() (B, *int) {
	a := never_nil()
	b := B{}
	return b, a
}

// False negative
func f_false_negative() (B, *int) {
	b := B{}
	return b, nil
}

// False negative
func f_false_negative_simplest() *int {
	return nil
}

// Correct
func f_inlined() (B, *int) {
	a := 1
	p := &a
	b := B{}
	return b, p
}

// Correct
func f_no_a() B {
	b := B{}
	return b
}

// Correct
func f_no_b() *int {
	a := never_nil()
	return a
}

The error only occurs on f and f_reordered:

f - result 1 (packagename.B) is always nil
func f() (*int, B) {
                ^

If the type is a primitive or an interface the bug does not occur.

benjajaja avatar Jul 17 '23 08:07 benjajaja

Is there an elegant way to deal with this?

lucasfcnunes avatar Dec 15 '23 15:12 lucasfcnunes