From e24bf474ef08ea55b0df1d23688dfea51216e4fe Mon Sep 17 00:00:00 2001 From: Michael Kriese <michael.kriese@visualon.de> Date: Fri, 22 Apr 2022 16:13:14 +0200 Subject: [PATCH] refactor(platform): more strict null checks (#15247) --- .../platform/bitbucket-server/index.spec.ts | 7 ++- .../platform/bitbucket-server/index.ts | 33 ++++++---- .../platform/bitbucket-server/utils.spec.ts | 4 +- .../platform/bitbucket-server/utils.ts | 3 +- .../platform/gitea/gitea-helper.spec.ts | 6 +- lib/modules/platform/gitea/gitea-helper.ts | 4 +- lib/modules/platform/gitea/index.spec.ts | 28 +++++---- lib/modules/platform/gitea/index.ts | 63 +++++++++++-------- lib/modules/platform/types.ts | 6 +- tsconfig.strict.json | 3 - 10 files changed, 90 insertions(+), 67 deletions(-) diff --git a/lib/modules/platform/bitbucket-server/index.spec.ts b/lib/modules/platform/bitbucket-server/index.spec.ts index ee0b9bb1d6..35940110fa 100644 --- a/lib/modules/platform/bitbucket-server/index.spec.ts +++ b/lib/modules/platform/bitbucket-server/index.spec.ts @@ -1,3 +1,4 @@ +import is from '@sindresorhus/is'; import * as httpMock from '../../../../test/http-mock'; import { REPOSITORY_CHANGED, @@ -56,7 +57,7 @@ function repoMock( name: 'ssh', } : null, - ].filter(Boolean); + ].filter(is.truthy); } return { @@ -1119,7 +1120,7 @@ describe('modules/platform/bitbucket-server/index', () => { await bitbucket.findPr({ branchName: 'userName1/pullRequest1', }) - ).toBeUndefined(); + ).toBeNull(); }); }); @@ -1161,7 +1162,7 @@ describe('modules/platform/bitbucket-server/index', () => { prTitle: 'title', state: PrState.Closed, }) - ).toBeUndefined(); + ).toBeNull(); }); }); diff --git a/lib/modules/platform/bitbucket-server/index.ts b/lib/modules/platform/bitbucket-server/index.ts index 9440b56e89..3d318191f6 100644 --- a/lib/modules/platform/bitbucket-server/index.ts +++ b/lib/modules/platform/bitbucket-server/index.ts @@ -73,7 +73,7 @@ const defaults: { /* istanbul ignore next */ function updatePrVersion(pr: number, version: number): number { - const res = Math.max(config.prVersions.get(pr) || 0, version); + const res = Math.max(config.prVersions.get(pr) ?? 0, version); config.prVersions.set(pr, res); return res; } @@ -144,7 +144,8 @@ export async function getJsonFile( repoName?: string, branchOrTag?: string ): Promise<any | null> { - const raw = await getRawFile(fileName, repoName, branchOrTag); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + const raw = (await getRawFile(fileName, repoName, branchOrTag)) as string; if (fileName.endsWith('.json5')) { return JSON5.parse(raw); } @@ -193,7 +194,8 @@ export async function initRepo({ const gitUrl = utils.getRepoGitUrl( config.repositorySlug, - defaults.endpoint, + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + defaults.endpoint!, info, opts ); @@ -264,7 +266,8 @@ export async function getPr( reviewers: res.body.reviewers.map((r) => r.user.name), }; pr.hasReviewers = is.nonEmptyArray(pr.reviewers); - pr.version = updatePrVersion(pr.number, pr.version); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + pr.version = updatePrVersion(pr.number, pr.version!); return pr; } @@ -295,7 +298,7 @@ export async function getPrList(refreshCache?: boolean): Promise<Pr[]> { logger.debug(`getPrList()`); // istanbul ignore next if (!config.prList || refreshCache) { - const searchParams = { + const searchParams: Record<string, string> = { state: 'ALL', }; if (!config.ignorePrAuthor) { @@ -331,7 +334,7 @@ export async function findPr({ } else { logger.debug(`Renovate did not find a PR for branch #${branchName}`); } - return pr; + return pr ?? null; } // Returns the Pull Request for a branch. Null if not exists. @@ -503,7 +506,7 @@ export function findIssue(title: string): Promise<Issue | null> { // config error notifications, or "dependencyDashboard" // // Bitbucket Server does not have issues - return null; + return Promise.resolve(null); } /* istanbul ignore next */ @@ -516,7 +519,7 @@ export function ensureIssue({ // config error notifications, or "dependencyDashboard" // // Bitbucket Server does not have issues - return null; + return Promise.resolve(null); } /* istanbul ignore next */ @@ -563,7 +566,9 @@ export async function addReviewers( throw new Error(REPOSITORY_NOT_FOUND); } - const reviewersSet = new Set([...pr.reviewers, ...reviewers]); + // TODO: can `reviewers` be undefined? + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + const reviewersSet = new Set([...pr.reviewers!, ...reviewers]); await bitbucketServerHttp.putJson( `./rest/api/1.0/projects/${config.projectKey}/repos/${config.repositorySlug}/pull-requests/${prNo}`, @@ -759,7 +764,7 @@ export async function ensureCommentRemoval( // Pull Request const escapeHash = (input: string): string => - input ? input.replace(regEx(/#/g), '%23') : input; + input?.replace(regEx(/#/g), '%23'); export async function createPr({ sourceBranch, @@ -833,7 +838,8 @@ export async function createPr({ ...utils.prInfo(prInfoRes.body), }; - updatePrVersion(pr.number, pr.version); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + updatePrVersion(pr.number, pr.version!); // istanbul ignore if if (config.prList) { @@ -872,7 +878,7 @@ export async function updatePr({ description, version: pr.version, reviewers: pr.reviewers - .filter( + ?.filter( (name: string) => !bitbucketInvalidReviewers?.includes(name) ) .map((name: string) => ({ user: { name } })), @@ -886,7 +892,8 @@ export async function updatePr({ const newState = { [PrState.Open]: 'OPEN', [PrState.Closed]: 'DECLINED', - }[state]; + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + }[state!]; if ( newState && diff --git a/lib/modules/platform/bitbucket-server/utils.spec.ts b/lib/modules/platform/bitbucket-server/utils.spec.ts index 32a89e4d70..c8598c2107 100644 --- a/lib/modules/platform/bitbucket-server/utils.spec.ts +++ b/lib/modules/platform/bitbucket-server/utils.spec.ts @@ -7,7 +7,9 @@ import { } from './utils'; describe('modules/platform/bitbucket-server/utils', () => { - function createError(body: Partial<BitbucketErrorResponse> = undefined) { + function createError( + body: Partial<BitbucketErrorResponse> | undefined = undefined + ) { return partial<BitbucketError>({ response: partial<Response<BitbucketErrorResponse>>({ body }), }); diff --git a/lib/modules/platform/bitbucket-server/utils.ts b/lib/modules/platform/bitbucket-server/utils.ts index 1a6b47e660..8db6214787 100644 --- a/lib/modules/platform/bitbucket-server/utils.ts +++ b/lib/modules/platform/bitbucket-server/utils.ts @@ -175,7 +175,8 @@ export function getRepoGitUrl( protocol: defaultEndpoint.split(':')[0] as GitProtocol, auth: `${opts.username}:${opts.password}`, host: `${host}${pathname}${ - pathname.endsWith('/') ? '' : /* istanbul ignore next */ '/' + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + pathname!.endsWith('/') ? '' : /* istanbul ignore next */ '/' }scm`, repository, }); diff --git a/lib/modules/platform/gitea/gitea-helper.spec.ts b/lib/modules/platform/gitea/gitea-helper.spec.ts index c61e43b55f..ecea2bc3b6 100644 --- a/lib/modules/platform/gitea/gitea-helper.spec.ts +++ b/lib/modules/platform/gitea/gitea-helper.spec.ts @@ -16,7 +16,7 @@ describe('modules/platform/gitea/gitea-helper', () => { email: 'admin@example.com', }; - const otherMockUser: ght.User = { + const otherMockUser: ght.User & Required<Pick<ght.User, 'full_name'>> = { ...mockUser, username: 'renovate', full_name: 'Renovate Bot', @@ -296,8 +296,8 @@ describe('modules/platform/gitea/gitea-helper', () => { state: mockPR.state, title: mockPR.title, body: mockPR.body, - base: mockPR.base.ref, - head: mockPR.head.label, + base: mockPR.base?.ref, + head: mockPR.head?.label, assignees: [mockUser.username], labels: [mockLabel.id], }); diff --git a/lib/modules/platform/gitea/gitea-helper.ts b/lib/modules/platform/gitea/gitea-helper.ts index 2511526c86..adaa53a985 100644 --- a/lib/modules/platform/gitea/gitea-helper.ts +++ b/lib/modules/platform/gitea/gitea-helper.ts @@ -52,7 +52,7 @@ export interface Issue { export interface User { id: number; email: string; - full_name: string; + full_name?: string; username: string; } @@ -147,7 +147,7 @@ export type IssueUpdateParams = { }; export type IssueUpdateLabelsParams = { - labels: number[]; + labels?: number[]; }; export type IssueSearchParams = { diff --git a/lib/modules/platform/gitea/index.spec.ts b/lib/modules/platform/gitea/index.spec.ts index 197cdec0f4..0780d21c46 100644 --- a/lib/modules/platform/gitea/index.spec.ts +++ b/lib/modules/platform/gitea/index.spec.ts @@ -53,20 +53,22 @@ describe('modules/platform/gitea/index', () => { }, }); + type MockPr = ght.PR & Required<Pick<ght.PR, 'head' | 'base'>>; + const mockRepos: ght.Repo[] = [ partial<ght.Repo>({ full_name: 'a/b' }), partial<ght.Repo>({ full_name: 'c/d' }), ]; - const mockPRs: ght.PR[] = [ - partial<ght.PR>({ + const mockPRs: MockPr[] = [ + partial<MockPr>({ number: 1, title: 'Some PR', body: 'some random pull request', state: PrState.Open, diff_url: 'https://gitea.renovatebot.com/some/repo/pulls/1.diff', created_at: '2015-03-22T20:36:16Z', - closed_at: null, + closed_at: undefined, mergeable: true, base: { ref: 'some-base-branch' }, head: { @@ -75,7 +77,7 @@ describe('modules/platform/gitea/index', () => { repo: partial<ght.Repo>({ full_name: mockRepo.full_name }), }, }), - partial<ght.PR>({ + partial<MockPr>({ number: 2, title: 'Other PR', body: 'other random pull request', @@ -676,7 +678,7 @@ describe('modules/platform/gitea/index', () => { }); describe('createPr', () => { - const mockNewPR: ght.PR = { + const mockNewPR: MockPr = { number: 42, state: PrState.Open, head: { @@ -917,11 +919,11 @@ describe('modules/platform/gitea/index', () => { describe('getIssue', () => { it('should return the issue', async () => { - const mockIssue = mockIssues.find((i) => i.number === 1); + const mockIssue = mockIssues.find((i) => i.number === 1)!; helper.getIssue.mockResolvedValueOnce(mockIssue); await initFakeRepo(); - expect(await gitea.getIssue(mockIssue.number)).toHaveProperty( + expect(await gitea.getIssue?.(mockIssue.number)).toHaveProperty( 'number', mockIssue.number ); @@ -930,7 +932,7 @@ describe('modules/platform/gitea/index', () => { describe('findIssue', () => { it('should return existing open issue', async () => { - const mockIssue = mockIssues.find((i) => i.title === 'open-issue'); + const mockIssue = mockIssues.find((i) => i.title === 'open-issue')!; helper.searchIssues.mockResolvedValueOnce(mockIssues); helper.getIssue.mockResolvedValueOnce(mockIssue); await initFakeRepo(); @@ -942,7 +944,7 @@ describe('modules/platform/gitea/index', () => { }); it('should not return existing closed issue', async () => { - const mockIssue = mockIssues.find((i) => i.title === 'closed-issue'); + const mockIssue = mockIssues.find((i) => i.title === 'closed-issue')!; helper.searchIssues.mockResolvedValueOnce(mockIssues); await initFakeRepo(); @@ -1016,7 +1018,7 @@ describe('modules/platform/gitea/index', () => { }); it('should not reopen closed issue by default', async () => { - const closedIssue = mockIssues.find((i) => i.title === 'closed-issue'); + const closedIssue = mockIssues.find((i) => i.title === 'closed-issue')!; helper.searchIssues.mockResolvedValueOnce(mockIssues); helper.updateIssue.mockResolvedValueOnce(closedIssue); @@ -1150,7 +1152,7 @@ describe('modules/platform/gitea/index', () => { }); it('should reopen closed issue if desired', async () => { - const closedIssue = mockIssues.find((i) => i.title === 'closed-issue'); + const closedIssue = mockIssues.find((i) => i.title === 'closed-issue')!; helper.searchIssues.mockResolvedValueOnce(mockIssues); helper.updateIssue.mockResolvedValueOnce(closedIssue); @@ -1176,7 +1178,7 @@ describe('modules/platform/gitea/index', () => { }); it('should not update existing closed issue if desired', async () => { - const closedIssue = mockIssues.find((i) => i.title === 'closed-issue'); + const closedIssue = mockIssues.find((i) => i.title === 'closed-issue')!; helper.searchIssues.mockResolvedValueOnce(mockIssues); await initFakeRepo(); @@ -1336,7 +1338,7 @@ describe('modules/platform/gitea/index', () => { const res = await gitea.ensureComment({ number: 1, content: 'other-content', - topic: undefined, + topic: null, }); expect(res).toBe(true); diff --git a/lib/modules/platform/gitea/index.ts b/lib/modules/platform/gitea/index.ts index a2fcbd3f04..98e70459c5 100644 --- a/lib/modules/platform/gitea/index.ts +++ b/lib/modules/platform/gitea/index.ts @@ -124,14 +124,14 @@ function findCommentByTopic( comments: helper.Comment[], topic: string ): helper.Comment | null { - return comments.find((c) => c.body.startsWith(`### ${topic}\n\n`)); + return comments.find((c) => c.body.startsWith(`### ${topic}\n\n`)) ?? null; } function findCommentByContent( comments: helper.Comment[], content: string ): helper.Comment | null { - return comments.find((c) => c.body.trim() === content); + return comments.find((c) => c.body.trim() === content) ?? null; } function getLabelList(): Promise<helper.Label[]> { @@ -156,11 +156,11 @@ function getLabelList(): Promise<helper.Label[]> { .catch((err) => { // Will fail if owner of repo is not org or Gitea version < 1.12 logger.debug(`Unable to fetch organization labels`); - return []; + return [] as helper.Label[]; }); config.labelList = Promise.all([repoLabels, orgLabels]).then((labels) => - [].concat(...labels) + ([] as helper.Label[]).concat(...labels) ); } @@ -170,7 +170,7 @@ function getLabelList(): Promise<helper.Label[]> { async function lookupLabelByName(name: string): Promise<number | null> { logger.debug(`lookupLabelByName(${name})`); const labelList = await getLabelList(); - return labelList.find((l) => l.name === name)?.id; + return labelList.find((l) => l.name === name)?.id ?? null; } const platform: Platform = { @@ -217,7 +217,7 @@ const platform: Platform = { ): Promise<string | null> { const repo = repoName ?? config.repository; const contents = await helper.getRepoContents(repo, fileName, branchOrTag); - return contents.contentString; + return contents.contentString ?? null; }, async getJsonFile( @@ -225,7 +225,12 @@ const platform: Platform = { repoName?: string, branchOrTag?: string ): Promise<any | null> { - const raw = await platform.getRawFile(fileName, repoName, branchOrTag); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + const raw = (await platform.getRawFile( + fileName, + repoName, + branchOrTag + )) as string; if (fileName.endsWith('.json5')) { return JSON5.parse(raw); } @@ -240,7 +245,7 @@ const platform: Platform = { config = {} as any; config.repository = repository; - config.cloneSubmodules = cloneSubmodules; + config.cloneSubmodules = !!cloneSubmodules; // Attempt to fetch information about repository try { @@ -299,7 +304,7 @@ const platform: Platform = { url: defaults.endpoint, }); const gitEndpoint = URL.parse(repo.clone_url); - gitEndpoint.auth = opts.token; + gitEndpoint.auth = opts.token ?? null; // Initialize Git storage await git.initRepo({ @@ -342,7 +347,9 @@ const platform: Platform = { try { // Create new status for branch commit const branchCommit = git.getBranchCommit(branchName); - await helper.createCommitStatus(config.repository, branchCommit, { + // TODO: check branchCommit + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + await helper.createCommitStatus(config.repository, branchCommit!, { state: helper.renovateToGiteaStatusMapping[state] || 'pending', context, description, @@ -413,7 +420,7 @@ const platform: Platform = { { useCache: false } ) .then((prs) => { - const prList = prs.map(toRenovatePR).filter(Boolean); + const prList = prs.map(toRenovatePR).filter(is.truthy); logger.debug(`Retrieved ${prList.length} Pull Requests`); return prList; }); @@ -425,7 +432,7 @@ const platform: Platform = { async getPr(number: number): Promise<Pr | null> { // Search for pull request in cached list or attempt to query directly const prList = await platform.getPrList(); - let pr = prList.find((p) => p.number === number); + let pr = prList.find((p) => p.number === number) ?? null; if (pr) { logger.debug('Returning from cached PRs'); } else { @@ -435,7 +442,8 @@ const platform: Platform = { // Add pull request to cache for further lookups / queries if (config.prList !== null) { - (await config.prList).push(pr); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + (await config.prList).push(pr!); } } @@ -451,7 +459,7 @@ const platform: Platform = { branchName, prTitle: title, state = PrState.All, - }: FindPRConfig): Promise<Pr> { + }: FindPRConfig): Promise<Pr | null> { logger.debug(`findPr(${branchName}, ${title}, ${state})`); const prList = await platform.getPrList(); const pr = prList.find( @@ -489,7 +497,7 @@ const platform: Platform = { head, title, body, - labels: labels.filter(Boolean), + labels: labels.filter(is.number), }); const pr = toRenovatePR(gpr); @@ -582,7 +590,7 @@ const platform: Platform = { return config.issueList; }, - async getIssue(number: number, useCache = true): Promise<Issue> { + async getIssue(number: number, useCache = true): Promise<Issue | null> { try { const body = ( await helper.getIssue(config.repository, number, { @@ -599,7 +607,7 @@ const platform: Platform = { } }, - async findIssue(title: string): Promise<Issue> { + async findIssue(title: string): Promise<Issue | null> { const issueList = await platform.getIssueList(); const issue = issueList.find( (i) => i.state === 'open' && i.title === title @@ -609,7 +617,8 @@ const platform: Platform = { return null; } logger.debug(`Found Issue #${issue.number}`); - return platform.getIssue(issue.number); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + return getIssue!(issue.number!); }, async ensureIssue({ @@ -631,7 +640,9 @@ const platform: Platform = { } const labels = Array.isArray(labelNames) - ? await Promise.all(labelNames.map(lookupLabelByName)) + ? (await Promise.all(labelNames.map(lookupLabelByName))).filter( + is.number + ) : undefined; // Update any matching issues which currently exist @@ -656,7 +667,8 @@ const platform: Platform = { for (const issue of issues) { if (issue.state === 'open' && issue.number !== activeIssue.number) { logger.warn(`Closing duplicate Issue #${issue.number}`); - await helper.closeIssue(config.repository, issue.number); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + await helper.closeIssue(config.repository, issue.number!); } } @@ -676,7 +688,8 @@ const platform: Platform = { logger.debug(`Updating Issue #${activeIssue.number}`); const existingIssue = await helper.updateIssue( config.repository, - activeIssue.number, + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + activeIssue.number!, { body, title, @@ -698,7 +711,8 @@ const platform: Platform = { ) { await helper.updateIssueLabels( config.repository, - activeIssue.number, + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + activeIssue.number!, { labels, } @@ -731,7 +745,8 @@ const platform: Platform = { for (const issue of issueList) { if (issue.state === 'open' && issue.title === title) { logger.debug({ number: issue.number }, 'Closing issue'); - await helper.closeIssue(config.repository, issue.number); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion + await helper.closeIssue(config.repository, issue.number!); } } }, @@ -744,8 +759,6 @@ const platform: Platform = { } else { logger.warn({ issue, labelName }, 'Failed to lookup label for deletion'); } - - return null; }, getRepoForceRebase(): Promise<boolean> { diff --git a/lib/modules/platform/types.ts b/lib/modules/platform/types.ts index fadd50ec05..b592e81c4f 100644 --- a/lib/modules/platform/types.ts +++ b/lib/modules/platform/types.ts @@ -150,7 +150,7 @@ export type EnsureIssueResult = 'updated' | 'created'; export interface Platform { findIssue(title: string): Promise<Issue | null>; getIssueList(): Promise<Issue[]>; - getIssue?(number: number, useCache?: boolean): Promise<Issue>; + getIssue?(number: number, useCache?: boolean): Promise<Issue | null>; getVulnerabilityAlerts(): Promise<VulnerabilityAlert[]>; getRawFile( fileName: string, @@ -188,8 +188,8 @@ export interface Platform { | EnsureCommentRemovalConfigByContent ): Promise<void>; ensureComment(ensureComment: EnsureCommentConfig): Promise<boolean>; - getPr(number: number): Promise<Pr>; - findPr(findPRConfig: FindPRConfig): Promise<Pr>; + getPr(number: number): Promise<Pr | null>; + findPr(findPRConfig: FindPRConfig): Promise<Pr | null>; refreshPr?(number: number): Promise<void>; getBranchStatus(branchName: string): Promise<BranchStatus>; getBranchPr(branchName: string): Promise<Pr | null>; diff --git a/tsconfig.strict.json b/tsconfig.strict.json index d5cb4e1876..2131bb213e 100644 --- a/tsconfig.strict.json +++ b/tsconfig.strict.json @@ -39,11 +39,8 @@ "lib/config/validation.ts", "lib/modules/datasource/github-releases/test/index.ts", "lib/modules/platform/api.ts", - "lib/modules/platform/bitbucket-server/index.ts", - "lib/modules/platform/bitbucket-server/utils.ts", "lib/modules/platform/comment.ts", "lib/modules/platform/commit.ts", - "lib/modules/platform/gitea/index.ts", "lib/modules/platform/github/index.ts", "lib/modules/platform/gitlab/index.ts", "lib/modules/platform/index.ts", -- GitLab