Fix project loading with bad text-number connections (#2175)

* Fix project loading with bad text-number connections

Change-Id: I78c1666fd34346e46e68e112d02603e4de197fa5

* More thoroughly fix number detection and add unit tests

Change-Id: I52e646cbfab94cf4a317c9149146bd01fd0da49a

appinventor/blocklyeditor/src/blocks/text.js

@@ -26,17 +26,26 @@ Blockly.Blocks['text'] = {
     this.setOutput(true, [Blockly.Blocks.text.connectionCheck]);
     this.setTooltip(Blockly.Msg.LANG_TEXT_TEXT_TOOLTIP);
   },
+  errors: [{name:"checkInvalidNumber"}],
   typeblock: [{translatedName: Blockly.Msg.LANG_CATEGORY_TEXT}]
 };
 
 Blockly.Blocks.text.connectionCheck = function (myConnection, otherConnection) {
   var block = myConnection.sourceBlock_;
   var otherTypeArray = otherConnection.check_;
+  var shouldIgnoreError = Blockly.mainWorkspace.isLoading;
+  var value = block.getFieldValue('TEXT');
   for (var i = 0; i < otherTypeArray.length; i++) {
     if (otherTypeArray[i] == "String") {
       return true;
-    } else if (otherTypeArray[i] == "Number" && !isNaN(parseFloat(block.getFieldValue('TEXT')))) {
+    } else if (otherTypeArray[i] == "Number") {
+      if (shouldIgnoreError) {
+        // Error may be noted by WarningHandler's checkInvalidNumber
         return true;
+      } else if (Blockly.Blocks.Utilities.NUMBER_REGEX.test(value)) {
+        // Value passes a floating point regex
+        return !isNaN(parseFloat(value));
+      }
     } else if (otherTypeArray[i] == "Key") {
       return true;
     }

appinventor/blocklyeditor/src/blocks/utilities.js

@@ -85,3 +85,13 @@ Blockly.Blocks.Utilities.MAX_COLLAPSE = 4;
 
 // unicode multiplication symbol
 Blockly.Blocks.Utilities.times_symbol = '\u00D7';
+
+/**
+ * Regular expression for floating point numbers.
+ *
+ * @type {!RegExp}
+ * @const
+ */
+Blockly.Blocks.Utilities.NUMBER_REGEX =
+  new RegExp("^([-+]?[0-9]+)?(\\.[0-9]+)?([eE][-+]?[0-9]+)?$|" +
+    "^[-+]?[0-9]+(\\.[0-9]*)?([eE][-+]?[0-9]+)?$");

appinventor/blocklyeditor/src/field_textblockinput.js

@@ -28,6 +28,10 @@ goog.inherits(Blockly.FieldTextBlockInput, Blockly.FieldTextInput);
  * @param {string} text New text.
  */
 Blockly.FieldTextBlockInput.prototype.setText = function(text) {
+  if (text == null) {
+    // No change if null.
+    return;
+  }
   if (this.changeHandler_) {
     var validated = this.changeHandler_(text);
     // If the new text is invalid, validation returns null.

appinventor/blocklyeditor/src/msg/en/_messages.js

@@ -116,6 +116,7 @@ Blockly.Msg.en.switch_language_to_english = {
     Blockly.Msg.ERROR_COMPONENT_DOES_NOT_EXIST = "Component does not exist";
     Blockly.Msg.ERROR_BLOCK_IS_NOT_DEFINED = "This block is not defined. Delete this block!";
     Blockly.Msg.ERROR_BREAK_ONLY_IN_LOOP = "The break block should be used only within loops";
+    Blockly.Msg.ERROR_INVALID_NUMBER_CONTENT = 'This text block does not contain a valid number';
 
 // Colour Blocks.
     Blockly.Msg.LANG_COLOUR_PICKER_HELPURL = '/reference/blocks/colors.html#basic';

appinventor/blocklyeditor/src/rendered_connection.js

@@ -74,3 +74,8 @@ Blockly.RenderedConnection.prototype.disconnectInternal_ = function(parentBlock,
   // Reset visibility, since this block is now a top block.
   childBlock.getSvgRoot().style.display = 'block';
 };
+
+// TODO(ewpatton): Can be removed once we upgrade to a newer version of Blockly
+Blockly.Connection.prototype.getCheck = function() {
+  return this.check_;
+}

appinventor/blocklyeditor/src/warningHandler.js

@@ -14,6 +14,8 @@
 
 goog.provide('AI.Blockly.WarningHandler');
 
+goog.require('Blockly.Blocks.Utilities');
+
 Blockly.WarningHandler = function(workspace) {
   this.workspace = workspace;
   this.allBlockErrors = [{name:'checkReplErrors'}];
@@ -442,6 +444,27 @@ Blockly.WarningHandler.prototype['checkDropDownContainsValidValue'] = function(b
   return false;
 };
 
+/**
+ * Checks whether a text block in a number slot has a valid value. If not,
+ * an error is generated.
+ *
+ * @param {!Blockly.BlockSvg} block the text block to evaluate
+ */
+Blockly.WarningHandler.prototype['checkInvalidNumber'] = function(block) {
+  if (!block.outputConnection || !block.outputConnection.isConnected()) {
+    return;
+  }
+  var targetChecks = block.outputConnection.targetConnection.getCheck();
+  var value = block.getFieldValue('TEXT');
+  if (targetChecks && targetChecks.indexOf('String') == -1 &&
+      targetChecks.indexOf('Number') >= 0 &&
+      (value == '' || !Blockly.Blocks.Utilities.NUMBER_REGEX.test(value))) {
+    block.setErrorIconText(Blockly.Msg.ERROR_INVALID_NUMBER_CONTENT);
+    return true;
+  }
+  return false;
+};
+
 // Check if the block is not within a loop block (used for checking break block)
 // if so, create an error
 

appinventor/blocklyeditor/src/workspace_svg.js

@@ -471,6 +471,7 @@ Blockly.WorkspaceSvg.prototype.loadBlocksFile = function(formJson, blocksContent
   if (blocksContent.length != 0) {
     try {
       Blockly.Events.disable();
+      this.isLoading = true;
       if (Blockly.Versioning.upgrade(formJson, blocksContent, this)) {
         var self = this;
         setTimeout(function() {
@@ -478,6 +479,7 @@ Blockly.WorkspaceSvg.prototype.loadBlocksFile = function(formJson, blocksContent
         });
       }
     } finally {
+      this.isLoading = false;
       Blockly.Events.enable();
     }
     if (this.getCanvas() != null) {

appinventor/blocklyeditor/tests/com/google/appinventor/mocha/index.html

@@ -22,6 +22,7 @@
 
 <!--Add test files here-->
 <script src="block.js"></script>
+<script src="text.js"></script>
 
 <div id="blocklyDiv"></div>
 

appinventor/blocklyeditor/tests/com/google/appinventor/mocha/text.js

+// Copyright © 2020 Massachusetts Institute of Technology. All rights reserved.
+
+/**
+ * @license
+ * @fileoverview Block behavior tests. Not related to code generation.
+ *
+ */
+
+suite('Text Blocks', function() {
+
+  setup(function() {
+    this.workspace = Blockly.inject('blocklyDiv', {});
+  });
+
+  teardown(function() {
+    this.workspace.dispose();
+    Blockly.mainWorkspace = null;
+  })
+
+  suite('Text connection check', function() {
+
+    setup(function() {
+      Blockly.mainWorkspace.isLoading = false;
+    });
+
+    function mockConnection(value) {
+      return {
+        sourceBlock_: {
+          getFieldValue: function() {
+            return value;
+          }
+        }
+      };
+    }
+
+    var check = Blockly.Blocks.text.connectionCheck;
+    var number = { check_: ['Number'] };
+    var boolean = { check_: ['Boolean'] };
+    var string = { check_: ['String'] };
+    var key = { check_: ['Key'] };
+    var many = { check_: ['Number', 'String'] };
+    var invalidNumbers = [
+      'cat', '4cat', 'e', '  zero  ', '0x0', '.', '-', '+', '', '-e-', '+e+'
+    ];
+
+    test('Should allow String connection', function() {
+      chai.assert.isTrue(check(mockConnection(''), string));
+    });
+
+    test('Should allow Key connection', function() {
+      chai.assert.isTrue(check(mockConnection(''), key));
+    });
+
+    test('Should not connect to invalid type', function() {
+      chai.assert.isFalse(check(mockConnection(''), boolean));
+    });
+
+    test('Should connect to number when integer', function() {
+      ['1', '+1', '-1'].forEach(function(value) {
+        chai.assert.isTrue(check(mockConnection(value), number));
+      });
+    });
+
+    test('Should connect to number when floating point', function() {
+      ['3.14', '+3.14', '-3.14', '3.', '.14', '1e5', '1e-5', '1.e5', '1.e-5',
+        '.1e4', '.1e-4', '.1e+4', '3.14e+3'].forEach(function (value) {
+        chai.assert.isTrue(check(mockConnection(value), number));
+      });
+    });
+
+    test('Should not connect to number when not a number', function() {
+      invalidNumbers.forEach(function(value) {
+        chai.assert.isFalse(check(mockConnection(value), number));
+      })
+    });
+
+    test('Should allow invalid numbers during load', function() {
+      Blockly.mainWorkspace.isLoading = true;
+      invalidNumbers.forEach(function(value) {
+        chai.assert.isTrue(check(mockConnection(value), number));
+      })
+    });
+
+    test('Should connect with many checks', function() {
+      chai.assert.isTrue(check(mockConnection('cat'), many));
+    })
+  });
+})