From 14148acf214ea798d3c6ddbd93209f054722dc9b Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Wed, 20 Mar 2019 19:23:28 -0700 Subject: [PATCH] Fix some sneaky strict null issues for array/object access TypeScript always assumes that array index access and object index type accesses return a non-null value. This was causing a few strict null issues to be masked where we were returning `array[i] || null` but the return type was not marked as nullable The fix is to use `Map` types instead as these have the proper types --- src/vs/editor/common/modes.ts | 4 +-- .../modes/languageConfigurationRegistry.ts | 20 +++++-------- .../common/modes/tokenizationRegistry.ts | 29 +++++++++---------- .../common/modes/textToHtmlTokenizer.test.ts | 4 +-- .../electron-browser/releaseNotesEditor.ts | 2 +- 5 files changed, 27 insertions(+), 32 deletions(-) diff --git a/src/vs/editor/common/modes.ts b/src/vs/editor/common/modes.ts index d6008024b92..0b61c767d7c 100644 --- a/src/vs/editor/common/modes.ts +++ b/src/vs/editor/common/modes.ts @@ -1565,9 +1565,9 @@ export interface ITokenizationRegistry { /** * Get the tokenization support for a language. - * Returns null if not found. + * Returns `null` if not found. */ - get(language: string): ITokenizationSupport; + get(language: string): ITokenizationSupport | null; /** * Get the promise of a tokenization support for a language. diff --git a/src/vs/editor/common/modes/languageConfigurationRegistry.ts b/src/vs/editor/common/modes/languageConfigurationRegistry.ts index 8bea5db7e8d..713df07ca0d 100644 --- a/src/vs/editor/common/modes/languageConfigurationRegistry.ts +++ b/src/vs/editor/common/modes/languageConfigurationRegistry.ts @@ -57,7 +57,7 @@ export class RichEditSupport { public readonly indentationRules: IndentationRule | undefined; public readonly foldingRules: FoldingRules; - constructor(languageIdentifier: LanguageIdentifier, previous: RichEditSupport, rawConf: LanguageConfiguration) { + constructor(languageIdentifier: LanguageIdentifier, previous: RichEditSupport | undefined, rawConf: LanguageConfiguration) { this._languageIdentifier = languageIdentifier; this._brackets = null; @@ -175,34 +175,30 @@ export class LanguageConfigurationChangeEvent { export class LanguageConfigurationRegistryImpl { - private readonly _entries: RichEditSupport[]; + private readonly _entries = new Map(); private readonly _onDidChange = new Emitter(); public readonly onDidChange: Event = this._onDidChange.event; - constructor() { - this._entries = []; - } - public register(languageIdentifier: LanguageIdentifier, configuration: LanguageConfiguration): IDisposable { let previous = this._getRichEditSupport(languageIdentifier.id); let current = new RichEditSupport(languageIdentifier, previous, configuration); - this._entries[languageIdentifier.id] = current; + this._entries.set(languageIdentifier.id, current); this._onDidChange.fire({ languageIdentifier }); return toDisposable(() => { - if (this._entries[languageIdentifier.id] === current) { - this._entries[languageIdentifier.id] = previous; + if (this._entries.get(languageIdentifier.id) === current) { + this._entries.set(languageIdentifier.id, previous); this._onDidChange.fire({ languageIdentifier }); } }); } - private _getRichEditSupport(languageId: LanguageId): RichEditSupport { - return this._entries[languageId] || null; + private _getRichEditSupport(languageId: LanguageId): RichEditSupport | undefined { + return this._entries.get(languageId); } public getIndentationRules(languageId: LanguageId) { - let value = this._entries[languageId]; + const value = this._entries.get(languageId); if (!value) { return null; diff --git a/src/vs/editor/common/modes/tokenizationRegistry.ts b/src/vs/editor/common/modes/tokenizationRegistry.ts index 82ad12da605..3d5d0097cb5 100644 --- a/src/vs/editor/common/modes/tokenizationRegistry.ts +++ b/src/vs/editor/common/modes/tokenizationRegistry.ts @@ -7,11 +7,12 @@ import { Color } from 'vs/base/common/color'; import { Emitter, Event } from 'vs/base/common/event'; import { IDisposable, toDisposable } from 'vs/base/common/lifecycle'; import { ColorId, ITokenizationRegistry, ITokenizationSupport, ITokenizationSupportChangedEvent } from 'vs/editor/common/modes'; +import { withUndefinedAsNull } from 'vs/base/common/types'; export class TokenizationRegistryImpl implements ITokenizationRegistry { - private readonly _map: { [language: string]: ITokenizationSupport }; - private readonly _promises: { [language: string]: Thenable }; + private readonly _map = new Map(); + private readonly _promises = new Map>(); private readonly _onDidChange = new Emitter(); public readonly onDidChange: Event = this._onDidChange.event; @@ -19,8 +20,6 @@ export class TokenizationRegistryImpl implements ITokenizationRegistry { private _colorMap: Color[] | null; constructor() { - this._map = Object.create(null); - this._promises = Object.create(null); this._colorMap = null; } @@ -32,13 +31,13 @@ export class TokenizationRegistryImpl implements ITokenizationRegistry { } public register(language: string, support: ITokenizationSupport) { - this._map[language] = support; + this._map.set(language, support); this.fire([language]); return toDisposable(() => { - if (this._map[language] !== support) { + if (this._map.get(language) !== support) { return; } - delete this._map[language]; + this._map.delete(language); this.fire([language]); }); } @@ -48,13 +47,13 @@ export class TokenizationRegistryImpl implements ITokenizationRegistry { let registration: IDisposable | null = null; let isDisposed: boolean = false; - this._promises[language] = supportPromise.then(support => { - delete this._promises[language]; + this._promises.set(language, supportPromise.then(support => { + this._promises.delete(language); if (isDisposed || !support) { return; } registration = this.register(language, support); - }); + })); return toDisposable(() => { isDisposed = true; @@ -69,21 +68,21 @@ export class TokenizationRegistryImpl implements ITokenizationRegistry { if (support) { return Promise.resolve(support); } - const promise = this._promises[language]; + const promise = this._promises.get(language); if (promise) { - return promise.then(_ => this.get(language)); + return promise.then(_ => this.get(language)!); } return null; } - public get(language: string): ITokenizationSupport { - return (this._map[language] || null); + public get(language: string): ITokenizationSupport | null { + return withUndefinedAsNull(this._map.get(language)); } public setColorMap(colorMap: Color[]): void { this._colorMap = colorMap; this._onDidChange.fire({ - changedLanguages: Object.keys(this._map), + changedLanguages: Array.from(this._map.keys()), changedColorMap: true }); } diff --git a/src/vs/editor/test/common/modes/textToHtmlTokenizer.test.ts b/src/vs/editor/test/common/modes/textToHtmlTokenizer.test.ts index d7efc51bf89..781ea1bb6a5 100644 --- a/src/vs/editor/test/common/modes/textToHtmlTokenizer.test.ts +++ b/src/vs/editor/test/common/modes/textToHtmlTokenizer.test.ts @@ -18,7 +18,7 @@ suite('Editor Modes - textToHtmlTokenizer', () => { test('TextToHtmlTokenizer 1', () => { let mode = new Mode(); - let support = TokenizationRegistry.get(mode.getId()); + let support = TokenizationRegistry.get(mode.getId())!; let actual = tokenizeToString('.abc..def...gh', support); let expected = [ @@ -38,7 +38,7 @@ suite('Editor Modes - textToHtmlTokenizer', () => { test('TextToHtmlTokenizer 2', () => { let mode = new Mode(); - let support = TokenizationRegistry.get(mode.getId()); + let support = TokenizationRegistry.get(mode.getId())!; let actual = tokenizeToString('.abc..def...gh\n.abc..def...gh', support); let expected1 = [ diff --git a/src/vs/workbench/contrib/update/electron-browser/releaseNotesEditor.ts b/src/vs/workbench/contrib/update/electron-browser/releaseNotesEditor.ts index 13292e55484..fa0ae8abbe9 100644 --- a/src/vs/workbench/contrib/update/electron-browser/releaseNotesEditor.ts +++ b/src/vs/workbench/contrib/update/electron-browser/releaseNotesEditor.ts @@ -209,7 +209,7 @@ export class ReleaseNotesManager { renderer.code = (code, lang) => { const modeId = this._modeService.getModeIdForLanguageName(lang); - return `${tokenizeToString(code, modeId ? TokenizationRegistry.get(modeId) : undefined)}`; + return `${tokenizeToString(code, modeId ? TokenizationRegistry.get(modeId)! : undefined)}`; }; return renderer; } -- GitLab