Skip to content

Commit

Permalink
cue/load: refactor loader
Browse files Browse the repository at this point in the history 
This step prepares for duplicating much of the
loader logic so that it can be modified for module
loading. It factors out functions that can (probably) be used
by both implementations into `loader_common.go`
and defines an interface type that contains the loader
methods that are actually used by the driving logic outside.

There are still other bits which can/should be refactored,
but this is a start.

The only externally visible behavior that changes as a
result of this CL is that when there's an invalid import path,
the `Instance.Dir` field is not populated because there's
no loader-agnostic way of finding the absolute path of
an invalid import path (and the result was questionable
anyway, so it's hopefully very unlikely that users will be
relying on that particular behavior).

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I8357850fbcca7ef394498df7deae929b28652d68
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/549500
Unity-Result: CUEcueckoo <cueckoo@cuelang.org>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Marcel van Lohuizen <mpvl@gmail.com>
  • Loading branch information
@rogpeppe
rogpeppe committed Feb 22, 2023
1 parent 87b91c9 commit c28d75a
Show file tree
Hide file tree
Showing 9 changed files with 155 additions and 132 deletions.
25 changes: 7 additions & 18 deletions cue/load/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,6 @@ type Config struct {
// If Context is nil, the load cannot be cancelled.
Context *build.Context

loader *loader

// A Module is a collection of packages and instances that are within the
// directory hierarchy rooted at the module root. The module root can be
// marked with a cue.mod file.
Expand Down Expand Up @@ -279,8 +277,6 @@ type Config struct {
Stdin io.Reader

fileSystem

loadFunc build.LoadFunc
}

func (c *Config) stdin() io.Reader {
Expand Down Expand Up @@ -374,6 +370,9 @@ func (c *Config) absDirFromImportPath(pos token.Pos, p importPath) (absDir, name
// with the module property.
// - c.loader != nil
// - c.cache != ""
//
// It does not initialize c.Context, because that requires the
// loader in order to use for build.Loader.
func (c Config) complete() (cfg *Config, err error) {
// Each major CUE release should add a tag here.
// Old tags should not be removed. That is, the cue1.x tag is present
Expand Down Expand Up @@ -412,18 +411,6 @@ func (c Config) complete() (cfg *Config, err error) {
if err := c.loadModule(); err != nil {
return nil, err
}
c.loader = &loader{
cfg: &c,
buildTags: make(map[string]bool),
}
c.loadFunc = c.loader.loadFunc()

if c.Context == nil {
c.Context = build.NewContext(
build.Loader(c.loadFunc),
build.ParseFile(c.loader.cfg.ParseFile),
)
}
return &c, nil
}

Expand Down Expand Up @@ -471,8 +458,10 @@ func (c Config) findRoot(absDir string) string {
}
}

func (c Config) newErrInstance(pos token.Pos, path importPath, err error) *build.Instance {
i := c.newInstance(pos, path)
func (c *Config) newErrInstance(err error) *build.Instance {
i := c.Context.NewInstance("", nil)
i.Root = c.ModuleRoot
i.Module = c.Module
i.Err = errors.Promote(err, "instance")
return i
}
82 changes: 38 additions & 44 deletions cue/load/import.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (l *loader) importPkg(pos token.Pos, p *build.Instance) []*build.Instance {
return retErr(err)
}

fp := newFileProcessor(cfg, p)
fp := newFileProcessor(cfg, p, l.tagger)

if p.PkgName == "" {
if l.cfg.Package == "*" {
Expand Down Expand Up @@ -209,60 +209,54 @@ func (l *loader) importPkg(pos token.Pos, p *build.Instance) []*build.Instance {
return all
}

// loadFunc creates a LoadFunc that can be used to create new build.Instances.
func (l *loader) loadFunc() build.LoadFunc {

return func(pos token.Pos, path string) *build.Instance {
cfg := l.cfg

impPath := importPath(path)
if isLocalImport(path) {
return cfg.newErrInstance(pos, impPath,
errors.Newf(pos, "relative import paths not allowed (%q)", path))
}
// _loadFunc is the method used for the value of l.loadFunc.
func (l *loader) _loadFunc(pos token.Pos, path string) *build.Instance {
impPath := importPath(path)
if isLocalImport(path) {
return l.cfg.newErrInstance(errors.Newf(pos, "relative import paths not allowed (%q)", path))
}

// is it a builtin?
if strings.IndexByte(strings.Split(path, "/")[0], '.') == -1 {
if l.cfg.StdRoot != "" {
p := cfg.newInstance(pos, impPath)
_ = l.importPkg(pos, p)
return p
}
return nil
// is it a builtin?
if strings.IndexByte(strings.Split(path, "/")[0], '.') == -1 {
if l.cfg.StdRoot != "" {
p := l.newInstance(pos, impPath)
_ = l.importPkg(pos, p)
return p
}

p := cfg.newInstance(pos, impPath)
_ = l.importPkg(pos, p)
return p
return nil
}

p := l.newInstance(pos, impPath)
_ = l.importPkg(pos, p)
return p
}

// newRelInstance returns a build instance from the given
// relative import path.
func (c *Config) newRelInstance(pos token.Pos, path, pkgName string) *build.Instance {
func (l *loader) newRelInstance(pos token.Pos, path, pkgName string) *build.Instance {
if !isLocalImport(path) {
panic(fmt.Errorf("non-relative import path %q passed to newRelInstance", path))
}
fs := c.fileSystem
fs := l.cfg.fileSystem

var err errors.Error
dir := path

p := c.Context.NewInstance(path, c.loadFunc)
p := l.cfg.Context.NewInstance(path, l.loadFunc)
p.PkgName = pkgName
p.DisplayPath = filepath.ToSlash(path)
// p.ImportPath = string(dir) // compute unique ID.
p.Root = c.ModuleRoot
p.Module = c.Module
p.Root = l.cfg.ModuleRoot
p.Module = l.cfg.Module

dir = filepath.Join(c.Dir, filepath.FromSlash(path))
dir = filepath.Join(l.cfg.Dir, filepath.FromSlash(path))

if path != cleanImport(path) {
err = errors.Append(err, c.loader.errPkgf(nil,
err = errors.Append(err, l.errPkgf(nil,
"non-canonical import path: %q should be %q", path, pathpkg.Clean(path)))
}

if importPath, e := c.importPathFromAbsDir(fsPath(dir), path); e != nil {
if importPath, e := l.importPathFromAbsDir(fsPath(dir), path); e != nil {
// Detect later to keep error messages consistent.
} else {
p.ImportPath = string(importPath)
Expand All @@ -282,19 +276,19 @@ func (c *Config) newRelInstance(pos token.Pos, path, pkgName string) *build.Inst
return p
}

func (c *Config) importPathFromAbsDir(absDir fsPath, key string) (importPath, errors.Error) {
if c.ModuleRoot == "" {
func (l *loader) importPathFromAbsDir(absDir fsPath, key string) (importPath, errors.Error) {
if l.cfg.ModuleRoot == "" {
return "", errors.Newf(token.NoPos,
"cannot determine import path for %q (root undefined)", key)
}

dir := filepath.Clean(string(absDir))
if !strings.HasPrefix(dir, c.ModuleRoot) {
if !strings.HasPrefix(dir, l.cfg.ModuleRoot) {
return "", errors.Newf(token.NoPos,
"cannot determine import path for %q (dir outside of root)", key)
}

pkg := filepath.ToSlash(dir[len(c.ModuleRoot):])
pkg := filepath.ToSlash(dir[len(l.cfg.ModuleRoot):])
switch {
case strings.HasPrefix(pkg, "/cue.mod/"):
pkg = pkg[len("/cue.mod/"):]
Expand All @@ -311,14 +305,14 @@ func (c *Config) importPathFromAbsDir(absDir fsPath, key string) (importPath, er
"invalid package %q (root of %s)", key, pkgDir)
}

case c.Module == "":
case l.cfg.Module == "":
return "", errors.Newf(token.NoPos,
"cannot determine import path for %q (no module)", key)
default:
pkg = c.Module + pkg
pkg = l.cfg.Module + pkg
}

name := c.Package
name := l.cfg.Package
switch name {
case "_", "*":
name = ""
Expand All @@ -327,15 +321,15 @@ func (c *Config) importPathFromAbsDir(absDir fsPath, key string) (importPath, er
return addImportQualifier(importPath(pkg), name)
}

func (c *Config) newInstance(pos token.Pos, p importPath) *build.Instance {
dir, name, err := c.absDirFromImportPath(pos, p)
i := c.Context.NewInstance(dir, c.loadFunc)
func (l *loader) newInstance(pos token.Pos, p importPath) *build.Instance {
dir, name, err := l.cfg.absDirFromImportPath(pos, p)
i := l.cfg.Context.NewInstance(dir, l.loadFunc)
i.Dir = dir
i.PkgName = name
i.DisplayPath = string(p)
i.ImportPath = string(p)
i.Root = c.ModuleRoot
i.Module = c.Module
i.Root = l.cfg.ModuleRoot
i.Module = l.cfg.Module
i.Err = errors.Append(i.Err, err)

return i
Expand Down
4 changes: 2 additions & 2 deletions cue/load/import_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ func testMod(dir string) string {
func getInst(pkg, cwd string) (*build.Instance, error) {
c, _ := (&Config{Dir: cwd}).complete()
l := loader{cfg: c}
inst := c.newRelInstance(token.NoPos, pkg, c.Package)
inst := l.newRelInstance(token.NoPos, pkg, c.Package)
p := l.importPkg(token.NoPos, inst)[0]
return p, p.Err
}

func TestEmptyImport(t *testing.T) {
c, _ := (&Config{}).complete()
l := loader{cfg: c}
inst := c.newInstance(token.NoPos, "")
inst := l.newInstance(token.NoPos, "")
p := l.importPkg(token.NoPos, inst)[0]
if p.Err == nil {
t.Fatal(`Import("") returned nil error.`)
Expand Down
24 changes: 15 additions & 9 deletions cue/load/instances.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ package load
import (
"cuelang.org/go/cue/ast"
"cuelang.org/go/cue/build"
"cuelang.org/go/cue/token"
"cuelang.org/go/internal/filetypes"

// Trigger the unconditional loading of all core builtin packages if load
Expand All @@ -43,11 +42,18 @@ func Instances(args []string, c *Config) []*build.Instance {
}
newC, err := c.complete()
if err != nil {
return []*build.Instance{c.newErrInstance(token.NoPos, "", err)}
return []*build.Instance{c.newErrInstance(err)}
}
c = newC
tg := newTagger(c)
l := newLoader(c, tg)

l := c.loader
if c.Context == nil {
c.Context = build.NewContext(
build.Loader(l.buildLoadFunc()),
build.ParseFile(c.ParseFile),
)
}

// TODO: require packages to be placed before files. At some point this
// could be relaxed.
Expand All @@ -60,7 +66,7 @@ func Instances(args []string, c *Config) []*build.Instance {
if len(args) == 0 || i > 0 {
for _, m := range l.importPaths(args[:i]) {
if m.Err != nil {
inst := c.newErrInstance(token.NoPos, "", m.Err)
inst := c.newErrInstance(m.Err)
a = append(a, inst)
continue
}
Expand All @@ -71,7 +77,7 @@ func Instances(args []string, c *Config) []*build.Instance {
if args = args[i:]; len(args) > 0 {
files, err := filetypes.ParseArgs(args)
if err != nil {
return []*build.Instance{c.newErrInstance(token.NoPos, "", err)}
return []*build.Instance{c.newErrInstance(err)}
}
a = append(a, l.cueFilesPackage(files))
}
Expand All @@ -81,27 +87,27 @@ func Instances(args []string, c *Config) []*build.Instance {
if err != nil {
p.ReportError(err)
}
l.tags = append(l.tags, tags...)
tg.tags = append(tg.tags, tags...)
}

// TODO(api): have API call that returns an error which is the aggregate
// of all build errors. Certain errors, like these, hold across builds.
if err := injectTags(c.Tags, l); err != nil {
if err := tg.injectTags(c.Tags); err != nil {
for _, p := range a {
p.ReportError(err)
}
return a
}

if l.replacements == nil {
if tg.replacements == nil {
return a
}

for _, p := range a {
for _, f := range p.Files {
ast.Walk(f, nil, func(n ast.Node) {
if ident, ok := n.(*ast.Ident); ok {
if v, ok := l.replacements[ident.Node]; ok {
if v, ok := tg.replacements[ident.Node]; ok {
ident.Node = v
}
}
Expand Down
Loading

0 comments on commit c28d75a

Please sign in to comment.