Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/doc: help does not seem to be accurate for the "two arguments" mode #49830

Closed
jroimartin opened this issue Nov 28, 2021 · 4 comments
Closed

cmd/doc: help does not seem to be accurate for the "two arguments" mode #49830

jroimartin opened this issue Nov 28, 2021 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@jroimartin
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version go1.17.3 linux/amd64

$ go version
go version devel go1.18-0fa53e41f1 Sat Nov 27 23:29:50 2021 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes, both in the latest stable and tip.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOOS="linux"
GOARCH="amd64"

What did you do?

$ go doc template New
package template // import "html/template"

func New(name string) *Template
    New allocates a new HTML template with the given name.

What did you expect to see? / What did you see instead?

go help doc shows the following information regarding running "go doc" with two arguments:

When run with two arguments, the first must be a full package path (not just a
suffix), and the second is a symbol, or symbol with method or struct field.
This is similar to the syntax accepted by godoc:

        go doc <pkg> <sym>[.<methodOrField>]

In go doc template New, template is a suffix. However, go doc shows documentation for html/template's New function. Which seems to be the same behavior of calling go doc with one argument (i.e. go doc template.New). The documentation explicitly says "the first (argument) must be a full package path (not just a suffix)".

It seems to me that the documentation is outdated. Also, if I'm not wrong, nowadays godoc does not accept that syntax anymore, as it only provides an http server mode.

Extra information

findNextPackage is called when go doc is run with two arguments:

go/src/cmd/doc/main.go

Lines 214 to 229 in 0fa53e4

case 2:
// Package must be findable and importable.
pkg, err := build.Import(args[0], wd, build.ImportComment)
if err == nil {
return pkg, args[0], args[1], false
}
for {
packagePath, ok := findNextPackage(arg)
if !ok {
break
}
if pkg, err := build.ImportDir(packagePath, build.ImportComment); err == nil {
return pkg, arg, args[1], true
}
}
return nil, args[0], args[1], false

and:

go/src/cmd/doc/main.go

Lines 378 to 380 in 0fa53e4

// findNextPackage returns the next full file name path that matches the
// (perhaps partial) package path pkg. The boolean reports if any match was found.
func findNextPackage(pkg string) (string, bool) {

@mknyszek mknyszek changed the title cmd/go: go doc's help does not seem to be accurate for the "two arguments" mode cmd/doc: help does not seem to be accurate for the "two arguments" mode Nov 30, 2021
@mknyszek mknyszek added this to the Backlog milestone Nov 30, 2021
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 30, 2021
@mknyszek
Copy link
Contributor

CC @mvdan via https://dev.golang.org/owners

Maybe also @rsc?

@mvdan
Copy link
Member

mvdan commented Nov 30, 2021

Also, if I'm not wrong, nowadays godoc does not accept that syntax anymore, as it only provides an http server mode.

That's certainly right - the docs should be updated to no longer mention the godoc CLI, which has been removed for some time. Merging such a CL would be easy, if you're up for sending one.

The documentation explicitly says "the first (argument) must be a full package path (not just a suffix)".

Perhaps try looking at the git blame or log to see whether the code or docs were changed most recently. My guess is that the docs used to be true, but then the code was improved, and the docs weren't updated. But it could also be the other way around - that the code's behavior was never intentional and we should update it to match the docs.

It would also be worth checking out the tests. If there are any tests that break when the code is changed to match the docs, then that's a pretty strong signal that it's the docs which are wrong.

@jroimartin
Copy link
Contributor Author

jroimartin commented Nov 30, 2021 •
edited

@mvdan it seems that the change was first introduced in e3442b4:

cmd/doc: search for packages in the two-arg case

When given one argument, as in

	go doc binary.BigEndian

doc would search for the package, but when given two, as in

	go doc binary BigEndian

it would not. Fix the inconsistency.

Actually, there is a test that tests this behavior:

go/src/cmd/doc/doc_test.go

Lines 920 to 966 in 18934e1

// Test the code to look up packages when given two args. First test case is
// go doc binary BigEndian
// This needs to find encoding/binary.BigEndian, which means
// finding the package encoding/binary given only "binary".
// Second case is
// go doc rand Float64
// which again needs to find math/rand and not give up after crypto/rand,
// which has no such function.
func TestTwoArgLookup(t *testing.T) {
if testing.Short() {
t.Skip("scanning file system takes too long")
}
maybeSkip(t)
var b bytes.Buffer // We don't care about the output.
{
var flagSet flag.FlagSet
err := do(&b, &flagSet, []string{"binary", "BigEndian"})
if err != nil {
t.Errorf("unexpected error %q from binary BigEndian", err)
}
}
{
var flagSet flag.FlagSet
err := do(&b, &flagSet, []string{"rand", "Float64"})
if err != nil {
t.Errorf("unexpected error %q from rand Float64", err)
}
}
{
var flagSet flag.FlagSet
err := do(&b, &flagSet, []string{"bytes", "Foo"})
if err == nil {
t.Errorf("expected error from bytes Foo")
} else if !strings.Contains(err.Error(), "no symbol Foo") {
t.Errorf("unexpected error %q from bytes Foo", err)
}
}
{
var flagSet flag.FlagSet
err := do(&b, &flagSet, []string{"nosuchpackage", "Foo"})
if err == nil {
// actually present in the user's filesystem
} else if !strings.Contains(err.Error(), "no such package") {
t.Errorf("unexpected error %q from nosuchpackage Foo", err)
}
}
}

I'll send a CL later updating the docs.

@gopherbot
Copy link

Change https://golang.org/cl/367497 mentions this issue: cmd/go: update "go help doc" docs

@golang golang locked and limited conversation to collaborators Dec 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@jroimartin @mknyszek @mvdan @gopherbot