feat: node refactor (#1316)

* refactor: node reader interface

* refactor: rewrite Taskfile() as anon recursive func

* chore: NewNodeFromIncludedTaskfile

* chore: changelog

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+- Prep work for remote Taskfiles (#1316 by @pd93).
+
 ## v3.29.1 - 2023-08-26
 
 - Update to Go 1.21 (bump minimum version to 1.20) (#1302 by @pd93)
@@ -9,8 +13,8 @@
 - Fix bug in usage of special variables like `{{.USER_WORKING_DIR}}` in
   combination with `includes` (#1046, #1205, #1250, #1293, #1312, #1274 by
   @andarto, #1309 by @andreynering).
-- Fix bug on `--status` flag. Running this flag should not have side-effects:
+- Fix bug on `--status` flag. Running this flag should not have side-effects: it
-  it should not update the checksum on `.task`, only report its status (#1305,
+  should not update the checksum on `.task`, only report its status (#1305,
   #1307 by @visciang, #1313 by @andreynering).
 
 ## v3.28.0 - 2023-07-24

errors/errors_taskfile.go

@@ -7,7 +7,7 @@ import (
 // TaskfileNotFoundError is returned when no appropriate Taskfile is found when
 // searching the filesystem.
 type TaskfileNotFoundError struct {
-	Dir  string
+	URI  string
 	Walk bool
 }
 
@@ -16,7 +16,7 @@ func (err TaskfileNotFoundError) Error() string {
 	if err.Walk {
 		walkText = " (or any of the parent directories)"
 	}
-	return fmt.Sprintf(`task: No Taskfile found in "%s"%s. Use "task --init" to create a new one`, err.Dir, walkText)
+	return fmt.Sprintf(`task: No Taskfile found at "%s"%s`, err.URI, walkText)
 }
 
 func (err TaskfileNotFoundError) Code() int {
@@ -38,12 +38,12 @@ func (err TaskfileAlreadyExistsError) Code() int {
 // TaskfileInvalidError is returned when the Taskfile contains syntax errors or
 // cannot be parsed for some reason.
 type TaskfileInvalidError struct {
-	FilePath string
+	URI string
-	Err      error
+	Err error
 }
 
 func (err TaskfileInvalidError) Error() string {
-	return fmt.Sprintf("task: Failed to parse %s:\n%v", err.FilePath, err.Err)
+	return fmt.Sprintf("task: Failed to parse %s:\n%v", err.URI, err.Err)
 }
 
 func (err TaskfileInvalidError) Code() int {

setup.go

@@ -76,13 +76,15 @@ func (e *Executor) setCurrentDir() error {
 
 func (e *Executor) readTaskfile() error {
 	var err error
-	e.Taskfile, e.Dir, err = read.Taskfile(&read.ReaderNode{
+	e.Taskfile, err = read.Taskfile(&read.FileNode{
 		Dir:        e.Dir,
 		Entrypoint: e.Entrypoint,
-		Parent:     nil,
-		Optional:   false,
 	})
-	return err
+	if err != nil {
+		return err
+	}
+	e.Dir = filepath.Dir(e.Taskfile.Location)
+	return nil
 }
 
 func (e *Executor) setupFuzzyModel() {

taskfile/read/node.go

@@ -0,0 +1,35 @@
+package read
+
+import (
+	"strings"
+
+	"github.com/go-task/task/v3/taskfile"
+)
+
+type Node interface {
+	Read() (*taskfile.Taskfile, error)
+	Parent() Node
+	Optional() bool
+	Location() string
+}
+
+func NewNodeFromIncludedTaskfile(parent Node, includedTaskfile taskfile.IncludedTaskfile) (Node, error) {
+	switch getScheme(includedTaskfile.Taskfile) {
+	// TODO: Add support for other schemes.
+	// If no other scheme matches, we assume it's a file.
+	// This also allows users to explicitly set a file:// scheme.
+	default:
+		path, err := includedTaskfile.FullTaskfilePath()
+		if err != nil {
+			return nil, err
+		}
+		return NewFileNode(parent, path, includedTaskfile.Optional)
+	}
+}
+
+func getScheme(uri string) string {
+	if i := strings.Index(uri, "://"); i != -1 {
+		return uri[:i]
+	}
+	return ""
+}

taskfile/read/node_base.go

@@ -0,0 +1,18 @@
+package read
+
+// BaseNode is a generic node that implements the Parent() and Optional()
+// methods of the NodeReader interface. It does not implement the Read() method
+// and it designed to be embedded in other node types so that this boilerplate
+// code does not need to be repeated.
+type BaseNode struct {
+	parent   Node
+	optional bool
+}
+
+func (node *BaseNode) Parent() Node {
+	return node.parent
+}
+
+func (node *BaseNode) Optional() bool {
+	return node.optional
+}

taskfile/read/node_file.go

@@ -0,0 +1,70 @@
+package read
+
+import (
+	"os"
+	"path/filepath"
+
+	"gopkg.in/yaml.v3"
+
+	"github.com/go-task/task/v3/errors"
+	"github.com/go-task/task/v3/internal/filepathext"
+	"github.com/go-task/task/v3/taskfile"
+)
+
+// A FileNode is a node that reads a taskfile from the local filesystem.
+type FileNode struct {
+	BaseNode
+	Dir        string
+	Entrypoint string
+}
+
+func NewFileNode(parent Node, path string, optional bool) (*FileNode, error) {
+	path, err := exists(path)
+	if err != nil {
+		return nil, err
+	}
+
+	return &FileNode{
+		BaseNode: BaseNode{
+			parent:   parent,
+			optional: optional,
+		},
+		Dir:        filepath.Dir(path),
+		Entrypoint: filepath.Base(path),
+	}, nil
+}
+
+func (node *FileNode) Location() string {
+	return filepathext.SmartJoin(node.Dir, node.Entrypoint)
+}
+
+func (node *FileNode) Read() (*taskfile.Taskfile, error) {
+	if node.Dir == "" {
+		d, err := os.Getwd()
+		if err != nil {
+			return nil, err
+		}
+		node.Dir = d
+	}
+
+	path, err := existsWalk(node.Location())
+	if err != nil {
+		return nil, err
+	}
+	node.Dir = filepath.Dir(path)
+	node.Entrypoint = filepath.Base(path)
+
+	f, err := os.Open(path)
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+
+	var t taskfile.Taskfile
+	if err := yaml.NewDecoder(f).Decode(&t); err != nil {
+		return nil, &errors.TaskfileInvalidError{URI: filepathext.TryAbsToRel(path), Err: err}
+	}
+
+	t.Location = path
+	return &t, nil
+}

taskfile/read/taskfile.go

@@ -6,8 +6,6 @@ import (
 	"path/filepath"
 	"runtime"
 
-	"gopkg.in/yaml.v3"
-
 	"github.com/go-task/task/v3/errors"
 	"github.com/go-task/task/v3/internal/filepathext"
 	"github.com/go-task/task/v3/internal/sysinfo"
@@ -31,193 +29,158 @@ var (
 	}
 )
 
-type ReaderNode struct {
-	Dir        string
-	Entrypoint string
-	Optional   bool
-	Parent     *ReaderNode
-}
-
 // Taskfile reads a Taskfile for a given directory
 // Uses current dir when dir is left empty. Uses Taskfile.yml
 // or Taskfile.yaml when entrypoint is left empty
-func Taskfile(readerNode *ReaderNode) (*taskfile.Taskfile, string, error) {
+func Taskfile(node Node) (*taskfile.Taskfile, error) {
-	if readerNode.Dir == "" {
+	var _taskfile func(Node) (*taskfile.Taskfile, error)
-		d, err := os.Getwd()
+	_taskfile = func(node Node) (*taskfile.Taskfile, error) {
+		t, err := node.Read()
 		if err != nil {
-			return nil, "", err
+			return nil, err
-		}
-		readerNode.Dir = d
-	}
-
-	path, err := existsWalk(filepathext.SmartJoin(readerNode.Dir, readerNode.Entrypoint))
-	if err != nil {
-		return nil, "", err
-	}
-	readerNode.Dir = filepath.Dir(path)
-	readerNode.Entrypoint = filepath.Base(path)
-
-	t, err := readTaskfile(path)
-	if err != nil {
-		return nil, "", err
-	}
-
-	// Annotate any included Taskfile reference with a base directory for resolving relative paths
-	_ = t.Includes.Range(func(key string, includedFile taskfile.IncludedTaskfile) error {
-		// Set the base directory for resolving relative paths, but only if not already set
-		if includedFile.BaseDir == "" {
-			includedFile.BaseDir = readerNode.Dir
-			t.Includes.Set(key, includedFile)
-		}
-		return nil
-	})
-
-	err = t.Includes.Range(func(namespace string, includedTask taskfile.IncludedTaskfile) error {
-		if t.Version.Compare(taskfile.V3) >= 0 {
-			tr := templater.Templater{Vars: t.Vars, RemoveNoValue: true}
-			includedTask = taskfile.IncludedTaskfile{
-				Taskfile:       tr.Replace(includedTask.Taskfile),
-				Dir:            tr.Replace(includedTask.Dir),
-				Optional:       includedTask.Optional,
-				Internal:       includedTask.Internal,
-				Aliases:        includedTask.Aliases,
-				AdvancedImport: includedTask.AdvancedImport,
-				Vars:           includedTask.Vars,
-				BaseDir:        includedTask.BaseDir,
-			}
-			if err := tr.Err(); err != nil {
-				return err
-			}
 		}
 
-		path, err := includedTask.FullTaskfilePath()
+		// Annotate any included Taskfile reference with a base directory for resolving relative paths
-		if err != nil {
+		if node, isFileNode := node.(*FileNode); isFileNode {
-			return err
+			_ = t.Includes.Range(func(key string, includedFile taskfile.IncludedTaskfile) error {
-		}
+				// Set the base directory for resolving relative paths, but only if not already set
-
+				if includedFile.BaseDir == "" {
-		path, err = exists(path)
+					includedFile.BaseDir = node.Dir
-		if err != nil {
+					t.Includes.Set(key, includedFile)
-			if includedTask.Optional {
+				}
-				return nil
+				return nil
-			}
+			})
-			return err
+		}
-		}
+
-
+		err = t.Includes.Range(func(namespace string, includedTask taskfile.IncludedTaskfile) error {
-		includeReaderNode := &ReaderNode{
+			if t.Version.Compare(taskfile.V3) >= 0 {
-			Dir:        filepath.Dir(path),
+				tr := templater.Templater{Vars: t.Vars, RemoveNoValue: true}
-			Entrypoint: filepath.Base(path),
+				includedTask = taskfile.IncludedTaskfile{
-			Parent:     readerNode,
+					Taskfile:       tr.Replace(includedTask.Taskfile),
-			Optional:   includedTask.Optional,
+					Dir:            tr.Replace(includedTask.Dir),
-		}
+					Optional:       includedTask.Optional,
-
+					Internal:       includedTask.Internal,
-		if err := checkCircularIncludes(includeReaderNode); err != nil {
+					Aliases:        includedTask.Aliases,
-			return err
+					AdvancedImport: includedTask.AdvancedImport,
-		}
+					Vars:           includedTask.Vars,
-
+					BaseDir:        includedTask.BaseDir,
-		includedTaskfile, _, err := Taskfile(includeReaderNode)
+				}
-		if err != nil {
+				if err := tr.Err(); err != nil {
-			if includedTask.Optional {
+					return err
-				return nil
-			}
-			return err
-		}
-
-		if t.Version.Compare(taskfile.V3) >= 0 && len(includedTaskfile.Dotenv) > 0 {
-			return ErrIncludedTaskfilesCantHaveDotenvs
-		}
-
-		if includedTask.AdvancedImport {
-			dir, err := includedTask.FullDirPath()
-			if err != nil {
-				return err
-			}
-
-			// nolint: errcheck
-			includedTaskfile.Vars.Range(func(k string, v taskfile.Var) error {
-				o := v
-				o.Dir = dir
-				includedTaskfile.Vars.Set(k, o)
-				return nil
-			})
-			// nolint: errcheck
-			includedTaskfile.Env.Range(func(k string, v taskfile.Var) error {
-				o := v
-				o.Dir = dir
-				includedTaskfile.Env.Set(k, o)
-				return nil
-			})
-
-			for _, task := range includedTaskfile.Tasks.Values() {
-				task.Dir = filepathext.SmartJoin(dir, task.Dir)
-				if task.IncludeVars == nil {
-					task.IncludeVars = &taskfile.Vars{}
 				}
-				task.IncludeVars.Merge(includedTask.Vars)
-				task.IncludedTaskfileVars = includedTaskfile.Vars
-				task.IncludedTaskfile = &includedTask
 			}
-		}
 
-		if err = taskfile.Merge(t, includedTaskfile, &includedTask, namespace); err != nil {
+			includeReaderNode, err := NewNodeFromIncludedTaskfile(node, includedTask)
-			return err
-		}
-
-		if includedTaskfile.Tasks.Get("default") != nil && t.Tasks.Get(namespace) == nil {
-			defaultTaskName := fmt.Sprintf("%s:default", namespace)
-			task := t.Tasks.Get(defaultTaskName)
-			task.Aliases = append(task.Aliases, namespace)
-			task.Aliases = append(task.Aliases, includedTask.Aliases...)
-			t.Tasks.Set(defaultTaskName, task)
-		}
-
-		return nil
-	})
-	if err != nil {
-		return nil, "", err
-	}
-
-	if t.Version.Compare(taskfile.V3) < 0 {
-		path = filepathext.SmartJoin(readerNode.Dir, fmt.Sprintf("Taskfile_%s.yml", runtime.GOOS))
-		if _, err = os.Stat(path); err == nil {
-			osTaskfile, err := readTaskfile(path)
 			if err != nil {
-				return nil, "", err
+				if includedTask.Optional {
+					return nil
+				}
+				return err
 			}
-			if err = taskfile.Merge(t, osTaskfile, nil); err != nil {
+
-				return nil, "", err
+			if err := checkCircularIncludes(includeReaderNode); err != nil {
+				return err
+			}
+
+			includedTaskfile, err := _taskfile(includeReaderNode)
+			if err != nil {
+				if includedTask.Optional {
+					return nil
+				}
+				return err
+			}
+
+			if t.Version.Compare(taskfile.V3) >= 0 && len(includedTaskfile.Dotenv) > 0 {
+				return ErrIncludedTaskfilesCantHaveDotenvs
+			}
+
+			if includedTask.AdvancedImport {
+				dir, err := includedTask.FullDirPath()
+				if err != nil {
+					return err
+				}
+
+				// nolint: errcheck
+				includedTaskfile.Vars.Range(func(k string, v taskfile.Var) error {
+					o := v
+					o.Dir = dir
+					includedTaskfile.Vars.Set(k, o)
+					return nil
+				})
+				// nolint: errcheck
+				includedTaskfile.Env.Range(func(k string, v taskfile.Var) error {
+					o := v
+					o.Dir = dir
+					includedTaskfile.Env.Set(k, o)
+					return nil
+				})
+
+				for _, task := range includedTaskfile.Tasks.Values() {
+					task.Dir = filepathext.SmartJoin(dir, task.Dir)
+					if task.IncludeVars == nil {
+						task.IncludeVars = &taskfile.Vars{}
+					}
+					task.IncludeVars.Merge(includedTask.Vars)
+					task.IncludedTaskfileVars = includedTaskfile.Vars
+					task.IncludedTaskfile = &includedTask
+				}
+			}
+
+			if err = taskfile.Merge(t, includedTaskfile, &includedTask, namespace); err != nil {
+				return err
+			}
+
+			if includedTaskfile.Tasks.Get("default") != nil && t.Tasks.Get(namespace) == nil {
+				defaultTaskName := fmt.Sprintf("%s:default", namespace)
+				task := t.Tasks.Get(defaultTaskName)
+				task.Aliases = append(task.Aliases, namespace)
+				task.Aliases = append(task.Aliases, includedTask.Aliases...)
+				t.Tasks.Set(defaultTaskName, task)
+			}
+
+			return nil
+		})
+		if err != nil {
+			return nil, err
+		}
+
+		if t.Version.Compare(taskfile.V3) < 0 {
+			if node, isFileNode := node.(*FileNode); isFileNode {
+				path := filepathext.SmartJoin(node.Dir, fmt.Sprintf("Taskfile_%s.yml", runtime.GOOS))
+				if _, err = os.Stat(path); err == nil {
+					osNode := &FileNode{
+						BaseNode: BaseNode{
+							parent:   node,
+							optional: false,
+						},
+						Entrypoint: path,
+						Dir:        node.Dir,
+					}
+					osTaskfile, err := osNode.Read()
+					if err != nil {
+						return nil, err
+					}
+					if err = taskfile.Merge(t, osTaskfile, nil); err != nil {
+						return nil, err
+					}
+				}
 			}
 		}
-	}
 
-	// Set the location of the Taskfile
+		for _, task := range t.Tasks.Values() {
-	t.Location = path
+			// If the task is not defined, create a new one
-
+			if task == nil {
-	for _, task := range t.Tasks.Values() {
+				task = &taskfile.Task{}
-		// If the task is not defined, create a new one
+			}
-		if task == nil {
+			// Set the location of the taskfile for each task
-			task = &taskfile.Task{}
+			if task.Location.Taskfile == "" {
+				task.Location.Taskfile = t.Location
+			}
 		}
-		// Set the location of the taskfile for each task
-		if task.Location.Taskfile == "" {
-			task.Location.Taskfile = path
-		}
-	}
 
-	return t, readerNode.Dir, nil
+		return t, nil
-}
-
-func readTaskfile(file string) (*taskfile.Taskfile, error) {
-	f, err := os.Open(file)
-	if err != nil {
-		return nil, err
 	}
-	defer f.Close()
+	return _taskfile(node)
-
-	var t taskfile.Taskfile
-	if err := yaml.NewDecoder(f).Decode(&t); err != nil {
-		return nil, &errors.TaskfileInvalidError{FilePath: filepathext.TryAbsToRel(file), Err: err}
-	}
-	return &t, nil
 }
 
 func exists(path string) (string, error) {
@@ -236,7 +199,7 @@ func exists(path string) (string, error) {
 		}
 	}
 
-	return "", errors.TaskfileNotFoundError{Dir: path, Walk: false}
+	return "", errors.TaskfileNotFoundError{URI: path, Walk: false}
 }
 
 func existsWalk(path string) (string, error) {
@@ -261,7 +224,7 @@ func existsWalk(path string) (string, error) {
 		// Error if we reached the root directory and still haven't found a file
 		// OR if the user id of the directory changes
 		if path == parentPath || (parentOwner != owner) {
-			return "", errors.TaskfileNotFoundError{Dir: origPath, Walk: false}
+			return "", errors.TaskfileNotFoundError{URI: origPath, Walk: false}
 		}
 
 		owner = parentOwner
@@ -269,22 +232,22 @@ func existsWalk(path string) (string, error) {
 	}
 }
 
-func checkCircularIncludes(node *ReaderNode) error {
+func checkCircularIncludes(node Node) error {
 	if node == nil {
 		return errors.New("task: failed to check for include cycle: node was nil")
 	}
-	if node.Parent == nil {
+	if node.Parent() == nil {
 		return errors.New("task: failed to check for include cycle: node.Parent was nil")
 	}
 	curNode := node
-	basePath := filepathext.SmartJoin(node.Dir, node.Entrypoint)
+	location := node.Location()
-	for curNode.Parent != nil {
+	for curNode.Parent() != nil {
-		curNode = curNode.Parent
+		curNode = curNode.Parent()
-		curPath := filepathext.SmartJoin(curNode.Dir, curNode.Entrypoint)
+		curLocation := curNode.Location()
-		if curPath == basePath {
+		if curLocation == location {
 			return fmt.Errorf("task: include cycle detected between %s <--> %s",
-				curPath,
+				curLocation,
-				filepathext.SmartJoin(node.Parent.Dir, node.Parent.Entrypoint),
+				node.Parent().Location(),
 			)
 		}
 	}