On Thu, Jun 27, 2019 at 10:06 AM Axel Wagner <axel.wagner...@googlemail.com> wrote: > > Can you post some working code to the playground? ISTM that the code you > shown wouldn't compile (X has both a Next field and a Next method). Also, the > receiver is called X, but you return an *Item. That's not totally > hypothetical as a follow-up, because in general, linked lists are an example > of where IMO a nil-value can be made a completely useful implementation of an > interface, by putting a nil-check in the methods. However, it's hard to talk > about it concretely, without working code to adapt to show the idea :)
Sorry for the sloppy code. The first email I sent has a working example to illustrate. My real code where I had this was in the context of an ssh-based library with a bastion host. Here's a summary version: https://play.golang.org/p/tNmWWb_28qZ When I first wrote this, I had a Host struct, with func (h Host) GetVia() *Host . After it worked, I changed the Host to an interface, HostIntf and added Via() HostIntf function to it, and ended up with an enabled recursion, because h.Via() is never nil. > > On Thu, Jun 27, 2019 at 5:46 PM Burak Serdar <bser...@ieee.org> wrote: >> >> - function returns an interface Y >> - there is a return X where X is not of type Y and is a reference >> - X is not assigned a value in at least one code path >> - there are no nil-checks for X > > > IMO this is not sufficient to avoid false positives. For one, if nil is a > valid implementation of an interface, returning a nil-value of that type is > of course totally fine. Then, with the sort.StringSlice example, there are no > nil-checks in any methods - but there don't *have* to be, because `len` on a > nil-slice returns zero and the contract of the interface guarantees that Swap > and Less are never called with out-of-bound indices. Note, that there If you do "return sort.StringSlice(nil)", then the return value is created/assigned in the function body, so it wouldn't alert. It would only alert if the function gets a reference value from somewhere else, converts it to an interface and returns it. So the following would be ok: func f() sort.Interface { return sort.StringSlice(nil) } But this would be a false-positive: func g() sort.StringSlice { return sort.StringSlice(nil) } func f() sort.Interface { return g() } is no way to check for this programmatically - for a tool, if StringSlice.Swap is ever called, with any arguments, on a nil-value, it would panic. > So the implementation of sort.StringSlice is totally fine as is and it's > totally reasonable to use a nil-value as a sort.Interface (e.g. say you > declare a variable of that type and then fill it conditionally with append - > you might end up with a nil-slice) but would be reported as a false-positive. You are talking about analyzing what happens after function returns. I am talking about only analyzing the function that causes the possible nil-value-interface. Above, only the function f() would be analyzed, and the first copy would be ok, and the second copy not, which, in a way makes sense because f() doesn't know what g() does, and if g() returns a nil instead of a nil-value-interface, then f() will hide that nil. > > Come to think of it, this actually returns false-positives even with the most > strict implementation of the check - that is, if it only triggers if *all* > code-paths at the interface creation-point return nil and if *all* code-paths > in the method actually panic. Because even that check would falsely flag > sort.StringSlice. > > Even more sensible then, to implement this as a separate tool, to see what > kind of false-positives it comes up with and how many true-positives it finds > :) > >> >> >> then it is likely that function will mask a nil-return. >> >> >> > >> > If that sounds interesting, I would recommend trying to implement that as >> > a tool out-of-tree, to give the community opportunity to test it out. If >> > you base it on the analysis package, it would be easy to integrate into >> > other tools (and maybe eventually vet) when the time comes :) >> > >> > On Thu, Jun 27, 2019 at 4:20 PM Burak Serdar <bser...@ieee.org> wrote: >> >> >> >> This happened to me more than once, and not only with errors. >> >> >> >> For a function that is declared to return an interface, if you return >> >> a nil-pointer with a type other than that interface, the returned >> >> interface is not nil. >> >> >> >> Here's an example: >> >> >> >> https://play.golang.org/p/dpd76zyN9Fv >> >> >> >> I think go vet should warn about this case. What do you think? >> >> >> >> -- >> >> You received this message because you are subscribed to the Google Groups >> >> "golang-nuts" group. >> >> To unsubscribe from this group and stop receiving emails from it, send an >> >> email to golang-nuts+unsubscr...@googlegroups.com. >> >> To view this discussion on the web visit >> >> https://groups.google.com/d/msgid/golang-nuts/CAMV2RqrpdCa4t6VrBrkbU%3DrbdNhC_oz%2BusTD9PA6VgCb%3D2SXJw%40mail.gmail.com. >> >> For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAMV2Rqo9s7Lz85ShVuFx1KjEDf0t95jBDDHUDVXfLwSaD-5Wvw%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
