Chore: Stricter typing in type guards (#77809)

* Stricter typing in type guards

* create utility isObject fn

* better isAlertStateWithReason

* better name

* restore "data !== undefined"
pull/76967/head
Josh Hunt 2 years ago committed by GitHub
parent 3509a5abb9
commit 4a3c148298
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 76
      .betterer.results
  2. 61
      packages/grafana-data/src/types/data.test.ts
  3. 24
      packages/grafana-data/src/types/data.ts
  4. 5
      packages/grafana-data/src/utils/OptionsUIBuilders.ts
  5. 22
      public/app/features/alerting/unified/api/alertmanager.ts
  6. 10
      public/app/features/alerting/unified/api/ruler.ts
  7. 6
      public/app/features/dashboard/components/DashExportModal/DashboardExporter.ts
  8. 9
      public/app/features/variables/state/actions.ts
  9. 7
      public/app/types/unified-alerting-dto.ts

@ -515,39 +515,38 @@ exports[`better eslint`] = {
[0, 0, 0, "Unexpected any. Specify a different type.", "58"], [0, 0, 0, "Unexpected any. Specify a different type.", "58"],
[0, 0, 0, "Unexpected any. Specify a different type.", "59"], [0, 0, 0, "Unexpected any. Specify a different type.", "59"],
[0, 0, 0, "Unexpected any. Specify a different type.", "60"], [0, 0, 0, "Unexpected any. Specify a different type.", "60"],
[0, 0, 0, "Unexpected any. Specify a different type.", "61"], [0, 0, 0, "Do not use any type assertions.", "61"],
[0, 0, 0, "Do not use any type assertions.", "62"], [0, 0, 0, "Unexpected any. Specify a different type.", "62"],
[0, 0, 0, "Unexpected any. Specify a different type.", "63"], [0, 0, 0, "Do not use any type assertions.", "63"],
[0, 0, 0, "Do not use any type assertions.", "64"], [0, 0, 0, "Unexpected any. Specify a different type.", "64"],
[0, 0, 0, "Unexpected any. Specify a different type.", "65"], [0, 0, 0, "Do not use any type assertions.", "65"],
[0, 0, 0, "Do not use any type assertions.", "66"], [0, 0, 0, "Unexpected any. Specify a different type.", "66"],
[0, 0, 0, "Unexpected any. Specify a different type.", "67"], [0, 0, 0, "Do not use any type assertions.", "67"],
[0, 0, 0, "Do not use any type assertions.", "68"], [0, 0, 0, "Unexpected any. Specify a different type.", "68"],
[0, 0, 0, "Unexpected any. Specify a different type.", "69"], [0, 0, 0, "Do not use any type assertions.", "69"],
[0, 0, 0, "Do not use any type assertions.", "70"], [0, 0, 0, "Unexpected any. Specify a different type.", "70"],
[0, 0, 0, "Unexpected any. Specify a different type.", "71"], [0, 0, 0, "Do not use any type assertions.", "71"],
[0, 0, 0, "Do not use any type assertions.", "72"], [0, 0, 0, "Unexpected any. Specify a different type.", "72"],
[0, 0, 0, "Unexpected any. Specify a different type.", "73"], [0, 0, 0, "Do not use any type assertions.", "73"],
[0, 0, 0, "Do not use any type assertions.", "74"], [0, 0, 0, "Unexpected any. Specify a different type.", "74"],
[0, 0, 0, "Unexpected any. Specify a different type.", "75"], [0, 0, 0, "Unexpected any. Specify a different type.", "75"],
[0, 0, 0, "Unexpected any. Specify a different type.", "76"], [0, 0, 0, "Do not use any type assertions.", "76"],
[0, 0, 0, "Do not use any type assertions.", "77"], [0, 0, 0, "Unexpected any. Specify a different type.", "77"],
[0, 0, 0, "Unexpected any. Specify a different type.", "78"], [0, 0, 0, "Unexpected any. Specify a different type.", "78"],
[0, 0, 0, "Unexpected any. Specify a different type.", "79"], [0, 0, 0, "Do not use any type assertions.", "79"],
[0, 0, 0, "Do not use any type assertions.", "80"], [0, 0, 0, "Unexpected any. Specify a different type.", "80"],
[0, 0, 0, "Unexpected any. Specify a different type.", "81"], [0, 0, 0, "Unexpected any. Specify a different type.", "81"],
[0, 0, 0, "Unexpected any. Specify a different type.", "82"], [0, 0, 0, "Do not use any type assertions.", "82"],
[0, 0, 0, "Do not use any type assertions.", "83"], [0, 0, 0, "Unexpected any. Specify a different type.", "83"],
[0, 0, 0, "Unexpected any. Specify a different type.", "84"], [0, 0, 0, "Unexpected any. Specify a different type.", "84"],
[0, 0, 0, "Unexpected any. Specify a different type.", "85"], [0, 0, 0, "Do not use any type assertions.", "85"],
[0, 0, 0, "Do not use any type assertions.", "86"], [0, 0, 0, "Unexpected any. Specify a different type.", "86"],
[0, 0, 0, "Unexpected any. Specify a different type.", "87"], [0, 0, 0, "Unexpected any. Specify a different type.", "87"],
[0, 0, 0, "Unexpected any. Specify a different type.", "88"], [0, 0, 0, "Do not use any type assertions.", "88"],
[0, 0, 0, "Do not use any type assertions.", "89"], [0, 0, 0, "Unexpected any. Specify a different type.", "89"],
[0, 0, 0, "Unexpected any. Specify a different type.", "90"], [0, 0, 0, "Unexpected any. Specify a different type.", "90"],
[0, 0, 0, "Unexpected any. Specify a different type.", "91"], [0, 0, 0, "Do not use any type assertions.", "91"],
[0, 0, 0, "Do not use any type assertions.", "92"], [0, 0, 0, "Unexpected any. Specify a different type.", "92"]
[0, 0, 0, "Unexpected any. Specify a different type.", "93"]
], ],
"packages/grafana-data/src/utils/Registry.ts:5381": [ "packages/grafana-data/src/utils/Registry.ts:5381": [
[0, 0, 0, "Unexpected any. Specify a different type.", "0"] [0, 0, 0, "Unexpected any. Specify a different type.", "0"]
@ -1847,14 +1846,6 @@ exports[`better eslint`] = {
[0, 0, 0, "Styles should be written using objects.", "4"], [0, 0, 0, "Styles should be written using objects.", "4"],
[0, 0, 0, "Styles should be written using objects.", "5"] [0, 0, 0, "Styles should be written using objects.", "5"]
], ],
"public/app/features/alerting/unified/api/alertmanager.ts:5381": [
[0, 0, 0, "Unexpected any. Specify a different type.", "0"],
[0, 0, 0, "Unexpected any. Specify a different type.", "1"]
],
"public/app/features/alerting/unified/api/ruler.ts:5381": [
[0, 0, 0, "Do not use any type assertions.", "0"],
[0, 0, 0, "Do not use any type assertions.", "1"]
],
"public/app/features/alerting/unified/components/AlertLabel.tsx:5381": [ "public/app/features/alerting/unified/components/AlertLabel.tsx:5381": [
[0, 0, 0, "Styles should be written using objects.", "0"] [0, 0, 0, "Styles should be written using objects.", "0"]
], ],
@ -2968,12 +2959,11 @@ exports[`better eslint`] = {
[0, 0, 0, "Unexpected any. Specify a different type.", "2"], [0, 0, 0, "Unexpected any. Specify a different type.", "2"],
[0, 0, 0, "Unexpected any. Specify a different type.", "3"], [0, 0, 0, "Unexpected any. Specify a different type.", "3"],
[0, 0, 0, "Unexpected any. Specify a different type.", "4"], [0, 0, 0, "Unexpected any. Specify a different type.", "4"],
[0, 0, 0, "Unexpected any. Specify a different type.", "5"], [0, 0, 0, "Do not use any type assertions.", "5"],
[0, 0, 0, "Do not use any type assertions.", "6"], [0, 0, 0, "Unexpected any. Specify a different type.", "6"],
[0, 0, 0, "Unexpected any. Specify a different type.", "7"], [0, 0, 0, "Do not use any type assertions.", "7"],
[0, 0, 0, "Do not use any type assertions.", "8"], [0, 0, 0, "Do not use any type assertions.", "8"],
[0, 0, 0, "Do not use any type assertions.", "9"], [0, 0, 0, "Unexpected any. Specify a different type.", "9"]
[0, 0, 0, "Unexpected any. Specify a different type.", "10"]
], ],
"public/app/features/dashboard/components/DashNav/DashNavButton.tsx:5381": [ "public/app/features/dashboard/components/DashNav/DashNavButton.tsx:5381": [
[0, 0, 0, "Styles should be written using objects.", "0"] [0, 0, 0, "Styles should be written using objects.", "0"]
@ -5405,8 +5395,7 @@ exports[`better eslint`] = {
[0, 0, 0, "Do not use any type assertions.", "3"], [0, 0, 0, "Do not use any type assertions.", "3"],
[0, 0, 0, "Do not use any type assertions.", "4"], [0, 0, 0, "Do not use any type assertions.", "4"],
[0, 0, 0, "Do not use any type assertions.", "5"], [0, 0, 0, "Do not use any type assertions.", "5"],
[0, 0, 0, "Do not use any type assertions.", "6"], [0, 0, 0, "Do not use any type assertions.", "6"]
[0, 0, 0, "Unexpected any. Specify a different type.", "7"]
], ],
"public/app/features/variables/state/keyedVariablesReducer.ts:5381": [ "public/app/features/variables/state/keyedVariablesReducer.ts:5381": [
[0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "0"],
@ -7438,8 +7427,7 @@ exports[`better eslint`] = {
[0, 0, 0, "Do not use any type assertions.", "1"] [0, 0, 0, "Do not use any type assertions.", "1"]
], ],
"public/app/types/unified-alerting-dto.ts:5381": [ "public/app/types/unified-alerting-dto.ts:5381": [
[0, 0, 0, "Do not use any type assertions.", "0"], [0, 0, 0, "Do not use any type assertions.", "0"]
[0, 0, 0, "Do not use any type assertions.", "1"]
], ],
"public/test/core/redux/reduxTester.ts:5381": [ "public/test/core/redux/reduxTester.ts:5381": [
[0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "0"],

@ -0,0 +1,61 @@
import { isObject, isTruthy } from './data';
describe('isObject', () => {
it.each([
// [value, expected]
// These are objects
[{}, true],
[[], true],
[{ a: 1 }, true],
[new Date(), true],
[new Error(), true],
// These are not!
[parseInt('blabla', 10), false], // NaN
[null, false],
[undefined, false],
[-Infinity, false],
[-42, false],
[0, false],
[-0, false],
[42, false],
[Infinity, false],
['foo', false],
[true, false],
[Symbol(), false],
[() => {}, false],
])('should return %p for %p', (input, expected) => {
expect(isObject(input)).toBe(expected);
});
});
describe('isTruthy', () => {
it.each([
// [value, expected]
// These are truthy
[true, true],
[-Infinity, true],
[-42, true],
[42, true],
[Infinity, true],
['foo', true],
[{}, true],
[[], true],
[() => {}, true],
[Symbol(), true],
[new Date(), true],
// These are falsy
[false, false],
[0, false],
[-0, false],
['', false],
[null, false],
[undefined, false],
[parseInt('blabla', 10), false], // NaN
])('should return %p for %p', (input, expected) => {
expect(isTruthy(input)).toBe(expected);
});
});

@ -213,3 +213,27 @@ export interface DataConfigSource {
type Truthy<T> = T extends false | '' | 0 | null | undefined ? never : T; type Truthy<T> = T extends false | '' | 0 | null | undefined ? never : T;
export const isTruthy = <T>(value: T): value is Truthy<T> => Boolean(value); export const isTruthy = <T>(value: T): value is Truthy<T> => Boolean(value);
/**
* Serves no runtime purpose - only used to make typescript check a value has been correctly
* narrowed to an object
*/
function identityObject(value: object): object {
return value;
}
/**
* Utility type predicate to check if a value is typeof object, but excludes "null".
*
* We normally discourage the use of type predicates in favor of just inline typescript narrowing,
* but this is a special case to handle null annoyingly being typeof object
*/
export function isObject(value: unknown): value is object {
if (typeof value === 'object' && value !== null) {
identityObject(value);
return true;
}
return false;
}

@ -16,6 +16,7 @@ import {
StandardEditorContext, StandardEditorContext,
} from '../field'; } from '../field';
import { PanelOptionsSupplier } from '../panel/PanelPlugin'; import { PanelOptionsSupplier } from '../panel/PanelPlugin';
import { isObject } from '../types';
import { OptionsEditorItem, OptionsUIRegistryBuilder } from '../types/OptionsUIRegistryBuilder'; import { OptionsEditorItem, OptionsUIRegistryBuilder } from '../types/OptionsUIRegistryBuilder';
import { FieldConfigEditorProps, FieldConfigPropertyItem, FieldConfigEditorConfig } from '../types/fieldOverrides'; import { FieldConfigEditorProps, FieldConfigPropertyItem, FieldConfigEditorConfig } from '../types/fieldOverrides';
import { PanelOptionsEditorConfig, PanelOptionsEditorItem } from '../types/panel'; import { PanelOptionsEditorConfig, PanelOptionsEditorItem } from '../types/panel';
@ -204,8 +205,8 @@ export class NestedPanelOptionsBuilder<TSub = any> implements OptionsEditorItem<
}; };
} }
export function isNestedPanelOptions(item: any): item is NestedPanelOptionsBuilder { export function isNestedPanelOptions(item: unknown): item is NestedPanelOptionsBuilder {
return item.id === 'nested-panel-options'; return isObject(item) && 'id' in item && item.id === 'nested-panel-options';
} }
/** /**

@ -1,6 +1,6 @@
import { lastValueFrom } from 'rxjs'; import { lastValueFrom } from 'rxjs';
import { urlUtil } from '@grafana/data'; import { isObject, urlUtil } from '@grafana/data';
import { getBackendSrv, isFetchError } from '@grafana/runtime'; import { getBackendSrv, isFetchError } from '@grafana/runtime';
import { import {
AlertmanagerAlert, AlertmanagerAlert,
@ -206,18 +206,24 @@ function receiversResponseContainsErrors(result: TestReceiversResult) {
); );
} }
function isTestReceiversResult(data: any): data is TestReceiversResult { function isTestReceiversResult(data: unknown): data is TestReceiversResult {
const receivers = data?.receivers; if (isObject(data) && 'receivers' in data && Array.isArray(data.receivers)) {
return data.receivers.every(isSingleTestRecieverResult);
if (Array.isArray(receivers)) {
return receivers.every(
(receiver: any) => typeof receiver.name === 'string' && Array.isArray(receiver.grafana_managed_receiver_configs)
);
} }
return false; return false;
} }
function isSingleTestRecieverResult(receiver: unknown): receiver is TestReceiversResult {
return (
isObject(receiver) &&
'name' in receiver &&
typeof receiver.name === 'string' &&
'grafana_managed_receiver_configs' in receiver &&
Array.isArray(receiver.grafana_managed_receiver_configs)
);
}
function getReceiverResultError(receiversResult: TestReceiversResult) { function getReceiverResultError(receiversResult: TestReceiversResult) {
return receiversResult.receivers return receiversResult.receivers
.flatMap((receiver) => .flatMap((receiver) =>

@ -1,5 +1,6 @@
import { lastValueFrom } from 'rxjs'; import { lastValueFrom } from 'rxjs';
import { isObject } from '@grafana/data';
import { FetchResponse, getBackendSrv } from '@grafana/runtime'; import { FetchResponse, getBackendSrv } from '@grafana/runtime';
import { RulerDataSourceConfig } from 'app/types/unified-alerting'; import { RulerDataSourceConfig } from 'app/types/unified-alerting';
import { PostableRulerRuleGroupDTO, RulerRuleGroupDTO, RulerRulesConfigDTO } from 'app/types/unified-alerting-dto'; import { PostableRulerRuleGroupDTO, RulerRuleGroupDTO, RulerRulesConfigDTO } from 'app/types/unified-alerting-dto';
@ -155,8 +156,13 @@ async function rulerGetRequest<T>(url: string, empty: T, params?: Record<string,
} }
function isResponseError(error: unknown): error is FetchResponse<ErrorResponseMessage> { function isResponseError(error: unknown): error is FetchResponse<ErrorResponseMessage> {
const hasErrorMessage = (error as FetchResponse<ErrorResponseMessage>).data != null; if (!isObject(error)) {
const hasErrorCode = Number.isFinite((error as FetchResponse<ErrorResponseMessage>).status); return false;
}
const hasErrorMessage = 'data' in error && error.data !== null && error.data !== undefined;
const hasErrorCode = 'status' in error && Number.isFinite(error.status);
return hasErrorCode && hasErrorMessage; return hasErrorCode && hasErrorMessage;
} }

@ -52,8 +52,10 @@ interface PanelWithExportableLibraryPanel {
libraryPanel: LibraryPanel; libraryPanel: LibraryPanel;
} }
function isExportableLibraryPanel(p: any): p is PanelWithExportableLibraryPanel { function isExportableLibraryPanel(
return p.libraryPanel && typeof p.libraryPanel.name === 'string' && typeof p.libraryPanel.uid === 'string'; p: PanelModel | PanelWithExportableLibraryPanel
): p is PanelWithExportableLibraryPanel {
return Boolean(p.libraryPanel?.name && p.libraryPanel?.uid);
} }
interface DataSources { interface DataSources {

@ -5,6 +5,7 @@ import {
getDataSourceRef, getDataSourceRef,
isDataSourceRef, isDataSourceRef,
isEmptyObject, isEmptyObject,
isObject,
LoadingState, LoadingState,
TimeRange, TimeRange,
TypedVariableModel, TypedVariableModel,
@ -1105,10 +1106,6 @@ export function upgradeLegacyQueries(
}; };
} }
function isDataQueryType(query: any): query is DataQuery { function isDataQueryType(query: unknown): query is DataQuery {
if (!query) { return isObject(query) && 'refId' in query && typeof query.refId === 'string';
return false;
}
return query.hasOwnProperty('refId') && typeof query.refId === 'string';
} }

@ -38,11 +38,8 @@ export function isGrafanaAlertState(state: string): state is GrafanaAlertState {
export function isAlertStateWithReason( export function isAlertStateWithReason(
state: PromAlertingRuleState | GrafanaAlertStateWithReason state: PromAlertingRuleState | GrafanaAlertStateWithReason
): state is GrafanaAlertStateWithReason { ): state is GrafanaAlertStateWithReason {
return ( const propAlertingRuleStateValues: string[] = Object.values(PromAlertingRuleState);
state !== null && return state !== null && state !== undefined && !propAlertingRuleStateValues.includes(state);
typeof state !== 'undefined' &&
!Object.values(PromAlertingRuleState).includes(state as PromAlertingRuleState)
);
} }
export function mapStateWithReasonToBaseState( export function mapStateWithReasonToBaseState(

Loading…
Cancel
Save