Skip to content

Commit

Permalink
chore(null): strict null checks for injected drawing-controller (#6272)
Browse files Browse the repository at this point in the history
#### Details

This PR adds the injected DrawingController + a few related/dependent
files to the strict null check list.

The most interesting of the changes was updating the `DrawerInitData`
type to not be generic. Previously, it was genericized over the data
type an individual drawer expected (in particular, svg-drawer expects to
only be used with something special). But the generic type usage meant
that in practice, the individual drawers were written looking like they
could safely assume what type of input they were given with the relevant
`any` conversions happening in the drawer controller/initiator, which
wasn't really accurate - if a type assumption was wrong, it'd be the
individual drawer that would have to be aware of the mistake. This
change moves the type assertions in question to the drawer.

Overall, these changes enabled autoadding several more assessment
folders and brought us from 78% to 80% null checked:

```
❯ yarn null:progress
yarn run v1.22.19
$ node ./tools/strict-null-checks/progress.js
## Web strict-null progress

**80%** complete (**1244**/1562 non-test files)

*Contribute at [#2869](#2869). Last update: Fri Dec 16 2022*
```

##### Motivation

#2869

##### Context

n/a

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a"
in the checkbox -->
- [x] Addresses an existing issue: #2869
- [x] Ran `yarn null:autoadd`
- [x] Ran `yarn fastpass`
- [x] Added/updated relevant unit test(s) (and ran `yarn test`)
- [x] 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 Dec 17, 2022
1 parent ba04582 commit 2967c3f
Show file tree
Hide file tree
Showing 16 changed files with 91 additions and 87 deletions.
2 changes: 1 addition & 1 deletion src/DetailsView/components/base-visual-helper-toggle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export abstract class BaseVisualHelperToggle extends React.Component<VisualHelpe
});
}

private renderNoMatchingElementsMessage(isDisabled: boolean): JSX.Element {
private renderNoMatchingElementsMessage(isDisabled: boolean): JSX.Element | null {
if (isDisabled) {
return (
<span className="no-matching-elements">
Expand Down
20 changes: 10 additions & 10 deletions src/injected/drawing-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ export interface VisualizationWindowMessage {
visualizationType?: VisualizationType;
isEnabled: boolean;
configId: string;
elementResults?: AssessmentVisualizationInstance[];
featureFlagStoreData?: FeatureFlagStoreData;
elementResults: AssessmentVisualizationInstance[] | null;
featureFlagStoreData: FeatureFlagStoreData;
}

export class DrawingController {
Expand Down Expand Up @@ -69,7 +69,7 @@ export class DrawingController {
};

private async enableVisualization(
elementResults: AssessmentVisualizationInstance[] | undefined,
elementResults: AssessmentVisualizationInstance[] | null,
configId: string,
): Promise<void> {
if (elementResults) {
Expand All @@ -96,7 +96,7 @@ export class DrawingController {
const childFrameResults = elementResultsByFrame.filter(results => results.frame != null);
await this.allFramesMessenger.sendCommandToMultipleFrames(
DrawingController.triggerVisualizationCommand,
childFrameResults.map(results => results.frame),
childFrameResults.map(results => results.frame!),
(_frame, index) =>
this.createEnableVisualizationPayload(
configId,
Expand All @@ -110,12 +110,12 @@ export class DrawingController {

await this.allFramesMessenger.sendCommandToAllFrames(
DrawingController.triggerVisualizationCommand,
this.createEnableVisualizationPayload(configId),
this.createEnableVisualizationPayload(configId, null),
);
}

private enableVisualizationInCurrentFrame = async (
currentFrameResults: AssessmentVisualizationInstance[],
currentFrameResults: AssessmentVisualizationInstance[] | null,
configId: string,
): Promise<void> => {
const drawer = this.getDrawer(configId);
Expand All @@ -128,10 +128,10 @@ export class DrawingController {

private createEnableVisualizationPayload(
configId: string,
frameResults?: AssessmentVisualizationInstance[],
frameResults: AssessmentVisualizationInstance[] | null,
): VisualizationWindowMessage {
return {
elementResults: frameResults ?? null,
elementResults: frameResults,
isEnabled: true,
featureFlagStoreData: this.featureFlagStoreData,
configId: configId,
Expand Down Expand Up @@ -164,8 +164,8 @@ export class DrawingController {
}

private getInitialElements(
currentFrameResults: AssessmentVisualizationInstance[],
): AssessmentVisualizationInstance[] {
currentFrameResults: AssessmentVisualizationInstance[] | null,
): AssessmentVisualizationInstance[] | null {
if (currentFrameResults == null) {
return null;
}
Expand Down
1 change: 1 addition & 0 deletions src/injected/drawing-initiator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export class DrawingInitiator {
isEnabled: false,
featureFlagStoreData: featureFlagStoreData,
configId: configId,
elementResults: null,
};

await this.drawingController.processRequest(visualizationMessage);
Expand Down
2 changes: 1 addition & 1 deletion src/injected/visualization/base-drawer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export abstract class BaseDrawer implements Drawer {
this.drawerUtils = drawerUtils;
}

public abstract initialize(config: DrawerInitData<any>): void;
public abstract initialize(config: DrawerInitData): void;

public drawLayout = async (): Promise<void> => {
this.addListeners();
Expand Down
7 changes: 4 additions & 3 deletions src/injected/visualization/drawer.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { AssessmentVisualizationInstance } from 'injected/frameCommunicators/html-element-axe-results-helper';
import { FeatureFlagStoreData } from '../../common/types/store-data/feature-flag-store-data';

export interface Drawer {
initialize(config: DrawerInitData<any>): void;
initialize(config: DrawerInitData): void;
isOverlayEnabled: boolean;
drawLayout(): Promise<void>;
eraseLayout(): void;
}

export interface DrawerInitData<T> {
data: T[];
export interface DrawerInitData {
data: AssessmentVisualizationInstance[] | null;
featureFlagStoreData: FeatureFlagStoreData;
}
4 changes: 2 additions & 2 deletions src/injected/visualization/highlight-box-drawer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ export class HighlightBoxDrawer extends BaseDrawer {
this.dialogRenderer = this.formatter?.getDialogRenderer();
}

public initialize(config: DrawerInitData<HtmlElementAxeResults>): void {
this.elementResults = config.data;
public initialize(config: DrawerInitData): void {
this.elementResults = config.data ?? [];
this.eraseLayout();
}

Expand Down
2 changes: 1 addition & 1 deletion src/injected/visualization/null-drawer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import { Drawer, DrawerInitData } from './drawer';

export class NullDrawer implements Drawer {
public initialize(drawerData: DrawerInitData<any>): void {}
public initialize(drawerData: DrawerInitData): void {}
public isOverlayEnabled: boolean = false;
public drawLayout = async (): Promise<void> => {};
public eraseLayout(): void {}
Expand Down
4 changes: 2 additions & 2 deletions src/injected/visualization/single-target-drawer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ export class SingleTargetDrawer implements Drawer {
this.formatter = formatter;
}

public initialize(drawerInfo: DrawerInitData<HtmlElementAxeResults>): void {
public initialize(drawerInfo: DrawerInitData): void {
this.eraseLayout();
const elementResults = drawerInfo.data;
const elementResults = drawerInfo.data ?? [];
const myDocument = this.drawerUtils.getDocumentElement();
this.target = this.getFirstElementTarget(myDocument, elementResults);
}
Expand Down
14 changes: 6 additions & 8 deletions src/injected/visualization/svg-drawer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,20 +42,18 @@ export class SVGDrawer extends BaseDrawer {
this.failureItems = [];
}

public initialize(
drawerInfo: DrawerInitData<TabStopVisualizationInstance | TabbedElementData>,
): void {
const visualizationInstances: TabStopVisualizationInstance[] = drawerInfo.data.map(
(element: TabStopVisualizationInstance) => {
public initialize(drawerInfo: DrawerInitData): void {
const originalVisualizationInstances = drawerInfo.data ?? [];
const annotatedVisualizationInstances: TabStopVisualizationInstance[] =
originalVisualizationInstances.map((element: TabStopVisualizationInstance) => {
return {
propertyBag: {
tabOrder: this.getTabOrder(element),
},
...element,
};
},
);
this.updateTabbedElements(visualizationInstances);
});
this.updateTabbedElements(annotatedVisualizationInstances);
this.tabOrderedItems = this.allVisualizedItems.filter(item => item.tabOrder != null);
this.failureItems = this.allVisualizedItems.filter(item => item.isFailure);
}
Expand Down
10 changes: 9 additions & 1 deletion src/injected/window-initializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,21 @@ export class WindowInitializer {
const htmlElementUtils = new HTMLElementUtils();
this.clientUtils = new ClientUtils(window);

const extensionId = browserAdapter.getExtensionId();
if (extensionId == null) {
logger.error(
'Aborting Accessibility Insights initialization - extension instance is unloaded',
);
return;
}

this.actionMessageDispatcher = new RemoteActionMessageDispatcher(
this.browserAdapter.sendMessageToFrames,
null,
logger,
);

const telemetrySanitizer = new ExceptionTelemetrySanitizer(browserAdapter.getExtensionId());
const telemetrySanitizer = new ExceptionTelemetrySanitizer(extensionId);
const exceptionTelemetryListener = new ExceptionTelemetryListener(
TelemetryEventSource.TargetPage,
this.actionMessageDispatcher.sendTelemetry,
Expand Down
10 changes: 5 additions & 5 deletions src/tests/unit/tests/injected/drawing-controller.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { HtmlElementAxeResults } from 'common/types/store-data/visualization-scan-result-data';
import { AllFramesMessenger } from 'injected/frameCommunicators/all-frames-messenger';
import {
CommandMessage,
Expand All @@ -23,12 +22,13 @@ import { HighlightBoxDrawer } from '../../../../injected/visualization/highlight
class VisualizationWindowMessageStubBuilder {
private isEnabled: boolean;
private configId: string;
private elementResults?: AssessmentVisualizationInstance[];
private elementResults: AssessmentVisualizationInstance[] | null;
private featureFlagStoreData?: FeatureFlagStoreData;

public constructor(configId: string) {
this.configId = configId;
this.featureFlagStoreData = getDefaultFeatureFlagsWeb();
this.elementResults = null;
}

public setVisualizationEnabled(): VisualizationWindowMessageStubBuilder {
Expand All @@ -42,7 +42,7 @@ class VisualizationWindowMessageStubBuilder {
}

public setElementResults(
results: AssessmentVisualizationInstance[],
results: AssessmentVisualizationInstance[] | null,
): VisualizationWindowMessageStubBuilder {
this.elementResults = results;
return this;
Expand Down Expand Up @@ -158,7 +158,7 @@ describe('DrawingControllerTest', () => {
const iframeResults = ['iframeContent'];
const iframeElement = 'iframeElement';
const targetFrame = iframeElement as any;
const visibleResultStub = {} as HtmlElementAxeResults;
const visibleResultStub = {} as AssessmentVisualizationInstance;
const disabledResultStub = {
isVisualizationEnabled: false,
} as AssessmentVisualizationInstance;
Expand Down Expand Up @@ -207,7 +207,7 @@ describe('DrawingControllerTest', () => {
})
.verifiable(Times.once());

const expected: DrawerInitData<HtmlElementAxeResults> = {
const expected: DrawerInitData = {
data: [visibleResultStub],
featureFlagStoreData,
};
Expand Down
1 change: 1 addition & 0 deletions src/tests/unit/tests/injected/drawing-initiator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ describe('DrawingInitiatorTest', () => {
isEnabled: false,
featureFlagStoreData: getDefaultFeatureFlagsWeb(),
configId: configId,
elementResults: null,
};

drawingControllerMock
Expand Down
10 changes: 6 additions & 4 deletions src/tests/unit/tests/injected/visualization/drawer.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { getDefaultFeatureFlagsWeb } from 'common/feature-flags';
import { HtmlElementAxeResults } from 'common/types/store-data/visualization-scan-result-data';
import { WindowUtils } from 'common/window-utils';
import { BoundingRect } from 'injected/bounding-rect';
import { ClientUtils } from 'injected/client-utils';
import { DialogRenderer } from 'injected/dialog-renderer';
import { AssessmentVisualizationInstance } from 'injected/frameCommunicators/html-element-axe-results-helper';
import { ShadowUtils } from 'injected/shadow-utils';
import { DrawerInitData } from 'injected/visualization/drawer';
import { DrawerUtils } from 'injected/visualization/drawer-utils';
Expand Down Expand Up @@ -1195,20 +1195,22 @@ describe('Drawer', () => {
verifyOverlayStyle(overlays[2], element4Config);
});

function createDrawerInfo<T>(elementResults: T[]): DrawerInitData<T> {
function createDrawerInfo<T>(
elementResults: AssessmentVisualizationInstance[] | null,
): DrawerInitData {
return {
data: elementResults,
featureFlagStoreData: getDefaultFeatureFlagsWeb(),
};
}

function createElementResults(ids: string[]): HtmlElementAxeResults[] {
function createElementResults(ids: string[]): AssessmentVisualizationInstance[] {
return ids.map(id => {
return {
ruleResults: {},
target: [id],
targetIndex: 0,
};
} as AssessmentVisualizationInstance;
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { HtmlElementAxeResults } from 'common/types/store-data/visualization-scan-result-data';
import { AssessmentVisualizationInstance } from 'injected/frameCommunicators/html-element-axe-results-helper';
import { IMock, Mock, Times } from 'typemoq';

import { getDefaultFeatureFlagsWeb } from '../../../../../common/feature-flags';
Expand Down Expand Up @@ -29,12 +29,12 @@ describe('SingleTargetDrawer Tests', () => {

const testSubject = new SingleTargetDrawer(drawerUtilsMock.object, formatterMock.object);

const drawerInfo: DrawerInitData<HtmlElementAxeResults> = {
const drawerInfo: DrawerInitData = {
data: [
{
target: ['body'],
},
] as HtmlElementAxeResults[],
} as AssessmentVisualizationInstance,
],
featureFlagStoreData: getDefaultFeatureFlagsWeb(),
};

Expand All @@ -53,12 +53,12 @@ describe('SingleTargetDrawer Tests', () => {

const testSubject = new SingleTargetDrawer(drawerUtilsMock.object, formatterMock.object);

const drawerInfo: DrawerInitData<HtmlElementAxeResults> = {
const drawerInfo: DrawerInitData = {
data: [
{
target: ['body'],
},
] as HtmlElementAxeResults[],
} as AssessmentVisualizationInstance,
],
featureFlagStoreData: getDefaultFeatureFlagsWeb(),
};

Expand All @@ -84,12 +84,12 @@ describe('SingleTargetDrawer Tests', () => {

const testSubject = new SingleTargetDrawer(drawerUtilsMock.object, formatterMock.object);

const drawerInfo: DrawerInitData<HtmlElementAxeResults> = {
const drawerInfo: DrawerInitData = {
data: [
{
target: ['body'],
},
] as HtmlElementAxeResults[],
} as AssessmentVisualizationInstance,
],
featureFlagStoreData: getDefaultFeatureFlagsWeb(),
};

Expand All @@ -115,12 +115,12 @@ describe('SingleTargetDrawer Tests', () => {

const testSubject = new SingleTargetDrawer(drawerUtilsMock.object, formatterMock.object);

const drawerInfo: DrawerInitData<HtmlElementAxeResults> = {
const drawerInfo: DrawerInitData = {
data: [
{
target: ['body'],
},
] as HtmlElementAxeResults[],
} as AssessmentVisualizationInstance,
],
featureFlagStoreData: getDefaultFeatureFlagsWeb(),
};

Expand Down
11 changes: 8 additions & 3 deletions src/tests/unit/tests/injected/visualization/svg-drawer.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { TabbedElementData } from 'common/types/store-data/visualization-scan-result-data';
import { TabStopVisualizationInstance } from 'injected/frameCommunicators/html-element-axe-results-helper';
import {
AssessmentVisualizationInstance,
TabStopVisualizationInstance,
} from 'injected/frameCommunicators/html-element-axe-results-helper';
import { IMock, It, Mock, Times } from 'typemoq';

import { getDefaultFeatureFlagsWeb } from '../../../../../common/feature-flags';
Expand Down Expand Up @@ -47,7 +50,9 @@ describe('SVGDrawer', () => {
shadowUtilsMock.setup(x => x.getShadowContainer()).returns(() => shadowContainer);
});

function createDrawerInfo<T>(elementResults: T[]): DrawerInitData<T> {
function createDrawerInfo(
elementResults: AssessmentVisualizationInstance[] | null,
): DrawerInitData {
return {
data: elementResults,
featureFlagStoreData: getDefaultFeatureFlagsWeb(),
Expand Down Expand Up @@ -176,7 +181,7 @@ describe('SVGDrawer', () => {
null,
);

testSubject.initialize(createDrawerInfo(tabbedElements));
testSubject.initialize(createDrawerInfo(tabbedElements as any));
expect((testSubject as any).allVisualizedItems).toEqual(expectedAllVisualizedItems);
drawerUtilsMock.verifyAll();
});
Expand Down
Loading

0 comments on commit 2967c3f

Please sign in to comment.