Don't generate stubs for already implemented methods

[timsolov]

No description provided.

[josharian]

Thanks! Looks good overall. A bunch of local suggested changes...

[josharian]

This might be simpler and safer as two conditionals:

// strip leading *
if _, ok := v.(*ast.StarExpr); ok {
  v = v.X
}
if _, ok := v.(*ast.Ident); ok {
  return v.Name
}

[timsolov]

We can't assign v.X to v, because v is not an interface{}.

[sevki]

I think josh's point is more that instead of having a switch statement the flow can be simplified by first casting the starexpr to ident then treating both cases as ident thus reducing the number of returns to 1

func getName(v interface{}) string {

// strip leading *
  if n, ok := v.Type.(*ast.StarExpr); ok {
    v = n.X
  }
  if _, ok := v.Type.(*ast.Ident); ok {
    return v.Name
  }
  return fmt.Sprintf("%T is not a supported type", v) 
}

[timsolov]

I understood at first time. But as I thought @josharian meant to do it inside getReceiver func and not move this code to external func like you propose.

If I replace my switch to if I'll receive compiler error:
cannot use n.X (variable of type ast.Expr) as * ast.Field value in assignment

So this is a small piece of code I think It's not needed create new func for it. And propose don't change it.

@sevki, in your func v.Type will not work because v interface{} has no this property and you've to do additional type assertions what will increase your code.

[sevki]

sorry, I didn't mean to backseat drive your PR. I was just looking at this exact problem today and was delighted to see the PR, thought some more activity might help speed things alone :) just ended up creating noise ☹️

[timsolov]

Hi Josh! I did almost everything you ask. Please check it.

[josharian]

On a second read, I think we should remove this break.

Without the break, we build a list of all methods declared.

With the break, we do O(num methods requested X num methods declared) work, which is quadratic.

The number of methods requested is likely to be small in almost all cases, but I can imagine it blowing up painfully in large, autogenerated files. My years of working on the compiler has taught me that people do weird and wonderful things. :)

What do you think?

[timsolov]

Hm, but what will it give us? This cycle only search for first method which already implemented and when we've found it we save it to map. Here the logic is finish. But what do you propose it should do after it founds method? I don't understand why we should continue comparison after our job is finished even if we have large autogenerated file?

[josharian]

Sorry, I did a bad job explaining.

But here's a simpler idea: Convert fns from a []string to a map[string]bool. Then do a map lookup here instead of a loop. This will avoid the O(n) loop that could in theory create quadratic behavior.

[timsolov]

fns is not a []string. fns is []Func which is returned by funcs()

[josharian]

I'll make the change in a follow-up. Thanks again for the contribution! This is a much-requested feature.

[josharian]

Thanks again. Just one outstanding question before merge...