+
Skip to content

Nil Receivers assignment to interface False positive #337

Open
@rwinograd

Description

@rwinograd

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)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载