Skip to content

Commit

Permalink
chore(null): null strictness for injected/components, /visualizations…
Browse files Browse the repository at this point in the history
…, deps (#6244)

#### Details

This PR continues the work from #6226 and #6223, adding another layer of
injected files to the strict null check list.

The only case with a notable behavioral change is that axe-core
technically documents in its typings that it's allowed to omit a "how to
fix" section from results, but if it were to actually do so, our cards
UI would currently crash. I don't think axe-core actually omits that
info in practice (I suspect it's there to accomodate custom rules), but
now if it were to do so, we'd omit the section from the card rather than
crashing (similar to how we handle results with missing snippets).

This ended up enabling a *lot* of files to be autoadded! It brings us
from ~72% null-strict to ~78%

```markdown
## Web strict-null progress

**78%** complete (**1210**/1561 non-test files)

*Contribute at [#2869](#2869). Last update: Mon Dec 05 2022*
Done in 1.00s.
```

##### 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 6, 2022
1 parent a4b6b6e commit 0301cfe
Show file tree
Hide file tree
Showing 23 changed files with 198 additions and 147 deletions.
4 changes: 2 additions & 2 deletions src/ad-hoc-visualizations/issues/get-notification-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import { DictionaryStringTo } from 'types/common-types';

export const getNotificationMessage = (
selectorMap: DictionaryStringTo<any>,
warnings: ScanIncompleteWarningId[],
warnings?: ScanIncompleteWarningId[],
) => {
if (isEmpty(selectorMap) && isEmpty(warnings)) {
if (warnings == null || (isEmpty(selectorMap) && isEmpty(warnings))) {
return 'Congratulations!\n\nAutomated checks found no issues on this page.';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export interface AssessmentVisualizationConfiguration {
selectorMap: DictionaryStringTo<any>,
testStep?: string,
warnings?: ScanIncompleteWarningId[],
) => string;
) => string | null;
getDrawer: (provider: DrawerProvider, testStep?: string) => Drawer;
getSwitchToTargetTabOnScan: (testStep?: string) => boolean;
getInstanceIdentiferGenerator: (
Expand Down
23 changes: 13 additions & 10 deletions src/common/target-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,15 @@ export class TargetHelper {
}
};

public static getTargetFromSelector = (selector: string): Target | undefined => {
// This returns undefined for undefined input for compatibility with non-strict-null-compatible
// usages that we haven't investigated yet. The "!selector" clause can/should be removed once
// #2869 is complete.
public static getTargetFromSelector = (selector: string): Target => {
if (selector === '') {
return [];
}
if (!selector) {
return;
return undefined!;
}
const selectors: string[] = selector.split(';');
const shadowDomSelectors = selectors.map(selectors => {
Expand All @@ -78,14 +81,14 @@ export class TargetHelper {
return shadowDomSelectors;
};

public static getSelectorFromTarget = (target: Target): string | undefined => {
if (target) {
return target
.map((targets: string | string[]) =>
typeof targets === 'string' ? targets : targets.join(','),
)
.join(';');
}
// This returns undefined for undefined input for compatibility with non-strict-null-compatible
// usages that we haven't investigated yet. The ?s can/should be removed once #2869 is complete.
public static getSelectorFromTarget = (target: Target): string => {
return target
?.map((targets: string | string[]) =>
typeof targets === 'string' ? targets : targets.join(','),
)
?.join(';');
};

public static getSelectorFromTargetElement = (
Expand Down
2 changes: 1 addition & 1 deletion src/common/types/create-issue-details-text-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ export interface CreateIssueDetailsTextData {
identifier: string;
conciseName: string;
};
howToFixSummary: string;
howToFixSummary?: string;
snippet?: string;
}
3 changes: 2 additions & 1 deletion src/injected/analyzers/analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ export interface AnalyzerConfiguration {
}

export interface RuleAnalyzerConfiguration extends AnalyzerConfiguration {
rules: string[];
// null implies "the scanner's default rule set"
rules: string[] | null;
resultProcessor: (
scanner: ScannerUtils,
) => (results: ScanResults) => DictionaryStringTo<HtmlElementAxeResults>;
Expand Down
8 changes: 6 additions & 2 deletions src/injected/analyzers/batched-rule-analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import { ScannerUtils } from '../scanner-utils';
import { RuleAnalyzerConfiguration } from './analyzer';
import { RuleAnalyzer } from './rule-analyzer';

export type IResultRuleFilter = (results: ScanResults, rules: string[]) => ScanResults;
export type IResultRuleFilter = (
results: ScanResults | undefined,
rules: string[] | null,
) => ScanResults;

export class BatchedRuleAnalyzer extends RuleAnalyzer {
private static batchConfigs: RuleAnalyzerConfiguration[] = [];
Expand Down Expand Up @@ -42,7 +45,8 @@ export class BatchedRuleAnalyzer extends RuleAnalyzer {
BatchedRuleAnalyzer.batchConfigs.push(config);
}

protected getRulesToRun(): string[] {
// null implies "the scanner's default rule set"
protected getRulesToRun(): string[] | null {
return null;
}

Expand Down
5 changes: 3 additions & 2 deletions src/injected/analyzers/rule-analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class RuleAnalyzer extends BaseAnalyzer {
const exclude = scopingState[ScopingInputTypes.exclude];

const scanOptions: ScanOptions = {
testsToRun: this.getRulesToRun(),
testsToRun: this.getRulesToRun() ?? undefined,
include: include,
exclude: exclude,
};
Expand All @@ -65,7 +65,8 @@ export class RuleAnalyzer extends BaseAnalyzer {
});
};

protected getRulesToRun(): string[] {
// null implies "the scanner's default rule set"
protected getRulesToRun(): string[] | null {
return this.config.rules;
}

Expand Down
2 changes: 1 addition & 1 deletion src/injected/analyzers/tab-stops-analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export class TabStopsAnalyzer extends BaseAnalyzer {
scanIncompleteWarningDetector: ScanIncompleteWarningDetector,
logger: Logger,
private readonly tabStopsDoneAnalyzingTracker: TabStopsDoneAnalyzingTracker,
private readonly tabStopsRequirementResultProcessor: TabStopsRequirementResultProcessor,
private readonly tabStopsRequirementResultProcessor: TabStopsRequirementResultProcessor | null,
private readonly debounceImpl: typeof debounce = debounce,
) {
super(config, sendMessageDelegate, scanIncompleteWarningDetector, logger);
Expand Down
4 changes: 2 additions & 2 deletions src/injected/components/command-bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export const CommandBar = NamedFC<CommandBarProps>('CommandBar', props => {
);
};

const renderInspectMessage = (): JSX.Element => {
const renderInspectMessage = (): JSX.Element | undefined => {
if (props.shouldShowInspectButtonMessage()) {
return (
<div role="alert" className="insights-dialog-inspect-disabled">
Expand All @@ -107,7 +107,7 @@ export const CommandBar = NamedFC<CommandBarProps>('CommandBar', props => {
}
};

const renderCopyIssueDetailsMessage = (): JSX.Element => {
const renderCopyIssueDetailsMessage = (): JSX.Element | undefined => {
if (props.shouldShowInsecureOriginPageMessage) {
return (
<div role="alert" className="copy-issue-details-button-help">
Expand Down
38 changes: 24 additions & 14 deletions src/injected/components/details-dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -206,27 +206,33 @@ export class DetailsDialog extends React.Component<DetailsDialogProps, DetailsDi
);
}

private renderRuleName(rule: DecoratedAxeNodeResult): JSX.Element {
const fixUrl = (url: string) => {
if (url.indexOf('://') >= 0) {
return url;
} else {
const { browserAdapter } = this.props.deps;
return browserAdapter.getUrl(url);
}
};
private renderRuleLink(rule: DecoratedAxeNodeResult): JSX.Element {
const url = rule.helpUrl;
if (url == null) {
return <>rule.ruleId</>;
}

let sanitizedUrl = url;
if (url.indexOf('://') < 0) {
const { browserAdapter } = this.props.deps;
sanitizedUrl = browserAdapter.getUrl(url);
}

return <NewTabLink href={sanitizedUrl}>{rule.ruleId}</NewTabLink>;
}

private renderRuleName(rule: DecoratedAxeNodeResult): JSX.Element {
const ruleNameID = 'rule-name';

return (
<section className="insights-dialog-rule-name" aria-labelledby={ruleNameID}>
{this.renderSectionTitle('Rule name', ruleNameID)}
<NewTabLink href={fixUrl(rule.helpUrl)}>{rule.ruleId}</NewTabLink>
{this.renderRuleLink(rule)}
</section>
);
}

private renderSuccessCriteria(ruleGuidanceLinks: HyperlinkDefinition[]): JSX.Element {
private renderSuccessCriteria(ruleGuidanceLinks: HyperlinkDefinition[]): JSX.Element | null {
if (isEmpty(ruleGuidanceLinks)) {
return null;
}
Expand Down Expand Up @@ -257,18 +263,22 @@ export class DetailsDialog extends React.Component<DetailsDialogProps, DetailsDi
}

private renderFixInstructions(ruleResult: DecoratedAxeNodeResult): JSX.Element {
const allChecks = ruleResult.all ?? [];
const noneChecks = ruleResult.none ?? [];
const anyChecks = ruleResult.any ?? [];

return (
<div className="insights-dialog-fix-instruction-container">
<FixInstructionPanel
deps={this.props.deps}
checkType={CheckType.All}
checks={ruleResult.all.concat(ruleResult.none)}
checks={allChecks.concat(noneChecks)}
renderTitleElement={this.renderSectionTitle}
/>
<FixInstructionPanel
deps={this.props.deps}
checkType={CheckType.Any}
checks={ruleResult.any}
checks={anyChecks}
renderTitleElement={this.renderSectionTitle}
/>
</div>
Expand All @@ -279,7 +289,7 @@ export class DetailsDialog extends React.Component<DetailsDialogProps, DetailsDi
return (
<div className="insights-dialog-content">
{this.renderRuleName(rule)}
{this.renderSuccessCriteria(rule.guidanceLinks)}
{this.renderSuccessCriteria(rule.guidanceLinks ?? [])}
{this.renderPathSelector()}
{this.renderCommandBar()}
{this.renderFixInstructions(rule)}
Expand Down
2 changes: 1 addition & 1 deletion src/injected/details-dialog-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export class DetailsDialogHandler {
return;
}

let parentLayer = dialogContainer;
let parentLayer: HTMLElement | null = dialogContainer;

while (parentLayer != null) {
if (parentLayer.classList.contains('ms-Layer--fixed')) {
Expand Down
6 changes: 3 additions & 3 deletions src/injected/main-window-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export class MainWindowContext {
toolData: ToolData,
issueFilingServiceProvider: IssueFilingServiceProvider,
): void {
window.mainWindowContext = new MainWindowContext(
window['mainWindowContext'] = new MainWindowContext(
devToolStore,
userConfigStore,
devToolActionMessageCreator,
Expand All @@ -77,9 +77,9 @@ export class MainWindowContext {
}

public static fromWindow(windowObj: Window): MainWindowContext {
if (windowObj.mainWindowContext == null) {
if (windowObj['mainWindowContext'] == null) {
throw new Error('No window.mainWindowContext found');
}
return windowObj.mainWindowContext;
return windowObj['mainWindowContext'];
}
}
7 changes: 2 additions & 5 deletions src/injected/visualization/drawer-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { TableHeadersAttributeFormatter } from 'injected/visualization/table-hea
import { BrowserAdapter } from '../../common/browser-adapters/browser-adapter';
import { HTMLElementUtils } from '../../common/html-element-utils';
import { TabbableElementsHelper } from '../../common/tabbable-elements-helper';
import { DeepPartial } from '../../common/types/deep-partial';
import { WindowUtils } from '../../common/window-utils';
import { ClientUtils } from '../client-utils';
import { DetailsDialogHandler } from '../details-dialog-handler';
Expand All @@ -19,7 +18,7 @@ import { CenterPositionCalculator } from './center-position-calculator';
import { CustomWidgetsFormatter } from './custom-widgets-formatter';
import { Drawer } from './drawer';
import { DrawerUtils } from './drawer-utils';
import { Formatter, SVGDrawerConfiguration } from './formatter';
import { Formatter, IPartialSVGDrawerConfiguration } from './formatter';
import { FrameFormatter } from './frame-formatter';
import { HeadingFormatter } from './heading-formatter';
import { HighlightBoxDrawer } from './highlight-box-drawer';
Expand All @@ -35,8 +34,6 @@ import { SVGShapeFactory } from './svg-shape-factory';
import { SVGSolidShadowFilterFactory } from './svg-solid-shadow-filter-factory';
import { TabStopsFormatter } from './tab-stops-formatter';

export type IPartialSVGDrawerConfiguration = DeepPartial<SVGDrawerConfiguration>;

export class DrawerProvider {
constructor(
private readonly htmlElementUtils: HTMLElementUtils,
Expand All @@ -60,7 +57,7 @@ export class DrawerProvider {
return new SingleTargetDrawer(this.drawerUtils, new SingleTargetFormatter(className));
}

public createSVGDrawer(config: IPartialSVGDrawerConfiguration = null): Drawer {
public createSVGDrawer(config: IPartialSVGDrawerConfiguration | null = null): Drawer {
const tabbableElementsHelper = new TabbableElementsHelper(this.htmlElementUtils);
const centerPositionCalculator = new CenterPositionCalculator(
this.drawerUtils,
Expand Down
3 changes: 3 additions & 0 deletions src/injected/visualization/formatter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { DeepPartial } from 'common/types/deep-partial';
import { BoundingRect } from '../bounding-rect';
import { DialogRenderer } from '../dialog-renderer';
import { AxeResultsWithFrameLevel } from '../frameCommunicators/html-element-axe-results-helper';
Expand Down Expand Up @@ -72,6 +73,8 @@ export interface SVGDrawerConfiguration {
failureBoxConfig: FailureBoxConfig;
}

export type IPartialSVGDrawerConfiguration = DeepPartial<SVGDrawerConfiguration>;

export interface SingleTargetDrawerConfiguration {
injectedClassName: string;
}
Expand Down
2 changes: 1 addition & 1 deletion src/injected/visualization/highlight-box-formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export class HighlightBoxFormatter extends FailureInstanceFormatter {
super();
}

public getDialogRenderer(): DialogRenderer {
public getDialogRenderer(): DialogRenderer | null {
return null;
}

Expand Down
Loading

0 comments on commit 0301cfe

Please sign in to comment.