Skip to content

Commit

Permalink
Chore: refactor reporting logic (refs #9161) (#9168)
Browse files Browse the repository at this point in the history
  • Loading branch information
not-an-aardvark committed Aug 30, 2017
1 parent 5ab0434 commit d672aef
Show file tree
Hide file tree
Showing 6 changed files with 978 additions and 1,134 deletions.
225 changes: 96 additions & 129 deletions lib/linter.js
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

0 comments on commit d672aef

Please sign in to comment.