fix: Remove references to deprecated variable-related methods (#9572)

blocks/procedures.ts

@@ -308,7 +308,9 @@ const PROCEDURE_DEF_COMMON = {
     while (paramBlock && !paramBlock.isInsertionMarker()) {
       const varName = paramBlock.getFieldValue('NAME');
       this.arguments_.push(varName);
-      const variable = this.workspace.getVariable(varName, '')!;
+      const variable = this.workspace
+        .getVariableMap()
+        .getVariable(varName, '')!;
       this.argumentVarModels_.push(variable);
 
       this.paramIds_.push(paramBlock.id);
@@ -374,13 +376,13 @@ const PROCEDURE_DEF_COMMON = {
     oldId: string,
     newId: string,
   ) {
-    const oldVariable = this.workspace.getVariableById(oldId)!;
+    const oldVariable = this.workspace.getVariableMap().getVariableById(oldId)!;
     if (oldVariable.getType() !== '') {
       // Procedure arguments always have the empty type.
       return;
     }
     const oldName = oldVariable.getName();
-    const newVar = this.workspace.getVariableById(newId)!;
+    const newVar = this.workspace.getVariableMap().getVariableById(newId)!;
 
     let change = false;
     for (let i = 0; i < this.argumentVarModels_.length; i++) {

core/block.ts

@@ -1161,9 +1161,9 @@ export class Block {
     const vars = [];
     for (const field of this.getFields()) {
       if (field.referencesVariables()) {
-        const model = this.workspace.getVariableById(
-          field.getValue() as string,
-        );
+        const model = this.workspace
+          .getVariableMap()
+          .getVariableById(field.getValue() as string);
         // Check if the variable actually exists (and isn't just a potential
         // variable).
         if (model) {

core/flyout_base.ts

@@ -813,7 +813,9 @@ export abstract class Flyout
   createBlock(originalBlock: BlockSvg): BlockSvg {
     let newBlock = null;
     eventUtils.disable();
-    const variablesBeforeCreation = this.targetWorkspace.getAllVariables();
+    const variablesBeforeCreation = this.targetWorkspace
+      .getVariableMap()
+      .getAllVariables();
     this.targetWorkspace.setResizesEnabled(false);
     try {
       newBlock = this.placeNewBlock(originalBlock);

core/serialization/blocks.ts

@@ -391,7 +391,7 @@ export function appendInternal(
   }
   eventUtils.disable();
 
-  const variablesBeforeCreation = workspace.getAllVariables();
+  const variablesBeforeCreation = workspace.getVariableMap().getAllVariables();
   let block;
   try {
     block = appendPrivate(state, workspace, {parentConnection, isShadow});

core/serialization/variables.ts

@@ -32,7 +32,10 @@ export class VariableSerializer implements ISerializer {
    *     variables.
    */
   save(workspace: Workspace): IVariableState[] | null {
-    const variableStates = workspace.getAllVariables().map((v) => v.save());
+    const variableStates = workspace
+      .getVariableMap()
+      .getAllVariables()
+      .map((v) => v.save());
     return variableStates.length ? variableStates : null;
   }
 

core/variables.ts

@@ -727,7 +727,7 @@ export function getVariable(
   // Try to just get the variable, by ID if possible.
   if (id) {
     // Look in the real variable map before checking the potential variable map.
-    variable = workspace.getVariableById(id);
+    variable = workspace.getVariableMap().getVariableById(id);
     if (!variable && potentialVariableMap) {
       variable = potentialVariableMap.getVariableById(id);
     }
@@ -742,7 +742,7 @@ export function getVariable(
       throw Error('Tried to look up a variable by name without a type');
     }
     // Otherwise look up by name and type.
-    variable = workspace.getVariable(opt_name, opt_type);
+    variable = workspace.getVariableMap().getVariable(opt_name, opt_type);
     if (!variable && potentialVariableMap) {
       variable = potentialVariableMap.getVariable(opt_name, opt_type);
     }
@@ -809,7 +809,7 @@ export function getAddedVariables(
   workspace: Workspace,
   originalVariables: IVariableModel<IVariableState>[],
 ): IVariableModel<IVariableState>[] {
-  const allCurrentVariables = workspace.getAllVariables();
+  const allCurrentVariables = workspace.getVariableMap().getAllVariables();
   const addedVariables = [];
   if (originalVariables.length !== allCurrentVariables.length) {
     for (let i = 0; i < allCurrentVariables.length; i++) {

core/xml.ts

@@ -635,7 +635,7 @@ export function domToBlockInternal(
 ): Block {
   // Create top-level block.
   eventUtils.disable();
-  const variablesBeforeCreation = workspace.getAllVariables();
+  const variablesBeforeCreation = workspace.getVariableMap().getAllVariables();
   let topBlock;
   try {
     topBlock = domToBlockHeadless(xmlBlock, workspace);

tests/mocha/block_test.js

@@ -2450,7 +2450,7 @@ suite('Blocks', function () {
         const blockA = createRenderedBlock(this.workspace, 'variable_block');
 
         blockA.setCollapsed(true);
-        const variable = this.workspace.getVariable('x', '');
+        const variable = this.workspace.getVariableMap().getVariable('x', '');
         this.variableMap.renameVariable(variable, 'y');
 
         this.clock.runAll();

tests/mocha/blocks/procedures_test.js

@@ -26,12 +26,12 @@ suite('Procedures', function () {
   setup(function () {
     sharedTestSetup.call(this, {fireEventsNow: false});
     this.workspace = Blockly.inject('blocklyDiv', {});
-    this.workspace.createVariable('preCreatedVar', '', 'preCreatedVarId');
-    this.workspace.createVariable(
-      'preCreatedTypedVar',
-      'type',
-      'preCreatedTypedVarId',
-    );
+    this.workspace
+      .getVariableMap()
+      .createVariable('preCreatedVar', '', 'preCreatedVarId');
+    this.workspace
+      .getVariableMap()
+      .createVariable('preCreatedTypedVar', 'type', 'preCreatedTypedVarId');
     defineRowBlock();
     this.variableMap = this.workspace.getVariableMap();
   });
@@ -433,7 +433,7 @@ suite('Procedures', function () {
       this.clock.runAll();
 
       assert.isNotNull(
-        this.workspace.getVariable('param1', ''),
+        this.workspace.getVariableMap().getVariable('param1', ''),
         'Expected the old variable to continue to exist',
       );
     });
@@ -453,7 +453,9 @@ suite('Procedures', function () {
       this.clock.runAll();
       mutatorIcon.setBubbleVisible(false);
 
-      const variable = this.workspace.getVariable('param1', '');
+      const variable = this.workspace
+        .getVariableMap()
+        .getVariable('param1', '');
       this.variableMap.renameVariable(variable, 'new name');
 
       assert.isNotNull(
@@ -480,7 +482,9 @@ suite('Procedures', function () {
         .connection.connect(paramBlock.previousConnection);
       this.clock.runAll();
 
-      const variable = this.workspace.getVariable('param1', '');
+      const variable = this.workspace
+        .getVariableMap()
+        .getVariable('param1', '');
       this.variableMap.renameVariable(variable, 'new name');
 
       assert.equal(
@@ -506,7 +510,9 @@ suite('Procedures', function () {
       this.clock.runAll();
       mutatorIcon.setBubbleVisible(false);
 
-      const variable = this.workspace.getVariable('param1', '');
+      const variable = this.workspace
+        .getVariableMap()
+        .getVariable('param1', '');
       this.variableMap.renameVariable(variable, 'new name');
 
       assert.isNotNull(
@@ -535,7 +541,9 @@ suite('Procedures', function () {
       this.clock.runAll();
       mutatorIcon.setBubbleVisible(false);
 
-      const variable = this.workspace.getVariable('param1', '');
+      const variable = this.workspace
+        .getVariableMap()
+        .getVariable('param1', '');
       this.variableMap.renameVariable(variable, 'preCreatedVar');
 
       assert.isNotNull(
@@ -562,7 +570,9 @@ suite('Procedures', function () {
         .connection.connect(paramBlock.previousConnection);
       this.clock.runAll();
 
-      const variable = this.workspace.getVariable('param1', '');
+      const variable = this.workspace
+        .getVariableMap()
+        .getVariable('param1', '');
       this.variableMap.renameVariable(variable, 'preCreatedVar');
 
       assert.equal(
@@ -588,7 +598,9 @@ suite('Procedures', function () {
       this.clock.runAll();
       mutatorIcon.setBubbleVisible(false);
 
-      const variable = this.workspace.getVariable('param1', '');
+      const variable = this.workspace
+        .getVariableMap()
+        .getVariable('param1', '');
       this.variableMap.renameVariable(variable, 'preCreatedVar');
 
       assert.isNotNull(

tests/mocha/event_test.js

@@ -833,11 +833,9 @@ suite('Events', function () {
         title: 'Variable events',
         testCases: variableEventTestCases,
         setup: (thisObj) => {
-          thisObj.variable = thisObj.workspace.createVariable(
-            'name1',
-            'type1',
-            'id1',
-          );
+          thisObj.variable = thisObj.workspace
+            .getVariableMap()
+            .createVariable('name1', 'type1', 'id1');
         },
       },
       {
@@ -1550,7 +1548,9 @@ suite('Events', function () {
       );
 
       // Expect the workspace to have a variable with ID 'test_var_id'.
-      assert.isNotNull(this.workspace.getVariableById(TEST_VAR_ID));
+      assert.isNotNull(
+        this.workspace.getVariableMap().getVariableById(TEST_VAR_ID),
+      );
     });
   });
   suite('Disable orphans', function () {

tests/mocha/field_variable_test.js

@@ -171,7 +171,7 @@ suite('Variable Fields', function () {
 
   suite('setValue', function () {
     setup(function () {
-      this.workspace.createVariable('name2', null, 'id2');
+      this.workspace.getVariableMap().createVariable('name2', null, 'id2');
       this.field = new Blockly.FieldVariable(null);
       initVariableField(this.workspace, this.field);
 
@@ -209,8 +209,8 @@ suite('Variable Fields', function () {
       assert.include(dropdownOptions[dropdownOptions.length - 1][0], 'Delete');
     };
     test('Contains variables created before field', function () {
-      this.workspace.createVariable('name1', '', 'id1');
-      this.workspace.createVariable('name2', '', 'id2');
+      this.workspace.getVariableMap().createVariable('name1', '', 'id1');
+      this.workspace.getVariableMap().createVariable('name2', '', 'id2');
       // Expect that the dropdown options will contain the variables that exist
       const fieldVariable = initVariableField(
         this.workspace,
@@ -228,22 +228,22 @@ suite('Variable Fields', function () {
         new Blockly.FieldVariable('name1'),
       );
       // Expect that variables created after field creation will show up too.
-      this.workspace.createVariable('name2', '', 'id2');
+      this.workspace.getVariableMap().createVariable('name2', '', 'id2');
       assertDropdownContents(fieldVariable, [
         ['name1', 'id1'],
         ['name2', 'id2'],
       ]);
     });
     test('Contains variables created before and after field', function () {
-      this.workspace.createVariable('name1', '', 'id1');
-      this.workspace.createVariable('name2', '', 'id2');
+      this.workspace.getVariableMap().createVariable('name1', '', 'id1');
+      this.workspace.getVariableMap().createVariable('name2', '', 'id2');
       // Expect that the dropdown options will contain the variables that exist
       const fieldVariable = initVariableField(
         this.workspace,
         new Blockly.FieldVariable('name1'),
       );
       // Expect that variables created after field creation will show up too.
-      this.workspace.createVariable('name3', '', 'id3');
+      this.workspace.getVariableMap().createVariable('name3', '', 'id3');
       assertDropdownContents(fieldVariable, [
         ['name1', 'id1'],
         ['name2', 'id2'],
@@ -254,9 +254,9 @@ suite('Variable Fields', function () {
 
   suite('Validators', function () {
     setup(function () {
-      this.workspace.createVariable('name1', null, 'id1');
-      this.workspace.createVariable('name2', null, 'id2');
-      this.workspace.createVariable('name3', null, 'id3');
+      this.workspace.getVariableMap().createVariable('name1', null, 'id1');
+      this.workspace.getVariableMap().createVariable('name2', null, 'id2');
+      this.workspace.getVariableMap().createVariable('name3', null, 'id3');
       this.variableField = initVariableField(
         this.workspace,
         new Blockly.FieldVariable('name1'),
@@ -281,7 +281,9 @@ suite('Variable Fields', function () {
       });
       test('New Value', function () {
         // Must create the var so that the field doesn't throw an error.
-        this.workspace.createVariable('thing2', null, 'other2');
+        this.workspace
+          .getVariableMap()
+          .createVariable('thing2', null, 'other2');
         this.variableField.setValue('other2');
         assertFieldValue(this.variableField, 'id2', 'name2');
       });
@@ -368,8 +370,8 @@ suite('Variable Fields', function () {
   });
   suite('Get variable types', function () {
     setup(function () {
-      this.workspace.createVariable('name1', 'type1');
-      this.workspace.createVariable('name2', 'type2');
+      this.workspace.getVariableMap().createVariable('name1', 'type1');
+      this.workspace.getVariableMap().createVariable('name2', 'type2');
     });
     test('variableTypes is explicit', function () {
       // Expect that since variableTypes is defined, it will be the return
@@ -467,7 +469,7 @@ suite('Variable Fields', function () {
   });
   suite('Renaming Variables', function () {
     setup(function () {
-      this.workspace.createVariable('name1', null, 'id1');
+      this.workspace.getVariableMap().createVariable('name1', null, 'id1');
       Blockly.defineBlocksWithJsonArray([
         {
           'type': 'field_variable_test_block',
@@ -584,7 +586,7 @@ suite('Variable Fields', function () {
     });
 
     test('ID', function () {
-      this.workspace.createVariable('test', '', 'id1');
+      this.workspace.getVariableMap().createVariable('test', '', 'id1');
       const block = Blockly.serialization.blocks.append(
         {
           'type': 'variables_get',

tests/mocha/test_helpers/procedures.js

@@ -18,7 +18,9 @@ import {assert} from '../../../node_modules/chai/index.js';
 function assertBlockVarModels(block, varIds) {
   const expectedVarModels = [];
   for (let i = 0; i < varIds.length; i++) {
-    expectedVarModels.push(block.workspace.getVariableById(varIds[i]));
+    expectedVarModels.push(
+      block.workspace.getVariableMap().getVariableById(varIds[i]),
+    );
   }
   assert.sameDeepOrderedMembers(block.getVarModels(), expectedVarModels);
 }

tests/mocha/test_helpers/variables.js

@@ -15,7 +15,11 @@ import {assert} from '../../../node_modules/chai/index.js';
  * @param {!string} id The expected id of the variable.
  */
 export function assertVariableValues(container, name, type, id) {
-  const variable = container.getVariableById(id);
+  const variableMap =
+    container instanceof Blockly.Workspace
+      ? container.getVariableMap()
+      : container;
+  const variable = variableMap.getVariableById(id);
   assert.isDefined(variable);
   assert.equal(variable.name, name);
   assert.equal(variable.type, type);

tests/mocha/test_helpers/workspace.js

@@ -8,7 +8,6 @@ import * as eventUtils from '../../../build/src/core/events/utils.js';
 import {assert} from '../../../node_modules/chai/index.js';
 import {workspaceTeardown} from './setup_teardown.js';
 import {assertVariableValues} from './variables.js';
-import {assertWarnings} from './warnings.js';
 
 export function testAWorkspace() {
   setup(function () {
@@ -102,7 +101,8 @@ export function testAWorkspace() {
     test('deleteVariableById(id2) one usage', function () {
       // Deleting variable one usage should not trigger confirm dialog.
       const stub = sinon.stub(window, 'confirm').returns(true);
-      this.variableMap.deleteVariableById('id2');
+      const id2 = this.variableMap.getVariableById('id2');
+      Blockly.Variables.deleteVariable(this.workspace, id2);
 
       sinon.assert.notCalled(stub);
       const variable = this.variableMap.getVariableById('id2');
@@ -116,7 +116,8 @@ export function testAWorkspace() {
     test('deleteVariableById(id1) multiple usages confirm', function () {
       // Deleting variable with multiple usages triggers confirm dialog.
       const stub = sinon.stub(window, 'confirm').returns(true);
-      this.variableMap.deleteVariableById('id1');
+      const id1 = this.variableMap.getVariableById('id1');
+      Blockly.Variables.deleteVariable(this.workspace, id1);
 
       sinon.assert.calledOnce(stub);
       const variable = this.variableMap.getVariableById('id1');
@@ -130,7 +131,8 @@ export function testAWorkspace() {
     test('deleteVariableById(id1) multiple usages cancel', function () {
       // Deleting variable with multiple usages triggers confirm dialog.
       const stub = sinon.stub(window, 'confirm').returns(false);
-      this.variableMap.deleteVariableById('id1');
+      const id1 = this.variableMap.getVariableById('id1');
+      Blockly.Variables.deleteVariable(this.workspace, id1);
 
       sinon.assert.calledOnce(stub);
       assertVariableValues(this.variableMap, 'name1', 'type1', 'id1');
@@ -143,13 +145,14 @@ export function testAWorkspace() {
     });
   });
 
-  suite('renameVariableById', function () {
+  suite('renameVariable', function () {
     setup(function () {
       this.variableMap.createVariable('name1', 'type1', 'id1');
     });
 
     test('No references rename to name2', function () {
-      this.variableMap.renameVariableById('id1', 'name2');
+      const id1 = this.variableMap.getVariableById('id1');
+      this.variableMap.renameVariable(id1, 'name2');
       assertVariableValues(this.variableMap, 'name2', 'type1', 'id1');
       // Renaming should not have created a new variable.
       assert.equal(this.variableMap.getAllVariables().length, 1);
@@ -158,7 +161,8 @@ export function testAWorkspace() {
     test('Reference exists rename to name2', function () {
       createVarBlocksNoEvents(this.workspace, ['id1']);
 
-      this.variableMap.renameVariableById('id1', 'name2');
+      const id1 = this.variableMap.getVariableById('id1');
+      this.variableMap.renameVariable(id1, 'name2');
       assertVariableValues(this.variableMap, 'name2', 'type1', 'id1');
       // Renaming should not have created a new variable.
       assert.equal(this.variableMap.getAllVariables().length, 1);
@@ -168,7 +172,8 @@ export function testAWorkspace() {
     test('Reference exists different capitalization rename to Name1', function () {
       createVarBlocksNoEvents(this.workspace, ['id1']);
 
-      this.variableMap.renameVariableById('id1', 'Name1');
+      const id1 = this.variableMap.getVariableById('id1');
+      this.variableMap.renameVariable(id1, 'Name1');
       assertVariableValues(this.variableMap, 'Name1', 'type1', 'id1');
       // Renaming should not have created a new variable.
       assert.equal(this.variableMap.getAllVariables().length, 1);
@@ -180,7 +185,8 @@ export function testAWorkspace() {
         this.variableMap.createVariable('name2', 'type1', 'id2');
         createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
 
-        this.variableMap.renameVariableById('id1', 'name2');
+        const id1 = this.variableMap.getVariableById('id1');
+        this.variableMap.renameVariable(id1, 'name2');
 
         // The second variable should remain unchanged.
         assertVariableValues(this.workspace, 'name2', 'type1', 'id2');
@@ -199,7 +205,8 @@ export function testAWorkspace() {
         this.variableMap.createVariable('name2', 'type2', 'id2');
         createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
 
-        this.variableMap.renameVariableById('id1', 'name2');
+        const id1 = this.variableMap.getVariableById('id1');
+        this.variableMap.renameVariable(id1, 'name2');
 
         // Variables with different type are allowed to have the same name.
         assertVariableValues(this.variableMap, 'name2', 'type1', 'id1');
@@ -214,7 +221,8 @@ export function testAWorkspace() {
         this.variableMap.createVariable('name2', 'type1', 'id2');
         createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
 
-        this.variableMap.renameVariableById('id1', 'Name2');
+        const id1 = this.variableMap.getVariableById('id1');
+        this.variableMap.renameVariable(id1, 'Name2');
 
         // The second variable should be updated.
         assertVariableValues(this.variableMap, 'Name2', 'type1', 'id2');
@@ -233,7 +241,8 @@ export function testAWorkspace() {
         this.variableMap.createVariable('name2', 'type2', 'id2');
         createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
 
-        this.variableMap.renameVariableById('id1', 'Name2');
+        const id1 = this.variableMap.getVariableById('id1');
+        this.variableMap.renameVariable(id1, 'Name2');
 
         // Variables with different type are allowed to have the same name.
         assertVariableValues(this.variableMap, 'Name2', 'type1', 'id1');
@@ -1250,8 +1259,8 @@ export function testAWorkspace() {
 
     suite('Variables', function () {
       function createTwoVarsDifferentTypes(workspace) {
-        workspace.createVariable('name1', 'type1', 'id1');
-        workspace.createVariable('name2', 'type2', 'id2');
+        workspace.getVariableMap().createVariable('name1', 'type1', 'id1');
+        workspace.getVariableMap().createVariable('name2', 'type2', 'id2');
       }
 
       suite('createVariable', function () {
@@ -1262,11 +1271,11 @@ export function testAWorkspace() {
           this.workspace.undo();
           this.clock.runAll();
           assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
-          assert.isNull(this.workspace.getVariableById('id2'));
+          assert.isNull(this.variableMap.getVariableById('id2'));
 
           this.workspace.undo();
-          assert.isNull(this.workspace.getVariableById('id1'));
-          assert.isNull(this.workspace.getVariableById('id2'));
+          assert.isNull(this.variableMap.getVariableById('id1'));
+          assert.isNull(this.variableMap.getVariableById('id2'));
         });
 
         test('Undo and redo', function () {
@@ -1276,7 +1285,7 @@ export function testAWorkspace() {
           this.workspace.undo();
           this.clock.runAll();
           assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
-          assert.isNull(this.workspace.getVariableById('id2'));
+          assert.isNull(this.variableMap.getVariableById('id2'));
 
           this.workspace.undo(true);
 
@@ -1286,13 +1295,13 @@ export function testAWorkspace() {
 
           this.workspace.undo();
           this.workspace.undo();
-          assert.isNull(this.workspace.getVariableById('id1'));
-          assert.isNull(this.workspace.getVariableById('id2'));
+          assert.isNull(this.variableMap.getVariableById('id1'));
+          assert.isNull(this.variableMap.getVariableById('id2'));
           this.workspace.undo(true);
 
           // Expect that variable 'id1' is recreated
           assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
-          assert.isNull(this.workspace.getVariableById('id2'));
+          assert.isNull(this.variableMap.getVariableById('id2'));
         });
       });
 
@@ -1300,13 +1309,15 @@ export function testAWorkspace() {
         test('Undo only no usages', function () {
           createTwoVarsDifferentTypes(this.workspace);
           this.clock.runAll();
-          this.workspace.deleteVariableById('id1');
-          this.workspace.deleteVariableById('id2');
+          const id1 = this.variableMap.getVariableById('id1');
+          Blockly.Variables.deleteVariable(this.workspace, id1);
+          const id2 = this.variableMap.getVariableById('id2');
+          Blockly.Variables.deleteVariable(this.workspace, id2);
           this.clock.runAll();
 
           this.workspace.undo();
           this.clock.runAll();
-          assert.isNull(this.workspace.getVariableById('id1'));
+          assert.isNull(this.variableMap.getVariableById('id1'));
           assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
 
           this.workspace.undo();
@@ -1320,14 +1331,16 @@ export function testAWorkspace() {
           // Create blocks to refer to both of them.
           createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
           this.clock.runAll();
-          this.workspace.deleteVariableById('id1');
-          this.workspace.deleteVariableById('id2');
+          const id1 = this.variableMap.getVariableById('id1');
+          Blockly.Variables.deleteVariable(this.workspace, id1);
+          const id2 = this.variableMap.getVariableById('id2');
+          Blockly.Variables.deleteVariable(this.workspace, id2);
           this.clock.runAll();
 
           this.workspace.undo();
           this.clock.runAll();
           assertBlockVarModelName(this.workspace, 0, 'name2');
-          assert.isNull(this.workspace.getVariableById('id1'));
+          assert.isNull(this.variableMap.getVariableById('id1'));
           assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
 
           this.workspace.undo();
@@ -1341,20 +1354,22 @@ export function testAWorkspace() {
         test('Reference exists no usages', function () {
           createTwoVarsDifferentTypes(this.workspace);
           this.clock.runAll();
-          this.workspace.deleteVariableById('id1');
-          this.workspace.deleteVariableById('id2');
+          const id1 = this.variableMap.getVariableById('id1');
+          Blockly.Variables.deleteVariable(this.workspace, id1);
+          const id2 = this.variableMap.getVariableById('id2');
+          Blockly.Variables.deleteVariable(this.workspace, id2);
           this.clock.runAll();
 
           this.workspace.undo();
           this.clock.runAll();
-          assert.isNull(this.workspace.getVariableById('id1'));
+          assert.isNull(this.variableMap.getVariableById('id1'));
           assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
 
           this.workspace.undo(true);
           this.clock.runAll();
           // Expect that both variables are deleted
-          assert.isNull(this.workspace.getVariableById('id1'));
-          assert.isNull(this.workspace.getVariableById('id2'));
+          assert.isNull(this.variableMap.getVariableById('id1'));
+          assert.isNull(this.variableMap.getVariableById('id2'));
 
           this.workspace.undo();
           this.clock.runAll();
@@ -1366,7 +1381,7 @@ export function testAWorkspace() {
           this.workspace.undo(true);
           this.clock.runAll();
           // Expect that variable 'id2' is recreated
-          assert.isNull(this.workspace.getVariableById('id1'));
+          assert.isNull(this.variableMap.getVariableById('id1'));
           assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
         });
 
@@ -1375,22 +1390,24 @@ export function testAWorkspace() {
           // Create blocks to refer to both of them.
           createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
           this.clock.runAll();
-          this.workspace.deleteVariableById('id1');
-          this.workspace.deleteVariableById('id2');
+          const id1 = this.variableMap.getVariableById('id1');
+          Blockly.Variables.deleteVariable(this.workspace, id1);
+          const id2 = this.variableMap.getVariableById('id2');
+          Blockly.Variables.deleteVariable(this.workspace, id2);
           this.clock.runAll();
 
           this.workspace.undo();
           this.clock.runAll();
           assertBlockVarModelName(this.workspace, 0, 'name2');
-          assert.isNull(this.workspace.getVariableById('id1'));
+          assert.isNull(this.variableMap.getVariableById('id1'));
           assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
 
           this.workspace.undo(true);
           this.clock.runAll();
           // Expect that both variables are deleted
           assert.equal(this.workspace.getTopBlocks(false).length, 0);
-          assert.isNull(this.workspace.getVariableById('id1'));
-          assert.isNull(this.workspace.getVariableById('id2'));
+          assert.isNull(this.variableMap.getVariableById('id1'));
+          assert.isNull(this.variableMap.getVariableById('id2'));
 
           this.workspace.undo();
           this.clock.runAll();
@@ -1405,84 +1422,19 @@ export function testAWorkspace() {
           this.clock.runAll();
           // Expect that variable 'id2' is recreated
           assertBlockVarModelName(this.workspace, 0, 'name2');
-          assert.isNull(this.workspace.getVariableById('id1'));
+          assert.isNull(this.variableMap.getVariableById('id1'));
           assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
         });
-
-        test('Delete same variable twice no usages', function () {
-          this.workspace.createVariable('name1', 'type1', 'id1');
-          this.workspace.deleteVariableById('id1');
-          this.clock.runAll();
-          const workspace = this.workspace;
-          assertWarnings(() => {
-            workspace.deleteVariableById('id1');
-          }, /Can't delete/);
-
-          // Check the undoStack only recorded one delete event.
-          const undoStack = this.workspace.undoStack_;
-          assert.equal(undoStack[undoStack.length - 1].type, 'var_delete');
-          assert.notEqual(undoStack[undoStack.length - 2].type, 'var_delete');
-
-          // Undo delete
-          this.workspace.undo();
-          this.clock.runAll();
-          assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
-
-          // Redo delete
-          this.workspace.undo(true);
-          this.clock.runAll();
-          assert.isNull(this.workspace.getVariableById('id1'));
-
-          // Redo delete, nothing should happen
-          this.workspace.undo(true);
-          this.clock.runAll();
-          assert.isNull(this.workspace.getVariableById('id1'));
-        });
-
-        test('Delete same variable twice with usages', function () {
-          this.workspace.createVariable('name1', 'type1', 'id1');
-          createVarBlocksNoEvents(this.workspace, ['id1']);
-          this.clock.runAll();
-          this.workspace.deleteVariableById('id1');
-          this.clock.runAll();
-          const workspace = this.workspace;
-          assertWarnings(() => {
-            workspace.deleteVariableById('id1');
-          }, /Can't delete/);
-
-          // Check the undoStack only recorded one delete event.
-          const undoStack = this.workspace.undoStack_;
-          assert.equal(undoStack[undoStack.length - 1].type, 'var_delete');
-          assert.equal(undoStack[undoStack.length - 2].type, 'delete');
-          assert.notEqual(undoStack[undoStack.length - 3].type, 'var_delete');
-
-          // Undo delete
-          this.workspace.undo();
-          this.clock.runAll();
-          assertBlockVarModelName(this.workspace, 0, 'name1');
-          assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
-
-          // Redo delete
-          this.workspace.undo(true);
-          this.clock.runAll();
-          assert.equal(this.workspace.getTopBlocks(false).length, 0);
-          assert.isNull(this.workspace.getVariableById('id1'));
-
-          // Redo delete, nothing should happen
-          this.workspace.undo(true);
-          this.clock.runAll();
-          assert.equal(this.workspace.getTopBlocks(false).length, 0);
-          assert.isNull(this.workspace.getVariableById('id1'));
-        });
       });
 
-      suite('renameVariableById', function () {
+      suite('renameVariable', function () {
         setup(function () {
           this.variableMap.createVariable('name1', 'type1', 'id1');
         });
 
         test('Reference exists no usages rename to name2', function () {
-          this.variableMap.renameVariableById('id1', 'name2');
+          const id1 = this.variableMap.getVariableById('id1');
+          this.variableMap.renameVariable(id1, 'name2');
           this.clock.runAll();
 
           this.workspace.undo();
@@ -1496,7 +1448,8 @@ export function testAWorkspace() {
 
         test('Reference exists with usages rename to name2', function () {
           createVarBlocksNoEvents(this.workspace, ['id1']);
-          this.variableMap.renameVariableById('id1', 'name2');
+          const id1 = this.variableMap.getVariableById('id1');
+          this.variableMap.renameVariable(id1, 'name2');
           this.clock.runAll();
 
           this.workspace.undo();
@@ -1511,7 +1464,8 @@ export function testAWorkspace() {
         });
 
         test('Reference exists different capitalization no usages rename to Name1', function () {
-          this.variableMap.renameVariableById('id1', 'Name1');
+          const id1 = this.variableMap.getVariableById('id1');
+          this.variableMap.renameVariable(id1, 'Name1');
           this.clock.runAll();
 
           this.workspace.undo();
@@ -1525,7 +1479,8 @@ export function testAWorkspace() {
 
         test('Reference exists different capitalization with usages rename to Name1', function () {
           createVarBlocksNoEvents(this.workspace, ['id1']);
-          this.variableMap.renameVariableById('id1', 'Name1');
+          const id1 = this.variableMap.getVariableById('id1');
+          this.variableMap.renameVariable(id1, 'Name1');
           this.clock.runAll();
 
           this.workspace.undo();
@@ -1542,7 +1497,8 @@ export function testAWorkspace() {
         suite('Two variables rename overlap', function () {
           test('Same type no usages rename variable with id1 to name2', function () {
             this.variableMap.createVariable('name2', 'type1', 'id2');
-            this.variableMap.renameVariableById('id1', 'name2');
+            const id1 = this.variableMap.getVariableById('id1');
+            this.variableMap.renameVariable(id1, 'name2');
             this.clock.runAll();
 
             this.workspace.undo();
@@ -1563,7 +1519,8 @@ export function testAWorkspace() {
               'id2',
             );
             createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
-            this.variableMap.renameVariableById('id1', 'name2');
+            const id1 = this.variableMap.getVariableById('id1');
+            this.variableMap.renameVariable(id1, 'name2');
             this.clock.runAll();
 
             this.workspace.undo();
@@ -1581,7 +1538,8 @@ export function testAWorkspace() {
 
           test('Same type different capitalization no usages rename variable with id1 to Name2', function () {
             this.variableMap.createVariable('name2', 'type1', 'id2');
-            this.variableMap.renameVariableById('id1', 'Name2');
+            const id1 = this.variableMap.getVariableById('id1');
+            this.variableMap.renameVariable(id1, 'Name2');
             this.clock.runAll();
 
             this.workspace.undo();
@@ -1596,9 +1554,10 @@ export function testAWorkspace() {
           });
 
           test('Same type different capitalization with usages rename variable with id1 to Name2', function () {
-            this.workspace.createVariable('name2', 'type1', 'id2');
+            this.variableMap.createVariable('name2', 'type1', 'id2');
             createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
-            this.variableMap.renameVariableById('id1', 'Name2');
+            const id1 = this.variableMap.getVariableById('id1');
+            this.variableMap.renameVariable(id1, 'Name2');
             this.clock.runAll();
 
             this.workspace.undo();
@@ -1618,7 +1577,8 @@ export function testAWorkspace() {
 
           test('Different type no usages rename variable with id1 to name2', function () {
             this.variableMap.createVariable('name2', 'type2', 'id2');
-            this.variableMap.renameVariableById('id1', 'name2');
+            const id1 = this.variableMap.getVariableById('id1');
+            this.variableMap.renameVariable(id1, 'name2');
             this.clock.runAll();
 
             this.workspace.undo();
@@ -1635,7 +1595,8 @@ export function testAWorkspace() {
           test('Different type with usages rename variable with id1 to name2', function () {
             this.variableMap.createVariable('name2', 'type2', 'id2');
             createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
-            this.variableMap.renameVariableById('id1', 'name2');
+            const id1 = this.variableMap.getVariableById('id1');
+            this.variableMap.renameVariable(id1, 'name2');
             this.clock.runAll();
 
             this.workspace.undo();
@@ -1655,7 +1616,8 @@ export function testAWorkspace() {
 
           test('Different type different capitalization no usages rename variable with id1 to Name2', function () {
             this.variableMap.createVariable('name2', 'type2', 'id2');
-            this.variableMap.renameVariableById('id1', 'Name2');
+            const id1 = this.variableMap.getVariableById('id1');
+            this.variableMap.renameVariable(id1, 'Name2');
             this.clock.runAll();
 
             this.workspace.undo();
@@ -1672,7 +1634,8 @@ export function testAWorkspace() {
           test('Different type different capitalization with usages rename variable with id1 to Name2', function () {
             this.variableMap.createVariable('name2', 'type2', 'id2');
             createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
-            this.variableMap.renameVariableById('id1', 'Name2');
+            const id1 = this.variableMap.getVariableById('id1');
+            this.variableMap.renameVariable(id1, 'Name2');
             this.clock.runAll();
 
             this.workspace.undo();

tests/mocha/variable_map_test.js

@@ -388,35 +388,6 @@ suite('Variable Map', function () {
           );
         });
       });
-
-      suite('deleting by ID', function () {
-        test('delete events are fired when a variable is deleted', function () {
-          this.variableMap.createVariable('test name', 'test type', 'test id');
-          this.variableMap.deleteVariableById('test id');
-
-          assertEventFired(
-            this.eventSpy,
-            Blockly.Events.VarDelete,
-            {
-              varType: 'test type',
-              varName: 'test name',
-              varId: 'test id',
-            },
-            this.workspace.id,
-          );
-        });
-
-        test('delete events are not fired when a variable does not exist', function () {
-          this.variableMap.deleteVariableById('test id');
-
-          assertEventNotFired(
-            this.eventSpy,
-            Blockly.Events.VarDelete,
-            {},
-            this.workspace.id,
-          );
-        });
-      });
     });
 
     suite('variable rename events', function () {
@@ -473,44 +444,6 @@ suite('Variable Map', function () {
           );
         });
       });
-
-      suite('renaming by ID', function () {
-        test('rename events are fired when a variable is renamed', function () {
-          this.variableMap.createVariable('test name', 'test type', 'test id');
-          this.variableMap.renameVariableById('test id', 'new test name');
-
-          assertEventFired(
-            this.eventSpy,
-            Blockly.Events.VarRename,
-            {
-              oldName: 'test name',
-              newName: 'new test name',
-              varId: 'test id',
-            },
-            this.workspace.id,
-          );
-        });
-
-        test('rename events are not fired if the variable name already matches', function () {
-          this.variableMap.createVariable('test name', 'test type', 'test id');
-          this.variableMap.renameVariableById('test id', 'test name');
-
-          assertEventNotFired(
-            this.eventSpy,
-            Blockly.Events.VarRename,
-            {},
-            this.workspace.id,
-          );
-        });
-
-        test('renaming throws if the variable does not exist', function () {
-          // Not sure why this throws when the other one doesn't but might
-          // as well test it.
-          assert.throws(() => {
-            this.variableMap.renameVariableById('test id', 'test name');
-          }, `Tried to rename a variable that didn't exist`);
-        });
-      });
     });
 
     suite('variable type change events', function () {

tests/mocha/xml_test.js

@@ -449,7 +449,7 @@ suite('XML', function () {
       createGenUidStubWithReturns('1');
       this.variableMap.createVariable('name1');
       const resultDom = Blockly.Xml.variablesToDom(
-        this.workspace.getAllVariables(),
+        this.workspace.getVariableMap().getAllVariables(),
       );
       assert.equal(resultDom.children.length, 1);
       const resultVariableDom = resultDom.children[0];
@@ -471,7 +471,7 @@ suite('XML', function () {
       Blockly.Events.enable();
 
       const resultDom = Blockly.Xml.variablesToDom(
-        this.workspace.getAllVariables(),
+        this.workspace.getVariableMap().getAllVariables(),
       );
       assert.equal(resultDom.children.length, 2);
       assertVariableDom(resultDom.children[0], null, 'id1', 'name1');
@@ -479,7 +479,7 @@ suite('XML', function () {
     });
     test('No variables', function () {
       const resultDom = Blockly.Xml.variablesToDom(
-        this.workspace.getAllVariables(),
+        this.workspace.getVariableMap().getAllVariables(),
       );
       assert.equal(resultDom.children.length, 0);
     });