Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Use ESLint's CLIEngine API rather than the command line (fixes #797) #873

Merged
merged 3 commits into from
Apr 6, 2017
Merged

Use ESLint's CLIEngine API rather than the command line (fixes #797) #873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
154a63b
Use ESLint's CLIEngine API rather than the command line (fixes #797)
not-an-aardvark Apr 4, 2017
8d366ca
Handle long line in a better way
not-an-aardvark Apr 6, 2017
b2c9f76
Commit message ignored because of a matching ignore pattern
not-an-aardvark Apr 6, 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
6 changes: 0 additions & 6 deletions lib/reporter.js
View file
Open in desktop

This file was deleted.

39 changes: 13 additions & 26 deletions lib/worker-helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ exports.refreshModulesPath = refreshModulesPath;
exports.getESLintInstance = getESLintInstance;
exports.getConfigPath = getConfigPath;
exports.getRelativePath = getRelativePath;
exports.getArgv = getArgv;
exports.getCLIEngineOptions = getCLIEngineOptions;

var _path = require('path');

Expand Down Expand Up @@ -103,13 +103,13 @@ function getESLintFromDirectory(modulesDir, config, projectPath) {

try {
// eslint-disable-next-line import/no-dynamic-require
return require(_path2.default.join(ESLintDirectory, 'lib', 'cli.js'));
return require(ESLintDirectory);
} catch (e) {
if (config.useGlobalEslint && e.code === 'MODULE_NOT_FOUND') {
throw new Error('ESLint not found, Please install or make sure Atom is getting $PATH correctly');
}
// eslint-disable-next-line import/no-dynamic-require
return require(_path2.default.join(Cache.ESLINT_LOCAL_PATH, 'lib', 'cli.js'));
return require(Cache.ESLINT_LOCAL_PATH);
}
}

Expand Down Expand Up @@ -158,22 +158,21 @@ function getRelativePath(fileDir, filePath, config) {
return _path2.default.basename(filePath);
}

function getArgv(type, config, rules, filePath, fileDir, givenConfigPath) {
function getCLIEngineOptions(type, config, rules, filePath, fileDir, givenConfigPath) {
let configPath;
if (givenConfigPath === null) {
configPath = config.eslintrcPath || null;
} else configPath = givenConfigPath;

const argv = [process.execPath, 'a-b-c' // dummy value for eslint executable
];
if (type === 'lint') {
argv.push('--stdin');
}
argv.push('--format', _path2.default.join(__dirname, 'reporter.js'));
const cliEngineConfig = {
rules,
ignore: !config.disableEslintIgnore,
fix: type === 'fix'
};

const ignoreFile = config.disableEslintIgnore ? null : (0, _atomLinter.findCached)(fileDir, '.eslintignore');
if (ignoreFile) {
argv.push('--ignore-path', ignoreFile);
cliEngineConfig.ignorePath = ignoreFile;
}

if (config.eslintRulesDir) {
Expand All @@ -182,24 +181,12 @@ function getArgv(type, config, rules, filePath, fileDir, givenConfigPath) {
rulesDir = (0, _atomLinter.findCached)(fileDir, rulesDir);
}
if (rulesDir) {
argv.push('--rulesdir', rulesDir);
cliEngineConfig.rulePaths = [rulesDir];
}
}
if (configPath) {
argv.push('--config', (0, _resolveEnv2.default)(configPath));
}
if (rules && Object.keys(rules).length > 0) {
argv.push('--rule', JSON.stringify(rules));
}
if (config.disableEslintIgnore) {
argv.push('--no-ignore');
}
if (type === 'lint') {
argv.push('--stdin-filename', filePath);
} else if (type === 'fix') {
argv.push(filePath);
argv.push('--fix');
cliEngineConfig.configFile = (0, _resolveEnv2.default)(configPath);
}

return argv;
return cliEngineConfig;
}
65 changes: 39 additions & 26 deletions lib/worker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,32 +33,43 @@ const ignoredMessages = [
// supress warning that the current file is ignored by eslint by default
'File ignored by default. Use a negated ignore pattern (like "--ignore-pattern \'!<relative' + '/path/to/filename>\'") to override.', 'File ignored by default. Use "--ignore-pattern \'!node_modules/*\'" to override.', 'File ignored by default. Use "--ignore-pattern \'!bower_components/*\'" to override.'];

function lintJob(argv, contents, eslint, configPath, config) {
const noProjectConfig = configPath === null || (0, _isConfigAtHomeRoot.isConfigAtHomeRoot)(configPath);
if (noProjectConfig && config.disableWhenNoEslintConfig) {
return [];
}
eslint.execute(argv, contents);
return global.__LINTER_ESLINT_RESPONSE.filter(e => !ignoredMessages.includes(e.message));
function shouldBeReported(problem) {
return !ignoredMessages.includes(problem.message);
}

function lintJob(_ref) {
let cliEngineOptions = _ref.cliEngineOptions,
contents = _ref.contents,
eslint = _ref.eslint,
filePath = _ref.filePath;

const cliEngine = new eslint.CLIEngine(cliEngineOptions);

return typeof contents === 'string' ? cliEngine.executeOnText(contents, filePath) : cliEngine.executeOnFiles([filePath]);
}

function fixJob(argv, eslint) {
const exit = eslint.execute(argv);
if (exit === 0) {
function fixJob(_ref2) {
let cliEngineOptions = _ref2.cliEngineOptions,
eslint = _ref2.eslint,
filePath = _ref2.filePath;

const report = lintJob({ cliEngineOptions, eslint, filePath });

eslint.CLIEngine.outputFixes(report);

if (!report.results.length || !report.results[0].messages.filter(shouldBeReported).length) {
return 'Linter-ESLint: Fix complete.';
}
return 'Linter-ESLint: Fix attempt complete, but linting errors remain.';
}

(0, _processCommunication.create)().onRequest('job', (_ref, job) => {
let contents = _ref.contents,
type = _ref.type,
config = _ref.config,
filePath = _ref.filePath,
projectPath = _ref.projectPath,
rules = _ref.rules;

global.__LINTER_ESLINT_RESPONSE = [];
(0, _processCommunication.create)().onRequest('job', (_ref3, job) => {
let contents = _ref3.contents,
type = _ref3.type,
config = _ref3.config,
filePath = _ref3.filePath,
projectPath = _ref3.projectPath,
rules = _ref3.rules;

if (config.disableFSCache) {
_atomLinter.FindCache.clear();
Expand All @@ -69,16 +80,18 @@ function fixJob(argv, eslint) {
const configPath = Helpers.getConfigPath(fileDir);
const relativeFilePath = Helpers.getRelativePath(fileDir, filePath, config);

const argv = Helpers.getArgv(type, config, rules, relativeFilePath, fileDir, configPath);
const cliEngineOptions = Helpers.getCLIEngineOptions(type, config, rules, relativeFilePath, fileDir, configPath);

if (type === 'lint') {
job.response = lintJob(argv, contents, eslint, configPath, config);
const noProjectConfig = configPath === null || (0, _isConfigAtHomeRoot.isConfigAtHomeRoot)(configPath);
if (noProjectConfig && config.disableWhenNoEslintConfig) {
job.response = [];
} else if (type === 'lint') {
const report = lintJob({ cliEngineOptions, contents, eslint, filePath });
job.response = report.results.length ? report.results[0].messages.filter(shouldBeReported) : [];
} else if (type === 'fix') {
job.response = fixJob(argv, eslint);
job.response = fixJob({ cliEngineOptions, eslint, filePath });
} else if (type === 'debug') {
const modulesDir = _path2.default.dirname((0, _atomLinter.findCached)(fileDir, 'node_modules/eslint') || '');
job.response = Helpers.findESLintDirectory(modulesDir, config);
}
});

process.exit = function () {/* Stop eslint from closing the daemon */};
});
0 ...eslint/lib/node_modules/eslint/lib/cli.js → ...eslint/lib/node_modules/eslint/lib/api.js
View file
Open in desktop
File renamed without changes.
3 changes: 3 additions & 0 deletions spec/fixtures/global-eslint/lib/node_modules/eslint/package.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 ...bal-eslint/node_modules/eslint/lib/cli.js → ...bal-eslint/node_modules/eslint/lib/api.js
View file
Open in desktop
File renamed without changes.
3 changes: 3 additions & 0 deletions spec/fixtures/global-eslint/node_modules/eslint/package.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 ...ing/eslint/node_modules/eslint/lib/cli.js → ...ing/eslint/node_modules/eslint/lib/api.js
View file
Open in desktop
File renamed without changes.
3 changes: 3 additions & 0 deletions spec/fixtures/indirect-local-eslint/testing/eslint/node_modules/eslint/package.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 ...cal-eslint/node_modules/eslint/lib/cli.js → ...cal-eslint/node_modules/eslint/lib/api.js
View file
Open in desktop
File renamed without changes.
3 changes: 3 additions & 0 deletions spec/fixtures/local-eslint/node_modules/eslint/package.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion spec/linter-eslint-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,21 @@ describe('The eslint provider for Linter', () => {
beforeEach(() => {
atom.config.set('linter-eslint.disableEslintIgnore', false)
})
it('will not give warnings for the file', () => {
it('will not give warnings when linting the file', () => {
waitsForPromise(() =>
atom.workspace.open(ignoredPath).then(editor =>
lint(editor).then(messages => expect(messages.length).toBe(0))
)
)
})

it('will not give warnings when autofixing the file', () => {
waitsForPromise(() =>
atom.workspace.open(ignoredPath).then(editor =>
fix(editor).then(result => expect(result).toBe('Linter-ESLint: Fix complete.'))
Copy link
Member

Choose a reason for hiding this comment

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

This works for now, and it was the previous behavior, but I think a better thing to do would be to actually tell the user that nothing happened because the file was ignored. :)

Copy link
Member

Choose a reason for hiding this comment

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

It used to be that way, but users complained, which is why it's this way in the first place. Can't please everyone I guess.

Personally I would prefer to have the messages shown myself.

Copy link
Member

Choose a reason for hiding this comment

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

I think you mean we used to show the 'file ignored' messages when linting. And yeah, that's noisy. But when a user manually runs the fix command, we should show a meaningful message. I'll push a PR later if I can get around to it.

)
)
})
})

describe('fixes errors', () => {
Expand Down
4 changes: 0 additions & 4 deletions src/reporter.js
View file
Open in desktop

This file was deleted.

37 changes: 11 additions & 26 deletions src/worker-helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,15 @@ export function getESLintFromDirectory(modulesDir, config, projectPath) {
const { path: ESLintDirectory } = findESLintDirectory(modulesDir, config, projectPath)
try {
// eslint-disable-next-line import/no-dynamic-require
return require(Path.join(ESLintDirectory, 'lib', 'cli.js'))
return require(ESLintDirectory)
} catch (e) {
if (config.useGlobalEslint && e.code === 'MODULE_NOT_FOUND') {
throw new Error(
'ESLint not found, Please install or make sure Atom is getting $PATH correctly'
)
}
// eslint-disable-next-line import/no-dynamic-require
return require(Path.join(Cache.ESLINT_LOCAL_PATH, 'lib', 'cli.js'))
return require(Cache.ESLINT_LOCAL_PATH)
}
}

Expand Down Expand Up @@ -135,24 +135,21 @@ export function getRelativePath(fileDir, filePath, config) {
return Path.basename(filePath)
}

export function getArgv(type, config, rules, filePath, fileDir, givenConfigPath) {
export function getCLIEngineOptions(type, config, rules, filePath, fileDir, givenConfigPath) {
let configPath
if (givenConfigPath === null) {
configPath = config.eslintrcPath || null
} else configPath = givenConfigPath

const argv = [
process.execPath,
'a-b-c' // dummy value for eslint executable
]
if (type === 'lint') {
argv.push('--stdin')
const cliEngineConfig = {
rules,
ignore: !config.disableEslintIgnore,
fix: type === 'fix'
}
argv.push('--format', Path.join(__dirname, 'reporter.js'))

const ignoreFile = config.disableEslintIgnore ? null : findCached(fileDir, '.eslintignore')
if (ignoreFile) {
argv.push('--ignore-path', ignoreFile)
cliEngineConfig.ignorePath = ignoreFile
}

if (config.eslintRulesDir) {
Expand All @@ -161,24 +158,12 @@ export function getArgv(type, config, rules, filePath, fileDir, givenConfigPath)
rulesDir = findCached(fileDir, rulesDir)
}
if (rulesDir) {
argv.push('--rulesdir', rulesDir)
cliEngineConfig.rulePaths = [rulesDir]
}
}
if (configPath) {
argv.push('--config', resolveEnv(configPath))
}
if (rules && Object.keys(rules).length > 0) {
argv.push('--rule', JSON.stringify(rules))
}
if (config.disableEslintIgnore) {
argv.push('--no-ignore')
}
if (type === 'lint') {
argv.push('--stdin-filename', filePath)
} else if (type === 'fix') {
argv.push(filePath)
argv.push('--fix')
cliEngineConfig.configFile = resolveEnv(configPath)
}

return argv
return cliEngineConfig
}
45 changes: 26 additions & 19 deletions src/worker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,30 @@ const ignoredMessages = [
'File ignored by default. Use "--ignore-pattern \'!bower_components/*\'" to override.',
]

function lintJob(argv, contents, eslint, configPath, config) {
const noProjectConfig = (configPath === null || isConfigAtHomeRoot(configPath))
if (noProjectConfig && config.disableWhenNoEslintConfig) {
return []
}
eslint.execute(argv, contents)
return global.__LINTER_ESLINT_RESPONSE
.filter(e => !ignoredMessages.includes(e.message))
function shouldBeReported(problem) {
return !ignoredMessages.includes(problem.message)
}

function lintJob({ cliEngineOptions, contents, eslint, filePath }) {
const cliEngine = new eslint.CLIEngine(cliEngineOptions)

return typeof contents === 'string'
? cliEngine.executeOnText(contents, filePath)
: cliEngine.executeOnFiles([filePath])
}

function fixJob(argv, eslint) {
const exit = eslint.execute(argv)
if (exit === 0) {
function fixJob({ cliEngineOptions, eslint, filePath }) {
const report = lintJob({ cliEngineOptions, eslint, filePath })

eslint.CLIEngine.outputFixes(report)

if (!report.results.length || !report.results[0].messages.filter(shouldBeReported).length) {
return 'Linter-ESLint: Fix complete.'
}
return 'Linter-ESLint: Fix attempt complete, but linting errors remain.'
}

create().onRequest('job', ({ contents, type, config, filePath, projectPath, rules }, job) => {
global.__LINTER_ESLINT_RESPONSE = []

if (config.disableFSCache) {
FindCache.clear()
}
Expand All @@ -54,16 +57,20 @@ create().onRequest('job', ({ contents, type, config, filePath, projectPath, rule
const configPath = Helpers.getConfigPath(fileDir)
const relativeFilePath = Helpers.getRelativePath(fileDir, filePath, config)

const argv = Helpers.getArgv(type, config, rules, relativeFilePath, fileDir, configPath)
const cliEngineOptions = Helpers.getCLIEngineOptions(
type, config, rules, relativeFilePath, fileDir, configPath
)

if (type === 'lint') {
job.response = lintJob(argv, contents, eslint, configPath, config)
const noProjectConfig = (configPath === null || isConfigAtHomeRoot(configPath))
if (noProjectConfig && config.disableWhenNoEslintConfig) {
job.response = []
Copy link
Member

Choose a reason for hiding this comment

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

Good call moving this check down here, it should definitely apply to lint jobs and fix jobs. 👍

} else if (type === 'lint') {
const report = lintJob({ cliEngineOptions, contents, eslint, filePath })
job.response = report.results.length ? report.results[0].messages.filter(shouldBeReported) : []
} else if (type === 'fix') {
job.response = fixJob(argv, eslint)
job.response = fixJob({ cliEngineOptions, eslint, filePath })
} else if (type === 'debug') {
const modulesDir = Path.dirname(findCached(fileDir, 'node_modules/eslint') || '')
job.response = Helpers.findESLintDirectory(modulesDir, config)
}
})

process.exit = function () { /* Stop eslint from closing the daemon */ }