x/pkgsite, x/website, x/tools/godoc: factory functions are displayed in a way that can cause confusion

[mleonhard]

What did you do?

My golang program needs to find out if a path refers to an existing directory. I need to find out how to do that. Here's what I did:

1. Did a web search for golang isdir
2. Clicked on the first result, os - The Go Programming Language
3. Searched in the page (CTRL-F) for isdir.
4. Found type FileInfo. It contains an IsDir() function. The description says "A FileInfo describes a file and is returned by Stat and Lstat."
5. Spent a minute searching around the page for the proper Stat function. Found type File with a Stat() function. Unfortunately, to create a File struct, one must actually create a normal file on the filesystem.
6. Saw the Stat(string) function indented under type FileInfo in the index. Clicked on it. This is the same struct that I just looked at. Where is the top-level Stat method?
7. Confused, I went back to web search and searched for "golang check if directory exists".
8. Clicked the first result, Check if a file or directory exists in Go (Golang .... That page has an example that starts with fileinfo, err := os.Stat("temp.txt").
9. I went back to the os package docs page and scanned the index. There is no Stat top-level function. This is so strange.
10. Tried typing the function in my editor. It's there, with documentation.
11. Began filing this ticket.
12. Finally noticed that the Stat function is indented under type FileInfo but it's not actually a method on that struct:

    type FileInfo
    func Lstat(name string) (FileInfo, error)
    func Stat(name string) (FileInfo, error)

What did you expect to see?

I expected to see func Stat(name string) (FileInfo, error) unindented, in the top section of the index with the module-level functions. Like this:

Constants
Variables
func Chdir(dir string) error
func Chmod(name string, mode FileMode) error
...
func Setenv(key, value string) error
func Stat(name string) (FileInfo, error)  // <--- here
func Symlink(oldname, newname string) error
...
type File
...
type FileInfo
...
type FileMode
...

What did you see instead?

I saw func Stat(name string) (FileInfo, error) appearing out of order with the other top-level functions in os, indented under type FileInfo as if it is a method of FileInfo.

Constants
Variables
func Chdir(dir string) error
func Chmod(name string, mode FileMode) error
...
func UserConfigDir() (string, error)
func UserHomeDir() (string, error)
type File
...
type FileInfo
    func Lstat(name string) (FileInfo, error)
    func Stat(name string) (FileInfo, error)  // <--- here
type FileMode
...

[andybons]

@dmitshur

[zigo101]

see https://github.com/golang/go/issues/39813, https://github.com/golang/go/issues/38666 and https://github.com/golang/go/issues/28006

[dmitshur]

Thanks for reporting. As @go101 pointed out via related issues, this is a heuristic and it's working as intended. Stat is considered to be a "factory function", so it's displayed underneath FileInfo.

[mleonhard]

@dmitshur The purpose of documentation is to help users learn quickly. Any heuristic that affects documentation must be serve the purpose of the documentation. Since this heuristic is currently causing confusion and slowing down user learning, it cannot be working as intended. For every issue reported, 10-100 people encountered the issue and chose not to report it.

There are many ways to save readers from confusing factory functions with struct receiver funcs. Here are three I thought of:

• Modify the docs generator to always include all top-level functions at the top level in the index, where users expect to find them. Do not change the rest of the index or document. Retain alleged factory functions under their respective structs in the index, just stop removing the top-level links of alleged factory functions.
• Add a clarifying comment after the alleged factory function in the index. Example:

  Constants
  Variables
  func Chdir(dir string) error
  func Chmod(name string, mode FileMode) error
  ...
  func UserConfigDir() (string, error)
  func UserHomeDir() (string, error)
  type File
  ...
  type FileInfo
     func Lstat(name string) (FileInfo, error)  // Top-level func that generates FileInfo.
     func Stat(name string) (FileInfo, error)  // Top-level func that generates FileInfo.
  type FileMode
  ...

• Don't indent factory functions in the index. They will appear out of order with the rest of the top-level funcs, but they will no longer have indentation that implies that they are struct receiver funcs (object methods).

• Include a sentence next to the alleged factory function's signature in the page body explaining that the function is a top-level function that returns a FileInfo struct. Example:

  func Stat
  
  func Stat(name string) (FileInfo, error)
  Stat returns a FileInfo describing the named file. If there is an error, it will be of type *PathError.
  This is a top-level function that generates a FileInfo struct.   <--- add this

Will you please reconsider fixing this issue?

EDIT: s/generator func/factory func/g

@seankhliao I see that you downvoted my issue report. I think this means that you know something that I don't. Would you please explain it?

[dmitshur]

My intention was to suggest we use #39813 for tracking improvements to the heuristic of when to consider a function as a "factory function". However, this issue seems slightly different, as it's about the way factory functions are displayed. Reopening so we can consider this further.

/cc @julieqiu @griesemer

[tooolbox]

The purpose of documentation is to help users learn quickly. Any heuristic that affects documentation must be serve the purpose of the documentation. Since this heuristic is currently causing confusion and slowing down user learning, it cannot be working as intended. For every issue reported, 10-100 people encountered the issue and chose not to report it.

@mleonhard I'm sorry that you had some confusion on this.

Generally speaking, it's easy to recognize constructors because they have no receiver. Take for example the Cuelang Godoc, in particular the Instance type. It's easy to see that Build and Merge are the two constructors, and the rest are methods. Looking down the types I can easily see that Attribute and Iterator have no constructors, Option has only ways to make them, Value has two constructors, etc.

That said, I could see introducing a very small change to call a little more attention to these methods. Perhaps if the circle next to them was filled rather than hollow? I think that would make it easier to spot whilst scanning the overall package, without negatively affecting what seems to be a long-lasting and battle-tested structure.

(...Hm, I realize now that golang.org/pkg is actually a little different from godoc.org, for example missing the bullet, but it could be added.)

[aathan]

In the case of context this appears not to affect all "factory functions" but only some (per #54077)