Skip to content

Commit

Permalink
New: Support glob path on command line (fixes #3402)
Browse files Browse the repository at this point in the history
  • Loading branch information
BYK committed Aug 28, 2015
1 parent a7f6b67 commit 8e39613
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 271 deletions.
72 changes: 60 additions & 12 deletions lib/cli-engine.js
Expand Up @@ -21,10 +21,11 @@ var fs = require("fs"),

assign = require("object-assign"),
debug = require("debug"),
glob = require("glob"),
shell = require("shelljs"),

rules = require("./rules"),
eslint = require("./eslint"),
traverse = require("./util/traverse"),
IgnoredPaths = require("./ignored-paths"),
Config = require("./config"),
util = require("./util"),
Expand Down Expand Up @@ -232,7 +233,7 @@ function processFile(filename, configHelper) {
*/
function createIgnoreResult(filePath) {
return {
filePath: filePath,
filePath: path.resolve(filePath),
messages: [
{
fatal: false,
Expand All @@ -256,6 +257,31 @@ function isErrorMessage(message) {
return message.severity === 2;
}


/**
* Checks if a provided path is a directory and returns a glob string matching
* all files under that directory if so, the path itself otherwise.
*
* Reason for this is that `glob` needs `/**` to collect all the files under a
* directory where as our previous implementation without `glob` simply walked
* a directory that is passed. So this is to maintain backwards compatibility.
*
* Also makes sure all path separators are POSIX style for `glob` compatibility.
*
* @param {string} pathname The directory path to be modified
* @returns {string} The glob path or the file path itself
*/
function processPath(pathname) {
var newPath = pathname;

if (shell.test("-d", pathname)) {
newPath = path.join(pathname, "**");
}

return newPath.replace(/\\/g, "/");
}


//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -372,29 +398,51 @@ CLIEngine.prototype = {
executeOnFiles: function(files) {

var results = [],
processed = [],
processed = {},
options = this.options,
configHelper = new Config(options),
ignoredPaths = IgnoredPaths.load(options),
exclude = ignoredPaths.contains.bind(ignoredPaths),
ignoredPaths = options.ignore ? IgnoredPaths.load(options).patterns : [],
extensions = options.extensions.map(function(ext) {
return ext.charAt(0) === "." ? ext : "." + ext;
}),
globOptions,
stats;

traverse({
files: files,
extensions: options.extensions,
exclude: options.ignore ? exclude : false
}, function(filename) {
files = files.map(processPath);
ignoredPaths = ignoredPaths.map(processPath);

globOptions = {
nodir: true,
realpath: true,
ignore: ignoredPaths
};

/**
* Executes the linter on a file defined by the `filename`. Skips
* unsupported file extensions and any files that are already linted.
* @param {string} filename The file to be linted
* @returns {void}
*/
function executeOnFile(filename) {
if (extensions.indexOf(path.extname(filename)) === -1 || processed[filename]) {
return;
}

debug("Processing " + filename);

processed.push(filename);
processed[filename] = true;
results.push(processFile(filename, configHelper));
}

files.forEach(function(pattern) {
glob.sync(pattern, globOptions).forEach(executeOnFile);
});

// only warn for files explicitly passed on the command line
if (options.ignore) {
files.forEach(function(file) {
if (fs.statSync(path.resolve(file)).isFile() && processed.indexOf(file) === -1) {
var fullPath = path.resolve(file);
if (shell.test("-e", fullPath) && !processed[fullPath]) {
results.push(createIgnoreResult(file));
}
});
Expand Down
107 changes: 0 additions & 107 deletions lib/util/traverse.js

This file was deleted.

3 changes: 2 additions & 1 deletion package.json
Expand Up @@ -44,6 +44,7 @@
"espree": "^2.2.4",
"estraverse": "^4.1.0",
"estraverse-fb": "^1.3.1",
"glob": "^5.0.14",
"globals": "^8.5.0",
"handlebars": "^3.0.3",
"inquirer": "^0.9.0",
Expand All @@ -59,6 +60,7 @@
"optionator": "^0.5.0",
"path-is-absolute": "^1.0.0",
"path-is-inside": "^1.0.1",
"shelljs": "^0.3.0",
"strip-json-comments": "~1.0.1",
"text-table": "~0.2.0",
"user-home": "^1.0.0",
Expand Down Expand Up @@ -87,7 +89,6 @@
"proxyquire": "^1.0.0",
"rewire": "^2.3.4",
"semver": "^4.1.0",
"shelljs": "^0.3.0",
"shelljs-nodecli": "~0.1.0",
"sinon": "1.14.1",
"through": "^2.3.6"
Expand Down
31 changes: 28 additions & 3 deletions tests/lib/cli-engine.js
Expand Up @@ -120,7 +120,7 @@ describe("CLIEngine", function() {
assert.equal(report.results.length, 1);
assert.equal(report.errorCount, 0);
assert.equal(report.warningCount, 1);
assert.equal(report.results[0].filePath, "tests/fixtures/passing.js");
assert.equal(report.results[0].filePath, path.resolve("tests/fixtures/passing.js"));
assert.equal(report.results[0].messages[0].severity, 1);
assert.equal(report.results[0].messages[0].message, "File ignored because of your .eslintignore file. Use --no-ignore to override.");
assert.equal(report.results[0].errorCount, 0);
Expand Down Expand Up @@ -157,9 +157,22 @@ describe("CLIEngine", function() {
configFile: path.join(__dirname, "..", "..", ".eslintrc")
});

var report = engine.executeOnFiles(["lib/cli.js"]);
assert.equal(report.results.length, 1);
var report = engine.executeOnFiles(["lib/cli*.js"]);
assert.equal(report.results.length, 2);
assert.equal(report.results[0].messages.length, 0);
assert.equal(report.results[1].messages.length, 0);
});

it("should handle multiple patterns with overlapping files", function() {

engine = new CLIEngine({
configFile: path.join(__dirname, "..", "..", ".eslintrc")
});

var report = engine.executeOnFiles(["lib/cli*.js", "lib/cli.?s", "lib/{cli,cli-engine}.js"]);
assert.equal(report.results.length, 2);
assert.equal(report.results[0].messages.length, 0);
assert.equal(report.results[1].messages.length, 0);
});

it("should report zero messages when given a directory with a .js2 file", function() {
Expand All @@ -185,6 +198,18 @@ describe("CLIEngine", function() {
assert.equal(report.results[1].messages.length, 0);
});

it("should report zero messages when given a pattern with a .js and a .js2 file", function() {

engine = new CLIEngine({
extensions: [".js", ".js2"]
});

var report = engine.executeOnFiles(["tests/fixtures/files/*.?s*"]);
assert.equal(report.results.length, 2);
assert.equal(report.results[0].messages.length, 0);
assert.equal(report.results[1].messages.length, 0);
});

it("should return one error message when given a config with rules with options and severity level set to error", function() {

engine = new CLIEngine({
Expand Down
12 changes: 5 additions & 7 deletions tests/lib/cli.js
Expand Up @@ -19,8 +19,6 @@ var assert = require("chai").assert,

proxyquire = proxyquire.noCallThru().noPreserveCache();

/* global tempdir, mkdir, rm, cp */

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------
Expand All @@ -42,9 +40,9 @@ describe("cli", function() {

// copy into clean area so as not to get "infected" by this project's .eslintrc files
before(function() {
fixtureDir = tempdir() + "/eslint/fixtures";
mkdir("-p", fixtureDir);
cp("-r", "./tests/fixtures/.", fixtureDir);
fixtureDir = sh.tempdir() + "/eslint/fixtures";
sh.mkdir("-p", fixtureDir);
sh.cp("-r", "./tests/fixtures/.", fixtureDir);
});

beforeEach(function() {
Expand All @@ -58,7 +56,7 @@ describe("cli", function() {
});

after(function() {
rm("-r", fixtureDir);
sh.rm("-r", fixtureDir);
});

describe("execute()", function() {
Expand Down Expand Up @@ -496,7 +494,7 @@ describe("cli", function() {

cli.execute(code);

assert.include(fs.readFileSync("tests/output/eslint-output.txt", "utf8"), "tests/fixtures/single-quoted.js");
assert.include(fs.readFileSync("tests/output/eslint-output.txt", "utf8"), path.join("tests", "fixtures", "single-quoted.js"));
assert.isTrue(console.log.notCalled);
});

Expand Down

0 comments on commit 8e39613

Please sign in to comment.