Best-case effort with type resolution

This is a rather long post, but bear with me.

Consider the following code:

// The LimitReader function in the io package accepts an io.Reader r and a number of bytes n,
// and returns another Reader that reads from r but reports an end-of-file condition after n bytes.
// Implement it.

import "io"

type Limited struct {
reader io.Reader
limit int64
cur int64
}

func (limited *Limited) Read(p []byte) (int, error) {
n, err := limited.reader.Read(p)
if limited.cur + int64(n) > limited.limit {
return n, io.EOF
}
limited.cur += int64(n)
return n, err
}
func LimitReader(r io.Reader, n int64) io.Reader {
return &Limited{reader: r, limit: n}
}

As you can see, the type *LimitReader "is a" Reader since it implements the method that satisfies the contract of the io.Reader interface.

Now consider the test code:

var expLim int64 = 4
var expPrefix = "four"
reader := LimitReader(bytes.NewBufferString(expPrefix+ " and more, but clip at 4"), expLim) // line 13.
buf := make([]byte, expLim) // exactly four bytes long
act, err := reader.Read(buf) // line 15.
// we should be good here!
if err != nil {
t.Errorf(fmt.Sprintf("error -- expected: %v, actual: %v", nil, err))
}
if expLim != int64(act) {
t.Errorf(fmt.Sprintf("bytes read -- expected: %v, actual: %v", expLim, act))
}

Now, I am at line number 15 in the editor and I want to go to the implementation of the Read method:

Clearly, I press Cmd+B and it takes me to io/io.go (Line 77) which is just the definition of the type of the variable (and not the expression): 

What would be delightful is if the IDE were to jump to the implementation of that method in the expression (as far as possible). In this case, line 13, a call to the function LimitReader returns an instance of *Limited and I couldn't find a way to actually jump to its Read method:

 

Does the IDE have enough context to be able to jump here and if that fails, degenerate to the method definition in type of the variable?

5 comments
Comment actions Permalink
Official comment

Hi Cedar,

> Now I'm on the 15th line in the editor and I want to go to the implementation of the Read method:

> It is clear that I click Cmd + B, and this leads me to io / io.go (line 77), which is simply a definition of the type of a variable (and not an expression):

Go to the implementation is Cmd + Alt + B. However, it will show you all the possible implementations here.

> Does the IDE have enough context to go here, and if that fails, degenerate into a method definition in a variable type?

Yes, IDE has enough content, but analyzing each return expression in each function is rather expensive, instead of relying on the type of the return value of the function.

There may be more complicated cases like this:

func LimitReader(r io.Reader, n int64) io.Reader {
if n < 64 {
return &Limited{reader: r, limit: n}
}
return &AnotherLimited{reader: r, limit: n}
}

And the cost grows exponentially since a return expression may contain a delegation of another function:

func LimitReader(r io.Reader, n int64) io.Reader {
if n < 64 {
return &Limited{reader: r, limit: n}
}
return AnotherReader(r, n)
}

If the delegating function is defined in another file, it is even worse, because due to architectural restrictions we won't be able to put the result in the index, so we will have to calculate return-type for each resolution, which means that IDE will parse all related files.
Theoretically, it will be possible to create a huge project with a function that will require to parse the entire project for inferring return type of the function. It's more or less ok for a compiler but not really good for an editor.

So this analyzing is not on the plan. If you're sure the function returns `*LimitedReader` instance it's better to explicitly tell this in the function's signature.

Comment actions Permalink
Official comment

Also, we have a feature request to add filtering to `Implementations` action and make it more usable in your case. Please check https://youtrack.jetbrains.com/issue/GO-6586.

Comment actions Permalink

Thanks @Alexander and @Marat.

> If you're sure the function returns `*LimitedReader` instance it's better to explicitly tell this in the function's signature.

Well, that defeats the purpose of interface-based programming. And I totally understand that such an inference of dynamic types from an interface type can get really convoluted in dynamic languages like Go. My point was about making developers' lives just a little bit more comfortable perhaps by using heuristics (ML? ;)). In the example that I gave (which I suspect to be a representative of a good portion of all use-cases), I really thought that the IDE should walk an extra mile ...

0
Comment actions Permalink

What Alexander wrote above is correct.

In Go, it is preferred to accept interfaces and return concrete types. For references, you can search on Google using something such as "golang accept interface return type" or read this article: https://blog.chewxy.com/2018/03/18/golang-interfaces/ which is one of the many talking about this.

The reason is that in Go it is preferred for the consumer of an API to define what behavior it accepts, which is where the interface type comes into play, and for the API itself to define all the possible options to use it, which is where the concrete type comes in. That makes the code flexible by allowing a consumer of the API to change the implementation of the API or which behavior it needs from that API, which translated means that it allows consumers to accept different types and what methods they need from those types.

The code referenced in the example above should be rewritten to something like this:

 

func LimitReader(r io.Reader, n int64) *Limited {
return &Limited{reader: r, limit: n}
}

and then the consumer code can be written like this if an interface is really needed. But as Alexander wrote, it's probably not needed since the implementation is already known and it's a single implementation that's needed. Which is another point of Go, that if only a type implements the specific interface then it's preferred to use the type directly:

var io.Reader reader := LimitReader(bytes.NewBufferString(expPrefix+ " and more, but clip at 4"), expLim) // line 13.

I hope this helps to understand the issue.

We should also probably add an inspection to help prevent such mistakes in code and warn users about returning interface types instead of concrete types.

1
Comment actions Permalink

Ah, thank you Florin for your explanation. Makes sense. After I make changes to the LimitReader function as you suggested, things are much better. Indeed, I should learn Go's spirit: "accept interfaces and return concrete types".

0

Please sign in to leave a comment.