cmd/cue/cmd: simplify parseArgs in preparation of proto support

The parseArgs code had some pretty complicated logic.
This now separates out the cases for normal instances
and user-selected files to avoid at least one level of
comingling.

The goal of this refactoring is to ultimately allow schema
and value files to be separated early on, which, in turn,
will allow schema values to be computed early.
Such schema will then allow value formats to be interpreted
by the type of which they will ultimately land in CUE.

This is necessary to parse protobuf, which cannot be parsed
without a schema.

Issue #5

Change-Id: I2046948a8cf9bc5425ed437247dc89416d0fff76
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/9304
Reviewed-by: Marcel van Lohuizen <[email protected]>
Reviewed-by: CUE cueckoo <[email protected]>

Author: mpvl

cmd/cue/cmd/common.go

@@ -139,6 +139,12 @@ type buildPlan struct {
 	expressions []ast.Expr // only evaluate these expressions within results
 	schema      ast.Expr   // selects schema in instance for orphaned values
 
+	// orphan placement flags.
+	perFile    bool
+	useList    bool
+	path       []string
+	useContext bool
+
 	// outFile defines the file to output to. Default is CUE stdout.
 	outFile *build.File
 
@@ -388,41 +394,41 @@ func parseArgs(cmd *Command, args []string, cfg *config) (p *buildPlan, err erro
 		return nil, errors.Newf(token.NoPos, "invalid args")
 	}
 
+	if err := p.parsePlacementFlags(); err != nil {
+		return nil, err
+	}
+
 	for _, b := range builds {
 		if b.Err != nil {
 			return nil, b.Err
 		}
-		var ok bool
-		if b.User || p.importing {
-			ok, err = p.placeOrphans(b)
-			if err != nil {
-				return nil, err
+		switch {
+		case !b.User:
+			if p.importing {
+				if err = p.placeOrphans(b); err != nil {
+					return nil, err
+				}
 			}
-		}
-		if !b.User {
 			p.insts = append(p.insts, b)
-			continue
-		}
-		addedUser := false
-		if len(b.BuildFiles) > 0 {
-			addedUser = true
-			p.insts = append(p.insts, b)
-		}
-		if ok {
-			continue
-		}
-
-		if len(b.OrphanedFiles) == 0 {
-			continue
-		}
 
-		if p.orphanInstance != nil {
+		case p.orphanInstance != nil:
 			return nil, errors.Newf(token.NoPos,
 				"builds contain two file packages")
+
+		default:
+			p.orphanInstance = b
 		}
-		p.orphanInstance = b
-		p.encConfig.Stream = true
+	}
 
+	switch b := p.orphanInstance; {
+	case b == nil:
+	case p.usePlacement() || p.importing:
+		p.insts = append(p.insts, b)
+		if err = p.placeOrphans(b); err != nil {
+			return nil, err
+		}
+
+	default:
 		for _, f := range b.OrphanedFiles {
 			switch f.Encoding {
 			case build.Protobuf, build.YAML, build.JSON, build.Text:
@@ -466,7 +472,7 @@ func parseArgs(cmd *Command, args []string, cfg *config) (p *buildPlan, err erro
 			}
 		}
 
-		if !addedUser && len(b.Files) > 0 {
+		if len(b.Files) > 0 {
 			p.insts = append(p.insts, b)
 		} else if len(p.orphaned) == 0 {
 			// Instance with only a single build: just print the file.

cmd/cue/cmd/orphans.go

@@ -33,32 +33,40 @@ import (
 
 // This file contains logic for placing orphan files within a CUE namespace.
 
-func (b *buildPlan) placeOrphans(i *build.Instance) (ok bool, err error) {
-	var (
-		perFile    = flagFiles.Bool(b.cmd)
-		useList    = flagList.Bool(b.cmd)
-		path       = flagPath.StringArray(b.cmd)
-		useContext = flagWithContext.Bool(b.cmd)
-	)
-	if !b.importing && !perFile && !useList && len(path) == 0 {
-		if useContext {
-			return false, fmt.Errorf(
+func (b *buildPlan) usePlacement() bool {
+	return b.perFile || b.useList || len(b.path) > 0
+}
+
+func (b *buildPlan) parsePlacementFlags() error {
+	cmd := b.cmd
+	b.perFile = flagFiles.Bool(cmd)
+	b.useList = flagList.Bool(cmd)
+	b.path = flagPath.StringArray(cmd)
+	b.useContext = flagWithContext.Bool(cmd)
+
+	if !b.importing && !b.perFile && !b.useList && len(b.path) == 0 {
+		if b.useContext {
+			return fmt.Errorf(
 				"flag %q must be used with at least one of flag %q, %q, or %q",
 				flagWithContext, flagPath, flagList, flagFiles,
 			)
 		}
-		// TODO: should we remove this optimization? This is really added as a
-		// conversion safety.
-		if len(i.OrphanedFiles)+len(i.BuildFiles) <= 1 || b.cfg.noMerge {
-			return false, err
-		}
+	} else if b.schema != nil {
+		return fmt.Errorf(
+			"cannot combine --%s flag with flag %q, %q, or %q",
+			flagSchema, flagPath, flagList, flagFiles,
+		)
 	}
+	return nil
+}
+
+func (b *buildPlan) placeOrphans(i *build.Instance) error {
 
 	pkg := b.encConfig.PkgName
 	if pkg == "" {
 		pkg = i.PkgName
 	} else if pkg != "" && i.PkgName != "" && i.PkgName != pkg && !flagForce.Bool(b.cmd) {
-		return false, fmt.Errorf(
+		return fmt.Errorf(
 			"%q flag clashes with existing package name (%s vs %s)",
 			flagPackage, pkg, i.PkgName,
 		)
@@ -68,7 +76,7 @@ func (b *buildPlan) placeOrphans(i *build.Instance) (ok bool, err error) {
 
 	re, err := regexp.Compile(b.cfg.fileFilter)
 	if err != nil {
-		return false, err
+		return err
 	}
 
 	for _, f := range i.OrphanedFiles {
@@ -95,14 +103,14 @@ func (b *buildPlan) placeOrphans(i *build.Instance) (ok bool, err error) {
 			}
 		}
 		if err := d.Err(); err != nil {
-			return false, err
+			return err
 		}
 
-		if perFile {
+		if b.perFile {
 			for i, obj := range objs {
 				f, err := placeOrphans(b.cmd, d.Filename(), pkg, obj)
 				if err != nil {
-					return false, err
+					return err
 				}
 				f.Filename = newName(d.Filename(), i)
 				files = append(files, f)
@@ -112,13 +120,13 @@ func (b *buildPlan) placeOrphans(i *build.Instance) (ok bool, err error) {
 		// TODO: consider getting rid of this requirement. It is important that
 		// import will catch conflicts ahead of time then, though, and report
 		// this messages as a possible solution if there are conflicts.
-		if b.importing && len(objs) > 1 && len(path) == 0 && !useList {
-			return false, fmt.Errorf(
+		if b.importing && len(objs) > 1 && len(b.path) == 0 && !b.useList {
+			return fmt.Errorf(
 				"%s, %s, or %s flag needed to handle multiple objects in file %s",
 				flagPath, flagList, flagFiles, shortFile(i.Root, f))
 		}
 
-		if !useList && len(path) == 0 && !useContext {
+		if !b.useList && len(b.path) == 0 && !b.useContext {
 			for _, f := range objs {
 				if pkg := c.PkgName; pkg != "" {
 					internal.SetPackage(f, pkg, false)
@@ -129,7 +137,7 @@ func (b *buildPlan) placeOrphans(i *build.Instance) (ok bool, err error) {
 			// TODO: handle imports correctly, i.e. for proto.
 			f, err := placeOrphans(b.cmd, d.Filename(), pkg, objs...)
 			if err != nil {
-				return false, err
+				return err
 			}
 			f.Filename = newName(d.Filename(), 0)
 			files = append(files, f)
@@ -139,15 +147,15 @@ func (b *buildPlan) placeOrphans(i *build.Instance) (ok bool, err error) {
 	b.imported = append(b.imported, files...)
 	for _, f := range files {
 		if err := i.AddSyntax(f); err != nil {
-			return false, err
+			return err
 		}
 		i.BuildFiles = append(i.BuildFiles, &build.File{
 			Filename: f.Filename,
 			Encoding: build.CUE,
 			Source:   f,
 		})
 	}
-	return true, nil
+	return nil
 }
 
 func placeOrphans(cmd *Command, filename, pkg string, objs ...*ast.File) (*ast.File, error) {