chore(eslint): add @typescript-eslint/no-unsafe-type-assertion rule (#1534)

Instead of using type assertions to narrow a type, it's better to rely on type guards, which help avoid potential runtime errors caused by unsafe type assertions.

Currently the rule is enforced in `common-utils` pkg

Dup of https://github.com/hyperdxio/hyperdx/pull/679

.changeset/pink-pugs-tickle.md

@@ -0,0 +1,7 @@
+---
+"@hyperdx/common-utils": patch
+"@hyperdx/api": patch
+"@hyperdx/app": patch
+---
+
+chore(eslint): enable @typescript-eslint/no-unsafe-type-assertion rule (warn)

.changeset/tough-owls-check.md

@@ -0,0 +1,5 @@
+---
+"@hyperdx/common-utils": patch
+---
+
+refactor(common-utils): improve type safety and linting for type assertions

packages/api/eslint.config.mjs

@@ -42,6 +42,7 @@ export default [
       '@typescript-eslint/no-empty-object-type': 'warn',
       '@typescript-eslint/no-explicit-any': 'off',
       '@typescript-eslint/no-floating-promises': 'error',
+      '@typescript-eslint/no-unsafe-type-assertion': 'warn',
       '@typescript-eslint/no-namespace': 'warn',
       '@typescript-eslint/no-unused-vars': [
         'warn',

packages/app/eslint.config.mjs

@@ -21,6 +21,7 @@ export default [
       'next-env.d.ts',
       'playwright-report/**',
       '.next/**',
+      '.storybook/**',
       'node_modules/**',
       'out/**',
       'build/**',
@@ -32,6 +33,8 @@ export default [
       '**/*.config.mjs',
       'eslint.config.mjs',
       'public/__ENV.js',
+      'global-setup.js',
+      'scripts/**',
     ],
   },
   {
@@ -50,6 +53,7 @@ export default [
       '@typescript-eslint/no-empty-function': 'warn',
       '@typescript-eslint/no-explicit-any': 'off',
       '@typescript-eslint/no-unsafe-function-type': 'warn',
+      '@typescript-eslint/no-unsafe-type-assertion': 'warn',
       '@typescript-eslint/no-unused-expressions': 'warn',
       '@typescript-eslint/no-unused-vars': [
         'warn',
@@ -94,6 +98,7 @@ export default [
         ecmaFeatures: {
           jsx: true,
         },
+        project: './tsconfig.json',
         tsconfigRootDir: import.meta.dirname,
       },
       globals: {
@@ -119,6 +124,13 @@ export default [
       },
     },
   },
+  {
+    // Disable type-checked rules for JS files (not part of TypeScript project)
+    files: ['**/*.{js,jsx}'],
+    rules: {
+      '@typescript-eslint/no-unsafe-type-assertion': 'off',
+    },
+  },
   {
     files: ['tests/e2e/**/*.{ts,js}'],
     ...playwrightPlugin.configs['flat/recommended'],

packages/common-utils/eslint.config.mjs

@@ -37,6 +37,7 @@ export default [
       '@typescript-eslint/no-empty-object-type': 'warn',
       '@typescript-eslint/no-explicit-any': 'off',
       '@typescript-eslint/no-floating-promises': 'error',
+      '@typescript-eslint/no-unsafe-type-assertion': 'error',
       '@typescript-eslint/no-namespace': 'warn',
       '@typescript-eslint/no-unused-vars': [
         'warn',
@@ -90,5 +91,12 @@ export default [
       },
     },
   },
+  {
+    // Disable unsafe type assertion rule for test files (mocking requires type assertions)
+    files: ['src/**/*.test.ts', 'src/**/__tests__/**/*.ts'],
+    rules: {
+      '@typescript-eslint/no-unsafe-type-assertion': 'off',
+    },
+  },
 ];
 

packages/common-utils/src/clickhouse/browser.ts

@@ -98,6 +98,7 @@ export class ClickhouseClient extends BaseClickhouseClient {
 
   protected async __query<Format extends DataFormat>({
     query,
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- default generic value
     format = 'JSON' as Format,
     query_params = {},
     abort_signal,
@@ -123,6 +124,7 @@ export class ClickhouseClient extends BaseClickhouseClient {
         : {}),
     };
 
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- client library type mismatch
     return this.getClient().query({
       query,
       query_params,

packages/common-utils/src/clickhouse/index.ts

@@ -470,11 +470,10 @@ export abstract class BaseClickhouseClient {
       debugSql = query;
     }
 
-    // eslint-disable-next-line no-console
     console.debug('--------------------------------------------------------');
-    // eslint-disable-next-line no-console
+
     console.debug('Sending Query:', debugSql);
-    // eslint-disable-next-line no-console
+
     console.debug('--------------------------------------------------------');
   }
 
@@ -742,6 +741,7 @@ export function chSqlToAliasMap(
       replaceJsonExpressions(sql);
 
     const parser = new SQLParser.Parser();
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- astify returns union type
     const ast = parser.astify(sqlWithReplacements, {
       database: 'Postgresql',
       parseOptions: { includeLocations: true },
@@ -795,9 +795,10 @@ export function filterColumnMetaByType(
   meta: Array<ColumnMetaType>,
   types: JSDataType[],
 ): Array<ColumnMetaType> | undefined {
-  return meta.filter(column =>
-    types.includes(convertCHDataTypeToJSType(column.type) as JSDataType),
-  );
+  return meta.filter(column => {
+    const jsType = convertCHDataTypeToJSType(column.type);
+    return jsType != null && types.includes(jsType);
+  });
 }
 
 export function inferTimestampColumn(

packages/common-utils/src/clickhouse/node.ts

@@ -25,6 +25,7 @@ export class ClickhouseClient extends BaseClickhouseClient {
 
   protected async __query<Format extends DataFormat>({
     query,
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- default generic value
     format = 'JSON' as Format,
     query_params = {},
     abort_signal,
@@ -38,6 +39,7 @@ export class ClickhouseClient extends BaseClickhouseClient {
     );
 
     // TODO: Custom error handling
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- client library type mismatch
     return this.getClient().query({
       query,
       query_params,

packages/common-utils/src/core/metadata.ts

@@ -30,7 +30,7 @@ export class MetadataCache {
 
   async getOrFetch<T>(key: string, query: () => Promise<T>): Promise<T> {
     // Check if value exists in cache
-    const cachedValue = this.cache.get(key) as T | undefined;
+    const cachedValue: T | undefined = this.cache.get(key);
     if (cachedValue != null) {
       return cachedValue;
     }
@@ -194,9 +194,9 @@ export class Metadata {
             connectionId,
             clickhouse_settings: this.getClickHouseSettings(),
           })
-          .then(res => res.json())
+          .then(res => res.json<ColumnMeta>())
           .then(d => d.data);
-        return columns as ColumnMeta[];
+        return columns;
       },
     );
   }
@@ -353,13 +353,15 @@ export class Metadata {
             max_rows_to_read: '0',
           },
         })
-        .then(res => res.json<Record<string, unknown>>())
+        .then(res => res.json<{ keysArr?: string[]; key?: string }>())
         .then(d => {
           let output: string[];
           if (strategy === 'groupUniqArrayArray') {
-            output = d.data[0].keysArr as string[];
+            output = d.data[0].keysArr ?? [];
           } else {
-            output = d.data.map(row => row.key) as string[];
+            output = d.data
+              .map(row => row.key)
+              .filter((k): k is string => Boolean(k));
           }
 
           return output.filter(r => r);
@@ -496,8 +498,8 @@ export class Metadata {
             ...this.getClickHouseSettings(),
           },
         })
-        .then(res => res.json<Record<string, unknown>>())
-        .then(d => d.data.map(row => row.value as string));
+        .then(res => res.json<{ value: string }>())
+        .then(d => d.data.map(row => row.value));
       return values;
     });
   }
@@ -765,10 +767,11 @@ export class Metadata {
           })
           .then(res => res.json<any>());
 
-        // TODO: Fix type issues mentioned in HDX-1548. value is not acually a
+        // TODO: Fix type issues mentioned in HDX-1548. value is not actually a
         // string[], sometimes it's { [key: string]: string; }
         return Object.entries(json?.data?.[0]).map(([key, value]) => ({
           key: keys[parseInt(key.replace('param', ''))],
+          // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- intentional, see HDX-1548
           value: (value as string[])?.filter(Boolean), // remove nulls
         }));
       },

packages/common-utils/src/core/renderChartConfig.ts

@@ -168,6 +168,7 @@ const fastifySQL = ({
   // Parse the SQL AST
   try {
     const parser = new SQLParser.Parser();
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- astify returns union type, we expect Select
     const ast = parser.astify(rawSQL, {
       database: 'Postgresql',
     }) as SQLParser.Select;
@@ -187,9 +188,11 @@ const fastifySQL = ({
       }
 
       let colExpr;
+
       switch (node.type) {
         case 'column_ref': {
           // FIXME: handle 'Value' type?
+          // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
           const _n = node as ColumnRef;
           // @ts-ignore
           if (typeof _n.column !== 'string') {
@@ -199,6 +202,7 @@ const fastifySQL = ({
           break;
         }
         case 'binary_expr': {
+          // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
           const _n = node as SQLParser.Expr;
           if (Array.isArray(_n.left)) {
             for (const left of _n.left) {
@@ -218,6 +222,7 @@ const fastifySQL = ({
           break;
         }
         case 'function': {
+          // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
           const _n = node as SQLParser.Function;
 
           if (_n.args?.type === 'expr_list') {
@@ -231,6 +236,7 @@ const fastifySQL = ({
                 _n.args?.value?.[0]?.type === 'column_ref' &&
                 _n.args?.value?.[1]?.type === 'single_quote_string'
               ) {
+                // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- incomplete library types
                 colExpr = `${_n.name?.name?.[0]?.value}(${(_n.args?.value?.[0] as any)?.column.expr.value}, '${_n.args?.value?.[1]?.value}')`;
               }
             }
@@ -250,6 +256,7 @@ const fastifySQL = ({
       if (colExpr) {
         const materializedField = materializedFields.get(colExpr);
         if (materializedField) {
+          // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
           const _n = node as ColumnRef;
           // reset the node ref
           for (const key in _n) {
@@ -971,7 +978,8 @@ async function renderWith(
           // results in schema conformance.
           const resolvedSql = sql
             ? sql
-            : await renderChartConfig(chartConfig as ChartConfig, metadata);
+            : // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- intentional, see comment above
+              await renderChartConfig(chartConfig as ChartConfig, metadata);
 
           if (clause.isSubquery === false) {
             return chSql`(${resolvedSql}) AS ${{ Identifier: clause.name }}`;
@@ -1269,6 +1277,7 @@ async function translateMetricChartConfig(
     // Render the various clauses from the user input so they can be woven into the CTE queries. The dateRange
     // is manipulated to search forward/back one bucket window to ensure that there is enough data to compute
     // a reasonable value on the ends of the series.
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
     const cteChartConfig = {
       ...chartConfig,
       from: {
@@ -1374,6 +1383,7 @@ export async function renderChartConfig(
     chSql`${orderBy?.sql ? chSql`ORDER BY ${orderBy}` : ''}`,
     //chSql`${fill?.sql ? chSql`WITH FILL ${fill}` : ''}`,
     chSql`${limit?.sql ? chSql`LIMIT ${limit}` : ''}`,
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- settings type narrowing
     chSql`${'settings' in chartConfig ? chSql`SETTINGS ${chartConfig.settings as ChSql}` : []}`,
   ]);
 }

packages/common-utils/src/queryParser.ts

@@ -29,6 +29,29 @@ export function parse(query: string): lucene.AST {
 
 const IMPLICIT_FIELD = '<implicit>';
 
+// Type guards for lucene AST types
+function isNodeTerm(node: lucene.Node | lucene.AST): node is lucene.NodeTerm {
+  return 'term' in node && node.term != null;
+}
+
+function isNodeRangedTerm(
+  node: lucene.Node | lucene.AST,
+): node is lucene.NodeRangedTerm {
+  return 'inclusive' in node && node.inclusive != null;
+}
+
+function isBinaryAST(ast: lucene.AST | lucene.Node): ast is lucene.BinaryAST {
+  return 'right' in ast && ast.right != null;
+}
+
+function isLeftOnlyAST(
+  ast: lucene.AST | lucene.Node,
+): ast is lucene.LeftOnlyAST {
+  return (
+    'left' in ast && ast.left != null && !('right' in ast && ast.right != null)
+  );
+}
+
 const CLICK_HOUSE_JSON_NUMBER_TYPES = [
   'Int8',
   'Int16',
@@ -641,8 +664,8 @@ async function nodeTerm(
   const isImplicitField = node.field === IMPLICIT_FIELD;
 
   // NodeTerm
-  if ((node as lucene.NodeTerm).term != null) {
-    const nodeTerm = node as lucene.NodeTerm;
+  if (isNodeTerm(node)) {
+    const nodeTerm = node;
     let term = decodeSpecialTokens(nodeTerm.term);
     // We should only negate the search for negated bare terms (ex. '-5')
     // This meeans the field is implicit and the prefix is -
@@ -714,8 +737,8 @@ async function nodeTerm(
     // TODO: Handle regex, similarity, boost, prefix
   }
   // NodeRangedTerm
-  if ((node as lucene.NodeRangedTerm).inclusive != null) {
-    const rangedTerm = node as lucene.NodeRangedTerm;
+  if (isNodeRangedTerm(node)) {
+    const rangedTerm = node;
     return serializer.range(
       field,
       rangedTerm.term_min,
@@ -760,18 +783,18 @@ async function serialize(
   // Node Scenarios:
   // 1. NodeTerm: Single term ex. "foo:bar"
   // 2. NodeRangedTerm: Two terms ex. "foo:[bar TO qux]"
-  if ((ast as lucene.NodeTerm).term != null) {
-    return await nodeTerm(ast as lucene.NodeTerm, serializer, context);
+  if (isNodeTerm(ast)) {
+    return await nodeTerm(ast, serializer, context);
   }
-  if ((ast as lucene.NodeRangedTerm).inclusive != null) {
-    return await nodeTerm(ast as lucene.NodeTerm, serializer, context);
+  if (isNodeRangedTerm(ast)) {
+    return await nodeTerm(ast, serializer, context);
   }
 
   // AST Scenarios:
   // 1. BinaryAST: Two terms ex. "foo:bar AND baz:qux"
   // 2. LeftOnlyAST: Single term ex. "foo:bar"
-  if ((ast as lucene.BinaryAST).right != null) {
-    const binaryAST = ast as lucene.BinaryAST;
+  if (isBinaryAST(ast)) {
+    const binaryAST = ast;
     const operator = serializer.operator(binaryAST.operator, context);
     const parenthesized = binaryAST.parenthesized;
 
@@ -786,8 +809,8 @@ async function serialize(
     return serialized;
   }
 
-  if ((ast as lucene.LeftOnlyAST).left != null) {
-    const leftOnlyAST = ast as lucene.LeftOnlyAST;
+  if (isLeftOnlyAST(ast)) {
+    const leftOnlyAST = ast;
     const parenthesized = leftOnlyAST.parenthesized;
 
     const newContext = createSerializerContext(context, leftOnlyAST);

packages/common-utils/src/types.ts

@@ -31,6 +31,7 @@ export const MetricTableSchema = z
         ...acc,
         [key]: z.string().optional(),
       }),
+      // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- reduce builds complete object at runtime
       {} as Record<MetricsDataType, z.ZodString>,
     ),
   )