-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add any-file-contents rule #290
base: main
Are you sure you want to change the base?
Changes from all commits
3505753
1009423
913ce14
0c197ec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
{ | ||
"$schema": "http://json-schema.org/draft-07/schema", | ||
"$id": "https://raw.githubusercontent.com/todogroup/repolinter/master/rules/any-file-contents-config.json", | ||
"type": "object", | ||
"properties": { | ||
"nocase": { | ||
"type": "boolean", | ||
"default": false | ||
}, | ||
"globsAny": { | ||
"type": "array", | ||
"items": { "type": "string" } | ||
}, | ||
"content": { "type": "string" }, | ||
"flags": { "type": "string" }, | ||
"human-readable-content": { "type": "string" }, | ||
"fail-on-non-existent": { | ||
"type": "boolean", | ||
"default": false | ||
} | ||
}, | ||
"required": ["content"], | ||
"oneOf": [{ "required": ["globsAny"] }, { "required": ["files"] }] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
// Copyright 2017 TODO Group. All rights reserved. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
// eslint-disable-next-line no-unused-vars | ||
const Result = require('../lib/result') | ||
// eslint-disable-next-line no-unused-vars | ||
const FileSystem = require('../lib/file_system') | ||
const fileContents = require('./file-contents') | ||
|
||
/** | ||
* Check that at least one file within a list contains a regular expression. | ||
* | ||
* @param {FileSystem} fs A filesystem object configured with filter paths and target directories | ||
* @param {object} options The rule configuration | ||
* @returns {Promise<Result>} The lint rule result | ||
*/ | ||
function anyFileContents(fs, options) { | ||
return fileContents(fs, options, false, true) | ||
} | ||
|
||
module.exports = anyFileContents |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,9 +32,9 @@ function getContext(matchedLine, regexMatch, contextLength) { | |
* @returns {Promise<Result>} The lint rule result | ||
* @ignore | ||
*/ | ||
async function fileContents(fs, options, not = false) { | ||
async function fileContents(fs, options, not = false, any = false) { | ||
// support legacy configuration keys | ||
const fileList = options.globsAll || options.files | ||
const fileList = (any ? options.globsAny : options.globsAll) || options.files | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's not change the You can move this part to |
||
const files = await fs.findAllFiles(fileList, !!options.nocase) | ||
const regexFlags = options.flags || '' | ||
|
||
|
@@ -272,7 +272,9 @@ async function fileContents(fs, options, not = false) { | |
} | ||
|
||
const filteredResults = results.filter(r => r !== null) | ||
const passed = !filteredResults.find(r => !r.passed) | ||
const passed = any | ||
? filteredResults.some(r => r.passed) | ||
: !filteredResults.find(r => !r.passed) | ||
return new Result('', filteredResults, passed) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
// Copyright 2017 TODO Group. All rights reserved. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
const chai = require('chai') | ||
const expect = chai.expect | ||
const FileSystem = require('../../lib/file_system') | ||
|
||
describe('rule', () => { | ||
describe('any_file_contents', () => { | ||
const anyFileContents = require('../../rules/any-file-contents') | ||
|
||
it('returns passes if requested file contents exists in exactly one file', async () => { | ||
/** @type {any} */ | ||
const mockfs = { | ||
findAllFiles() { | ||
return ['x.md', 'CONTRIBUTING.md', 'README.md'] | ||
}, | ||
getFileContents(file) { | ||
return file === 'README.md' ? 'foo' : 'bar' | ||
}, | ||
targetDir: '.' | ||
} | ||
const ruleopts = { | ||
globsAny: ['README*', 'x.md', 'CONTRIBUTING.md'], | ||
content: '[abcdef][oO0][^q]' | ||
} | ||
|
||
const actual = await anyFileContents(mockfs, ruleopts) | ||
expect(actual.passed).to.equal(true) | ||
expect(actual.targets).to.have.length(3) | ||
expect(actual.targets[2]).to.deep.include({ | ||
passed: true, | ||
path: 'README.md' | ||
}) | ||
}) | ||
|
||
it('returns passes if requested file contents exists in two files', async () => { | ||
/** @type {any} */ | ||
const mockfs = { | ||
findAllFiles() { | ||
return ['x.md', 'CONTRIBUTING.md', 'README.md'] | ||
}, | ||
getFileContents(file) { | ||
return file === 'README.md' ? 'bar' : 'foo' | ||
}, | ||
targetDir: '.' | ||
} | ||
const ruleopts = { | ||
globsAny: ['README*', 'x.md', 'CONTRIBUTING.md'], | ||
content: '[abcdef][oO0][^q]' | ||
} | ||
|
||
const actual = await anyFileContents(mockfs, ruleopts) | ||
|
||
expect(actual.passed).to.equal(true) | ||
expect(actual.targets).to.have.length(3) | ||
expect(actual.targets[0]).to.deep.include({ | ||
passed: true, | ||
path: 'x.md' | ||
}) | ||
expect(actual.targets[1]).to.deep.include({ | ||
passed: true, | ||
path: 'CONTRIBUTING.md' | ||
}) | ||
expect(actual.targets[2]).to.deep.include({ | ||
passed: false, | ||
path: 'README.md' | ||
}) | ||
}) | ||
|
||
it('returns fails if the requested file contents does not exist in any file', async () => { | ||
/** @type {any} */ | ||
const mockfs = { | ||
findAllFiles() { | ||
return ['x.md', 'CONTRIBUTING.md', 'README.md'] | ||
}, | ||
getFileContents() { | ||
return 'bar' | ||
}, | ||
targetDir: '.' | ||
} | ||
const ruleopts = { | ||
globsAny: ['README*', 'x.md', 'CONTRIBUTING.md'], | ||
content: '[abcdef][oO0][^q]' | ||
} | ||
|
||
const actual = await anyFileContents(mockfs, ruleopts) | ||
expect(actual.passed).to.equal(false) | ||
expect(actual.targets).to.have.length(3) | ||
expect(actual.targets[0]).to.deep.include({ | ||
passed: false, | ||
path: 'x.md' | ||
}) | ||
expect(actual.targets[1]).to.deep.include({ | ||
passed: false, | ||
path: 'CONTRIBUTING.md' | ||
}) | ||
}) | ||
|
||
it('returns failure if no file exists with failure flag enabled', async () => { | ||
/** @type {any} */ | ||
const mockfs = { | ||
findAllFiles() { | ||
return [] | ||
}, | ||
getFileContents() {}, | ||
targetDir: '.' | ||
} | ||
|
||
const ruleopts = { | ||
globsAny: ['README.md', 'READMOI.md'], | ||
content: 'foo', | ||
'fail-on-non-existent': true | ||
} | ||
|
||
const actual = await anyFileContents(mockfs, ruleopts) | ||
|
||
expect(actual.passed).to.equal(false) | ||
}) | ||
|
||
it('should handle broken symlinks', async () => { | ||
const brokenSymlink = './tests/rules/broken_symlink_for_test' | ||
const stat = require('fs').lstatSync(brokenSymlink) | ||
expect(stat.isSymbolicLink()).to.equal(true) | ||
const fs = new FileSystem(require('path').resolve('.')) | ||
|
||
const ruleopts = { | ||
globsAny: [brokenSymlink], | ||
lineCount: 1, | ||
patterns: ['something'], | ||
'fail-on-non-existent': true | ||
} | ||
const actual = await anyFileContents(fs, ruleopts) | ||
expect(actual.passed).to.equal(false) | ||
}) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quote:
I think it doesn't make sense for the rule to pass if no files are returned. It would make sense if the rule fails if no files are returned., since we are looking for at least there is one file has this thing there.
That means the
fail-on-non-existent
would besuccess-on-non-existent
.