Remove variable shadowing

Refactor to remove all no-shadow warnings.

.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 }],

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);

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;
 	}

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);
 		}
 	}

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;

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

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() {

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) {

packages/master/web/pages/instances/instance.jsx

@@ -200,8 +200,10 @@ export class InstanceView extends Component {
 						this.getData();
 					}}
 					>
-						{this.state.slaves.map(slave => <Option defaultValue={instance.assigned_slave} value={slave.id} key={slave.id}>
-							{slave.name}
+						{this.state.slaves.map(newSlave => <Option
+							defaultValue={instance.assigned_slave} value={newSlave.id} key={newSlave.id}
+						>
+							{newSlave.name}
 						</Option>)}
 					</Select>
 				}

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 {

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");
 			});
 		});
 	});

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) {

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")
 				);
 			});

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");