From 3f845744aa14c143a59150b4a0d0757308d2079b Mon Sep 17 00:00:00 2001 From: Michal Dorner Date: Sun, 30 Aug 2020 21:18:14 +0200 Subject: [PATCH] Export files matching rules (#32) * Export files matching rules * Improve debug output * Fix PR test workflow * Always quote output path + fix PR test * Use proper single quote escaping in workflow file * Improve error handling and docs for list-files input parameter --- .../workflows/pull-request-verification.yml | 26 +++-- __tests__/filter.test.ts | 58 +++++----- __tests__/shell-escape.test.ts | 16 +++ action.yml | 11 +- dist/index.js | 102 +++++++++++------- src/filter.ts | 24 +++-- src/main.ts | 86 ++++++++------- src/shell-escape.ts | 7 ++ 8 files changed, 205 insertions(+), 125 deletions(-) create mode 100644 __tests__/shell-escape.test.ts create mode 100644 src/shell-escape.ts diff --git a/.github/workflows/pull-request-verification.yml b/.github/workflows/pull-request-verification.yml index 660f26e..f248d7c 100644 --- a/.github/workflows/pull-request-verification.yml +++ b/.github/workflows/pull-request-verification.yml @@ -80,24 +80,32 @@ jobs: id: filter with: token: '' + list-files: shell filters: | - add: + added: - added: "add.txt" - rm: + deleted: - deleted: "README.md" modified: - modified: "LICENSE" any: - added|deleted|modified: "*" - - name: Print changed files - run: echo '${{steps.filter.outputs.files}}' | jq . + - name: Print 'added_files' + run: echo ${{steps.filter.outputs.added_files}} + - name: Print 'modified_files' + run: echo ${{steps.filter.outputs.modified_files}} + - name: Print 'deleted_files' + run: echo ${{steps.filter.outputs.deleted_files}} - name: filter-test + # only single quotes are supported in GH action literal + # single quote needs to be escaped with single quote + # '''add.txt''' resolves to string 'add.txt' if: | - steps.filter.outputs.add != 'true' - || steps.filter.outputs.rm != 'true' + steps.filter.outputs.added != 'true' + || steps.filter.outputs.deleted != 'true' || steps.filter.outputs.modified != 'true' || steps.filter.outputs.any != 'true' - || !contains(fromJSON(steps.filter.outputs.files).added,'add.txt') - || !contains(fromJSON(steps.filter.outputs.files).modified,'LICENSE') - || !contains(fromJSON(steps.filter.outputs.files).deleted,'README.md') + || steps.filter.outputs.added_files != '''add.txt''' + || steps.filter.outputs.modified_files != '''LICENSE''' + || steps.filter.outputs.deleted_files != '''README.md''' run: exit 1 diff --git a/__tests__/filter.test.ts b/__tests__/filter.test.ts index 15bb9f3..74b9c7c 100644 --- a/__tests__/filter.test.ts +++ b/__tests__/filter.test.ts @@ -1,4 +1,4 @@ -import Filter from '../src/filter' +import {Filter} from '../src/filter' import {File, ChangeStatus} from '../src/file' describe('yaml filter parsing tests', () => { @@ -25,8 +25,9 @@ describe('matching tests', () => { src: "src/**/*.js" ` let filter = new Filter(yaml) - const match = filter.match(modified(['src/app/module/file.js'])) - expect(match.src).toBeTruthy() + const files = modified(['src/app/module/file.js']) + const match = filter.match(files) + expect(match.src).toEqual(files) }) test('matches single rule in single group', () => { const yaml = ` @@ -34,8 +35,9 @@ describe('matching tests', () => { - src/**/*.js ` const filter = new Filter(yaml) - const match = filter.match(modified(['src/app/module/file.js'])) - expect(match.src).toBeTruthy() + const files = modified(['src/app/module/file.js']) + const match = filter.match(files) + expect(match.src).toEqual(files) }) test('no match when file is in different folder', () => { @@ -45,7 +47,7 @@ describe('matching tests', () => { ` const filter = new Filter(yaml) const match = filter.match(modified(['not_src/other_file.js'])) - expect(match.src).toBeFalsy() + expect(match.src).toEqual([]) }) test('match only within second groups ', () => { @@ -56,9 +58,10 @@ describe('matching tests', () => { - test/**/*.js ` const filter = new Filter(yaml) - const match = filter.match(modified(['test/test.js'])) - expect(match.src).toBeFalsy() - expect(match.test).toBeTruthy() + const files = modified(['test/test.js']) + const match = filter.match(files) + expect(match.src).toEqual([]) + expect(match.test).toEqual(files) }) test('match only withing second rule of single group', () => { @@ -68,18 +71,20 @@ describe('matching tests', () => { - test/**/*.js ` const filter = new Filter(yaml) - const match = filter.match(modified(['test/test.js'])) - expect(match.src).toBeTruthy() + const files = modified(['test/test.js']) + const match = filter.match(files) + expect(match.src).toEqual(files) }) test('matches anything', () => { const yaml = ` any: - - "**/*" + - "**" ` const filter = new Filter(yaml) - const match = filter.match(modified(['test/test.js'])) - expect(match.any).toBeTruthy() + const files = modified(['test/test.js']) + const match = filter.match(files) + expect(match.any).toEqual(files) }) test('globbing matches path where file or folder name starts with dot', () => { @@ -88,8 +93,9 @@ describe('matching tests', () => { - "**/*.js" ` const filter = new Filter(yaml) - const match = filter.match(modified(['.test/.test.js'])) - expect(match.dot).toBeTruthy() + const files = modified(['.test/.test.js']) + const match = filter.match(files) + expect(match.dot).toEqual(files) }) test('matches path based on rules included using YAML anchor', () => { @@ -101,9 +107,10 @@ describe('matching tests', () => { - *shared - src/**/* ` - let filter = new Filter(yaml) - const match = filter.match(modified(['config/settings.yml'])) - expect(match.src).toBeTruthy() + const filter = new Filter(yaml) + const files = modified(['config/settings.yml']) + const match = filter.match(files) + expect(match.src).toEqual(files) }) }) @@ -115,7 +122,7 @@ describe('matching specific change status', () => { ` let filter = new Filter(yaml) const match = filter.match(modified(['file.js'])) - expect(match.add).toBeFalsy() + expect(match.add).toEqual([]) }) test('match added file as added', () => { @@ -124,17 +131,20 @@ describe('matching specific change status', () => { - added: "**/*" ` let filter = new Filter(yaml) - const match = filter.match([{status: ChangeStatus.Added, filename: 'file.js'}]) - expect(match.add).toBeTruthy() + const files = [{status: ChangeStatus.Added, filename: 'file.js'}] + const match = filter.match(files) + expect(match.add).toEqual(files) }) + test('matches when multiple statuses are configured', () => { const yaml = ` addOrModify: - added|modified: "**/*" ` let filter = new Filter(yaml) - const match = filter.match([{status: ChangeStatus.Modified, filename: 'file.js'}]) - expect(match.addOrModify).toBeTruthy() + const files = [{status: ChangeStatus.Modified, filename: 'file.js'}] + const match = filter.match(files) + expect(match.addOrModify).toEqual(files) }) }) diff --git a/__tests__/shell-escape.test.ts b/__tests__/shell-escape.test.ts new file mode 100644 index 0000000..232cdd7 --- /dev/null +++ b/__tests__/shell-escape.test.ts @@ -0,0 +1,16 @@ +import shellEscape from '../src/shell-escape' + +test('simple path escaped', () => { + expect(shellEscape('file')).toBe("'file'") +}) + +test('path with space is wrapped with single quotes', () => { + expect(shellEscape('file with space')).toBe("'file with space'") +}) + +test('path with quote is divided into quoted segments and escaped quote', () => { + expect(shellEscape("file'with quote")).toBe("'file'\\''with quote'") +}) +test('path with leading quote does not have double quotes at beginning', () => { + expect(shellEscape("'file-leading-quote")).toBe("\\''file-leading-quote'") +}) diff --git a/action.yml b/action.yml index 072036b..d8411a9 100644 --- a/action.yml +++ b/action.yml @@ -18,9 +18,14 @@ inputs: filters: description: 'Path to the configuration file or YAML string with filters definition' required: false -outputs: - files: - description: 'Changed files grouped by status - added, deleted or modified.' + list-files: + description: | + Enables listing of files matching the filter: + 'none' - Disables listing of matching files (default). + 'json' - Matching files paths are serialized as JSON array. + 'shell' - Matching files paths are escaped and space-delimited. Output is usable as command line argument list in linux shell. + required: false + default: none runs: using: 'node12' main: 'dist/index.js' diff --git a/dist/index.js b/dist/index.js index 636c54e..d3f584a 100644 --- a/dist/index.js +++ b/dist/index.js @@ -4533,9 +4533,10 @@ Object.defineProperty(exports, "__esModule", { value: true }); const fs = __importStar(__webpack_require__(747)); const core = __importStar(__webpack_require__(470)); const github = __importStar(__webpack_require__(469)); -const filter_1 = __importDefault(__webpack_require__(235)); +const filter_1 = __webpack_require__(235); const file_1 = __webpack_require__(258); const git = __importStar(__webpack_require__(136)); +const shell_escape_1 = __importDefault(__webpack_require__(751)); function run() { return __awaiter(this, void 0, void 0, function* () { try { @@ -4546,22 +4547,21 @@ function run() { const token = core.getInput('token', { required: false }); const filtersInput = core.getInput('filters', { required: true }); const filtersYaml = isPathInput(filtersInput) ? getConfigFileContent(filtersInput) : filtersInput; - const filter = new filter_1.default(filtersYaml); + const listFiles = core.getInput('list-files', { required: false }).toLowerCase() || 'none'; + if (!isExportFormat(listFiles)) { + core.setFailed(`Input parameter 'list-files' is set to invalid value '${listFiles}'`); + return; + } + const filter = new filter_1.Filter(filtersYaml); const files = yield getChangedFiles(token); - let results; if (files === null) { // Change detection was not possible - core.info('All filters will be set to true.'); - results = {}; - for (const key of Object.keys(filter.rules)) { - results[key] = true; - } + exportNoMatchingResults(filter); } else { - results = filter.match(files); + const results = filter.match(files); + exportResults(results, listFiles); } - exportFiles(files !== null && files !== void 0 ? files : []); - exportResults(results); } catch (error) { core.setFailed(error.message); @@ -4669,35 +4669,42 @@ function getChangedFilesFromApi(token, pullRequest) { return files; }); } -function exportFiles(files) { - var _a; - const output = {}; - output[file_1.ChangeStatus.Added] = []; - output[file_1.ChangeStatus.Deleted] = []; - output[file_1.ChangeStatus.Modified] = []; - for (const file of files) { - const arr = (_a = output[file.status]) !== null && _a !== void 0 ? _a : []; - arr.push(file.filename); - output[file.status] = arr; - } - core.setOutput('files', output); - // Files grouped by status - for (const [status, paths] of Object.entries(output)) { - core.startGroup(`${status.toUpperCase()} files:`); - for (const filename of paths) { - core.info(filename); - } - core.endGroup(); +function exportNoMatchingResults(filter) { + core.info('All filters will be set to true but no matched files will be exported.'); + for (const key of Object.keys(filter.rules)) { + core.setOutput(key, true); } } -function exportResults(results) { - core.startGroup('Filters results:'); - for (const [key, value] of Object.entries(results)) { - core.info(`${key}: ${value}`); +function exportResults(results, format) { + for (const [key, files] of Object.entries(results)) { + const value = files.length > 0; + core.startGroup(`Filter ${key} = ${value}`); + core.info('Matching files:'); + for (const file of files) { + core.info(`${file.filename} [${file.status}]`); + } core.setOutput(key, value); + if (format !== 'none') { + const filesValue = serializeExport(files, format); + core.setOutput(`${key}_files`, filesValue); + } } core.endGroup(); } +function serializeExport(files, format) { + const fileNames = files.map(file => file.filename); + switch (format) { + case 'json': + return JSON.stringify(fileNames); + case 'shell': + return fileNames.map(shell_escape_1.default).join(' '); + default: + return ''; + } +} +function isExportFormat(value) { + return value === 'none' || value === 'shell' || value === 'json'; +} run(); @@ -4785,6 +4792,7 @@ var __importStar = (this && this.__importStar) || function (mod) { return result; }; Object.defineProperty(exports, "__esModule", { value: true }); +exports.Filter = void 0; const jsyaml = __importStar(__webpack_require__(414)); const minimatch = __importStar(__webpack_require__(595)); // Minimatch options used in all matchers @@ -4812,15 +4820,16 @@ class Filter { this.rules[key] = this.parseFilterItemYaml(item); } } - // Returns dictionary with match result per rule match(files) { const result = {}; for (const [key, patterns] of Object.entries(this.rules)) { - const match = files.some(file => patterns.some(rule => (rule.status === undefined || rule.status.includes(file.status)) && rule.matcher.match(file.filename))); - result[key] = match; + result[key] = files.filter(file => this.isMatch(file, patterns)); } return result; } + isMatch(file, patterns) { + return patterns.some(rule => (rule.status === undefined || rule.status.includes(file.status)) && rule.matcher.match(file.filename)); + } parseFilterItemYaml(item) { if (Array.isArray(item)) { return flat(item.map(i => this.parseFilterItemYaml(i))); @@ -4849,7 +4858,7 @@ class Filter { throw new Error(`Invalid filter YAML format: ${message}.`); } } -exports.default = Filter; +exports.Filter = Filter; // 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) { @@ -15288,6 +15297,23 @@ function sync (path, options) { module.exports = require("fs"); +/***/ }), + +/***/ 751: +/***/ (function(__unusedmodule, exports) { + +"use strict"; + +// Credits to https://github.com/xxorax/node-shell-escape +Object.defineProperty(exports, "__esModule", { value: true }); +function shellEscape(value) { + return `'${value.replace(/'/g, "'\\''")}'` + .replace(/^(?:'')+/g, '') // unduplicate single-quote at the beginning + .replace(/\\'''/g, "\\'"); // remove non-escaped single-quote if there are enclosed between 2 escaped +} +exports.default = shellEscape; + + /***/ }), /***/ 753: diff --git a/src/filter.ts b/src/filter.ts index e5d354f..c6070bd 100644 --- a/src/filter.ts +++ b/src/filter.ts @@ -23,7 +23,11 @@ interface FilterRuleItem { matcher: minimatch.IMinimatch // Matches the filename } -export default class Filter { +export interface FilterResults { + [key: string]: File[] +} + +export class Filter { rules: {[key: string]: FilterRuleItem[]} = {} // Creates instance of Filter and load rules from YAML if it's provided @@ -49,20 +53,20 @@ export default class Filter { } } - // Returns dictionary with match result per rule - match(files: File[]): {[key: string]: boolean} { - const result: {[key: string]: boolean} = {} + match(files: File[]): FilterResults { + const result: FilterResults = {} for (const [key, patterns] of Object.entries(this.rules)) { - const match = files.some(file => - patterns.some( - rule => (rule.status === undefined || rule.status.includes(file.status)) && rule.matcher.match(file.filename) - ) - ) - result[key] = match + result[key] = files.filter(file => this.isMatch(file, patterns)) } return result } + private isMatch(file: File, patterns: FilterRuleItem[]): boolean { + return patterns.some( + rule => (rule.status === undefined || rule.status.includes(file.status)) && rule.matcher.match(file.filename) + ) + } + private parseFilterItemYaml(item: FilterItemYaml): FilterRuleItem[] { if (Array.isArray(item)) { return flat(item.map(i => this.parseFilterItemYaml(i))) diff --git a/src/main.ts b/src/main.ts index 4498b05..458833a 100644 --- a/src/main.ts +++ b/src/main.ts @@ -3,16 +3,12 @@ import * as core from '@actions/core' import * as github from '@actions/github' import {Webhooks} from '@octokit/webhooks' -import Filter from './filter' +import {Filter, FilterResults} from './filter' import {File, ChangeStatus} from './file' import * as git from './git' +import shellEscape from './shell-escape' -interface FilterResults { - [key: string]: boolean -} -interface ActionOutput { - [key: string]: string[] -} +type ExportFormat = 'none' | 'json' | 'shell' async function run(): Promise { try { @@ -24,24 +20,23 @@ async function run(): Promise { const token = core.getInput('token', {required: false}) const filtersInput = core.getInput('filters', {required: true}) const filtersYaml = isPathInput(filtersInput) ? getConfigFileContent(filtersInput) : filtersInput + const listFiles = core.getInput('list-files', {required: false}).toLowerCase() || 'none' + + if (!isExportFormat(listFiles)) { + core.setFailed(`Input parameter 'list-files' is set to invalid value '${listFiles}'`) + return + } const filter = new Filter(filtersYaml) const files = await getChangedFiles(token) - let results: FilterResults if (files === null) { // Change detection was not possible - core.info('All filters will be set to true.') - results = {} - for (const key of Object.keys(filter.rules)) { - results[key] = true - } + exportNoMatchingResults(filter) } else { - results = filter.match(files) + const results = filter.match(files) + exportResults(results, listFiles) } - - exportFiles(files ?? []) - exportResults(results) } catch (error) { core.setFailed(error.message) } @@ -154,36 +149,45 @@ async function getChangedFilesFromApi( return files } -function exportFiles(files: File[]): void { - const output: ActionOutput = {} - output[ChangeStatus.Added] = [] - output[ChangeStatus.Deleted] = [] - output[ChangeStatus.Modified] = [] - - for (const file of files) { - const arr = output[file.status] ?? [] - arr.push(file.filename) - output[file.status] = arr - } - core.setOutput('files', output) - - // Files grouped by status - for (const [status, paths] of Object.entries(output)) { - core.startGroup(`${status.toUpperCase()} files:`) - for (const filename of paths) { - core.info(filename) - } - core.endGroup() +function exportNoMatchingResults(filter: Filter): void { + core.info('All filters will be set to true but no matched files will be exported.') + for (const key of Object.keys(filter.rules)) { + core.setOutput(key, true) } } -function exportResults(results: FilterResults): void { - core.startGroup('Filters results:') - for (const [key, value] of Object.entries(results)) { - core.info(`${key}: ${value}`) +function exportResults(results: FilterResults, format: ExportFormat): void { + for (const [key, files] of Object.entries(results)) { + const value = files.length > 0 + core.startGroup(`Filter ${key} = ${value}`) + core.info('Matching files:') + for (const file of files) { + core.info(`${file.filename} [${file.status}]`) + } + core.setOutput(key, value) + if (format !== 'none') { + const filesValue = serializeExport(files, format) + core.setOutput(`${key}_files`, filesValue) + } } core.endGroup() } +function serializeExport(files: File[], format: ExportFormat): string { + const fileNames = files.map(file => file.filename) + switch (format) { + case 'json': + return JSON.stringify(fileNames) + case 'shell': + return fileNames.map(shellEscape).join(' ') + default: + return '' + } +} + +function isExportFormat(value: string): value is ExportFormat { + return value === 'none' || value === 'shell' || value === 'json' +} + run() diff --git a/src/shell-escape.ts b/src/shell-escape.ts new file mode 100644 index 0000000..6dfe46d --- /dev/null +++ b/src/shell-escape.ts @@ -0,0 +1,7 @@ +// Credits to https://github.com/xxorax/node-shell-escape + +export default function shellEscape(value: string): string { + return `'${value.replace(/'/g, "'\\''")}'` + .replace(/^(?:'')+/g, '') // unduplicate single-quote at the beginning + .replace(/\\'''/g, "\\'") // remove non-escaped single-quote if there are enclosed between 2 escaped +}