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.
captbaritone · Oct 4, 2019 · 7c2674d · 7c2674d
1 parent 8a37587
commit 7c2674d
Showing 1 changed file with 88 additions and 79 deletions.
diff --git a/lib/linter/linter.js b/lib/linter/linter.js
@@ -295,7 +295,11 @@ function getDirectiveComments(filename, ast, ruleMapper, warnInlineConfig) {
         const directiveText = match[1];
         const lineCommentSupported = /^eslint-disable-(next-)?line$/u.test(directiveText);
 
-        if (warnInlineConfig && (lineCommentSupported || comment.type === "Block")) {
+        if (comment.type === "Line" && !lineCommentSupported) {
+            return;
+        }
+
+        if (warnInlineConfig) {
             const kind = comment.type === "Block" ? `/*${directiveText}*/` : `//${directiveText}`;
 
             problems.push(createLintingProblem({
@@ -307,100 +311,105 @@ function getDirectiveComments(filename, ast, ruleMapper, warnInlineConfig) {
             return;
         }
 
+        if (lineCommentSupported && comment.loc.start.line !== comment.loc.end.line) {
+            const message = `${directiveText} comment should not span multiple lines.`;
+
+            problems.push(createLintingProblem({
+                ruleId: null,
+                message,
+                loc: comment.loc
+            }));
+            return;
+        }
+
         const directiveValue = trimmedCommentText.slice(match.index + directiveText.length);
         let directiveType = "";
 
-        if (lineCommentSupported) {
-            if (comment.loc.start.line === comment.loc.end.line) {
-                directiveType = directiveText.slice("eslint-".length);
-            } else {
-                const message = `${directiveText} comment should not span multiple lines.`;
+        switch (directiveText) {
+            case "eslint-disable-next-line":
+                directiveType = "disable-next-line";
+                break;
 
-                problems.push(createLintingProblem({
-                    ruleId: null,
-                    message,
-                    loc: comment.loc
-                }));
-            }
-        } else if (comment.type === "Block") {
-            switch (directiveText) {
-                case "exported":
-                    Object.assign(exportedVariables, commentParser.parseStringConfig(directiveValue, comment));
-                    break;
+            case "eslint-disable-line":
+                directiveType = "disable-line";
+                break;
+
+            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-disable":
+                directiveType = "disable";
+                break;
+
+            case "eslint-enable":
+                directiveType = "enable";
+                break;
 
-                case "globals":
-                case "global":
-                    for (const [id, { value }] of Object.entries(commentParser.parseStringConfig(directiveValue, comment))) {
-                        let normalizedValue;
+            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;
             }
+
+            // no default
         }
 
         if (directiveType !== "") {