From 06031efc091b6a585e9fe7f122c30c9796a0506a Mon Sep 17 00:00:00 2001
From: Andrey Nering <andrey.nering@gmail.com>
Date: Sat, 8 Jul 2017 10:13:56 -0300
Subject: [PATCH 1/2] cyclic: refactor to return error instead

---
 cyclic.go      | 22 +++++++++++-----------
 cyclic_test.go | 10 ++++------
 errors.go      |  4 ++--
 task.go        |  4 ++--
 4 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/cyclic.go b/cyclic.go
index f7c9387d..07726528 100644
--- a/cyclic.go
+++ b/cyclic.go
@@ -1,29 +1,29 @@
 package task
 
-// HasCyclicDep checks if a task tree has any cyclic dependency
-func (e *Executor) HasCyclicDep() bool {
+// CheckCyclicDep checks if a task tree has any cyclic dependency
+func (e *Executor) CheckCyclicDep() error {
 	visits := make(map[string]struct{}, len(e.Tasks))
 
-	var checkCyclicDep func(string, *Task) bool
-	checkCyclicDep = func(name string, t *Task) bool {
+	var checkCyclicDep func(string, *Task) error
+	checkCyclicDep = func(name string, t *Task) error {
 		if _, ok := visits[name]; ok {
-			return false
+			return ErrCyclicDepDetected
 		}
 		visits[name] = struct{}{}
 		defer delete(visits, name)
 
 		for _, d := range t.Deps {
-			if !checkCyclicDep(d.Task, e.Tasks[d.Task]) {
-				return false
+			if err := checkCyclicDep(d.Task, e.Tasks[d.Task]); err != nil {
+				return err
 			}
 		}
-		return true
+		return nil
 	}
 
 	for k, v := range e.Tasks {
-		if !checkCyclicDep(k, v) {
-			return true
+		if err := checkCyclicDep(k, v); err != nil {
+			return err
 		}
 	}
-	return false
+	return nil
 }
diff --git a/cyclic_test.go b/cyclic_test.go
index c0c00c61..88b20ef0 100644
--- a/cyclic_test.go
+++ b/cyclic_test.go
@@ -4,6 +4,8 @@ import (
 	"testing"
 
 	"github.com/go-task/task"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestCyclicDepCheck(t *testing.T) {
@@ -18,9 +20,7 @@ func TestCyclicDepCheck(t *testing.T) {
 		},
 	}
 
-	if !isCyclic.HasCyclicDep() {
-		t.Error("Task should be cyclic")
-	}
+	assert.Equal(t, task.ErrCyclicDepDetected, isCyclic.CheckCyclicDep(), "task should be cyclic")
 
 	isNotCyclic := &task.Executor{
 		Tasks: task.Tasks{
@@ -34,7 +34,5 @@ func TestCyclicDepCheck(t *testing.T) {
 		},
 	}
 
-	if isNotCyclic.HasCyclicDep() {
-		t.Error("Task should not be cyclic")
-	}
+	assert.NoError(t, isNotCyclic.CheckCyclicDep())
 }
diff --git a/errors.go b/errors.go
index bb207674..2325d2ed 100644
--- a/errors.go
+++ b/errors.go
@@ -6,8 +6,8 @@ import (
 )
 
 var (
-	// ErrCyclicDependencyDetected is returned when a cyclic dependency was found in the Taskfile
-	ErrCyclicDependencyDetected = errors.New("task: cyclic dependency detected")
+	// ErrCyclicDepDetected is returned when a cyclic dependency was found in the Taskfile
+	ErrCyclicDepDetected = errors.New("task: cyclic dependency detected")
 	// ErrTaskfileAlreadyExists is returned on creating a Taskfile if one already exists
 	ErrTaskfileAlreadyExists = errors.New("task: A Taskfile already exists")
 )
diff --git a/task.go b/task.go
index d9d7c003..b3b8702a 100644
--- a/task.go
+++ b/task.go
@@ -61,8 +61,8 @@ type Task struct {
 
 // Run runs Task
 func (e *Executor) Run(args ...string) error {
-	if e.HasCyclicDep() {
-		return ErrCyclicDependencyDetected
+	if err := e.CheckCyclicDep(); err != nil {
+		return err
 	}
 
 	if e.Stdin == nil {

From ac48ee066e1987993c94733935d291c0107db36d Mon Sep 17 00:00:00 2001
From: Andrey Nering <andrey.nering@gmail.com>
Date: Sat, 8 Jul 2017 10:25:15 -0300
Subject: [PATCH 2/2] fix panic for invalid task in cyclic dep detection

resolves partly #37
---
 cyclic.go      |  7 ++++++-
 cyclic_test.go | 18 ++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/cyclic.go b/cyclic.go
index 07726528..1fb395ae 100644
--- a/cyclic.go
+++ b/cyclic.go
@@ -13,7 +13,12 @@ func (e *Executor) CheckCyclicDep() error {
 		defer delete(visits, name)
 
 		for _, d := range t.Deps {
-			if err := checkCyclicDep(d.Task, e.Tasks[d.Task]); err != nil {
+			// FIXME: ignoring by now. should return an error instead?
+			task, ok := e.Tasks[d.Task]
+			if !ok {
+				continue
+			}
+			if err := checkCyclicDep(d.Task, task); err != nil {
 				return err
 			}
 		}
diff --git a/cyclic_test.go b/cyclic_test.go
index 88b20ef0..602378cd 100644
--- a/cyclic_test.go
+++ b/cyclic_test.go
@@ -35,4 +35,22 @@ func TestCyclicDepCheck(t *testing.T) {
 	}
 
 	assert.NoError(t, isNotCyclic.CheckCyclicDep())
+
+	inexixtentTask := &task.Executor{
+		Tasks: task.Tasks{
+			"task-a": &task.Task{
+				Deps: []*task.Dep{&task.Dep{Task: "invalid-task"}},
+			},
+		},
+	}
+
+	// FIXME: by now Task should ignore non existent tasks
+	// in the future we should improve the detection of
+	// tasks called with interpolation?
+	//     task:
+	//       deps:
+	//         - task: "task{{.VARIABLE}}"
+	//       vars:
+	//         VARIABLE: something
+	assert.NoError(t, inexixtentTask.CheckCyclicDep())
 }