x/tools/go/packages: Package.Fset is nil when using NeedSyntax

[gordonklaus]

What did you do?

package main

import (
	"fmt"
	"golang.org/x/tools/go/packages"
)

func main() {
	conf := &packages.Config{
		Mode: packages.NeedSyntax,
	}
	pkgs, err := packages.Load(conf, "fmt")
	if err != nil {
		panic(err)
	}
	fmt.Println(pkgs[0].Fset == nil)
}

What did you expect to see?

false

What did you see instead?

true

Rationale

This is apparently working as intended, as the documentation reads:

	// Fset provides position information for Types, TypesInfo, and Syntax.
	// It is set only when Types is set.
	Fset *token.FileSet

However, I see no reason why Fset should not be exposed when NeedSyntax is requested. In particular, Fset is needed for looking up position information for syntax elements.

[thanm]

@matloob per owners

[adonovan]

I agree that this is a bug.

In my experience, the packages.Config.Mode API is just hateful. One is forced to choose between the compound Load... modes, whose names are misleading, whose use triggers a deprecation warning in the IDE, and whose current incremental and undocumented declarations do not aid comprehension at all; or the fine-grained Need... modes, which are riddled with bugs such as this one due to the explosion of combinations in the implementation, and which by design invite bugs in other code because of the exponential number of possible "flavors" of Package that result.

Every time I use Mode, I try to do the right thing and specify just the bits I need, and then realize through painful trial and error that I need to enable some extra bit such as NeedTypes just to get access to the FileSet, defeating any possible benefit of the fine-grained interface. Apologies for my part in getting us into this mess.

We should fix the bugs in the fine-grained implementation, but more importantly I think we should un-deprecate the compound modes, creating new ones as needed with appropriate names for the major use cases, and document exactly what every existing mode is good for.

[dominikh]

Another fun interaction of modes: without NeedModule, go/types.Config.GoVersion won't be set. Which is slightly surprising, because NeedModule is documented as populating the Module field, not as being relevant to type-checking.

[adonovan]

Similar problems with mode bits:

• x/tools/go/packages: Packages.{Syntax,TypeInfo} fields are empty unless NeedTypes #63517
• x/tools/go/packages: NeedDeps has no effect #56633
• x/tools/go/packages: NeedSyntax without NeedFiles/NeedCompiledGoFiles produces empty slice #56677
• x/tools/go/packages: docs for Config.Mode exception buried in LoadMode docs rather than Load #43850

[gopherbot]

Change https://go.dev/cl/588141 mentions this issue: go/packages: document fields that are part of JSON schema