Merge pull request #1562 from rachel-fenichel/bugfix/matching_names

Fix #1559

core/variables.js

@@ -275,30 +275,41 @@ Blockly.Variables.generateUniqueName = function(workspace) {
  */
 Blockly.Variables.createVariableButtonHandler = function(
     workspace, opt_callback, opt_type) {
+  var type = opt_type || '';
   // This function needs to be named so it can be called recursively.
   var promptAndCheckWithAlert = function(defaultName) {
     Blockly.Variables.promptName(Blockly.Msg.NEW_VARIABLE_TITLE, defaultName,
-      function(text) {
-        if (text) {
-          if (workspace.getVariable(text, opt_type)) {
-            Blockly.alert(Blockly.Msg.VARIABLE_ALREADY_EXISTS.replace('%1',
-                text.toLowerCase()),
-                function() {
-                  promptAndCheckWithAlert(text);  // Recurse
-                });
+        function(text) {
+          if (text) {
+            var existing =
+                Blockly.Variables.nameUsedWithAnyType_(text, workspace);
+            if (existing) {
+              var lowerCase = text.toLowerCase();
+              if (existing.type == type) {
+                var msg = Blockly.Msg.VARIABLE_ALREADY_EXISTS.replace(
+                    '%1', lowerCase);
+              } else {
+                var msg = Blockly.Msg.VARIABLE_ALREADY_EXISTS_FOR_ANOTHER_TYPE;
+                msg = msg.replace('%1', lowerCase).replace('%2', existing.type);
+              }
+              Blockly.alert(msg,
+                  function() {
+                    promptAndCheckWithAlert(text);  // Recurse
+                  });
+            } else {
+              // No conflict
+              workspace.createVariable(text, type);
+              if (opt_callback) {
+                opt_callback(text);
+              }
+            }
           } else {
-            workspace.createVariable(text, opt_type);
+            // User canceled prompt.
             if (opt_callback) {
-              opt_callback(text);
+              opt_callback(null);
             }
           }
-        } else {
-          // User canceled prompt without a value.
-          if (opt_callback) {
-            opt_callback(null);
-          }
-        }
-      });
+        });
   };
   promptAndCheckWithAlert('');
 };
@@ -340,12 +351,24 @@ Blockly.Variables.renameVariable = function(workspace, variable,
     Blockly.Variables.promptName(promptText, defaultName,
         function(newName) {
           if (newName) {
-            workspace.renameVariableById(variable.getId(), newName);
-            if (opt_callback) {
-              opt_callback(newName);
+            var existing = Blockly.Variables.nameUsedWithOtherType_(newName,
+                variable.type, workspace);
+            if (existing) {
+              var msg = Blockly.Msg.VARIABLE_ALREADY_EXISTS_FOR_ANOTHER_TYPE
+                  .replace('%1', newName.toLowerCase())
+                  .replace('%2', existing.type);
+              Blockly.alert(msg,
+                  function() {
+                    promptAndCheckWithAlert(newName);  // Recurse
+                  });
+            } else {
+              workspace.renameVariableById(variable.getId(), newName);
+              if (opt_callback) {
+                opt_callback(newName);
+              }
             }
           } else {
-            // User canceled prompt without a value.
+            // User canceled prompt.
             if (opt_callback) {
               opt_callback(null);
             }
@@ -378,6 +401,50 @@ Blockly.Variables.promptName = function(promptText, defaultText, callback) {
   });
 };
 
+/**
+ * Check whether there exists a variable with the given name but a different
+ * type.
+ * @param {string} name The name to search for.
+ * @param {string} type The type to exclude from the search.
+ * @param {!Blockly.Workspace} workspace The workspace to search for the
+ *     variable.
+ * @return {?Blockly.VariableModel} The variable with the given name and a
+ *     different type, or null if none was found.
+ * @private
+ */
+Blockly.Variables.nameUsedWithOtherType_ = function(name, type, workspace) {
+  var allVariables = workspace.getVariableMap().getAllVariables();
+
+  name = name.toLowerCase();
+  for (var i = 0, variable; variable = allVariables[i]; i++) {
+    if (variable.name.toLowerCase() == name && variable.type != type) {
+      return variable;
+    }
+  }
+  return null;
+};
+
+/**
+ * Check whether there exists a variable with the given name of any type.
+ * @param {string} name The name to search for.
+ * @param {!Blockly.Workspace} workspace The workspace to search for the
+ *     variable.
+ * @return {?Blockly.VariableModel} The variable with the given name, or null if
+ *    none was found.
+ * @private
+ */
+Blockly.Variables.nameUsedWithAnyType_ = function(name, workspace) {
+  var allVariables = workspace.getVariableMap().getAllVariables();
+
+  name = name.toLowerCase();
+  for (var i = 0, variable; variable = allVariables[i]; i++) {
+    if (variable.name.toLowerCase() == name) {
+      return variable;
+    }
+  }
+  return null;
+};
+
 /**
  * Generate XML string for variable field.
  * @param {!Blockly.VariableModel} variableModel The variable model to generate

msg/messages.js

@@ -136,7 +136,7 @@ Blockly.Msg.NEW_VARIABLE_TITLE = 'New variable name:';
 /// alert - Tells the user that the name they entered is already in use.
 Blockly.Msg.VARIABLE_ALREADY_EXISTS = 'A variable named "%1" already exists.';
 /// alert - Tells the user that the name they entered is already in use for another type.
-Blockly.Msg.VARIABLE_ALREADY_EXISTS_FOR_ANOTHER_TYPE = 'A variable named "%1" already exists for another variable of type "%2".';
+Blockly.Msg.VARIABLE_ALREADY_EXISTS_FOR_ANOTHER_TYPE = 'A variable named "%1" already exists for another type: "%2".';
 
 // Variable deletion.
 /// confirm -  Ask the user to confirm their deletion of multiple uses of a variable.