From 4b94c6c397dbd9037a84827c6140aae7f13749eb Mon Sep 17 00:00:00 2001 From: Teddy Katz Date: Wed, 30 Aug 2017 14:56:34 -0400 Subject: [PATCH] Chore: make parse() a pure function in Linter (refs #9161) (#9183) --- lib/linter.js | 358 +++++++++++++++++++++++++------------------------- 1 file changed, 180 insertions(+), 178 deletions(-) diff --git a/lib/linter.js b/lib/linter.js index 5825f0f15a2..f84b6e6add1 100755 --- a/lib/linter.js +++ b/lib/linter.js @@ -616,18 +616,16 @@ function getRuleOptions(ruleConfig) { * optimization of functions, so it's best to keep the try-catch as isolated * as possible * @param {string} text The text to parse. - * @param {Object} config The ESLint configuration object. + * @param {Object} providedParserOptions Options to pass to the parser + * @param {string} parserName The name of the parser * @param {string} filePath The path to the file being parsed. - * @returns {ASTNode|CustomParseResult} The AST or parse result if successful, - * or null if not. - * @param {Array} messages Messages array for the linter object - * @returns {*} parsed text if successful otherwise null + * @returns {{success: false, error: Problem}|{success: true,ast: ASTNode, services: Object}} + * An object containing the AST and parser services if parsing was successful, or the error if parsing failed * @private */ -function parse(text, config, filePath, messages) { +function parse(text, providedParserOptions, parserName, filePath) { - let parser; - const parserOptions = Object.assign({}, config.parserOptions, { + const parserOptions = Object.assign({}, providedParserOptions, { loc: true, range: true, raw: true, @@ -636,20 +634,23 @@ function parse(text, config, filePath, messages) { filePath }); + let parser; + try { - parser = require(config.parser); + parser = require(parserName); } catch (ex) { - messages.push({ - ruleId: null, - fatal: true, - severity: 2, - source: null, - message: ex.message, - line: 0, - column: 0 - }); - - return null; + return { + success: false, + error: { + ruleId: null, + fatal: true, + severity: 2, + source: null, + message: ex.message, + line: 0, + column: 0 + } + }; } /* @@ -660,28 +661,38 @@ function parse(text, config, filePath, messages) { */ try { if (typeof parser.parseForESLint === "function") { - return parser.parseForESLint(text, parserOptions); + const parseResult = parser.parseForESLint(text, parserOptions); + + return { + success: true, + ast: parseResult.ast, + services: parseResult.services || {} + }; } - return parser.parse(text, parserOptions); + return { + success: true, + ast: parser.parse(text, parserOptions), + services: {} + }; } catch (ex) { // If the message includes a leading line number, strip it: - const message = ex.message.replace(/^line \d+:/i, "").trim(); - const source = (ex.lineNumber) ? SourceCode.splitLines(text)[ex.lineNumber - 1] : null; - - messages.push({ - ruleId: null, - fatal: true, - severity: 2, - source, - message: `Parsing error: ${message}`, - - line: ex.lineNumber, - column: ex.column - }); + const message = `Parsing error: ${ex.message.replace(/^line \d+:/i, "").trim()}`; + const source = ex.lineNumber ? SourceCode.splitLines(text)[ex.lineNumber - 1] : null; - return null; + return { + success: false, + error: { + ruleId: null, + fatal: true, + severity: 2, + source, + message, + line: ex.lineNumber, + column: ex.column + } + }; } } @@ -787,11 +798,18 @@ class Linter { * @returns {Object[]} The results as an array of messages or null if no messages. */ verify(textOrSourceCode, config, filenameOrOptions, saveState) { - const text = (typeof textOrSourceCode === "string") ? textOrSourceCode : null; - let ast, - parseResult, + let text, + parserServices, allowInlineConfig; + if (typeof textOrSourceCode === "string") { + this.sourceCode = null; + text = textOrSourceCode; + } else { + this.sourceCode = textOrSourceCode; + text = this.sourceCode.text; + } + // evaluate arguments if (typeof filenameOrOptions === "object") { this.currentFilename = filenameOrOptions.filename; @@ -806,7 +824,7 @@ class Linter { } // search and apply "eslint-env *". - const envInFile = findEslintEnv(text || textOrSourceCode.text); + const envInFile = findEslintEnv(text); config = Object.assign({}, config); @@ -821,174 +839,158 @@ class Linter { // process initial config to make it safe to extend config = prepareConfig(config, this.environments); - // only do this for text - if (text !== null) { + if (this.sourceCode) { + parserServices = {}; + } else { // there's no input, just exit here if (text.trim().length === 0) { this.sourceCode = new SourceCode(text, blankScriptAST); - return this.messages; + return []; } - parseResult = parse( + const parseResult = parse( stripUnicodeBOM(text).replace(astUtils.SHEBANG_MATCHER, (match, captured) => `//${captured}`), - config, - this.currentFilename, - this.messages + config.parserOptions, + config.parser, + this.currentFilename ); - // if this result is from a parseForESLint() method, normalize - if (parseResult && parseResult.ast) { - ast = parseResult.ast; - } else { - ast = parseResult; - parseResult = null; + if (!parseResult.success) { + return [parseResult.error]; } - if (ast) { - this.sourceCode = new SourceCode(text, ast); - } + parserServices = parseResult.services; + this.sourceCode = new SourceCode(text, parseResult.ast); + } - } else { - this.sourceCode = textOrSourceCode; - ast = this.sourceCode.ast; + // parse global comments and modify config + if (allowInlineConfig !== false) { + config = modifyConfigsFromComments(this.currentFilename, this.sourceCode.ast, config, this); } - const emitter = new EventEmitter(); + // ensure that severities are normalized in the config + ConfigOps.normalize(config); - emitter.setMaxListeners(Infinity); + const emitter = new EventEmitter().setMaxListeners(Infinity); - // if espree failed to parse the file, there's no sense in setting up rules - if (ast) { + /* + * Create a frozen object with the ruleContext properties and methods that are shared by all rules. + * All rule contexts will inherit from this object. This avoids the performance penalty of copying all the + * properties once for each rule. + */ + const sharedTraversalContext = Object.freeze( + Object.assign( + Object.create(BASE_TRAVERSAL_CONTEXT), + { + getAncestors: this.getAncestors.bind(this), + getDeclaredVariables: this.getDeclaredVariables.bind(this), + getFilename: this.getFilename.bind(this), + getScope: this.getScope.bind(this), + getSourceCode: () => this.sourceCode, + markVariableAsUsed: this.markVariableAsUsed.bind(this), + parserOptions: config.parserOptions, + parserPath: config.parser, + parserServices, + settings: config.settings + } + ) + ); - // parse global comments and modify config - if (allowInlineConfig !== false) { - config = modifyConfigsFromComments(this.currentFilename, ast, config, this); - } + // enable appropriate rules + Object.keys(config.rules).filter(ruleId => getRuleSeverity(config.rules[ruleId]) > 0).forEach(ruleId => { + let ruleCreator = this.rules.get(ruleId); - // ensure that severities are normalized in the config - ConfigOps.normalize(config); + if (!ruleCreator) { + const replacementMsg = getRuleReplacementMessage(ruleId); - /* - * Create a frozen object with the ruleContext properties and methods that are shared by all rules. - * All rule contexts will inherit from this object. This avoids the performance penalty of copying all the - * properties once for each rule. - */ - const sharedTraversalContext = Object.freeze( + if (replacementMsg) { + ruleCreator = createStubRule(replacementMsg); + } else { + ruleCreator = createStubRule(`Definition for rule '${ruleId}' was not found`); + } + this.rules.define(ruleId, ruleCreator); + } + + const severity = getRuleSeverity(config.rules[ruleId]); + const ruleContext = Object.freeze( Object.assign( - Object.create(BASE_TRAVERSAL_CONTEXT), + Object.create(sharedTraversalContext), { - getAncestors: this.getAncestors.bind(this), - getDeclaredVariables: this.getDeclaredVariables.bind(this), - getFilename: this.getFilename.bind(this), - getScope: this.getScope.bind(this), - getSourceCode: () => this.sourceCode, - markVariableAsUsed: this.markVariableAsUsed.bind(this), - parserOptions: config.parserOptions, - parserPath: config.parser, - parserServices: parseResult && parseResult.services || {}, - settings: config.settings + id: ruleId, + options: getRuleOptions(config.rules[ruleId]), + report: lodash.flow([ + createReportTranslator({ ruleId, severity, sourceCode: this.sourceCode }), + problem => { + if (problem.fix && ruleCreator.meta && !ruleCreator.meta.fixable) { + throw new Error("Fixable rules should export a `meta.fixable` property."); + } + if (!isDisabledByReportingConfig(this.reportingConfig, ruleId, problem)) { + this.messages.push(problem); + } + } + ]) } ) ); - // enable appropriate rules - Object.keys(config.rules).filter(ruleId => getRuleSeverity(config.rules[ruleId]) > 0).forEach(ruleId => { - let ruleCreator; - - ruleCreator = this.rules.get(ruleId); - - if (!ruleCreator) { - const replacementMsg = getRuleReplacementMessage(ruleId); - - if (replacementMsg) { - ruleCreator = createStubRule(replacementMsg); - } else { - ruleCreator = createStubRule(`Definition for rule '${ruleId}' was not found`); - } - this.rules.define(ruleId, ruleCreator); - } - - const severity = getRuleSeverity(config.rules[ruleId]); - const ruleContext = Object.freeze( - Object.assign( - Object.create(sharedTraversalContext), - { - id: ruleId, - options: getRuleOptions(config.rules[ruleId]), - report: lodash.flow([ - createReportTranslator({ ruleId, severity, sourceCode: this.sourceCode }), - problem => { - if (problem.fix && ruleCreator.meta && !ruleCreator.meta.fixable) { - throw new Error("Fixable rules should export a `meta.fixable` property."); - } - if (!isDisabledByReportingConfig(this.reportingConfig, ruleId, problem)) { - this.messages.push(problem); - } - } - ]) - } - ) - ); - - try { - const rule = ruleCreator.create - ? ruleCreator.create(ruleContext) - : ruleCreator(ruleContext); - - // add all the selectors from the rule as listeners - Object.keys(rule).forEach(selector => { - emitter.on(selector, timing.enabled - ? timing.time(ruleId, rule[selector]) - : rule[selector] - ); - }); - } catch (ex) { - ex.message = `Error while loading rule '${ruleId}': ${ex.message}`; - throw ex; - } - }); + try { + const rule = ruleCreator.create + ? ruleCreator.create(ruleContext) + : ruleCreator(ruleContext); + + // add all the selectors from the rule as listeners + Object.keys(rule).forEach(selector => { + emitter.on(selector, timing.enabled + ? timing.time(ruleId, rule[selector]) + : rule[selector] + ); + }); + } catch (ex) { + ex.message = `Error while loading rule '${ruleId}': ${ex.message}`; + throw ex; + } + }); - // save config so rules can access as necessary - this.currentConfig = config; - this.traverser = new Traverser(); - - const ecmaFeatures = this.currentConfig.parserOptions.ecmaFeatures || {}; - const ecmaVersion = this.currentConfig.parserOptions.ecmaVersion || 5; - - // gather scope data that may be needed by the rules - this.scopeManager = eslintScope.analyze(ast, { - ignoreEval: true, - nodejsScope: ecmaFeatures.globalReturn, - impliedStrict: ecmaFeatures.impliedStrict, - ecmaVersion, - sourceType: this.currentConfig.parserOptions.sourceType || "script", - fallback: Traverser.getKeys - }); + // save config so rules can access as necessary + this.currentConfig = config; + this.traverser = new Traverser(); + + const ecmaFeatures = this.currentConfig.parserOptions.ecmaFeatures || {}; + const ecmaVersion = this.currentConfig.parserOptions.ecmaVersion || 5; + + // gather scope data that may be needed by the rules + this.scopeManager = eslintScope.analyze(this.sourceCode.ast, { + ignoreEval: true, + nodejsScope: ecmaFeatures.globalReturn, + impliedStrict: ecmaFeatures.impliedStrict, + ecmaVersion, + sourceType: this.currentConfig.parserOptions.sourceType || "script", + fallback: Traverser.getKeys + }); - this.currentScopes = this.scopeManager.scopes; + this.currentScopes = this.scopeManager.scopes; - // augment global scope with declared global variables - addDeclaredGlobals(ast, this.currentScopes[0], this.currentConfig, this.environments); + // augment global scope with declared global variables + addDeclaredGlobals(this.sourceCode.ast, this.currentScopes[0], this.currentConfig, this.environments); - const eventGenerator = new CodePathAnalyzer(new NodeEventGenerator(emitter)); + const eventGenerator = new CodePathAnalyzer(new NodeEventGenerator(emitter)); - /* - * Each node has a type property. Whenever a particular type of - * node is found, an event is fired. This allows any listeners to - * automatically be informed that this type of node has been found - * and react accordingly. - */ - this.traverser.traverse(ast, { - enter(node, parent) { - node.parent = parent; - eventGenerator.enterNode(node); - }, - leave(node) { - eventGenerator.leaveNode(node); - } - }); - } + /* + * Each node has a type property. Whenever a particular type of + * node is found, an event is fired. This allows any listeners to + * automatically be informed that this type of node has been found + * and react accordingly. + */ + this.traverser.traverse(this.sourceCode.ast, { + enter(node, parent) { + node.parent = parent; + eventGenerator.enterNode(node); + }, + leave(node) { + eventGenerator.leaveNode(node); + } + }); // sort by line and column this.messages.sort((a, b) => {