Extend filter syntax with optional specification of file status: add, modified, deleted (#22)

* Add support for specification of change type (add,modified,delete)

* Use NULL as separator in git-diff command output

* Improve PR test workflow

* Fix the workflow file

src/file.ts

@@ -0,0 +1,13 @@
+export interface File {
+  filename: string
+  status: ChangeStatus
+}
+
+export enum ChangeStatus {
+  Added = 'added',
+  Copied = 'copied',
+  Deleted = 'deleted',
+  Modified = 'modified',
+  Renamed = 'renamed',
+  Unmerged = 'unmerged'
+}

src/filter.ts

@@ -1,49 +1,101 @@
 import * as jsyaml from 'js-yaml'
 import * as minimatch from 'minimatch'
+import {File, ChangeStatus} from './file'
+
+// Type definition of object we expect to load from YAML
+interface FilterYaml {
+  [name: string]: FilterItemYaml
+}
+type FilterItemYaml =
+  | string // Filename pattern, e.g. "path/to/*.js"
+  | {[changeTypes: string]: string} // Change status and filename, e.g. added|modified: "path/to/*.js"
+  | FilterItemYaml[] // Supports referencing another rule via YAML anchor
+
+// Minimatch options used in all matchers
+const MinimatchOptions: minimatch.IOptions = {
+  dot: true
+}
+
+// Internal representation of one item in named filter rule
+// Created as simplified form of data in FilterItemYaml
+interface FilterRuleItem {
+  status?: ChangeStatus[] // Required change status of the matched files
+  matcher: minimatch.IMinimatch // Matches the filename
+}
 
 export default class Filter {
-  rules: {[key: string]: minimatch.IMinimatch[]} = {}
+  rules: {[key: string]: FilterRuleItem[]} = {}
 
-  constructor(yaml: string) {
-    const doc = jsyaml.safeLoad(yaml)
-    if (typeof doc !== 'object') {
-      this.throwInvalidFormatError()
-    }
-
-    const opts: minimatch.IOptions = {
-      dot: true
-    }
-
-    for (const name of Object.keys(doc)) {
-      const patternsNode = doc[name]
-      if (!Array.isArray(patternsNode)) {
-        this.throwInvalidFormatError()
-      }
-      const patterns = flat(patternsNode) as string[]
-      if (!patterns.every(x => typeof x === 'string')) {
-        this.throwInvalidFormatError()
-      }
-      this.rules[name] = patterns.map(x => new minimatch.Minimatch(x, opts))
+  // Creates instance of Filter and load rules from YAML if it's provided
+  constructor(yaml?: string) {
+    if (yaml) {
+      this.load(yaml)
     }
   }
 
-  // Returns dictionary with match result per rules group
-  match(paths: string[]): {[key: string]: boolean} {
+  // Load rules from YAML string
+  load(yaml: string): void {
+    const doc = jsyaml.safeLoad(yaml) as FilterYaml
+    if (typeof doc !== 'object') {
+      this.throwInvalidFormatError('Root element is not an object')
+    }
+
+    for (const [key, item] of Object.entries(doc)) {
+      this.rules[key] = this.parseFilterItemYaml(item)
+    }
+  }
+
+  // Returns dictionary with match result per rule
+  match(files: File[]): {[key: string]: boolean} {
     const result: {[key: string]: boolean} = {}
     for (const [key, patterns] of Object.entries(this.rules)) {
-      const match = paths.some(fileName => patterns.some(rule => rule.match(fileName)))
+      const match = files.some(file =>
+        patterns.some(
+          rule => (rule.status === undefined || rule.status.includes(file.status)) && rule.matcher.match(file.filename)
+        )
+      )
       result[key] = match
     }
     return result
   }
 
-  private throwInvalidFormatError(): never {
-    throw new Error('Invalid filter YAML format: Expected dictionary of string arrays')
+  private parseFilterItemYaml(item: FilterItemYaml): FilterRuleItem[] {
+    if (Array.isArray(item)) {
+      return flat(item.map(i => this.parseFilterItemYaml(i)))
+    }
+
+    if (typeof item === 'string') {
+      return [{status: undefined, matcher: new minimatch.Minimatch(item, MinimatchOptions)}]
+    }
+
+    if (typeof item === 'object') {
+      return Object.entries(item).map(([key, pattern]) => {
+        if (typeof key !== 'string' || typeof pattern !== 'string') {
+          this.throwInvalidFormatError(
+            `Expected [key:string]= pattern:string, but [${key}:${typeof key}]= ${pattern}:${typeof pattern} found`
+          )
+        }
+        return {
+          status: key
+            .split('|')
+            .map(x => x.trim())
+            .filter(x => x.length > 0)
+            .map(x => x.toLowerCase()) as ChangeStatus[],
+          matcher: new minimatch.Minimatch(pattern, MinimatchOptions)
+        }
+      })
+    }
+
+    this.throwInvalidFormatError(`Unexpected element type '${typeof item}'`)
+  }
+
+  private throwInvalidFormatError(message: string): never {
+    throw new Error(`Invalid filter YAML format: ${message}.`)
   }
 }
 
-// Creates a new array with all sub-array elements recursively concatenated
+// Creates a new array with all sub-array elements concatenated
 // In future could be replaced by Array.prototype.flat (supported on Node.js 11+)
-function flat(arr: any[]): any[] {
-  return arr.reduce((acc, val) => acc.concat(Array.isArray(val) ? flat(val) : val), [])
+function flat<T>(arr: T[][]): T[] {
+  return arr.reduce((acc, val) => acc.concat(val), [])
 }

src/git.ts

@@ -1,4 +1,6 @@
 import {exec} from '@actions/exec'
+import * as core from '@actions/core'
+import {File, ChangeStatus} from './file'
 
 export const NULL_SHA = '0000000000000000000000000000000000000000'
 export const FETCH_HEAD = 'FETCH_HEAD'
@@ -10,9 +12,9 @@ export async function fetchCommit(ref: string): Promise<void> {
   }
 }
 
-export async function getChangedFiles(ref: string): Promise<string[]> {
+export async function getChangedFiles(ref: string, cmd = exec): Promise<File[]> {
   let output = ''
-  const exitCode = await exec('git', ['diff-index', '--name-only', ref], {
+  const exitCode = await cmd('git', ['diff-index', '--name-status', '-z', ref], {
     listeners: {
       stdout: (data: Buffer) => (output += data.toString())
     }
@@ -22,10 +24,20 @@ export async function getChangedFiles(ref: string): Promise<string[]> {
     throw new Error(`Couldn't determine changed files`)
   }
 
-  return output
-    .split('\n')
-    .map(s => s.trim())
-    .filter(s => s.length > 0)
+  // Previous command uses NULL as delimiters and output is printed to stdout.
+  // We have to make sure next thing written to stdout will start on new line.
+  // Otherwise things like ::set-output wouldn't work.
+  core.info('')
+
+  const tokens = output.split('\u0000').filter(s => s.length > 0)
+  const files: File[] = []
+  for (let i = 0; i + 1 < tokens.length; i += 2) {
+    files.push({
+      status: statusMap[tokens[i]],
+      filename: tokens[i + 1]
+    })
+  }
+  return files
 }
 
 export function isTagRef(ref: string): boolean {
@@ -44,3 +56,12 @@ export function trimRefsHeads(ref: string): string {
 function trimStart(ref: string, start: string): string {
   return ref.startsWith(start) ? ref.substr(start.length) : ref
 }
+
+const statusMap: {[char: string]: ChangeStatus} = {
+  A: ChangeStatus.Added,
+  C: ChangeStatus.Copied,
+  D: ChangeStatus.Deleted,
+  M: ChangeStatus.Modified,
+  R: ChangeStatus.Renamed,
+  U: ChangeStatus.Unmerged
+}

src/main.ts

@@ -4,6 +4,7 @@ import * as github from '@actions/github'
 import {Webhooks} from '@octokit/webhooks'
 
 import Filter from './filter'
+import {File, ChangeStatus} from './file'
 import * as git from './git'
 
 async function run(): Promise<void> {
@@ -53,7 +54,7 @@ function getConfigFileContent(configPath: string): string {
   return fs.readFileSync(configPath, {encoding: 'utf8'})
 }
 
-async function getChangedFiles(token: string): Promise<string[] | null> {
+async function getChangedFiles(token: string): Promise<File[] | null> {
   if (github.context.eventName === 'pull_request') {
     const pr = github.context.payload.pull_request as Webhooks.WebhookPayloadPullRequestPullRequest
     return token ? await getChangedFilesFromApi(token, pr) : await getChangedFilesFromGit(pr.base.sha)
@@ -64,7 +65,7 @@ async function getChangedFiles(token: string): Promise<string[] | null> {
   }
 }
 
-async function getChangedFilesFromPush(): Promise<string[] | null> {
+async function getChangedFilesFromPush(): Promise<File[] | null> {
   const push = github.context.payload as Webhooks.WebhookPayloadPush
 
   // No change detection for pushed tags
@@ -86,7 +87,7 @@ async function getChangedFilesFromPush(): Promise<string[] | null> {
 }
 
 // Fetch base branch and use `git diff` to determine changed files
-async function getChangedFilesFromGit(ref: string): Promise<string[]> {
+async function getChangedFilesFromGit(ref: string): Promise<File[]> {
   core.debug('Fetching base branch and using `git diff-index` to determine changed files')
   await git.fetchCommit(ref)
   // FETCH_HEAD will always point to the just fetched commit
@@ -98,11 +99,11 @@ async function getChangedFilesFromGit(ref: string): Promise<string[]> {
 async function getChangedFilesFromApi(
   token: string,
   pullRequest: Webhooks.WebhookPayloadPullRequestPullRequest
-): Promise<string[]> {
+): Promise<File[]> {
   core.debug('Fetching list of modified files from Github API')
   const client = new github.GitHub(token)
   const pageSize = 100
-  const files: string[] = []
+  const files: File[] = []
   for (let page = 0; page * pageSize < pullRequest.changed_files; page++) {
     const response = await client.pulls.listFiles({
       owner: github.context.repo.owner,
@@ -112,7 +113,25 @@ async function getChangedFilesFromApi(
       per_page: pageSize
     })
     for (const row of response.data) {
-      files.push(row.filename)
+      // There's no obvious use-case for detection of renames
+      // Therefore we treat it as if rename detection in git diff was turned off.
+      // Rename is replaced by delete of original filename and add of new filename
+      if (row.status === ChangeStatus.Renamed) {
+        files.push({
+          filename: row.filename,
+          status: ChangeStatus.Added
+        })
+        files.push({
+          // 'previous_filename' for some unknown reason isn't in the type definition or documentation
+          filename: (<any>row).previous_filename as string,
+          status: ChangeStatus.Deleted
+        })
+      } else {
+        files.push({
+          filename: row.filename,
+          status: row.status as ChangeStatus
+        })
+      }
     }
   }