Skip to content

Commit

Permalink
Improve verify() performance
Browse files Browse the repository at this point in the history
  • Loading branch information
@not-an-aardvark
not-an-aardvark committed Aug 30, 2017
1 parent 636067f commit f516143
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 128 deletions.
156 changes: 80 additions & 76 deletions lib/linter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -685,33 +685,43 @@ function parse(text, config, filePath, messages) {
}
}

const RULE_CONTEXT_PASSTHROUGHS = [
"getAncestors",
"getDeclaredVariables",
"getFilename",
"getScope",
"getSourceCode",
"markVariableAsUsed",

// DEPRECATED
"getAllComments",
"getComments",
"getFirstToken",
"getFirstTokens",
"getJSDocComment",
"getLastToken",
"getLastTokens",
"getNodeByRangeIndex",
"getSource",
"getSourceLines",
"getTokenAfter",
"getTokenBefore",
"getTokenByRangeStart",
"getTokens",
"getTokensAfter",
"getTokensBefore",
"getTokensBetween"
];
// methods that exist on SourceCode object
const DEPRECATED_SOURCECODE_PASSTHROUGHS = {
getSource: "getText",
getSourceLines: "getLines",
getAllComments: "getAllComments",
getNodeByRangeIndex: "getNodeByRangeIndex",
getComments: "getComments",
getCommentsBefore: "getCommentsBefore",
getCommentsAfter: "getCommentsAfter",
getCommentsInside: "getCommentsInside",
getJSDocComment: "getJSDocComment",
getFirstToken: "getFirstToken",
getFirstTokens: "getFirstTokens",
getLastToken: "getLastToken",
getLastTokens: "getLastTokens",
getTokenAfter: "getTokenAfter",
getTokenBefore: "getTokenBefore",
getTokenByRangeStart: "getTokenByRangeStart",
getTokens: "getTokens",
getTokensAfter: "getTokensAfter",
getTokensBefore: "getTokensBefore",
getTokensBetween: "getTokensBetween"
};

const BASE_TRAVERSAL_CONTEXT = Object.freeze(
Object.keys(DEPRECATED_SOURCECODE_PASSTHROUGHS).reduce(
(contextInfo, methodName) =>
Object.assign(contextInfo, {
[methodName]() {
const sourceCode = this.getSourceCode();

return sourceCode[DEPRECATED_SOURCECODE_PASSTHROUGHS[methodName]].apply(sourceCode, arguments);
}
}),
{}
)
);

//------------------------------------------------------------------------------
// Public Interface
Expand Down Expand Up @@ -860,6 +870,29 @@ class Linter extends EventEmitter {
// ensure that severities are normalized in the config
ConfigOps.normalize(config);

/*
* 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: parseResult && parseResult.services || {},
settings: config.settings
}
)
);

// enable appropriate rules
Object.keys(config.rules).filter(ruleId => getRuleSeverity(config.rules[ruleId]) > 0).forEach(ruleId => {
let ruleCreator;
Expand All @@ -878,31 +911,27 @@ class Linter extends EventEmitter {
}

const severity = getRuleSeverity(config.rules[ruleId]);
const ruleContext = RULE_CONTEXT_PASSTHROUGHS.reduce(
(contextInfo, methodName) => Object.assign(contextInfo, { [methodName]: this[methodName].bind(this) }),
{
id: ruleId,
options: getRuleOptions(config.rules[ruleId]),
settings: config.settings,
parserOptions: config.parserOptions,
parserPath: config.parser,
parserServices: parseResult && parseResult.services || {},
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.");
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);
}
}
if (!isDisabledByReportingConfig(this.reportingConfig, ruleId, problem)) {
this.messages.push(problem);
}
}
])
}
])
}
)
);

Object.freeze(ruleContext);

try {
const rule = ruleCreator.create
? ruleCreator.create(ruleContext)
Expand Down Expand Up @@ -1215,33 +1244,8 @@ class Linter extends EventEmitter {
}
}

// methods that exist on SourceCode object
const externalMethods = {
getSource: "getText",
getSourceLines: "getLines",
getAllComments: "getAllComments",
getNodeByRangeIndex: "getNodeByRangeIndex",
getComments: "getComments",
getCommentsBefore: "getCommentsBefore",
getCommentsAfter: "getCommentsAfter",
getCommentsInside: "getCommentsInside",
getJSDocComment: "getJSDocComment",
getFirstToken: "getFirstToken",
getFirstTokens: "getFirstTokens",
getLastToken: "getLastToken",
getLastTokens: "getLastTokens",
getTokenAfter: "getTokenAfter",
getTokenBefore: "getTokenBefore",
getTokenByRangeStart: "getTokenByRangeStart",
getTokens: "getTokens",
getTokensAfter: "getTokensAfter",
getTokensBefore: "getTokensBefore",
getTokensBetween: "getTokensBetween"
};

// copy over methods
Object.keys(externalMethods).forEach(methodName => {
const exMethodName = externalMethods[methodName];
Object.keys(DEPRECATED_SOURCECODE_PASSTHROUGHS).forEach(methodName => {
const exMethodName = DEPRECATED_SOURCECODE_PASSTHROUGHS[methodName];

// Applies the SourceCode methods to the Linter prototype
Object.defineProperty(Linter.prototype, methodName, {
Expand Down
134 changes: 82 additions & 52 deletions lib/report-translator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
//------------------------------------------------------------------------------

const assert = require("assert");
const lodash = require("lodash");
const ruleFixer = require("./util/rule-fixer");

//------------------------------------------------------------------------------
Expand Down Expand Up @@ -42,7 +41,7 @@ function normalizeMultiArgReportCall() {

// If there is one argument, it is considered to be a new-style call already.
if (arguments.length === 1) {
return arguments[0];
return Object.assign({}, arguments[0]);
}

// If the second argument is a string, the arguments are interpreted as [node, message, data, fix].
Expand Down Expand Up @@ -83,31 +82,33 @@ function assertValidNodeInfo(descriptor) {

/**
* Normalizes a MessageDescriptor to always have a `loc` with `start` and `end` properties
* @param {MessageDescriptor} descriptor A descriptor for the report from a rule
* @returns {MessageDescriptor} A new MessageDescriptor that inferred the `start` and `end` properties from
* the `node` of the old descriptor, or inferred the `start` from the `loc` of the old descriptor.
* @param {MessageDescriptor} descriptor A descriptor for the report from a rule. This descriptor may be mutated
* by this function.
* @returns {MessageDescriptor} The updated MessageDescriptor that infers the `start` and `end` properties from
* the `node` of the original descriptor, or infers the `start` from the `loc` of the original descriptor.
*/
function normalizeReportLoc(descriptor) {
if (descriptor.loc) {
if (descriptor.loc.start) {
return descriptor;
}
return Object.assign({}, descriptor, { loc: { start: descriptor.loc, end: null } });
return Object.assign(descriptor, { loc: { start: descriptor.loc, end: null } });
}

return Object.assign({}, descriptor, { loc: descriptor.node.loc });
return Object.assign(descriptor, { loc: descriptor.node.loc });
}

/**
* Interpolates data placeholders in report messages
* @param {MessageDescriptor} descriptor The report message descriptor
* @param {MessageDescriptor} descriptor The report message descriptor. This descriptor may be mutated
* by this function.
* @returns {MessageDescriptor} An new descriptor with a message containing the interpolated data
*/
function normalizeMessagePlaceholders(descriptor) {
if (!descriptor.data) {
return descriptor;
}
return Object.assign({}, descriptor, {
return Object.assign(descriptor, {
message: descriptor.message.replace(/\{\{\s*([^{}]+?)\s*\}\}/g, (fullMatch, term) => {
if (term in descriptor.data) {
return descriptor.data[term];
Expand Down Expand Up @@ -168,33 +169,73 @@ function mergeFixes(fixes, sourceCode) {
/**
* Gets one fix object from the given descriptor.
* If the descriptor retrieves multiple fixes, this merges those to one.
* @param {MessageDescriptor} descriptor The report descriptor. This descriptor may be mutated
* by this function.
* @param {SourceCode} sourceCode The source code object to get text between fixes.
* @param {Object} descriptor The report descriptor.
* @returns {Fix} The got fix object.
* @returns {MessageDescriptor} The updated descriptor.
*/
function normalizeFixes(sourceCode, descriptor) {
function normalizeFixes(descriptor, sourceCode) {
if (typeof descriptor.fix !== "function") {
return Object.assign({}, descriptor, { fix: null });
return Object.assign(descriptor, { fix: null });
}

// @type {null | Fix | Fix[] | IterableIterator<Fix>}
const fix = descriptor.fix(ruleFixer);

// Merge to one.
if (fix && Symbol.iterator in fix) {
return Object.assign({}, descriptor, { fix: mergeFixes(Array.from(fix), sourceCode) });
return Object.assign(descriptor, { fix: mergeFixes(Array.from(fix), sourceCode) });
}
return Object.assign({}, descriptor, { fix });
return Object.assign(descriptor, { fix });
}

/**
* Creates information about the report from a descriptor
* @param {MessageDescriptor} descriptor The message descriptor
* @param {string} ruleId The rule ID of the problem
* @param {(0|1|2)} severity The severity of the problem
* @returns {function(...args): {
* ruleId: string,
* severity: (0|1|2),
* message: string,
* line: number,
* column: number,
* endLine: (number|undefined),
* endColumn: (number|undefined),
* nodeType: (string|null),
* source: string,
* fix: ({text: string, range: [number, number]}|null)
* }} Information about the report
*/
function createProblemFromDescriptor(descriptor, ruleId, severity) {
const problem = {
ruleId,
severity,
message: descriptor.message,
line: descriptor.loc.start.line,
column: descriptor.loc.start.column + 1,
nodeType: descriptor.node && descriptor.node.type || null
};

if (descriptor.loc.end) {
problem.endLine = descriptor.loc.end.line;
problem.endColumn = descriptor.loc.end.column + 1;
}

if (descriptor.fix) {
problem.fix = descriptor.fix;
}

return problem;
}

/**
* Returns a function that converts the arguments of a `context.report` call from a rule into a reported
* problem for the Node.js API.
* @param {Object} metadata Metadata that will be added to the reports. This cannot be modified
* by rules.
* @param {string} metadata.ruleId The rule that the reported messages should be associated with
* @param {0|1|2} metadata.severity The severity that the messages should have
* @param {SourceCode} metadata.sourceCode The `SourceCode` instance for the text being linted
* problem for the Node.js API, without the `ruleId` and `severity` properties. The reported problem
* may be mutated by the claler after being returned from this function. However, none of the original arguments
* to this function should be mutated.
* @param {{ruleId: string, severity: number, sourceCode: SourceCode}} metadata Metadata for the reported problem
* @param {SourceCode} sourceCode The `SourceCode` instance for the text being linted
* @returns {function(...args): {
* ruleId: string,
* severity: (0|1|2),
Expand All @@ -209,38 +250,27 @@ function normalizeFixes(sourceCode, descriptor) {
* }}
* Information about the report
*/

module.exports = function createReportTranslator(metadata) {
const ruleId = metadata.ruleId;
const severity = metadata.severity;
const sourceCode = metadata.sourceCode;

return lodash.flow([
normalizeMultiArgReportCall,
assertValidNodeInfo,
normalizeReportLoc,
normalizeMessagePlaceholders,
lodash.partial(normalizeFixes, sourceCode),
descriptor => {
const problem = {
ruleId,
severity,
message: descriptor.message,
line: descriptor.loc.start.line,
column: descriptor.loc.start.column + 1,
nodeType: descriptor.node && descriptor.node.type || null,
source: sourceCode.lines[descriptor.loc.start.line - 1] || ""
};

if (descriptor.loc.end) {
problem.endLine = descriptor.loc.end.line;
problem.endColumn = descriptor.loc.end.column + 1;
}

if (descriptor.fix) {
problem.fix = descriptor.fix;
}
/*
* `createReportTranslator` gets called once per enabled rule per file. It needs to be very performant.
* The report translator itself (i.e. the function that `createReportTranslator` returns) gets
* called every time a rule reports a problem, which happens much less frequently (usually, the vast
* majority of rules don't report any problems for a given file).
*/
return function() {
const descriptor = normalizeMultiArgReportCall.apply(null, arguments);

return problem;
}
]);
assertValidNodeInfo(descriptor);
normalizeReportLoc(descriptor);
normalizeMessagePlaceholders(descriptor);
normalizeFixes(descriptor, metadata.sourceCode);

const problem = createProblemFromDescriptor(descriptor, metadata.ruleId, metadata.severity);

problem.source = metadata.sourceCode.lines[problem.line - 1] || "";

return problem;
};
};

0 comments on commit f516143

Please sign in to comment.