feat: Enable the creation of concurrent, but not connected, output and previous connectors (#5702)

* Add new connection checks and tests

* Enable concurrent output and previous connections on blocks

  * and remove assumption from unplug().

* Make changes compatible with new module system

* Fix issue do to Connection class change.

  . and change some var's to const's now that that appears to be ok ;-)

* Fix more code that assumes only one of output and previous connections

* Change function name as per PR comment

* Fix lint errors

* Fix clang format issues

core/block.js

@@ -469,7 +469,8 @@ Block.prototype.initModel = function() {
 Block.prototype.unplug = function(opt_healStack) {
   if (this.outputConnection) {
     this.unplugFromRow_(opt_healStack);
-  } else if (this.previousConnection) {
+  }
+  if (this.previousConnection) {
     this.unplugFromStack_(opt_healStack);
   }
 };
@@ -771,10 +772,12 @@ Block.prototype.setParent = function(newParent) {
 
   // Check that block is connected to new parent if new parent is not null and
   //    that block is not connected to superior one if new parent is null.
-  const connection = this.previousConnection || this.outputConnection;
-  const isConnected = !!(connection && connection.targetBlock());
+  const targetBlock =
+      (this.previousConnection && this.previousConnection.targetBlock()) ||
+      (this.outputConnection && this.outputConnection.targetBlock());
+  const isConnected = !!targetBlock;
 
-  if (isConnected && newParent && connection.targetBlock() !== newParent) {
+  if (isConnected && newParent && targetBlock !== newParent) {
     throw Error('Block connected to superior one that is not new parent.');
   } else if (!isConnected && newParent) {
     throw Error('Block not connected to new parent.');
@@ -1186,11 +1189,6 @@ Block.prototype.setPreviousStatement = function(newBoolean, opt_check) {
       opt_check = null;
     }
     if (!this.previousConnection) {
-      if (this.outputConnection) {
-        throw Error(
-            'Remove output connection prior to adding previous ' +
-            'connection.');
-      }
       this.previousConnection =
           this.makeConnection_(ConnectionType.PREVIOUS_STATEMENT);
     }
@@ -1249,11 +1247,6 @@ Block.prototype.setOutput = function(newBoolean, opt_check) {
       opt_check = null;
     }
     if (!this.outputConnection) {
-      if (this.previousConnection) {
-        throw Error(
-            'Remove previous connection prior to adding output ' +
-            'connection.');
-      }
       this.outputConnection = this.makeConnection_(ConnectionType.OUTPUT_VALUE);
     }
     this.outputConnection.setCheck(opt_check);

core/connection.js

@@ -63,6 +63,7 @@ Connection.REASON_CHECKS_FAILED = 4;
 Connection.REASON_DIFFERENT_WORKSPACES = 5;
 Connection.REASON_SHADOW_PARENT = 6;
 Connection.REASON_DRAG_CHECKS_FAILED = 7;
+Connection.REASON_PREVIOUS_AND_OUTPUT = 8;
 
 /**
  * Connection this connection connects to.  Null if not connected.

core/connection_checker.js

@@ -123,6 +123,8 @@ ConnectionChecker.prototype.getErrorMessage = function(errorCode, a, b) {
       return 'Connecting non-shadow to shadow block.';
     case Connection.REASON_DRAG_CHECKS_FAILED:
       return 'Drag checks failed.';
+    case Connection.REASON_PREVIOUS_AND_OUTPUT:
+      return 'Block would have an output and a previous connection.';
     default:
       return 'Unknown connection failure: this should never happen!';
   }
@@ -140,23 +142,41 @@ ConnectionChecker.prototype.doSafetyChecks = function(a, b) {
   if (!a || !b) {
     return Connection.REASON_TARGET_NULL;
   }
-  let blockA;
-  let blockB;
+  let superiorBlock;
+  let inferiorBlock;
+  let superiorConnection;
+  let inferiorConnection;
   if (a.isSuperior()) {
-    blockA = a.getSourceBlock();
-    blockB = b.getSourceBlock();
+    superiorBlock = a.getSourceBlock();
+    inferiorBlock = b.getSourceBlock();
+    superiorConnection = a;
+    inferiorConnection = b;
   } else {
-    blockB = a.getSourceBlock();
-    blockA = b.getSourceBlock();
+    inferiorBlock = a.getSourceBlock();
+    superiorBlock = b.getSourceBlock();
+    inferiorConnection = a;
+    superiorConnection = b;
   }
-  if (blockA === blockB) {
+  if (superiorBlock === inferiorBlock) {
     return Connection.REASON_SELF_CONNECTION;
-  } else if (b.type !== internalConstants.OPPOSITE_TYPE[a.type]) {
+  } else if (
+      inferiorConnection.type !==
+      internalConstants.OPPOSITE_TYPE[superiorConnection.type]) {
     return Connection.REASON_WRONG_TYPE;
-  } else if (blockA.workspace !== blockB.workspace) {
+  } else if (superiorBlock.workspace !== inferiorBlock.workspace) {
     return Connection.REASON_DIFFERENT_WORKSPACES;
-  } else if (blockA.isShadow() && !blockB.isShadow()) {
+  } else if (superiorBlock.isShadow() && !inferiorBlock.isShadow()) {
     return Connection.REASON_SHADOW_PARENT;
+  } else if (
+      inferiorConnection.type === ConnectionType.OUTPUT_VALUE &&
+      inferiorBlock.previousConnection &&
+      inferiorBlock.previousConnection.isConnected()) {
+    return Connection.REASON_PREVIOUS_AND_OUTPUT;
+  } else if (
+      inferiorConnection.type === ConnectionType.PREVIOUS_STATEMENT &&
+      inferiorBlock.outputConnection &&
+      inferiorBlock.outputConnection.isConnected()) {
+    return Connection.REASON_PREVIOUS_AND_OUTPUT;
   }
   return Connection.CAN_CONNECT;
 };

core/events/events_block_move.js

@@ -167,7 +167,11 @@ BlockMove.prototype.run = function(forward) {
     const xy = block.getRelativeToSurfaceXY();
     block.moveBy(coordinate.x - xy.x, coordinate.y - xy.y);
   } else {
-    const blockConnection = block.outputConnection || block.previousConnection;
+    let blockConnection = block.outputConnection;
+    if (!blockConnection ||
+        (block.previousConnection && block.previousConnection.isConnected())) {
+      blockConnection = block.previousConnection;
+    }
     let parentConnection;
     const connectionType = blockConnection.type;
     if (inputName) {

core/keyboard_nav/ast_node.js

@@ -231,6 +231,25 @@ ASTNode.createWorkspaceNode = function(workspace, wsCoordinate) {
   return new ASTNode(ASTNode.types.WORKSPACE, workspace, params);
 };
 
+/**
+ * Gets the parent connection on a block.
+ * This is either an output connection, previous connection or undefined.
+ * If both connections exist return the one that is actually connected
+ * to another block.
+ * @param {!Block} block The block to find the parent connection on.
+ * @return {Connection} The connection connecting to the parent of the
+ *     block.
+ * @private
+ */
+const getParentConnection = function(block) {
+  let topConnection = block.outputConnection;
+  if (!topConnection ||
+      (block.previousConnection && block.previousConnection.isConnected())) {
+    topConnection = block.previousConnection;
+  }
+  return topConnection;
+};
+
 /**
  * Creates an AST node for the top position on a block.
  * This is either an output connection, previous connection, or block.
@@ -240,7 +259,7 @@ ASTNode.createWorkspaceNode = function(workspace, wsCoordinate) {
  */
 ASTNode.createTopNode = function(block) {
   let astNode;
-  const topConnection = block.previousConnection || block.outputConnection;
+  const topConnection = getParentConnection(block);
   if (topConnection) {
     astNode = ASTNode.createConnectionNode(topConnection);
   } else {
@@ -466,7 +485,7 @@ ASTNode.prototype.navigateBetweenStacks_ = function(forward) {
  * @private
  */
 ASTNode.prototype.findTopASTNodeForBlock_ = function(block) {
-  const topConnection = block.previousConnection || block.outputConnection;
+  const topConnection = getParentConnection(block);
   if (topConnection) {
     return /** @type {!ASTNode} */ (
         ASTNode.createConnectionNode(topConnection));
@@ -490,8 +509,7 @@ ASTNode.prototype.getOutAstNodeForBlock_ = function(block) {
   // If the block doesn't have a previous connection then it is the top of the
   // substack.
   const topBlock = block.getTopStackBlock();
-  const topConnection =
-      topBlock.previousConnection || topBlock.outputConnection;
+  const topConnection = getParentConnection(topBlock);
   // If the top connection has a parentInput, create an AST node pointing to
   // that input.
   if (topConnection && topConnection.targetConnection &&
@@ -642,7 +660,7 @@ ASTNode.prototype.prev = function() {
 
     case ASTNode.types.BLOCK: {
       const block = /** @type {!Block} */ (this.location_);
-      const topConnection = block.previousConnection || block.outputConnection;
+      const topConnection = getParentConnection(block);
       return ASTNode.createConnectionNode(topConnection);
     }
     case ASTNode.types.PREVIOUS: {

tests/mocha/connection_checker_test.js

@@ -233,6 +233,74 @@ suite('Connection checker', function() {
             Blockly.Connection.CAN_CONNECT);
       });
     });
+    suite('Output and Previous', function() {
+      /**
+       * Update two connections to target each other.
+       * @param {Connection} first The first connection to update.
+       * @param {Connection} second The second connection to update.
+       */
+      const connectReciprocally = function(first, second) {
+        if (!first || !second) {
+          throw Error('Cannot connect null connections.');
+        }
+        first.targetConnection = second;
+        second.targetConnection = first;
+      };
+      test('Output connected, adding previous', function() {
+        const outBlock = {
+          isShadow: function() {
+          },
+        };
+        const inBlock = {
+          isShadow: function() {
+          },
+        };
+        const outCon = new Blockly.Connection(outBlock, Blockly.OUTPUT_VALUE);
+        const inCon = new Blockly.Connection(inBlock, Blockly.INPUT_VALUE);
+        outBlock.outputConnection = outCon;
+        inBlock.inputConnection = inCon;
+        connectReciprocally(inCon, outCon);
+        const prevCon = new Blockly.Connection(outBlock, Blockly.PREVIOUS_STATEMENT);
+        const nextBlock = {
+          isShadow: function() {
+          },
+        };
+        const nextCon = new Blockly.Connection(nextBlock, Blockly.NEXT_STATEMENT);
+
+        assertReasonHelper(
+            this.checker,
+            prevCon,
+            nextCon,
+            Blockly.Connection.REASON_PREVIOUS_AND_OUTPUT);
+      });
+      test('Previous connected, adding output', function() {
+        const prevBlock = {
+          isShadow: function() {
+          },
+        };
+        const nextBlock = {
+          isShadow: function() {
+          },
+        };
+        const prevCon = new Blockly.Connection(prevBlock, Blockly.PREVIOUS_STATEMENT);
+        const nextCon = new Blockly.Connection(nextBlock, Blockly.NEXT_STATEMENT);
+        prevBlock.previousConnection = prevCon;
+        nextBlock.nextConnection = nextCon;
+        connectReciprocally(prevCon, nextCon);
+        const outCon = new Blockly.Connection(prevBlock, Blockly.OUTPUT_VALUE);
+        const inBlock = {
+          isShadow: function() {
+          },
+        };
+        const inCon = new Blockly.Connection(inBlock, Blockly.INPUT_VALUE);
+
+        assertReasonHelper(
+            this.checker,
+            outCon,
+            inCon,
+            Blockly.Connection.REASON_PREVIOUS_AND_OUTPUT);
+      });
+    });
   });
   suite('Check Types', function() {
     setup(function() {