Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 11 commits into from
Dec 19, 2022
22 changes: 14 additions & 8 deletions src/assessments/assessment-builder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -249,7 +255,7 @@ export class AssessmentBuilder {
private static getRequirementConfig(
requirements: Requirement[],
requirementKey: string,
): Requirement {
): Requirement | undefined {
return requirements.find(req => req.key === requirementKey);
}

Expand Down
14 changes: 8 additions & 6 deletions src/assessments/assessments-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
34 changes: 17 additions & 17 deletions src/assessments/assessments-requirements-filter.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand Down
Original file line number Diff line number Diff line change
@@ -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} />;
}
10 changes: 5 additions & 5 deletions src/assessments/types/assessments-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
5 changes: 3 additions & 2 deletions src/assessments/types/report-instance-field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(', ') },
Expand Down
20 changes: 10 additions & 10 deletions src/background/create-initial-assessment-test-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -89,7 +89,7 @@ function allRequirementsAreScanned(requirements: string[], persistedTest: Assess
function getDefaultTestResult(): AssessmentData {
return {
fullAxeResultsMap: null,
generatedAssessmentInstancesMap: null,
generatedAssessmentInstancesMap: undefined,
manualTestStepResultMap: {},
testStepStatus: {},
};
Expand All @@ -116,7 +116,7 @@ function constructRequirementStatus(

function constructManualRequirementResultMap(
requirements: string[],
persistedMap: RequirementIdToResultMap,
persistedMap?: RequirementIdToResultMap,
): RequirementIdToResultMap {
return constructMapFromRequirementTo<ManualTestStepResult>(
requirements,
Expand All @@ -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> = {};
Expand All @@ -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];
Expand All @@ -156,7 +156,7 @@ function constructGeneratedAssessmentInstancesMap(
}
});
if (isEmpty(map)) {
return null;
return undefined;
}
return map;
}
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 | 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' ||
Expand Down
Loading