From 62df9af3eecc40192699015db4c638efdb603840 Mon Sep 17 00:00:00 2001 From: Mads Jon Nielsen Date: Fri, 3 Apr 2026 11:15:37 +0200 Subject: [PATCH] fix: resolve SonarCloud code smells - Replace String#replace with String#replaceAll where appropriate - Use node: prefixed imports (node:crypto, node:path) - Use globalThis instead of global - Use Number.parseInt, Number.isFinite instead of globals - Extract nested ternary into if/else in job.ts - Remove unnecessary spread in object literal - Fix Promise.allSettled receiving non-Promise values - Use .find() instead of .filter()[0] - Use for-of instead of indexed for loop - Use new Array() instead of Array() - Make static property readonly - Throw Error object instead of string literal - Combine multiple Array#push calls - Compare with undefined directly instead of typeof - Use String.raw for backslash escaping - Flip negated ternary conditions --- src/argv.ts | 2 +- src/git-data.ts | 2 +- src/job.ts | 35 +++++++++++++++++++++-------------- src/parallel.ts | 2 +- src/parser-includes.ts | 8 ++++---- src/parser.ts | 2 +- src/schema-error.ts | 4 ++-- src/utils.ts | 22 +++++++++++----------- src/validator.ts | 10 +++++----- src/variables-from-files.ts | 4 ++-- 10 files changed, 49 insertions(+), 42 deletions(-) diff --git a/src/argv.ts b/src/argv.ts index cdd078b94..b8fe7e280 100644 --- a/src/argv.ts +++ b/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); } } diff --git a/src/git-data.ts b/src/git-data.ts index d49072f6a..9d2a72fd3 100644 --- a/src/git-data.ts +++ b/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; diff --git a/src/job.ts b/src/job.ts index acab376fa..195b4c44f 100644 --- a/src/job.ts +++ b/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 = ""; 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}`]); + })); } } diff --git a/src/parallel.ts b/src/parallel.ts index 04f3b07a4..af1512de8 100644 --- a/src/parallel.ts +++ b/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]; diff --git a/src/parser-includes.ts b/src/parser-includes.ts index b404ebf8c..5f934b4c7 100644 --- a/src/parser-includes.ts +++ b/src/parser-includes.ts @@ -390,7 +390,7 @@ export class ParserIncludes { const cache = new Map(); 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)); diff --git a/src/parser.ts b/src/parser.ts index ef885bc30..18641ce54 100644 --- a/src/parser.ts +++ b/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, }); diff --git a/src/schema-error.ts b/src/schema-error.ts index abfdb36f4..9150f7331 100644 --- a/src/schema-error.ts +++ b/src/schema-error.ts @@ -17,11 +17,11 @@ const SLASH_REGEX = /\//g; const AJV_ERROR_KEYWORD_WEIGHT_MAP: Partial> = {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 => { diff --git a/src/utils.ts b/src/utils.ts index 64dafc519..51a7d9eb8 100644 --- a/src/utils.ts +++ b/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 // https://regexr.com/85sjo const pattern1 = /\s*(?(?:=~)|(?:!~))\s*\/(?.*?[^\\])\/(?[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*(?=~|!~)\s*(["'])(?(?:\\.|[^\\])*?)\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 { 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(":", ""), }, }; diff --git a/src/validator.ts b/src/validator.ts index 513236f09..242af5d50 100644 --- a/src/validator.ts +++ b/src/validator.ts @@ -143,15 +143,15 @@ For further troubleshooting, consider either of the following: } static async run (jobs: ReadonlyArray, 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) { diff --git a/src/variables-from-files.ts b/src/variables-from-files.ts index c057764bd..b44e5e526 100644 --- a/src/variables-from-files.ts +++ b/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 = /(?git@.*?)=(?.*?)=(?.*)/.exec(remoteVariables[i]); + for (const remoteVariable of remoteVariables) { + const match = /(?git@.*?)=(?.*?)=(?.*)/.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;