Repository: palantir/policy-bot (go)

Issue:
feature request: Allow conditional policy on file deletion/additon
Today we allow conditional policies on aspects of file modification like number of lines added/deleted/modified. Certain workflows would benefit from extending this to also allow conditional policies on the addition of new files of deletion of existing files which cannot be accurately expressed in terms of existing file modification knobs.

@asvoboda 

Pull request:
add file_not_deleted predicate
This adds the `file_not_deleted` predicate, which accepts a list of regex patterns used verify whether files required by a policy rule have been deleted as part of a PR. If any matching file is removed, the predicate fails, allowing rules that depend on the file's existence to be skipped.

- Added logic for `file_not_deleted` predicate
- Updated unit tests to verify functionality
- Updated readme to document the new predicate

See discussion from [previous PR](https://github.com/palantir/policy-bot/pull/934).

Closes #246

Patch:
diff --git a/README.md b/README.md
index 9a48bc8..c36ecf1 100644
--- a/README.md
+++ b/README.md
@@ -268,6 +268,17 @@ if:
     paths:
       - "^config/.*$"
 
+  # "file_not_deleted" 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.
+  #
+  # 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_deleted:
+    paths:
+      - "^\\.github/workflows/.*\\.ya?ml$"
+
   # "has_author_in" is satisfied if the user who opened the pull request is in
   # the users list or belongs to any of the listed organizations or teams. The
   # `users` field can contain a GitHub App by appending `[bot]` to the end of
diff --git a/policy/predicate/file.go b/policy/predicate/file.go
index 1f82064..48ca2c1 100644
--- a/policy/predicate/file.go
+++ b/policy/predicate/file.go
@@ -181,6 +181,60 @@ func (pred *NoChangedFiles) 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) {
+	var paths []string
+	for _, r := range pred.Paths {
+		paths = append(paths, r.String())
+	}
+
+	predicateResult := common.PredicateResult{
+		Satisfied:         true,
+		ValuePhrase:       "deleted files",
+		Values:            []string{},
+		ConditionPhrase:   "match path patterns",
+		ReverseSkipPhrase: true,
+		ConditionValues:   paths,
+	}
+
+	changedFiles, err := prctx.ChangedFiles()
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to list changed files")
+	}
+
+	if len(changedFiles) == 0 {
+		predicateResult.Description = "No files were changed"
+		return &predicateResult, nil
+	}
+
+	deletedFiles := []string{}
+	for _, f := range changedFiles {
+		if f.Status == pull.FileDeleted {
+			deletedFiles = append(deletedFiles, f.Filename)
+
+			if anyMatches(pred.Paths, f.Filename) {
+				predicateResult.Satisfied = false
+				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 *FileNotDeleted) Trigger() common.Trigger {
+	return common.TriggerCommit
+}
+
 type ModifiedLines struct {
 	Additions ComparisonExpr `yaml:"additions"`
 	Deletions ComparisonExpr `yaml:"deletions"`
diff --git a/policy/predicate/predicates.go b/policy/predicate/predicates.go
index 8bded60..937f1f6 100644
--- a/policy/predicate/predicates.go
+++ b/policy/predicate/predicates.go
@@ -18,6 +18,7 @@ type Predicates struct {
 	ChangedFiles     *ChangedFiles     `yaml:"changed_files"`
 	NoChangedFiles   *NoChangedFiles   `yaml:"no_changed_files"`
 	OnlyChangedFiles *OnlyChangedFiles `yaml:"only_changed_files"`
+	FileNotDeleted   *FileNotDeleted   `yaml:"file_not_deleted"`
 
 	HasAuthorIn             *HasAuthorIn             `yaml:"has_author_in"`
 	HasContributorIn        *HasContributorIn        `yaml:"has_contributor_in"`
@@ -59,6 +60,9 @@ func (p *Predicates) Predicates() []Predicate {
 	if p.OnlyChangedFiles != nil {
 		ps = append(ps, Predicate(p.OnlyChangedFiles))
 	}
+	if p.FileNotDeleted != nil {
+		ps = append(ps, Predicate(p.FileNotDeleted))
+	}
 
 	if p.HasAuthorIn != nil {
 		ps = append(ps, Predicate(p.HasAuthorIn))


Test patch:
diff --git a/policy/predicate/file_test.go b/policy/predicate/file_test.go
index bbe286a..7417d51 100644
--- a/policy/predicate/file_test.go
+++ b/policy/predicate/file_test.go
@@ -364,6 +364,95 @@ func TestOnlyChangedFiles(t *testing.T) {
 	})
 }
 
+func TestFileNotDeleted(t *testing.T) {
+	p := &FileNotDeleted{
+		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 deleted",
+			[]*pull.File{
+				{
+					Filename: "workflows/workflow.yaml",
+					Status:   pull.FileDeleted,
+				},
+				{
+					Filename: "actions/action.yaml",
+					Status:   pull.FileModified,
+				},
+			},
+			&common.PredicateResult{
+				Satisfied:       false,
+				Values:          []string{"workflows/workflow.yaml"},
+				ConditionValues: []string{"workflows/.*\\.yaml", "actions/.*\\.yaml"},
+			},
+		},
+		{
+			"multiple files deleted",
+			[]*pull.File{
+				{
+					Filename: "workflows/workflow.yaml",
+					Status:   pull.FileDeleted,
+				},
+				{
+					Filename: "actions/action.yaml",
+					Status:   pull.FileDeleted,
+				},
+			},
+			&common.PredicateResult{
+				Satisfied:       false,
+				Values:          []string{"workflows/workflow.yaml"},
+				ConditionValues: []string{"workflows/.*\\.yaml", "actions/.*\\.yaml"},
+			},
+		},
+		{
+			"file deleted not matching",
+			[]*pull.File{
+				{
+					Filename: "some/otherfile.yaml",
+					Status:   pull.FileDeleted,
+				},
+			},
+			&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},