feat: update extends algorithm

reviewpad · marcelosousa · Dec 9, 2022 · Dec 8, 2022 · Dec 8, 2022 · Dec 8, 2022
commit dd61ba1dbbf3fd0f54eb17221a508f0c75f90656
diff --git a/engine/exec_test.go b/engine/exec_test.go
@@ -14,7 +14,6 @@ import (
 	"github.com/migueleliasweb/go-github-mock/src/mock"
 	gh "github.com/reviewpad/reviewpad/v3/codehost/github"
 	"github.com/reviewpad/reviewpad/v3/engine"
-	"github.com/reviewpad/reviewpad/v3/engine/testutils"
 	"github.com/reviewpad/reviewpad/v3/handler"
 	"github.com/reviewpad/reviewpad/v3/lang/aladino"
 	"github.com/reviewpad/reviewpad/v3/utils"
@@ -24,6 +23,8 @@ import (
 func TestEval_WhenGitHubRequestsFail(t *testing.T) {
 	tests := map[string]struct {
 		inputReviewpadFilePath string
+		inputContext           context.Context
+		inputGitHubClient      *gh.GithubClient
 		clientOptions          []mock.MockBackendOption
 		wantErr                string
 	}{
@@ -91,12 +92,12 @@ func TestEval_WhenGitHubRequestsFail(t *testing.T) {
 				assert.FailNow(t, fmt.Sprintf("engine MockEnvWith: %v", err))
 			}
 
-			reviewpadFileData, err := utils.LoadFile(test.inputReviewpadFilePath)
+			reviewpadFileData, err := utils.ReadFile(test.inputReviewpadFilePath)
 			if err != nil {
 				assert.FailNow(t, fmt.Sprintf("Error reading reviewpad file: %v", err))
 			}
 
-			reviewpadFile, err := testutils.ParseReviewpadFile(context.Background(), nil, reviewpadFileData)
+			reviewpadFile, err := engine.Load(test.inputContext, test.inputGitHubClient, reviewpadFileData)
 			if err != nil {
 				assert.FailNow(t, "Error parsing reviewpad file: %v", err)
 			}
@@ -112,6 +113,8 @@ func TestEval_WhenGitHubRequestsFail(t *testing.T) {
 func TestEval(t *testing.T) {
 	tests := map[string]struct {
 		inputReviewpadFilePath string
+		inputContext           context.Context
+		inputGitHubClient      *gh.GithubClient
 		clientOptions          []mock.MockBackendOption
 		targetEntity           *handler.TargetEntity
 		eventData              *handler.EventData
@@ -372,12 +375,12 @@ func TestEval(t *testing.T) {
 				assert.FailNow(t, fmt.Sprintf("engine MockEnvWith: %v", err))
 			}
 
-			reviewpadFileData, err := utils.LoadFile(test.inputReviewpadFilePath)
+			reviewpadFileData, err := utils.ReadFile(test.inputReviewpadFilePath)
 			if err != nil {
 				assert.FailNow(t, fmt.Sprintf("Error reading reviewpad file: %v", err))
 			}
 
-			reviewpadFile, err := testutils.ParseReviewpadFile(context.Background(), nil, reviewpadFileData)
+			reviewpadFile, err := engine.Load(test.inputContext, test.inputGitHubClient, reviewpadFileData)
 			if err != nil {
 				assert.FailNow(t, fmt.Sprintf("Error parsing reviewpad file: %v", err))
 			}

diff --git a/engine/lang.go b/engine/lang.go
@@ -309,142 +309,60 @@ func (r *ReviewpadFile) appendLabels(o *ReviewpadFile) {
 	}
 }
 
-func (r *ReviewpadFile) appendRules(o *ReviewpadFile) {
-	if r.Rules == nil {
-		r.Rules = make([]PadRule, 0)
-	}
-
-	r.Rules = append(r.Rules, o.Rules...)
-}
-
 func (r *ReviewpadFile) appendGroups(o *ReviewpadFile) {
-	if r.Groups == nil {
-		r.Groups = make([]PadGroup, 0)
-	}
-
-	r.Groups = append(r.Groups, o.Groups...)
-}
-
-func (r *ReviewpadFile) appendWorkflows(o *ReviewpadFile) {
-	if r.Workflows == nil {
-		r.Workflows = make([]PadWorkflow, 0)
-	}
-
-	r.Workflows = append(r.Workflows, o.Workflows...)
-}
-
-func (r *ReviewpadFile) appendPipelines(o *ReviewpadFile) {
-	if r.Pipelines == nil {
-		r.Pipelines = make([]PadPipeline, 0)
-	}
-
-	r.Pipelines = append(r.Pipelines, o.Pipelines...)
-}
+	updatedGroups := make([]PadGroup, 0)
 
-func extendLabels(left, right map[string]PadLabel) map[string]PadLabel {
-	if left == nil && right == nil {
-		return nil
-	}
-
-	if left == nil {
-		left = make(map[string]PadLabel)
-	}
-
-	for labelName, label := range right {
-		if _, ok := left[labelName]; !ok {
-			left[labelName] = label
+	for _, group := range r.Groups {
+		if _, ok := findGroup(o.Groups, group.Name); !ok {
+			updatedGroups = append(updatedGroups, group)
 		}
 	}
 
-	return left
+	r.Groups = append(updatedGroups, o.Groups...)
 }
 
-func extendGroups(left, right []PadGroup) []PadGroup {
-	if left == nil && right == nil {
-		return nil
-	}
-
-	if left == nil {
-		left = make([]PadGroup, 0)
-	}
-
-	filteredGroups := make([]PadGroup, 0)
-
-	for _, group := range right {
-		if _, ok := findGroup(left, group.Name); !ok {
-			filteredGroups = append(filteredGroups, group)
-		}
-	}
-
-	return append(filteredGroups, left...)
-}
-
-func extendRules(left, right []PadRule) []PadRule {
-	if left == nil && right == nil {
-		return nil
-	}
-
-	if left == nil {
-		left = make([]PadRule, 0)
-	}
-
-	filteredRules := make([]PadRule, 0)
+func (r *ReviewpadFile) appendRules(o *ReviewpadFile) {
+	updatedRules := make([]PadRule, 0)
 
-	for _, rule := range right {
-		if _, ok := findRule(left, rule.Name); !ok {
-			filteredRules = append(filteredRules, rule)
+	for _, rule := range r.Rules {
+		if _, ok := findRule(o.Rules, rule.Name); !ok {
+			updatedRules = append(updatedRules, rule)
 		}
 	}
 
-	return append(filteredRules, left...)
+	r.Rules = append(updatedRules, o.Rules...)
 }
 
-func extendWorkflows(left, right []PadWorkflow) []PadWorkflow {
-	if left == nil && right == nil {
-		return nil
-	}
-
-	if left == nil {
-		left = make([]PadWorkflow, 0)
-	}
-
-	filteredWorkflows := make([]PadWorkflow, 0)
+func (r *ReviewpadFile) appendWorkflows(o *ReviewpadFile) {
+	updatedWorkflows := make([]PadWorkflow, 0)
 
-	for _, workflow := range right {
-		if _, ok := findWorkflow(left, workflow.Name); !ok {
-			filteredWorkflows = append(filteredWorkflows, workflow)
+	for _, workflow := range r.Workflows {
+		if _, ok := findWorkflow(o.Workflows, workflow.Name); !ok {
+			updatedWorkflows = append(updatedWorkflows, workflow)
 		}
 	}
 
-	return append(filteredWorkflows, left...)
+	r.Workflows = append(updatedWorkflows, o.Workflows...)
 }
 
-func extendPipelines(left, right []PadPipeline) []PadPipeline {
-	if left == nil && right == nil {
-		return nil
-	}
-
-	if left == nil {
-		left = make([]PadPipeline, 0)
-	}
-
-	filteredPipelines := make([]PadPipeline, 0)
+func (r *ReviewpadFile) appendPipelines(o *ReviewpadFile) {
+	updatedPipelines := make([]PadPipeline, 0)
 
-	for _, pipeline := range right {
-		if _, ok := findPipeline(left, pipeline.Name); !ok {
-			filteredPipelines = append(filteredPipelines, pipeline)
+	for _, pipeline := range r.Pipelines {
+		if _, ok := findPipeline(o.Pipelines, pipeline.Name); !ok {
+			updatedPipelines = append(updatedPipelines, pipeline)
 		}
 	}
 
-	return append(filteredPipelines, left...)
+	r.Pipelines = append(updatedPipelines, o.Pipelines...)
 }
 
-func (r *ReviewpadFile) extends(o *ReviewpadFile) {
-	r.Labels = extendLabels(r.Labels, o.Labels)
-	r.Groups = extendGroups(r.Groups, o.Groups)
-	r.Rules = extendRules(r.Rules, o.Rules)
-	r.Workflows = extendWorkflows(r.Workflows, o.Workflows)
-	r.Pipelines = extendPipelines(r.Pipelines, o.Pipelines)
+func (r *ReviewpadFile) extend(o *ReviewpadFile) {
+	r.appendLabels(o)
+	r.appendGroups(o)
+	r.appendRules(o)
+	r.appendWorkflows(o)
+	r.appendPipelines(o)
 }
 
 func findGroup(groups []PadGroup, name string) (*PadGroup, bool) {

diff --git a/engine/loader.go b/engine/loader.go
@@ -155,6 +155,7 @@ func transform(file *ReviewpadFile) *ReviewpadFile {
 		Mode:         file.Mode,
 		IgnoreErrors: file.IgnoreErrors,
 		Imports:      file.Imports,
+		Extends:      file.Extends,
 		Groups:       file.Groups,
 		Rules:        transformedRules,
 		Labels:       file.Labels,
@@ -253,26 +254,21 @@ func loadExtension(ctx context.Context, githubClient *gh.GithubClient, extension
 // precedence: current file > extends file
 // Post-condition: ReviewpadFile without extends statements
 func processExtends(ctx context.Context, githubClient *gh.GithubClient, file *ReviewpadFile, env *LoadEnv) (*ReviewpadFile, error) {
-	for i := len(file.Extends) - 1; i >= 0; i-- {
-		extension := file.Extends[i]
-
-		eFile, eHash, err := loadExtension(ctx, githubClient, extension)
+	extendedFile := &ReviewpadFile{}
+	for _, extensionUri := range file.Extends {
+		eFile, eHash, err := loadExtension(ctx, githubClient, extensionUri)
 		if err != nil {
 			return nil, err
 		}
 
-		// check for cycles
 		if _, ok := env.Stack[eHash]; ok {
 			return nil, fmt.Errorf("loader: cyclic extends dependency")
 		}
 
-		// optimize visits
 		if _, ok := env.Visited[eHash]; ok {
 			continue
 		}
 
-		// DFS call inline imports
-		// update the environment
 		env.Stack[eHash] = true
 		env.Visited[eHash] = true
 
@@ -284,11 +280,13 @@ func processExtends(ctx context.Context, githubClient *gh.GithubClient, file *Re
 		// remove from the stack
 		delete(env.Stack, eHash)
 
-		file.extends(extensionFile)
+		extendedFile.extend(extensionFile)
 	}
 
+	extendedFile.extend(file)
+
 	// reset all extends
-	file.Extends = []string{}
+	extendedFile.Extends = []string{}
 
-	return file, nil
+	return extendedFile, nil
 }
diff --git a/engine/loader_test.go b/engine/loader_test.go
@@ -15,7 +15,6 @@ import (
 	"github.com/migueleliasweb/go-github-mock/src/mock"
 	gh "github.com/reviewpad/reviewpad/v3/codehost/github"
 	"github.com/reviewpad/reviewpad/v3/engine"
-	"github.com/reviewpad/reviewpad/v3/engine/testutils"
 	"github.com/reviewpad/reviewpad/v3/lang/aladino"
 	"github.com/reviewpad/reviewpad/v3/utils"
 	"github.com/stretchr/testify/assert"
@@ -131,23 +130,34 @@ func TestLoad(t *testing.T) {
 				[]mock.MockBackendOption{
 					mock.WithRequestMatchHandler(
 						mock.GetReposContentsByOwnerByRepoByPath,
-						http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+						http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 							utils.MustWriteBytes(w, mock.MustMarshal(
 								[]github.RepositoryContent{
 									{
-										Name:        github.String("reviewpad_with_no_extends.yml"),
-										DownloadURL: github.String("https://raw.githubusercontent.com/reviewpad_with_no_extends.yml"),
+										Name:        github.String("reviewpad_with_no_extends_a.yml"),
+										DownloadURL: github.String("https://raw.githubusercontent.com/reviewpad_with_no_extends_a.yml"),
+									},
+									{
+										Name:        github.String("reviewpad_with_no_extends_b.yml"),
+										DownloadURL: github.String("https://raw.githubusercontent.com/reviewpad_with_no_extends_b.yml"),
 									},
 								},
 							))
 						}),
 					),
 					mock.WithRequestMatch(
 						mock.EndpointPattern{
-							Pattern: "/reviewpad_with_no_extends.yml",
+							Pattern: "/reviewpad_with_no_extends_a.yml",
+							Method:  "GET",
+						},
+						httpmock.File("testdata/loader/reviewpad_with_no_extends_a.yml").Bytes(),
+					),
+					mock.WithRequestMatch(
+						mock.EndpointPattern{
+							Pattern: "/reviewpad_with_no_extends_b.yml",
 							Method:  "GET",
 						},
-						httpmock.File("testdata/loader/reviewpad_with_no_extends.yml").Bytes(),
+						httpmock.File("testdata/loader/reviewpad_with_no_extends_b.yml").Bytes(),
 					),
 				},
 				nil,
@@ -166,18 +176,18 @@ func TestLoad(t *testing.T) {
 
 			var wantReviewpadFile *engine.ReviewpadFile
 			if test.wantReviewpadFilePath != "" {
-				wantReviewpadFileData, err := utils.LoadFile(test.wantReviewpadFilePath)
+				wantReviewpadFileData, err := utils.ReadFile(test.wantReviewpadFilePath)
 				if err != nil {
 					assert.FailNow(t, "Error reading reviewpad file: %v", err)
 				}
 
-				wantReviewpadFile, err = testutils.ParseReviewpadFile(test.inputContext, test.inputGitHubClient, wantReviewpadFileData)
+				wantReviewpadFile, err = engine.Load(test.inputContext, test.inputGitHubClient, wantReviewpadFileData)
 				if err != nil {
 					assert.FailNow(t, "Error parsing reviewpad file: %v", err)
 				}
 			}
 
-			reviewpadFileData, err := utils.LoadFile(test.inputReviewpadFilePath)
+			reviewpadFileData, err := utils.ReadFile(test.inputReviewpadFilePath)
 			if err != nil {
 				assert.FailNow(t, "Error reading reviewpad file: %v", err)
 			}

diff --git a/engine/testdata/loader/process/reviewpad_with_extends_after_processing.yml b/engine/testdata/loader/process/reviewpad_with_extends_after_processing.yml
@@ -6,9 +6,9 @@ api-version: reviewpad.com/v3.x
 
 labels:
   small:
-    color: #aa12ab
+    color: '#aa12ab'
   medium:
-    color: #a8c3f7
+    color: '#a8c3f7'
 
 groups:
   - name: owners
@@ -36,7 +36,13 @@ workflows:
     if:
       - rule: $isElementOf($author(), $group("owners"))
     then:
-      - $info("owner has authored a PR")
+      - $info("bob has authored a PR")
+
+  - name: check-title
+    if:
+      - 'true'
+    then:
+      - $titleLint()
 
   - name: add-label-with-small-size
     if:

diff --git a/engine/testdata/loader/reviewpad_with_extends.yml b/engine/testdata/loader/reviewpad_with_extends.yml
@@ -5,11 +5,12 @@
 api-version: reviewpad.com/v3.x
 
 extends:
-  - https://github.com/reviewpad/reviewpad/blob/main/engine/testdata/reviewpad_with_no_extends.yml
+  - https://github.com/reviewpad/reviewpad/blob/main/engine/testdata/reviewpad_with_no_extends_a.yml
+  - https://github.com/reviewpad/reviewpad/blob/main/engine/testdata/reviewpad_with_no_extends_b.yml
 
 labels:
   small:
-    color: #aa12ab
+    color: '#aa12ab'
 
 groups:
   - name: owners