fix: DH-17599: Change illegal characters to underscores in variable names (#2409)

- Add `makeVariableName` method to create valid variable names
- Add/update unit tests.

Author: vbabich

packages/console/src/csv/CsvInputBar.tsx

@@ -89,7 +89,7 @@ class CsvInputBar extends Component<CsvInputBarProps, CsvInputBarState> {
     // Set the table name from a file
     if (!tableNameSet && file != null && !tableName) {
       const dotIndex = file.name.lastIndexOf('.');
-      const fileTableName = DbNameValidator.legalizeTableName(
+      const fileTableName = DbNameValidator.makeVariableName(
         file.name.substring(0, dotIndex)
       );
       this.setState({

packages/utils/src/DbNameValidator.test.ts

@@ -3,9 +3,26 @@ import DbNameValidator from './DbNameValidator';
 const TABLE_PREFIX = 'table_';
 const COLUMN_PREFIX = 'column_';
 
-const VALID_TABLE_NAME = '$+@abc-123_ABC';
-const INVALID_TABLE_NAMES = ['%^&ab-c', '-abc', '-'];
-const CLEANED_INVALID_TABLE_NAMES = ['ab-c', '-abc', '-'];
+const VALID_TABLE_NAMES = ['$+@abc-123_ABC', '$'];
+const VARIABLE_NAMES_FROM_VALID = ['___abc_123_ABC', '_'];
+
+const INVALID_TABLE_NAMES = ['%^&ab-c', '-a_b c', '-', '0', '%', ''];
+const LEGALIZED_INVALID_TABLE_NAMES = [
+  'ab-c',
+  'table_-a_b_c',
+  'table_-',
+  'table_0',
+  'table_0',
+  'table_0',
+];
+const VARIABLE_NAMES_FROM_INVALID = [
+  'ab_c',
+  'table__a_b_c',
+  'table__',
+  'table_0',
+  'table_0',
+  'table_0',
+];
 
 const VALID_COL_NAME = 'abc123_ABC';
 const INVALID_COL_NAME = '@abc123_ABC-123';
@@ -16,8 +33,8 @@ const RESERVED_JAVA_WORD = 'return';
 const RESERVED_DEEPHAVEN_WORD = 'not';
 
 describe('Table name validation', () => {
-  it('Returns true on valid table names', () => {
-    expect(DbNameValidator.isValidTableName(VALID_TABLE_NAME)).toBe(true);
+  it.each(VALID_TABLE_NAMES)('Returns true on valid table name %s', name => {
+    expect(DbNameValidator.isValidTableName(name)).toBe(true);
   });
 
   it.each(INVALID_TABLE_NAMES)(
@@ -49,14 +66,15 @@ describe('Column name validation', () => {
 });
 
 describe('legalizeTableName', () => {
-  it('Does not change a valid table name', () => {
-    expect(DbNameValidator.legalizeTableName(VALID_TABLE_NAME)).toBe(
-      VALID_TABLE_NAME
-    );
+  it.each(VALID_TABLE_NAMES)('Does not change a valid table name %s', name => {
+    expect(DbNameValidator.legalizeTableName(name)).toBe(name);
   });
 
   it.each(
-    INVALID_TABLE_NAMES.map((name, i) => [name, CLEANED_INVALID_TABLE_NAMES[i]])
+    INVALID_TABLE_NAMES.map((name, i) => [
+      name,
+      LEGALIZED_INVALID_TABLE_NAMES[i],
+    ])
   )('Legalize an invalid table name %s > %s', (invalid, cleaned) => {
     expect(DbNameValidator.legalizeTableName(invalid)).toBe(cleaned);
   });
@@ -72,6 +90,25 @@ describe('legalizeTableName', () => {
   });
 });
 
+describe('makeVariableName', () => {
+  it.each(
+    VALID_TABLE_NAMES.map((name, i) => [name, VARIABLE_NAMES_FROM_VALID[i]])
+  )(
+    'Makes a variable name for a valid table name %s > %s',
+    (invalid, variableName) => {
+      expect(DbNameValidator.makeVariableName(invalid)).toBe(variableName);
+    }
+  );
+  it.each(
+    INVALID_TABLE_NAMES.map((name, i) => [name, VARIABLE_NAMES_FROM_INVALID[i]])
+  )(
+    'Makes a variable name for an invalid table name %s > %s',
+    (invalid, variableName) => {
+      expect(DbNameValidator.makeVariableName(invalid)).toBe(variableName);
+    }
+  );
+});
+
 describe('legalizeColumnName', () => {
   it('Does not change a valid column name', () => {
     expect(DbNameValidator.legalizeColumnName(VALID_COL_NAME)).toBe(

packages/utils/src/DbNameValidator.ts

@@ -62,14 +62,20 @@ const JAVA_KEYWORDS = new Set([
   'false',
 ]);
 
+// The '$' character is not valid in Deephaven table and column names,
+// yet it is treated as valid in the DbNameValidator Java class.
+// TODO: Update the regexes once (or if) DH-19169 is merged.
+
 // From io.deephaven.db.tables.utils.DBNameValidator#STERILE_TABLE_AND_NAMESPACE_REGEX
 const STERILE_TABLE_AND_NAMESPACE_REGEX = /[^a-zA-Z0-9_$\-+@]/g;
 
 // From io.deephaven.db.tables.utils.DBNameValidator#STERILE_COLUMN_AND_QUERY_REGEX
 const STERILE_COLUMN_AND_QUERY_REGEX = /[^A-Za-z0-9_$]/g;
 
 // From io.deephaven.db.tables.utils.DBNameValidator#TABLE_NAME_PATTERN
-const TABLE_NAME_PATTERN = /^[a-zA-Z_$][a-zA-Z0-9_$\-+@]*$/g;
+const TABLE_NAME_PATTERN = /^[a-zA-Z_$][a-zA-Z0-9_$\-+@]*$/;
+
+const STERILE_VARIABLE_NAME_REGEX = /[^a-zA-Z0-9_]/g;
 
 function columnNameReplacer(input: string): string {
   // Replace all dashes and spaces with underscores
@@ -108,19 +114,28 @@ class DbNameValidator {
     // Remove illegal characters
     legalName = legalName.replace(regex, '');
 
-    // Check if the name ended up blank
+    // Check if the name ended up blank and append the prefix.
+    // Note, io.deephaven.db.tables.utils.DBNameValidator throws an exception in this case.
     if (!legalName) {
-      legalName = prefix + i;
+      return prefix + i;
     }
 
-    // If name starts with a number, append prefix to the front
-    if (!Number.isNaN(Number(legalName.charAt(0)))) {
+    // If name starts with a number or a dash, append the prefix.
+    // Note, io.deephaven.db.tables.utils.DBNameValidator throws an exception for names starting with a dash.
+    if (/^[0-9-]/.test(legalName)) {
       legalName = prefix + legalName;
     }
 
     return legalName;
   };
 
+  /**
+   * Get a legal table name based on the passed in string.
+   * Follows the same rules as DBNameValidator.java except that here
+   * we prepend a prefix in cases where the Java class throws an exception.
+   * @param name The name to legalize
+   * @returns Legalized table name
+   */
   static legalizeTableName = (name: string): string =>
     DbNameValidator.legalize(
       name,
@@ -131,6 +146,17 @@ class DbNameValidator {
       0
     );
 
+  /**
+   * Make a valid variable name based on the passed in string.
+   * @param name The name to get the variable name for
+   * @returns Variable name
+   */
+  static makeVariableName = (name: string): string =>
+    DbNameValidator.legalizeTableName(name).replace(
+      STERILE_VARIABLE_NAME_REGEX,
+      '_'
+    );
+
   static legalizeColumnNames = (headers: string[]): string[] => {
     const legalHeaders: string[] = [];
     headers.forEach((header, i) => {