Skip to content

Commit

Permalink
Chore: Clean up inline directive parsing (#12375)
Browse files Browse the repository at this point in the history 
* 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 @kaicataldo
captbaritone authored and kaicataldo committed Oct 8, 2019
1 parent 84467c0 commit 7ffb22f
Showing 1 changed file with 87 additions and 89 deletions.
176 changes: 87 additions & 89 deletions lib/linter/linter.js
View file
Expand Up @@ -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,
Expand All @@ -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
}
});

Expand Down

0 comments on commit 7ffb22f

Please sign in to comment.