Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Dont lint ignored files #86

Merged
merged 3 commits into from
Mar 19, 2017
Merged

Dont lint ignored files #86

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3a83659
Don't call processString if file is ignored.
LucasHill Mar 19, 2017
9355e9b
Remove no longer neccessary filtering.
LucasHill Mar 19, 2017
84e9ccc
Cleanup comments.
LucasHill Mar 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 15 additions & 32 deletions lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ const stringify = require('json-stable-stringify');
const path = require('path');
const escapeStringRegexp = require('escape-string-regexp');
const BUILD_DIR_REGEXP = new RegExp(`(${escapeStringRegexp(path.sep)})?build(${escapeStringRegexp(path.sep)})?$`);
const IGNORED_FILE_MESSAGE_REGEXP = /(?:File ignored by default\.)|(?:File ignored because of a matching ignore pattern\.)/;

/**
* Calculates the severity of a eslint.linter.verify result
Expand All @@ -31,32 +30,6 @@ function getResultSeverity(resultMessages = []) {
}, 0);
}

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole filtering logic is superseded by the new guard which keeps ignored files from being linted on at all. So it is no longer necessary. I kept the tests in to ensure however.

* Ignores messages that are about ignored files as they are intended
* but we are processing a file at a time
*
* @param {Array} errors result errors
* @returns {Array} filtered errors
*/
function filterIgnoredFileMessages(errors) {
return errors.filter((error) => !IGNORED_FILE_MESSAGE_REGEXP.test(error.message));
}

/**
* Filters all ignored file messages from result object
* @param {Object} result result errors
* @returns {Object} filtered results
*/
function filterAllIgnoredFileMessages(result) {
const resultOutput = result;

resultOutput.results.forEach((resultItem) => {
resultItem.messages = filterIgnoredFileMessages(resultItem.messages);
});

return resultOutput;
}

function isString(x) {
return toString.call(x) === '[object String]';
}
Expand Down Expand Up @@ -156,8 +129,8 @@ EslintValidationFilter.prototype.cacheKeyProcessString = function cacheKeyProces
return value;
}

var filePath = path.join(this.eslintrc, relativePath);
var isIgnoredFile = this.cli.isPathIgnored(filePath);
const filePath = path.join(this.eslintrc, relativePath);
const isIgnoredFile = this.cli.isPathIgnored(filePath);

return md5Hex([
content,
Expand All @@ -168,16 +141,26 @@ EslintValidationFilter.prototype.cacheKeyProcessString = function cacheKeyProces
]);
};

EslintValidationFilter.prototype.getDestFilePath = function getDestFilePath(relativePath) {
const filterPath = Filter.prototype.getDestFilePath.call(this, relativePath);
const fullPath = path.join(this.eslintrc, relativePath);

if(filterPath && !this.cli.isPathIgnored(fullPath)) {
return filterPath;
} else {
return null
}
}

EslintValidationFilter.prototype.processString = function processString(content, relativePath) {
// verify file content
const configPath = path.join(this.eslintrc, relativePath);
const report = this.cli.executeOnText(content, configPath);
const filteredReport = filterAllIgnoredFileMessages(report);

const toCache = { report, output: content };

if (this.testGenerator && Array.isArray(filteredReport.results)) {
const result = filteredReport.results[0] || {};
if (this.testGenerator && Array.isArray(report.results)) {
const result = report.results[0] || {};
const messages = result.messages || [];

toCache.output = this.testGenerator(relativePath, messages, result);
Expand Down
26 changes: 25 additions & 1 deletion test/main-tests/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const FIXTURES_PATH_ESLINTIGNORE_FOR_WARNING = path.resolve(process.cwd(), './te
const FIXTURES_PATH_ESLINTRC = path.join(FIXTURES_PATH, '.eslintrc.js');
const FIXTURES_PATH_ESLINTRC_ALTERNATE = path.join(FIXTURES_PATH, '.eslintrc-alternate.js');
const FIXTURE_FILE_PATH_ALERT = 'fixtures/alert.js';
const JS_FIXTURES = fs.readdirSync(FIXTURES_PATH).filter((name) => /\.js$/.test(name));
const JS_FIXTURES = fs.readdirSync(FIXTURES_PATH).filter((name) => /\.js$/.test(name) && !/^.eslintrc/.test(name));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before it was actually linting the eslintrc files. The eslint API considers these an "ignored file" by default, and they are no longer linted, so the count of JS_FIXTURES needed to be reduced to exclude them.



describe('EslintValidationFilter', function() {
Expand Down Expand Up @@ -180,6 +180,30 @@ describe('EslintValidationFilter', function() {
});
});

it('should not call processString for ignored files', function() {
const {
processStringSpy
} = this.setupSpies();

function runNonpersistent() {
return runEslint(FIXTURES_PATH, {
persist: false,
options: {
ignorePath: FIXTURES_PATH_ESLINTIGNORE
}
});
}

// Run twice to guarantee one run would be from cache if persisting
const promise = runNonpersistent();

return promise.then(function() {
// check that it did not use the cache
expect(processStringSpy, 'Doesn\'t call processString for ignored files')
.to.have.callCount(JS_FIXTURES.length - 1);
});
});

it('should allow disabling the cache', function() {
const {
processStringSpy,
Expand Down