Skip to content

Commit

Permalink
refactor: strict null checks for util (#15150)
Browse files Browse the repository at this point in the history
  • Loading branch information
viceice committed Apr 17, 2022
1 parent 6330414 commit 955b442
Show file tree
Hide file tree
Showing 11 changed files with 67 additions and 58 deletions.
10 changes: 6 additions & 4 deletions lib/util/exec/buildpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { id as composerVersioningId } from '../../modules/versioning/composer';
import { id as npmVersioningId } from '../../modules/versioning/npm';
import { id as pep440VersioningId } from '../../modules/versioning/pep440';
import { id as semverVersioningId } from '../../modules/versioning/semver';
import type { ToolConfig, ToolConstraint } from './types';
import type { Opt, ToolConfig, ToolConstraint } from './types';

const allToolConfig: Record<string, ToolConfig> = {
bundler: {
Expand Down Expand Up @@ -61,7 +61,9 @@ export function isBuildpack(): boolean {
return !!process.env.BUILDPACK;
}

export function isDynamicInstall(toolConstraints?: ToolConstraint[]): boolean {
export function isDynamicInstall(
toolConstraints?: Opt<ToolConstraint[]>
): boolean {
const { binarySource } = GlobalConfig.get();
if (binarySource !== 'install') {
return false;
Expand Down Expand Up @@ -125,9 +127,9 @@ export async function resolveConstraint(
}

export async function generateInstallCommands(
toolConstraints: ToolConstraint[]
toolConstraints: Opt<ToolConstraint[]>
): Promise<string[]> {
const installCommands = [];
const installCommands: string[] = [];
if (toolConstraints?.length) {
for (const toolConstraint of toolConstraints) {
const toolVersion = await resolveConstraint(toolConstraint);
Expand Down
5 changes: 3 additions & 2 deletions lib/util/exec/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function dockerEnvVars(extraEnv: ExtraEnv, childEnv: ExtraEnv): string[] {
return extraEnvKeys.filter((key) => is.nonEmptyString(childEnv[key]));
}

function getCwd({ cwd, cwdFile }: ExecOptions): string {
function getCwd({ cwd, cwdFile }: ExecOptions): string | undefined {
const defaultCwd = GlobalConfig.get('localDir');
const paramCwd = cwdFile
? upath.join(defaultCwd, upath.dirname(cwdFile))
Expand Down Expand Up @@ -139,7 +139,8 @@ export async function exec(
opts: ExecOptions = {}
): Promise<ExecResult> {
const { docker } = opts;
const dockerChildPrefix = GlobalConfig.get('dockerChildPrefix');
const dockerChildPrefix =
GlobalConfig.get('dockerChildPrefix') ?? 'renovate_';

const { rawCommands, rawOptions } = await prepareRawExec(cmd, opts);
const useDocker = isDocker(docker);
Expand Down
5 changes: 1 addition & 4 deletions lib/util/exec/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ export type Opt<T> = T | null | undefined;
export type VolumesPair = [string, string];
export type VolumeOption = Opt<string | VolumesPair>;

export type DockerExtraCommand = Opt<string>;
export type DockerExtraCommands = Opt<DockerExtraCommand[]>;

export interface DockerOptions {
image: string;
tag?: Opt<string>;
Expand Down Expand Up @@ -48,7 +45,7 @@ export interface ExecOptions {
extraEnv?: Opt<ExtraEnv>;
docker?: Opt<DockerOptions>;
toolConstraints?: Opt<ToolConstraint[]>;
preCommands?: DockerExtraCommands;
preCommands?: Opt<string[]>;
// Following are pass-through to child process
maxBuffer?: number | undefined;
timeout?: number | undefined;
Expand Down
2 changes: 1 addition & 1 deletion lib/util/git/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ describe('util/git/index', () => {
};
await git.commitFiles({
branchName: 'renovate/branch_with_changes',
files: [file],
files: [file, { type: 'addition', path: 'dummy', contents: null }],
message: 'Create something',
});
const branchFiles = await git.getBranchFiles(
Expand Down
36 changes: 20 additions & 16 deletions lib/util/git/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ async function getDefaultBranch(git: SimpleGit): Promise<string> {
res = (await git.raw(['remote', 'show', 'origin']))
.split('\n')
.map((line) => line.trim())
.find((line) => line.startsWith(headPrefix))
.find((line) => line.startsWith(headPrefix))!
.replace(headPrefix, '');
}
return res.replace('origin/', '').trim();
Expand All @@ -144,15 +144,16 @@ async function getDefaultBranch(git: SimpleGit): Promise<string> {

let config: LocalConfig = {} as any;

let git: SimpleGit | undefined;
// TODO: can be undefined
let git: SimpleGit;
let gitInitialized: boolean;

let privateKeySet = false;

export const GIT_MINIMUM_VERSION = '2.33.0'; // git show-current

export async function validateGitVersion(): Promise<boolean> {
let version: string;
let version: string | undefined;
const globalGit = simpleGit();
try {
const raw = await globalGit.raw(['--version']);
Expand Down Expand Up @@ -248,7 +249,7 @@ async function cleanLocalBranches(): Promise<void> {
}
}

export function setGitAuthor(gitAuthor: string): void {
export function setGitAuthor(gitAuthor: string | undefined): void {
const gitAuthorParsed = parseGitAuthor(
gitAuthor || 'Renovate Bot <renovate@whitesourcesoftware.com>'
);
Expand Down Expand Up @@ -325,8 +326,8 @@ export async function syncGit(): Promise<void> {
return;
}
gitInitialized = true;
const { localDir } = GlobalConfig.get();
logger.debug('Initializing git repository into ' + localDir);
const localDir = GlobalConfig.get('localDir')!;
logger.debug(`Initializing git repository into ${localDir}`);
const gitHead = upath.join(localDir, '.git/HEAD');
let clone = true;

Expand Down Expand Up @@ -355,7 +356,7 @@ export async function syncGit(): Promise<void> {
if (clone) {
const cloneStart = Date.now();
try {
const opts = [];
const opts: string[] = [];
if (config.fullClone) {
logger.debug('Performing full clone');
} else {
Expand Down Expand Up @@ -506,10 +507,10 @@ export async function getFileList(): Promise<string[]> {
}
return files
.split(newlineRegex)
.filter(Boolean)
.filter(is.string)
.filter((line) => line.startsWith('100'))
.map((line) => line.split(regEx(/\t/)).pop())
.filter((file: string) =>
.map((line) => line.split(regEx(/\t/)).pop()!)
.filter((file) =>
submodules.every((submodule: string) => !file.startsWith(submodule))
);
}
Expand Down Expand Up @@ -557,7 +558,7 @@ export async function isBranchModified(branchName: string): Promise<boolean> {
return false;
}
// Retrieve the author of the most recent commit
let lastAuthor: string;
let lastAuthor: string | undefined;
try {
lastAuthor = (
await git.raw([
Expand Down Expand Up @@ -691,7 +692,7 @@ export async function deleteBranch(branchName: string): Promise<void> {
}

export async function mergeBranch(branchName: string): Promise<void> {
let status: StatusResult;
let status: StatusResult | undefined;
try {
await syncGit();
await git.reset(ResetMode.HARD);
Expand Down Expand Up @@ -742,7 +743,9 @@ export async function getBranchLastCommitTime(
}
}

export async function getBranchFiles(branchName: string): Promise<string[]> {
export async function getBranchFiles(
branchName: string
): Promise<string[] | null> {
await syncGit();
try {
const diff = await gitRetry(() =>
Expand Down Expand Up @@ -815,7 +818,7 @@ export async function prepareCommit({
message,
force = false,
}: CommitFilesConfig): Promise<CommitResult | null> {
const { localDir } = GlobalConfig.get();
const localDir = GlobalConfig.get('localDir')!;
await syncGit();
logger.debug(`Preparing files for committing to branch ${branchName}`);
await handleCommitAuth(localDir);
Expand Down Expand Up @@ -848,7 +851,6 @@ export async function prepareCommit({
// This is usually a git submodule update
logger.trace({ fileName }, 'Adding directory commit');
} else if (file.contents === null) {
// istanbul ignore next
continue;
} else {
let contents: Buffer;
Expand Down Expand Up @@ -1135,7 +1137,9 @@ const treeShaRegex = regEx(/tree\s+(?<treeSha>[0-9a-f]{40})\s*/);
*/
export async function listCommitTree(commitSha: string): Promise<TreeItem[]> {
const commitOutput = await git.catFile(['-p', commitSha]);
const { treeSha } = treeShaRegex.exec(commitOutput)?.groups ?? {};
const { treeSha } =
treeShaRegex.exec(commitOutput)?.groups ??
/* istanbul ignore next: will never happen */ {};
const contents = await git.catFile(['-p', treeSha]);
const lines = contents.split(newlineRegex);
const result: TreeItem[] = [];
Expand Down
9 changes: 3 additions & 6 deletions lib/util/git/private-key.spec.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import { mocked } from '../../../test/util';
import * as exec_ from '../exec';
import {
configSigningKey,
setPrivateKey,
writePrivateKey,
} from './private-key';
import { configSigningKey, writePrivateKey } from './private-key';
import { setPrivateKey } from '.';

jest.mock('fs-extra');
jest.mock('../exec');
Expand All @@ -20,7 +17,7 @@ describe('util/git/private-key', () => {

it('throws error if failing', async () => {
setPrivateKey('some-key');
exec.exec.mockResolvedValueOnce({
exec.exec.mockRejectedValueOnce({
stderr: `something wrong`,
stdout: '',
});
Expand Down
8 changes: 4 additions & 4 deletions lib/util/git/private-key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import { logger } from '../../logger';
import { exec } from '../exec';
import { newlineRegex } from '../regex';

let gitPrivateKey: string;
let keyId: string;
let gitPrivateKey: string | undefined;
let keyId: string | undefined;

export function setPrivateKey(key: string): void {
gitPrivateKey = key?.trim();
Expand All @@ -21,10 +21,10 @@ async function importKey(): Promise<void> {
await fs.outputFile(keyFileName, gitPrivateKey);
const { stdout, stderr } = await exec(`gpg --import ${keyFileName}`);
logger.debug({ stdout, stderr }, 'Private key import result');
keyId = (stdout + stderr)
keyId = `${stdout}${stderr}`
.split(newlineRegex)
.find((line) => line.includes('secret key imported'))
.replace('gpg: key ', '')
?.replace('gpg: key ', '')
.split(':')
.shift();
await fs.remove(keyFileName);
Expand Down
2 changes: 1 addition & 1 deletion lib/util/git/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export interface LocalConfig extends StorageConfig {
branchCommits: Record<string, CommitSha>;
branchIsModified: Record<string, boolean>;
ignoredAuthors: string[];
gitAuthorName?: string;
gitAuthorName?: string | null;
gitAuthorEmail?: string;

writeGitDone?: boolean;
Expand Down
19 changes: 18 additions & 1 deletion lib/util/package-rules.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ describe('util/package-rules', () => {
},
{
excludePackagePatterns: ['*'],
matchPackageNames: ['b'],
},
{
matchUpdateTypes: ['bump'],
Expand Down Expand Up @@ -340,6 +339,10 @@ describe('util/package-rules', () => {
matchDatasources: [OrbDatasource.id, DockerDatasource.id],
x: 1,
},
{
matchDatasources: [DockerDatasource.id],
y: 1,
},
],
};
const dep = {
Expand All @@ -349,6 +352,7 @@ describe('util/package-rules', () => {
};
const res = applyPackageRules({ ...config, ...dep });
expect(res.x).toBe(1);
expect(res.y).toBeUndefined();
});

it('filters branches with matching branch', () => {
Expand Down Expand Up @@ -446,6 +450,10 @@ describe('util/package-rules', () => {
matchUpdateTypes: ['minor', 'patch'],
x: 1,
},
{
matchUpdateTypes: ['minor'],
y: 1,
},
],
};
const dep = {
Expand All @@ -455,6 +463,7 @@ describe('util/package-rules', () => {
};
const res = applyPackageRules({ ...config, ...dep });
expect(res.x).toBe(1);
expect(res.y).toBeUndefined();
});

it('matches matchSourceUrlPrefixes', () => {
Expand Down Expand Up @@ -649,6 +658,14 @@ describe('util/package-rules', () => {
},
});
expect(res2.x).toBeUndefined();
const res3 = applyPackageRules({
...config,
...{
depName: 'test',
lockedVersion: '^1.0.0',
},
});
expect(res3.x).toBe(1);
});

it('checks if matchCurrentVersion selector is valid and satisfies the condition on pinned to range overlap', () => {
Expand Down

0 comments on commit 955b442

Please sign in to comment.