Skip to content

Commit

Permalink
chore(null): add strict null checks for first layer of src/common/ (#…
Browse files Browse the repository at this point in the history
…2990)

#### Description of changes

This PR adds all of the files matched by `yarn null:find` of form `/src/common/**/*.ts` to the strict null checks list.

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox -->
- [x] Addresses an existing issue: part of #2869
- [x] Ran `yarn fastpass`
- [n/a] Added/updated relevant unit test(s) (and ran `yarn test`)
- [n/a] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage`
- [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`.
- [n/a] (UI changes only) Added screenshots/GIFs to description above
- [n/a] (UI changes only) Verified usability with NVDA/JAWS
  • Loading branch information
dbjorge authored Jun 30, 2020
1 parent 401f462 commit e9c0126
Show file tree
Hide file tree
Showing 15 changed files with 59 additions and 46 deletions.
4 changes: 2 additions & 2 deletions src/DetailsView/details-view-initializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,13 @@ const browserAdapterFactory = new BrowserAdapterFactory(userAgentParser);
const browserAdapter = browserAdapterFactory.makeFromUserAgent();

const urlParser = new UrlParser();
const tabId = urlParser.getIntParam(window.location.href, 'tabId');
const tabId: number | null = urlParser.getIntParam(window.location.href, 'tabId');
const dom = document;
const documentElementSetter = new DocumentManipulator(dom);

initializeFabricIcons();

if (isNaN(tabId) === false) {
if (tabId != null) {
browserAdapter.getTab(
tabId,
(tab: Tab): void => {
Expand Down
2 changes: 1 addition & 1 deletion src/common/axe-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ export class AxeInfo {
constructor(private axe: typeof Axe) {}

public get version(): string {
return this.axe.version;
return (this.axe as any).version;
}
}
2 changes: 1 addition & 1 deletion src/common/browser-adapters/browser-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export interface BrowserAdapter {
insertCSSInTab(tabId: number, details: ExtensionTypes.InjectDetails): Promise<void>;
getRunTimeId(): string;
createNotification(options: Notifications.CreateNotificationOptions): Promise<string>;
getRuntimeLastError(): chrome.runtime.LastError;
getRuntimeLastError(): chrome.runtime.LastError | undefined;
isAllowedFileSchemeAccess(): Promise<boolean>;
getManageExtensionUrl(): string;
addListenerOnConnect(callback: (port: chrome.runtime.Port) => void): void;
Expand Down
8 changes: 6 additions & 2 deletions src/common/browser-adapters/firefox-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ export class FirefoxAdapter extends WebExtensionBrowserAdapter {
}

// Firefox will fail window creation requests with the "focused" property
public createTabInNewWindow(url: string): Promise<Tabs.Tab> {
return browser.windows.create({ url }).then(window => window.tabs[0]);
public async createTabInNewWindow(url: string): Promise<Tabs.Tab> {
const newWindow = await browser.windows.create({ url });
if (newWindow.tabs == null) {
throw new Error('Browser created a window with no tabs');
}
return newWindow.tabs[0];
}
}
17 changes: 13 additions & 4 deletions src/common/browser-adapters/webextension-browser-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ export abstract class WebExtensionBrowserAdapter
if (tab) {
onResolve(tab);
} else {
onReject();
if (onReject != null) {
onReject();
}
}
});
}
Expand All @@ -90,8 +92,12 @@ export abstract class WebExtensionBrowserAdapter
return browser.tabs.create({ url, active: true, pinned: false });
}

public createTabInNewWindow(url: string): Promise<Tabs.Tab> {
return browser.windows.create({ url, focused: true }).then(window => window.tabs[0]);
public async createTabInNewWindow(url: string): Promise<Tabs.Tab> {
const newWindow = await browser.windows.create({ url, focused: true });
if (newWindow.tabs == null) {
throw new Error('Browser created a window with no tabs');
}
return newWindow.tabs[0];
}

public updateTab(
Expand All @@ -110,6 +116,9 @@ export abstract class WebExtensionBrowserAdapter

public async switchToTab(tabId: number): Promise<void> {
const tab = await this.updateTab(tabId, { active: true });
if (tab.windowId == null) {
throw new Error('Browser indicated an orphan tab with no windowId');
}
await this.updateWindow(tab.windowId, { focused: true });
}

Expand All @@ -133,7 +142,7 @@ export abstract class WebExtensionBrowserAdapter
return browser.storage.local.remove(key);
}

public getRuntimeLastError(): chrome.runtime.LastError {
public getRuntimeLastError(): chrome.runtime.LastError | undefined {
return chrome.runtime.lastError;
}

Expand Down
2 changes: 1 addition & 1 deletion src/common/flux/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export class Action<TPayload> {
private listeners: ((payload: TPayload) => void)[] = [];
private scope: string = 'DEFAULT_SCOPE';

public invoke(payload: TPayload, scope: string = null): void {
public invoke(payload: TPayload, scope?: string): void {
const activeScope = scope ?? this.scope;
if (Action.executingScopes[activeScope]) {
throw new Error(
Expand Down
13 changes: 8 additions & 5 deletions src/common/html-element-utils.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
export class HTMLElementUtils {
constructor(private dom?: Document, private clientWindow?: Window) {
private readonly dom: Document;
private readonly clientWindow: Window;

constructor(dom?: Document, clientWindow?: Window) {
this.dom = dom || document;
this.clientWindow = clientWindow || window;
}

public getContentWindow(frame: HTMLIFrameElement): Window {
public getContentWindow(frame: HTMLIFrameElement): Window | null {
return frame.contentWindow;
}

Expand All @@ -22,7 +25,7 @@ export class HTMLElementUtils {
return this.dom.body;
}

public querySelector(selector: string): Element {
public querySelector(selector: string): Element | null {
return this.dom.querySelector(selector);
}

Expand All @@ -38,7 +41,7 @@ export class HTMLElementUtils {
return element.tagName.toLowerCase();
}

public getCurrentFocusedElement(): Element {
public getCurrentFocusedElement(): Element | null {
return this.dom.activeElement;
}

Expand All @@ -50,7 +53,7 @@ export class HTMLElementUtils {
return this.clientWindow.getComputedStyle(element);
}

public getClientRects(element: Element): ClientRectList {
public getClientRects(element: Element): DOMRectList {
return element.getClientRects();
}

Expand Down
2 changes: 1 addition & 1 deletion src/common/navigator-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export class NavigatorUtils {
return userAgent;
}

private getVersion(userAgent: string, versionPrefix: string): string {
private getVersion(userAgent: string, versionPrefix: string): string | null {
const versionOffset = userAgent.indexOf(versionPrefix);
if (versionOffset === -1) {
return null;
Expand Down
10 changes: 5 additions & 5 deletions src/common/tabbable-elements-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { HTMLElementUtils } from './html-element-utils';
export class TabbableElementsHelper {
constructor(private htmlElementUtils: HTMLElementUtils) {}

public getCurrentFocusedElement(): Element {
public getCurrentFocusedElement(): Element | null {
return this.htmlElementUtils.getCurrentFocusedElement();
}

Expand All @@ -17,13 +17,13 @@ export class TabbableElementsHelper {
const result =
style.visibility !== 'hidden' &&
style.display !== 'none' &&
offsetHeight &&
offsetWidth &&
offsetHeight !== 0 &&
offsetWidth !== 0 &&
clientRects.length > 0;
return result;
};

public getAncestorMap(element: HTMLElement): HTMLMapElement {
public getAncestorMap(element: HTMLElement): HTMLMapElement | null {
if (!element.parentElement || element.parentNode instanceof Document) {
return null;
}
Expand All @@ -39,7 +39,7 @@ export class TabbableElementsHelper {
return this.getAncestorMap(parent);
}

public getMappedImage(map: HTMLMapElement): HTMLImageElement {
public getMappedImage(map: HTMLMapElement): HTMLImageElement | null {
const mapName: string = map.name;

if (!mapName) {
Expand Down
3 changes: 2 additions & 1 deletion src/common/types/property-bag/column-value-bag.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
export type BagOf<T> = { [K in string]: T };
export type ScalarColumnValue = string | number | Date | boolean;
export type ScalarColumnValue = string | number | Date | boolean | undefined;
export type ColumnValue = ScalarColumnValue | BagOf<ScalarColumnValue>;
export type ColumnValueBag = BagOf<ColumnValue>;

export function isScalarColumnValue(value): value is ScalarColumnValue {
return (
value === undefined ||
typeof value === 'string' ||
typeof value === 'boolean' ||
typeof value === 'number' ||
Expand Down
8 changes: 6 additions & 2 deletions src/common/url-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@
// Licensed under the MIT License.

export class UrlParser {
public getIntParam(urlString: string, key: string): number {
public getIntParam(urlString: string, key: string): number | null {
const url = new URL(urlString);
return parseInt(url.searchParams.get(key), 10);
const rawParamValue = url.searchParams.get(key);
if (rawParamValue == null) {
return null;
}
return parseInt(rawParamValue, 10);
}

public areURLsEqual(urlA: string, urlB: string): boolean {
Expand Down
2 changes: 1 addition & 1 deletion src/popup/target-tab-finder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class TargetTabFinder {
private getTabInfo = (): Promise<Tab> => {
const tabIdInUrl = this.urlParser.getIntParam(this.win.location.href, 'tabId');

if (isNaN(tabIdInUrl)) {
if (tabIdInUrl == null) {
return this.browserAdapter
.tabsQuery({ active: true, currentWindow: true })
.then(tabs => tabs.pop());
Expand Down
2 changes: 1 addition & 1 deletion src/tests/unit/tests/common/url-parser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe('UrlParserTest', () => {
];

test.each(invalidCases)('return invalid value', (testCase: ValidCase) => {
expect(testSubject.getIntParam(testCase.url, testCase.key)).toBeNaN();
expect(testSubject.getIntParam(testCase.url, testCase.key)).toBeNull();
});
});

Expand Down
4 changes: 2 additions & 2 deletions src/tests/unit/tests/popup/target-tab-finder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ describe('TargetTabFinderTest', () => {
setupGetTabIdParamFromUrl(tabId);
setupGetTabCall();
} else {
setupGetTabIdParamFromUrl(NaN);
setupGetTabIdParamFromUrl(null);
setupTabQueryCall();
}

Expand All @@ -97,7 +97,7 @@ describe('TargetTabFinderTest', () => {
.catch(error => expect(error).toEqual(`Tab with Id ${tabId} not found`));
});

function setupGetTabIdParamFromUrl(tabIdValue: number): void {
function setupGetTabIdParamFromUrl(tabIdValue: number | null): void {
urlParserMock
.setup(p => p.getIntParam(windowStub.location.href, 'tabId'))
.returns(() => tabIdValue);
Expand Down
26 changes: 9 additions & 17 deletions tsconfig.strictNullChecks.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,9 @@
"./src/background/telemetry/telemetry-client.ts",
"./src/background/usage-logger.ts",
"./src/background/user-stored-data-cleaner.ts",
"./src/common/axe-info.ts",
"./src/common/base-store.ts",
"./src/common/blob-provider.ts",
"./src/common/browser-adapters/app-data-adapter.ts",
"./src/common/browser-adapters/browser-adapter.ts",
"./src/common/browser-adapters/commands-adapter.ts",
"./src/common/browser-adapters/storage-adapter.ts",
"./src/common/components/blocking-dialog.tsx",
"./src/common/components/cards/card-interaction-support.ts",
"./src/common/components/cards/simple-card-row.tsx",
Expand All @@ -100,33 +97,24 @@
"./src/common/extension-telemetry-events.ts",
"./src/common/fabric-icons.ts",
"./src/common/file-url-provider.ts",
"./src/common/flux/event-handler-list.ts",
"./src/common/get-inner-text-from-jsx-element.ts",
"./src/common/html-element-utils.ts",
"./src/common/is-supported-browser.ts",
"./src/common/itab.ts",
"./src/common/message-creators/store-action-message-creator.ts",
"./src/common/message.ts",
"./src/common/messages.ts",
"./src/common/navigator-utils.ts",
"./src/common/platform.ts",
"./src/common/stores/client-stores-hub.ts",
"./src/common/stores/store-names.ts",
"./src/common/tabbable-elements-helper.ts",
"./src/common/types/deep-partial.ts",
"./src/common/types/details-view-pivot-type.ts",
"./src/common/types/dev-tools-open-message.ts",
"./src/common/types/idefault-constructor.ts",
"./src/common/types/link-component-type.ts",
"./src/common/types/manual-test-status.ts",
"./src/common/types/property-bag/column-value-bag.ts",
"./src/common/types/property-bag/contrast.ts",
"./src/common/types/property-bag/cues.ts",
"./src/common/types/property-bag/custom-widgets-property-bag.ts",
"./src/common/types/property-bag/default-widget.ts",
"./src/common/types/property-bag/image-function.ts",
"./src/common/types/property-bag/link-function-property-bag.ts",
"./src/common/types/property-bag/link-purpose-property-bag.ts",
"./src/common/types/property-bag/meaningful-image.ts",
"./src/common/types/property-bag/text-alternative-property-bag.ts",
"./src/common/types/property-bag/widget-function-property-bag.ts",
"./src/common/types/scan-incomplete-warnings.ts",
"./src/common/types/store-data/assessment-result-data.ts",
"./src/common/types/store-data/card-selection-store-data.ts",
Expand All @@ -147,6 +135,7 @@
"./src/common/types/store-update-message.ts",
"./src/common/types/visualization-type.ts",
"./src/common/uid-generator.ts",
"./src/common/url-parser.ts",
"./src/common/url-validator.ts",
"./src/common/window-utils.ts",
"./src/content/guidance-tags.ts",
Expand Down Expand Up @@ -269,13 +258,16 @@
],
"include": [
"src/common/action/**/*",
"src/common/icons/**/*",
"src/common/browser-adapters/**/*",
"src/common/constants/**/*",
"src/common/flux/**/*",
"src/common/icons/**/*",
"src/common/indexedDB/**/*",
"src/common/logging/**/*",
"src/common/promises/**/*",
"src/common/react/**/*",
"src/common/styles/**/*",
"src/common/types/property-bag/**/*",
"src/content/settings/**/*",
"src/content/strings/**/*",
"src/debug-tools/controllers/**/*",
Expand Down

0 comments on commit e9c0126

Please sign in to comment.