NONE: Reenable more lint rules (#2576)

.eslintrc.json

@@ -142,8 +142,6 @@
 			// TODO: Re-enable gradually
 			"@typescript-eslint/no-confusing-void-expression": "off",
 			"@typescript-eslint/no-throw-literal": "off",
-			"@typescript-eslint/no-dynamic-delete": "off",
-			"@typescript-eslint/no-base-to-string": "off",
 			"@typescript-eslint/no-unnecessary-condition": "off"
 		}
 	}

src/github/installation.test.ts

@@ -73,7 +73,7 @@ describe("InstallationWebhookHandler", () => {
 
 		it("should not set security permissions accepted field if the payload doesn't contain secret_scanning_alerts permission", async () => {
 			const webhookContext = getWebhookContext({ cloud: true });
-			delete webhookContext.payload.installation.permissions["secret_scanning_alerts"];
+			delete webhookContext.payload.installation.permissions.secret_scanning_alerts;
 			await installationWebhookHandler(webhookContext, jiraClient, util, GITHUB_INSTALLATION_ID);
 			const subscription = await Subscription.findOneForGitHubInstallationId(GITHUB_INSTALLATION_ID, undefined);
 			expect(subscription?.isSecurityPermissionsAccepted).toBeFalsy();
@@ -82,7 +82,7 @@ describe("InstallationWebhookHandler", () => {
 
 		it("should not set security permissions accepted field if the payload doesn't contain security_events permission", async () => {
 			const webhookContext = getWebhookContext({ cloud: true });
-			delete webhookContext.payload.installation.permissions["security_events"];
+			delete webhookContext.payload.installation.permissions.security_events;
 			await installationWebhookHandler(webhookContext, jiraClient, util, GITHUB_INSTALLATION_ID);
 			const subscription = await Subscription.findOneForGitHubInstallationId(GITHUB_INSTALLATION_ID, undefined);
 			expect(subscription?.isSecurityPermissionsAccepted).toBeFalsy();
@@ -91,7 +91,7 @@ describe("InstallationWebhookHandler", () => {
 
 		it("should not set security permissions accepted field if the payload doesn't contain vulnerability_alerts permission", async () => {
 			const webhookContext = getWebhookContext({ cloud: true });
-			delete webhookContext.payload.installation.permissions["vulnerability_alerts"];
+			delete webhookContext.payload.installation.permissions.vulnerability_alerts;
 			await installationWebhookHandler(webhookContext, jiraClient, util, GITHUB_INSTALLATION_ID);
 			const subscription = await Subscription.findOneForGitHubInstallationId(GITHUB_INSTALLATION_ID, undefined);
 			expect(subscription?.isSecurityPermissionsAccepted).toBeFalsy();

src/rest/routes/deferred/deferred-installation-url.ts

@@ -9,7 +9,7 @@ import {
 import { envVars } from "config/env";
 import { InvalidArgumentError } from "config/errors";
 
-export const DeferredInstallationUrlRoute = errorWrapper("GetDeferredInstallationUrl", async function DeferredInstallationUrlRoute(req: Request, res: Response<GetDeferredInstallationUrl, BaseLocals>) {
+export const DeferredInstallationUrlRoute = errorWrapper("GetDeferredInstallationUrl", async function DeferredInstallationUrlRoute(req: Request<unknown, unknown, unknown, { gitHubInstallationId?: string, gitHubOrgName?: string }, BaseLocals>, res: Response<GetDeferredInstallationUrl, BaseLocals>) {
 	const { gitHubInstallationId, gitHubOrgName } = req.query;
 	const { installation } = res.locals;
 

src/routes/api/data-cleanup/cleanup-reposyncstates.ts

@@ -105,6 +105,6 @@ const safeParseResult = (result: object, commitToDB: boolean) => {
 			return result[0].map((r: object) => safeJsonStringify(r)).join("\n") as string;
 		}
 	} catch (_) {
-		return result;
+		return "ERROR Failed to parse result";
 	}
 };

src/routes/github/create-branch/github-branches-get.test.ts

@@ -56,6 +56,7 @@ describe("GitHub Branches Get", () => {
 
 	it.each(["owner", "repo"])("Should 400 when missing required fields", async (attribute) => {
 		res.status.mockReturnValue(res);
+		// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
 		delete req.params[attribute];
 		await GithubBranchesGet(req, res);
 		expect(res.status).toHaveBeenCalledWith(400);

src/routes/github/create-branch/github-create-branch-post.test.ts

@@ -79,6 +79,7 @@ describe("github-create-branch", () => {
 
 	it.each(["owner", "repo", "sourceBranchName", "newBranchName"])("Should 400 when missing required fields", async (attribute) => {
 		res.status.mockReturnValue(res);
+		// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
 		delete req.body[attribute];
 		await GithubCreateBranchPost(req , res);
 		expect(res.status).toHaveBeenCalledWith(400);

src/routes/github/create-branch/github-remove-session.test.ts

@@ -21,6 +21,7 @@ describe("GitHub Remove Session", () => {
 	});
 
 	it.each(["gitHubAppConfig"])("Should 401 when missing required fields", (attribute) => {
+		// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
 		delete res.locals[attribute];
 		GithubRemoveSession(req, res);
 		expect(res.sendStatus).toHaveBeenCalledWith(401);

src/routes/github/github-oauth.ts

@@ -160,6 +160,7 @@ export const GithubOAuthCallbackGet = async (req: Request, res: Response, next:
 		next(new UIDisplayableError(400, "No state was found"));
 		return;
 	}
+	// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
 	delete req.session[stateKey];
 
 	req.log.info({ query: req.query }, `Received request to ${req.url}`);
@@ -202,7 +203,7 @@ export const GithubAuthMiddleware = async (req: Request, res: Response, next: Ne
 			req.session.githubToken = undefined;
 			req.session.githubRefreshToken = undefined;
 
-			delete query["resetGithubToken"];
+			delete query.resetGithubToken;
 
 			const newUrl = originalUrl.split("?")[0] + "?" + queryToQueryString(query);
 			req.log.info("Github Token reset for URL: ", newUrl);

src/routes/github/manifest/github-manifest-complete-get.ts

@@ -1,12 +1,13 @@
 import { Request, Response } from "express";
+import { ParamsDictionary } from "express-serve-static-core";
 import { GitHubServerApp } from "~/src/models/github-server-app";
 import { Errors } from "config/errors";
 import { createAnonymousClient } from "utils/get-github-client-config";
 import { sendAnalytics } from "utils/analytics-client";
 import { AnalyticsEventTypes, AnalyticsTrackEventsEnum, AnalyticsTrackSource } from "interfaces/common";
 import { GheConnectConfigTempStorage } from "utils/ghe-connect-config-temp-storage";
 
-export const GithubManifestCompleteGet = async (req: Request, res: Response) => {
+export const GithubManifestCompleteGet = async (req: Request<ParamsDictionary, unknown, unknown, { code: string }>, res: Response) => {
 	const uuid = req.params.uuid;
 
 	const tempStorage = new GheConnectConfigTempStorage();

src/routes/github/subscription/github-subscription-delete.test.ts

@@ -208,7 +208,9 @@ describe("delete-github-subscription", () => {
 	});
 
 	it.each([["installationId"], ["jiraHost"]])("Should 400 when missing body.%s", async (property) => {
+		// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
 		delete req.body[property];
+		// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
 		delete res.locals[property];
 
 		res.status.mockReturnValue(res);

src/routes/jira/jira-connected-repos-get.test.ts

@@ -99,9 +99,9 @@ describe("jira-connected-repos-get", () => {
 				const newRepoSyncStatesData: any[] = [];
 				for (let newRepoStateNo = 1; newRepoStateNo < 50; newRepoStateNo++) {
 					const newRepoSyncState = { ...repoSyncState.dataValues };
-					delete newRepoSyncState["id"];
-					delete newRepoSyncState["commitStatus"];
-					delete newRepoSyncState["branchStatus"];
+					delete newRepoSyncState.id;
+					delete newRepoSyncState.commitStatus;
+					delete newRepoSyncState.branchStatus;
 					newRepoSyncState["repoId"] = repoSyncState.repoId + newRepoStateNo;
 					newRepoSyncState["repoName"] = repoSyncState.repoName + newRepoStateNo.toString();
 					newRepoSyncState["repoFullName"] = repoSyncState.repoFullName + String(newRepoStateNo).padStart(3, "0");

src/sync/installation.test.ts

@@ -386,8 +386,8 @@ describe("sync/installation", () => {
 				const	newRepoSyncStatesData: any[] = [];
 				for (let newRepoStateNo = 1; newRepoStateNo < 50; newRepoStateNo++) {
 					const newRepoSyncState = { ...repoSyncState.get() };
-					delete newRepoSyncState["id"];
-					delete newRepoSyncState["branchStatus"];
+					delete newRepoSyncState.id;
+					delete newRepoSyncState.branchStatus;
 					newRepoSyncState["repoId"] = repoSyncState.repoId + newRepoStateNo;
 					if (newRepoStateNo < 49) {
 						// the last one should be main

src/sync/scheduler.test.ts

@@ -18,9 +18,9 @@ describe("scheduler", () => {
 		const newRepoSyncStatesData: any[] = [];
 		for (let newRepoStateNo = 1; newRepoStateNo < 500; newRepoStateNo++) {
 			const newRepoSyncState = { ...repoSyncState.get() };
-			delete newRepoSyncState["id"];
-			delete newRepoSyncState["commitStatus"];
-			delete newRepoSyncState["branchStatus"];
+			delete newRepoSyncState.id;
+			delete newRepoSyncState.commitStatus;
+			delete newRepoSyncState.branchStatus;
 			newRepoSyncState["repoId"] = repoSyncState.repoId + newRepoStateNo;
 			newRepoSyncState["repoName"] = repoSyncState.repoName + newRepoStateNo.toString();
 			newRepoSyncState["repoFullName"] = repoSyncState.repoFullName + newRepoStateNo.toString();

src/util/workers-health-monitor.ts

@@ -96,7 +96,7 @@ export const startMonitorOnWorker = (parentLogger: Logger, workerConfig: {
 const logRunningProcesses = (logger: Logger) => {
 	exec("ps aux", (err, stdout) => {
 		if (err) {
-			logger.error({ err }, `exec error: ${err.toString()}`);
+			logger.error({ err }, `exec error: ${err.message}`);
 			return;
 		}
 
@@ -117,7 +117,7 @@ export const startMonitorOnMaster = (parentLogger: Logger, config: {
 	logger.info(config, "master config");
 
 	const registeredWorkers: Record<string, boolean> = { }; // pid => true
-	const liveWorkers: Record<string, number> = { }; // pid => timestamp
+	const liveWorkers: Map<string, number> = new Map(); // pid => timestamp
 
 	const registerNewWorkers = () => {
 		logInfoSampled(logger, "monRegWorkers", `registering workers`, 100);
@@ -130,7 +130,7 @@ export const startMonitorOnMaster = (parentLogger: Logger, config: {
 					registeredWorkers[workerPid] = true;
 					worker.on("message", () => {
 						logInfoSampled(logger, "workerIsAlive:" + workerPid.toString(), `received message from worker ${workerPid}, marking as live`, 100);
-						liveWorkers[workerPid] = Date.now();
+						liveWorkers.set(String(workerPid), Date.now());
 					});
 					worker.on("exit", (code, signal) => {
 						G("/tmp/*", (err: Error | null, tmpFiles: string[]) => {
@@ -210,14 +210,14 @@ export const startMonitorOnMaster = (parentLogger: Logger, config: {
 			logInfoSampled(logger, `removing dead workers`, `removing dead workers`, 100);
 			const keysToKill: Array<string> = [];
 			const now = Date.now();
-			Object.keys(liveWorkers).forEach((key) => {
-				if (now - liveWorkers[key] > config.workerUnresponsiveThresholdMsecs) {
+			liveWorkers.forEach((value, key) => {
+				if (now - value > config.workerUnresponsiveThresholdMsecs) {
 					keysToKill.push(key);
 				}
 			});
 			keysToKill.forEach((key) => {
 				logger.info(`remove worker with pid=${key} from live workers`);
-				delete liveWorkers[key];
+				liveWorkers.delete(key);
 				logRunningProcesses(logger);
 			});
 		} else {
@@ -226,7 +226,7 @@ export const startMonitorOnMaster = (parentLogger: Logger, config: {
 	};
 
 	const maybeSendShutdownToAllWorkers = () => {
-		const nLiveWorkers = Object.keys(liveWorkers).length;
+		const nLiveWorkers = liveWorkers.size;
 		if (areWorkersReady() && (nLiveWorkers < config.numberOfWorkersThreshold)) {
 			logger.info({
 				nLiveWorkers

test/e2e/utils/jira.ts

@@ -7,7 +7,7 @@ const data = testData.jira;
 export const jiraLogin = async (page: Page, roleName: keyof JiraTestDataRoles, saveState = false): Promise<Page> => {
 	const role = data.roles[roleName];
 	if (!role.username || !role.password) {
-		throw "Jira username or password missing";
+		throw new Error("Jira username or password missing");
 	}
 	await page.goto(data.urls.login);
 	await page.waitForLoadState();

test/setup/env-test.ts

@@ -24,5 +24,6 @@ export const resetEnvVars = () => {
 	// Reset original keys back to process.env
 	originalKeys.forEach(key => process.env[key] = originalEnvVars[key]);
 	// Removing keys that's been added during the test
+	// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
 	difference(newKeys, originalKeys).forEach(key => delete process.env[key]);
 };