From 00a1f6aea7454c9f2039556e7d1e8a650885ec28 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Thu, 22 Oct 2020 02:27:51 +0200 Subject: [PATCH] Remove variable shadowing Refactor to remove all no-shadow warnings. --- .eslintrc.js | 2 +- packages/lib/config/classes.js | 7 +-- packages/lib/factorio/export.js | 12 ++--- packages/lib/factorio/patch.js | 4 +- packages/lib/link/connectors.js | 8 ++-- packages/lib/prometheus.js | 46 +++++++++---------- packages/lib/users.js | 10 ++-- packages/master/master.js | 14 +++--- .../master/web/pages/instances/instance.jsx | 6 ++- packages/slave/slave.js | 26 ++++++----- test/integration/clusterio.js | 27 ++++++----- test/integration/index.js | 4 +- test/lib/config/classes.js | 6 +-- test/lib/hash.js | 1 - 14 files changed, 87 insertions(+), 86 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index ff5a0c5b..58be78bd 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -192,7 +192,7 @@ module.exports = { "no-script-url": "error", "no-self-compare": "error", "no-sequences": "error", - "no-shadow": "warn", + "no-shadow": ["error", { "allow": ["yargs"] }], "no-spaced-func": "error", "no-sync": "error", "no-tabs": ["error", { "allowIndentationTabs": true }], diff --git a/packages/lib/config/classes.js b/packages/lib/config/classes.js index 1a2cca36..b9c56d98 100644 --- a/packages/lib/config/classes.js +++ b/packages/lib/config/classes.js @@ -443,12 +443,7 @@ class ConfigGroup { } let prev = this._fields.get(name); - let updated = {}; - if (prev !== null) { - for (let [key, value] of Object.entries(prev)) { - updated[key] = value; - } - } + let updated = {...prev || {}}; updated[prop] = value; this._fields.set(name, updated); diff --git a/packages/lib/factorio/export.js b/packages/lib/factorio/export.js index 16bd78b4..97c051e1 100644 --- a/packages/lib/factorio/export.js +++ b/packages/lib/factorio/export.js @@ -130,19 +130,19 @@ async function exportItems(server, modVersions, items) { let pos = 0; let iconCache = new Map(); - async function loadIcon(path, iconSize, iconMipmaps) { - let icon = iconCache.get(path); + async function loadIcon(iconPath, iconSize, iconMipmaps) { + let icon = iconCache.get(iconPath); if (icon === undefined) { - let fileContent = await loadFile(server, modVersions, path); + let fileContent = await loadFile(server, modVersions, iconPath); if (fileContent) { icon = await Jimp.read(fileContent); icon.crop(0, 0, iconSize, iconSize); - iconCache.set(path, icon); + iconCache.set(iconPath, icon); } else { icon = null; - console.log(`Warning: ${path} not found`); + console.log(`Warning: ${iconPath} not found`); } - iconCache.set(path, icon); + iconCache.set(iconPath, icon); } return icon; } diff --git a/packages/lib/factorio/patch.js b/packages/lib/factorio/patch.js index ea1b6fb5..e4150a53 100644 --- a/packages/lib/factorio/patch.js +++ b/packages/lib/factorio/patch.js @@ -169,8 +169,8 @@ function reorderDependencies(modules) { // the module depends on a module that satisfy any of these conditions. let remaining = new Map(); - for (let modules of hold.values()) { - for (let module of modules) { + for (let heldModules of hold.values()) { + for (let module of heldModules) { remaining.set(module.name, module); } } diff --git a/packages/lib/link/connectors.js b/packages/lib/link/connectors.js index 948bd463..37f9c05e 100644 --- a/packages/lib/link/connectors.js +++ b/packages/lib/link/connectors.js @@ -371,8 +371,8 @@ class WebSocketClientConnector extends WebSocketBaseConnector { this._sessionToken = data.session_token; this._heartbeatInterval = data.heartbeat_interval; this.startHeartbeat(); - for (let message of this._sendBuffer) { - this._socket.send(JSON.stringify(message)); + for (let bufferedMessage of this._sendBuffer) { + this._socket.send(JSON.stringify(bufferedMessage)); } this._startedReconnect = null; this._connected = true; @@ -384,8 +384,8 @@ class WebSocketClientConnector extends WebSocketBaseConnector { this._heartbeatInterval = data.heartbeat_interval; this.startHeartbeat(); this._dropSendBufferSeq(data.last_seq); - for (let message of this._sendBuffer) { - this._socket.send(JSON.stringify(message)); + for (let bufferedMessage of this._sendBuffer) { + this._socket.send(JSON.stringify(bufferedMessage)); } this._startedReconnect = null; this._connected = true; diff --git a/packages/lib/prometheus.js b/packages/lib/prometheus.js index b461d60d..9f80d767 100644 --- a/packages/lib/prometheus.js +++ b/packages/lib/prometheus.js @@ -13,9 +13,9 @@ class Metric { throw new Error(`Invalid name '${name}'`); } - for (let name of labels) { - if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(name)) { - throw new Error(`Invalid label '${name}'`); + for (let label of labels) { + if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(label)) { + throw new Error(`Invalid label '${label}'`); } } @@ -106,16 +106,28 @@ function keyToLabels(key) { return labels; } +function removeMatchingLabels(mapping, labels) { + for (let key of mapping.keys()) { + let candidate = keyToLabels(key); + let hasLabels = Object.entries(labels).every(([name, value]) => ( + candidate.get(name) === value + )); + if (hasLabels) { + mapping.delete(key); + } + } +} + class ValueCollector extends Collector { constructor(type, name, help, options, childClass) { let labels = []; let register = true; let callback = null; - for (let [name, value] of Object.entries(options)) { - if (name === "labels") { labels = value; } - else if (name === "register") { register = value; } - else if (name === "callback") { callback = value; } - else { throw new Error(`Unrecognized option '${name}'`); } + for (let [key, value] of Object.entries(options)) { + if (key === "labels") { labels = value; } + else if (key === "register") { register = value; } + else if (key === "callback") { callback = value; } + else { throw new Error(`Unrecognized option '${key}'`); } } // Make sure we don't register to the default registry if metric throws. @@ -164,18 +176,6 @@ class ValueCollector extends Collector { throw new Error("labels cannot be empty"); } - function removeMatchingLabels(mapping, labels) { - for (let key of mapping.keys()) { - let candidate = keyToLabels(key); - let hasLabels = Object.entries(labels).every(([name, value]) => ( - candidate.get(name) === value - )); - if (hasLabels) { - mapping.delete(key); - } - } - } - removeMatchingLabels(this._values, labels); removeMatchingLabels(this._children, labels); } @@ -341,11 +341,11 @@ async function* expositionLines(resultsIterator) { } async function exposition(resultsIterator = defaultRegistry.collect()) { - let exposition = ""; + let lines = ""; for await (let line of expositionLines(resultsIterator)) { - exposition += line; + lines += line; } - return exposition; + return lines; } // HTTP Content-Type for the exposition format that's implemented diff --git a/packages/lib/users.js b/packages/lib/users.js index 7f69c390..865572d1 100644 --- a/packages/lib/users.js +++ b/packages/lib/users.js @@ -64,11 +64,11 @@ function definePermission({ name, title, description, grantByDefault=false }) { * @static */ class Role { - constructor({ id, name, description, permissions }) { - this.id = id; - this.name = name; - this.description = description; - this.permissions = new Set(permissions); + constructor(serializedRole) { + this.id = serializedRole.id; + this.name = serializedRole.name; + this.description = serializedRole.description; + this.permissions = new Set(serializedRole.permissions); } serialize() { diff --git a/packages/master/master.js b/packages/master/master.js index c361f98f..fc1d5081 100755 --- a/packages/master/master.js +++ b/packages/master/master.js @@ -38,7 +38,7 @@ let masterPlugins = new Map(); let pluginInfos = new Map(); let stopAcceptingNewSessions = false; let debugEvents = new events.EventEmitter(); -let pluginList = {}; +let loadedPlugins = {}; let devMiddleware; // homebrew modules @@ -1290,7 +1290,7 @@ wss.on("connection", function (socket, req) { // Start connection handshake. socket.send(JSON.stringify({ seq: null, type: "hello", data: { version, - plugins: pluginList, + plugins: loadedPlugins, }})); function attachHandler() { @@ -1444,20 +1444,20 @@ async function handleHandshake(message, socket, req, attachHandler) { } // handle plugins on the master -async function pluginManagement(pluginInfos) { +async function pluginManagement() { let startPluginLoad = Date.now(); - masterPlugins = await loadPlugins(pluginInfos); + masterPlugins = await loadPlugins(); console.log("All plugins loaded in "+(Date.now() - startPluginLoad)+"ms"); } -async function loadPlugins(pluginInfos) { +async function loadPlugins() { let plugins = new Map(); for (let pluginInfo of pluginInfos) { if (!masterConfig.group(pluginInfo.name).get("enabled")) { continue; } - pluginList[pluginInfo.name] = pluginInfo.version; + loadedPlugins[pluginInfo.name] = pluginInfo.version; let pluginLoadStarted = Date.now(); let MasterPlugin = libPlugin.BaseMasterPlugin; @@ -1747,7 +1747,7 @@ async function startServer() { } // Load plugins - await pluginManagement(pluginInfos); + await pluginManagement(); // Only start listening for connections after all plugins have loaded if (httpPort) { diff --git a/packages/master/web/pages/instances/instance.jsx b/packages/master/web/pages/instances/instance.jsx index 7d85ab16..c0ccdf13 100644 --- a/packages/master/web/pages/instances/instance.jsx +++ b/packages/master/web/pages/instances/instance.jsx @@ -200,8 +200,10 @@ export class InstanceView extends Component { this.getData(); }} > - {this.state.slaves.map(slave => )} } diff --git a/packages/slave/slave.js b/packages/slave/slave.js index c0e80f6d..71974e5f 100755 --- a/packages/slave/slave.js +++ b/packages/slave/slave.js @@ -900,12 +900,13 @@ class Slave extends libLink.Link { this.updateInstances().catch((err) => { console.error("ERROR: Unexpected error updating instances"); console.error(err); - this.shutdown().catch((err) => { - setBlocking(true); - console.error("ERROR: Unexpected error during shutdown"); - console.error(err); - process.exit(1); - }); + return this.shutdown(); + + }).catch((err) => { + setBlocking(true); + console.error("ERROR: Unexpected error during shutdown"); + console.error(err); + process.exit(1); }); }); @@ -919,12 +920,13 @@ class Slave extends libLink.Link { this.connector.connect().catch((err) => { console.error("ERROR: Unexpected error reconnecting to master"); console.error(err); - this.shutdown().catch((err) => { - setBlocking(true); - console.error("ERROR: Unexpected error during shutdown"); - console.error(err); - process.exit(1); - }); + return this.shutdown(); + + }).catch((err) => { + setBlocking(true); + console.error("ERROR: Unexpected error during shutdown"); + console.error(err); + process.exit(1); }); } else { diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index 5f37a252..96b90146 100644 --- a/test/integration/clusterio.js +++ b/test/integration/clusterio.js @@ -249,8 +249,11 @@ describe("Integration of Clusterio", function() { let args = "--description \"A temp role\" --permissions core.control.connect"; await execCtl(`role create temp ${args}`); let result = await libLink.messages.listRoles.send(getControl()); - let role = result.list.find(role => role.name === "temp"); - assert.deepEqual(role, { id: 5, name: "temp", description: "A temp role", permissions: ["core.control.connect"] }); + let tempRole = result.list.find(role => role.name === "temp"); + assert.deepEqual( + tempRole, + { id: 5, name: "temp", description: "A temp role", permissions: ["core.control.connect"] } + ); }); }); @@ -259,8 +262,8 @@ describe("Integration of Clusterio", function() { let args = "--name new --description \"A new role\" --permissions"; await execCtl(`role edit temp ${args}`); let result = await libLink.messages.listRoles.send(getControl()); - let role = result.list.find(role => role.name === "new"); - assert.deepEqual(role, { id: 5, name: "new", description: "A new role", permissions: [] }); + let newRole = result.list.find(role => role.name === "new"); + assert.deepEqual(newRole, { id: 5, name: "new", description: "A new role", permissions: [] }); }); }); @@ -268,8 +271,8 @@ describe("Integration of Clusterio", function() { it("should modify the given role", async function() { await execCtl("role delete new"); let result = await libLink.messages.listRoles.send(getControl()); - let role = result.list.find(role => role.name === "new"); - assert(!role, "Role was not deleted"); + let newRole = result.list.find(role => role.name === "new"); + assert(!newRole, "Role was not deleted"); }); }); @@ -283,8 +286,8 @@ describe("Integration of Clusterio", function() { it("should create the given user", async function() { await execCtl("user create temp"); let result = await libLink.messages.listUsers.send(getControl()); - let user = result.list.find(user => user.name === "temp"); - assert(user, "user was not created"); + let tempUser = result.list.find(user => user.name === "temp"); + assert(tempUser, "user was not created"); }); }); @@ -292,8 +295,8 @@ describe("Integration of Clusterio", function() { it("should set the roles on the user", async function() { await execCtl("user set-roles temp Admin"); let result = await libLink.messages.listUsers.send(getControl()); - let user = result.list.find(user => user.name === "temp"); - assert.deepEqual(user.roles, [0]); + let tempUser = result.list.find(user => user.name === "temp"); + assert.deepEqual(tempUser.roles, [0]); }); }); @@ -301,8 +304,8 @@ describe("Integration of Clusterio", function() { it("should delete the user", async function() { await execCtl("user delete temp"); let result = await libLink.messages.listUsers.send(getControl()); - let user = result.list.find(user => user.name === "temp"); - assert(!user, "user was note deleted"); + let tempUser = result.list.find(user => user.name === "temp"); + assert(!tempUser, "user was note deleted"); }); }); }); diff --git a/test/integration/index.js b/test/integration/index.js index e1bdf4cd..bf5b2662 100644 --- a/test/integration/index.js +++ b/test/integration/index.js @@ -44,10 +44,10 @@ function slowTest(test) { test.timeout(20000); } -async function get(path) { +async function get(urlPath) { let res = await phin({ method: "GET", - url: `https://localhost:4443${path}`, + url: `https://localhost:4443${urlPath}`, core: { rejectUnauthorized: false }, }); if (res.statusCode !== 200) { diff --git a/test/lib/config/classes.js b/test/lib/config/classes.js index d423f3e4..ac3edbb0 100644 --- a/test/lib/config/classes.js +++ b/test/lib/config/classes.js @@ -415,14 +415,14 @@ describe("lib/config/classes", function() { describe(".registerGroup()", function() { it("should throw if the class is finalized", function() { - class TestConfig extends classes.Config { } - TestConfig.finalize(); + class FinalizedConfig extends classes.Config { } + FinalizedConfig.finalize(); class TestGroup extends classes.ConfigGroup { } TestGroup.groupName = "test"; TestGroup.finalize(); assert.throws( - () => TestConfig.registerGroup(TestGroup), + () => FinalizedConfig.registerGroup(TestGroup), new Error("Cannot register group on finalized config") ); }); diff --git a/test/lib/hash.js b/test/lib/hash.js index b6ddf742..60b0aa7c 100644 --- a/test/lib/hash.js +++ b/test/lib/hash.js @@ -2,7 +2,6 @@ const assert = require("assert").strict; const fs = require("fs"); const path = require("path"); -const stream = require("stream"); const libHash = require("@clusterio/lib/hash");