From ee913b3cb34d1233c256fa1a26db3562a640dc87 Mon Sep 17 00:00:00 2001 From: Noah Koontz Date: Thu, 1 Oct 2020 10:02:04 -0700 Subject: [PATCH] feat: add file-remove fix (#181) --- docs/fixes.md | 24 +++++-- fixes/file-remove-config.json | 12 ++++ fixes/file-remove.js | 35 ++++++++++ fixes/fixes.js | 3 +- rulesets/schema.json | 3 +- tests/fixes/file_remove_tests.js | 113 +++++++++++++++++++++++++++++++ 6 files changed, 181 insertions(+), 9 deletions(-) create mode 100644 fixes/file-remove-config.json create mode 100644 fixes/file-remove.js create mode 100644 tests/fixes/file_remove_tests.js diff --git a/docs/fixes.md b/docs/fixes.md index 94b6c1ec..a0a71efa 100644 --- a/docs/fixes.md +++ b/docs/fixes.md @@ -8,6 +8,7 @@ Below is a complete list of fixes that Repolinter can run, along with their conf - [Reference](#reference) - [`file-create`](#file-create) - [`file-modify`](#file-modify) + - [`file-remove`](#file-remove) ## Reference @@ -25,10 +26,19 @@ Creates a file. Optionally removes or replaces files that failed. Modify a file that failed a rule. -| Input | Required | Type | Default | Description | -| --------------------- | -------- | ---------------------------------------------------------------- | ----------------------------------- | --------------------------------------------------------------------------------------------------------------------------- | -| `text` | **Yes** | `string` \| `{ url: string }` \| `{ file: string }` | | The text to create the file with. Specify an object with the `url` or `file` property to pull text from a URL or file. | -| `files` | No | `string[]` | Failing files specified by the rule | A list of globs to modify files with. If this value is omitted, file-modify will instead target files that failed the rule. | -| `skip-paths-matching` | No | `{ extensions?: string[], patterns?: string[], flags?: string }` | `{}` | Use this option to exclude paths from `files`, either by file extension or by regular expression. | -| `write_mode` | No | `"prepend"` \| `"append"` | `"append"` | How file-modify should edit the file. | -| `newlines` | No | `{ begin?: number, end?: number }` | `{ begin: 0, end: 0 }` | How many newlines should be added to the start or end of `text`. This property allows formatters to print `text` without these newlines, creating a better user experience. | +| Input | Required | Type | Default | Description | +| --------------------- | -------- | ---------------------------------------------------------------- | ----------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `text` | **Yes** | `string` \| `{ url: string }` \| `{ file: string }` | | The text to create the file with. Specify an object with the `url` or `file` property to pull text from a URL or file. | +| `files` | No | `string[]` | Failing files specified by the rule | A list of globs to modify files with. If this value is omitted, file-modify will instead target files that failed the rule. | +| `skip-paths-matching` | No | `{ extensions?: string[], patterns?: string[], flags?: string }` | `{}` | Use this option to exclude paths from `files`, either by file extension or by regular expression. | +| `write_mode` | No | `"prepend"` \| `"append"` | `"append"` | How file-modify should edit the file. | +| `newlines` | No | `{ begin?: number, end?: number }` | `{ begin: 0, end: 0 }` | How many newlines should be added to the start or end of `text`. This property allows formatters to print `text` without these newlines, creating a better user experience. | + +### `file-remove` + +Removes a file or files. + +| Input | Required | Type | Default | Description | +| ---------- | -------- | ---------- | ----------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | +| `globsAll` | No | `string[]` | Failing files specified by the rule | The list of globs to remove files for. If this value is omitted, file-remove will instead target files that failed the rule. | +| `nocase` | No | `boolean` | `false` | Set to `true` to perform an case insensitive search with `globsAll`. | diff --git a/fixes/file-remove-config.json b/fixes/file-remove-config.json new file mode 100644 index 00000000..8c7aa850 --- /dev/null +++ b/fixes/file-remove-config.json @@ -0,0 +1,12 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema", + "$id": "https://raw.githubusercontent.com/todogroup/repolinter/master/fixes/file-remove-config.json", + "type": "object", + "properties": { + "globsAll": { + "type": "array", + "items": { "type": "string" } + }, + "nocase": { "type": "boolean", "default": false } + } +} diff --git a/fixes/file-remove.js b/fixes/file-remove.js new file mode 100644 index 00000000..e6d6c6d7 --- /dev/null +++ b/fixes/file-remove.js @@ -0,0 +1,35 @@ +// Copyright 2017 TODO Group. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +const Result = require('../lib/result') +// eslint-disable-next-line no-unused-vars +const FileSystem = require('../lib/file_system') + +/** + * Removes a file or a list of files. + * + * @param {FileSystem} fs A filesystem object configured with filter paths and target directories + * @param {object} options The rule configuration + * @param {string[]} targets The files to modify (will be overridden by options if present) + * @param {boolean} dryRun If true, repolinter will report suggested fixes, but will make no disk modifications. + * @returns {Promise} The fix result + */ +async function fileRemove (fs, options, targets, dryRun = false) { + // overwrite the targets with the files specified by options + if (options.globsAll && options.globsAll.length) { + targets = await fs.findAllFiles(options.globsAll, !!options.nocase) + } + // check that any targets exist + if (targets.length === 0) { + return new Result('Found no files to remove', [], false) + } + // remove the files + if (!dryRun) { + await Promise.all(targets.map(async t => fs.removeFile(t))) + } + // create a result + const removeTargets = targets.map(t => { return { passed: true, path: t, message: 'Remove file' } }) + return new Result('', removeTargets, true) +} + +module.exports = fileRemove diff --git a/fixes/fixes.js b/fixes/fixes.js index 235d51df..5b56cd90 100644 --- a/fixes/fixes.js +++ b/fixes/fixes.js @@ -3,5 +3,6 @@ module.exports = [ 'file-create', - 'file-modify' + 'file-modify', + 'file-remove' ] diff --git a/rulesets/schema.json b/rulesets/schema.json index eeb29cbb..38445bb0 100644 --- a/rulesets/schema.json +++ b/rulesets/schema.json @@ -66,7 +66,8 @@ "default": {}, "allOf": [ { "if": { "properties": { "type": { "const": "file-modify" } } }, "then": { "properties": { "options": { "$ref": "../fixes/file-modify-config.json" } } } }, - { "if": { "properties": { "type": { "const": "file-create" } } }, "then": { "properties": { "options": { "$ref": "../fixes/file-create-config.json" } } } } + { "if": { "properties": { "type": { "const": "file-create" } } }, "then": { "properties": { "options": { "$ref": "../fixes/file-create-config.json" } } } }, + { "if": { "properties": { "type": { "const": "file-remove" } } }, "then": { "properties": { "options": { "$ref": "../fixes/file-remove-config.json" } } } } ] }, "policyInfo": { "type": "string" }, diff --git a/tests/fixes/file_remove_tests.js b/tests/fixes/file_remove_tests.js new file mode 100644 index 00000000..808a56b0 --- /dev/null +++ b/tests/fixes/file_remove_tests.js @@ -0,0 +1,113 @@ +// Copyright 2017 TODO Group. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +const chai = require('chai') +const expect = chai.expect + +describe('fixes', () => { + describe('file-remove', () => { + const fileRemove = require('../../fixes/file-remove') + + it('removes a file', async () => { + const removePaths = [] + /** @type {any} */ + const mockFs = { + removeFile (path) { + removePaths.push(path) + } + } + + const res = await fileRemove(mockFs, {}, ['myfile'], false) + expect(res.passed).to.equal(true) + expect(res.targets).to.have.length(1) + expect(res.targets[0].passed).to.equal(true) + expect(res.targets[0].path).to.equal('myfile') + expect(removePaths).to.deep.equal(['myfile']) + }) + + it('does nothing if dryRun is true', async () => { + const removePaths = [] + /** @type {any} */ + const mockFs = { + removeFile (path) { + removePaths.push(path) + } + } + + const res = await fileRemove(mockFs, {}, ['myfile'], true) + expect(res.passed).to.equal(true) + expect(res.targets).to.have.length(1) + expect(res.targets[0].passed).to.equal(true) + expect(res.targets[0].path).to.equal('myfile') + expect(removePaths).to.deep.equal([]) + }) + + it('removes multiple files', async () => { + const removePaths = [] + /** @type {any} */ + const mockFs = { + removeFile (path) { + removePaths.push(path) + } + } + + const res = await fileRemove(mockFs, {}, ['myfile', 'otherfile'], false) + expect(res.passed).to.equal(true) + expect(res.targets).to.have.length(2) + expect(res.targets[0].passed).to.equal(true) + expect(res.targets[0].path).to.equal('myfile') + expect(res.targets[1].passed).to.equal(true) + expect(res.targets[1].path).to.equal('otherfile') + expect(removePaths).to.deep.equal(['myfile', 'otherfile']) + }) + + it('uses the glob option', async () => { + const removePaths = [] + /** @type {any} */ + const mockFs = { + removeFile (path) { + removePaths.push(path) + }, + findAllFiles () { + return ['myfile.txt'] + } + } + + const res = await fileRemove(mockFs, { globsAll: ['myfile'] }, [], false) + expect(res.passed).to.equal(true) + expect(res.targets).to.have.length(1) + expect(res.targets[0].passed).to.equal(true) + expect(res.targets[0].path).to.equal('myfile.txt') + expect(removePaths).to.deep.equal(['myfile.txt']) + }) + + it('overrides targets with the glob option', async () => { + const removePaths = [] + /** @type {any} */ + const mockFs = { + removeFile (path) { + removePaths.push(path) + }, + findAllFiles () { + return ['myfile.txt'] + } + } + + const res = await fileRemove(mockFs, { globsAll: ['myfile'] }, ['otherfile'], false) + expect(res.passed).to.equal(true) + expect(res.targets).to.have.length(1) + expect(res.targets[0].passed).to.equal(true) + expect(res.targets[0].path).to.equal('myfile.txt') + expect(removePaths).to.deep.equal(['myfile.txt']) + }) + + it('returns failure if no files are found', async () => { + /** @type {any} */ + const mockFs = {} + + const res = await fileRemove(mockFs, {}, [], false) + expect(res.passed).to.equal(false) + expect(res.targets).to.have.length(0) + }) + }) +})