go.tools/go/loader: simplify command-line syntax.

Previously, each word could be a package import path or a
comma-separated list of *.go file names.  Now, if the
first word ends with ".go", all words are assumed to be
Go source files.  This makes it impossible to specify
two ad-hoc packages from source files, but no-one needs that.

FromArgs also takes a boolean indicating whether tests
are wanted or not.

Also: ssadump: add -test flag to set that boolean.
For the oracle it's always true.

LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/61470047

cmd/ssadump/main.go

@@ -9,7 +9,6 @@ import (
 	"flag"
 	"fmt"
 	"go/build"
-	"log"
 	"os"
 	"runtime"
 	"runtime/pprof"
@@ -32,6 +31,8 @@ L	build distinct packages seria[L]ly instead of in parallel.
 N	build [N]aive SSA form: don't replace local loads/stores with registers.
 `)
 
+var testFlag = flag.Bool("test", false, "Loads test code (*_test.go) for imported packages.")
+
 var runFlag = flag.Bool("run", false, "Invokes the SSA interpreter on the program.")
 
 var interpFlag = flag.String("interp", "", `Options controlling the SSA test interpreter.
@@ -45,14 +46,15 @@ Usage: ssadump [<flag> ...] <args> ...
 Use -help flag to display options.
 
 Examples:
-% ssadump -build=FPG hello.go         # quickly dump SSA form of a single package
+% ssadump -build=FPG hello.go            # quickly dump SSA form of a single package
-% ssadump -run -interp=T hello.go     # interpret a program, with tracing
+% ssadump -run -interp=T hello.go        # interpret a program, with tracing
-% ssadump -run unicode -- -test.v     # interpret the unicode package's tests, verbosely
+% ssadump -run -test unicode -- -test.v  # interpret the unicode package's tests, verbosely
 ` + loader.FromArgsUsage +
 	`
-When -run is specified, ssadump will find the first package that
+When -run is specified, ssadump will run the program.
-defines a main function and run it in the interpreter.
+The entry point depends on the -test flag:
-If none is found, the tests of each package will be run instead.
+if clear, it runs the first package named main.
+if set, it runs the tests of each package.
 `
 
 var cpuprofile = flag.String("cpuprofile", "", "write cpu profile to file")
@@ -70,6 +72,13 @@ func init() {
 }
 
 func main() {
+	if err := doMain(); err != nil {
+		fmt.Fprintf(os.Stderr, "ssadump: %s.\n", err)
+		os.Exit(1)
+	}
+}
+
+func doMain() error {
 	flag.Parse()
 	args := flag.Args()
 
@@ -109,7 +118,7 @@ func main() {
 		case 'L':
 			mode |= ssa.BuildSerially
 		default:
-			log.Fatalf("Unknown -build option: '%c'.", c)
+			return fmt.Errorf("unknown -build option: '%c'", c)
 		}
 	}
 
@@ -121,7 +130,8 @@ func main() {
 		case 'R':
 			interpMode |= interp.DisableRecover
 		default:
-			log.Fatalf("Unknown -interp option: '%c'.", c)
+			fmt.Fprintf(os.Stderr, "ssadump: unknown -interp option: '%c'.", c)
+			os.Exit(1)
 		}
 	}
 
@@ -134,16 +144,17 @@ func main() {
 	if *cpuprofile != "" {
 		f, err := os.Create(*cpuprofile)
 		if err != nil {
-			log.Fatal(err)
+			fmt.Fprintln(os.Stderr, err)
+			os.Exit(1)
 		}
 		pprof.StartCPUProfile(f)
 		defer pprof.StopCPUProfile()
 	}
 
 	// Use the initial packages from the command line.
-	args, err := conf.FromArgs(args)
+	args, err := conf.FromArgs(args, *testFlag)
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	// The interpreter needs the runtime package.
@@ -154,7 +165,7 @@ func main() {
 	// Load, parse and type-check the whole program.
 	iprog, err := conf.Load()
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 	// Create and build SSA-form program representation.
@@ -163,29 +174,38 @@ func main() {
 
 	// Run the interpreter.
 	if *runFlag {
-		// If a package named "main" defines func main, run that.
-		// Otherwise run all packages' tests.
 		var main *ssa.Package
 		pkgs := prog.AllPackages()
-		for _, pkg := range pkgs {
+		if *testFlag {
-			if pkg.Object.Name() == "main" && pkg.Func("main") != nil {
+			// If -test, run all packages' tests.
-				main = pkg
+			if len(pkgs) > 0 {
-				break
+				main = prog.CreateTestMainPackage(pkgs...)
+			}
+			if main == nil {
+				return fmt.Errorf("no tests")
+			}
+		} else {
+			// Otherwise, run main.main.
+			for _, pkg := range pkgs {
+				if pkg.Object.Name() == "main" {
+					main = pkg
+					if main.Func("main") == nil {
+						return fmt.Errorf("no func main() in main package")
+					}
+					break
+				}
+			}
+			if main == nil {
+				return fmt.Errorf("no main package")
 			}
 		}
-		if main == nil && len(pkgs) > 0 {
-			// TODO(adonovan): only run tests if -test flag specified.
-			main = prog.CreateTestMainPackage(pkgs...)
-		}
-		if main == nil {
-			log.Fatal("No main package and no tests")
-		}
 
-		if runtime.GOARCH != conf.Build.GOARCH {
+		if runtime.GOARCH != build.Default.GOARCH {
-			log.Fatalf("Cross-interpretation is not yet supported (target has GOARCH %s, interpreter has %s).",
+			return fmt.Errorf("cross-interpretation is not yet supported (target has GOARCH %s, interpreter has %s)",
-				conf.Build.GOARCH, runtime.GOARCH)
+				build.Default.GOARCH, runtime.GOARCH)
 		}
 
 		interp.Interpret(main, interpMode, conf.TypeChecker.Sizes, main.Object.Path(), args)
 	}
+	return nil
 }

go/loader/importer_test.go

@@ -14,7 +14,7 @@ import (
 
 func loadFromArgs(args []string) (prog *loader.Program, rest []string, err error) {
 	conf := &loader.Config{}
-	rest, err = conf.FromArgs(args)
+	rest, err = conf.FromArgs(args, true)
 	if err == nil {
 		prog, err = conf.Load()
 	}
@@ -39,11 +39,10 @@ func TestLoadFromArgs(t *testing.T) {
 	}
 
 	// Successful load.
-	args = []string{"fmt", "errors", "testdata/a.go,testdata/b.go", "--", "surplus"}
+	args = []string{"fmt", "errors", "--", "surplus"}
 	prog, rest, err := loadFromArgs(args)
 	if err != nil {
-		t.Errorf("loadFromArgs(%q) failed: %s", args, err)
+		t.Fatalf("loadFromArgs(%q) failed: %s", args, err)
-		return
 	}
 	if got, want := fmt.Sprint(rest), "[surplus]"; got != want {
 		t.Errorf("loadFromArgs(%q) rest: got %s, want %s", args, got, want)
@@ -54,7 +53,7 @@ func TestLoadFromArgs(t *testing.T) {
 		pkgnames = append(pkgnames, info.Pkg.Path())
 	}
 	// Only the first import path (currently) contributes tests.
-	if got, want := fmt.Sprint(pkgnames), "[fmt_test P]"; got != want {
+	if got, want := fmt.Sprint(pkgnames), "[fmt_test]"; got != want {
 		t.Errorf("Created: got %s, want %s", got, want)
 	}
 
@@ -82,3 +81,25 @@ func TestLoadFromArgs(t *testing.T) {
 		}
 	}
 }
+
+func TestLoadFromArgsSource(t *testing.T) {
+	// mixture of *.go/non-go.
+	args := []string{"testdata/a.go", "fmt"}
+	prog, _, err := loadFromArgs(args)
+	if err == nil {
+		t.Errorf("loadFromArgs(%q) succeeded, want failure", args)
+	} else {
+		// "named files must be .go files: fmt": ok
+	}
+
+	// successful load
+	args = []string{"testdata/a.go", "testdata/b.go"}
+	prog, _, err = loadFromArgs(args)
+	if err != nil {
+		t.Errorf("loadFromArgs(%q) failed: %s", args, err)
+		return
+	}
+	if len(prog.Created) != 1 || prog.Created[0].Pkg.Path() != "P" {
+		t.Errorf("loadFromArgs(%q): got %v, want [P]", prog.Created)
+	}
+}

go/loader/loader.go

@@ -23,7 +23,7 @@
 //      // Use the command-line arguments to specify
 //      // a set of initial packages to load from source.
 //      // See FromArgsUsage for help.
-//      rest, err := conf.FromArgs(os.Args[1:])
+//      rest, err := conf.FromArgs(os.Args[1:], wantTests)
 //
 //      // Parse the specified files and create an ad-hoc package with path "foo".
 //      // All files must have the same 'package' declaration.
@@ -228,15 +228,14 @@ func (conf *Config) ParseFile(filename string, src interface{}, mode parser.Mode
 // FromArgs may wish to include in their -help output.
 const FromArgsUsage = `
 <args> is a list of arguments denoting a set of initial packages.
-Each argument may take one of two forms:
+It may take one of two forms:
 
-1. A comma-separated list of *.go source files.
+1. A list of *.go source files.
 
    All of the specified files are loaded, parsed and type-checked
-   as a single package.
+   as a single package.  All the files must belong to the same directory.
-   All the files must belong to the same directory.
 
-2. An import path.
+2. A list of import paths, each denoting a package.
 
    The package's directory is found relative to the $GOROOT and
    $GOPATH using similar logic to 'go build', and the *.go files in
@@ -248,9 +247,8 @@ Each argument may take one of two forms:
    the non-*_test.go files are included in the primary package.  Test
    files whose package declaration ends with "_test" are type-checked
    as another package, the 'external' test package, so that a single
-   import path may denote two packages.  This behaviour may be
+   import path may denote two packages.  (Whether this behaviour is
-   disabled by prefixing the import path with "notest:",
+   enabled is tool-specific, and may depend on additional flags.)
-   e.g. "notest:fmt".
 
    Due to current limitations in the type-checker, only the first
    import path of the command line will contribute any tests.
@@ -266,33 +264,40 @@ A '--' argument terminates the list of packages.
 // set of initial packages to be specified; see FromArgsUsage message
 // for details.
 //
-func (conf *Config) FromArgs(args []string) (rest []string, err error) {
+func (conf *Config) FromArgs(args []string, xtest bool) (rest []string, err error) {
-	for len(args) > 0 {
+	for i, arg := range args {
-		arg := args[0]
-		args = args[1:]
 		if arg == "--" {
+			rest = args[i+1:]
+			args = args[:i]
 			break // consume "--" and return the remaining args
 		}
+	}
 
-		if strings.HasSuffix(arg, ".go") {
+	if len(args) > 0 && strings.HasSuffix(args[0], ".go") {
-			// Assume arg is a comma-separated list of *.go files
+		// Assume args is a list of a *.go files
-			// denoting a single ad-hoc package.
+		// denoting a single ad-hoc package.
-			err = conf.CreateFromFilenames("", strings.Split(arg, ",")...)
+		for _, arg := range args {
-		} else {
+			if !strings.HasSuffix(arg, ".go") {
-			// Assume arg is a directory name denoting a
+				return nil, fmt.Errorf("named files must be .go files: %s", arg)
-			// package, perhaps plus an external test
-			// package unless prefixed by "notest:".
-			if path := strings.TrimPrefix(arg, "notest:"); path != arg {
-				conf.Import(path)
-			} else {
-				err = conf.ImportWithTests(path)
 			}
 		}
-		if err != nil {
+		err = conf.CreateFromFilenames("", args...)
-			return nil, err
+	} else {
+		// Assume args are directories each denoting a
+		// package and (perhaps) an external test, iff xtest.
+		for _, arg := range args {
+			if xtest {
+				err = conf.ImportWithTests(arg)
+				if err != nil {
+					break
+				}
+			} else {
+				conf.Import(arg)
+			}
 		}
 	}
-	return args, nil
+
+	return
 }
 
 // CreateFromFilenames is a convenience function that parses the
@@ -371,7 +376,8 @@ func (conf *Config) Import(path string) {
 	if conf.ImportPkgs == nil {
 		conf.ImportPkgs = make(map[string]bool)
 	}
-	conf.ImportPkgs[path] = false // unaugmented source package
+	// Subtle: adds value 'false' unless value is already true.
+	conf.ImportPkgs[path] = conf.ImportPkgs[path] // unaugmented source package
 }
 
 // PathEnclosingInterval returns the PackageInfo and ast.Node that

go/ssa/stdlib_test.go

@@ -53,7 +53,7 @@ func TestStdlib(t *testing.T) {
 	t0 := time.Now()
 
 	var conf loader.Config
-	if _, err := conf.FromArgs(allPackages()); err != nil {
+	if _, err := conf.FromArgs(allPackages(), true); err != nil {
 		t.Errorf("FromArgs failed: %v", err)
 		return
 	}

oracle/oracle.go

@@ -225,7 +225,7 @@ func Query(args []string, mode, pos string, ptalog io.Writer, buildContext *buil
 	conf := loader.Config{Build: buildContext, SourceImports: true}
 
 	// Determine initial packages.
-	args, err := conf.FromArgs(args)
+	args, err := conf.FromArgs(args, true)
 	if err != nil {
 		return nil, err
 	}