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

nilaway doesn't detect guard on err variable when assignment to variable happens in branch, within loop #110

Open
daboross opened this issue Nov 17, 2023 · 1 comment
Labels
false positive Requires more analysis and support

Comments

@daboross
Copy link

daboross commented Nov 17, 2023

I think this is a pretty minimal example:

package minimal

import "fmt"

type Thing struct {
	field int
}

func LoopProcess(thing *Thing) (*Thing, error) {
	var err error

	for i := 0; i < 10; i++ {
		switch i {
		case 0:
			thing, err = process(thing)
		default:
			thing, err = process(thing)
		}
		if err != nil {
			return nil, err
		}
	}

	return thing, nil
}

func process(thing *Thing) (*Thing, error) {
	if thing.field == 0 {
		return nil, fmt.Errorf("")
	}

	return thing, nil
}

nilaway reports this as a possible nil panic:

$ nilaway ./main.go
/home/dross/minimal/main.go:28:5: error: Potential nil panic detected. Observed nil flow from source to dereference point:
	-> minimal/main.go:15:25: result 0 of `process()` lacking guarding; passed as arg `thing` to `process()`
	-> minimal/main.go:28:5: function parameter `thing` accessed field `field`

If I replace the loop with two successive calls, it's fine with it:

func LoopProcess(thing *Thing, i1, i2 int) (*Thing, error) {
	var err error

	switch i1 {
	case 0:
		thing, err = process(thing)
	default:
		thing, err = process(thing)
	}
	if err != nil {
		return nil, err
	}

	switch i2 {
	case 0:
		thing, err = process(thing)
	default:
		thing, err = process(thing)
	}
	if err != nil {
		return nil, err
	}

	return thing, nil
}

Equally so if I remove the conditional within the loop:

func LoopProcess(thing *Thing) (*Thing, error) {
	var err error

	for i := 0; i < 10; i++ {
		thing, err = process(thing)
		if err != nil {
			return nil, err
		}
	}

	return thing, nil
}

Note, replacing the switch with an if/else also reproduces the error, as does an if with no else.

@daboross daboross changed the title nilaway doesn't detect guard on err variable reused in loop nilaway doesn't detect guard on err variable reused in loop when assignment happens in if Nov 17, 2023
@daboross daboross changed the title nilaway doesn't detect guard on err variable reused in loop when assignment happens in if nilaway doesn't detect guard on err variable reused in loop when assignment happens in case Nov 17, 2023
@daboross daboross changed the title nilaway doesn't detect guard on err variable reused in loop when assignment happens in case nilaway doesn't detect guard on err variable when assignment to variable happens in branch, within loop Nov 17, 2023
@sonalmahajan15
Copy link
Contributor

Hmm, this is interesting. We'll investigate more into this issue and hopefully come up with a fix soon. Thanks for reporting, @daboross! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false positive Requires more analysis and support
Projects
None yet
Development

No branches or pull requests

2 participants