feat: clarify variable and procedure constants (#5743)

* chore: move dynamic category names into their respective files

* feat: create NameType enum on Names

* chore: use NameType enum for Names helper functions

* docs: update comments for category names

core/blockly.js

@@ -545,15 +545,37 @@ exports.DRAG_STICKY = internalConstants.DRAG_STICKY;
 exports.DRAG_BEGIN = internalConstants.DRAG_BEGIN;
 exports.DRAG_FREE = internalConstants.DRAG_FREE;
 exports.OPPOSITE_TYPE = internalConstants.OPPOSITE_TYPE;
-exports.VARIABLE_CATEGORY_NAME = internalConstants.VARIABLE_CATEGORY_NAME;
-exports.VARIABLE_DYNAMIC_CATEGORY_NAME =
-    internalConstants.VARIABLE_DYNAMIC_CATEGORY_NAME;
-exports.PROCEDURE_CATEGORY_NAME = internalConstants.PROCEDURE_CATEGORY_NAME;
 exports.RENAME_VARIABLE_ID = internalConstants.RENAME_VARIABLE_ID;
 exports.DELETE_VARIABLE_ID = internalConstants.DELETE_VARIABLE_ID;
 exports.COLLAPSED_INPUT_NAME = constants.COLLAPSED_INPUT_NAME;
 exports.COLLAPSED_FIELD_NAME = constants.COLLAPSED_FIELD_NAME;
 
+/**
+ * String for use in the "custom" attribute of a category in toolbox XML.
+ * This string indicates that the category should be dynamically populated with
+ * variable blocks.
+ * @const {string}
+ * @alias Blockly.VARIABLE_CATEGORY_NAME
+ */
+exports.VARIABLE_CATEGORY_NAME = Variables.CATEGORY_NAME;
+
+/**
+ * String for use in the "custom" attribute of a category in toolbox XML.
+ * This string indicates that the category should be dynamically populated with
+ * variable blocks.
+ * @const {string}
+ * @alias Blockly.VARIABLE_DYNAMIC_CATEGORY_NAME
+ */
+exports.VARIABLE_DYNAMIC_CATEGORY_NAME = VariablesDynamic.CATEGORY_NAME;
+/**
+ * String for use in the "custom" attribute of a category in toolbox XML.
+ * This string indicates that the category should be dynamically populated with
+ * procedure blocks.
+ * @const {string}
+ * @alias Blockly.PROCEDURE_CATEGORY_NAME
+ */
+exports.PROCEDURE_CATEGORY_NAME = Procedures.CATEGORY_NAME;
+
 // Re-export submodules that no longer declareLegacyNamespace.
 exports.ASTNode = ASTNode;
 exports.BasicCursor = BasicCursor;

core/generator.js

@@ -19,11 +19,10 @@ goog.module('Blockly.Generator');
 
 const common = goog.require('Blockly.common');
 const deprecation = goog.require('Blockly.utils.deprecation');
-const internalConstants = goog.require('Blockly.internalConstants');
 /* eslint-disable-next-line no-unused-vars */
 const {Block} = goog.requireType('Blockly.Block');
 /* eslint-disable-next-line no-unused-vars */
-const {Names} = goog.requireType('Blockly.Names');
+const {Names, NameType} = goog.require('Blockly.Names');
 /* eslint-disable-next-line no-unused-vars */
 const {Workspace} = goog.requireType('Blockly.Workspace');
 
@@ -467,8 +466,8 @@ Object.defineProperties(Generator.prototype, {
  */
 Generator.prototype.provideFunction_ = function(desiredName, code) {
   if (!this.definitions_[desiredName]) {
-    const functionName = this.nameDB_.getDistinctName(
+    const functionName =
-        desiredName, internalConstants.PROCEDURE_CATEGORY_NAME);
+        this.nameDB_.getDistinctName(desiredName, NameType.PROCEDURE);
     this.functionNames_[desiredName] = functionName;
     let codeText = code.join('\n').replace(
         this.FUNCTION_NAME_PLACEHOLDER_REGEXP_, functionName);

core/internal_constants.js

@@ -196,36 +196,6 @@ OPPOSITE_TYPE[ConnectionType.PREVIOUS_STATEMENT] =
 
 exports.OPPOSITE_TYPE = OPPOSITE_TYPE;
 
-/**
- * String for use in the "custom" attribute of a category in toolbox XML.
- * This string indicates that the category should be dynamically populated with
- * variable blocks.
- * @const {string}
- * @alias Blockly.internalConstants.VARIABLE_CATEGORY_NAME
- */
-const VARIABLE_CATEGORY_NAME = 'VARIABLE';
-exports.VARIABLE_CATEGORY_NAME = VARIABLE_CATEGORY_NAME;
-
-/**
- * String for use in the "custom" attribute of a category in toolbox XML.
- * This string indicates that the category should be dynamically populated with
- * variable blocks.
- * @const {string}
- * @alias Blockly.internalConstants.VARIABLE_DYNAMIC_CATEGORY_NAME
- */
-const VARIABLE_DYNAMIC_CATEGORY_NAME = 'VARIABLE_DYNAMIC';
-exports.VARIABLE_DYNAMIC_CATEGORY_NAME = VARIABLE_DYNAMIC_CATEGORY_NAME;
-
-/**
- * String for use in the "custom" attribute of a category in toolbox XML.
- * This string indicates that the category should be dynamically populated with
- * procedure blocks.
- * @const {string}
- * @alias Blockly.internalConstants.PROCEDURE_CATEGORY_NAME
- */
-const PROCEDURE_CATEGORY_NAME = 'PROCEDURE';
-exports.PROCEDURE_CATEGORY_NAME = PROCEDURE_CATEGORY_NAME;
-
 /**
  * String for use in the dropdown created in field_variable.
  * This string indicates that this option in the dropdown is 'Rename

core/names.js

@@ -17,7 +17,6 @@ goog.module('Blockly.Names');
 
 const Msg = goog.require('Blockly.Msg');
 const Variables = goog.require('Blockly.Variables');
-const internalConstants = goog.require('Blockly.internalConstants');
 /* eslint-disable-next-line no-unused-vars */
 const {VariableMap} = goog.requireType('Blockly.VariableMap');
 /* eslint-disable-next-line no-unused-vars */
@@ -47,6 +46,25 @@ const Names = function(reservedWords, opt_variablePrefix) {
   this.reset();
 };
 
+/**
+ * Enum for the type of a name. Different name types may have different rules
+ * about collisions.
+ * When JavaScript (or most other languages) is generated, variable 'foo' and
+ * procedure 'foo' would collide.  However, Blockly has no such problems since
+ * variable get 'foo' and procedure call 'foo' are unambiguous.
+ * Therefore, Blockly keeps a separate name type to disambiguate.
+ * getName('foo', 'VARIABLE') -> 'foo'
+ * getName('foo', 'PROCEDURE') -> 'foo2'
+ * @enum { string }
+ * @alias Blockly.Names.NameType
+ */
+const NameType = {
+  DEVELOPER_VARIABLE: 'DEVELOPER_VARIABLE',
+  VARIABLE: 'VARIABLE',
+  PROCEDURE: 'PROCEDURE',
+};
+exports.NameType = NameType;
+
 /**
  * Constant to separate developer variable names from user-defined variable
  * names when running generators.
@@ -54,16 +72,7 @@ const Names = function(reservedWords, opt_variablePrefix) {
  * will never be shown to the user in the workspace or stored in the variable
  * map.
  */
-Names.DEVELOPER_VARIABLE_TYPE = 'DEVELOPER_VARIABLE';
+Names.DEVELOPER_VARIABLE_TYPE = NameType.DEVELOPER_VARIABLE;
-
-/**
- * When JavaScript (or most other languages) is generated, variable 'foo' and
- * procedure 'foo' would collide.  However, Blockly has no such problems since
- * variable get 'foo' and procedure call 'foo' are unambiguous.
- * Therefore, Blockly keeps a separate realm name to disambiguate.
- * getName('foo', 'VARIABLE') -> 'foo'
- * getName('foo', 'PROCEDURE') -> 'foo2'
- */
 
 /**
  * Empty the database and start from scratch.  The reserved words are kept.
@@ -84,8 +93,7 @@ Names.prototype.setVariableMap = function(map) {
 
 /**
  * Get the name for a user-defined variable, based on its ID.
- * This should only be used for variables of realm
+ * This should only be used for variables of NameType VARIABLE.
- * internalConstants.VARIABLE_CATEGORY_NAME.
  * @param {string} id The ID to look up in the variable map.
  * @return {?string} The name of the referenced variable, or null if there was
  *     no variable map or the variable was not found in the map.
@@ -115,8 +123,7 @@ Names.prototype.getNameForUserVariable_ = function(id) {
 Names.prototype.populateVariables = function(workspace) {
   const variables = Variables.allUsedVarModels(workspace);
   for (let i = 0; i < variables.length; i++) {
-    this.getName(
+    this.getName(variables[i].getId(), NameType.VARIABLE);
-        variables[i].getId(), internalConstants.VARIABLE_CATEGORY_NAME);
   }
 };
 
@@ -130,7 +137,7 @@ Names.prototype.populateProcedures = function(workspace) {
   // Flatten the return vs no-return procedure lists.
   procedures = procedures[0].concat(procedures[1]);
   for (let i = 0; i < procedures.length; i++) {
-    this.getName(procedures[i][0], internalConstants.PROCEDURE_CATEGORY_NAME);
+    this.getName(procedures[i][0], NameType.PROCEDURE);
   }
 };
 
@@ -138,13 +145,13 @@ Names.prototype.populateProcedures = function(workspace) {
  * Convert a Blockly entity name to a legal exportable entity name.
  * @param {string} nameOrId The Blockly entity name (no constraints) or
  *     variable ID.
- * @param {string} realm The realm of entity in Blockly
+ * @param {NameType|string} type The type of the name in Blockly
  *     ('VARIABLE', 'PROCEDURE', 'DEVELOPER_VARIABLE', etc...).
  * @return {string} An entity name that is legal in the exported language.
  */
-Names.prototype.getName = function(nameOrId, realm) {
+Names.prototype.getName = function(nameOrId, type) {
   let name = nameOrId;
-  if (realm === internalConstants.VARIABLE_CATEGORY_NAME) {
+  if (type === NameType.VARIABLE) {
     const varName = this.getNameForUserVariable_(nameOrId);
     if (varName) {
       // Successful ID lookup.
@@ -153,31 +160,31 @@ Names.prototype.getName = function(nameOrId, realm) {
   }
   const normalizedName = name.toLowerCase();
 
-  const isVar = realm === internalConstants.VARIABLE_CATEGORY_NAME ||
+  const isVar =
-      realm === Names.DEVELOPER_VARIABLE_TYPE;
+      type === NameType.VARIABLE || type === NameType.DEVELOPER_VARIABLE;
 
   const prefix = isVar ? this.variablePrefix_ : '';
-  if (!(realm in this.db_)) {
+  if (!(type in this.db_)) {
-    this.db_[realm] = Object.create(null);
+    this.db_[type] = Object.create(null);
   }
-  const realmDb = this.db_[realm];
+  const typeDb = this.db_[type];
-  if (normalizedName in realmDb) {
+  if (normalizedName in typeDb) {
-    return prefix + realmDb[normalizedName];
+    return prefix + typeDb[normalizedName];
   }
-  const safeName = this.getDistinctName(name, realm);
+  const safeName = this.getDistinctName(name, type);
-  realmDb[normalizedName] = safeName.substr(prefix.length);
+  typeDb[normalizedName] = safeName.substr(prefix.length);
   return safeName;
 };
 
 /**
- * Return a list of all known user-created names in a specified realm.
+ * Return a list of all known user-created names of a specified name type.
- * @param {string} realm The realm of entity in Blockly
+ * @param {NameType|string} type The type of entity in Blockly
  *     ('VARIABLE', 'PROCEDURE', 'DEVELOPER_VARIABLE', etc...).
  * @return {!Array<string>} A list of Blockly entity names (no constraints).
  */
-Names.prototype.getUserNames = function(realm) {
+Names.prototype.getUserNames = function(type) {
-  const realmDb = this.db_[realm] || {};
+  const typeDb = this.db_[type] || {};
-  return Object.keys(realmDb);
+  return Object.keys(typeDb);
 };
 
 /**
@@ -186,11 +193,11 @@ Names.prototype.getUserNames = function(realm) {
  * Also check against list of reserved words for the current language and
  * ensure name doesn't collide.
  * @param {string} name The Blockly entity name (no constraints).
- * @param {string} realm The realm of entity in Blockly
+ * @param {NameType|string} type The type of entity in Blockly
  *     ('VARIABLE', 'PROCEDURE', 'DEVELOPER_VARIABLE', etc...).
  * @return {string} An entity name that is legal in the exported language.
  */
-Names.prototype.getDistinctName = function(name, realm) {
+Names.prototype.getDistinctName = function(name, type) {
   let safeName = this.safeName_(name);
   let i = '';
   while (this.dbReverse_[safeName + i] ||
@@ -200,8 +207,8 @@ Names.prototype.getDistinctName = function(name, realm) {
   }
   safeName += i;
   this.dbReverse_[safeName] = true;
-  const isVar = realm === internalConstants.VARIABLE_CATEGORY_NAME ||
+  const isVar =
-      realm === Names.DEVELOPER_VARIABLE_TYPE;
+      type === NameType.VARIABLE || type === NameType.DEVELOPER_VARIABLE;
   const prefix = isVar ? this.variablePrefix_ : '';
   return prefix + safeName;
 };

core/procedures.js

@@ -35,6 +35,18 @@ const {Workspace} = goog.require('Blockly.Workspace');
 goog.require('Blockly.Events.BlockChange');
 
 
+/**
+ * String for use in the "custom" attribute of a category in toolbox XML.
+ * This string indicates that the category should be dynamically populated with
+ * procedure blocks.
+ * See also Blockly.Variables.CATEGORY_NAME and
+ * Blockly.VariablesDynamic.CATEGORY_NAME.
+ * @const {string}
+ * @alias Blockly.Procedures.CATEGORY_NAME
+ */
+const CATEGORY_NAME = 'PROCEDURE';
+exports.CATEGORY_NAME = CATEGORY_NAME;
+
 /**
  * The default argument for a procedures_mutatorarg block.
  * @type {string}

core/variables.js

@@ -24,6 +24,17 @@ const {VariableModel} = goog.require('Blockly.VariableModel');
 /* eslint-disable-next-line no-unused-vars */
 const {Workspace} = goog.requireType('Blockly.Workspace');
 
+/**
+ * String for use in the "custom" attribute of a category in toolbox XML.
+ * This string indicates that the category should be dynamically populated with
+ * variable blocks.
+ * See also Blockly.Procedures.CATEGORY_NAME and
+ * Blockly.VariablesDynamic.CATEGORY_NAME.
+ * @const {string}
+ * @alias Blockly.Variables.CATEGORY_NAME
+ */
+const CATEGORY_NAME = 'VARIABLE';
+exports.CATEGORY_NAME = CATEGORY_NAME;
 
 /**
  * Find all user-created variables that are in use in the workspace.

core/variables_dynamic.js

@@ -26,6 +26,18 @@ const {VariableModel} = goog.require('Blockly.VariableModel');
 const {Workspace} = goog.requireType('Blockly.Workspace');
 
 
+/**
+ * String for use in the "custom" attribute of a category in toolbox XML.
+ * This string indicates that the category should be dynamically populated with
+ * variable blocks.
+ * See also Blockly.Variables.CATEGORY_NAME and
+ * Blockly.Procedures.CATEGORY_NAME.
+ * @const {string}
+ * @alias Blockly.VariablesDynamic.CATEGORY_NAME
+ */
+const CATEGORY_NAME = 'VARIABLE_DYNAMIC';
+exports.CATEGORY_NAME = CATEGORY_NAME;
+
 const stringButtonClickHandler = function(button) {
   Variables.createVariableButtonHandler(
       button.getTargetWorkspace(), undefined, 'String');

core/workspace_svg.js

@@ -226,20 +226,19 @@ const WorkspaceSvg = function(
   const Variables = goog.module.get('Blockly.Variables');
   if (Variables && Variables.flyoutCategory) {
     this.registerToolboxCategoryCallback(
-        internalConstants.VARIABLE_CATEGORY_NAME, Variables.flyoutCategory);
+        Variables.CATEGORY_NAME, Variables.flyoutCategory);
   }
 
   const VariablesDynamic = goog.module.get('Blockly.VariablesDynamic');
   if (VariablesDynamic && VariablesDynamic.flyoutCategory) {
     this.registerToolboxCategoryCallback(
-        internalConstants.VARIABLE_DYNAMIC_CATEGORY_NAME,
+        VariablesDynamic.CATEGORY_NAME, VariablesDynamic.flyoutCategory);
-        VariablesDynamic.flyoutCategory);
   }
 
   const Procedures = goog.module.get('Blockly.Procedures');
   if (Procedures && Procedures.flyoutCategory) {
     this.registerToolboxCategoryCallback(
-        internalConstants.PROCEDURE_CATEGORY_NAME, Procedures.flyoutCategory);
+        Procedures.CATEGORY_NAME, Procedures.flyoutCategory);
     this.addChangeListener(Procedures.mutatorOpenListener);
   }
 

tests/deps.js

@@ -85,7 +85,7 @@ goog.addDependency('../../core/flyout_button.js', ['Blockly.FlyoutButton'], ['Bl
 goog.addDependency('../../core/flyout_horizontal.js', ['Blockly.HorizontalFlyout'], ['Blockly.DropDownDiv', 'Blockly.Flyout', 'Blockly.Scrollbar', 'Blockly.WidgetDiv', 'Blockly.browserEvents', 'Blockly.registry', 'Blockly.utils.Rect', 'Blockly.utils.object', 'Blockly.utils.toolbox'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../core/flyout_metrics_manager.js', ['Blockly.FlyoutMetricsManager'], ['Blockly.MetricsManager', 'Blockly.utils.object'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../core/flyout_vertical.js', ['Blockly.VerticalFlyout'], ['Blockly.Block', 'Blockly.DropDownDiv', 'Blockly.Flyout', 'Blockly.Scrollbar', 'Blockly.WidgetDiv', 'Blockly.browserEvents', 'Blockly.constants', 'Blockly.registry', 'Blockly.utils.Rect', 'Blockly.utils.object', 'Blockly.utils.toolbox'], {'lang': 'es6', 'module': 'goog'});
-goog.addDependency('../../core/generator.js', ['Blockly.Generator'], ['Blockly.common', 'Blockly.internalConstants', 'Blockly.utils.deprecation'], {'lang': 'es6', 'module': 'goog'});
+goog.addDependency('../../core/generator.js', ['Blockly.Generator'], ['Blockly.Names', 'Blockly.common', 'Blockly.utils.deprecation'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../core/gesture.js', ['Blockly.Gesture'], ['Blockly.BlockDragger', 'Blockly.BubbleDragger', 'Blockly.Events.Click', 'Blockly.Events.utils', 'Blockly.Tooltip', 'Blockly.Touch', 'Blockly.Workspace', 'Blockly.WorkspaceDragger', 'Blockly.blockAnimations', 'Blockly.browserEvents', 'Blockly.common', 'Blockly.internalConstants', 'Blockly.registry', 'Blockly.utils.Coordinate'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../core/grid.js', ['Blockly.Grid'], ['Blockly.utils.Svg', 'Blockly.utils.dom', 'Blockly.utils.userAgent'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../core/icon.js', ['Blockly.Icon'], ['Blockly.browserEvents', 'Blockly.utils.Coordinate', 'Blockly.utils.Size', 'Blockly.utils.Svg', 'Blockly.utils.dom', 'Blockly.utils.svgMath'], {'lang': 'es6', 'module': 'goog'});
@@ -135,7 +135,7 @@ goog.addDependency('../../core/menuitem.js', ['Blockly.MenuItem'], ['Blockly.uti
 goog.addDependency('../../core/metrics_manager.js', ['Blockly.MetricsManager'], ['Blockly.IMetricsManager', 'Blockly.registry', 'Blockly.utils.Size', 'Blockly.utils.toolbox'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../core/msg.js', ['Blockly.Msg'], ['Blockly.utils.global'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../core/mutator.js', ['Blockly.Mutator'], ['Blockly.Bubble', 'Blockly.Events.BlockChange', 'Blockly.Events.BubbleOpen', 'Blockly.Events.utils', 'Blockly.Icon', 'Blockly.Options', 'Blockly.WorkspaceSvg', 'Blockly.internalConstants', 'Blockly.utils.Svg', 'Blockly.utils.dom', 'Blockly.utils.object', 'Blockly.utils.toolbox', 'Blockly.utils.xml'], {'lang': 'es6', 'module': 'goog'});
-goog.addDependency('../../core/names.js', ['Blockly.Names'], ['Blockly.Msg', 'Blockly.Variables', 'Blockly.internalConstants'], {'lang': 'es6', 'module': 'goog'});
+goog.addDependency('../../core/names.js', ['Blockly.Names'], ['Blockly.Msg', 'Blockly.Variables'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../core/options.js', ['Blockly.Options'], ['Blockly.Theme', 'Blockly.Themes.Classic', 'Blockly.registry', 'Blockly.utils.idGenerator', 'Blockly.utils.toolbox'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../core/positionable_helpers.js', ['Blockly.uiPosition'], ['Blockly.Scrollbar', 'Blockly.utils.Rect', 'Blockly.utils.toolbox'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../core/procedures.js', ['Blockly.Procedures'], ['Blockly.Events.BlockChange', 'Blockly.Events.utils', 'Blockly.Msg', 'Blockly.Names', 'Blockly.Variables', 'Blockly.Workspace', 'Blockly.Xml', 'Blockly.blocks', 'Blockly.utils.xml'], {'lang': 'es6', 'module': 'goog'});