Skip to content

Commit 29839d3

Browse files
firecowfirecow
authored
code: Sonarcloud code smells and stylistic improvements (#1515)
* Fix sonarcloud code smells * Use array join instead of multiline template strings, and remove unused capture group * Convert multiline template strings to array
1 parent e931fba commit 29839d3

File tree

5 files changed

+51
-64
lines changed

5 files changed

+51
-64
lines changed

‎src/git-data.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,7 @@ export class GitData {
7373
this.branches.default = gitRemoteDefaultBranch.replace("origin/", "");
7474
} catch (e: any) {
7575
if (e.stderr === "fatal: ref refs/remotes/origin/HEAD is not a symbolic ref") {
76-
writeStreams.stderr(chalk`{yellow Unable to retrieve default remote branch, falling back to \`${this.branches.default}\`.
77-
The default remote branch can be set via \`git remote set-head origin <default_branch>\`
78-
}`);
76+
writeStreams.stderr(chalk`{yellow Unable to retrieve default remote branch, falling back to \`${this.branches.default}\`. The default remote branch can be set via \`git remote set-head origin <default_branch>\`}`);
7977
} else {
8078
writeStreams.stderr(chalk`{yellow Unable to retrieve default remote branch, falling back to \`${this.branches.default}\`.\n}`);
8179
}

‎src/job.ts‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ export class Job {
604604

605605
let chownOpt = "0:0";
606606
let chmodOpt = "a+rw";
607-
if (this.argv.umask === false) {
607+
if (!this.argv.umask) {
608608
const {stdout} = await Utils.spawn(["docker", "run", "--rm", "--entrypoint", "sh", imageName, "-c", "echo \"$(id -u):$(id -g)\""]);
609609
chownOpt = stdout;
610610
if (chownOpt == "0:0") {
@@ -667,8 +667,6 @@ export class Job {
667667
if (this.jobData["coverage"]) {
668668
this._coveragePercent = await Utils.getCoveragePercent(argv.cwd, argv.stateDir, this.jobData["coverage"], safeJobName);
669669
}
670-
671-
this.cleanupResources();
672670
}
673671

674672
async cleanupResources () {
@@ -818,7 +816,7 @@ export class Job {
818816
dockerCmd += `--ulimit nofile=${this.argv.ulimit} `;
819817
}
820818

821-
if (this.argv.umask === true) {
819+
if (this.argv.umask) {
822820
dockerCmd += "--user 0:0 ";
823821
}
824822

@@ -1249,12 +1247,9 @@ export class Job {
12491247
await this.copyOut(cpCmd, stateDir, "artifacts", dockerCmdExtras);
12501248
endTime = process.hrtime(time);
12511249

1252-
if (reportDotenvs != null) {
1253-
reportDotenvs.forEach(async (reportDotenv) => {
1254-
if (!await fs.pathExists(`${cwd}/${stateDir}/artifacts/${safeJobName}/.gitlab-ci-reports/dotenv/${reportDotenv}`)) {
1255-
writeStreams.stderr(chalk`${this.formattedJobName} {yellow artifact reports dotenv '${reportDotenv}' could not be found}\n`);
1256-
}
1257-
});
1250+
for (const reportDotenv of reportDotenvs ?? []) {
1251+
if (await fs.pathExists(`${cwd}/${stateDir}/artifacts/${safeJobName}/.gitlab-ci-reports/dotenv/${reportDotenv}`)) continue;
1252+
writeStreams.stderr(chalk`${this.formattedJobName} {yellow artifact reports dotenv '${reportDotenv}' could not be found}\n`);
12581253
}
12591254

12601255
const readdir = await fs.readdir(`${cwd}/${stateDir}/artifacts/${safeJobName}`);
@@ -1348,7 +1343,7 @@ export class Job {
13481343
let dockerCmd = `${this.argv.containerExecutable} create --interactive `;
13491344
this.refreshLongRunningSilentTimeout(writeStreams);
13501345

1351-
if (this.argv.umask === true) {
1346+
if (this.argv.umask) {
13521347
dockerCmd += "--user 0:0 ";
13531348
}
13541349

‎src/parser-includes.ts‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -276,18 +276,15 @@ export class ParserIncludes {
276276
await fs.mkdirp(path.dirname(`${cwd}/${target}/${normalizedFile}`));
277277

278278
const gitCloneBranch = (ref === "HEAD") ? "" : `--branch ${ref}`;
279-
await Utils.bash(`
280-
cd ${cwd}/${stateDir} \\
281-
&& git clone ${gitCloneBranch} -n --depth=1 --filter=tree:0 \\
282-
${remote.schema}://${remote.host}:${remote.port}/${project}.git \\
283-
${cwd}/${target}.${ext} \\
284-
&& cd ${cwd}/${target}.${ext} \\
285-
&& git sparse-checkout set --no-cone ${normalizedFile} \\
286-
&& git checkout \\
287-
&& cd ${cwd}/${stateDir} \\
288-
&& cp ${cwd}/${target}.${ext}/${normalizedFile} \\
289-
${cwd}/${target}/${normalizedFile}
290-
`, cwd);
279+
await Utils.bashMulti([
280+
`cd ${cwd}/${stateDir}`,
281+
`git clone ${gitCloneBranch} -n --depth=1 --filter=tree:0 ${remote.schema}://${remote.host}:${remote.port}/${project}.git ${cwd}/${target}.${ext}`,
282+
`cd ${cwd}/${target}.${ext}`,
283+
`git sparse-checkout set --no-cone ${normalizedFile}`,
284+
"git checkout",
285+
`cd ${cwd}/${stateDir}`,
286+
`cp ${cwd}/${target}.${ext}/${normalizedFile} ${cwd}/${target}/${normalizedFile}`,
287+
], cwd);
291288
} else {
292289
await fs.mkdirp(`${cwd}/${target}`);
293290
await Utils.bash(`set -eou pipefail; git archive --remote=ssh://git@${remote.host}:${remote.port}/${project}.git ${ref} ${normalizedFile} | tar -f - -xC ${target}/`, cwd);

‎src/predefined-variables.ts‎

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,26 +17,12 @@ export function init ({gitData, argv, envMatchedVariables}: PredefinedVariablesO
1717
// 2. gitlab variables files
1818
// 3. values derieved implicitly from `git remote -v`
1919
// 4. default value
20-
const CI_SERVER_PROTOCOL = _variables["CI_SERVER_PROTOCOL"]
21-
?? ((gitData.remote.schema === "http" || gitData.remote.schema === "https")
22-
? gitData.remote.schema
23-
: "https");
24-
const CI_SERVER_PORT = _variables["CI_SERVER_PORT"]
25-
?? ((gitData.remote.schema === "http" || gitData.remote.schema === "https")
26-
? gitData.remote.port
27-
: "443");
28-
const CI_SERVER_SHELL_SSH_PORT = _variables["CI_SERVER_SHELL_SSH_PORT"]
29-
?? ((gitData.remote.schema === "ssh")
30-
? gitData.remote.port
31-
: "22");
32-
const CI_SERVER_HOST = _variables["CI_SERVER_HOST"]
33-
?? `${gitData.remote.host}`;
34-
const CI_SERVER_FQDN = _variables["CI_SERVER_FQDN"]
35-
?? (CI_SERVER_PORT == "443"
36-
? gitData.remote.host
37-
: `${gitData.remote.host}:${CI_SERVER_PORT}`);
38-
const CI_SERVER_URL = _variables["CI_SERVER_URL"]
39-
?? `${CI_SERVER_PROTOCOL}://${CI_SERVER_FQDN}`;
20+
const CI_SERVER_PROTOCOL = _variables["CI_SERVER_PROTOCOL"] ?? ((gitData.remote.schema === "http" || gitData.remote.schema === "https") ? gitData.remote.schema : "https");
21+
const CI_SERVER_PORT = _variables["CI_SERVER_PORT"] ?? ((gitData.remote.schema === "http" || gitData.remote.schema === "https") ? gitData.remote.port : "443");
22+
const CI_SERVER_SHELL_SSH_PORT = _variables["CI_SERVER_SHELL_SSH_PORT"] ?? ((gitData.remote.schema === "ssh") ? gitData.remote.port : "22");
23+
const CI_SERVER_HOST = _variables["CI_SERVER_HOST"] ?? `${gitData.remote.host}`;
24+
const CI_SERVER_FQDN = _variables["CI_SERVER_FQDN"] ?? (CI_SERVER_PORT == "443" ? gitData.remote.host : `${gitData.remote.host}:${CI_SERVER_PORT}`);
25+
const CI_SERVER_URL = _variables["CI_SERVER_URL"] ?? `${CI_SERVER_PROTOCOL}://${CI_SERVER_FQDN}`;
4026
const CI_PROJECT_ROOT_NAMESPACE = gitData.remote.group.split("/")[0];
4127
const CI_PROJECT_NAMESPACE = gitData.remote.group;
4228
const CI_DEPENDENCY_PROXY_SERVER = CI_SERVER_FQDN.includes(":") ? CI_SERVER_FQDN : `${CI_SERVER_HOST}:${CI_SERVER_PORT}`;

‎src/utils.ts‎

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ type ExpandWith = {
2727
};
2828

2929
export class Utils {
30+
static bashMulti (scripts: string[], cwd = process.cwd()): Promise<{stdout: string; stderr: string; exitCode: number}> {
31+
return execa(scripts.join(" && \\"), {shell: "bash", cwd});
32+
}
33+
3034
static bash (shellScript: string, cwd = process.cwd()): Promise<{stdout: string; stderr: string; exitCode: number}> {
3135
return execa(shellScript, {shell: "bash", cwd});
3236
}
@@ -225,16 +229,19 @@ export class Utils {
225229
}
226230
const _rhs = JSON.stringify(rhs); // JSON.stringify for escaping `"`
227231
const containsNonEscapedSlash = /(?<!\\)\//.test(_rhs);
228-
assert(!containsNonEscapedSlash, `Error attempting to evaluate the following rules:
229-
rules:
230-
- if: '${expandedEvalStr}'
231-
as rhs contains unescaped \`/\``);
232+
const assertMsg = [
233+
"Error attempting to evaluate the following rules:",
234+
" rules:",
235+
` - if: '${expandedEvalStr}'`,
236+
"as rhs contains unescaped quote",
237+
];
238+
assert(!containsNonEscapedSlash, assertMsg.join("\n"));
232239
return `.match(new RE2(${_rhs}, "${flags}")) ${_operator} null${remainingTokens}`;
233240
});
234241

235242
// Scenario when RHS is surrounded by single/double-quotes
236243
// https://regexr.com/85t0g
237-
const pattern2 = /\s*(?<operator>=~|!~)\s*(?<quote_type>["'])(?<rhs>(?:\\.|[^\\])*?)\2/g;
244+
const pattern2 = /\s*(?<operator>=~|!~)\s*(["'])(?<rhs>(?:\\.|[^\\])*?)\2/g;
238245
evalStr = evalStr.replace(pattern2, (_, operator, __, rhs) => {
239246
let _operator;
240247
switch (operator) {
@@ -248,8 +255,11 @@ as rhs contains unescaped \`/\``);
248255
throw operator;
249256
}
250257

251-
assert((/\/(.*)\/(\w*)/.test(rhs)), (`RHS (${rhs}) must be a regex pattern. Do not rely on this behavior!
252-
Refer to https://docs.gitlab.com/ee/ci/jobs/job_rules.html#unexpected-behavior-from-regular-expression-matching-with- for more info...`));
258+
const assertMsg = [
259+
"RHS (${rhs}) must be a regex pattern. Do not rely on this behavior!",
260+
"Refer to https://docs.gitlab.com/ee/ci/jobs/job_rules.html#unexpected-behavior-from-regular-expression-matching-with- for more info...",
261+
];
262+
assert((/\/(.*)\/(\w*)/.test(rhs)), assertMsg.join("\n"));
253263

254264
const regex = /\/(?<pattern>.*)\/(?<flags>[igmsuy]*)/;
255265
const _rhs = rhs.replace(regex, (_: string, pattern: string, flags: string) => {
@@ -270,15 +280,16 @@ Refer to https://docs.gitlab.com/ee/ci/jobs/job_rules.html#unexpected-behavior-f
270280
res = (0, eval)(evalStr); // https://esbuild.github.io/content-types/#direct-eval
271281
delete (global as any).RE2; // Cleanup
272282
} catch (err) {
273-
assert(false, (`
274-
Error attempting to evaluate the following rules:
275-
rules:
276-
- if: '${expandedEvalStr}'
277-
as
278-
\`\`\`javascript
279-
${evalStr}
280-
\`\`\`
281-
`));
283+
const assertMsg = [
284+
"Error attempting to evaluate the following rules:",
285+
" rules:",
286+
` - if: '${expandedEvalStr}'`,
287+
"as",
288+
"```javascript",
289+
`${evalStr}`,
290+
"```",
291+
];
292+
assert(false, assertMsg.join("\n"));
282293
}
283294
return Boolean(res);
284295
}
@@ -303,8 +314,8 @@ ${evalStr}
303314
if (!Array.isArray(ruleChanges)) ruleChanges = ruleChanges.paths;
304315

305316
// NOTE: https://docs.gitlab.com/ee/ci/yaml/#ruleschanges
306-
// Glob patterns are interpreted with Ruby's [File.fnmatch](https://docs.ruby-lang.org/en/master/File.html#method-c-fnmatch)
307-
// with the flags File::FNM_PATHNAME | File::FNM_DOTMATCH | File::FNM_EXTGLOB.
317+
// Glob patterns are interpreted with Ruby's [File.fnmatch](https://docs.ruby-lang.org/en/master/File.html#method-c-fnmatch)
318+
// with the flags File::FNM_PATHNAME | File::FNM_DOTMATCH | File::FNM_EXTGLOB.
308319
return micromatch.some(GitData.changedFiles(`origin/${defaultBranch}`), ruleChanges, {
309320
nonegate: true,
310321
noextglob: true,

0 commit comments

Comments
 (0)