refactor: deprecate and clean up variable-related methods.

core/field_variable.ts

@@ -420,7 +420,7 @@ export class FieldVariable extends FieldDropdown {
     if (variableTypes === null) {
       // If variableTypes is null, return all variable types.
       if (this.sourceBlock_ && !this.sourceBlock_.isDeadOrDying()) {
-        return this.sourceBlock_.workspace.getVariableTypes();
+        return this.sourceBlock_.workspace.getVariableMap().getTypes();
       }
     }
     variableTypes = variableTypes || [''];

core/variable_map.ts

@@ -84,14 +84,9 @@ export class VariableMap
       // The IDs may match if the rename is a simple case change (name1 ->
       // Name1).
       if (!conflictVar || conflictVar.getId() === variable.getId()) {
-        this.renameVariableAndUses_(variable, newName, blocks);
+        this.renameVariableAndUses(variable, newName, blocks);
       } else {
-        this.renameVariableWithConflict_(
-          variable,
-          newName,
-          conflictVar,
-          blocks,
-        );
+        this.renameVariableWithConflict(variable, newName, conflictVar, blocks);
       }
     } finally {
       eventUtils.setGroup(existingGroup);
@@ -120,10 +115,17 @@ export class VariableMap
    * Rename a variable by updating its name in the variable map. Identify the
    * variable to rename with the given ID.
    *
+   * @deprecated v12, use VariableMap.renameVariable.
    * @param id ID of the variable to rename.
    * @param newName New variable name.
    */
   renameVariableById(id: string, newName: string) {
+    deprecation.warn(
+      'VariableMap.renameVariableById',
+      'v12',
+      'v13',
+      'VariableMap.renameVariable',
+    );
     const variable = this.getVariableById(id);
     if (!variable) {
       throw Error("Tried to rename a variable that didn't exist. ID: " + id);
@@ -140,7 +142,7 @@ export class VariableMap
    * @param newName New variable name.
    * @param blocks The list of all blocks in the workspace.
    */
-  private renameVariableAndUses_(
+  private renameVariableAndUses(
     variable: IVariableModel<IVariableState>,
     newName: string,
     blocks: Block[],
@@ -165,7 +167,7 @@ export class VariableMap
    * @param conflictVar The variable that was already using newName.
    * @param blocks The list of all blocks in the workspace.
    */
-  private renameVariableWithConflict_(
+  private renameVariableWithConflict(
     variable: IVariableModel<IVariableState>,
     newName: string,
     conflictVar: IVariableModel<IVariableState>,
@@ -176,7 +178,7 @@ export class VariableMap
 
     if (newName !== oldCase) {
       // Simple rename to change the case and update references.
-      this.renameVariableAndUses_(conflictVar, newName, blocks);
+      this.renameVariableAndUses(conflictVar, newName, blocks);
     }
 
     // These blocks now refer to a different variable.
@@ -295,9 +297,10 @@ export class VariableMap
   }
 
   /**
-   * @deprecated v12 - Delete a variables by the passed in ID and all of its
-   * uses from this workspace. May prompt the user for confirmation.
+   * Delete a variables by the passed in ID and all of its uses from this
+   * workspace. May prompt the user for confirmation.
    *
+   * @deprecated v12, use Blockly.Variables.deleteVariable.
    * @param id ID of variable to delete.
    */
   deleteVariableById(id: string) {
@@ -378,29 +381,6 @@ export class VariableMap
     return [...this.variableMap.keys()];
   }
 
-  /**
-   * Return all variable and potential variable types.  This list always
-   * contains the empty string.
-   *
-   * @param ws The workspace used to look for potential variables. This can be
-   *     different than the workspace stored on this object if the passed in ws
-   *     is a flyout workspace.
-   * @returns List of variable types.
-   * @internal
-   */
-  getVariableTypes(ws: Workspace | null): string[] {
-    const variableTypes = new Set<string>(this.variableMap.keys());
-    if (ws && ws.getPotentialVariableMap()) {
-      for (const key of ws.getPotentialVariableMap()!.getTypes()) {
-        variableTypes.add(key);
-      }
-    }
-    if (!variableTypes.has('')) {
-      variableTypes.add('');
-    }
-    return Array.from(variableTypes.values());
-  }
-
   /**
    * Return all variables of all types.
    *
@@ -417,9 +397,16 @@ export class VariableMap
   /**
    * Returns all of the variable names of all types.
    *
+   * @deprecated v12, use Blockly.Variables.getAllVariables.
    * @returns All of the variable names of all types.
    */
   getAllVariableNames(): string[] {
+    deprecation.warn(
+      'VariableMap.getAllVariableNames',
+      'v12',
+      'v13',
+      'Blockly.Variables.getAllVariables',
+    );
     const names: string[] = [];
     for (const variables of this.variableMap.values()) {
       for (const variable of variables.values()) {
@@ -430,8 +417,9 @@ export class VariableMap
   }
 
   /**
-   * @deprecated v12 - Find all the uses of a named variable.
+   * Find all the uses of a named variable.
    *
+   * @deprecated v12, use Blockly.Variables.getVariableUsesById.
    * @param id ID of the variable to find.
    * @returns Array of block usages.
    */

core/variable_model.ts

@@ -119,21 +119,6 @@ export class VariableModel implements IVariableModel<IVariableState> {
     // add the variable to that variable map, and fire a VAR_CREATE event.
     workspace.createVariable(state['name'], state['type'], state['id']);
   }
-
-  /**
-   * A custom compare function for the VariableModel objects.
-   *
-   * @param var1 First variable to compare.
-   * @param var2 Second variable to compare.
-   * @returns -1 if name of var1 is less than name of var2, 0 if equal, and 1 if
-   *     greater.
-   * @internal
-   */
-  static compareByName(var1: VariableModel, var2: VariableModel): number {
-    return var1
-      .getName()
-      .localeCompare(var2.getName(), undefined, {sensitivity: 'base'});
-  }
 }
 
 registry.register(

core/workspace.ts

@@ -25,6 +25,7 @@ import type {IConnectionChecker} from './interfaces/i_connection_checker.js';
 import {Options} from './options.js';
 import * as registry from './registry.js';
 import * as arrayUtils from './utils/array.js';
+import * as deprecation from './utils/deprecation.js';
 import * as idGenerator from './utils/idgenerator.js';
 import * as math from './utils/math.js';
 import type * as toolbox from './utils/toolbox.js';
@@ -381,13 +382,19 @@ export class Workspace implements IASTNodeLocation {
 
   /* Begin functions that are just pass-throughs to the variable map. */
   /**
-   * Rename a variable by updating its name in the variable map. Identify the
-   * variable to rename with the given ID.
+   * @deprecated v12 - Rename a variable by updating its name in the variable
+   * map. Identify the variable to rename with the given ID.
    *
    * @param id ID of the variable to rename.
    * @param newName New variable name.
    */
   renameVariableById(id: string, newName: string) {
+    deprecation.warn(
+      'Blockly.Workspace.renameVariableById',
+      'v12',
+      'v13',
+      'Blockly.Workspace.getVariableMap().renameVariable',
+    );
     const variable = this.variableMap.getVariableById(id);
     if (!variable) return;
     this.variableMap.renameVariable(variable, newName);
@@ -396,6 +403,7 @@ export class Workspace implements IASTNodeLocation {
   /**
    * Create a variable with a given name, optional type, and optional ID.
    *
+   * @deprecated v12, use Blockly.Workspace.getVariableMap().createVariable.
    * @param name The name of the variable. This must be unique across variables
    *     and procedures.
    * @param opt_type The type of the variable like 'int' or 'string'.
@@ -409,6 +417,12 @@ export class Workspace implements IASTNodeLocation {
     opt_type?: string | null,
     opt_id?: string | null,
   ): IVariableModel<IVariableState> {
+    deprecation.warn(
+      'Blockly.Workspace.createVariable',
+      'v12',
+      'v13',
+      'Blockly.Workspace.getVariableMap().createVariable',
+    );
     return this.variableMap.createVariable(
       name,
       opt_type ?? undefined,
@@ -419,20 +433,34 @@ export class Workspace implements IASTNodeLocation {
   /**
    * Find all the uses of the given variable, which is identified by ID.
    *
+   * @deprecated v12, use Blockly.Workspace.getVariableMap().getVariableUsesById
    * @param id ID of the variable to find.
    * @returns Array of block usages.
    */
   getVariableUsesById(id: string): Block[] {
+    deprecation.warn(
+      'Blockly.Workspace.getVariableUsesById',
+      'v12',
+      'v13',
+      'Blockly.Workspace.getVariableMap().getVariableUsesById',
+    );
     return Variables.getVariableUsesById(this, id);
   }
 
   /**
    * Delete a variables by the passed in ID and all of its uses from this
    * workspace. May prompt the user for confirmation.
    *
+   * @deprecated v12, use Blockly.Workspace.getVariableMap().deleteVariable.
    * @param id ID of variable to delete.
    */
   deleteVariableById(id: string) {
+    deprecation.warn(
+      'Blockly.Workspace.deleteVariableById',
+      'v12',
+      'v13',
+      'Blockly.Workspace.getVariableMap().deleteVariable',
+    );
     const variable = this.variableMap.getVariableById(id);
     if (!variable) {
       console.warn(`Can't delete non-existent variable: ${id}`);
@@ -445,6 +473,7 @@ export class Workspace implements IASTNodeLocation {
    * Find the variable by the given name and return it. Return null if not
    * found.
    *
+   * @deprecated v12, use Blockly.Workspace.getVariableMap().getVariable.
    * @param name The name to check for.
    * @param opt_type The type of the variable.  If not provided it defaults to
    *     the empty string, which is a specific type.
@@ -454,62 +483,81 @@ export class Workspace implements IASTNodeLocation {
     name: string,
     opt_type?: string,
   ): IVariableModel<IVariableState> | null {
+    deprecation.warn(
+      'Blockly.Workspace.getVariable',
+      'v12',
+      'v13',
+      'Blockly.Workspace.getVariableMap().getVariable',
+    );
     // TODO (#1559): Possibly delete this function after resolving #1559.
     return this.variableMap.getVariable(name, opt_type);
   }
 
   /**
    * Find the variable by the given ID and return it. Return null if not found.
    *
+   * @deprecated v12, use Blockly.Workspace.getVariableMap().getVariableById.
    * @param id The ID to check for.
    * @returns The variable with the given ID.
    */
   getVariableById(id: string): IVariableModel<IVariableState> | null {
+    deprecation.warn(
+      'Blockly.Workspace.getVariableById',
+      'v12',
+      'v13',
+      'Blockly.Workspace.getVariableMap().getVariableById',
+    );
     return this.variableMap.getVariableById(id);
   }
 
   /**
    * Find the variable with the specified type. If type is null, return list of
    *     variables with empty string type.
    *
+   * @deprecated v12, use Blockly.Workspace.getVariableMap().getVariablesOfType.
    * @param type Type of the variables to find.
    * @returns The sought after variables of the passed in type. An empty array
    *     if none are found.
    */
   getVariablesOfType(type: string | null): IVariableModel<IVariableState>[] {
-    return this.variableMap.getVariablesOfType(type ?? '');
-  }
-
-  /**
-   * Return all variable types.
-   *
-   * @returns List of variable types.
-   * @internal
-   */
-  getVariableTypes(): string[] {
-    const variableTypes = new Set<string>(this.variableMap.getTypes());
-    (this.potentialVariableMap?.getTypes() ?? []).forEach((t) =>
-      variableTypes.add(t),
+    deprecation.warn(
+      'Blockly.Workspace.getVariablesOfType',
+      'v12',
+      'v13',
+      'Blockly.Workspace.getVariableMap().getVariablesOfType',
     );
-    variableTypes.add('');
-    return Array.from(variableTypes.values());
+    return this.variableMap.getVariablesOfType(type ?? '');
   }
 
   /**
    * Return all variables of all types.
    *
+   * @deprecated v12, use Blockly.Workspace.getVariableMap().getAllVariables.
    * @returns List of variable models.
    */
   getAllVariables(): IVariableModel<IVariableState>[] {
+    deprecation.warn(
+      'Blockly.Workspace.getAllVariables',
+      'v12',
+      'v13',
+      'Blockly.Workspace.getVariableMap().getAllVariables',
+    );
     return this.variableMap.getAllVariables();
   }
 
   /**
    * Returns all variable names of all types.
    *
+   * @deprecated v12, use Blockly.Workspace.getVariableMap().getAllVariables.
    * @returns List of all variable names of all types.
    */
   getAllVariableNames(): string[] {
+    deprecation.warn(
+      'Blockly.Workspace.getAllVariableNames',
+      'v12',
+      'v13',
+      'Blockly.Workspace.getVariableMap().getAllVariables',
+    );
     return this.variableMap.getAllVariables().map((v) => v.getName());
   }
   /* End functions that are just pass-throughs to the variable map. */

tests/mocha/field_variable_test.js

@@ -388,8 +388,7 @@ suite('Variable Fields', function () {
       fieldVariable.variableTypes = null;
 
       const resultTypes = fieldVariable.getVariableTypes();
-      // The empty string is always one of the options.
-      assert.deepEqual(resultTypes, ['type1', 'type2', '']);
+      assert.deepEqual(resultTypes, ['type1', 'type2']);
     });
     test('variableTypes is the empty list', function () {
       const fieldVariable = new Blockly.FieldVariable('name1');

tests/mocha/variable_map_test.js

@@ -187,24 +187,6 @@ suite('Variable Map', function () {
     });
   });
 
-  suite('getVariableTypes', function () {
-    test('Trivial', function () {
-      this.variableMap.createVariable('name1', 'type1', 'id1');
-      this.variableMap.createVariable('name2', 'type1', 'id2');
-      this.variableMap.createVariable('name3', 'type2', 'id3');
-      this.variableMap.createVariable('name4', 'type3', 'id4');
-      const resultArray = this.variableMap.getVariableTypes();
-      // The empty string is always an option.
-      assert.deepEqual(resultArray, ['type1', 'type2', 'type3', '']);
-    });
-
-    test('None', function () {
-      // The empty string is always an option.
-      const resultArray = this.variableMap.getVariableTypes();
-      assert.deepEqual(resultArray, ['']);
-    });
-  });
-
   suite('getVariablesOfType', function () {
     test('Trivial', function () {
       const var1 = this.variableMap.createVariable('name1', 'type1', 'id1');