Skip to content
Permalink
Browse files

Chore: Clean up inline directive parsing (#12375)

* Give variable name to matched text

This simply makes the code a bit easier to read by giving a name to `match[1]`.

* Refactor: Untangle logic for parsing directives

There are a few thing going on in this function which were getting
conflated:

1. Parsing a `directiveType` out of the comment.
2. Ignoring directives that are in line comments but only support block
   comments.
3. Warning on and ignoring line comments that span multiple lines.

Previously these three pieces of functionality were tightly coupled
which made the code harder to read. After this change each task is
handled independently of the other.

* Core: Consolidate handling disable directives

Rather than conditionally set a mutable value and check for it at the end of the switch statement, we can actually just handle it inline by using a fallthrough.
  • Loading branch information
captbaritone authored and kaicataldo committed Oct 8, 2019
1 parent 84467c0 commit 7ffb22f61cf1622511a7fe42b5ead7c3b216df5e
Showing with 87 additions and 89 deletions.
  1. +87 −89 lib/linter/linter.js
@@ -292,10 +292,15 @@ function getDirectiveComments(filename, ast, ruleMapper, warnInlineConfig) {
if (!match) {
return;
}
const lineCommentSupported = /^eslint-disable-(next-)?line$/u.test(match[1]);
const directiveText = match[1];
const lineCommentSupported = /^eslint-disable-(next-)?line$/u.test(directiveText);

if (warnInlineConfig && (lineCommentSupported || comment.type === "Block")) {
const kind = comment.type === "Block" ? `/*${match[1]}*/` : `//${match[1]}`;
if (comment.type === "Line" && !lineCommentSupported) {
return;
}

if (warnInlineConfig) {
const kind = comment.type === "Block" ? `/*${directiveText}*/` : `//${directiveText}`;

problems.push(createLintingProblem({
ruleId: null,
@@ -306,108 +311,101 @@ function getDirectiveComments(filename, ast, ruleMapper, warnInlineConfig) {
return;
}

const directiveValue = trimmedCommentText.slice(match.index + match[1].length);
let directiveType = "";
if (lineCommentSupported && comment.loc.start.line !== comment.loc.end.line) {
const message = `${directiveText} comment should not span multiple lines.`;

if (lineCommentSupported) {
if (comment.loc.start.line === comment.loc.end.line) {
directiveType = match[1].slice("eslint-".length);
} else {
const message = `${match[1]} comment should not span multiple lines.`;
problems.push(createLintingProblem({
ruleId: null,
message,
loc: comment.loc
}));
return;
}

problems.push(createLintingProblem({
ruleId: null,
message,
loc: comment.loc
}));
const directiveValue = trimmedCommentText.slice(match.index + directiveText.length);

switch (directiveText) {
case "eslint-disable":
case "eslint-enable":
case "eslint-disable-next-line":
case "eslint-disable-line": {
const directiveType = directiveText.slice("eslint-".length);
const options = { type: directiveType, loc: comment.loc, value: directiveValue, ruleMapper };
const { directives, directiveProblems } = createDisableDirectives(options);

disableDirectives.push(...directives);
problems.push(...directiveProblems);
break;
}
} else if (comment.type === "Block") {
switch (match[1]) {
case "exported":
Object.assign(exportedVariables, commentParser.parseStringConfig(directiveValue, comment));
break;

case "globals":
case "global":
for (const [id, { value }] of Object.entries(commentParser.parseStringConfig(directiveValue, comment))) {
let normalizedValue;
case "exported":
Object.assign(exportedVariables, commentParser.parseStringConfig(directiveValue, comment));
break;

case "globals":
case "global":
for (const [id, { value }] of Object.entries(commentParser.parseStringConfig(directiveValue, comment))) {
let normalizedValue;

try {
normalizedValue = ConfigOps.normalizeConfigGlobal(value);
} catch (err) {
problems.push(createLintingProblem({
ruleId: null,
loc: comment.loc,
message: err.message
}));
continue;
}

if (enabledGlobals[id]) {
enabledGlobals[id].comments.push(comment);
enabledGlobals[id].value = normalizedValue;
} else {
enabledGlobals[id] = {
comments: [comment],
value: normalizedValue
};
}
}
break;

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

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

if (rule === null) {
problems.push(createLintingProblem({ ruleId: name, loc: comment.loc }));
return;
}

try {
normalizedValue = ConfigOps.normalizeConfigGlobal(value);
validator.validateRuleOptions(rule, name, ruleValue);
} catch (err) {
problems.push(createLintingProblem({
ruleId: null,
loc: comment.loc,
message: err.message
ruleId: name,
message: err.message,
loc: comment.loc
}));
continue;
}

if (enabledGlobals[id]) {
enabledGlobals[id].comments.push(comment);
enabledGlobals[id].value = normalizedValue;
} else {
enabledGlobals[id] = {
comments: [comment],
value: normalizedValue
};
// do not apply the config, if found invalid options.
return;
}
}
break;

case "eslint-disable":
directiveType = "disable";
break;

case "eslint-enable":
directiveType = "enable";
break;

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

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

if (rule === null) {
problems.push(createLintingProblem({ ruleId: name, loc: comment.loc }));
return;
}

try {
validator.validateRuleOptions(rule, name, ruleValue);
} catch (err) {
problems.push(createLintingProblem({
ruleId: name,
message: err.message,
loc: comment.loc
}));

// do not apply the config, if found invalid options.
return;
}

configuredRules[name] = ruleValue;
});
} else {
problems.push(parseResult.error);
}

break;
configuredRules[name] = ruleValue;
});
} else {
problems.push(parseResult.error);
}

// no default
break;
}
}

if (directiveType !== "") {
const options = { type: directiveType, loc: comment.loc, value: directiveValue, ruleMapper };
const { directives, directiveProblems } = createDisableDirectives(options);

disableDirectives.push(...directives);
problems.push(...directiveProblems);
// no default
}
});

0 comments on commit 7ffb22f

Please sign in to comment.
You can’t perform that action at this time.