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

Chore: refactor directive comment processing #10007

Merged
merged 1 commit into from Feb 24, 2018
Merged

Chore: refactor directive comment processing #10007

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
214 changes: 98 additions & 116 deletions lib/linter.js
View file
Open in desktop
Expand Up @@ -179,12 +179,12 @@ function parseListConfig(string) {
* scope.
* @param {Scope} globalScope The global scope.
* @param {Object} config The existing configuration data.
* @param {{exportedVariables: Object, enabledGlobals: Object}} commentDirectives Directives from comment configuration
* @param {Environments} envContext Env context
* @returns {void}
*/
function addDeclaredGlobals(globalScope, config, envContext) {
function addDeclaredGlobals(globalScope, config, commentDirectives, envContext) {
const declaredGlobals = {},
exportedGlobals = {},
explicitGlobals = {},
builtin = envContext.get("builtin");

Expand All @@ -199,9 +199,8 @@ function addDeclaredGlobals(globalScope, config, envContext) {
}
});

Object.assign(exportedGlobals, config.exported);
Object.assign(declaredGlobals, config.globals);
Object.assign(explicitGlobals, config.astGlobals);
Object.assign(explicitGlobals, commentDirectives.enabledGlobals);

Object.keys(declaredGlobals).forEach(name => {
let variable = globalScope.set.get(name);
Expand Down Expand Up @@ -229,7 +228,7 @@ function addDeclaredGlobals(globalScope, config, envContext) {
});

// mark all exported variables as such
Object.keys(exportedGlobals).forEach(name => {
Object.keys(commentDirectives.exportedVariables).forEach(name => {
const variable = globalScope.set.get(name);

if (variable) {
Expand Down Expand Up @@ -283,108 +282,90 @@ function createDisableDirectives(type, loc, value) {
* where reporting is disabled or enabled and merges them with reporting config.
* @param {string} filename The file being checked.
* @param {ASTNode} ast The top node of the AST.
* @param {Object} config The existing configuration data.
* @param {function(string): {create: Function}} ruleMapper A map from rule IDs to defined rules
* @returns {{config: Object, problems: Problem[], disableDirectives: DisableDirective[]}}
* Modified config object, along with any problems encountered while parsing config comments
* @returns {{configuredRules: Object, enabledGlobals: Object, exportedVariables: Object, problems: Problem[], disableDirectives: DisableDirective[]}}
* A collection of the directive comments that were found, along with any problems that occurred when parsing
*/
function modifyConfigsFromComments(filename, ast, config, ruleMapper) {

const commentConfig = {
exported: {},
astGlobals: {},
rules: {},
env: {}
};
const commentRules = {};
function getDirectiveComments(filename, ast, ruleMapper) {
const configuredRules = {};
const enabledGlobals = {};
const exportedVariables = {};
const problems = [];
const disableDirectives = [];

ast.comments.filter(token => token.type !== "Shebang").forEach(comment => {
const trimmedCommentText = comment.value.trim();
const match = /^(eslint(-\w+){0,3}|exported|globals?)(\s|$)/.exec(trimmedCommentText);

let value = comment.value.trim();
const match = /^(eslint(-\w+){0,3}|exported|globals?)(\s|$)/.exec(value);

if (match) {
value = value.slice(match.index + match[1].length);
if (comment.type === "Block") {
switch (match[1]) {
case "exported":
Object.assign(commentConfig.exported, parseBooleanConfig(value, comment));
break;

case "globals":
case "global":
Object.assign(commentConfig.astGlobals, parseBooleanConfig(value, comment));
break;

case "eslint-disable":
[].push.apply(disableDirectives, createDisableDirectives("disable", comment.loc.start, value));
break;

case "eslint-disable-line":
if (comment.loc.start.line === comment.loc.end.line) {
[].push.apply(disableDirectives, createDisableDirectives("disable-line", comment.loc.start, value));
}
break;

case "eslint-disable-next-line":
if (comment.loc.start.line === comment.loc.end.line) {
[].push.apply(disableDirectives, createDisableDirectives("disable-next-line", comment.loc.start, value));
}
break;

case "eslint-enable":
[].push.apply(disableDirectives, createDisableDirectives("enable", comment.loc.start, value));
break;

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

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

try {
validator.validateRuleOptions(ruleMapper(name), name, ruleValue);
} catch (err) {
problems.push({
ruleId: name,
severity: 2,
source: null,
message: err.message,
line: comment.loc.start.line,
column: comment.loc.start.column + 1,
endLine: comment.loc.end.line,
endColumn: comment.loc.end.column + 1,
nodeType: null
});
}
commentRules[name] = ruleValue;
});
} else {
problems.push(parseResult.error);
}
if (!match) {
return;
}

break;
const directiveValue = trimmedCommentText.slice(match.index + match[1].length);

if (/^eslint-disable-(next-)?line$/.test(match[1]) && comment.loc.start.line === comment.loc.end.line) {
const directiveType = match[1].slice("eslint-".length);

[].push.apply(disableDirectives, createDisableDirectives(directiveType, comment.loc.start, directiveValue));
} else if (comment.type === "Block") {
switch (match[1]) {
case "exported":
Object.assign(exportedVariables, parseBooleanConfig(directiveValue, comment));
break;

case "globals":
case "global":
Object.assign(enabledGlobals, parseBooleanConfig(directiveValue, comment));
break;

case "eslint-disable":
[].push.apply(disableDirectives, createDisableDirectives("disable", comment.loc.start, directiveValue));
break;

case "eslint-enable":
[].push.apply(disableDirectives, createDisableDirectives("enable", comment.loc.start, directiveValue));
break;

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

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

try {
validator.validateRuleOptions(ruleMapper(name), name, ruleValue);
} catch (err) {
problems.push({
ruleId: name,
severity: 2,
source: null,
message: err.message,
line: comment.loc.start.line,
column: comment.loc.start.column + 1,
endLine: comment.loc.end.line,
endColumn: comment.loc.end.column + 1,
nodeType: null
});
}
configuredRules[name] = ruleValue;
});
} else {
problems.push(parseResult.error);
}

// no default
}
} else { // comment.type === "Line"
if (match[1] === "eslint-disable-line") {
[].push.apply(disableDirectives, createDisableDirectives("disable-line", comment.loc.start, value));
} else if (match[1] === "eslint-disable-next-line") {
[].push.apply(disableDirectives, createDisableDirectives("disable-next-line", comment.loc.start, value));
break;
}

// no default
}
}
});

Object.assign(commentConfig.rules, commentRules);

return {
config: ConfigOps.merge(config, commentConfig),
configuredRules,
enabledGlobals,
exportedVariables,
problems,
disableDirectives
};
Expand Down Expand Up @@ -780,10 +761,11 @@ module.exports = class Linter {
// evaluate arguments
if (typeof filenameOrOptions === "object") {
providedFilename = filenameOrOptions.filename;
allowInlineConfig = filenameOrOptions.allowInlineConfig;
allowInlineConfig = filenameOrOptions.allowInlineConfig !== false;
reportUnusedDisableDirectives = filenameOrOptions.reportUnusedDisableDirectives;
} else {
providedFilename = filenameOrOptions;
allowInlineConfig = true;
}

if (typeof textOrSourceCode === "string") {
Expand Down Expand Up @@ -866,24 +848,23 @@ module.exports = class Linter {
}
}

const problems = [];
const sourceCode = lastSourceCodes.get(this);
let disableDirectives;
const commentDirectives = allowInlineConfig
? getDirectiveComments(filename, sourceCode.ast, ruleId => this.rules.get(ruleId))
: { configuredRules: {}, enabledGlobals: {}, exportedVariables: {}, problems: [], disableDirectives: [] };

// parse global comments and modify config
if (allowInlineConfig !== false) {
const modifyConfigResult = modifyConfigsFromComments(filename, sourceCode.ast, config, ruleId => this.rules.get(ruleId));
const configuredRules = Object.assign({}, config.rules, commentDirectives.configuredRules);

config = modifyConfigResult.config;
modifyConfigResult.problems.forEach(problem => problems.push(problem));
disableDirectives = modifyConfigResult.disableDirectives;
} else {
disableDirectives = [];
}
// augment global scope with declared global variables
addDeclaredGlobals(
sourceCode.scopeManager.scopes[0],
config,
{ exportedVariables: commentDirectives.exportedVariables, enabledGlobals: commentDirectives.enabledGlobals },
this.environments
);

const emitter = createEmitter();
const traverser = new Traverser();
const scopeManager = sourceCode.scopeManager;

/*
* Create a frozen object with the ruleContext properties and methods that are shared by all rules.
Expand All @@ -895,11 +876,11 @@ module.exports = class Linter {
Object.create(BASE_TRAVERSAL_CONTEXT),
{
getAncestors: () => traverser.parents(),
getDeclaredVariables: scopeManager.getDeclaredVariables.bind(scopeManager),
getDeclaredVariables: sourceCode.scopeManager.getDeclaredVariables.bind(sourceCode.scopeManager),
getFilename: () => filename,
getScope: () => getScope(scopeManager, traverser.current(), config.parserOptions.ecmaVersion),
getScope: () => getScope(sourceCode.scopeManager, traverser.current(), config.parserOptions.ecmaVersion),
getSourceCode: () => sourceCode,
markVariableAsUsed: name => markVariableAsUsed(scopeManager, traverser.current(), config.parserOptions, name),
markVariableAsUsed: name => markVariableAsUsed(sourceCode.scopeManager, traverser.current(), config.parserOptions, name),
parserOptions: config.parserOptions,
parserPath: config.parser,
parserServices: sourceCode.parserServices,
Expand All @@ -921,9 +902,11 @@ module.exports = class Linter {
)
);

const lintingProblems = [];

// enable appropriate rules
Object.keys(config.rules).forEach(ruleId => {
const severity = ConfigOps.getRuleSeverity(config.rules[ruleId]);
Object.keys(configuredRules).forEach(ruleId => {
const severity = ConfigOps.getRuleSeverity(configuredRules[ruleId]);

if (severity === 0) {
return;
Expand All @@ -937,7 +920,7 @@ module.exports = class Linter {
Object.create(sharedTraversalContext),
{
id: ruleId,
options: getRuleOptions(config.rules[ruleId]),
options: getRuleOptions(configuredRules[ruleId]),
report() {

/*
Expand All @@ -958,7 +941,7 @@ module.exports = class Linter {
if (problem.fix && rule.meta && !rule.meta.fixable) {
throw new Error("Fixable rules should export a `meta.fixable` property.");
}
problems.push(problem);
lintingProblems.push(problem);

/*
* This is used to avoid breaking rules that used monkeypatch Linter, and relied on
Expand Down Expand Up @@ -1000,9 +983,6 @@ module.exports = class Linter {
}
});

// augment global scope with declared global variables
addDeclaredGlobals(scopeManager.scopes[0], config, this.environments);

const eventGenerator = new CodePathAnalyzer(new NodeEventGenerator(emitter));

/*
Expand All @@ -1023,8 +1003,10 @@ module.exports = class Linter {
});

return applyDisableDirectives({
directives: disableDirectives,
problems: problems.sort((problemA, problemB) => problemA.line - problemB.line || problemA.column - problemB.column),
directives: commentDirectives.disableDirectives,
problems: lintingProblems
.concat(commentDirectives.problems)
.sort((problemA, problemB) => problemA.line - problemB.line || problemA.column - problemB.column),
reportUnusedDisableDirectives
});
}
Expand Down