Skip to content

Commit 0bc8770

Browse files
firecowkevingerman
authored andcommitted
Fix SonarCloud code smells (firecow#1822)
1 parent c072951 commit 0bc8770

10 files changed

Lines changed: 49 additions & 42 deletions

File tree

src/argv.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ export class Argv {
138138
if (value === "true") this.map.set(argKey, true);
139139
else if (value === "false") this.map.set(argKey, false);
140140
else if (value === "null") this.map.set(argKey, null);
141-
else if (!isNaN(Number(value))) this.map.set(argKey, Number(value));
141+
else if (Number.isFinite(Number(value))) this.map.set(argKey, Number(value));
142142
else this.map.set(argKey, value);
143143
}
144144
}

src/git-data.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ export class GitData {
152152
assert(gitRemoteMatch?.groups != null, "git remote get-url origin didn't provide valid matches");
153153

154154
const {stdout} = await Utils.spawn(["ssh", "-G", `${gitRemoteMatch.groups.username}@${gitRemoteMatch.groups.host}`]);
155-
const port = stdout.split("\n").filter((line) => line.startsWith("port "))[0].split(" ")[1];
155+
const port = stdout.split("\n").find((line) => line.startsWith("port "))!.split(" ")[1];
156156
this.remote.host = gitRemoteMatch.groups.host;
157157
this.remote.group = gitRemoteMatch.groups.group;
158158
this.remote.project = gitRemoteMatch.groups.project;

src/job.ts

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ import {Parser} from "./parser.js";
1818
import {resolveIncludeLocal, validateIncludeLocal} from "./parser-includes.js";
1919
import {globbySync} from "globby";
2020
import terminalLink from "terminal-link";
21-
import * as crypto from "crypto";
22-
import * as path from "path";
21+
import * as crypto from "node:crypto";
22+
import * as path from "node:path";
2323

2424
const GCL_SHELL_PROMPT_PLACEHOLDER = "<gclShellPromptPlaceholder>";
2525
interface JobOptions {
@@ -163,7 +163,13 @@ export class Job {
163163
this.allowFailure = jobData.allow_failure ?? false;
164164
this.dependencies = jobData.dependencies || null;
165165
this.rules = jobData.rules || null;
166-
this.environment = typeof jobData.environment === "string" ? {name: jobData.environment} : (jobData.environment ? {...jobData.environment} : jobData.environment);
166+
if (typeof jobData.environment === "string") {
167+
this.environment = {name: jobData.environment, url: null, deployment_tier: null, action: null};
168+
} else if (jobData.environment) {
169+
this.environment = {...jobData.environment};
170+
} else {
171+
this.environment = jobData.environment;
172+
}
167173

168174
const matrixVariables = opt.matrixVariables ?? {};
169175
const fileVariables = Utils.findEnvMatchedVariables(variablesFromFiles, this.fileVariablesDir);
@@ -236,7 +242,7 @@ export class Job {
236242
}
237243
// Set GCL_PROJECT_DIR_ON_HOST if docker image
238244
if (this.imageName(this._variables)) {
239-
this._variables = {...this._variables, ...{GCL_PROJECT_DIR_ON_HOST: cwd}};
245+
this._variables = {...this._variables, GCL_PROJECT_DIR_ON_HOST: cwd};
240246
}
241247

242248
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
327333
predefinedVariables["CI_PIPELINE_ID"] = `${this.pipelineIid + 1000}`;
328334
predefinedVariables["CI_PIPELINE_IID"] = `${this.pipelineIid}`;
329335
predefinedVariables["CI_JOB_NAME"] = `${this.name}`;
330-
predefinedVariables["CI_JOB_NAME_SLUG"] = `${this.name.replace(/[^a-z\d]+/ig, "-").replace(/^-/, "").slice(0, 63).replace(/-$/, "").toLowerCase()}`;
336+
predefinedVariables["CI_JOB_NAME_SLUG"] = `${this.name.replaceAll(/[^a-z\d]+/ig, "-").replace(/^-/, "").slice(0, 63).replace(/-$/, "").toLowerCase()}`;
331337
predefinedVariables["CI_JOB_STAGE"] = `${this.stage}`;
332338
predefinedVariables["CI_BUILDS_DIR"] = ciBuildsDir;
333339
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
365371
// 1. Lowercase, replace non-alphanumeric with '-', and squeeze repeating '-'
366372
let slug = name
367373
.toLowerCase()
368-
.replace(/[^a-z0-9]/g, "-")
369-
.replace(/-+/g, "-");
374+
.replaceAll(/[^a-z0-9]/g, "-")
375+
.replaceAll(/-+/g, "-");
370376

371377
// 2. Must start with a letter
372378
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
10291035

10301036
for (const [key, val] of Object.entries(expanded)) {
10311037
// Replacing `'` with `'\''` to correctly handle single quotes(if `val` contains `'`) in shell commands
1032-
dockerCmd += ` -e '${key}=${val.toString().replace(/'/g, "'\\''")}' \\\n`;
1038+
dockerCmd += ` -e '${key}=${val.toString().replaceAll("'", String.raw`'\''`)}' \\\n`;
10331039
}
10341040

10351041
if (this.imageEntrypoint) {
@@ -1588,7 +1594,7 @@ If you know what you're doing and would like to suppress this warning, use one o
15881594

15891595
for (const [key, val] of Object.entries(expanded)) {
15901596
// Replacing `'` with `'\''` to correctly handle single quotes(if `val` contains `'`) in shell commands
1591-
dockerCmd += ` -e '${key}=${val.toString().replace(/'/g, "'\\''")}' \\\n`;
1597+
dockerCmd += ` -e '${key}=${val.toString().replaceAll("'", String.raw`'\''`)}' \\\n`;
15921598
}
15931599

15941600
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
16711677
});
16721678
} finally {
16731679
// Kill all wait-for-it containers, when one have been successful
1674-
await Promise.allSettled(Object.keys(imageInspect[0].Config.ExposedPorts).map((port) => {
1675-
if (!port.endsWith("/tcp")) return;
1676-
const portNum = parseInt(port.replace("/tcp", ""));
1677-
return Utils.spawn([this.argv.containerExecutable, "rm", "-vf", `gcl-wait-for-it-${this.jobId}-${serviceIndex}-${portNum}`]);
1678-
}));
1680+
await Promise.allSettled(Object.keys(imageInspect[0].Config.ExposedPorts)
1681+
.filter((port) => port.endsWith("/tcp"))
1682+
.map((port) => {
1683+
const portNum = Number.parseInt(port.replace("/tcp", ""));
1684+
return Utils.spawn([this.argv.containerExecutable, "rm", "-vf", `gcl-wait-for-it-${this.jobId}-${serviceIndex}-${portNum}`]);
1685+
}));
16791686
}
16801687
}
16811688

src/parallel.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export function isPlainParallel (jobData: any) {
77

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

src/parser-includes.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ export class ParserIncludes {
390390
const cache = new Map<string, string[]>();
391391
return async (path: string) => {
392392
let result = cache.get(path);
393-
if (typeof result !== "undefined") return result;
393+
if (result !== undefined) return result;
394394

395395
result = (await Utils.getTrackedFiles(path)).map(p => `${path}/${p}`);
396396
cache.set(path, result);
@@ -429,15 +429,15 @@ export async function resolveIncludeLocal (pattern: string, cwd: string) {
429429
pattern = `${cwd}${pattern}`;
430430

431431
// escape all special regex metacharacters
432-
pattern = pattern.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
432+
pattern = pattern.replaceAll(/[.*+?^${}()|[\]\\]/g, String.raw`\$&`);
433433

434434
// `**` matches anything
435435
const anything = ".*?";
436-
pattern = pattern.replace(/\\\*\\\*/g, anything);
436+
pattern = pattern.replaceAll(/\\\*\\\*/g, anything);
437437

438438
// `*` matches anything except for `/`
439439
const anything_but_not_slash = "([^/])*?";
440-
pattern = pattern.replace(/\\\*/g, anything_but_not_slash);
440+
pattern = pattern.replaceAll(/\\\*/g, anything_but_not_slash);
441441

442442
const re2js = RE2JS.compile(`^${pattern}`);
443443
return repoFiles.filter((f: any) => re2js.matches(f));

src/parser.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ export class Parser {
183183
gitData,
184184
variablesFromFiles,
185185
matrixVariables: parallelMatrixVariables,
186-
nodeIndex: (jobData.parallel != null) ? nodeIndex : null,
186+
nodeIndex: (jobData.parallel == null) ? null : nodeIndex,
187187
nodesTotal: parallelMatrixVariablesList.length,
188188
expandVariables: this.expandVariables,
189189
});

src/schema-error.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ const SLASH_REGEX = /\//g;
1717
const AJV_ERROR_KEYWORD_WEIGHT_MAP: Partial<Record<DefinedError["keyword"], number>> = {enum: 1, type: 0};
1818

1919
const pointerToDotNotation = (pointer: string): string => {
20-
return pointer.replace(SLASH_REGEX, ".");
20+
return pointer.replaceAll(SLASH_REGEX, ".");
2121
};
2222

2323
const cleanAjvMessage = (message: string): string => {
24-
return message.replace(QUOTES_REGEX, "'").replace(NOT_REGEX, "not");
24+
return message.replaceAll(QUOTES_REGEX, "'").replaceAll(NOT_REGEX, "not");
2525
};
2626

2727
const getLastSegment = (path: string): string => {

src/utils.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {GitData} from "./git-data.js";
1313
import {globbySync} from "globby";
1414
import micromatch from "micromatch";
1515
import {AxiosRequestConfig} from "axios";
16-
import path from "path";
16+
import path from "node:path";
1717
import {Argv} from "./argv.js";
1818

1919
type RuleResultOpt = {
@@ -56,7 +56,7 @@ export class Utils {
5656
}
5757

5858
static safeBashString (s: string) {
59-
return `'${s.replace(/'/g, "'\"'\"'")}'`; // replaces `'` with `'"'"'`
59+
return `'${s.replaceAll("'", "'\"'\"'")}'`;
6060
}
6161

6262
static forEachRealJob (gitlabData: any, callback: (jobName: string, jobData: any) => void) {
@@ -242,7 +242,7 @@ export class Utils {
242242
// Scenario when RHS is a <regex>
243243
// https://regexr.com/85sjo
244244
const pattern1 = /\s*(?<operator>(?:=~)|(?:!~))\s*\/(?<rhs>.*?[^\\])\/(?<flags>[igmsuy]*)(\s|$|\))/g;
245-
evalStr = evalStr.replace(pattern1, (_, operator, rhs, flags, remainingTokens) => {
245+
evalStr = evalStr.replaceAll(pattern1, (_, operator, rhs, flags, remainingTokens) => {
246246
let _operator;
247247
switch (operator) {
248248
case "=~":
@@ -270,7 +270,7 @@ export class Utils {
270270
// Scenario when RHS is surrounded by single/double-quotes
271271
// https://regexr.com/85t0g
272272
const pattern2 = /\s*(?<operator>=~|!~)\s*(["'])(?<rhs>(?:\\.|[^\\])*?)\2/g;
273-
evalStr = evalStr.replace(pattern2, (_, operator, __, rhs) => {
273+
evalStr = evalStr.replaceAll(pattern2, (_, operator, __, rhs) => {
274274
let _operator;
275275
switch (operator) {
276276
case "=~":
@@ -297,16 +297,16 @@ export class Utils {
297297
return `.matchRE2JS(${_rhs}) ${_operator} null`;
298298
});
299299

300-
evalStr = evalStr.replace(/null.matchRE2JS\(.+?\)\s*!=\s*null/g, "false");
301-
evalStr = evalStr.replace(/null.matchRE2JS\(.+?\)\s*==\s*null/g, "true");
300+
evalStr = evalStr.replaceAll(/null.matchRE2JS\(.+?\)\s*!=\s*null/g, "false");
301+
evalStr = evalStr.replaceAll(/null.matchRE2JS\(.+?\)\s*==\s*null/g, "true");
302302

303303
evalStr = evalStr.trim();
304304

305305
let res;
306306
try {
307-
(global as any).RE2JS = RE2JS; // Assign RE2JS to the global object
307+
(globalThis as any).RE2JS = RE2JS;
308308
res = (0, eval)(evalStr); // indirect eval
309-
delete (global as any).RE2JS; // Cleanup
309+
delete (globalThis as any).RE2JS;
310310
} catch {
311311
const assertMsg = [
312312
"Error attempting to evaluate the following rules:",
@@ -408,7 +408,7 @@ export class Utils {
408408
}
409409
}
410410

411-
static gclRegistryPrefix: string = "registry.gcl.local";
411+
static readonly gclRegistryPrefix: string = "registry.gcl.local";
412412
static async startDockerRegistry (argv: Argv): Promise<void> {
413413
const gclRegistryCertVol = `${this.gclRegistryPrefix}.certs`;
414414
const gclRegistryDataVol = `${this.gclRegistryPrefix}.data`;
@@ -484,7 +484,7 @@ export class Utils {
484484
} catch (err) {
485485
await this.stopDockerRegistry(argv.containerExecutable);
486486
if ((err as ExecaError).timedOut) {
487-
throw "local docker registry port check timed out";
487+
throw new Error("local docker registry port check timed out", {cause: err});
488488
}
489489
throw err;
490490
}
@@ -509,7 +509,7 @@ export class Utils {
509509
return {
510510
proxy: {
511511
host: proxyUrl.hostname,
512-
port: proxyUrl.port ? parseInt(proxyUrl.port, 10) : 8080,
512+
port: proxyUrl.port ? Number.parseInt(proxyUrl.port, 10) : 8080,
513513
protocol: proxyUrl.protocol.replace(":", ""),
514514
},
515515
};

src/validator.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -143,15 +143,15 @@ For further troubleshooting, consider either of the following:
143143
}
144144

145145
static async run (jobs: ReadonlyArray<Job>, stages: readonly string[]) {
146-
const warnings: string[] = [];
147146
this.scriptBlank(jobs);
148147
this.arrayOfStrings(jobs);
149-
warnings.push(...this.needs(jobs, stages));
150148
this.dependencies(jobs, stages);
151149
this.dependenciesContainment(jobs);
152-
warnings.push(...this.potentialIllegalJobName(jobs.map(j => j.baseName)));
153-
warnings.push(...this.artifacts(jobs));
154-
return warnings;
150+
return [
151+
...this.needs(jobs, stages),
152+
...this.potentialIllegalJobName(jobs.map(j => j.baseName)),
153+
...this.artifacts(jobs),
154+
];
155155
}
156156

157157
private static artifacts (jobs: ReadonlyArray<Job>) {

src/variables-from-files.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ export class VariablesFromFiles {
3535
let homeFileData: any = {};
3636

3737
if (remoteVariables && !autoCompleting) {
38-
for (let i = 0; i < remoteVariables.length; i++) {
39-
const match = /(?<url>git@.*?)=(?<file>.*?)=(?<ref>.*)/.exec(remoteVariables[i]);
38+
for (const remoteVariable of remoteVariables) {
39+
const match = /(?<url>git@.*?)=(?<file>.*?)=(?<ref>.*)/.exec(remoteVariable);
4040
assert(match != null, "--remote-variables is malformed use 'git@gitlab.com:firecow/example.git=gitlab-variables.yml=master' syntax");
4141
const url = match.groups?.url;
4242
const file = match.groups?.file;

0 commit comments

Comments
 (0)