refactor(util/exec): Fix strict null errors (#12909)

Co-authored-by: Rhys Arkins <rhys@arkins.net> Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
renovatebot · Dec 10, 2021 · 995df4d · 995df4d
1 parent a86eceb
commit 995df4d
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 72 deletions.
diff --git a/lib/util/exec/buildpack.ts b/lib/util/exec/buildpack.ts
@@ -74,21 +74,21 @@ export async function resolveConstraint(
   }
 
   const pkgReleases = await getPkgReleases(toolConfig);
-  if (!pkgReleases?.releases?.length) {
-    throw new Error('No tool releases found.');
-  }
-
-  const allVersions = pkgReleases.releases.map((r) => r.version);
-  const matchingVersions = allVersions.filter(
-    (v) => !constraint || versioning.matches(v, constraint)
-  );
+  const releases = pkgReleases?.releases ?? [];
+  const versions = releases.map((r) => r.version);
+  const resolvedVersion = versions
+    .filter((v) => !constraint || versioning.matches(v, constraint))
+    .pop();
 
-  if (matchingVersions.length) {
-    const resolvedVersion = matchingVersions.pop();
+  if (resolvedVersion) {
     logger.debug({ toolName, constraint, resolvedVersion }, 'Resolved version');
     return resolvedVersion;
   }
-  const latestVersion = allVersions.filter((v) => versioning.isStable(v)).pop();
+
+  const latestVersion = versions.filter((v) => versioning.isStable(v)).pop();
+  if (!latestVersion) {
+    throw new Error('No tool releases found.');
+  }
   logger.warn(
     { toolName, constraint, latestVersion },
     'No matching tool versions found for constraint - using latest version'

diff --git a/lib/util/exec/docker/index.ts b/lib/util/exec/docker/index.ts
@@ -54,13 +54,17 @@ function uniq<T = unknown>(
 
 function prepareVolumes(volumes: VolumeOption[] = []): string[] {
   const expanded: (VolumesPair | null)[] = volumes.map(expandVolumeOption);
-  const filtered: VolumesPair[] = expanded.filter((vol) => vol !== null);
+  const filtered: VolumesPair[] = expanded.filter(
+    (vol): vol is VolumesPair => vol !== null
+  );
   const unique: VolumesPair[] = uniq<VolumesPair>(filtered, volumesEql);
   return unique.map(([from, to]) => `-v "${from}":"${to}"`);
 }
 
 function prepareCommands(commands: Opt<string>[]): string[] {
-  return commands.filter((command) => command && typeof command === 'string');
+  return commands.filter<string>((command): command is string =>
+    is.string(command)
+  );
 }
 
 export async function getDockerTag(
@@ -97,9 +101,8 @@ export async function getDockerTag(
       logger.debug('Filtering out unstable versions');
       versions = versions.filter((version) => ver.isStable(version));
     }
-    versions = versions.sort(ver.sortVersions.bind(ver));
-    if (versions.length) {
-      const version = versions.pop();
+    const version = versions.sort(ver.sortVersions.bind(ver)).pop();
+    if (version) {
       logger.debug(
         { depName, scheme, constraint, version },
         `Found compatible image version`
@@ -117,12 +120,12 @@ export async function getDockerTag(
   return 'latest';
 }
 
-function getContainerName(image: string, prefix?: string): string {
-  return `${prefix || 'renovate_'}${image}`.replace(regEx(/\//g), '_');
+function getContainerName(image: string, prefix?: string | undefined): string {
+  return `${prefix ?? 'renovate_'}${image}`.replace(regEx(/\//g), '_');
 }
 
-function getContainerLabel(prefix: string): string {
-  return `${prefix || 'renovate_'}child`;
+function getContainerLabel(prefix: string | undefined): string {
+  return `${prefix ?? 'renovate_'}child`;
 }
 
 export async function removeDockerContainer(
@@ -199,7 +202,7 @@ export async function generateDockerCommand(
 ): Promise<string> {
   const { envVars, cwd, tagScheme, tagConstraint } = options;
   let image = options.image;
-  const volumes = options.volumes || [];
+  const volumes = options.volumes ?? [];
   const {
     localDir,
     cacheDir,
@@ -221,7 +224,7 @@ export async function generateDockerCommand(
   if (envVars) {
     result.push(
       ...uniq(envVars)
-        .filter((x) => typeof x === 'string')
+        .filter(is.string)
         .map((e) => `-e ${e}`)
     );
   }
@@ -232,11 +235,11 @@ export async function generateDockerCommand(
 
   image = `${ensureTrailingSlash(dockerImagePrefix ?? 'renovate')}${image}`;
 
-  let tag: string;
+  let tag: string | null = null;
   if (options.tag) {
     tag = options.tag;
   } else if (tagConstraint) {
-    const tagVersioning = tagScheme || 'semver';
+    const tagVersioning = tagScheme ?? 'semver';
     tag = await getDockerTag(image, tagConstraint, tagVersioning);
     logger.debug(
       { image, tagConstraint, tagVersioning, tag },

diff --git a/lib/util/exec/index.spec.ts b/lib/util/exec/index.spec.ts
@@ -713,7 +713,7 @@ describe('util/exec/index', () => {
       actualCmd.push(execCmd);
       actualOpts.push(execOpts);
       callback(null, { stdout: '', stderr: '' });
-      return undefined;
+      return undefined as never;
     });
     GlobalConfig.set({ cacheDir, localDir: cwd, ...adminConfig });
     await exec(cmd as string, inOpts);
@@ -729,7 +729,7 @@ describe('util/exec/index', () => {
     cpExec.mockImplementation((execCmd, execOpts, callback) => {
       actualCmd.push(execCmd);
       callback(null, { stdout: '', stderr: '' });
-      return undefined;
+      return undefined as never;
     });
 
     GlobalConfig.set({ binarySource: 'global' });

diff --git a/lib/util/exec/index.ts b/lib/util/exec/index.ts
@@ -1,3 +1,4 @@
+import is from '@sindresorhus/is';
 import { dirname, join } from 'upath';
 import { GlobalConfig } from '../../config/global';
 import { TEMPORARY_ERROR } from '../../constants/error-messages';
@@ -11,50 +12,50 @@ import type {
   ExecOptions,
   ExecResult,
   ExtraEnv,
+  Opt,
   RawExecOptions,
 } from './types';
 
 function getChildEnv({
-  extraEnv = {},
+  extraEnv,
   env: forcedEnv = {},
-}: ExecOptions): ExtraEnv<string> {
+}: ExecOptions): Record<string, string> {
   const globalConfigEnv = GlobalConfig.get('customEnvVariables');
 
-  const inheritedKeys = Object.entries(extraEnv).reduce(
-    (acc, [key, val]) =>
-      val === null || val === undefined ? acc : [...acc, key],
-    []
-  );
+  const inheritedKeys: string[] = [];
+  for (const [key, val] of Object.entries(extraEnv ?? {})) {
+    if (is.string(val)) {
+      inheritedKeys.push(key);
+    }
+  }
+
   const parentEnv = getChildProcessEnv(inheritedKeys);
-  const childEnv = Object.entries({
+  const combinedEnv = {
     ...extraEnv,
     ...parentEnv,
     ...globalConfigEnv,
     ...forcedEnv,
-  }).reduce(
-    (acc, [key, val]) =>
-      val === null || val === undefined
-        ? acc
-        : { ...acc, [key]: val.toString() },
-    {}
-  );
-  return childEnv;
+  };
+
+  const result: Record<string, string> = {};
+  for (const [key, val] of Object.entries(combinedEnv)) {
+    if (is.string(val)) {
+      result[key] = `${val}`;
+    }
+  }
+
+  return result;
 }
 
-function dockerEnvVars(
-  extraEnv: ExtraEnv,
-  childEnv: ExtraEnv<string>
-): string[] {
+function dockerEnvVars(extraEnv: ExtraEnv, childEnv: ExtraEnv): string[] {
   const extraEnvKeys = Object.keys(extraEnv || {});
-  return extraEnvKeys.filter(
-    (key) => typeof childEnv[key] === 'string' && childEnv[key].length > 0
-  );
+  return extraEnvKeys.filter((key) => is.nonEmptyString(childEnv[key]));
 }
 
 function getCwd({ cwd, cwdFile }: ExecOptions): string {
   const defaultCwd = GlobalConfig.get('localDir');
   const paramCwd = cwdFile ? join(defaultCwd, dirname(cwdFile)) : cwd;
-  return paramCwd || defaultCwd;
+  return paramCwd ?? defaultCwd;
 }
 
 function getRawExecOptions(opts: ExecOptions): RawExecOptions {
@@ -78,11 +79,11 @@ function getRawExecOptions(opts: ExecOptions): RawExecOptions {
   }
 
   // Set default max buffer size to 10MB
-  rawExecOptions.maxBuffer = rawExecOptions.maxBuffer || 10 * 1024 * 1024;
+  rawExecOptions.maxBuffer = rawExecOptions.maxBuffer ?? 10 * 1024 * 1024;
   return rawExecOptions;
 }
 
-function isDocker({ docker }: ExecOptions): boolean {
+function isDocker(docker: Opt<DockerOptions>): docker is DockerOptions {
   const { binarySource } = GlobalConfig.get();
   return binarySource === 'docker' && !!docker;
 }
@@ -103,7 +104,7 @@ async function prepareRawExec(
 
   let rawCommands = typeof cmd === 'string' ? [cmd] : cmd;
 
-  if (isDocker(opts)) {
+  if (isDocker(docker)) {
     logger.debug('Using docker to execute');
     const extraEnv = { ...opts.extraEnv, ...customEnvVariables };
     const childEnv = getChildEnv(opts);
@@ -112,7 +113,7 @@ async function prepareRawExec(
     const dockerOptions: DockerOptions = { ...docker, cwd, envVars };
     const preCommands = [
       ...(await generateInstallCommands(opts.toolConstraints)),
-      ...(opts.preCommands || []),
+      ...(opts.preCommands ?? []),
     ];
     const dockerCommand = await generateDockerCommand(
       rawCommands,
@@ -136,12 +137,12 @@ export async function exec(
   opts: ExecOptions = {}
 ): Promise<ExecResult> {
   const { docker } = opts;
-  const { dockerChildPrefix } = GlobalConfig.get();
+  const dockerChildPrefix = GlobalConfig.get('dockerChildPrefix');
 
   const { rawCommands, rawOptions } = await prepareRawExec(cmd, opts);
-  const useDocker = isDocker(opts);
+  const useDocker = isDocker(docker);
 
-  let res: ExecResult | null = null;
+  let res: ExecResult = { stdout: '', stderr: '' };
   for (const rawCmd of rawCommands) {
     const startTime = Date.now();
     if (useDocker) {
@@ -173,17 +174,15 @@ export async function exec(
       throw err;
     }
     const durationMs = Math.round(Date.now() - startTime);
-    if (res) {
-      logger.debug(
-        {
-          cmd: rawCmd,
-          durationMs,
-          stdout: res.stdout,
-          stderr: res.stderr,
-        },
-        'exec completed'
-      );
-    }
+    logger.debug(
+      {
+        cmd: rawCmd,
+        durationMs,
+        stdout: res.stdout,
+        stderr: res.stderr,
+      },
+      'exec completed'
+    );
   }
 
   return res;

diff --git a/test/exec-util.ts b/test/exec-util.ts
@@ -50,7 +50,7 @@ export function mockExecAll(
       throw execResult;
     }
     callback(null, execResult);
-    return undefined;
+    return undefined as never;
   });
   return snapshots;
 }
@@ -67,7 +67,7 @@ export function mockExecSequence(
         throw execResult;
       }
       callback(null, execResult);
-      return undefined;
+      return undefined as never;
     });
   });
   return snapshots;

diff --git a/tsconfig.strict.json b/tsconfig.strict.json
@@ -26,8 +26,7 @@
     "lib/util/cache/*.ts",
     "lib/util/clone.ts",
     "lib/util/date.ts",
-    "lib/util/exec/common.ts",
-    "lib/util/exec/types.ts",
+    "lib/util/exec",
     "lib/util/fs",
     "lib/util/git/config.ts",
     "lib/util/git/types.ts",
@@ -68,6 +67,13 @@
     "lib/logger/err-serializer.spec.ts",
     "lib/manager/gradle/shallow/types.ts",
     "lib/util/cache/*.spec.ts",
+    "lib/util/exec/buildpack.spec.ts",
+    "lib/util/exec/buildpack.ts",
+    "lib/util/exec/docker/index.spec.ts",
+    "lib/util/exec/docker/index.ts",
+    "lib/util/exec/env.spec.ts",
+    "lib/util/exec/index.spec.ts",
+    "lib/util/exec/index.ts",
     "lib/util/fs/index.spec.ts"
   ]
 }