From 7c2674dc225c75c3d446d489dc63e809fb91f58b Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Fri, 4 Oct 2019 10:15:00 -0700 Subject: [PATCH] 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. --- lib/linter/linter.js | 167 +++++++++++++++++++++++-------------------- 1 file changed, 88 insertions(+), 79 deletions(-) diff --git a/lib/linter/linter.js b/lib/linter/linter.js index 7a7189e816b..3a6284aaa57 100644 --- 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 !== "") {