go/doc,x/pkgsite: rethink order for factory functions

[pjebs]

For my package: https://godoc.org/github.com/rocketlaunchr/dataframe-go/forecast

I have a function:
func Forecast(ctx context.Context, sdf interface{}, r *dataframe.Range, alg ForecastingAlgorithm, cfg interface{}, n uint, evalFunc EvaluationFunc) (interface{}, []Confidence, float64, error) which returns an interface{} as the first return value.

Unfortunately my function is categorized under type Confidence when it should not be.

I assume this issue would also be present in pkg.go.dev, but my package has been banned from there (despite the person in charge saying it will be allowed many many months ago), so I can't check if this issue persists.

[cagedmantis]

/cc @dmitshur

[dmitshur]

Thanks for reporting. This behavior is a part of the go/doc package, so it affects all places that use it for displaying Go documentation.

The Forecast function is being considered as a "factory function" for the type Confidence because it returns a slice of that type, and all other return value types are predeclared type.

This is working as intended given the current implementation, see the relevant code in go/doc here.

Note that this is a heuristic, so it is expected it may not produce absolutely ideal results in all cases. The heuristic was chosen with a goal of working as well as possible as often as possible. Based on your description, it sounds this is a case where it's not optimal.

We could consider changing the heuristic to require associated type of factory functions to be the first result type, rather than the first visible result type. In order to gain confidence that it would be a net positive to make such a change, we could test it out on a large corpus of Go packages and evaluate whether it results in more changes that are more helpful than unhelpful.

/cc @griesemer @julieqiu

[go101]

This is a design issue which has been reported for several times: https://github.com/golang/go/issues/28006 and https://github.com/golang/go/issues/38666

Personally, I would like to list such functions both in the function list and the method list of a type. (BTW, I have implemented this in an experimental project.)

[julieqiu]

Let's close this in favor of using #39813 as the tracking issue for considering changes to the "factory function" behavior.

[julieqiu]

(Meant to close #40927.)

[tooolbox]

The factory-function heuristics work great the vast majority of the time. It may be surprising initially, but once you understand the organization it's very valuable. Perhaps we need to work on the "surprising" aspect, but breaking them out from under their respective types is a grass-is-greener proposition, since we trade one use case for another and it's not clear it's an improvement.

Personally, I would like to list such functions both in the function list and the method list of a type.

I understand the sentiment, but I believe this has a couple of harmful effects:

• It bloats the Index so the package looks bigger than it actually is.
• A reader then needs to compare the two occurrences in the Index to be sure they're actually the same, rather than some close spelling.

My suggestion is, if your package has constructor methods that you expect to require regular usage, and you want to spotlight them, put some examples in the Overview section at the top. In #28006 you mention net/http but I have never wondered how to use that package because the first thing you see in the docs are examples of http.Get() and http.Post().

We could consider changing the heuristic to require associated type of factory functions to be the first result type, rather than the first visible result type. In order to gain confidence that it would be a net positive to make such a change, we could test it out on a large corpus of Go packages and evaluate whether it results in more changes that are more helpful than unhelpful.

This sounds like a change that could handle @pjebs specific issue without negative impacts.