Skip to content

Commit

Permalink
chore(null): null check fixes (#5985)
Browse files Browse the repository at this point in the history
#### Details

This PR fixes strict null checks for a few different files. These files were selected by running `yarn null:find`, added to `tsconfig.strictNullChecks.json` one-by-one, judged on whether their resolutions were straightforward enough to not require their own PR (removed if complex), and addressed.

I recommend going through this PR commit-by-commit, as each file from `yarn null:find` is scoped to a single commit.
* `src/common/rule-based-view-model-provider.ts`: allow various methods to return `null`
* `src/background/stores/tab-store.ts`: update `TabStoreData` to accept undefined values for `url`, `title`, and `id`, as that is what is returned from the `chrome.tabs` API
* `accept undefined values for `description`, `url`, and `guidance` for RuleResults, as returned by axe
* `src/DetailsView/components/interactive-header.tsx`: update InteractiveHeader to exclude `items` (an optional param) instead of passing `null`
* make `originalResult` optional and default to undefined, as it is optional in the data returned from axe
 
##### Motivation

#2869

##### Context

More PRs like this to follow, trying to keep PRs relatively small and well-chunked.

#### 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

Co-authored-by: madalynrose <[email protected]>
  • Loading branch information
madalynrose and madalynrose authored Sep 1, 2022
1 parent a86ec90 commit bf1b162
Show file tree
Hide file tree
Showing 22 changed files with 64 additions and 59 deletions.
3 changes: 1 addition & 2 deletions src/DetailsView/components/interactive-header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export const InteractiveHeader = NamedFC<InteractiveHeaderProps>('InteractiveHea
const isNavCollapsed = props.narrowModeStatus.isHeaderAndNavCollapsed;
const getNavMenu = () => {
if (isNavCollapsed === false) {
return null;
return;
}

return (
Expand All @@ -53,7 +53,6 @@ export const InteractiveHeader = NamedFC<InteractiveHeaderProps>('InteractiveHea
return (
<Header
deps={props.deps}
items={null}
farItems={getFarItems()}
navMenu={getNavMenu()}
showHeaderTitle={props.showHeaderTitle}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export function getSaveButtonForAssessment(props: SaveAssessmentButtonFactoryPro

const currentDate = props.deps.getCurrentDate();
const fileDate = props.deps.fileNameBuilder.getDateSegment(currentDate);
const targetPageTitle = props.tabStoreData.title;
const targetPageTitle = props.tabStoreData.title || '';
const fileTitle = props.deps.fileNameBuilder.getTitleSegment(targetPageTitle);
const fileName = `SavedAssessment_${fileDate}_${fileTitle}.a11ywebassessment`;
const fileURL = props.deps.fileURLProvider.provideURL([assessmentData], 'application/json');
Expand Down
3 changes: 0 additions & 3 deletions src/background/stores/tab-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ export class TabStore extends PersistentStore<TabStoreData> {

public getDefaultState(): TabStoreData {
const defaultValues: TabStoreData = {
url: null,
title: null,
id: null,
isClosed: false,
isChanged: false,
isPageHidden: false,
Expand Down
4 changes: 2 additions & 2 deletions src/common/components/cards/rule-resources.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ export const RuleResources = NamedFC<RuleResourcesProps>('RuleResources', ({ dep
};

const renderGuidanceLinks = () => (
<GuidanceLinks links={rule.guidance} LinkComponent={deps.LinkComponent} />
<GuidanceLinks links={rule.guidance!} LinkComponent={deps.LinkComponent} />
);
const renderGuidanceTags = () => <GuidanceTags deps={deps} links={rule.guidance} />;
const renderGuidanceTags = () => <GuidanceTags deps={deps} links={rule.guidance!} />;

return (
<div className={styles.ruleMoreResources}>
Expand Down
10 changes: 5 additions & 5 deletions src/common/rule-based-view-model-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ export type GetCardViewData = (
rules: UnifiedRule[],
results: UnifiedResult[],
cardSelectionViewData: CardSelectionViewData,
) => CardsViewModel;
) => CardsViewModel | null;

export const getCardViewData: GetCardViewData = (
rules: UnifiedRule[],
results: UnifiedResult[],
cardSelectionViewData: CardSelectionViewData,
): CardsViewModel => {
): CardsViewModel | null => {
if (results == null || rules == null || cardSelectionViewData == null) {
return null;
}
Expand All @@ -37,7 +37,7 @@ export const getCardViewData: GetCardViewData = (
const isInstanceDisplayed = result.status === 'fail' || result.status === 'unknown';
let ruleResult = getExistingRuleFromResults(result.ruleId, ruleResults);

if (!ruleResult) {
if (ruleResult == null) {
const rule = getUnifiedRule(result.ruleId, rules);
if (!rule) {
continue;
Expand Down Expand Up @@ -77,7 +77,7 @@ export const getCardViewData: GetCardViewData = (
const getExistingRuleFromResults = (
ruleId: string,
ruleResults: CardRuleResult[],
): CardRuleResult => {
): CardRuleResult | null => {
const ruleResultIndex: number = getRuleResultIndex(ruleId, ruleResults);

return ruleResultIndex !== -1 ? ruleResults[ruleResultIndex] : null;
Expand Down Expand Up @@ -132,7 +132,7 @@ const createCardResult = (
};
};

const getUnifiedRule = (id: string, rules: UnifiedRule[]): UnifiedRule =>
const getUnifiedRule = (id: string, rules: UnifiedRule[]): UnifiedRule | undefined =>
rules.find(rule => rule.id === id);

const getRuleResultIndex = (ruleId: string, ruleResults: CardRuleResult[]): number =>
Expand Down
16 changes: 9 additions & 7 deletions src/common/telemetry-data-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -389,10 +389,10 @@ export class TelemetryDataFactory {
testVisualizationType,
) => {
const passedRuleResults: DictionaryStringTo<number> = this.generateTelemetryRuleResult(
analyzerResult.originalResult.passes,
analyzerResult.originalResult?.passes,
);
const failedRuleResults: DictionaryStringTo<number> = this.generateTelemetryRuleResult(
analyzerResult.originalResult.violations,
analyzerResult.originalResult?.violations,
);
const telemetry: IssuesAnalyzerScanTelemetryData = {
...this.forTestScan(
Expand All @@ -415,13 +415,13 @@ export class TelemetryDataFactory {
testVisualizationType,
) => {
const passedRuleResults: DictionaryStringTo<number> = this.generateTelemetryRuleResult(
analyzerResult.originalResult.passes,
analyzerResult.originalResult?.passes,
);
const failedRuleResults: DictionaryStringTo<number> = this.generateTelemetryRuleResult(
analyzerResult.originalResult.violations,
analyzerResult.originalResult?.violations,
);
const incompleteRuleResults: DictionaryStringTo<number> = this.generateTelemetryRuleResult(
analyzerResult.originalResult.incomplete,
analyzerResult.originalResult?.incomplete,
);
const telemetry: NeedsReviewAnalyzerScanTelemetryData = {
...this.forTestScan(
Expand Down Expand Up @@ -457,9 +457,11 @@ export class TelemetryDataFactory {
return mouseEvent.detail === 0 ? 'keypress' : 'mouseclick';
}

private generateTelemetryRuleResult(axeRule: AxeRule[]): DictionaryStringTo<number> {
private generateTelemetryRuleResult(
axeRule: AxeRule[] | undefined,
): DictionaryStringTo<number> {
const ruleResults: DictionaryStringTo<number> = {};
axeRule.forEach(element => {
axeRule?.forEach(element => {
const key: string = element.id;
if (key != null) {
ruleResults[key] = element.nodes.length;
Expand Down
2 changes: 1 addition & 1 deletion src/common/types/axe-analyzer-result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { DictionaryStringTo } from 'types/common-types';

export interface AxeAnalyzerResult {
results: DictionaryStringTo<any>;
originalResult: ScanResults;
originalResult?: ScanResults;
include?: SingleElementSelector[];
exclude?: SingleElementSelector[];
}
6 changes: 3 additions & 3 deletions src/common/types/create-issue-details-text-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import { GuidanceLink } from 'common/types/store-data/guidance-links';
export interface CreateIssueDetailsTextData {
rule: {
id: string;
description: string;
url: string;
guidance: GuidanceLink[];
description?: string;
url?: string;
guidance?: GuidanceLink[];
};
targetApp: {
name?: string;
Expand Down
6 changes: 3 additions & 3 deletions src/common/types/store-data/card-view-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ export type CardRuleResultStatus = InstanceResultStatus | 'inapplicable';
export interface CardRuleResult {
id: string;
nodes: CardResult[];
description: string;
url: string;
guidance: GuidanceLink[];
description?: string;
url?: string;
guidance?: GuidanceLink[];
isExpanded: boolean;
}
export type CardRuleResultsByStatus = {
Expand Down
6 changes: 3 additions & 3 deletions src/common/types/store-data/tab-store-data.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
export interface TabStoreData {
url: string;
title: string;
id: number;
url?: string;
title?: string;
id?: number;
isClosed: boolean;
isChanged: boolean;
isPageHidden: boolean;
Expand Down
6 changes: 3 additions & 3 deletions src/common/types/store-data/unified-data-interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ export type ScanMetadata = {

export interface UnifiedRule {
id: string;
description: string;
url: string;
guidance: GuidanceLink[];
description?: string;
url?: string;
guidance?: GuidanceLink[];
}

export interface UnifiedScanResultStoreData {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export type DecoratedAxeNodeResult = {
html?: string;
help?: string;
id?: string;
guidanceLinks: GuidanceLink[];
guidanceLinks?: GuidanceLink[];
helpUrl?: string;
} & CheckData;

Expand Down
3 changes: 1 addition & 2 deletions src/injected/analyzers/base-analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ export class BaseAnalyzer implements Analyzer {
protected visualizationType: VisualizationType;
protected emptyResults: AxeAnalyzerResult = {
results: {},
originalResult: null,
};

constructor(
Expand Down Expand Up @@ -44,7 +43,7 @@ export class BaseAnalyzer implements Analyzer {
config: AnalyzerConfiguration,
): Message {
const messageType = config.analyzerMessageType;
const originalAxeResult = analyzerResult.originalResult;
const originalAxeResult = analyzerResult.originalResult!;
const payload: ScanCompletedPayload<any> = {
key: config.key,
selectorMap: analyzerResult.results,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,18 @@ export const createIssueDetailsBuilderForUnified = (
]
: [];

const ruleDetailsSection = () => {
const { description, url, id } = data.rule;
if (description && url && id) {
return `${snippet(description)} (${link(url, id)})`;
}
return '';
};

const lines = [
sectionHeader('Issue'),
sectionHeaderSeparator(),
`${snippet(data.rule.description)} (${link(data.rule.url, data.rule.id)})`,
ruleDetailsSection(),
sectionSeparator(),

sectionHeader('Target application'),
Expand Down
10 changes: 9 additions & 1 deletion src/issue-filing/common/create-issue-details-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,18 @@ export const createIssueDetailsBuilder = (markup: MarkupFormatter): IssueDetails
]
: [];

const ruleDetailsSection = () => {
const { description, url, id } = data.rule;
if (description && url && id) {
return `${snippet(description)} (${link(url, id)})`;
}
return '';
};

const lines = [
sectionHeader('Issue'),
sectionHeaderSeparator(),
`${snippet(data.rule.description)} (${link(data.rule.url, data.rule.id)})`,
ruleDetailsSection(),
sectionSeparator(),

sectionHeader('Target application'),
Expand Down
6 changes: 4 additions & 2 deletions src/issue-filing/common/issue-filing-url-string-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@ const getSelectorLastPart = (selector: string): string => {
};

const standardizeTags = (data: CreateIssueDetailsTextData): string[] => {
const guidanceLinkTextTags = data.rule.guidance.map(link => link.text.toUpperCase());
const guidanceLinkTextTags = data.rule.guidance
? data.rule.guidance.map(link => link.text.toUpperCase())
: [];
const tagsFromGuidanceLinkTags: string[] = [];
data.rule.guidance.map(link =>
data.rule.guidance?.map(link =>
link.tags ? link.tags.map(tag => tagsFromGuidanceLinkTags.push(tag.displayText)) : [],
);
return guidanceLinkTextTags.concat(tagsFromGuidanceLinkTags);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export const ruleDetailAutomationId = 'rule-detail';
export type MinimalRuleHeaderProps = {
rule: {
id: string;
description: string;
description?: string;
nodes: any[];
};
outcomeType: InstanceOutcomeType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,10 @@ exports[`DetailsViewBody render render 1`] = `
}
tabStoreData={
Object {
"id": null,
"isChanged": false,
"isClosed": false,
"isOriginChanged": false,
"isPageHidden": false,
"title": null,
"url": null,
}
}
visualizationConfigurationFactory={Object {}}
Expand Down Expand Up @@ -467,13 +464,10 @@ exports[`DetailsViewBody render render 1`] = `
}
tabStoreData={
Object {
"id": null,
"isChanged": false,
"isClosed": false,
"isOriginChanged": false,
"isPageHidden": false,
"title": null,
"url": null,
}
}
visualizationConfigurationFactory={Object {}}
Expand Down Expand Up @@ -810,13 +804,10 @@ exports[`DetailsViewBody render render 1`] = `
}
tabStoreData={
Object {
"id": null,
"isChanged": false,
"isClosed": false,
"isOriginChanged": false,
"isPageHidden": false,
"title": null,
"url": null,
}
}
visualizationConfigurationFactory={Object {}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,11 @@ exports[`InteractiveHeader render: isNavCollapsed equals false 1`] = `
featureFlagData={null}
/>
}
items={null}
narrowModeStatus={
Object {
"isHeaderAndNavCollapsed": false,
}
}
navMenu={null}
/>
`;

Expand All @@ -28,7 +26,6 @@ exports[`InteractiveHeader render: isNavCollapsed equals true 1`] = `
featureFlagData={null}
/>
}
items={null}
narrowModeStatus={
Object {
"isHeaderAndNavCollapsed": true,
Expand Down Expand Up @@ -65,13 +62,11 @@ exports[`InteractiveHeader render: tabClosed equals false 1`] = `
}
/>
}
items={null}
narrowModeStatus={
Object {
"isHeaderAndNavCollapsed": false,
}
}
navMenu={null}
/>
`;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe('BaseAnalyzer', () => {
payload: {
key: configStub.key,
selectorMap: resultsStub,
scanResult: null,
scanResult: undefined,
testType: typeStub,
scanIncompleteWarnings,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ describe('TabStopsAnalyzer', () => {
payload: {
key: configStub.key,
selectorMap: {},
scanResult: null,
testType: visualizationTypeStub,
scanIncompleteWarnings: [],
scanResult: undefined,
},
};
});
Expand Down
Loading

0 comments on commit bf1b162

Please sign in to comment.