chore(null): strict null checks for assessment-provider and deps (#6274)

#### Details

This PR adds `assessment-builder`, `assessment-provider`, and most
individual assessment `test-steps` to the strict null check config.

The only particularly interesting conversion was in
`assessments-requirements-filter` - I ended up converting a pattern that
used a mutating `filter` callback followed by a `map` to use a single
`reduce` call to implement a `filterMap` pattern, so that typescript
could see the relevant type information within the same scope instead of
being split into several scopes.

```md
## Web strict-null progress

**81%** complete (**1272**/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

src/assessments/assessment-builder.tsx

@@ -23,17 +23,17 @@ import { DrawerProvider } from 'injected/visualization/drawer-provider';
 import { cloneDeep } from 'lodash';
 import { DictionaryStringTo } from 'types/common-types';
 import { Assessment, AssistedAssessment, ManualAssessment } from './types/iassessment';
-import { ReportInstanceField } from './types/report-instance-field';
+import { ReportInstanceField, ReportInstanceFields } from './types/report-instance-field';
 import { Requirement } from './types/requirement';
 
 export class AssessmentBuilder {
     private static applyDefaultReportFieldMap(requirement: Requirement): void {
         const { comment, snippet, path, manualSnippet, manualPath } = ReportInstanceField.common;
 
-        const defaults = requirement.isManual
+        const defaults: ReportInstanceFields = requirement.isManual
             ? [comment, manualPath, manualSnippet]
             : [path, snippet];
-        const specified = requirement.reportInstanceFields || [];
+        const specified: ReportInstanceFields = requirement.reportInstanceFields ?? [];
 
         requirement.reportInstanceFields = [...defaults, ...specified];
     }
@@ -133,11 +133,14 @@ export class AssessmentBuilder {
             selectorMap: DictionaryStringTo<any>,
             requirement?: string,
         ) => {
+            if (requirement == null) {
+                return null;
+            }
             const requirementConfig = AssessmentBuilder.getRequirementConfig(
                 requirements,
                 requirement,
             );
-            if (requirementConfig.getNotificationMessage == null) {
+            if (requirementConfig?.getNotificationMessage == null) {
                 return null;
             }
             return requirementConfig.getNotificationMessage(selectorMap);
@@ -180,7 +183,7 @@ export class AssessmentBuilder {
                 requirements,
                 analyzerConfig.key,
             );
-            if (requirementConfig.getAnalyzer == null) {
+            if (requirementConfig?.getAnalyzer == null) {
                 return provider.createBaseAnalyzer(analyzerConfig);
             }
             return requirementConfig.getAnalyzer(provider, analyzerConfig);
@@ -195,7 +198,7 @@ export class AssessmentBuilder {
                 requirements,
                 requirement,
             );
-            if (requirementConfig.getDrawer == null) {
+            if (requirementConfig?.getDrawer == null) {
                 return provider.createNullDrawer();
             }
             return requirementConfig.getDrawer(provider, featureFlagStoreData);
@@ -205,11 +208,14 @@ export class AssessmentBuilder {
             selectorMap: DictionaryStringTo<any>,
             requirement?: string,
         ) => {
+            if (requirement == null) {
+                return null;
+            }
             const requirementConfig = AssessmentBuilder.getRequirementConfig(
                 requirements,
                 requirement,
             );
-            if (requirementConfig.getNotificationMessage == null) {
+            if (requirementConfig?.getNotificationMessage == null) {
                 return null;
             }
             return requirementConfig.getNotificationMessage(selectorMap);
@@ -249,7 +255,7 @@ export class AssessmentBuilder {
     private static getRequirementConfig(
         requirements: Requirement[],
         requirementKey: string,
-    ): Requirement {
+    ): Requirement | undefined {
         return requirements.find(req => req.key === requirementKey);
     }
 

src/assessments/assessments-provider.ts

@@ -18,27 +18,27 @@ export class AssessmentsProviderImpl implements AssessmentsProvider {
         return this.assessments.slice();
     }
 
-    public forType(visualizationType: VisualizationType): Assessment {
+    public forType(visualizationType: VisualizationType): Assessment | undefined {
         return this.all().find(a => a.visualizationType === visualizationType);
     }
 
     public isValidType(visualizationType: VisualizationType): boolean {
         return this.forType(visualizationType) != null;
     }
 
-    public forKey(key: string): Assessment {
+    public forKey(key: string): Assessment | undefined {
         return this.all().find(a => a.key === key);
     }
 
-    public forRequirementKey(key: string): Assessment {
-        return this.all().find(a => a.requirements.find(r => r.key === key));
+    public forRequirementKey(key: string): Assessment | undefined {
+        return this.all().find(a => a.requirements.find(r => r.key === key) != null);
     }
 
     public isValidKey(key: string): boolean {
         return this.forKey(key) != null;
     }
 
-    public getStep(visualizationType: VisualizationType, key: string): Requirement {
+    public getStep(visualizationType: VisualizationType, key: string): Requirement | null {
         const assessment = this.forType(visualizationType);
         if (!assessment) {
             return null;
@@ -51,7 +51,9 @@ export class AssessmentsProviderImpl implements AssessmentsProvider {
         return { ...steps[index] };
     }
 
-    public getStepMap(visualizationType: VisualizationType): DictionaryStringTo<Requirement> {
+    public getStepMap(
+        visualizationType: VisualizationType,
+    ): DictionaryStringTo<Requirement> | null {
         const assessment = this.forType(visualizationType);
         if (!assessment) {
             return null;

src/assessments/assessments-requirements-filter.ts

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 import { AutomatedChecks } from 'assessments/automated-checks/assessment';
+import { Assessment } from 'assessments/types/iassessment';
 import { VisualizationType } from 'common/types/visualization-type';
 import { DictionaryStringTo } from 'types/common-types';
 
@@ -11,25 +12,24 @@ export function assessmentsProviderForRequirements(
     assessmentProvider: AssessmentsProvider,
     requirementToVisualizationTypeMap: DictionaryStringTo<VisualizationType>,
 ): AssessmentsProvider {
-    const assessments = assessmentProvider
-        .all()
-        .map(assessment => {
-            let type: VisualizationType;
-            const requirements = assessment.requirements.filter(req => {
-                if (requirementToVisualizationTypeMap[req.key]) {
-                    type = requirementToVisualizationTypeMap[req.key];
-                    return true;
-                }
-                return false;
-            });
+    const assessments: Assessment[] = assessmentProvider.all().reduce((accumulator, assessment) => {
+        // This is a filterMap operation; it can be simplified if/when lodash merges
+        // https://github.com/lodash/lodash/issues/5300
+        const filteredRequirements = assessment.requirements.filter(
+            req => requirementToVisualizationTypeMap[req.key] != null,
+        );
+        if (filteredRequirements.length > 0) {
+            const lastRequirement = filteredRequirements[filteredRequirements.length - 1];
+            const visualizationType = requirementToVisualizationTypeMap[lastRequirement.key];
 
-            return {
+            accumulator.push({
                 ...assessment,
-                requirements,
-                visualizationType: type,
-            };
-        })
-        .filter(assessment => assessment.requirements.length > 0);
+                requirements: filteredRequirements,
+                visualizationType,
+            });
+        }
+        return accumulator;
+    }, [] as Assessment[]);
 
     const mediumPassAutomatedChecks = { ...AutomatedChecks };
     mediumPassAutomatedChecks.visualizationType = VisualizationType.AutomatedChecksMediumPass;

src/assessments/headings/headings-instance-details-column-renderer.tsx

@@ -10,16 +10,13 @@ export function headingsAssessmentInstanceDetailsColumnRenderer(
     item: InstanceTableRow<HeadingsAssessmentProperties>,
 ): JSX.Element {
     const propertyBag = item.instance.propertyBag;
-    const textContent = propertyBag ? propertyBag.headingText : null;
-    const headingLevel = propertyBag ? propertyBag.headingLevel : null;
-    const labelText = headingLevel ? `H${item.instance.propertyBag.headingLevel}` : 'N/A';
-    const headingStyle = headingLevel ? HeadingFormatter.headingStyles[headingLevel] : null;
-    const background = headingStyle ? headingStyle.outlineColor : '#767676';
-    let customClass: string = null;
+    const textContent = propertyBag?.headingText ?? '';
+    const headingLevel = propertyBag?.headingLevel ?? null;
 
-    if (headingLevel == null) {
-        customClass = 'not-applicable';
-    }
+    const labelText = headingLevel != null ? `H${headingLevel}` : 'N/A';
+    const headingStyle = headingLevel != null ? HeadingFormatter.headingStyles[headingLevel] : null;
+    const customClass = headingLevel != null ? undefined : 'not-applicable';
+    const background = headingStyle != null ? headingStyle.outlineColor : '#767676';
 
     return (
         <AssessmentInstanceDetailsColumn

src/assessments/landmarks/landmarks-instance-details-column-renderer.tsx

@@ -11,9 +11,9 @@ export function landmarksAssessmentInstanceDetailsColumnRenderer(
     item: InstanceTableRow<LandmarksAssessmentProperties>,
 ): JSX.Element {
     const propertyBag = item.instance.propertyBag;
-    const background = LandmarkFormatter.getStyleForLandmarkRole(propertyBag.role).outlineColor;
-    let textContent = propertyBag.role;
-    if (propertyBag.label != null) {
+    const background = LandmarkFormatter.getStyleForLandmarkRole(propertyBag?.role).outlineColor;
+    let textContent = propertyBag?.role ?? '';
+    if (propertyBag?.label != null) {
         textContent += `: ${propertyBag.label}`;
     }
 

src/assessments/links/test-steps/label-in-name-get-completed-requirement-details.ts

@@ -2,29 +2,24 @@
 // Licensed under the MIT License.
 
 import { LinksTestStep } from 'assessments/links/test-steps/test-steps';
-import {
-    AssessmentData,
-    GeneratedAssessmentInstance,
-    TestStepResult,
-} from 'common/types/store-data/assessment-result-data';
+import { AssessmentData, TestStepResult } from 'common/types/store-data/assessment-result-data';
 import { ManualTestStatus } from 'common/types/store-data/manual-test-status';
-import { forEach } from 'lodash';
+import { forOwn } from 'lodash';
 
 export const labelInNameGetCompletedRequirementDetails = (assessmentData: AssessmentData) => {
     let expectedPasses = 0;
     let expectedFailures = 0;
     let confirmedPasses = 0;
     let confirmedFailures = 0;
-    forEach(Object.keys(assessmentData.generatedAssessmentInstancesMap), key => {
-        const instance: GeneratedAssessmentInstance =
-            assessmentData.generatedAssessmentInstancesMap[key];
+
+    forOwn(assessmentData.generatedAssessmentInstancesMap ?? {}, instance => {
         const testStepResult: TestStepResult = instance.testStepResults[LinksTestStep.labelInName];
         if (!testStepResult) {
             return;
         }
-        const labelContainsVisibleText = instance.propertyBag['labelContainsVisibleText'];
 
-        if (labelContainsVisibleText === undefined) {
+        const labelContainsVisibleText = instance.propertyBag?.['labelContainsVisibleText'];
+        if (labelContainsVisibleText == null) {
             return;
         }
 

src/assessments/page/frametitle-instance-details-column-renderer.tsx

@@ -10,7 +10,9 @@ export function frameTitleInstanceDetailsColumnRenderer(
     item: InstanceTableRow<FrameAssessmentProperties>,
 ): JSX.Element {
     const propertyBag = item.instance.propertyBag;
-    const frameTitle = propertyBag ? propertyBag.frameTitle : null;
+    // Undefined frameTitles shouldn't occur; frames without titles are expected to be omitted from
+    // the instance list, since they're already covered by an automated check failure.
+    const frameTitle = propertyBag?.frameTitle ?? '';
     const frameType = propertyBag ? propertyBag.frameType : 'default';
     const frameConfig = FrameFormatter.frameStyles[frameType];
 

src/assessments/page/pagetitle-instance-details-column-renderer.tsx

@@ -1,12 +1,17 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 import { InstanceTableRow } from 'assessments/types/instance-table-data';
+import { PageAssessmentProperties } from 'common/types/store-data/assessment-result-data';
 import { AssessmentInstanceDetailsColumn } from 'DetailsView/components/assessment-instance-details-column';
 import * as React from 'react';
 
-export function pageTitleInstanceDetailsColumnRenderer(item: InstanceTableRow<any>): JSX.Element {
+export function pageTitleInstanceDetailsColumnRenderer(
+    item: InstanceTableRow<PageAssessmentProperties>,
+): JSX.Element {
     const propertyBag = item.instance.propertyBag;
-    const textContent = propertyBag ? propertyBag.pageTitle : null;
+    // Undefined pageTitles shouldn't occur in practice; the browser will infer the title to be
+    // the last URL path component rather than emitting an undefined page title
+    const textContent = propertyBag?.pageTitle ?? '';
 
     return <AssessmentInstanceDetailsColumn textContent={textContent} />;
 }

src/assessments/types/assessments-provider.ts

@@ -8,12 +8,12 @@ import { Requirement } from './requirement';
 export interface AssessmentsProvider {
     all(): ReadonlyArray<Readonly<Assessment>>;
     isValidType(visualizationType: VisualizationType): boolean;
-    forType(visualizationType: VisualizationType): Readonly<Assessment>;
+    forType(visualizationType: VisualizationType): Readonly<Assessment> | undefined;
     isValidKey(key: string): boolean;
-    forKey(key: string): Readonly<Assessment>;
-    forRequirementKey(key: string): Readonly<Assessment>;
-    getStep(visualizationType: VisualizationType, key: string): Readonly<Requirement>;
+    forKey(key: string): Readonly<Assessment> | undefined;
+    forRequirementKey(key: string): Readonly<Assessment> | undefined;
+    getStep(visualizationType: VisualizationType, key: string): Readonly<Requirement> | null;
     getStepMap(
         visualizationType: VisualizationType,
-    ): Readonly<DictionaryStringTo<Readonly<Requirement>>>;
+    ): Readonly<DictionaryStringTo<Readonly<Requirement>>> | null;
 }

src/assessments/types/report-instance-field.ts

@@ -46,15 +46,16 @@ function fromColumnValueBagField<PB extends ColumnValueBag>(
 function fromPropertyBagFunction<PB>(
     label: string,
     key: string,
-    accessor: (bag: PB) => string,
+    accessor: (bag: PB) => string | null,
 ): ReportInstanceField<HasPropertyBag<PB>> {
     function getValue(i: HasPropertyBag<PB>): ColumnValue {
         return i.propertyBag && accessor(i.propertyBag);
     }
     return { key, label, getValue };
 }
 
-const common: ReportInstanceFieldMap = {
+type CommonReportInstanceFieldKey = 'comment' | 'snippet' | 'path' | 'manualSnippet' | 'manualPath';
+const common: { [key in CommonReportInstanceFieldKey]: ReportInstanceField } = {
     comment: { key: 'comment', label: 'Comment', getValue: i => i.description },
     snippet: { key: 'snippet', label: 'Snippet', getValue: i => i.html },
     path: { key: 'path', label: 'Path', getValue: i => i.target && i.target.join(', ') },

src/background/create-initial-assessment-test-data.ts

@@ -56,14 +56,14 @@ export const createAutomatedChecksInitialAssessmentTestData: InitialDataCreator
 };
 
 function getDefaultInitialTestData(requirements: string[]): AssessmentData {
-    return getInitialTestDataUsingPersistedData(requirements, {}, {}, null);
+    return getInitialTestDataUsingPersistedData(requirements, {}, {}, undefined);
 }
 
 function getInitialTestDataUsingPersistedData(
     requirements: string[],
     persistedRequirementsStatus: ManualTestStatusData,
-    persistedManualMap: RequirementIdToResultMap,
-    persistedGeneratedMap: InstanceIdToInstanceDataMap,
+    persistedManualMap?: RequirementIdToResultMap,
+    persistedGeneratedMap?: InstanceIdToInstanceDataMap,
 ): AssessmentData {
     const testData: AssessmentData = getDefaultTestResult();
     testData.testStepStatus = constructRequirementStatus(requirements, persistedRequirementsStatus);
@@ -89,7 +89,7 @@ function allRequirementsAreScanned(requirements: string[], persistedTest: Assess
 function getDefaultTestResult(): AssessmentData {
     return {
         fullAxeResultsMap: null,
-        generatedAssessmentInstancesMap: null,
+        generatedAssessmentInstancesMap: undefined,
         manualTestStepResultMap: {},
         testStepStatus: {},
     };
@@ -116,7 +116,7 @@ function constructRequirementStatus(
 
 function constructManualRequirementResultMap(
     requirements: string[],
-    persistedMap: RequirementIdToResultMap,
+    persistedMap?: RequirementIdToResultMap,
 ): RequirementIdToResultMap {
     return constructMapFromRequirementTo<ManualTestStepResult>(
         requirements,
@@ -127,7 +127,7 @@ function constructManualRequirementResultMap(
 
 function constructMapFromRequirementTo<T>(
     requirements: string[],
-    persistedMap: DictionaryStringTo<T>,
+    persistedMap: DictionaryStringTo<T> | undefined,
     getDefaultData: (req: string) => T,
 ): DictionaryStringTo<T> {
     const map: DictionaryStringTo<T> = {};
@@ -141,11 +141,11 @@ function constructMapFromRequirementTo<T>(
 
 function constructGeneratedAssessmentInstancesMap(
     requirements: string[],
-    persistedMap: InstanceIdToInstanceDataMap,
-): InstanceIdToInstanceDataMap {
+    persistedMap?: InstanceIdToInstanceDataMap,
+): InstanceIdToInstanceDataMap | undefined {
     const map: InstanceIdToInstanceDataMap = {};
     if (isEmpty(persistedMap)) {
-        return null;
+        return undefined;
     }
     Object.keys(persistedMap).forEach(instanceId => {
         const instanceData: GeneratedAssessmentInstance = persistedMap[instanceId];
@@ -156,7 +156,7 @@ function constructGeneratedAssessmentInstancesMap(
         }
     });
     if (isEmpty(map)) {
-        return null;
+        return undefined;
     }
     return map;
 }

src/common/types/property-bag/column-value-bag.ts

@@ -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 | undefined;
+export type ScalarColumnValue = string | number | Date | boolean | undefined | null;
 export type ColumnValue = ScalarColumnValue | BagOf<ScalarColumnValue>;
 export type ColumnValueBag = BagOf<ColumnValue>;
 
 export function isScalarColumnValue(value): value is ScalarColumnValue {
     return (
+        value === null ||
         value === undefined ||
         typeof value === 'string' ||
         typeof value === 'boolean' ||