Repository: palantir/policy-bot (go)

Issue:
Add additional file enforcement predicates
The goal is to add a total of four additional, standalone file-related predicates:

- `file_deleted`: satisfied if a specified file was deleted as part of a PR
- `file_added`: satisfied if a specified file was added
-  `file_not_deleted`: satisfied if a specified file was NOT deleted (pending #935)
- `file_not_added`: satisfied if a specified file was NOT added

This will give users more flexibility in file change enforcement as part of their overall policy.

See discussions from #246 and #934 for more detail.

Pull request:
add new file predicates for better PR control
Added new predicates to enhance PR policy controls:

- file_added: Trigger rules when specified files are added
- file_not_added: Trigger rules when specified files are not added
- file_deleted: Trigger rules when specified files are deleted

Changes include:
- Implemented helper function getPathString
- Added unit tests for each predicate
- Updated README with example usage

Closes #936.

Patch:
diff --git a/README.md b/README.md
index c36ecf1..b96999f 100644
--- a/README.md
+++ b/README.md
@@ -268,7 +268,44 @@ if:
     paths:
       - "^config/.*$"
 
-  # "file_not_deleted" is satisfied if none of the files matching any regular expression
+  # "file_added" is satisfied if any file matching any regular expression in the "paths"
+  # list has been added in the pull request, allowing enforcement of rules when
+  # specific types of files are added. If no matching files are added, the predicate
+  # fails.
+  #
+  # Note: Double-quote strings must escape backslashes while single/plain do not.
+  # See the Notes on YAML Syntax section of this README for more information.
+  file_added:
+    paths:
+      - "^config/.*\\.yaml$"
+      - "^server/.*\\.go$"
+
+  # "file_not_added" is the negation of "file_added". This predicate is
+  # satisfied if none of the files matching any regular expression
+  # in the "paths" list have been added in the pull request. If any matching file
+  # was added, the predicate fails, allowing rules to be skipped that depend on
+  # files not being added.
+  #
+  # Note: Double-quote strings must escape backslashes while single/plain do not.
+  # See the Notes on YAML Syntax section of this README for more information.
+  file_not_added:
+    paths:
+      - "^deprecated/.*$"
+
+  # "file_deleted" is satisfied if any file matching any regular expression in the "paths"
+  # list has been deleted in the pull request, allowing enforcement of rules when
+  # specific types of files are deleted. If no matching files are deleted, the
+  # predicate fails.
+  #
+  # Note: Double-quote strings must escape backslashes while single/plain do not.
+  # See the Notes on YAML Syntax section of this README for more information.
+  file_deleted:
+    paths:
+      - "^legacy/.*\\.js$"
+      - "^deprecated/.*\\.go$"
+
+  # "file_not_deleted" is the negation of "file_deleted". This predicate is
+  # satisfied if none of the files matching any regular expression
   # in the "paths" list have been deleted in the pull request. If any matching file
   # was deleted, the predicate fails, allowing rules that depend on the file's existence
   # to be skipped.
diff --git a/policy/predicate/file.go b/policy/predicate/file.go
index ae8c7db..b012083 100644
--- a/policy/predicate/file.go
+++ b/policy/predicate/file.go
@@ -25,6 +25,14 @@ import (
 	"github.com/pkg/errors"
 )
 
+func getPathStrings(patterns []common.Regexp) []string {
+	var paths []string
+	for _, r := range patterns {
+		paths = append(paths, r.String())
+	}
+	return paths
+}
+
 type ChangedFiles struct {
 	Paths       []common.Regexp `yaml:"paths,omitempty"`
 	IgnorePaths []common.Regexp `yaml:"ignore,omitempty"`
@@ -33,15 +41,8 @@ type ChangedFiles struct {
 var _ Predicate = &ChangedFiles{}
 
 func (pred *ChangedFiles) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) {
-	var paths, ignorePaths []string
-
-	for _, path := range pred.Paths {
-		paths = append(paths, path.String())
-	}
-
-	for _, ignorePath := range pred.IgnorePaths {
-		ignorePaths = append(ignorePaths, ignorePath.String())
-	}
+	paths := getPathStrings(pred.Paths)
+	ignorePaths := getPathStrings(pred.IgnorePaths)
 
 	predicateResult := common.PredicateResult{
 		ValuePhrase:     "changed files",
@@ -92,11 +93,7 @@ type OnlyChangedFiles struct {
 var _ Predicate = &OnlyChangedFiles{}
 
 func (pred *OnlyChangedFiles) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) {
-	var paths []string
-
-	for _, path := range pred.Paths {
-		paths = append(paths, path.String())
-	}
+	paths := getPathStrings(pred.Paths)
 
 	predicateResult := common.PredicateResult{
 		ValuePhrase:     "changed files",
@@ -181,25 +178,66 @@ func (pred *NoChangedFiles) Trigger() common.Trigger {
 	return common.TriggerCommit
 }
 
-type FileNotDeleted struct {
+type FileAdded struct {
 	Paths []common.Regexp `yaml:"paths"`
 }
 
-var _ Predicate = &FileNotDeleted{}
+var _ Predicate = &FileAdded{}
 
-func (pred *FileNotDeleted) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) {
-	var paths []string
-	for _, r := range pred.Paths {
-		paths = append(paths, r.String())
+func (pred *FileAdded) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) {
+	paths := getPathStrings(pred.Paths)
+	predicateResult := common.PredicateResult{
+		Satisfied:       false,
+		ValuePhrase:     "added files",
+		Values:          []string{},
+		ConditionPhrase: "match path patterns",
+		ConditionValues: paths,
 	}
 
+	changedFiles, err := prctx.ChangedFiles()
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to list changed files")
+	}
+
+	addedFiles := []string{}
+	for _, f := range changedFiles {
+		if f.Status == pull.FileAdded {
+			addedFiles = append(addedFiles, f.Filename)
+
+			if anyMatches(pred.Paths, f.Filename) {
+				predicateResult.Satisfied = true
+				predicateResult.Values = []string{f.Filename}
+				predicateResult.Description = f.Filename + " was added"
+				return &predicateResult, nil
+			}
+		}
+	}
+
+	predicateResult.Values = addedFiles
+	predicateResult.Description = "No added files match the specified patterns"
+	return &predicateResult, nil
+}
+
+func (pred *FileAdded) Trigger() common.Trigger {
+	return common.TriggerCommit
+}
+
+type FileNotAdded struct {
+	Paths []common.Regexp `yaml:"paths"`
+}
+
+var _ Predicate = &FileNotAdded{}
+
+func (pred *FileNotAdded) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) {
+	paths := getPathStrings(pred.Paths)
+
 	predicateResult := common.PredicateResult{
 		Satisfied:         true,
-		ValuePhrase:       "deleted files",
+		ValuePhrase:       "added files",
 		Values:            []string{},
 		ConditionPhrase:   "match path patterns",
-		ReverseSkipPhrase: true,
 		ConditionValues:   paths,
+		ReverseSkipPhrase: true,
 	}
 
 	changedFiles, err := prctx.ChangedFiles()
@@ -207,9 +245,94 @@ func (pred *FileNotDeleted) Evaluate(ctx context.Context, prctx pull.Context) (*
 		return nil, errors.Wrap(err, "failed to list changed files")
 	}
 
-	if len(changedFiles) == 0 {
-		predicateResult.Description = "No files were changed"
-		return &predicateResult, nil
+	addedFiles := []string{}
+	for _, f := range changedFiles {
+		if f.Status == pull.FileAdded {
+			addedFiles = append(addedFiles, f.Filename)
+
+			if anyMatches(pred.Paths, f.Filename) {
+				predicateResult.Satisfied = false
+				predicateResult.Values = []string{f.Filename}
+				predicateResult.Description = f.Filename + " was added"
+				return &predicateResult, nil
+			}
+		}
+	}
+
+	predicateResult.Values = addedFiles
+	predicateResult.Description = "No added files match the specified patterns"
+	return &predicateResult, nil
+}
+
+func (pred *FileNotAdded) Trigger() common.Trigger {
+	return common.TriggerCommit
+}
+
+type FileDeleted struct {
+	Paths []common.Regexp `yaml:"paths"`
+}
+
+var _ Predicate = &FileDeleted{}
+
+func (pred *FileDeleted) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) {
+	paths := getPathStrings(pred.Paths)
+
+	predicateResult := common.PredicateResult{
+		Satisfied:       false,
+		ValuePhrase:     "deleted files",
+		Values:          []string{},
+		ConditionPhrase: "match path patterns",
+		ConditionValues: paths,
+	}
+
+	changedFiles, err := prctx.ChangedFiles()
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to list changed files")
+	}
+
+	deletedFiles := []string{}
+	for _, f := range changedFiles {
+		if f.Status == pull.FileDeleted {
+			deletedFiles = append(deletedFiles, f.Filename)
+
+			if anyMatches(pred.Paths, f.Filename) {
+				predicateResult.Satisfied = true
+				predicateResult.Values = []string{f.Filename}
+				predicateResult.Description = f.Filename + " was deleted"
+				return &predicateResult, nil
+			}
+		}
+	}
+
+	predicateResult.Values = deletedFiles
+	predicateResult.Description = "No deleted files match the specified patterns"
+	return &predicateResult, nil
+}
+
+func (pred *FileDeleted) Trigger() common.Trigger {
+	return common.TriggerCommit
+}
+
+type FileNotDeleted struct {
+	Paths []common.Regexp `yaml:"paths"`
+}
+
+var _ Predicate = &FileNotDeleted{}
+
+func (pred *FileNotDeleted) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) {
+	paths := getPathStrings(pred.Paths)
+	predicateResult := common.PredicateResult{
+		Satisfied:         true,
+		ValuePhrase:       "deleted files",
+		Values:            []string{},
+		ConditionPhrase:   "match path patterns",
+		ConditionValues:   paths,
+		ReverseSkipPhrase: true,
+	}
+
+	changedFiles, err := prctx.ChangedFiles()
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to list changed files")
 	}
 
 	deletedFiles := []string{}
diff --git a/policy/predicate/predicates.go b/policy/predicate/predicates.go
index 847e435..de28c29 100644
--- a/policy/predicate/predicates.go
+++ b/policy/predicate/predicates.go
@@ -18,6 +18,9 @@ type Predicates struct {
 	ChangedFiles     *ChangedFiles     `yaml:"changed_files,omitempty"`
 	NoChangedFiles   *NoChangedFiles   `yaml:"no_changed_files,omitempty"`
 	OnlyChangedFiles *OnlyChangedFiles `yaml:"only_changed_files,omitempty"`
+	FileAdded        *FileAdded        `yaml:"file_added,omitempty"`
+	FileNotAdded     *FileNotAdded     `yaml:"file_not_added,omitempty"`
+	FileDeleted      *FileDeleted      `yaml:"file_deleted,omitempty"`
 	FileNotDeleted   *FileNotDeleted   `yaml:"file_not_deleted,omitempty"`
 
 	HasAuthorIn             *HasAuthorIn             `yaml:"has_author_in,omitempty"`
@@ -64,6 +67,15 @@ func (p *Predicates) Predicates() []Predicate {
 	if p.OnlyChangedFiles != nil {
 		ps = append(ps, Predicate(p.OnlyChangedFiles))
 	}
+	if p.FileAdded != nil {
+		ps = append(ps, Predicate(p.FileAdded))
+	}
+	if p.FileNotAdded != nil {
+		ps = append(ps, Predicate(p.FileNotAdded))
+	}
+	if p.FileDeleted != nil {
+		ps = append(ps, Predicate(p.FileDeleted))
+	}
 	if p.FileNotDeleted != nil {
 		ps = append(ps, Predicate(p.FileNotDeleted))
 	}


Test patch:
diff --git a/policy/predicate/file_test.go b/policy/predicate/file_test.go
index 7417d51..bc19f44 100644
--- a/policy/predicate/file_test.go
+++ b/policy/predicate/file_test.go
@@ -453,6 +453,245 @@ func TestFileNotDeleted(t *testing.T) {
 	})
 }
 
+func TestFileAdded(t *testing.T) {
+	p := &FileAdded{
+		Paths: []common.Regexp{
+			common.NewCompiledRegexp(regexp.MustCompile("app/.*\\.go")),
+			common.NewCompiledRegexp(regexp.MustCompile("server/.*\\.go")),
+		},
+	}
+
+	runFileTests(t, p, []FileTestCase{
+		{
+			"no files",
+			[]*pull.File{},
+			&common.PredicateResult{
+				Satisfied:       false,
+				Values:          []string{},
+				ConditionValues: []string{"app/.*\\.go", "server/.*\\.go"},
+			},
+		},
+		{
+			"matching file added",
+			[]*pull.File{
+				{
+					Filename: "app/client.go",
+					Status:   pull.FileAdded,
+				},
+				{
+					Filename: "other/file.go",
+					Status:   pull.FileAdded,
+				},
+			},
+			&common.PredicateResult{
+				Satisfied:       true,
+				Values:          []string{"app/client.go"},
+				ConditionValues: []string{"app/.*\\.go", "server/.*\\.go"},
+			},
+		},
+		{
+			"no matching files added",
+			[]*pull.File{
+				{
+					Filename: "other/file1.go",
+					Status:   pull.FileAdded,
+				},
+				{
+					Filename: "other/file2.go",
+					Status:   pull.FileAdded,
+				},
+			},
+			&common.PredicateResult{
+				Satisfied:       false,
+				Values:          []string{"other/file1.go", "other/file2.go"},
+				ConditionValues: []string{"app/.*\\.go", "server/.*\\.go"},
+			},
+		},
+		{
+			"files exist but modified",
+			[]*pull.File{
+				{
+					Filename: "app/client.go",
+					Status:   pull.FileModified,
+				},
+				{
+					Filename: "server/server.go",
+					Status:   pull.FileModified,
+				},
+			},
+			&common.PredicateResult{
+				Satisfied:       false,
+				Values:          []string{},
+				ConditionValues: []string{"app/.*\\.go", "server/.*\\.go"},
+			},
+		},
+	})
+}
+
+func TestFileDeleted(t *testing.T) {
+	p := &FileDeleted{
+		Paths: []common.Regexp{
+			common.NewCompiledRegexp(regexp.MustCompile("app/.*\\.go")),
+			common.NewCompiledRegexp(regexp.MustCompile("server/.*\\.go")),
+		},
+	}
+
+	runFileTests(t, p, []FileTestCase{
+		{
+			"no files",
+			[]*pull.File{},
+			&common.PredicateResult{
+				Satisfied:       false,
+				Values:          []string{},
+				ConditionValues: []string{"app/.*\\.go", "server/.*\\.go"},
+			},
+		},
+		{
+			"matching file deleted",
+			[]*pull.File{
+				{
+					Filename: "app/client.go",
+					Status:   pull.FileDeleted,
+				},
+				{
+					Filename: "other/file.go",
+					Status:   pull.FileDeleted,
+				},
+			},
+			&common.PredicateResult{
+				Satisfied:       true,
+				Values:          []string{"app/client.go"},
+				ConditionValues: []string{"app/.*\\.go", "server/.*\\.go"},
+			},
+		},
+		{
+			"no matching files deleted",
+			[]*pull.File{
+				{
+					Filename: "other/file1.go",
+					Status:   pull.FileDeleted,
+				},
+				{
+					Filename: "other/file2.go",
+					Status:   pull.FileDeleted,
+				},
+			},
+			&common.PredicateResult{
+				Satisfied:       false,
+				Values:          []string{"other/file1.go", "other/file2.go"},
+				ConditionValues: []string{"app/.*\\.go", "server/.*\\.go"},
+			},
+		},
+		{
+			"files exist but modified",
+			[]*pull.File{
+				{
+					Filename: "app/client.go",
+					Status:   pull.FileModified,
+				},
+				{
+					Filename: "server/server.go",
+					Status:   pull.FileModified,
+				},
+			},
+			&common.PredicateResult{
+				Satisfied:       false,
+				Values:          []string{},
+				ConditionValues: []string{"app/.*\\.go", "server/.*\\.go"},
+			},
+		},
+	})
+}
+
+func TestFileNotAdded(t *testing.T) {
+	p := &FileNotAdded{
+		Paths: []common.Regexp{
+			common.NewCompiledRegexp(regexp.MustCompile("workflows/.*\\.yaml")),
+			common.NewCompiledRegexp(regexp.MustCompile("actions/.*\\.yaml")),
+		},
+	}
+
+	runFileTests(t, p, []FileTestCase{
+		{
+			"file not changed",
+			[]*pull.File{},
+			&common.PredicateResult{
+				Satisfied:       true,
+				Values:          []string{},
+				ConditionValues: []string{"workflows/.*\\.yaml", "actions/.*\\.yaml"},
+			},
+		},
+		{
+			"files exist and modified",
+			[]*pull.File{
+				{
+					Filename: "workflows/workflow.yaml",
+					Status:   pull.FileModified,
+				},
+				{
+					Filename: "actions/action.yaml",
+					Status:   pull.FileModified,
+				},
+			},
+			&common.PredicateResult{
+				Satisfied:       true,
+				Values:          []string{},
+				ConditionValues: []string{"workflows/.*\\.yaml", "actions/.*\\.yaml"},
+			},
+		},
+		{
+			"one file added",
+			[]*pull.File{
+				{
+					Filename: "workflows/workflow.yaml",
+					Status:   pull.FileAdded,
+				},
+				{
+					Filename: "actions/action.yaml",
+					Status:   pull.FileModified,
+				},
+			},
+			&common.PredicateResult{
+				Satisfied:       false,
+				Values:          []string{"workflows/workflow.yaml"},
+				ConditionValues: []string{"workflows/.*\\.yaml", "actions/.*\\.yaml"},
+			},
+		},
+		{
+			"multiple files added",
+			[]*pull.File{
+				{
+					Filename: "workflows/workflow.yaml",
+					Status:   pull.FileAdded,
+				},
+				{
+					Filename: "actions/action.yaml",
+					Status:   pull.FileAdded,
+				},
+			},
+			&common.PredicateResult{
+				Satisfied:       false,
+				Values:          []string{"workflows/workflow.yaml"},
+				ConditionValues: []string{"workflows/.*\\.yaml", "actions/.*\\.yaml"},
+			},
+		},
+		{
+			"file added not matching",
+			[]*pull.File{
+				{
+					Filename: "some/otherfile.yaml",
+					Status:   pull.FileAdded,
+				},
+			},
+			&common.PredicateResult{
+				Satisfied:       true,
+				Values:          []string{"some/otherfile.yaml"},
+				ConditionValues: []string{"workflows/.*\\.yaml", "actions/.*\\.yaml"},
+			},
+		},
+	})
+}
+
 func TestModifiedLines(t *testing.T) {
 	p := &ModifiedLines{
 		Additions: ComparisonExpr{Op: OpGreaterThan, Value: 100},