Skip to content
This repository has been archived by the owner on Nov 18, 2021. It is now read-only.

Commit

Permalink
cmd/cue/cmd: simplify parseArgs in preparation of proto support
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mpvl committed Apr 9, 2021
1 parent 14c5f24 commit add266a
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 51 deletions.
56 changes: 31 additions & 25 deletions cmd/cue/cmd/common.go
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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.
Expand Down
60 changes: 34 additions & 26 deletions cmd/cue/cmd/orphans.go
Expand Up @@ -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,
)
Expand All @@ -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 {
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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) {
Expand Down

0 comments on commit add266a

Please sign in to comment.