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: enable no-param-reassign on ESLint codebase #10065

Merged
merged 1 commit into from Mar 8, 2018
Merged

Chore: enable no-param-reassign on ESLint codebase #10065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 4 additions & 3 deletions Makefile.js
View file
Open in desktop
Expand Up @@ -345,9 +345,10 @@ function getFirstVersionOfFile(filePath) {

tags = splitCommandResultToLines(tags);
return tags.reduce((list, version) => {
version = semver.valid(version.trim());
if (version) {
list.push(version);
const validatedVersion = semver.valid(version.trim());

if (validatedVersion) {
list.push(validatedVersion);
}
return list;
}, []).sort(semver.compare)[0];
Expand Down
54 changes: 25 additions & 29 deletions lib/ast-utils.js
View file
Open in desktop
Expand Up @@ -84,11 +84,10 @@ function isES5Constructor(node) {
* @returns {Node|null} A found function node.
*/
function getUpperFunction(node) {
while (node) {
if (anyFunctionPattern.test(node.type)) {
return node;
for (let currentNode = node; currentNode; currentNode = currentNode.parent) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is completely personal preference, and can be ignored for this PR, but I don't like using for loops for something other then counters. It's unexpected, and makes it much harder to read the code.

if (anyFunctionPattern.test(currentNode.type)) {
return currentNode;
}
node = node.parent;
}
return null;
}
Expand Down Expand Up @@ -132,12 +131,10 @@ function isLoop(node) {
* @returns {boolean} `true` if the node is in a loop.
*/
function isInLoop(node) {
while (node && !isFunction(node)) {
if (isLoop(node)) {
for (let currentNode = node; currentNode && !isFunction(currentNode); currentNode = currentNode.parent) {
if (isLoop(currentNode)) {
return true;
}

node = node.parent;
}

return false;
Expand Down Expand Up @@ -204,16 +201,14 @@ function isArrayFromMethod(node) {
* @returns {boolean} Whether or not the node is a method which has `thisArg`.
*/
function isMethodWhichHasThisArg(node) {
while (node) {
if (node.type === "Identifier") {
return arrayMethodPattern.test(node.name);
}
if (node.type === "MemberExpression" && !node.computed) {
node = node.property;
continue;
for (
let currentNode = node;
currentNode.type === "MemberExpression" && !currentNode.computed;
currentNode = currentNode.property
) {
if (currentNode.property.type === "Identifier") {
return arrayMethodPattern.test(currentNode.property.name);
}

break;
}

return false;
Expand Down Expand Up @@ -631,9 +626,10 @@ module.exports = {
return false;
}
const isAnonymous = node.id === null;
let currentNode = node;

while (node) {
const parent = node.parent;
while (currentNode) {
const parent = currentNode.parent;

switch (parent.type) {

Expand All @@ -643,7 +639,7 @@ module.exports = {
*/
case "LogicalExpression":
case "ConditionalExpression":
node = parent;
currentNode = parent;
break;

/*
Expand All @@ -663,14 +659,14 @@ module.exports = {
if (func === null || !isCallee(func)) {
return true;
}
node = func.parent;
currentNode = func.parent;
break;
}
case "ArrowFunctionExpression":
if (node !== parent.body || !isCallee(parent)) {
if (currentNode !== parent.body || !isCallee(parent)) {
return true;
}
node = parent.parent;
currentNode = parent.parent;
break;

/*
Expand All @@ -685,7 +681,7 @@ module.exports = {
*/
case "Property":
case "MethodDefinition":
return parent.value !== node;
return parent.value !== currentNode;

/*
* e.g.
Expand Down Expand Up @@ -715,7 +711,7 @@ module.exports = {
case "VariableDeclarator":
return !(
isAnonymous &&
parent.init === node &&
parent.init === currentNode &&
parent.id.type === "Identifier" &&
startsWithUpperCase(parent.id.name)
);
Expand All @@ -728,7 +724,7 @@ module.exports = {
*/
case "MemberExpression":
return (
parent.object !== node ||
parent.object !== currentNode ||
parent.property.type !== "Identifier" ||
!bindOrCallOrApplyPattern.test(parent.property.name) ||
!isCallee(parent) ||
Expand All @@ -746,21 +742,21 @@ module.exports = {
if (isReflectApply(parent.callee)) {
return (
parent.arguments.length !== 3 ||
parent.arguments[0] !== node ||
parent.arguments[0] !== currentNode ||
isNullOrUndefined(parent.arguments[1])
);
}
if (isArrayFromMethod(parent.callee)) {
return (
parent.arguments.length !== 3 ||
parent.arguments[1] !== node ||
parent.arguments[1] !== currentNode ||
isNullOrUndefined(parent.arguments[2])
);
}
if (isMethodWhichHasThisArg(parent.callee)) {
return (
parent.arguments.length !== 2 ||
parent.arguments[0] !== node ||
parent.arguments[0] !== currentNode ||
isNullOrUndefined(parent.arguments[1])
);
}
Expand Down
57 changes: 29 additions & 28 deletions lib/cli-engine.js
View file
Open in desktop
Expand Up @@ -157,8 +157,9 @@ function processText(text, configHelper, filename, fix, allowInlineConfig, repor
fileExtension = path.extname(filename);
}

filename = filename || "<text>";
debug(`Linting ${filename}`);
const effectiveFilename = filename || "<text>";

debug(`Linting ${effectiveFilename}`);
const config = configHelper.getConfig(filePath);

if (config.plugins) {
Expand All @@ -177,18 +178,18 @@ function processText(text, configHelper, filename, fix, allowInlineConfig, repor
const autofixingEnabled = typeof fix !== "undefined" && (!processor || processor.supportsAutofix);

const fixedResult = linter.verifyAndFix(text, config, {
filename,
filename: effectiveFilename,
allowInlineConfig,
reportUnusedDisableDirectives,
fix: !!autofixingEnabled && fix,
preprocess: processor && (rawText => processor.preprocess(rawText, filename)),
postprocess: processor && (problemLists => processor.postprocess(problemLists, filename))
preprocess: processor && (rawText => processor.preprocess(rawText, effectiveFilename)),
postprocess: processor && (problemLists => processor.postprocess(problemLists, effectiveFilename))
});

const stats = calculateStatsPerFile(fixedResult.messages);

const result = {
filePath: filename,
filePath: effectiveFilename,
messages: fixedResult.messages,
errorCount: stats.errorCount,
warningCount: stats.warningCount,
Expand Down Expand Up @@ -302,10 +303,10 @@ function getCacheFile(cacheFile, cwd) {
* make sure the path separators are normalized for the environment/os
* keeping the trailing path separator if present
*/
cacheFile = path.normalize(cacheFile);
const normalizedCacheFile = path.normalize(cacheFile);

const resolvedCacheFile = path.resolve(cwd, cacheFile);
const looksLikeADirectory = cacheFile[cacheFile.length - 1] === path.sep;
const resolvedCacheFile = path.resolve(cwd, normalizedCacheFile);
const looksLikeADirectory = normalizedCacheFile.slice(-1) === path.sep;

/**
* return the name for the cache file in case the provided parameter is a directory
Expand Down Expand Up @@ -368,16 +369,16 @@ class CLIEngine {

/**
* Creates a new instance of the core CLI engine.
* @param {CLIEngineOptions} options The options for this instance.
* @param {CLIEngineOptions} providedOptions The options for this instance.
* @constructor
*/
constructor(options) {
constructor(providedOptions) {

options = Object.assign(
const options = Object.assign(
Object.create(null),
defaultOptions,
{ cwd: process.cwd() },
options
providedOptions
);

/**
Expand Down Expand Up @@ -605,20 +606,21 @@ class CLIEngine {
ignoredPaths = new IgnoredPaths(options);

// resolve filename based on options.cwd (for reporting, ignoredPaths also resolves)
if (filename && !path.isAbsolute(filename)) {
filename = path.resolve(options.cwd, filename);
}

if (filename && ignoredPaths.contains(filename)) {
const resolvedFilename = filename && !path.isAbsolute(filename)
? path.resolve(options.cwd, filename)
: filename;

if (resolvedFilename && ignoredPaths.contains(resolvedFilename)) {
if (warnIgnored) {
results.push(createIgnoreResult(filename, options.cwd));
results.push(createIgnoreResult(resolvedFilename, options.cwd));
}
} else {
results.push(
processText(
text,
configHelper,
filename,
resolvedFilename,
options.fix,
options.allowInlineConfig,
options.reportUnusedDisableDirectives,
Expand Down Expand Up @@ -672,31 +674,30 @@ class CLIEngine {
*/
getFormatter(format) {


// default is stylish
format = format || "stylish";
const resolvedFormatName = format || "stylish";

// only strings are valid formatters
if (typeof format === "string") {
if (typeof resolvedFormatName === "string") {

// replace \ with / for Windows compatibility
format = format.replace(/\\/g, "/");
const normalizedFormatName = resolvedFormatName.replace(/\\/g, "/");

const cwd = this.options ? this.options.cwd : process.cwd();
const namespace = naming.getNamespaceFromTerm(format);
const namespace = naming.getNamespaceFromTerm(normalizedFormatName);

let formatterPath;

// if there's a slash, then it's a file
if (!namespace && format.indexOf("/") > -1) {
formatterPath = path.resolve(cwd, format);
if (!namespace && normalizedFormatName.indexOf("/") > -1) {
formatterPath = path.resolve(cwd, normalizedFormatName);
} else {
try {
const npmFormat = naming.normalizePackageName(format, "eslint-formatter");
const npmFormat = naming.normalizePackageName(normalizedFormatName, "eslint-formatter");

formatterPath = resolver.resolve(npmFormat, `${cwd}/node_modules`);
} catch (e) {
formatterPath = `./formatters/${format}`;
formatterPath = `./formatters/${normalizedFormatName}`;
}
}

Expand Down
10 changes: 5 additions & 5 deletions lib/code-path-analysis/code-path-state.js
View file
Open in desktop
Expand Up @@ -164,13 +164,13 @@ function removeConnection(prevSegments, nextSegments) {
* Creates looping path.
*
* @param {CodePathState} state - The instance.
* @param {CodePathSegment[]} fromSegments - Segments which are source.
* @param {CodePathSegment[]} toSegments - Segments which are destination.
* @param {CodePathSegment[]} unflattenedFromSegments - Segments which are source.
* @param {CodePathSegment[]} unflattenedToSegments - Segments which are destination.
* @returns {void}
*/
function makeLooped(state, fromSegments, toSegments) {
fromSegments = CodePathSegment.flattenUnusedSegments(fromSegments);
toSegments = CodePathSegment.flattenUnusedSegments(toSegments);
function makeLooped(state, unflattenedFromSegments, unflattenedToSegments) {
const fromSegments = CodePathSegment.flattenUnusedSegments(unflattenedFromSegments);
const toSegments = CodePathSegment.flattenUnusedSegments(unflattenedToSegments);

const end = Math.min(fromSegments.length, toSegments.length);

Expand Down
17 changes: 11 additions & 6 deletions lib/code-path-analysis/code-path.js
View file
Open in desktop
Expand Up @@ -134,14 +134,19 @@ class CodePath {
* @returns {void}
*/
traverseSegments(options, callback) {
let resolvedOptions;
let resolvedCallback;

if (typeof options === "function") {
callback = options;
options = null;
resolvedCallback = options;
resolvedOptions = {};
} else {
resolvedOptions = options || {};
resolvedCallback = callback;
}

options = options || {};
const startSegment = options.first || this.internal.initialSegment;
const lastSegment = options.last;
const startSegment = resolvedOptions.first || this.internal.initialSegment;
const lastSegment = resolvedOptions.last;

let item = null;
let index = 0;
Expand Down Expand Up @@ -206,7 +211,7 @@ class CodePath {

// Call the callback when the first time.
if (!skippedSegment) {
callback.call(this, segment, controller);
resolvedCallback.call(this, segment, controller);
if (segment === lastSegment) {
controller.skip();
}
Expand Down
22 changes: 10 additions & 12 deletions lib/code-path-analysis/fork-context.js
View file
Open in desktop
Expand Up @@ -46,19 +46,15 @@ function isReachable(segment) {
function makeSegments(context, begin, end, create) {
const list = context.segmentsList;

if (begin < 0) {
begin = list.length + begin;
}
if (end < 0) {
end = list.length + end;
}
const normalizedBegin = begin >= 0 ? begin : list.length + begin;
const normalizedEnd = end >= 0 ? end : list.length + end;

const segments = [];

for (let i = 0; i < context.count; ++i) {
const allPrevSegments = [];

for (let j = begin; j <= end; ++j) {
for (let j = normalizedBegin; j <= normalizedEnd; ++j) {
allPrevSegments.push(list[j][i]);
}

Expand All @@ -79,18 +75,20 @@ function makeSegments(context, begin, end, create) {
* @returns {CodePathSegment[]} The merged segments.
*/
function mergeExtraSegments(context, segments) {
while (segments.length > context.count) {
let currentSegments = segments;

while (currentSegments.length > context.count) {
const merged = [];

for (let i = 0, length = segments.length / 2 | 0; i < length; ++i) {
for (let i = 0, length = currentSegments.length / 2 | 0; i < length; ++i) {
merged.push(CodePathSegment.newNext(
context.idGenerator.next(),
[segments[i], segments[i + length]]
[currentSegments[i], currentSegments[i + length]]
));
}
segments = merged;
currentSegments = merged;
}
return segments;
return currentSegments;
}

//------------------------------------------------------------------------------
Expand Down