go/packages: smooth API documentation for release
Change-Id: Ia3d06a320228831115eb0e0e99d45a84a812da03
Reviewed-on: https://go-review.googlesource.com/129636
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/go/packages/doc.go b/go/packages/doc.go
index 4f5a1a1..e5d23b5 100644
--- a/go/packages/doc.go
+++ b/go/packages/doc.go
@@ -6,13 +6,14 @@
Package packages loads Go packages for inspection and analysis.
NOTE: THIS PACKAGE IS NOT YET READY FOR WIDESPREAD USE:
- - The interface is still being revised and is likely to change.
- - The implementation depends on the Go 1.11 go command.
+ - The interface is still being revised and minor changes are likely.
+ - The implementation depends on the Go 1.11 go command;
+ support for earlier versions will be added soon.
- We intend to finalize the API before Go 1.11 is released.
The Load function takes as input a list of patterns and return a list of Package
structs describing individual packages matched by those patterns.
-The LoadMode controls the amounts of detail about the loaded packages.
+The LoadMode controls the amount of detail in the loaded packages.
The patterns are used as arguments to the underlying build tool,
such as the go command or Bazel, and are interpreted according to
@@ -40,18 +41,17 @@
multiple patterns: in general it is not possible to determine which
packages correspond to which patterns.
-Note that the list returned by Load (LoadAllSyntax in this case)
-only contains the packages matched by the patterns. Their dependencies
-can be found by walking the import graph using the Imports fields.
+Note that the list returned by Load contains only the packages matched
+by the patterns. Their dependencies can be found by walking the import
+graph using the Imports fields.
-The Load function can be configured by passing a non-nil Config struct as
-the first argument. If you pass nil for the Config Load will
-run in LoadAllSyntax mode, collecting the maximal amount of information
-it can.
+The Load function can be configured by passing a pointer to a Config as
+the first argument. A nil Config is equivalent to the zero Config, which
+causes Load to run in LoadFiles mode, collecting minimal information.
See the documentation for type Config for details.
-As noted earlier, the Config.Mode controls increasing amounts of detail
-about the loaded packages, with each mode returning all the data of the
+As noted earlier, the Config.Mode controls the amount of detail
+reported about the loaded packages, with each mode returning all the data of the
previous mode with some extra added. See the documentation for type LoadMode
for details.
@@ -114,12 +114,6 @@
no additional asymptotic cost to providing transitive information.
(This property might not be true of a hypothetical 5th build system.)
-This package provides no parse-but-don't-typecheck operation because most tools
-that need only untyped syntax (such as gofmt, goimports, and golint)
-seem not to care about any files other than the ones they are directly
-instructed to look at. Also, it is trivial for a client to supplement
-this functionality on top of a Metadata query.
-
In calls to TypeCheck, all initial packages, and any package that
transitively depends on one of them, must be loaded from source.
Consider A->B->C->D->E: if A,C are initial, A,B,C must be loaded from
@@ -169,15 +163,13 @@
in several times in sequence as is now possible in WholeProgram mode.
(TypeCheck mode has a similar one-shot restriction for a different reason.)
-Early drafts of this package supported "multi-shot" operation
-in the Metadata and WholeProgram modes, although this feature is not exposed
-through the API and will likely be removed.
+Early drafts of this package supported "multi-shot" operation.
Although it allowed clients to make a sequence of calls (or concurrent
calls) to Load, building up the graph of Packages incrementally,
it was of marginal value: it complicated the API
(since it allowed some options to vary across calls but not others),
it complicated the implementation,
-it cannot be made to work in TypeCheck mode, as explained above,
+it cannot be made to work in Types mode, as explained above,
and it was less efficient than making one combined call (when this is possible).
Among the clients we have inspected, none made multiple calls to load
but could not be easily and satisfactorily modified to make only a single call.
@@ -227,10 +219,6 @@
failed builds, import failures, import cycles, and so on, in a call to
Load?
-- Do we need a GeneratedBy map that maps the name of each generated Go
- source file in GoFiles to that of the original file, if known, or "" otherwise?
- Or are //line directives and "Generated" comments in those files enough?
-
- Support bazel, blaze, and go1.10 list, not just go1.11 list.
- Handle (and test) various partial success cases, e.g.
@@ -252,18 +240,4 @@
order. I suspect this is due to the breadth-first resolution now used
by go/types. Is that a bug? Discuss with gri.
-- https://github.com/golang/go/issues/25980 causes these commands to crash:
- $ GOPATH=/none ./gopackages -all all
- due to:
- $ GOPATH=/none go list -e -test -json all
- and:
- $ go list -e -test ./relative/path
-
-- Modify stringer to use go/packages, perhaps initially under flag control.
-
-- Bug: "gopackages fmt a.go" doesn't produce an error.
-
-- If necessary, add back an IsTest boolean or expose ForTests on the Package struct.
- IsTest was removed because we couldn't agree on a useful definition.
-
*/
diff --git a/go/packages/packages.go b/go/packages/packages.go
index 55a466e..34cd316 100644
--- a/go/packages/packages.go
+++ b/go/packages/packages.go
@@ -21,16 +21,14 @@
"golang.org/x/tools/go/gcexportdata"
)
-// A LoadMode specifies the amount of detail to return when loading packages.
-// The modes are all strictly additive, as you go through the list it increases
-// the amount of information available to you, but may also increase the cost
-// of collecting the information.
-// Load is always allowed to return more information than requested.
+// A LoadMode specifies the amount of detail to return when loading.
+// Higher-numbered modes cause Load to return more information,
+// but may be slower. Load may return more information than requested.
type LoadMode int
const (
// LoadFiles finds the packages and computes their source file lists.
- // Package fields: ID, Name, Errors, GoFiles, OtherFiles.
+ // Package fields: ID, Name, Errors, GoFiles, and OtherFiles.
LoadFiles LoadMode = iota
// LoadImports adds import information for each package
@@ -38,25 +36,26 @@
// Package fields added: Imports.
LoadImports
- // LoadTypes adds type information for the package's exported symbols for
- // packages matching the patterns.
- // Package fields added: Types, Fset, IllTyped.
- // This will use the type information provided by the build system if
- // possible, and the ExportFile field may be filled in.
+ // LoadTypes adds type information for package-level
+ // declarations in the packages matching the patterns.
+ // Package fields added: Types, Fset, and IllTyped.
+ // This mode uses type information provided by the build system when
+ // possible, and may fill in the ExportFile field.
LoadTypes
// LoadSyntax adds typed syntax trees for the packages matching the patterns.
- // Package fields added: Syntax, TypesInfo, for direct pattern matches only.
+ // Package fields added: Syntax, and TypesInfo, for direct pattern matches only.
LoadSyntax
// LoadAllSyntax adds typed syntax trees for the packages matching the patterns
// and all dependencies.
- // Package fields added: Types, Fset, IllTyped, Syntax, TypesInfo, for all
- // packages in import graph.
+ // Package fields added: Types, Fset, Illtyped, Syntax, and TypesInfo,
+ // for all packages in the import graph.
LoadAllSyntax
)
// An Config specifies details about how packages should be loaded.
+// The zero value is a valid configuration.
// Calls to Load do not modify this struct.
type Config struct {
// Mode controls the level of information returned for each package.
@@ -68,14 +67,14 @@
// If Context is nil, the load cannot be cancelled.
Context context.Context
- // Dir is the directory in which to run the build system tool
+ // Dir is the directory in which to run the build system's query tool
// that provides information about the packages.
// If Dir is empty, the tool is run in the current directory.
Dir string
- // Env is the environment to use when invoking the build system tool.
+ // Env is the environment to use when invoking the build system's query tool.
// If Env is nil, the current environment is used.
- // Like in os/exec's Cmd, only the last value in the slice for
+ // As in os/exec's Cmd, only the last value in the slice for
// each environment key is used. To specify the setting of only
// a few variables, append to the current environment, as in:
//
@@ -84,21 +83,19 @@
Env []string
// Flags is a list of command-line flags to be passed through to
- // the underlying query tool.
+ // the build system's query tool.
Flags []string
- // Error is called for each error encountered during package loading.
+ // Error is called for each error encountered during parsing and type-checking.
// It must be safe to call Error simultaneously from multiple goroutines.
- // In addition to calling Error, the loader will record each error
+ // In addition to calling Error, the loader records each error
// in the corresponding Package's Errors list.
- // If Error is nil, the loader will print errors to os.Stderr.
- // To disable printing of errors, set opt.Error = func(error){}.
- // TODO(rsc): What happens in the Metadata loader? Currently nothing.
+ // If Error is nil, the loader prints errors to os.Stderr.
+ // To disable printing of errors, set opt.Error = func(error) {}.
Error func(error)
- // Fset is the token.FileSet to use when parsing source files or
- // type information provided by the build system.
- // If Fset is nil, the loader will create one.
+ // Fset provides source position information for syntax trees and types.
+ // If Fset is nil, the loader will create a new FileSet.
Fset *token.FileSet
// ParseFile is called to read and parse each file
@@ -176,7 +173,7 @@
return driver(cfg, patterns...)
}
-// A Package describes a single loaded Go package.
+// A Package describes a loaded Go package.
type Package struct {
// ID is a unique identifier for a package,
// in a syntax provided by the underlying build system.
@@ -189,14 +186,16 @@
// Name is the package name as it appears in the package source code.
Name string
- // This is the package path as used by the types package.
- // This is used to map entries in the type information back to the package
- // they come from.
+ // PkgPath is the package path as used by the go/types package.
PkgPath string
- // Errors lists any errors encountered while loading the package.
- // TODO(rsc): Say something about the errors or at least their Strings,
- // as far as file:line being at the beginning and so on.
+ // Errors contains any errors encountered while parsing or type-checking the package.
+ // Possible error types include *scanner.ErrorList and types.Error,
+ // whose fields provide structured position information.
+ // Error strings are typically of the form "file:line: message" or
+ // "file:line:col: message".
+ // TODO(adonovan): export packageError as packages.Error
+ // and add that type to the list of structured errors.
Errors []error
// GoFiles lists the absolute file paths of the package's Go source files.
@@ -211,15 +210,15 @@
// including assembly, C, C++, Fortran, Objective-C, SWIG, and so on.
OtherFiles []string
- // ExportFile is the absolute path to a file containing the type information
- // provided by the build system.
+ // ExportFile is the absolute path to a file containing type
+ // information for the package as provided by the build system.
ExportFile string
// Imports maps import paths appearing in the package's Go source files
// to corresponding loaded Packages.
Imports map[string]*Package
- // Types is the type information for the package.
+ // Types provides type information for the package.
// Modes LoadTypes and above set this field for packages matching the
// patterns; type information for dependencies may be missing or incomplete.
// Mode LoadSyntaxAll sets this field for all packages, including dependencies.
@@ -233,13 +232,13 @@
// It is set only when Types is set.
IllTyped bool
- // Syntax is the package's syntax trees, for the files listed in GoFiles.
+ // Syntax is the package's syntax trees, for the files listed in CompiledGoFiles.
//
// Mode LoadSyntax sets this field for packages matching the patterns.
// Mode LoadSyntaxAll sets this field for all packages, including dependencies.
Syntax []*ast.File
- // TypesInfo is the type-checking results for the package's syntax trees.
+ // TypesInfo provides type information about the package's syntax trees.
// It is set only when Syntax is set.
TypesInfo *types.Info
}
@@ -276,6 +275,7 @@
// The imports are written out as just a map of path to package id.
// The errors are written using a custom type that tries to preserve the
// structure of error types we know about.
+//
// This method exists to enable support for additional build systems. It is
// not intended for use by clients of the API and we may change the format.
func (p *Package) MarshalJSON() ([]byte, error) {
@@ -669,8 +669,8 @@
for _, f := range lpkg.Syntax {
for _, imp := range f.Imports {
if imp.Path.Value == `"C"` {
- appendError(fmt.Errorf(`%s: import "C" ignored`,
- lpkg.Fset.Position(imp.Pos())))
+ err := types.Error{Fset: ld.Fset, Pos: imp.Pos(), Msg: `import "C" ignored`}
+ appendError(err)
break outer
}
}
