Typed variables pt6: Clean up and test variable type guards (take 2) (#54025)

* Add fixture-factories for all variable types

* clean up variable guards

* add tests for isMulti, hasCurrent, and hasOptions variable type guards

* create VariableSupport type to try and work around ts wonkiness

.betterer.results

@@ -5872,15 +5872,8 @@ exports[`better eslint`] = {
       [0, 0, 0, "Unexpected any. Specify a different type.", "32"]
     ],
     "public/app/features/variables/guard.ts:5381": [
-      [0, 0, 0, "Do not use any type assertions.", "0"],
-      [0, 0, 0, "Do not use any type assertions.", "1"],
-      [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.", "4"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "5"],
-      [0, 0, 0, "Do not use any type assertions.", "6"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "7"],
-      [0, 0, 0, "Do not use any type assertions.", "8"]
+      [0, 0, 0, "Unexpected any. Specify a different type.", "0"],
+      [0, 0, 0, "Unexpected any. Specify a different type.", "1"]
     ],
     "public/app/features/variables/inspect/NetworkGraph.tsx:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],

packages/grafana-data/src/types/datasource.ts

@@ -173,6 +173,13 @@ export interface DataSourceConstructor<
   new (instanceSettings: DataSourceInstanceSettings<TOptions>, ...args: any[]): DSType;
 }
 
+// VariableSupport is hoisted up to its own type to fix the wonky intermittent
+// 'variables is references directly or indirectly' error
+type VariableSupport<TQuery extends DataQuery, TOptions extends DataSourceJsonData> =
+  | StandardVariableSupport<DataSourceApi<TQuery, TOptions>>
+  | CustomVariableSupport<DataSourceApi<TQuery, TOptions>>
+  | DataSourceVariableSupport<DataSourceApi<TQuery, TOptions>>;
+
 /**
  * The main data source abstraction interface, represents an instance of a data source
  *
@@ -341,10 +348,7 @@ abstract class DataSourceApi<
    * Defines new variable support
    * @alpha -- experimental
    */
-  variables?:
-    | StandardVariableSupport<DataSourceApi<TQuery, TOptions>>
-    | CustomVariableSupport<DataSourceApi<TQuery, TOptions>>
-    | DataSourceVariableSupport<DataSourceApi<TQuery, TOptions>>;
+  variables?: VariableSupport<TQuery, TOptions>;
 
   /*
    * Optionally, use this method to set default values for a query

public/app/features/variables/guard.test.ts

@@ -1,4 +1,4 @@
-import { VariableSupportType } from '@grafana/data';
+import { TypedVariableModel, VariableSupportType, VariableType } from '@grafana/data';
 
 import { LegacyVariableQueryEditor } from './editor/LegacyVariableQueryEditor';
 import { StandardVariableQueryEditor } from './editor/getVariableQueryEditor';
@@ -9,7 +9,22 @@ import {
   hasStandardVariableSupport,
   isLegacyQueryEditor,
   isQueryEditor,
+  isMulti,
+  hasOptions,
+  hasCurrent,
 } from './guard';
+import {
+  createAdhocVariable,
+  createConstantVariable,
+  createCustomVariable,
+  createDashboardVariable,
+  createDatasourceVariable,
+  createIntervalVariable,
+  createOrgVariable,
+  createQueryVariable,
+  createTextBoxVariable,
+  createUserVariable,
+} from './state/__tests__/fixtures';
 
 describe('type guards', () => {
   describe('hasLegacyVariableSupport', () => {
@@ -135,6 +150,53 @@ describe('type guards', () => {
       });
     });
   });
+
+  interface VariableFacts {
+    variable: TypedVariableModel;
+    isMulti: boolean;
+    hasOptions: boolean;
+    hasCurrent: boolean;
+  }
+  // This structure is typed (because the key is a const string union) to ensure that we always
+  // test every type of variable, as new variables are added
+  type ExtraVariableTypes = 'org' | 'dashboard' | 'user';
+  // prettier-ignore
+  const variableFactsObj: Record<VariableType | ExtraVariableTypes, VariableFacts> = {
+    query:      { variable: createQueryVariable(),      isMulti: true,  hasOptions: true,  hasCurrent: true },
+    adhoc:      { variable: createAdhocVariable(),      isMulti: false, hasOptions: false, hasCurrent: false },
+    constant:   { variable: createConstantVariable(),   isMulti: false, hasOptions: true,  hasCurrent: true },
+    datasource: { variable: createDatasourceVariable(), isMulti: true,  hasOptions: true,  hasCurrent: true },
+    interval:   { variable: createIntervalVariable(),   isMulti: false, hasOptions: true,  hasCurrent: true },
+    textbox:    { variable: createTextBoxVariable(),    isMulti: false, hasOptions: true,  hasCurrent: true },
+    system:     { variable: createUserVariable(),       isMulti: false, hasOptions: false, hasCurrent: true },
+    user:       { variable: createUserVariable(),       isMulti: false, hasOptions: false, hasCurrent: true },
+    org:        { variable: createOrgVariable(),        isMulti: false, hasOptions: false, hasCurrent: true },
+    dashboard:  { variable: createDashboardVariable(),  isMulti: false, hasOptions: false, hasCurrent: true },
+    custom:     { variable: createCustomVariable(),     isMulti: true,  hasOptions: true,  hasCurrent: true },
+  };
+
+  const variableFacts = Object.values(variableFactsObj);
+
+  it.each(variableFacts)(
+    'isMulti correctly identifies variables with multi support: $variable.type should be $isMulti',
+    ({ variable, isMulti: expected }) => {
+      expect(isMulti(variable)).toBe(expected);
+    }
+  );
+
+  it.each(variableFacts)(
+    'hasOptions correctly identifies variables with options support: $variable.type should be $hasOptions',
+    ({ variable, hasOptions: expected }) => {
+      expect(hasOptions(variable)).toBe(expected);
+    }
+  );
+
+  it.each(variableFacts)(
+    'hasCurrent correctly identifies variables with options support: $variable.type should be $hasCurrent',
+    ({ variable, hasCurrent: expected }) => {
+      expect(hasCurrent(variable)).toBe(expected);
+    }
+  );
 });
 
 describe('isLegacyQueryEditor', () => {

public/app/features/variables/guard.ts

@@ -2,7 +2,6 @@ import { ComponentType } from 'react';
 import { Observable } from 'rxjs';
 
 import {
-  CustomVariableSupport,
   DataQuery,
   DataQueryRequest,
   DataQueryResponse,
@@ -11,7 +10,6 @@ import {
   DataSourceRef,
   MetricFindValue,
   StandardVariableQuery,
-  StandardVariableSupport,
   VariableModel,
   VariableSupportType,
 } from '@grafana/data';
@@ -43,27 +41,17 @@ export const isConstant = (model: VariableModel): model is ConstantVariableModel
 };
 
 export const isMulti = (model: VariableModel): model is VariableWithMultiSupport => {
-  const withMulti = model as VariableWithMultiSupport;
-  return withMulti.hasOwnProperty('multi') && typeof withMulti.multi === 'boolean';
+  return 'multi' in model;
 };
 
 export const hasOptions = (model: VariableModel): model is VariableWithOptions => {
-  return hasObjectProperty(model, 'options');
+  return 'options' in model;
 };
 
 export const hasCurrent = (model: VariableModel): model is VariableWithOptions => {
-  return hasObjectProperty(model, 'current');
+  return 'current' in model;
 };
 
-function hasObjectProperty(model: VariableModel, property: string): model is VariableWithOptions {
-  if (!model) {
-    return false;
-  }
-
-  const withProperty = model as Record<string, any>;
-  return withProperty.hasOwnProperty(property) && typeof withProperty[property] === 'object';
-}
-
 export function isLegacyAdHocDataSource(datasource: null | DataSourceRef | string): datasource is string {
   if (datasource === null) {
     return false;
@@ -92,7 +80,6 @@ interface DataSourceWithStandardVariableSupport<
 }
 
 interface DataSourceWithCustomVariableSupport<
-  VariableQuery extends DataQuery = any,
   TQuery extends DataQuery = DataQuery,
   TOptions extends DataSourceJsonData = DataSourceJsonData
 > extends DataSourceApi<TQuery, TOptions> {
@@ -139,9 +126,8 @@ export const hasStandardVariableSupport = <
     return false;
   }
 
-  const variableSupport = datasource.variables as StandardVariableSupport<DataSourceApi<TQuery, TOptions>>;
-
-  return Boolean(variableSupport.toDataQuery);
+  const variableSupport = datasource.variables;
+  return 'toDataQuery' in variableSupport && Boolean(variableSupport.toDataQuery);
 };
 
 export const hasCustomVariableSupport = <
@@ -149,7 +135,7 @@ export const hasCustomVariableSupport = <
   TOptions extends DataSourceJsonData = DataSourceJsonData
 >(
   datasource: DataSourceApi<TQuery, TOptions>
-): datasource is DataSourceWithCustomVariableSupport<any, TQuery, TOptions> => {
+): datasource is DataSourceWithCustomVariableSupport<TQuery, TOptions> => {
   if (!datasource.variables) {
     return false;
   }
@@ -158,9 +144,13 @@ export const hasCustomVariableSupport = <
     return false;
   }
 
-  const variableSupport = datasource.variables as CustomVariableSupport<DataSourceApi<TQuery, TOptions>>;
-
-  return Boolean(variableSupport.query) && Boolean(variableSupport.editor);
+  const variableSupport = datasource.variables;
+  return (
+    'query' in variableSupport &&
+    'editor' in variableSupport &&
+    Boolean(variableSupport.query) &&
+    Boolean(variableSupport.editor)
+  );
 };
 
 export const hasDatasourceVariableSupport = <

public/app/features/variables/state/__tests__/fixtures.ts

@@ -1,9 +1,16 @@
 import {
+  AdHocVariableModel,
   BaseVariableModel,
   ConstantVariableModel,
   CustomVariableModel,
+  DashboardVariableModel,
+  DataSourceVariableModel,
+  IntervalVariableModel,
   LoadingState,
+  OrgVariableModel,
   QueryVariableModel,
+  TextBoxVariableModel,
+  UserVariableModel,
   VariableHide,
   VariableOption,
   VariableRefresh,
@@ -59,7 +66,19 @@ export function createQueryVariable(input: Partial<QueryVariableModel> = {}): Qu
   };
 }
 
-export function createConstantVariable(input: Partial<ConstantVariableModel>): ConstantVariableModel {
+export function createAdhocVariable(input?: Partial<AdHocVariableModel>): AdHocVariableModel {
+  return {
+    ...createBaseVariableModel('adhoc'),
+    datasource: {
+      uid: 'abc-123',
+      type: 'prometheus',
+    },
+    filters: [],
+    ...input,
+  };
+}
+
+export function createConstantVariable(input: Partial<ConstantVariableModel> = {}): ConstantVariableModel {
   return {
     ...createBaseVariableModel('constant'),
     query: '',
@@ -70,7 +89,89 @@ export function createConstantVariable(input: Partial<ConstantVariableModel>): C
   };
 }
 
-export function createCustomVariable(input: Partial<CustomVariableModel>): CustomVariableModel {
+export function createDatasourceVariable(input: Partial<DataSourceVariableModel> = {}): DataSourceVariableModel {
+  return {
+    ...createBaseVariableModel('datasource'),
+    regex: '',
+    refresh: VariableRefresh.onDashboardLoad,
+    multi: false,
+    includeAll: false,
+    query: '',
+    current: createVariableOption('prom-prod', { text: 'Prometheus (main)', selected: true }),
+    options: [
+      createVariableOption('prom-prod', { text: 'Prometheus (main)', selected: true }),
+      createVariableOption('prom-dev'),
+    ],
+    ...input,
+  };
+}
+
+export function createIntervalVariable(input: Partial<IntervalVariableModel> = {}): IntervalVariableModel {
+  return {
+    ...createBaseVariableModel('interval'),
+    auto: false,
+    auto_count: 30,
+    auto_min: '10s',
+    refresh: VariableRefresh.onTimeRangeChanged,
+    query: '1m,10m,30m,1h,6h,12h,1d,7d,14d,30d',
+    options: [],
+    current: createVariableOption('10m'),
+    ...input,
+  };
+}
+
+export function createTextBoxVariable(input: Partial<TextBoxVariableModel> = {}): TextBoxVariableModel {
+  return {
+    ...createBaseVariableModel('textbox'),
+    originalQuery: null,
+    query: '',
+    current: createVariableOption('prom-prod'),
+    options: [],
+    ...input,
+  };
+}
+
+export function createUserVariable(input: Partial<UserVariableModel> = {}): UserVariableModel {
+  return {
+    ...createBaseVariableModel('system'),
+    current: {
+      value: {
+        login: 'biggus-chungus',
+        id: 0,
+        email: 'chungus@example.com',
+      },
+    },
+    ...input,
+  };
+}
+
+export function createOrgVariable(input: Partial<OrgVariableModel> = {}): OrgVariableModel {
+  return {
+    ...createBaseVariableModel('system'),
+    current: {
+      value: {
+        name: 'Big Chungus Corp.',
+        id: 3,
+      },
+    },
+    ...input,
+  };
+}
+
+export function createDashboardVariable(input: Partial<DashboardVariableModel> = {}): DashboardVariableModel {
+  return {
+    ...createBaseVariableModel('system'),
+    current: {
+      value: {
+        name: 'Chungus Monitoring',
+        uid: 'b1g',
+      },
+    },
+    ...input,
+  };
+}
+
+export function createCustomVariable(input: Partial<CustomVariableModel> = {}): CustomVariableModel {
   return {
     ...createBaseVariableModel('custom'),
     multi: false,