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.

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

Chore: Make parseJsonConfig() a pure function in Linter #9186

Merged
merged 1 commit into from Sep 1, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
69 changes: 44 additions & 25 deletions lib/linter.js
Expand Up @@ -87,10 +87,9 @@ function parseBooleanConfig(string, comment) {
* Parses a JSON-like config.
* @param {string} string The string to parse.
* @param {Object} location Start line and column of comments for potential error message.
* @param {Object[]} messages The messages queue for potential error message.
* @returns {Object} Result map object
* @returns {({success: true, config: Object}|{success: false, error: Problem})} Result map object
*/
function parseJsonConfig(string, location, messages) {
function parseJsonConfig(string, location) {
let items = {};

// Parses a JSON-like comment by the same way as parsing CLI option.
Expand All @@ -102,7 +101,10 @@ function parseJsonConfig(string, location, messages) {
// "no-alert: 2 no-console: 2" --> {"no-alert": "2 no-console: 2"}
// Should ignore that case as well.
if (ConfigOps.isEverySeverityValid(items)) {
return items;
return {
success: true,
config: items
};
}
} catch (ex) {

Expand All @@ -116,20 +118,25 @@ function parseJsonConfig(string, location, messages) {
try {
items = JSON.parse(`{${string}}`);
} catch (ex) {

messages.push({
ruleId: null,
fatal: true,
severity: 2,
source: null,
message: `Failed to parse JSON from '${string}': ${ex.message}`,
line: location.start.line,
column: location.start.column + 1
});
return {
success: false,
error: {
ruleId: null,
fatal: true,
severity: 2,
source: null,
message: `Failed to parse JSON from '${string}': ${ex.message}`,
line: location.start.line,
column: location.start.column + 1
}
};

}

return items;
return {
success: true,
config: items
};
}

/**
Expand Down Expand Up @@ -316,7 +323,8 @@ function enableReporting(reportingConfig, start, rulesToEnable) {
* @param {ASTNode} ast The top node of the AST.
* @param {Object} config The existing configuration data.
* @param {Linter} linterContext Linter context object
* @returns {Object} Modified config object
* @returns {{config: Object, problems: Problem[]}} Modified config object, along with any problems encountered
* while parsing config comments
*/
function modifyConfigsFromComments(filename, ast, config, linterContext) {

Expand All @@ -327,7 +335,7 @@ function modifyConfigsFromComments(filename, ast, config, linterContext) {
env: {}
};
const commentRules = {};
const messages = linterContext.messages;
const problems = [];
const reportingConfig = linterContext.reportingConfig;

ast.comments.forEach(comment => {
Expand Down Expand Up @@ -362,14 +370,19 @@ function modifyConfigsFromComments(filename, ast, config, linterContext) {
break;

case "eslint": {
const items = parseJsonConfig(value, comment.loc, messages);
const parseResult = parseJsonConfig(value, comment.loc);

if (parseResult.success) {
Object.keys(parseResult.config).forEach(name => {
const ruleValue = parseResult.config[name];

Object.keys(items).forEach(name => {
const ruleValue = items[name];
validator.validateRuleOptions(name, ruleValue, `${filename} line ${comment.loc.start.line}`, linterContext.rules);
commentRules[name] = ruleValue;
});
} else {
problems.push(parseResult.error);
}

validator.validateRuleOptions(name, ruleValue, `${filename} line ${comment.loc.start.line}`, linterContext.rules);
commentRules[name] = ruleValue;
});
break;
}

Expand Down Expand Up @@ -397,7 +410,10 @@ function modifyConfigsFromComments(filename, ast, config, linterContext) {
});
Object.assign(commentConfig.rules, commentRules);

return ConfigOps.merge(config, commentConfig);
return {
config: ConfigOps.merge(config, commentConfig),
problems
};
}

/**
Expand Down Expand Up @@ -866,7 +882,10 @@ class Linter {

// parse global comments and modify config
if (allowInlineConfig !== false) {
config = modifyConfigsFromComments(this.currentFilename, this.sourceCode.ast, config, this);
const modifyConfigResult = modifyConfigsFromComments(this.currentFilename, this.sourceCode.ast, config, this);

config = modifyConfigResult.config;
modifyConfigResult.problems.forEach(problem => this.messages.push(problem));
}

// ensure that severities are normalized in the config
Expand Down