From f6c63f2dcb7f7e7dbf6f626ea9cee59bcd26eaff Mon Sep 17 00:00:00 2001 From: Anthony Dresser Date: Tue, 8 Sep 2020 15:28:39 -0700 Subject: [PATCH] strict null chart (#12167) --- .../contrib/charts/browser/actions.ts | 14 ++-- .../contrib/charts/browser/chartView.ts | 80 +++++++++---------- .../contrib/charts/browser/countInsight.ts | 4 +- .../contrib/charts/browser/graphInsight.ts | 35 ++++---- .../contrib/charts/browser/insight.ts | 52 ++++++------ .../contrib/charts/browser/tableInsight.ts | 8 +- src/tsconfig.vscode.json | 1 - 7 files changed, 100 insertions(+), 94 deletions(-) diff --git a/src/sql/workbench/contrib/charts/browser/actions.ts b/src/sql/workbench/contrib/charts/browser/actions.ts index d4f1caf9e7..07bbcc1d70 100644 --- a/src/sql/workbench/contrib/charts/browser/actions.ts +++ b/src/sql/workbench/contrib/charts/browser/actions.ts @@ -46,18 +46,18 @@ export class CreateInsightAction extends Action { } public run(context: IChartActionContext): Promise { - let uriString: string = this.getActiveUriString(); + let uriString = this.getActiveUriString(); if (!uriString) { this.showError(localize('createInsightNoEditor', "Cannot create insight as the active editor is not a SQL Editor")); return Promise.resolve(false); } let uri: URI = URI.parse(uriString); - let queryFile: string = uri.fsPath; - let query: string = undefined; - let type = {}; + let queryFile = uri.fsPath; + let query: string | undefined = undefined; + let type: { [key: string]: any } = {}; let options = assign({}, context.options); - delete options.type; + delete (options as any).type; type[context.options.type] = options; // create JSON let config: IInsightsConfig = { @@ -92,7 +92,7 @@ export class CreateInsightAction extends Action { ); } - private getActiveUriString(): string { + private getActiveUriString(): string | undefined { let editor = this.editorService.activeEditor; if (editor instanceof QueryEditorInput) { return editor.uri; @@ -113,7 +113,7 @@ export class ConfigureChartAction extends Action { public static LABEL = localize('configureChartLabel', "Configure Chart"); public static ICON = 'settings'; - private dialog: ConfigureChartDialog; + private dialog?: ConfigureChartDialog; constructor(private _chart: ChartView, @IInstantiationService private readonly instantiationService: IInstantiationService) { diff --git a/src/sql/workbench/contrib/charts/browser/chartView.ts b/src/sql/workbench/contrib/charts/browser/chartView.ts index 5ccca7d4c8..b6a65e76e7 100644 --- a/src/sql/workbench/contrib/charts/browser/chartView.ts +++ b/src/sql/workbench/contrib/charts/browser/chartView.ts @@ -27,7 +27,6 @@ import { Checkbox } from 'sql/base/browser/ui/checkbox/checkbox'; import { IInsightOptions, ChartType } from 'sql/workbench/contrib/charts/common/interfaces'; import { ChartState } from 'sql/workbench/common/editor/query/chartState'; import * as nls from 'vs/nls'; -import { find } from 'vs/base/common/arrays'; import { INotificationService } from 'vs/platform/notification/common/notification'; import { Event, Emitter } from 'vs/base/common/event'; @@ -49,38 +48,38 @@ const altNameHash: { [oldName: string]: string } = { }; export class ChartView extends Disposable implements IPanelView { - private insight: Insight; - private _queryRunner: QueryRunner; - private _data: IInsightData; - private _currentData: { batchId: number, resultId: number }; + private insight?: Insight; + private _queryRunner?: QueryRunner; + private _data?: IInsightData; + private _currentData?: { batchId: number, resultId: number }; private taskbar: Taskbar; - private _createInsightAction: CreateInsightAction; + private _createInsightAction?: CreateInsightAction; private _copyAction: CopyAction; private _saveAction: SaveImageAction; - private _configureChartAction: ConfigureChartAction; + private _configureChartAction?: ConfigureChartAction; - private _state: ChartState; + private _state?: ChartState; private _options: IInsightOptions = { type: ChartType.Bar }; /** parent container */ - private container: HTMLElement; + private container?: HTMLElement; /** container for the options controls */ public readonly optionsControl: HTMLElement; /** container for type specific controls */ private typeControls: HTMLElement; /** container for the insight */ - private insightContainer: HTMLElement; + private insightContainer?: HTMLElement; /** container for the action bar */ private taskbarContainer: HTMLElement; /** container for the charting (includes insight and options) */ - private chartingContainer: HTMLElement; + private chartingContainer?: HTMLElement; private optionDisposables: IDisposable[] = []; - private optionMap: { [x: string]: { element: HTMLElement; set: (val) => void } } = {}; + private optionMap: { [x: string]: { element: HTMLElement; set: ((val: string) => void) | ((val: number) => void) | ((val: boolean) => void) } } = {}; private readonly _onOptionsChange: Emitter = this._register(new Emitter()); public readonly onOptionsChange: Event = this._onOptionsChange.event; @@ -115,19 +114,19 @@ export class ChartView extends Disposable implements IPanelView { const self = this; this._options = new Proxy(this._options, { - get: function (target, key) { + get: function (target, key: keyof IInsightOptions) { return target[key]; }, - set: function (target, key, value) { + set: function (target, key: keyof IInsightOptions, value: boolean | number | string) { let change = false; if (target[key] !== value) { change = true; } - target[key] = value; + (target[key] as any) = value; // mirror the change in our state if (self.state) { - self.state.options[key] = value; + (self.state.options[key] as any) = value; } if (change) { @@ -186,9 +185,9 @@ export class ChartView extends Disposable implements IPanelView { container.appendChild(this.container); if (this._data) { - this.insight.data = this._data; + this.insight!.data = this._data; } else { - this.queryRunner = this._queryRunner; + this.queryRunner = this._queryRunner!; } this.verifyOptions(); } @@ -201,7 +200,7 @@ export class ChartView extends Disposable implements IPanelView { layout(dimension: DOM.Dimension): void { if (this.insight) { - this.insight.layout(new DOM.Dimension(DOM.getContentWidth(this.insightContainer), DOM.getContentHeight(this.insightContainer))); + this.insight.layout(new DOM.Dimension(DOM.getContentWidth(this.insightContainer!), DOM.getContentHeight(this.insightContainer!))); } } @@ -276,7 +275,7 @@ export class ChartView extends Disposable implements IPanelView { this.updateActionbar(); for (let key in this.optionMap) { if (this.optionMap.hasOwnProperty(key)) { - let option = find(this.getChartTypeOptions(), e => e.configEntry === key); + let option = this.getChartTypeOptions().find(e => e.configEntry === key); if (option && option.if) { if (option.if(this._options)) { DOM.show(this.optionMap[key].element); @@ -322,8 +321,9 @@ export class ChartView extends Disposable implements IPanelView { const optionInput = DOM.$('div.option-input'); optionContainer.appendChild(label); optionContainer.appendChild(optionInput); - let setFunc: (val) => void; - let value = this.state ? this.state.options[option.configEntry] || option.default : option.default; + let setFunc: ((val: string) => void) | ((val: number) => void) | ((val: boolean) => void); + let entry = option.configEntry as keyof IInsightOptions; + let value = this.state ? this.state.options[entry] || option.default : option.default; switch (option.type) { case ControlType.checkbox: let checkbox = new Checkbox(optionInput, { @@ -331,8 +331,8 @@ export class ChartView extends Disposable implements IPanelView { ariaLabel: option.label, checked: value, onChange: () => { - if (this._options[option.configEntry] !== checkbox.checked) { - this._options[option.configEntry] = checkbox.checked; + if (this._options[entry] !== checkbox.checked) { + (this._options[entry] as any) = checkbox.checked; if (this.insight) { this.insight.options = this._options; } @@ -345,13 +345,13 @@ export class ChartView extends Disposable implements IPanelView { break; case ControlType.combo: //pass options into changeAltNames in order for SelectBox to show user-friendly names. - let dropdown = new SelectBox(option.displayableOptions || this.changeToAltNames(option.options), undefined, this._contextViewService); + let dropdown = new SelectBox(option.displayableOptions || this.changeToAltNames(option.options!), undefined!, this._contextViewService); dropdown.setAriaLabel(option.label); - dropdown.select(option.options.indexOf(value)); + dropdown.select(option.options!.indexOf(value)); dropdown.render(optionInput); dropdown.onDidSelect(e => { - if (this._options[option.configEntry] !== option.options[e.index]) { - this._options[option.configEntry] = option.options[e.index]; + if (this._options[entry] !== option.options![e.index]) { + (this._options[entry] as any) = option.options![e.index]; if (this.insight) { this.insight.options = this._options; } @@ -359,7 +359,7 @@ export class ChartView extends Disposable implements IPanelView { }); setFunc = (val: string) => { if (!isUndefinedOrNull(val)) { - dropdown.select(option.options.indexOf(val)); + dropdown.select(option.options!.indexOf(val)); } }; this.optionDisposables.push(attachSelectBoxStyler(dropdown, this._themeService)); @@ -369,8 +369,8 @@ export class ChartView extends Disposable implements IPanelView { input.setAriaLabel(option.label); input.value = value || ''; input.onDidChange(e => { - if (this._options[option.configEntry] !== e) { - this._options[option.configEntry] = e; + if (this._options[entry] !== e) { + (this._options[entry] as any) = e; if (this.insight) { this.insight.options = this._options; } @@ -388,8 +388,8 @@ export class ChartView extends Disposable implements IPanelView { numberInput.setAriaLabel(option.label); numberInput.value = value || ''; numberInput.onDidChange(e => { - if (this._options[option.configEntry] !== Number(e)) { - this._options[option.configEntry] = Number(e); + if (this._options[entry] !== Number(e)) { + (this._options[entry] as any) = Number(e); if (this.insight) { this.insight.options = this._options; } @@ -407,8 +407,8 @@ export class ChartView extends Disposable implements IPanelView { dateInput.setAriaLabel(option.label); dateInput.value = value || ''; dateInput.onDidChange(e => { - if (this._options[option.configEntry] !== e) { - this._options[option.configEntry] = e; + if (this._options[entry] !== e) { + (this._options[entry] as any) = e; if (this.insight) { this.insight.options = this._options; } @@ -422,9 +422,9 @@ export class ChartView extends Disposable implements IPanelView { this.optionDisposables.push(attachInputBoxStyler(dateInput, this._themeService)); break; } - this.optionMap[option.configEntry] = { element: optionContainer, set: setFunc }; + this.optionMap[entry] = { element: optionContainer, set: setFunc }; container.appendChild(optionContainer); - this._options[option.configEntry] = value; + (this._options[entry] as any) = value; } public set state(val: ChartState) { @@ -436,7 +436,7 @@ export class ChartView extends Disposable implements IPanelView { } public get state(): ChartState { - return this._state; + return this._state!; } public get options(): IInsightOptions { @@ -447,8 +447,8 @@ export class ChartView extends Disposable implements IPanelView { if (newOptions) { for (let key in newOptions) { if (newOptions.hasOwnProperty(key) && this.optionMap[key]) { - this._options[key] = newOptions[key]; - this.optionMap[key].set(newOptions[key]); + (this._options[key as keyof IInsightOptions] as any) = newOptions[key as keyof IInsightOptions]!; + (this.optionMap[key] as any).set(newOptions[key as keyof IInsightOptions]); } } } diff --git a/src/sql/workbench/contrib/charts/browser/countInsight.ts b/src/sql/workbench/contrib/charts/browser/countInsight.ts index 58bda17468..029d986a3f 100644 --- a/src/sql/workbench/contrib/charts/browser/countInsight.ts +++ b/src/sql/workbench/contrib/charts/browser/countInsight.ts @@ -8,11 +8,11 @@ import 'vs/css!./media/countInsight'; import { IInsight } from './interfaces'; import { $, clearNode } from 'vs/base/browser/dom'; -import { InsightType } from 'sql/workbench/contrib/charts/common/interfaces'; +import { IInsightOptions, InsightType } from 'sql/workbench/contrib/charts/common/interfaces'; import { IInsightData } from 'sql/platform/dashboard/browser/insightRegistry'; export class CountInsight implements IInsight { - public options; + public options: IInsightOptions = { type: InsightType.Count }; public static readonly types = [InsightType.Count]; public readonly types = CountInsight.types; diff --git a/src/sql/workbench/contrib/charts/browser/graphInsight.ts b/src/sql/workbench/contrib/charts/browser/graphInsight.ts index 4212bf78ac..6160878a62 100644 --- a/src/sql/workbench/contrib/charts/browser/graphInsight.ts +++ b/src/sql/workbench/contrib/charts/browser/graphInsight.ts @@ -14,7 +14,6 @@ import { IThemeService, IColorTheme } from 'vs/platform/theme/common/themeServic import { IInsight, IPointDataSet, customMixin } from './interfaces'; import { IInsightOptions, DataDirection, ChartType, LegendPosition, DataType } from 'sql/workbench/contrib/charts/common/interfaces'; import { values } from 'vs/base/common/collections'; -import { find } from 'vs/base/common/arrays'; import { IInsightData } from 'sql/platform/dashboard/browser/insightRegistry'; const noneLineGraphs = [ChartType.Doughnut, ChartType.Pie]; @@ -43,12 +42,12 @@ const defaultOptions: IInsightOptions = { }; export class Graph implements IInsight { - private _options: IInsightOptions; + private _options: IInsightOptions = { type: ChartType.Bar }; private canvas: HTMLCanvasElement; - private chartjs: chartjs; - private _data: IInsightData; + private chartjs?: chartjs; + private _data?: IInsightData; - private originalType: ChartType; + private originalType?: ChartType; public static readonly types = [ChartType.Bar, ChartType.Doughnut, ChartType.HorizontalBar, ChartType.Line, ChartType.Pie, ChartType.Scatter, ChartType.TimeSeries]; public readonly types = Graph.types; @@ -62,7 +61,9 @@ export class Graph implements IInsight { this._theme = themeService.getColorTheme(); themeService.onDidColorThemeChange(e => { this._theme = e; - this.data = this._data; + if (this._data) { + this.data = this._data; + } }); this.options = mixin(options, defaultOptions, false); @@ -84,8 +85,8 @@ export class Graph implements IInsight { } - public getCanvasData(): string { - return this.chartjs.toBase64Image(); + public getCanvasData(): string | undefined { + return this.chartjs && this.chartjs.toBase64Image(); } public set data(data: IInsightData) { @@ -155,7 +156,7 @@ export class Graph implements IInsight { } chartData = chartData.map((c, i) => { - return mixin(c, getColors(this.options.type, i, c.data.length), false); + return mixin(c, getColors(this.options.type, i, c.data!.length), false); }); if (this.chartjs) { @@ -166,7 +167,7 @@ export class Graph implements IInsight { this.chartjs.options = this.transformOptions(this.options); this.chartjs.update({ duration: 0 }); } else { - this.chartjs = new chartjs.Chart(this.canvas.getContext('2d'), { + this.chartjs = new chartjs.Chart(this.canvas.getContext('2d')!, { data: { // we don't want to include lables for timeSeries labels: this.originalType === 'timeSeries' ? [] : labels, @@ -183,16 +184,16 @@ export class Graph implements IInsight { retval.maintainAspectRatio = false; let foregroundColor = this._theme.getColor(colors.editorForeground); - let foreground = foregroundColor ? foregroundColor.toString() : null; + let foreground = foregroundColor ? foregroundColor.toString() : undefined; let gridLinesColor = this._theme.getColor(editorLineNumbers); - let gridLines = gridLinesColor ? gridLinesColor.toString() : null; + let gridLines = gridLinesColor ? gridLinesColor.toString() : undefined; let backgroundColor = this._theme.getColor(colors.editorBackground); - let background = backgroundColor ? backgroundColor.toString() : null; + let background = backgroundColor ? backgroundColor.toString() : undefined; if (options) { retval.scales = {}; // we only want to include axis if it is a axis based graph type - if (!find(noneLineGraphs, x => x === options.type as ChartType)) { + if (!noneLineGraphs.find(x => x === options.type as ChartType)) { retval.scales.xAxes = [{ scaleLabel: { fontColor: foreground, @@ -215,7 +216,7 @@ export class Graph implements IInsight { retval.scales = mixin(retval.scales, { xAxes: [{ ticks: { min: options.xAxisMin } }] }, true, customMixin); } - retval.scales.yAxes = [{ + retval.scales!.yAxes = [{ scaleLabel: { fontColor: foreground, labelString: options.yAxisLabel, @@ -290,7 +291,9 @@ export class Graph implements IInsight { this.options.dataType = DataType.Point; this.options.dataDirection = DataDirection.Horizontal; } - this.data = this._data; + if (this._data) { + this.data = this._data; + } } public get options(): IInsightOptions { diff --git a/src/sql/workbench/contrib/charts/browser/insight.ts b/src/sql/workbench/contrib/charts/browser/insight.ts index 52f21f7b8d..de9533ee78 100644 --- a/src/sql/workbench/contrib/charts/browser/insight.ts +++ b/src/sql/workbench/contrib/charts/browser/insight.ts @@ -13,7 +13,6 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { Dimension, clearNode } from 'vs/base/browser/dom'; import { deepClone } from 'vs/base/common/objects'; import { IInsightOptions, ChartType, DataDirection, InsightType } from 'sql/workbench/contrib/charts/common/interfaces'; -import { find } from 'vs/base/common/arrays'; import { IInsightData } from 'sql/platform/dashboard/browser/insightRegistry'; const defaultOptions: IInsightOptions = { @@ -22,15 +21,15 @@ const defaultOptions: IInsightOptions = { }; export class Insight { - private _insight: IInsight; + private _insight?: IInsight; - public get insight(): IInsight { + public get insight(): IInsight | undefined { return this._insight; } - private _options: IInsightOptions; - private _data: IInsightData; - private dim: Dimension; + private _options?: IInsightOptions; + private _data?: IInsightData; + private dim?: Dimension; constructor( private container: HTMLElement, options: IInsightOptions = defaultOptions, @@ -42,14 +41,16 @@ export class Insight { public layout(dim: Dimension) { this.dim = dim; - this.insight.layout(dim); + if (this.insight) { + this.insight.layout(dim); + } } - public set options(val: IInsightOptions) { + public set options(val: IInsightOptions | undefined) { this._options = deepClone(val); - if (this.insight) { + if (this.insight && this.options) { // check to see if we need to change the insight type - if (!find(this.insight.types, x => x === this.options.type)) { + if (!this.insight.types.find(x => x === this.options!.type)) { this.buildInsight(); } else { this.insight.options = this.options; @@ -57,7 +58,7 @@ export class Insight { } } - public get options(): IInsightOptions { + public get options(): IInsightOptions | undefined { return this._options; } @@ -75,28 +76,33 @@ export class Insight { clearNode(this.container); - let ctor = this.findctor(this.options.type); + if (this.options) { - if (ctor) { - this._insight = this._instantiationService.createInstance(ctor, this.container, this.options); - this.insight.layout(this.dim); - if (this._data) { - this.insight.data = this._data; + const ctor = this.findctor(this.options.type); + + if (ctor) { + this._insight = this._instantiationService.createInstance(ctor, this.container, this.options); + if (this.dim) { + this.insight!.layout(this.dim); + } + if (this._data) { + this.insight!.data = this._data; + } } } } public get isCopyable(): boolean { - return !!find(Graph.types, x => x === this.options.type as ChartType); + return !!this.options && !!Graph.types.find(x => x === this.options!.type as ChartType); } - private findctor(type: ChartType | InsightType): IInsightCtor { - if (find(Graph.types, x => x === type as ChartType)) { + private findctor(type: ChartType | InsightType): IInsightCtor | undefined { + if (Graph.types.find(x => x === type as ChartType)) { return Graph as IInsightCtor; - } else if (find(ImageInsight.types, x => x === type as InsightType)) { + } else if (ImageInsight.types.find(x => x === type as InsightType)) { return ImageInsight as IInsightCtor; - } else if (find(TableInsight.types, x => x === type as InsightType)) { + } else if (TableInsight.types.find(x => x === type as InsightType)) { return TableInsight as IInsightCtor; - } else if (find(CountInsight.types, x => x === type as InsightType)) { + } else if (CountInsight.types.find(x => x === type as InsightType)) { return CountInsight as IInsightCtor; } return undefined; diff --git a/src/sql/workbench/contrib/charts/browser/tableInsight.ts b/src/sql/workbench/contrib/charts/browser/tableInsight.ts index 1551252ae1..c2389296fa 100644 --- a/src/sql/workbench/contrib/charts/browser/tableInsight.ts +++ b/src/sql/workbench/contrib/charts/browser/tableInsight.ts @@ -12,7 +12,7 @@ import { CellSelectionModel } from 'sql/base/browser/ui/table/plugins/cellSelect import { $, Dimension } from 'vs/base/browser/dom'; import { Disposable } from 'vs/base/common/lifecycle'; import { IThemeService } from 'vs/platform/theme/common/themeService'; -import { InsightType } from 'sql/workbench/contrib/charts/common/interfaces'; +import { IInsightOptions, InsightType } from 'sql/workbench/contrib/charts/common/interfaces'; import { IInsightData } from 'sql/platform/dashboard/browser/insightRegistry'; export class TableInsight extends Disposable implements IInsight { @@ -21,7 +21,8 @@ export class TableInsight extends Disposable implements IInsight { private table: Table; private dataView: TableDataView; - private columns: Slick.Column[]; + private columns?: Slick.Column[]; + public options: IInsightOptions = { type: InsightType.Table }; constructor(container: HTMLElement, options: any, @IThemeService themeService: IThemeService @@ -47,9 +48,6 @@ export class TableInsight extends Disposable implements IInsight { layout(dim: Dimension) { this.table.layout(dim); } - - public options; - } function transformData(rows: string[][], columns: string[]): { [key: string]: string }[] { diff --git a/src/tsconfig.vscode.json b/src/tsconfig.vscode.json index ac9bdb344c..5fa2e6f5cd 100644 --- a/src/tsconfig.vscode.json +++ b/src/tsconfig.vscode.json @@ -62,7 +62,6 @@ "./sql/workbench/browser/**/*.ts", // 2646 errors "./sql/workbench/contrib/assessment/**/*.ts", // 62 errors "./sql/workbench/contrib/assessment/test/**/*.ts", - "./sql/workbench/contrib/charts/**/*.ts", // 63 errors "./sql/workbench/contrib/charts/test/**/*.ts", "./sql/workbench/contrib/commandLine/**/*.ts", // 292 errors "./sql/workbench/contrib/commandLine/test/**/*.ts",