Fix SonarCloud code smells (#1822)

Author: firecow

src/argv.ts

@@ -138,7 +138,7 @@ export class Argv {
                 if (value === "true") this.map.set(argKey, true);
                 else if (value === "false") this.map.set(argKey, false);
                 else if (value === "null") this.map.set(argKey, null);
-                else if (!isNaN(Number(value))) this.map.set(argKey, Number(value));
+                else if (Number.isFinite(Number(value))) this.map.set(argKey, Number(value));
                 else this.map.set(argKey, value);
             }
         }

src/git-data.ts

@@ -134,7 +134,7 @@ export class GitData {
                 assert(gitRemoteMatch?.groups != null, "git remote get-url origin didn't provide valid matches");
 
                 const {stdout} = await Utils.spawn(["ssh", "-G", `${gitRemoteMatch.groups.username}@${gitRemoteMatch.groups.host}`]);
-                const port = stdout.split("\n").filter((line) => line.startsWith("port "))[0].split(" ")[1];
+                const port = stdout.split("\n").find((line) => line.startsWith("port "))!.split(" ")[1];
                 this.remote.host = gitRemoteMatch.groups.host;
                 this.remote.group = gitRemoteMatch.groups.group;
                 this.remote.project = gitRemoteMatch.groups.project;

src/job.ts

@@ -18,8 +18,8 @@ import {Parser} from "./parser.js";
 import {resolveIncludeLocal, validateIncludeLocal} from "./parser-includes.js";
 import {globbySync} from "globby";
 import terminalLink from "terminal-link";
-import * as crypto from "crypto";
-import * as path from "path";
+import * as crypto from "node:crypto";
+import * as path from "node:path";
 
 const GCL_SHELL_PROMPT_PLACEHOLDER = "<gclShellPromptPlaceholder>";
 interface JobOptions {
@@ -163,7 +163,13 @@ export class Job {
         this.allowFailure = jobData.allow_failure ?? false;
         this.dependencies = jobData.dependencies || null;
         this.rules = jobData.rules || null;
-        this.environment = typeof jobData.environment === "string" ? {name: jobData.environment} : (jobData.environment ? {...jobData.environment} : jobData.environment);
+        if (typeof jobData.environment === "string") {
+            this.environment = {name: jobData.environment, url: null, deployment_tier: null, action: null};
+        } else if (jobData.environment) {
+            this.environment = {...jobData.environment};
+        } else {
+            this.environment = jobData.environment;
+        }
 
         const matrixVariables = opt.matrixVariables ?? {};
         const fileVariables = Utils.findEnvMatchedVariables(variablesFromFiles, this.fileVariablesDir);
@@ -236,7 +242,7 @@ export class Job {
         }
         // Set GCL_PROJECT_DIR_ON_HOST if docker image
         if (this.imageName(this._variables)) {
-            this._variables = {...this._variables, ...{GCL_PROJECT_DIR_ON_HOST: cwd}};
+            this._variables = {...this._variables, GCL_PROJECT_DIR_ON_HOST: cwd};
         }
 
         assert(this.scripts || this.trigger, chalk`{blueBright ${this.name}} must have script specified`);
@@ -327,7 +333,7 @@ If you know what you're doing and would like to suppress this warning, use one o
         predefinedVariables["CI_PIPELINE_ID"] = `${this.pipelineIid + 1000}`;
         predefinedVariables["CI_PIPELINE_IID"] = `${this.pipelineIid}`;
         predefinedVariables["CI_JOB_NAME"] = `${this.name}`;
-        predefinedVariables["CI_JOB_NAME_SLUG"] = `${this.name.replace(/[^a-z\d]+/ig, "-").replace(/^-/, "").slice(0, 63).replace(/-$/, "").toLowerCase()}`;
+        predefinedVariables["CI_JOB_NAME_SLUG"] = `${this.name.replaceAll(/[^a-z\d]+/ig, "-").replace(/^-/, "").slice(0, 63).replace(/-$/, "").toLowerCase()}`;
         predefinedVariables["CI_JOB_STAGE"] = `${this.stage}`;
         predefinedVariables["CI_BUILDS_DIR"] = ciBuildsDir;
         predefinedVariables["CI_PROJECT_DIR"] = this.ciProjectDir;
@@ -365,8 +371,8 @@ If you know what you're doing and would like to suppress this warning, use one o
         // 1. Lowercase, replace non-alphanumeric with '-', and squeeze repeating '-'
         let slug = name
             .toLowerCase()
-            .replace(/[^a-z0-9]/g, "-")
-            .replace(/-+/g, "-");
+            .replaceAll(/[^a-z0-9]/g, "-")
+            .replaceAll(/-+/g, "-");
 
         // 2. Must start with a letter
         if (!/^[a-z]/.test(slug)) {
@@ -1029,7 +1035,7 @@ If you know what you're doing and would like to suppress this warning, use one o
 
             for (const [key, val] of Object.entries(expanded)) {
                 // Replacing `'` with `'\''` to correctly handle single quotes(if `val` contains `'`) in shell commands
-                dockerCmd += `  -e '${key}=${val.toString().replace(/'/g, "'\\''")}' \\\n`;
+                dockerCmd += `  -e '${key}=${val.toString().replaceAll("'", String.raw`'\''`)}' \\\n`;
             }
 
             if (this.imageEntrypoint) {
@@ -1588,7 +1594,7 @@ If you know what you're doing and would like to suppress this warning, use one o
 
         for (const [key, val] of Object.entries(expanded)) {
             // Replacing `'` with `'\''` to correctly handle single quotes(if `val` contains `'`) in shell commands
-            dockerCmd += `  -e '${key}=${val.toString().replace(/'/g, "'\\''")}' \\\n`;
+            dockerCmd += `  -e '${key}=${val.toString().replaceAll("'", String.raw`'\''`)}' \\\n`;
         }
 
         const serviceEntrypoint = service.entrypoint;
@@ -1671,11 +1677,12 @@ If you know what you're doing and would like to suppress this warning, use one o
             });
         } finally {
             // Kill all wait-for-it containers, when one have been successful
-            await Promise.allSettled(Object.keys(imageInspect[0].Config.ExposedPorts).map((port) => {
-                if (!port.endsWith("/tcp")) return;
-                const portNum = parseInt(port.replace("/tcp", ""));
-                return Utils.spawn([this.argv.containerExecutable, "rm", "-vf", `gcl-wait-for-it-${this.jobId}-${serviceIndex}-${portNum}`]);
-            }));
+            await Promise.allSettled(Object.keys(imageInspect[0].Config.ExposedPorts)
+                .filter((port) => port.endsWith("/tcp"))
+                .map((port) => {
+                    const portNum = Number.parseInt(port.replace("/tcp", ""));
+                    return Utils.spawn([this.argv.containerExecutable, "rm", "-vf", `gcl-wait-for-it-${this.jobId}-${serviceIndex}-${portNum}`]);
+                }));
         }
     }
 

src/parallel.ts

@@ -7,7 +7,7 @@ export function isPlainParallel (jobData: any) {
 
 export function matrixVariablesList (jobData: any, jobName: string): {[key: string]: string}[] | null[] {
     if (isPlainParallel(jobData)) {
-        return Array(jobData.parallel).fill(null);
+        return new Array(jobData.parallel).fill(null);
     }
     if (jobData?.parallel?.matrix == null) {
         return [null];

src/parser-includes.ts

@@ -390,7 +390,7 @@ export class ParserIncludes {
         const cache = new Map<string, string[]>();
         return async (path: string) => {
             let result = cache.get(path);
-            if (typeof result !== "undefined") return result;
+            if (result !== undefined) return result;
 
             result = (await Utils.getTrackedFiles(path)).map(p => `${path}/${p}`);
             cache.set(path, result);
@@ -429,15 +429,15 @@ export async function resolveIncludeLocal (pattern: string, cwd: string) {
     pattern = `${cwd}${pattern}`;
 
     // escape all special regex metacharacters
-    pattern = pattern.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
+    pattern = pattern.replaceAll(/[.*+?^${}()|[\]\\]/g, String.raw`\$&`);
 
     // `**` matches anything
     const anything = ".*?";
-    pattern = pattern.replace(/\\\*\\\*/g, anything);
+    pattern = pattern.replaceAll(/\\\*\\\*/g, anything);
 
     // `*` matches anything except for `/`
     const anything_but_not_slash = "([^/])*?";
-    pattern = pattern.replace(/\\\*/g, anything_but_not_slash);
+    pattern = pattern.replaceAll(/\\\*/g, anything_but_not_slash);
 
     const re2js = RE2JS.compile(`^${pattern}`);
     return repoFiles.filter((f: any) => re2js.matches(f));

src/parser.ts

@@ -183,7 +183,7 @@ export class Parser {
                     gitData,
                     variablesFromFiles,
                     matrixVariables: parallelMatrixVariables,
-                    nodeIndex: (jobData.parallel != null) ? nodeIndex : null,
+                    nodeIndex: (jobData.parallel == null) ? null : nodeIndex,
                     nodesTotal: parallelMatrixVariablesList.length,
                     expandVariables: this.expandVariables,
                 });

src/schema-error.ts

@@ -17,11 +17,11 @@ const SLASH_REGEX = /\//g;
 const AJV_ERROR_KEYWORD_WEIGHT_MAP: Partial<Record<DefinedError["keyword"], number>> = {enum: 1, type: 0};
 
 const pointerToDotNotation = (pointer: string): string => {
-    return pointer.replace(SLASH_REGEX, ".");
+    return pointer.replaceAll(SLASH_REGEX, ".");
 };
 
 const cleanAjvMessage = (message: string): string => {
-    return message.replace(QUOTES_REGEX, "'").replace(NOT_REGEX, "not");
+    return message.replaceAll(QUOTES_REGEX, "'").replaceAll(NOT_REGEX, "not");
 };
 
 const getLastSegment = (path: string): string => {

src/utils.ts

@@ -13,7 +13,7 @@ import {GitData} from "./git-data.js";
 import {globbySync} from "globby";
 import micromatch from "micromatch";
 import {AxiosRequestConfig} from "axios";
-import path from "path";
+import path from "node:path";
 import {Argv} from "./argv.js";
 
 type RuleResultOpt = {
@@ -56,7 +56,7 @@ export class Utils {
     }
 
     static safeBashString (s: string) {
-        return `'${s.replace(/'/g, "'\"'\"'")}'`; // replaces `'` with `'"'"'`
+        return `'${s.replaceAll("'", "'\"'\"'")}'`;
     }
 
     static forEachRealJob (gitlabData: any, callback: (jobName: string, jobData: any) => void) {
@@ -242,7 +242,7 @@ export class Utils {
         // Scenario when RHS is a <regex>
         // https://regexr.com/85sjo
         const pattern1 = /\s*(?<operator>(?:=~)|(?:!~))\s*\/(?<rhs>.*?[^\\])\/(?<flags>[igmsuy]*)(\s|$|\))/g;
-        evalStr = evalStr.replace(pattern1, (_, operator, rhs, flags, remainingTokens) => {
+        evalStr = evalStr.replaceAll(pattern1, (_, operator, rhs, flags, remainingTokens) => {
             let _operator;
             switch (operator) {
                 case "=~":
@@ -270,7 +270,7 @@ export class Utils {
         // Scenario when RHS is surrounded by single/double-quotes
         // https://regexr.com/85t0g
         const pattern2 = /\s*(?<operator>=~|!~)\s*(["'])(?<rhs>(?:\\.|[^\\])*?)\2/g;
-        evalStr = evalStr.replace(pattern2, (_, operator, __, rhs) => {
+        evalStr = evalStr.replaceAll(pattern2, (_, operator, __, rhs) => {
             let _operator;
             switch (operator) {
                 case "=~":
@@ -297,16 +297,16 @@ export class Utils {
             return `.matchRE2JS(${_rhs}) ${_operator} null`;
         });
 
-        evalStr = evalStr.replace(/null.matchRE2JS\(.+?\)\s*!=\s*null/g, "false");
-        evalStr = evalStr.replace(/null.matchRE2JS\(.+?\)\s*==\s*null/g, "true");
+        evalStr = evalStr.replaceAll(/null.matchRE2JS\(.+?\)\s*!=\s*null/g, "false");
+        evalStr = evalStr.replaceAll(/null.matchRE2JS\(.+?\)\s*==\s*null/g, "true");
 
         evalStr = evalStr.trim();
 
         let res;
         try {
-            (global as any).RE2JS = RE2JS; // Assign RE2JS to the global object
+            (globalThis as any).RE2JS = RE2JS;
             res = (0, eval)(evalStr); // indirect eval
-            delete (global as any).RE2JS; // Cleanup
+            delete (globalThis as any).RE2JS;
         } catch {
             const assertMsg = [
                 "Error attempting to evaluate the following rules:",
@@ -408,7 +408,7 @@ export class Utils {
         }
     }
 
-    static gclRegistryPrefix: string = "registry.gcl.local";
+    static readonly gclRegistryPrefix: string = "registry.gcl.local";
     static async startDockerRegistry (argv: Argv): Promise<void> {
         const gclRegistryCertVol = `${this.gclRegistryPrefix}.certs`;
         const gclRegistryDataVol = `${this.gclRegistryPrefix}.data`;
@@ -484,7 +484,7 @@ export class Utils {
         } catch (err) {
             await this.stopDockerRegistry(argv.containerExecutable);
             if ((err as ExecaError).timedOut) {
-                throw "local docker registry port check timed out";
+                throw new Error("local docker registry port check timed out", {cause: err});
             }
             throw err;
         }
@@ -509,7 +509,7 @@ export class Utils {
             return {
                 proxy: {
                     host: proxyUrl.hostname,
-                    port: proxyUrl.port ? parseInt(proxyUrl.port, 10) : 8080,
+                    port: proxyUrl.port ? Number.parseInt(proxyUrl.port, 10) : 8080,
                     protocol: proxyUrl.protocol.replace(":", ""),
                 },
             };

src/validator.ts

@@ -143,15 +143,15 @@ For further troubleshooting, consider either of the following:
     }
 
     static async run (jobs: ReadonlyArray<Job>, stages: readonly string[]) {
-        const warnings: string[] = [];
         this.scriptBlank(jobs);
         this.arrayOfStrings(jobs);
-        warnings.push(...this.needs(jobs, stages));
         this.dependencies(jobs, stages);
         this.dependenciesContainment(jobs);
-        warnings.push(...this.potentialIllegalJobName(jobs.map(j => j.baseName)));
-        warnings.push(...this.artifacts(jobs));
-        return warnings;
+        return [
+            ...this.needs(jobs, stages),
+            ...this.potentialIllegalJobName(jobs.map(j => j.baseName)),
+            ...this.artifacts(jobs),
+        ];
     }
 
     private static artifacts (jobs: ReadonlyArray<Job>) {

src/variables-from-files.ts

@@ -35,8 +35,8 @@ export class VariablesFromFiles {
         let homeFileData: any = {};
 
         if (remoteVariables && !autoCompleting) {
-            for (let i = 0; i < remoteVariables.length; i++) {
-                const match = /(?<url>git@.*?)=(?<file>.*?)=(?<ref>.*)/.exec(remoteVariables[i]);
+            for (const remoteVariable of remoteVariables) {
+                const match = /(?<url>git@.*?)=(?<file>.*?)=(?<ref>.*)/.exec(remoteVariable);
                 assert(match != null, "--remote-variables is malformed use 'git@gitlab.com:firecow/example.git=gitlab-variables.yml=master' syntax");
                 const url = match.groups?.url;
                 const file = match.groups?.file;