False positive in error handling logic for "always-safe" case

[sonalmahajan15]

NilAway is encoded with the pattern that if the error return is non-nil, then inherently assume non-error returns to be nilable, and hence need guarding. However, this reports a false positive when the non-error return is always returned as non-nil, independent of the status of the error return, and hence does not really need to be guarded.

func retErr() (*int, error) {
	return new(int), errors.New("error")
}

func callRetErr() {
	v, err := retErr()
	if err != nil {
		print(err)
	}
	_ = *v // error: result 0 of `retErr()` lacking guarding; dereferenced
}

[Integralist]

I have a somewhat similar false positive...

// Library code

func (t *Thing) GetFoo() (*string, bool) {
	if t == nil || t.Foo == nil {
		return nil, false
	}
	return t.Foo, true
}

// Application code

if ptr, ok := t.GetFoo(); ok {
	v := *ptr // nilaway reports this deference hasn't been checked, when it has, by the GetFoo method
}

[sonalmahajan15]

@Integralist, you're absolutely right. However, the false positive you reported is due to a slightly different reason. It's occurring because NilAway doesn't currently understand the ok form of functions/methods. We have plans to address this and add support, which is being tracked in issue #77 on our roadmap.

[Integralist]

Amazing, thanks @sonalmahajan15 for clarifying 👍🏻

[kfcss]

@sonalmahajan15 I think this has been resolved?

[sonalmahajan15]

@kfcss , the problem in the second code snippet has been resolved by PR #157 , but the first code snippet (for which the issue was created) is still work in progress. It's proving to be a little tricky to handle this "always-safe" case.

[sonalmahajan15]

(Linking related issue #199)

[sircelsius]

@sonalmahajan15 any idea if this will be addressed soon?

[sonalmahajan15]

@sircelsius: I am working on addressing this issue at the moment. Hopefully I should have the support for "always-safe" case ready in a couple of weeks.