Skip to content

Commit 5d6b81a

Browse files
rebornixrebornix
committed
strict null checks
1 parent a9a8d55 commit 5d6b81a

38 files changed

+522
-348
lines changed

‎src/authentication/githubServer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export class GitHubManager {
4444
}
4545

4646
if (this._servers.has(host.authority)) {
47-
return this._servers.get(host.authority);
47+
return !!this._servers.get(host.authority);
4848
}
4949

5050
const keychainHosts = await listHosts();

‎src/authentication/keychain.ts

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ export type GlobalStateContext = { globalState: vscode.Memento };
3939
const SERVICE_ID = 'vscode-pull-request-github';
4040
export const ALL_HOSTS_KEY = 'keychain::all';
4141

42-
let defaultStorage: vscode.Memento = null;
43-
let defaultKeychain: Keytar = null;
42+
let defaultStorage: vscode.Memento | null = null;
43+
let defaultKeychain: Keytar | null = null;
4444

4545
const didChange = new vscode.EventEmitter<IHostConfiguration>();
4646
export const onDidChange = didChange.event;
@@ -50,10 +50,10 @@ export function init(ctx: GlobalStateContext, keychain: Keytar = systemKeychain)
5050
defaultKeychain = keychain;
5151
}
5252

53-
export async function getToken(host: string, { storage = defaultStorage, keychain = defaultKeychain } = {}): Promise<string> {
53+
export async function getToken(host: string, { storage = defaultStorage, keychain = defaultKeychain } = {}): Promise<string | null | undefined> {
5454
host = toCanonical(host);
55-
const token = keychain.getPassword(SERVICE_ID, toCanonical(host))
56-
.catch(() => storage.get(keyFor(host)));
55+
const token = keychain!.getPassword(SERVICE_ID, toCanonical(host))
56+
.catch(() => storage!.get(keyFor(host)));
5757

5858
// While we're transitioning everything out of configuration and into local storage, it's possible
5959
// that we end up in a state where a host is not in the hosts list (perhaps because it was removed
@@ -68,20 +68,20 @@ export async function getToken(host: string, { storage = defaultStorage, keychai
6868
export async function setToken(host: string, token: string, { storage = defaultStorage, keychain = defaultKeychain, emit = true } = {}) {
6969
if (!token) { return deleteToken(host, { storage, keychain, emit }); }
7070
host = toCanonical(host);
71-
await keychain.setPassword(SERVICE_ID, host, token)
72-
.catch(() => storage.update(keyFor(host), token));
71+
await keychain!.setPassword(SERVICE_ID, host, token)
72+
.catch(() => storage!.update(keyFor(host), token));
7373
await addHost(host, { storage });
7474
if (emit) { didChange.fire({ host, token }); }
7575
}
7676

7777
export async function deleteToken(host: string, { storage = defaultStorage, keychain = defaultKeychain, emit = true } = {}) {
7878
host = toCanonical(host);
79-
await keychain.deletePassword(SERVICE_ID, host)
80-
.catch(() => storage.update(keyFor(host), undefined));
81-
const hosts = storage.get<{ [key: string]: string }>(ALL_HOSTS_KEY, {});
79+
await keychain!.deletePassword(SERVICE_ID, host)
80+
.catch(() => storage!.update(keyFor(host), undefined));
81+
const hosts = storage!.get<{ [key: string]: string }>(ALL_HOSTS_KEY, {});
8282
delete hosts[host];
83-
storage.update(ALL_HOSTS_KEY, hosts);
84-
if (emit) { didChange.fire({ host, token: null }); }
83+
storage!.update(ALL_HOSTS_KEY, hosts);
84+
if (emit) { didChange.fire({ host, token: undefined }); }
8585
}
8686

8787
export async function migrateToken(host: string, token: string, { storage = defaultStorage, keychain = defaultKeychain, emit = false } = {}) {
@@ -94,17 +94,16 @@ export async function migrateToken(host: string, token: string, { storage = defa
9494
}
9595

9696
export async function listHosts({ storage = defaultStorage } = {}) {
97-
return Object.keys(storage.get(ALL_HOSTS_KEY) || {});
97+
return Object.keys(storage!.get(ALL_HOSTS_KEY) || {});
9898
}
9999

100100
async function addHost(host: string, { storage = defaultStorage }) {
101-
return storage.update(ALL_HOSTS_KEY, { ...storage.get(ALL_HOSTS_KEY), [host]: true });
101+
return storage!.update(ALL_HOSTS_KEY, { ...storage!.get(ALL_HOSTS_KEY), [host]: true });
102102
}
103103

104104
const SCHEME_RE = /^[a-z-]+:\/?\/?/;
105105
export function toCanonical(host: string): string {
106106
host = host.toLocaleLowerCase();
107-
if (!host) { return; }
108107
if (host.endsWith('/')) {
109108
host = host.slice(0, -1);
110109
}

‎src/commands.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,15 @@ import { Comment } from './common/comment';
2525
import { PullRequestManager } from './github/pullRequestManager';
2626
import { PullRequestModel } from './github/pullRequestModel';
2727

28-
const _onDidUpdatePR = new vscode.EventEmitter<PullRequest>();
29-
export const onDidUpdatePR: vscode.Event<PullRequest> = _onDidUpdatePR.event;
28+
const _onDidUpdatePR = new vscode.EventEmitter<PullRequest | null>();
29+
export const onDidUpdatePR: vscode.Event<PullRequest | null> = _onDidUpdatePR.event;
3030

3131
function ensurePR(prManager: PullRequestManager, pr?: PRNode | PullRequestModel): PullRequestModel {
3232
// If the command is called from the command palette, no arguments are passed.
3333
if (!pr) {
3434
if (!prManager.activePullRequest) {
3535
vscode.window.showErrorMessage('Unable to find current pull request.');
36-
return;
36+
throw new Error('Unable to find current pull request.');
3737
}
3838

3939
return prManager.activePullRequest;
@@ -65,6 +65,10 @@ export function registerCommands(context: vscode.ExtensionContext, prManager: Pu
6565

6666
context.subscriptions.push(vscode.commands.registerCommand('review.suggestDiff', async (e) => {
6767
try {
68+
if (prManager.activePullRequest === null) {
69+
return;
70+
}
71+
6872
const { indexChanges, workingTreeChanges } = prManager.repository.state;
6973

7074
if (!indexChanges.length) {
@@ -95,6 +99,10 @@ export function registerCommands(context: vscode.ExtensionContext, prManager: Pu
9599
// Reset HEAD and then apply reverse diff
96100
await vscode.commands.executeCommand('git.unstageAll');
97101

102+
if (vscode.workspace.rootPath === undefined) {
103+
throw new Error('Current workspace root path is undefined.');
104+
}
105+
98106
const tempFilePath = pathLib.resolve(vscode.workspace.rootPath, '.git', `${prManager.activePullRequest.prNumber}.diff`);
99107
writeFile(tempFilePath, diff, {}, async (writeError) => {
100108
if (writeError) {
@@ -121,7 +129,9 @@ export function registerCommands(context: vscode.ExtensionContext, prManager: Pu
121129
}));
122130

123131
context.subscriptions.push(vscode.commands.registerCommand('pr.openFileInGitHub', (e: GitFileChangeNode) => {
124-
vscode.commands.executeCommand('vscode.open', vscode.Uri.parse(e.blobUrl));
132+
if (e.blobUrl) {
133+
vscode.commands.executeCommand('vscode.open', vscode.Uri.parse(e.blobUrl));
134+
}
125135
}));
126136

127137
context.subscriptions.push(vscode.commands.registerCommand('pr.openDiffView', (fileChangeNode: GitFileChangeNode | InMemFileChangeNode) => {
@@ -216,7 +226,7 @@ export function registerCommands(context: vscode.ExtensionContext, prManager: Pu
216226
return vscode.window.showWarningMessage(`Are you sure you want to close this pull request on GitHub? This will close the pull request without merging.`, 'Yes', 'No').then(async value => {
217227
if (value === 'Yes') {
218228
try {
219-
let newComment: Comment;
229+
let newComment: Comment | null = null;
220230
if (message) {
221231
newComment = await prManager.createIssueComment(pullRequest, message);
222232
}
@@ -295,12 +305,12 @@ export function registerCommands(context: vscode.ExtensionContext, prManager: Pu
295305
};
296306

297307
if (fileChange.comments && fileChange.comments.length) {
298-
const sortedOutdatedComments = fileChange.comments.filter(comment => comment.position === null).sort((a, b) => {
299-
return a.originalPosition - b.originalPosition;
308+
const sortedOutdatedComments = fileChange.comments.filter(comment => comment.position === undefined).sort((a, b) => {
309+
return a.originalPosition! - b.originalPosition!;
300310
});
301311

302312
if (sortedOutdatedComments.length) {
303-
const diffLine = getDiffLineByPosition(fileChange.diffHunks, sortedOutdatedComments[0].originalPosition);
313+
const diffLine = getDiffLineByPosition(fileChange.diffHunks, sortedOutdatedComments[0].originalPosition!);
304314

305315
if (diffLine) {
306316
let lineNumber = Math.max(getZeroBased(diffLine.type === DiffChangeType.Delete ? diffLine.oldLineNumber : diffLine.newLineNumber), 0);
@@ -309,7 +319,7 @@ export function registerCommands(context: vscode.ExtensionContext, prManager: Pu
309319
}
310320
}
311321

312-
return vscode.commands.executeCommand('vscode.diff', previousFileUri, fileChange.filePath, `${fileChange.fileName} from ${commit.substr(0, 8)}`, options);
322+
return vscode.commands.executeCommand('vscode.diff', previousFileUri, fileChange.filePath, `${fileChange.fileName} from ${(commit || '').substr(0, 8)}`, options);
313323
}));
314324

315325
context.subscriptions.push(vscode.commands.registerCommand('pr.signin', async () => {

‎src/common/comment.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@ export interface Comment {
1616
id: number;
1717
pullRequestReviewId?: number;
1818
diffHunk: string;
19-
path: string;
20-
position: number;
19+
path?: string;
20+
position?: number;
2121
commitId?: string;
22-
originalPosition: number;
23-
originalCommitId: string;
22+
originalPosition?: number;
23+
originalCommitId?: string;
2424
user: IAccount;
2525
body: string;
2626
createdAt: string;

‎src/common/diffHunk.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ export function* parseDiffHunk(diffHunkPatch: string): IterableIterator<DiffHunk
9797
let lineReader = LineReader(diffHunkPatch);
9898

9999
let itr = lineReader.next();
100-
let diffHunk: DiffHunk = null;
100+
let diffHunk: DiffHunk | null = null;
101101
let positionInHunk = -1;
102102
let oldLine = -1;
103103
let newLine = -1;

‎src/common/diffPositionMapping.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export function getAbsolutePosition(comment: Comment, fileDiffHunks: DiffHunk[],
3434
let commentAbsolutePosition = -1;
3535
// Ignore outdated comments
3636
if (comment.position !== null) {
37-
let diffLine = getDiffLineByPosition(fileDiffHunks, comment.position);
37+
let diffLine = getDiffLineByPosition(fileDiffHunks, comment.position!);
3838

3939
if (diffLine) {
4040
if (isBase) {
@@ -60,19 +60,19 @@ export function getPositionInDiff(comment: Comment, fileDiffHunks: DiffHunk[], i
6060
let commentAbsolutePosition = -1;
6161
// Ignore outdated comments
6262
if (comment.position !== null) {
63-
let diffLine = getDiffLineByPosition(fileDiffHunks, comment.position);
63+
let diffLine = getDiffLineByPosition(fileDiffHunks, comment.position!);
6464

6565
if (diffLine) {
6666
if ((diffLine.type === DiffChangeType.Add && !isBase) || (diffLine.type === DiffChangeType.Delete && isBase)) {
67-
commentAbsolutePosition = comment.position;
67+
commentAbsolutePosition = comment.position!;
6868
}
6969
}
7070
}
7171

7272
return commentAbsolutePosition;
7373
}
7474

75-
export function getLastDiffLine(prPatch: string): DiffLine {
75+
export function getLastDiffLine(prPatch: string): DiffLine | null {
7676
let lastDiffLine = null;
7777
let prDiffReader = parseDiffHunk(prPatch);
7878
let prDiffIter = prDiffReader.next();
@@ -87,7 +87,7 @@ export function getLastDiffLine(prPatch: string): DiffLine {
8787
return lastDiffLine;
8888
}
8989

90-
export function getDiffLineByPosition(diffHunks: DiffHunk[], diffLineNumber: number): DiffLine {
90+
export function getDiffLineByPosition(diffHunks: DiffHunk[], diffLineNumber: number): DiffLine | null {
9191
for (let i = 0; i < diffHunks.length; i++) {
9292
let diffHunk = diffHunks[i];
9393
for (let j = 0; j < diffHunk.diffLines.length; j++) {
@@ -165,7 +165,7 @@ export function mapCommentsToHead(diffHunks: DiffHunk[], localDiff: string, comm
165165
const comment = comments[i];
166166

167167
// Diff line is null when the original line the comment was on has been removed
168-
const diffLine = getDiffLineByPosition(diffHunks, comment.position || comment.originalPosition);
168+
const diffLine = getDiffLineByPosition(diffHunks, comment.position || comment.originalPosition!);
169169
if (diffLine) {
170170
const positionInPr = diffLine.type === DiffChangeType.Delete ? diffLine.oldLineNumber : diffLine.newLineNumber;
171171
const newPosition = mapOldPositionToNew(localDiff, positionInPr);

‎src/common/net.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export const agent = getAgent();
1212
* @param {string} url the proxy URL, (default: `process.env.HTTPS_PROXY`)
1313
* @returns {https.Agent}
1414
*/
15-
function getAgent(url: string = process.env.HTTPS_PROXY): Agent {
15+
function getAgent(url: string | undefined = process.env.HTTPS_PROXY): Agent {
1616
if (!url) { return globalAgent; }
1717
try {
1818
const { hostname, port, username, password } = new URL(url);

‎src/common/octokit.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import * as REST from '@octokit/rest';
2+
3+
type Omit<T, K extends keyof T> = Pick<T, Exclude<keyof T, K>>;
4+
5+
declare namespace Github {
6+
export type PullRequestsGetResponse = Omit<REST.PullRequestsGetResponse, 'milestone'> & {
7+
milestone: null | REST.PullRequestsGetResponseMilestone
8+
};
9+
export type PullRequestsGetAllResponseItem = Omit<REST.PullRequestsGetAllResponseItem, 'milestone' | 'closed_at' | 'merged_at' | '_links'> & {
10+
milestone: null | REST.PullRequestsGetResponseMilestone
11+
};
12+
13+
export type PullRequestsGetAllResponseItemUser = REST.PullRequestsGetAllResponseItemUser;
14+
export type PullRequestsGetResponseHead = REST.PullRequestsGetResponseHead;
15+
export type PullRequestsCreateResponse = REST.PullRequestsCreateResponse;
16+
export type IssuesCreateCommentResponse = REST.IssuesCreateCommentResponse;
17+
export type IssuesEditCommentResponse = REST.IssuesEditCommentResponse;
18+
export type PullRequestsGetCommentsResponseItem = REST.PullRequestsGetCommentsResponseItem;
19+
export type PullRequestsEditCommentResponse = REST.PullRequestsEditCommentResponse;
20+
export type PullRequestsGetResponseHeadRepo = REST.PullRequestsGetResponseHeadRepo;
21+
}
22+
// Octokit.PullRequestsGetResponse | Octokit.PullRequestsGetAllResponseItem
23+
24+
export = Github;

‎src/common/protocol.ts

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ export class Protocol {
4242

4343
this.host = this.getHostName(this.url.authority);
4444
if (this.host) {
45-
this.repositoryName = this.getRepositoryName(this.url.path);
46-
this.owner = this.getOwnerName(this.url.path);
45+
this.repositoryName = this.getRepositoryName(this.url.path) || '';
46+
this.owner = this.getOwnerName(this.url.path) || '';
4747
}
4848
} catch (e) {
4949
Logger.appendLine(`Failed to parse '${uriString}'`);
@@ -72,8 +72,8 @@ export class Protocol {
7272
if (!sshConfig) { return false; }
7373
const { HostName, path } = sshConfig;
7474
this.host = HostName;
75-
this.owner = this.getOwnerName(path);
76-
this.repositoryName = this.getRepositoryName(path);
75+
this.owner = this.getOwnerName(path) || '';
76+
this.repositoryName = this.getRepositoryName(path) || '';
7777
this.type = ProtocolType.SSH;
7878
return true;
7979
}
@@ -117,7 +117,7 @@ export class Protocol {
117117
return null;
118118
}
119119

120-
normalizeUri(): vscode.Uri {
120+
normalizeUri(): vscode.Uri | null {
121121
if (this.type === ProtocolType.OTHER && !this.url) {
122122
return null;
123123
}
@@ -138,7 +138,7 @@ export class Protocol {
138138
}
139139
}
140140

141-
toString(): string {
141+
toString(): string | null {
142142
// based on Uri scheme for SSH https://tools.ietf.org/id/draft-salowey-secsh-uri-00.html#anchor1 and heuristics of how GitHub handles ssh url
143143
// sshUri = `ssh:`
144144
// - omitted
@@ -187,6 +187,16 @@ export class Protocol {
187187
}
188188

189189
equals(other: Protocol) {
190-
return this.normalizeUri().toString().toLocaleLowerCase() === other.normalizeUri().toString().toLocaleLowerCase();
190+
let normalizeUri = this.normalizeUri();
191+
if (!normalizeUri) {
192+
return false;
193+
}
194+
195+
let otherNormalizeUri = other.normalizeUri();
196+
if (!otherNormalizeUri) {
197+
return false;
198+
}
199+
200+
return normalizeUri.toString().toLocaleLowerCase() === otherNormalizeUri.toString().toLocaleLowerCase();
191201
}
192202
}

‎src/common/remote.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export class Remote {
1919

2020
public get normalizedHost(): string {
2121
const normalizedUri = this.gitProtocol.normalizeUri();
22-
return `${normalizedUri.scheme}://${normalizedUri.authority}`;
22+
return `${normalizedUri!.scheme}://${normalizedUri!.authority}`;
2323
}
2424

2525
constructor(
@@ -46,7 +46,10 @@ export class Remote {
4646
}
4747
}
4848

49-
export function parseRemote(remoteName: string, url: string, originalProtocol?: Protocol): Remote | null {
49+
export function parseRemote(remoteName: string, url: string | undefined, originalProtocol?: Protocol): Remote | null {
50+
if (!url) {
51+
return null;
52+
}
5053
let gitProtocol = new Protocol(url);
5154
if (originalProtocol) {
5255
gitProtocol.update({
@@ -64,5 +67,5 @@ export function parseRemote(remoteName: string, url: string, originalProtocol?:
6467
export function parseRepositoryRemotes(repository: Repository): Remote[] {
6568
return repository.state.remotes
6669
.map(r => parseRemote(r.name, r.fetchUrl || r.pushUrl))
67-
.filter(r => !!r);
70+
.filter(r => !!r) as Remote[];
6871
}

0 commit comments

Comments
 (0)