nilaway icon indicating copy to clipboard operation
nilaway copied to clipboard

Nil Receivers assignment to interface False positive

Open rwinograd opened this issue 10 months ago • 0 comments

I believe that there is a false positive that occurs when a receiver that correctly handles Nil is assigned to an interface. I'm including a slightly more verbose code sample to show some differences in handling:

package main

type SomeInterface interface {
	SomeMethod()
}

type NilHandlingStruct struct {
	ptr int
}

func (s *NilHandlingStruct) SomeMethod() {
	if s == nil {
		return
	}
	print(s.ptr)
}

type NonNilHandlingStruct struct {
	ptr int
}

func (s *NonNilHandlingStruct) SomeMethod() {
	print(s.ptr)
}

func main() {
	var s1 *NilHandlingStruct
	s1.SomeMethod()
	
	var i1 SomeInterface = s1
	i1.SomeMethod()

	var s2 *NonNilHandlingStruct
	s2.SomeMethod()

	var i2 SomeInterface = s2
	i2.SomeMethod()
}

In the above code, we have two structs with slightly different behaviour. NilHandlingStruct's SomeMethod receiver handles a nil such that it won't panic, while NonNilHandlingStruct's SomeMethod will panic.

Each of the items is handled slightly differently in nilaway

~/niltest/main.go:23:8: error: Potential nil panic detected. Observed nil flow from source to dereference point: 
	- niltest/main.go:34:2: unassigned variable `s2` used as receiver to call `SomeMethod()`
	- niltest/main.go:23:8: read by method receiver `s` accessed field `ptr`

~/niltest/main.go:31:2: error: Potential nil panic detected. Observed nil flow from source to dereference point: 
	- niltest/main.go:31:2: unassigned variable `s1` called `SomeMethod()` via the assignment(s):
		- `s1` to `i1` at niltest/main.go:30:6

~/niltest/main.go:37:2: error: Potential nil panic detected. Observed nil flow from source to dereference point: 
	- niltest/main.go:37:2: unassigned variable `s2` called `SomeMethod()` via the assignment(s):
		- `s2` to `i2` at niltest/main.go:36:6

and also handled differently in nilness Image

When I run this code:

Call site Panics? NilAway result Nillness Result
s1.SomeMethod() N Not Detected (TN) Detected (FP)
i1.SomeMethod() N Detected (FP) Not Detected (TN)
s2.SomeMethod() Y Detected (TP) Detected (TP)
i2.SomeMethod() Y Detected (TP) Not-detected (FN)

The FP result in NilAway is the most concerning, but it would be nice if the detection of i2.SomeMethod() was consistent with the detection of s2.SomeMethod() (i.e that we called out that the issue is read by method receiver s accessed field ptr)

rwinograd avatar Jun 20 '25 18:06 rwinograd