Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Chore: refactor reporting logic (refs #9161) #9168

Merged
merged 5 commits into from Aug 30, 2017
Merged

Chore: refactor reporting logic (refs #9161) #9168

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
015d843
Chore: refactor reporting logic (refs #9161)
not-an-aardvark Aug 27, 2017
636067f
Ensure `id` is passed to rule contexts
not-an-aardvark Aug 29, 2017
f516143
Improve verify() performance
not-an-aardvark Aug 29, 2017
600bda3
Fix incorrect tests
not-an-aardvark Aug 30, 2017
6973223
Fix outdated JSDoc comment
not-an-aardvark Aug 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
225 changes: 96 additions & 129 deletions lib/linter.js
View file
Open in desktop
Expand Up @@ -9,10 +9,10 @@
// Requirements
//------------------------------------------------------------------------------

const assert = require("assert"),
EventEmitter = require("events").EventEmitter,
const EventEmitter = require("events").EventEmitter,
eslintScope = require("eslint-scope"),
levn = require("levn"),
lodash = require("lodash"),
blankScriptAST = require("../conf/blank-script.json"),
defaultConfig = require("../conf/default-config-options.js"),
replacements = require("../conf/replacements.json"),
Expand All @@ -23,7 +23,7 @@ const assert = require("assert"),
NodeEventGenerator = require("./util/node-event-generator"),
SourceCode = require("./util/source-code"),
Traverser = require("./util/traverser"),
RuleContext = require("./rule-context"),
createReportTranslator = require("./report-translator"),
Rules = require("./rules"),
timing = require("./timing"),
astUtils = require("./ast-utils"),
Expand Down Expand Up @@ -685,6 +685,44 @@ function parse(text, config, filePath, messages) {
}
}

// 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 @@ -832,46 +870,82 @@ 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(key => getRuleSeverity(config.rules[key]) > 0).forEach(key => {
Object.keys(config.rules).filter(ruleId => getRuleSeverity(config.rules[ruleId]) > 0).forEach(ruleId => {
let ruleCreator;

ruleCreator = this.rules.get(key);
ruleCreator = this.rules.get(ruleId);

if (!ruleCreator) {
const replacementMsg = getRuleReplacementMessage(key);
const replacementMsg = getRuleReplacementMessage(ruleId);

if (replacementMsg) {
ruleCreator = createStubRule(replacementMsg);
} else {
ruleCreator = createStubRule(`Definition for rule '${key}' was not found`);
ruleCreator = createStubRule(`Definition for rule '${ruleId}' was not found`);
}
this.rules.define(key, ruleCreator);
this.rules.define(ruleId, ruleCreator);
}

const severity = getRuleSeverity(config.rules[key]);
const options = getRuleOptions(config.rules[key]);
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 ruleContext = new RuleContext(
key, this, severity, options,
config.settings, config.parserOptions, config.parser,
ruleCreator.meta,
(parseResult && parseResult.services ? parseResult.services : {})
);

const rule = ruleCreator.create ? ruleCreator.create(ruleContext)
const rule = ruleCreator.create
? ruleCreator.create(ruleContext)
: ruleCreator(ruleContext);

// add all the selectors from the rule as listeners
Object.keys(rule).forEach(selector => {
this.on(selector, timing.enabled
? timing.time(key, rule[selector])
? timing.time(ruleId, rule[selector])
: rule[selector]
);
});
} catch (ex) {
ex.message = `Error while loading rule '${key}': ${ex.message}`;
ex.message = `Error while loading rule '${ruleId}': ${ex.message}`;
throw ex;
}
});
Expand Down Expand Up @@ -933,88 +1007,6 @@ class Linter extends EventEmitter {
return this.messages;
}

/**
* Reports a message from one of the rules.
* @param {string} ruleId The ID of the rule causing the message.
* @param {number} severity The severity level of the rule as configured.
* @param {ASTNode} node The AST node that the message relates to.
* @param {Object=} location An object containing the error line and column
* numbers. If location is not provided the node's start location will
* be used.
* @param {string} message The actual message.
* @param {Object} opts Optional template data which produces a formatted message
* with symbols being replaced by this object's values.
* @param {Object} fix A fix command description.
* @param {Object} meta Metadata of the rule
* @returns {void}
*/
report(ruleId, severity, node, location, message, opts, fix, meta) {
if (node) {
assert.strictEqual(typeof node, "object", "Node must be an object");
}

let endLocation;

if (typeof location === "string") {
assert.ok(node, "Node must be provided when reporting error if location is not provided");

meta = fix;
fix = opts;
opts = message;
message = location;
location = node.loc.start;
endLocation = node.loc.end;
} else {
endLocation = location.end;
}

location = location.start || location;

if (isDisabledByReportingConfig(this.reportingConfig, ruleId, location)) {
return;
}

if (opts) {
message = message.replace(/\{\{\s*([^{}]+?)\s*\}\}/g, (fullMatch, term) => {
if (term in opts) {
return opts[term];
}

// Preserve old behavior: If parameter name not provided, don't replace it.
return fullMatch;
});
}

const problem = {
ruleId,
severity,
message,
line: location.line,
column: location.column + 1, // switch to 1-base instead of 0-base
nodeType: node && node.type,
source: this.sourceCode.lines[location.line - 1] || ""
};

// Define endLine and endColumn if exists.
if (endLocation) {
problem.endLine = endLocation.line;
problem.endColumn = endLocation.column + 1; // switch to 1-base instead of 0-base
}

// ensure there's range and text properties, otherwise it's not a valid fix
if (fix && Array.isArray(fix.range) && (typeof fix.text === "string")) {

// If rule uses fix, has metadata, but has no metadata.fixable, we should throw
if (meta && !meta.fixable) {
throw new Error("Fixable rules should export a `meta.fixable` property.");
}

problem.fix = fix;
}

this.messages.push(problem);
}

/**
* Gets the SourceCode object representing the parsed source.
* @returns {SourceCode} The SourceCode object.
Expand Down Expand Up @@ -1252,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