From c743a92bb9a38db46260d1385c4cb88d05ef737a Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Wed, 8 Jul 2020 17:28:45 -0700 Subject: [PATCH 01/18] Start work on connection type checker --- blocks/logic.js | 8 +- core/connection.js | 158 ++++++------------- core/connection_checks.js | 243 ++++++++++++++++++++++++++++++ core/connection_db.js | 21 ++- core/keyboard_nav/navigation.js | 2 + core/rendered_connection.js | 2 + core/workspace.js | 3 + core/workspace_svg.js | 4 +- tests/mocha/connection_db_test.js | 26 ++-- 9 files changed, 333 insertions(+), 134 deletions(-) create mode 100644 core/connection_checks.js diff --git a/blocks/logic.js b/blocks/logic.js index f473c040b..c0900fdb5 100644 --- a/blocks/logic.js +++ b/blocks/logic.js @@ -543,7 +543,9 @@ Blockly.Constants.Logic.LOGIC_COMPARE_ONCHANGE_MIXIN = { var blockB = this.getInputTargetBlock('B'); // Disconnect blocks that existed prior to this change if they don't match. if (blockA && blockB && - !blockA.outputConnection.checkType(blockB.outputConnection)) { + !this.workspace.connectionTypeChecker.checkType( + blockA.outputConnection, blockB.outputConnection)) { + //!blockA.outputConnection.checkType(blockB.outputConnection)) { // Mismatch between two inputs. Revert the block connections, // bumping away the newly connected block(s). Blockly.Events.setGroup(e.group); @@ -610,7 +612,9 @@ Blockly.Constants.Logic.LOGIC_TERNARY_ONCHANGE_MIXIN = { if ((blockA || blockB) && parentConnection) { for (var i = 0; i < 2; i++) { var block = (i == 1) ? blockA : blockB; - if (block && !block.outputConnection.checkType(parentConnection)) { + if (block && + block.workspace.connectionTypeChecker.checkType(block.outputConnection, parentConnection)) { + //!block.outputConnection.checkType(parentConnection)) { // Ensure that any disconnections are grouped with the causing event. Blockly.Events.setGroup(e.group); if (parentConnection === this.prevParentConnection_) { diff --git a/core/connection.js b/core/connection.js index 1fe595ad7..e0b24d7d4 100644 --- a/core/connection.js +++ b/core/connection.js @@ -16,6 +16,7 @@ goog.require('Blockly.Events'); goog.require('Blockly.Events.BlockMove'); goog.require('Blockly.Xml'); +goog.requireType('Blockly.ConnectionTypeChecker'); goog.requireType('Blockly.IASTNodeLocationWithBlock'); @@ -145,8 +146,10 @@ Blockly.Connection.prototype.connect_ = function(childConnection) { if (nextBlock && !nextBlock.isShadow()) { newBlock = nextBlock; } else { - if (orphanBlock.previousConnection.checkType( - newBlock.nextConnection)) { + if (orphanBlock.workspace.connectionTypeChecker.checkType( + orphanBlock.previousConnection, newBlock.nextConnection)) { + //orphanBlock.previousConnection.checkType( + //newBlock.nextConnection)) { newBlock.nextConnection.connect(orphanBlock.previousConnection); orphanBlock = null; } @@ -247,28 +250,29 @@ Blockly.Connection.prototype.isConnected = function() { * an error code otherwise. */ Blockly.Connection.prototype.canConnectWithReason = function(target) { - if (!target) { - return Blockly.Connection.REASON_TARGET_NULL; - } - if (this.isSuperior()) { - var blockA = this.sourceBlock_; - var blockB = target.getSourceBlock(); - } else { - var blockB = this.sourceBlock_; - var blockA = target.getSourceBlock(); - } - if (blockA && blockA == blockB) { - return Blockly.Connection.REASON_SELF_CONNECTION; - } else if (target.type != Blockly.OPPOSITE_TYPE[this.type]) { - return Blockly.Connection.REASON_WRONG_TYPE; - } else if (blockA && blockB && blockA.workspace !== blockB.workspace) { - return Blockly.Connection.REASON_DIFFERENT_WORKSPACES; - } else if (!this.checkType(target)) { - return Blockly.Connection.REASON_CHECKS_FAILED; - } else if (blockA.isShadow() && !blockB.isShadow()) { - return Blockly.Connection.REASON_SHADOW_PARENT; - } - return Blockly.Connection.CAN_CONNECT; + return this.sourceBlock_.workspace.connectionTypeChecker.canConnectWithReason(this, target); + // if (!target) { + // return Blockly.Connection.REASON_TARGET_NULL; + // } + // if (this.isSuperior()) { + // var blockA = this.sourceBlock_; + // var blockB = target.getSourceBlock(); + // } else { + // var blockB = this.sourceBlock_; + // var blockA = target.getSourceBlock(); + // } + // if (blockA && blockA == blockB) { + // return Blockly.Connection.REASON_SELF_CONNECTION; + // } else if (target.type != Blockly.OPPOSITE_TYPE[this.type]) { + // return Blockly.Connection.REASON_WRONG_TYPE; + // } else if (blockA && blockB && blockA.workspace !== blockB.workspace) { + // return Blockly.Connection.REASON_DIFFERENT_WORKSPACES; + // } else if (!this.checkType(target)) { + // return Blockly.Connection.REASON_CHECKS_FAILED; + // } else if (blockA.isShadow() && !blockB.isShadow()) { + // return Blockly.Connection.REASON_SHADOW_PARENT; + // } + // return Blockly.Connection.CAN_CONNECT; }; /** @@ -279,26 +283,11 @@ Blockly.Connection.prototype.canConnectWithReason = function(target) { * @package */ Blockly.Connection.prototype.checkConnection = function(target) { - switch (this.canConnectWithReason(target)) { - case Blockly.Connection.CAN_CONNECT: - break; - case Blockly.Connection.REASON_SELF_CONNECTION: - throw Error('Attempted to connect a block to itself.'); - case Blockly.Connection.REASON_DIFFERENT_WORKSPACES: - // Usually this means one block has been deleted. - throw Error('Blocks not on same workspace.'); - case Blockly.Connection.REASON_WRONG_TYPE: - throw Error('Attempt to connect incompatible types.'); - case Blockly.Connection.REASON_TARGET_NULL: - throw Error('Target connection is null.'); - case Blockly.Connection.REASON_CHECKS_FAILED: - var msg = 'Connection checks failed. '; - msg += this + ' expected ' + this.check_ + ', found ' + target.check_; - throw Error(msg); - case Blockly.Connection.REASON_SHADOW_PARENT: - throw Error('Connecting non-shadow to shadow block.'); - default: - throw Error('Unknown connection failure: this should never happen!'); + + var checker = this.sourceBlock_.workspace.connectionTypeChecker; + var reason = checker.canConnectWithReason(this, target); + if (reason != Blockly.Connection.CAN_CONNECT) { + throw Error(checker.getErrorMessage(reason, this, target)); } }; @@ -344,63 +333,8 @@ Blockly.Connection.prototype.canConnectToPrevious_ = function(candidate) { * @return {boolean} True if the connection is allowed, false otherwise. */ Blockly.Connection.prototype.isConnectionAllowed = function(candidate) { - // Don't consider insertion markers. - if (candidate.sourceBlock_.isInsertionMarker()) { - return false; - } - // Type checking. - var canConnect = this.canConnectWithReason(candidate); - if (canConnect != Blockly.Connection.CAN_CONNECT) { - return false; - } - - switch (candidate.type) { - case Blockly.PREVIOUS_STATEMENT: - return this.canConnectToPrevious_(candidate); - case Blockly.OUTPUT_VALUE: { - // Don't offer to connect an already connected left (male) value plug to - // an available right (female) value plug. - if ((candidate.isConnected() && - !candidate.targetBlock().isInsertionMarker()) || - this.isConnected()) { - return false; - } - break; - } - case Blockly.INPUT_VALUE: { - // Offering to connect the left (male) of a value block to an already - // connected value pair is ok, we'll splice it in. - // However, don't offer to splice into an immovable block. - if (candidate.isConnected() && - !candidate.targetBlock().isMovable() && - !candidate.targetBlock().isShadow()) { - return false; - } - break; - } - case Blockly.NEXT_STATEMENT: { - // Don't let a block with no next connection bump other blocks out of the - // stack. But covering up a shadow block or stack of shadow blocks is - // fine. Similarly, replacing a terminal statement with another terminal - // statement is allowed. - if (candidate.isConnected() && - !this.sourceBlock_.nextConnection && - !candidate.targetBlock().isShadow() && - candidate.targetBlock().nextConnection) { - return false; - } - break; - } - default: - throw Error('Unknown connection type in isConnectionAllowed'); - } - - // Don't let blocks try to connect to themselves or ones they nest. - if (Blockly.draggingConnections.indexOf(candidate) != -1) { - return false; - } - - return true; + var checker = this.sourceBlock_.workspace.connectionTypeChecker; + return checker.isConnectionAllowed(this, candidate); }; /** @@ -422,7 +356,13 @@ Blockly.Connection.prototype.connect = function(otherConnection) { // Already connected together. NOP. return; } - this.checkConnection(otherConnection); + + var checker = this.sourceBlock_.workspace.connectionTypeChecker; + var reason = checker.canConnectWithReason(this, otherConnection); + if (reason != Blockly.Connection.CAN_CONNECT) { + throw Error(checker.getErrorMessage(reason, this, otherConnection)); + } + var eventGroup = Blockly.Events.getGroup(); if (!eventGroup) { Blockly.Events.setGroup(true); @@ -598,18 +538,8 @@ Blockly.Connection.prototype.targetBlock = function() { * @return {boolean} True if the connections share a type. */ Blockly.Connection.prototype.checkType = function(otherConnection) { - if (!this.check_ || !otherConnection.check_) { - // One or both sides are promiscuous enough that anything will fit. - return true; - } - // Find any intersection in the check lists. - for (var i = 0; i < this.check_.length; i++) { - if (otherConnection.check_.indexOf(this.check_[i]) != -1) { - return true; - } - } - // No intersection. - return false; + return this.sourceBlock_.workspace.connectionTypeChecker.checkType( + this, otherConnection); }; /** diff --git a/core/connection_checks.js b/core/connection_checks.js new file mode 100644 index 000000000..03da43ccd --- /dev/null +++ b/core/connection_checks.js @@ -0,0 +1,243 @@ +'use strict'; + +goog.provide('Blockly.ConnectionTypeChecker'); + +goog.requireType('Blockly.Connection'); + +/** + * Class for connection type checking logic. + * @constructor + */ +Blockly.ConnectionTypeChecker = function() { + +}; + +/** + * Helper method that translates a connection error code into a string. + * @param {number} errorCode The error code. + * @param {!Blockly.Connection} one One of the two connections being checked. + * @param {!Blockly.Connection} two The second of the two connections being + * checked. + * @return {string} A developer-readable error string. + * @package + */ +Blockly.ConnectionTypeChecker.prototype.getErrorMessage = function(errorCode, + one, two) { + switch (errorCode) { + case Blockly.Connection.REASON_SELF_CONNECTION: + return 'Attempted to connect a block to itself.'; + case Blockly.Connection.REASON_DIFFERENT_WORKSPACES: + // Usually this means one block has been deleted. + return 'Blocks not on same workspace.'; + case Blockly.Connection.REASON_WRONG_TYPE: + return 'Attempt to connect incompatible types.'; + case Blockly.Connection.REASON_TARGET_NULL: + return 'Target connection is null.'; + case Blockly.Connection.REASON_CHECKS_FAILED: + var msg = 'Connection checks failed. '; + msg += one + ' expected ' + one.check_ + ', found ' + two.check_; + return msg; + case Blockly.Connection.REASON_SHADOW_PARENT: + return 'Connecting non-shadow to shadow block.'; + default: + return 'Unknown connection failure: this should never happen!'; + } +}; + +/** + * Checks whether the current connection can connect with the target + * connection. + * @param {Blockly.Connection} one Connection to check compatibility with. + * @param {Blockly.Connection} two Connection to check compatibility with. + * @return {number} Blockly.Connection.CAN_CONNECT if the connection is legal, + * an error code otherwise. + */ +Blockly.ConnectionTypeChecker.prototype.canConnectWithReason = function(one, two) { + if (!two) { + return Blockly.Connection.REASON_TARGET_NULL; + } + if (one.isSuperior()) { + var blockA = one.sourceBlock_; + var blockB = two.getSourceBlock(); + } else { + var blockB = one.sourceBlock_; + var blockA = two.getSourceBlock(); + } + + if (blockA && blockA == blockB) { + return Blockly.Connection.REASON_SELF_CONNECTION; + } else if (two.type != Blockly.OPPOSITE_TYPE[one.type]) { + return Blockly.Connection.REASON_WRONG_TYPE; + } else if (blockA && blockB && blockA.workspace !== blockB.workspace) { + return Blockly.Connection.REASON_DIFFERENT_WORKSPACES; + } else if (!this.checkType(one, two)) { + return Blockly.Connection.REASON_CHECKS_FAILED; + } else if (blockA.isShadow() && !blockB.isShadow()) { + return Blockly.Connection.REASON_SHADOW_PARENT; + } + return Blockly.Connection.CAN_CONNECT; +}; + +/** + * Checks whether the current connection and target connection are compatible + * and throws an exception if they are not. + * @param {!Blockly.Connection} one The connection to check compatibility + * with. + * @param {!Blockly.Connection} two The connection to check compatibility + * with. + * @package + */ +// Blockly.ConnectionTypeChecker.prototype.checkConnection = function(one, two) { +// var reason = one.canConnectWithReason(two); +// if (reason != Blockly.Connection.CAN_CONNECT) { +// throw Error(this.getErrorMessage_(reason, one, two)); +// } +// }; + +/** + * Is this connection compatible with another connection with respect to the + * value type system. E.g. square_root("Hello") is not compatible. + * @param {!Blockly.Connection} one Connection to compare. + * @param {!Blockly.Connection} two Connection to compare against. + * @return {boolean} True if the connections share a type. + */ +Blockly.ConnectionTypeChecker.prototype.checkType = function(one, two) { + var checkArrayOne = one.getCheck(); + var checkArrayTwo = two.getCheck(); + + if (!checkArrayOne || !checkArrayTwo) { + // One or both sides are promiscuous enough that anything will fit. + return true; + } + // Find any intersection in the check lists. + for (var i = 0; i < checkArrayOne.length; i++) { + if (checkArrayTwo.indexOf(checkArrayOne[i]) != -1) { + return true; + } + } + // No intersection. + return false; +}; + + +/** + * Check if the two connections can be dragged to connect to each other. + * @param {!Blockly.Connection} one The moving connection to check. + * @param {!Blockly.Connection} two A stationary connection to check. + * @return {boolean} True if the connection is allowed, false otherwise. + */ +Blockly.ConnectionTypeChecker.prototype.isConnectionAllowed = function(one, two) { + // Don't consider insertion markers. + if (two.sourceBlock_.isInsertionMarker()) { + return false; + } + // Type checking. + var canConnect = one.canConnectWithReason(two); + if (canConnect != Blockly.Connection.CAN_CONNECT) { + return false; + } + + switch (two.type) { + case Blockly.PREVIOUS_STATEMENT: + return one.canConnectToPrevious_(two); + case Blockly.OUTPUT_VALUE: { + // Don't offer to connect an already connected left (male) value plug to + // an available right (female) value plug. + if ((two.isConnected() && + !two.targetBlock().isInsertionMarker()) || + one.isConnected()) { + return false; + } + break; + } + case Blockly.INPUT_VALUE: { + // Offering to connect the left (male) of a value block to an already + // connected value pair is ok, we'll splice it in. + // However, don't offer to splice into an immovable block. + if (two.isConnected() && + !two.targetBlock().isMovable() && + !two.targetBlock().isShadow()) { + return false; + } + break; + } + case Blockly.NEXT_STATEMENT: { + // Don't let a block with no next connection bump other blocks out of the + // stack. But covering up a shadow block or stack of shadow blocks is + // fine. Similarly, replacing a terminal statement with another terminal + // statement is allowed. + if (two.isConnected() && + !one.sourceBlock_.nextConnection && + !two.targetBlock().isShadow() && + two.targetBlock().nextConnection) { + return false; + } + break; + } + default: + throw Error('Unknown connection type in isConnectionAllowed'); + } + + // Don't let blocks try to connect to themselves or ones they nest. + if (Blockly.draggingConnections.indexOf(two) != -1) { + return false; + } + + return true; +}; + +/** + * Check if the two connections can be dragged to connect to each other. + * @param {!Blockly.RenderedConnection} dragging The connection to check on a + * block that is being dragged. + * @param {!Blockly.RenderedConnection} candidate The stationary connection to + * check. + * @param {number=} maxRadius The maximum radius allowed for connections, in + * workspace units. + * @return {boolean} True if the connection is allowed, false otherwise. + */ +Blockly.ConnectionTypeChecker.prototype.canConnectDuringDrag = function( + dragging, candidate, maxRadius) { + if (dragging.distanceFrom(candidate) > maxRadius) { + return false; + } + + return this.isConnectionAllowed(dragging, candidate); +}; +/** + * Check if the two connections can be dragged to connect to each other. + * This is used by the connection database when searching for the closest + * connection. + * @param {!Blockly.Connection} one The connection to check, which must be a + * statement input or next connection. + * @param {!Blockly.Connection} two A nearby connection to check, which + * must be a previous connection. + * @return {boolean} True if the connection is allowed, false otherwise. + * @private + */ +Blockly.ConnectionTypeChecker.prototype.canConnectToPrevious_ = function(one, two) { + if (one.targetConnection) { + // This connection is already occupied. + // A next connection will never disconnect itself mid-drag. + return false; + } + + // Don't let blocks try to connect to themselves or ones they nest. + if (Blockly.draggingConnections.indexOf(two) != -1) { + return false; + } + + if (!two.targetConnection) { + return true; + } + + var targetBlock = two.targetBlock(); + // If it is connected to a real block, game over. + if (!targetBlock.isInsertionMarker()) { + return false; + } + // If it's connected to an insertion marker but that insertion marker + // is the first block in a stack, it's still fine. If that insertion + // marker is in the middle of a stack, it won't work. + return !targetBlock.getPreviousBlock(); +}; diff --git a/core/connection_db.js b/core/connection_db.js index 5d71e39c0..5a8283f12 100644 --- a/core/connection_db.js +++ b/core/connection_db.js @@ -16,6 +16,8 @@ goog.provide('Blockly.ConnectionDB'); goog.require('Blockly.RenderedConnection'); +goog.requireType('Blockly.ConnectionTypeChecker'); + /** * Database of connections. @@ -23,13 +25,14 @@ goog.require('Blockly.RenderedConnection'); * connections in an area may be looked up quickly using a binary search. * @constructor */ -Blockly.ConnectionDB = function() { +Blockly.ConnectionDB = function(typeChecker) { /** * Array of connections sorted by y position in workspace units. * @type {!Array.} * @private */ this.connections_ = []; + this.typeChecker_ = typeChecker; }; /** @@ -240,7 +243,8 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, var pointerMin = closestIndex - 1; while (pointerMin >= 0 && this.isInYRange_(pointerMin, conn.y, maxRadius)) { temp = this.connections_[pointerMin]; - if (conn.isConnectionAllowed(temp, bestRadius)) { + if (this.typeChecker_.canConnectDuringDrag(conn, temp, bestRadius)) { + //conn.isConnectionAllowed(temp, bestRadius)) { bestConnection = temp; bestRadius = temp.distanceFrom(conn); } @@ -251,7 +255,8 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, while (pointerMax < this.connections_.length && this.isInYRange_(pointerMax, conn.y, maxRadius)) { temp = this.connections_[pointerMax]; - if (conn.isConnectionAllowed(temp, bestRadius)) { + if (this.typeChecker_.canConnectDuringDrag(conn, temp, bestRadius)) { + //conn.isConnectionAllowed(temp, bestRadius)) { bestConnection = temp; bestRadius = temp.distanceFrom(conn); } @@ -270,12 +275,12 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, * Initialize a set of connection DBs for a workspace. * @return {!Array.} Array of databases. */ -Blockly.ConnectionDB.init = function() { +Blockly.ConnectionDB.init = function(typeChecker) { // Create four databases, one for each connection type. var dbList = []; - dbList[Blockly.INPUT_VALUE] = new Blockly.ConnectionDB(); - dbList[Blockly.OUTPUT_VALUE] = new Blockly.ConnectionDB(); - dbList[Blockly.NEXT_STATEMENT] = new Blockly.ConnectionDB(); - dbList[Blockly.PREVIOUS_STATEMENT] = new Blockly.ConnectionDB(); + dbList[Blockly.INPUT_VALUE] = new Blockly.ConnectionDB(typeChecker); + dbList[Blockly.OUTPUT_VALUE] = new Blockly.ConnectionDB(typeChecker); + dbList[Blockly.NEXT_STATEMENT] = new Blockly.ConnectionDB(typeChecker); + dbList[Blockly.PREVIOUS_STATEMENT] = new Blockly.ConnectionDB(typeChecker); return dbList; }; diff --git a/core/keyboard_nav/navigation.js b/core/keyboard_nav/navigation.js index 914882592..9a59ee5c3 100644 --- a/core/keyboard_nav/navigation.js +++ b/core/keyboard_nav/navigation.js @@ -18,6 +18,8 @@ goog.require('Blockly.ASTNode'); goog.require('Blockly.utils.Coordinate'); goog.require('Blockly.user.keyMap'); +goog.requireType('Blockly.ConnectionTypeChecker'); + /** * A function to call to give feedback to the user about logs, warnings, and * errors. You can override this to customize feedback (e.g. warning sounds, diff --git a/core/rendered_connection.js b/core/rendered_connection.js index b1d19fed8..a2bd877e2 100644 --- a/core/rendered_connection.js +++ b/core/rendered_connection.js @@ -19,6 +19,8 @@ goog.require('Blockly.utils.Coordinate'); goog.require('Blockly.utils.dom'); goog.require('Blockly.utils.object'); +goog.requireType('Blockly.ConnectionTypeChecker'); + /** * Class for a connection between blocks that may be rendered on screen. diff --git a/core/workspace.js b/core/workspace.js index 2c6d2ece3..b92750d3f 100644 --- a/core/workspace.js +++ b/core/workspace.js @@ -12,6 +12,7 @@ goog.provide('Blockly.Workspace'); +goog.require('Blockly.ConnectionTypeChecker'); goog.require('Blockly.Events'); goog.require('Blockly.Options'); goog.require('Blockly.utils'); @@ -103,6 +104,8 @@ Blockly.Workspace = function(opt_options) { * @private */ this.potentialVariableMap_ = null; + /** @type {Blockly.ConnectionTypeChecker} [description] */ + this.connectionTypeChecker = new Blockly.ConnectionTypeChecker(); }; /** diff --git a/core/workspace_svg.js b/core/workspace_svg.js index b1f996e13..e2fe81cd3 100644 --- a/core/workspace_svg.js +++ b/core/workspace_svg.js @@ -44,6 +44,7 @@ goog.require('Blockly.Xml'); goog.requireType('Blockly.blockRendering.Renderer'); goog.requireType('Blockly.IASTNodeLocationSvg'); goog.requireType('Blockly.IBoundedElement'); +goog.requireType('Blockly.ConnectionTypeChecker'); /** @@ -68,7 +69,8 @@ Blockly.WorkspaceSvg = function(options, this.setMetrics = options.setMetrics || Blockly.WorkspaceSvg.setTopLevelWorkspaceMetrics_; - this.connectionDBList = Blockly.ConnectionDB.init(); + + this.connectionDBList = Blockly.ConnectionDB.init(this.connectionTypeChecker); if (opt_blockDragSurface) { this.blockDragSurface_ = opt_blockDragSurface; diff --git a/tests/mocha/connection_db_test.js b/tests/mocha/connection_db_test.js index 99aabcda9..34165831e 100644 --- a/tests/mocha/connection_db_test.js +++ b/tests/mocha/connection_db_test.js @@ -6,7 +6,7 @@ suite('Connection Database', function() { setup(function() { - this.database = new Blockly.ConnectionDB(); + this.database = new Blockly.ConnectionDB(new Blockly.ConnectionTypeChecker()); this.assertOrder = function() { var length = this.database.connections_.length; @@ -194,18 +194,26 @@ suite('Connection Database', function() { suite('Search For Closest', function() { setup(function() { this.allowedStub = null; - - this.createCheckConnection = function(x, y) { - var checkConnection = this.createConnection(x, y, Blockly.NEXT_STATEMENT, - new Blockly.ConnectionDB()); - this.allowedStub = sinon.stub(checkConnection, 'isConnectionAllowed') - .callsFake(function(candidate, maxRadius) { - if (this.distanceFrom(candidate) > maxRadius) { + this.database.typeChecker_.canConnectDuringDrag = function( + dragging, candidate, maxRadius) { + if (dragging.distanceFrom(candidate) > maxRadius) { return false; } // Ignore non-distance parameters. return true; - }); + }; + + this.createCheckConnection = function(x, y) { + var checkConnection = this.createConnection(x, y, Blockly.NEXT_STATEMENT, + new Blockly.ConnectionDB()); + // this.allowedStub = sinon.stub(checkConnection, 'isConnectionAllowed') + // .callsFake(function(candidate, maxRadius) { + // if (this.distanceFrom(candidate) > maxRadius) { + // return false; + // } + // // Ignore non-distance parameters. + // return true; + // }); return checkConnection; }; }); From 8c17e325b5d8ede2df860899b84ab87f7b9de3ff Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 9 Jul 2020 13:19:35 -0700 Subject: [PATCH 02/18] Update connection tests to directly use the type checker --- core/connection.js | 106 +++++++++++--------------- core/connection_checks.js | 29 ++----- tests/mocha/connection_test.js | 134 ++++++++++++++++++++++++++------- 3 files changed, 156 insertions(+), 113 deletions(-) diff --git a/core/connection.js b/core/connection.js index e0b24d7d4..9e769a1a9 100644 --- a/core/connection.js +++ b/core/connection.js @@ -250,29 +250,7 @@ Blockly.Connection.prototype.isConnected = function() { * an error code otherwise. */ Blockly.Connection.prototype.canConnectWithReason = function(target) { - return this.sourceBlock_.workspace.connectionTypeChecker.canConnectWithReason(this, target); - // if (!target) { - // return Blockly.Connection.REASON_TARGET_NULL; - // } - // if (this.isSuperior()) { - // var blockA = this.sourceBlock_; - // var blockB = target.getSourceBlock(); - // } else { - // var blockB = this.sourceBlock_; - // var blockA = target.getSourceBlock(); - // } - // if (blockA && blockA == blockB) { - // return Blockly.Connection.REASON_SELF_CONNECTION; - // } else if (target.type != Blockly.OPPOSITE_TYPE[this.type]) { - // return Blockly.Connection.REASON_WRONG_TYPE; - // } else if (blockA && blockB && blockA.workspace !== blockB.workspace) { - // return Blockly.Connection.REASON_DIFFERENT_WORKSPACES; - // } else if (!this.checkType(target)) { - // return Blockly.Connection.REASON_CHECKS_FAILED; - // } else if (blockA.isShadow() && !blockB.isShadow()) { - // return Blockly.Connection.REASON_SHADOW_PARENT; - // } - // return Blockly.Connection.CAN_CONNECT; + return this.getConnectionTypeChecker().canConnectWithReason(this, target); }; /** @@ -284,57 +262,60 @@ Blockly.Connection.prototype.canConnectWithReason = function(target) { */ Blockly.Connection.prototype.checkConnection = function(target) { - var checker = this.sourceBlock_.workspace.connectionTypeChecker; + var checker = this.getConnectionTypeChecker(); var reason = checker.canConnectWithReason(this, target); if (reason != Blockly.Connection.CAN_CONNECT) { throw Error(checker.getErrorMessage(reason, this, target)); } }; -/** - * Check if the two connections can be dragged to connect to each other. - * This is used by the connection database when searching for the closest - * connection. - * @param {!Blockly.Connection} candidate A nearby connection to check, which - * must be a previous connection. - * @return {boolean} True if the connection is allowed, false otherwise. - * @private - */ -Blockly.Connection.prototype.canConnectToPrevious_ = function(candidate) { - if (this.targetConnection) { - // This connection is already occupied. - // A next connection will never disconnect itself mid-drag. - return false; - } - - // Don't let blocks try to connect to themselves or ones they nest. - if (Blockly.draggingConnections.indexOf(candidate) != -1) { - return false; - } - - if (!candidate.targetConnection) { - return true; - } - - var targetBlock = candidate.targetBlock(); - // If it is connected to a real block, game over. - if (!targetBlock.isInsertionMarker()) { - return false; - } - // If it's connected to an insertion marker but that insertion marker - // is the first block in a stack, it's still fine. If that insertion - // marker is in the middle of a stack, it won't work. - return !targetBlock.getPreviousBlock(); +Blockly.Connection.prototype.getConnectionTypeChecker = function() { + return this.sourceBlock_.workspace.connectionTypeChecker; }; +// /** +// * Check if the two connections can be dragged to connect to each other. +// * This is used by the connection database when searching for the closest +// * connection. +// * @param {!Blockly.Connection} candidate A nearby connection to check, which +// * must be a previous connection. +// * @return {boolean} True if the connection is allowed, false otherwise. +// * @private +// */ +// Blockly.Connection.prototype.canConnectToPrevious_ = function(candidate) { +// if (this.targetConnection) { +// // This connection is already occupied. +// // A next connection will never disconnect itself mid-drag. +// return false; +// } + +// // Don't let blocks try to connect to themselves or ones they nest. +// if (Blockly.draggingConnections.indexOf(candidate) != -1) { +// return false; +// } + +// if (!candidate.targetConnection) { +// return true; +// } + +// var targetBlock = candidate.targetBlock(); +// // If it is connected to a real block, game over. +// if (!targetBlock.isInsertionMarker()) { +// return false; +// } +// // If it's connected to an insertion marker but that insertion marker +// // is the first block in a stack, it's still fine. If that insertion +// // marker is in the middle of a stack, it won't work. +// return !targetBlock.getPreviousBlock(); +// }; + /** * Check if the two connections can be dragged to connect to each other. * @param {!Blockly.Connection} candidate A nearby connection to check. * @return {boolean} True if the connection is allowed, false otherwise. */ Blockly.Connection.prototype.isConnectionAllowed = function(candidate) { - var checker = this.sourceBlock_.workspace.connectionTypeChecker; - return checker.isConnectionAllowed(this, candidate); + return this.getConnectionTypeChecker().isConnectionAllowed(this, candidate); }; /** @@ -357,7 +338,7 @@ Blockly.Connection.prototype.connect = function(otherConnection) { return; } - var checker = this.sourceBlock_.workspace.connectionTypeChecker; + var checker = this.getConnectionTypeChecker(); var reason = checker.canConnectWithReason(this, otherConnection); if (reason != Blockly.Connection.CAN_CONNECT) { throw Error(checker.getErrorMessage(reason, this, otherConnection)); @@ -538,8 +519,7 @@ Blockly.Connection.prototype.targetBlock = function() { * @return {boolean} True if the connections share a type. */ Blockly.Connection.prototype.checkType = function(otherConnection) { - return this.sourceBlock_.workspace.connectionTypeChecker.checkType( - this, otherConnection); + return this.getConnectionTypeChecker().checkType(this, otherConnection); }; /** diff --git a/core/connection_checks.js b/core/connection_checks.js index 03da43ccd..ade3ef949 100644 --- a/core/connection_checks.js +++ b/core/connection_checks.js @@ -53,47 +53,32 @@ Blockly.ConnectionTypeChecker.prototype.getErrorMessage = function(errorCode, * an error code otherwise. */ Blockly.ConnectionTypeChecker.prototype.canConnectWithReason = function(one, two) { - if (!two) { + if (!one || !two) { return Blockly.Connection.REASON_TARGET_NULL; } if (one.isSuperior()) { - var blockA = one.sourceBlock_; + var blockA = one.getSourceBlock(); var blockB = two.getSourceBlock(); } else { - var blockB = one.sourceBlock_; + var blockB = one.getSourceBlock(); var blockA = two.getSourceBlock(); } - + // TODO (fenichel): The null checks seem like they're only for making tests + // work better. if (blockA && blockA == blockB) { return Blockly.Connection.REASON_SELF_CONNECTION; } else if (two.type != Blockly.OPPOSITE_TYPE[one.type]) { return Blockly.Connection.REASON_WRONG_TYPE; } else if (blockA && blockB && blockA.workspace !== blockB.workspace) { return Blockly.Connection.REASON_DIFFERENT_WORKSPACES; - } else if (!this.checkType(one, two)) { - return Blockly.Connection.REASON_CHECKS_FAILED; } else if (blockA.isShadow() && !blockB.isShadow()) { return Blockly.Connection.REASON_SHADOW_PARENT; + } else if (!this.checkType(one, two)) { + return Blockly.Connection.REASON_CHECKS_FAILED; } return Blockly.Connection.CAN_CONNECT; }; -/** - * Checks whether the current connection and target connection are compatible - * and throws an exception if they are not. - * @param {!Blockly.Connection} one The connection to check compatibility - * with. - * @param {!Blockly.Connection} two The connection to check compatibility - * with. - * @package - */ -// Blockly.ConnectionTypeChecker.prototype.checkConnection = function(one, two) { -// var reason = one.canConnectWithReason(two); -// if (reason != Blockly.Connection.CAN_CONNECT) { -// throw Error(this.getErrorMessage_(reason, one, two)); -// } -// }; - /** * Is this connection compatible with another connection with respect to the * value type system. E.g. square_root("Hello") is not compatible. diff --git a/tests/mocha/connection_test.js b/tests/mocha/connection_test.js index f78584cfa..a4ac148d7 100644 --- a/tests/mocha/connection_test.js +++ b/tests/mocha/connection_test.js @@ -4,11 +4,23 @@ * SPDX-License-Identifier: Apache-2.0 */ -suite('Connections', function() { +suite('Connection type checker', function() { + suiteSetup(function() { + this.checker = new Blockly.ConnectionTypeChecker(); + }); suite('Can Connect With Reason', function() { + function assertReasonHelper(checker, one, two, reason) { + chai.assert.equal(checker.canConnectWithReason(one, two), reason); + // Order should not matter. + chai.assert.equal(checker.canConnectWithReason(two, one), reason); + } + test('Target Null', function() { var connection = new Blockly.Connection({}, Blockly.INPUT_VALUE); - chai.assert.equal(connection.canConnectWithReason(null), + assertReasonHelper( + this.checker, + connection, + null, Blockly.Connection.REASON_TARGET_NULL); }); test('Target Self', function() { @@ -16,7 +28,10 @@ suite('Connections', function() { var connection1 = new Blockly.Connection(block, Blockly.INPUT_VALUE); var connection2 = new Blockly.Connection(block, Blockly.OUTPUT_VALUE); - chai.assert.equal(connection1.canConnectWithReason(connection2), + assertReasonHelper( + this.checker, + connection1, + connection2, Blockly.Connection.REASON_SELF_CONNECTION); }); test('Different Workspaces', function() { @@ -25,7 +40,10 @@ suite('Connections', function() { var connection2 = new Blockly.Connection( {workspace: 2}, Blockly.OUTPUT_VALUE); - chai.assert.equal(connection1.canConnectWithReason(connection2), + assertReasonHelper( + this.checker, + connection1, + connection2, Blockly.Connection.REASON_DIFFERENT_WORKSPACES); }); suite('Types', function() { @@ -46,51 +64,87 @@ suite('Connections', function() { inBlock, Blockly.INPUT_VALUE); }); test('Previous, Next', function() { - chai.assert.equal(this.previous.canConnectWithReason(this.next), + assertReasonHelper( + this.checker, + this.previous, + this.next, Blockly.Connection.CAN_CONNECT); }); test('Previous, Output', function() { - chai.assert.equal(this.previous.canConnectWithReason(this.output), + assertReasonHelper( + this.checker, + this.previous, + this.output, Blockly.Connection.REASON_WRONG_TYPE); }); test('Previous, Input', function() { - chai.assert.equal(this.previous.canConnectWithReason(this.input), + assertReasonHelper( + this.checker, + this.previous, + this.input, Blockly.Connection.REASON_WRONG_TYPE); }); test('Next, Previous', function() { - chai.assert.equal(this.next.canConnectWithReason(this.previous), + assertReasonHelper( + this.checker, + this.next, + this.previous, Blockly.Connection.CAN_CONNECT); }); test('Next, Output', function() { - chai.assert.equal(this.next.canConnectWithReason(this.output), + assertReasonHelper( + this.checker, + this.next, + this.output, Blockly.Connection.REASON_WRONG_TYPE); }); test('Next, Input', function() { - chai.assert.equal(this.next.canConnectWithReason(this.input), + assertReasonHelper( + this.checker, + this.next, + this.input, Blockly.Connection.REASON_WRONG_TYPE); }); test('Output, Previous', function() { - chai.assert.equal(this.output.canConnectWithReason(this.previous), + assertReasonHelper( + this.checker, + this.previous, + this.output, Blockly.Connection.REASON_WRONG_TYPE); }); test('Output, Next', function() { - chai.assert.equal(this.output.canConnectWithReason(this.next), + assertReasonHelper( + this.checker, + this.output, + this.next, Blockly.Connection.REASON_WRONG_TYPE); }); test('Output, Input', function() { - chai.assert.equal(this.output.canConnectWithReason(this.input), + assertReasonHelper( + this.checker, + this.output, + this.input, Blockly.Connection.CAN_CONNECT); }); test('Input, Previous', function() { - chai.assert.equal(this.input.canConnectWithReason(this.previous), + assertReasonHelper( + this.checker, + this.previous, + this.input, Blockly.Connection.REASON_WRONG_TYPE); }); test('Input, Next', function() { - chai.assert.equal(this.input.canConnectWithReason(this.next), + assertReasonHelper( + this.checker, + this.input, + this.next, Blockly.Connection.REASON_WRONG_TYPE); }); test('Input, Output', function() { - chai.assert.equal(this.input.canConnectWithReason(this.output), + assertReasonHelper( + this.checker, + this.input, + this.output, Blockly.Connection.CAN_CONNECT); }); }); @@ -101,7 +155,10 @@ suite('Connections', function() { var prev = new Blockly.Connection(prevBlock, Blockly.PREVIOUS_STATEMENT); var next = new Blockly.Connection(nextBlock, Blockly.NEXT_STATEMENT); - chai.assert.equal(prev.canConnectWithReason(next), + assertReasonHelper( + this.checker, + prev, + next, Blockly.Connection.CAN_CONNECT); }); test('Next Shadow', function() { @@ -110,7 +167,10 @@ suite('Connections', function() { var prev = new Blockly.Connection(prevBlock, Blockly.PREVIOUS_STATEMENT); var next = new Blockly.Connection(nextBlock, Blockly.NEXT_STATEMENT); - chai.assert.equal(prev.canConnectWithReason(next), + assertReasonHelper( + this.checker, + prev, + next, Blockly.Connection.REASON_SHADOW_PARENT); }); test('Prev and Next Shadow', function() { @@ -119,7 +179,10 @@ suite('Connections', function() { var prev = new Blockly.Connection(prevBlock, Blockly.PREVIOUS_STATEMENT); var next = new Blockly.Connection(nextBlock, Blockly.NEXT_STATEMENT); - chai.assert.equal(prev.canConnectWithReason(next), + assertReasonHelper( + this.checker, + prev, + next, Blockly.Connection.CAN_CONNECT); }); test('Output Shadow', function() { @@ -128,7 +191,10 @@ suite('Connections', function() { var outCon = new Blockly.Connection(outBlock, Blockly.OUTPUT_VALUE); var inCon = new Blockly.Connection(inBlock, Blockly.INPUT_VALUE); - chai.assert.equal(outCon.canConnectWithReason(inCon), + assertReasonHelper( + this.checker, + outCon, + inCon, Blockly.Connection.CAN_CONNECT); }); test('Input Shadow', function() { @@ -137,7 +203,10 @@ suite('Connections', function() { var outCon = new Blockly.Connection(outBlock, Blockly.OUTPUT_VALUE); var inCon = new Blockly.Connection(inBlock, Blockly.INPUT_VALUE); - chai.assert.equal(outCon.canConnectWithReason(inCon), + assertReasonHelper( + this.checker, + outCon, + inCon, Blockly.Connection.REASON_SHADOW_PARENT); }); test('Output and Input Shadow', function() { @@ -146,7 +215,10 @@ suite('Connections', function() { var outCon = new Blockly.Connection(outBlock, Blockly.OUTPUT_VALUE); var inCon = new Blockly.Connection(inBlock, Blockly.INPUT_VALUE); - chai.assert.equal(outCon.canConnectWithReason(inCon), + assertReasonHelper( + this.checker, + outCon, + inCon, Blockly.Connection.CAN_CONNECT); }); }); @@ -156,32 +228,38 @@ suite('Connections', function() { this.con1 = new Blockly.Connection({}, Blockly.PREVIOUS_STATEMENT); this.con2 = new Blockly.Connection({}, Blockly.NEXT_STATEMENT); }); + function assertCheckTypes(checker, one, two) { + chai.assert.isTrue(checker.checkType(one, two)); + // Order should not matter. + chai.assert.isTrue(checker.checkType(one, two)); + } test('No Types', function() { - chai.assert.isTrue(this.con1.checkType((this.con2))); + assertCheckTypes(this.checker, this.con1, this.con2); + chai.assert.isTrue(this.checker.checkType(this.con1, this.con2)); }); test('Same Type', function() { this.con1.setCheck('type1'); this.con2.setCheck('type1'); - chai.assert.isTrue(this.con1.checkType((this.con2))); + assertCheckTypes(this.checker, this.con1, this.con2); }); test('Same Types', function() { this.con1.setCheck(['type1', 'type2']); this.con2.setCheck(['type1', 'type2']); - chai.assert.isTrue(this.con1.checkType((this.con2))); + assertCheckTypes(this.checker, this.con1, this.con2); }); test('Single Same Type', function() { this.con1.setCheck(['type1', 'type2']); this.con2.setCheck(['type1', 'type3']); - chai.assert.isTrue(this.con1.checkType((this.con2))); + assertCheckTypes(this.checker, this.con1, this.con2); }); test('One Typed, One Promiscuous', function() { this.con1.setCheck('type1'); - chai.assert.isTrue(this.con1.checkType((this.con2))); + assertCheckTypes(this.checker, this.con1, this.con2); }); test('No Compatible Types', function() { this.con1.setCheck('type1'); this.con2.setCheck('type2'); - chai.assert.isFalse(this.con1.checkType((this.con2))); + chai.assert.isFalse(this.checker.checkType(this.con1, this.con2)); }); }); }); From bb8348befd4be91e97e4dd51059a868020d559e3 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 9 Jul 2020 16:56:48 -0700 Subject: [PATCH 03/18] Move to a single canConnect function, and update tests --- blocks/logic.js | 11 ++-- core/connection.js | 17 +++--- core/connection_checks.js | 94 +++++++++++++++++++------------ core/connection_db.js | 15 +++-- core/keyboard_nav/navigation.js | 10 ++-- tests/mocha/connection_db_test.js | 21 ++----- tests/mocha/index.html | 1 - 7 files changed, 89 insertions(+), 80 deletions(-) diff --git a/blocks/logic.js b/blocks/logic.js index c0900fdb5..6cea9d1d4 100644 --- a/blocks/logic.js +++ b/blocks/logic.js @@ -543,9 +543,9 @@ Blockly.Constants.Logic.LOGIC_COMPARE_ONCHANGE_MIXIN = { var blockB = this.getInputTargetBlock('B'); // Disconnect blocks that existed prior to this change if they don't match. if (blockA && blockB && - !this.workspace.connectionTypeChecker.checkType( - blockA.outputConnection, blockB.outputConnection)) { - //!blockA.outputConnection.checkType(blockB.outputConnection)) { + //!this.workspace.connectionTypeChecker.checkType( + //blockA.outputConnection, blockB.outputConnection)) { + !blockA.outputConnection.checkType(blockB.outputConnection)) { // Mismatch between two inputs. Revert the block connections, // bumping away the newly connected block(s). Blockly.Events.setGroup(e.group); @@ -613,8 +613,9 @@ Blockly.Constants.Logic.LOGIC_TERNARY_ONCHANGE_MIXIN = { for (var i = 0; i < 2; i++) { var block = (i == 1) ? blockA : blockB; if (block && - block.workspace.connectionTypeChecker.checkType(block.outputConnection, parentConnection)) { - //!block.outputConnection.checkType(parentConnection)) { + // !block.workspace.connectionTypeChecker.checkType( + // block.outputConnection, parentConnection)) { + !block.outputConnection.checkType(parentConnection)) { // Ensure that any disconnections are grouped with the causing event. Blockly.Events.setGroup(e.group); if (parentConnection === this.prevParentConnection_) { diff --git a/core/connection.js b/core/connection.js index 9e769a1a9..a3597de03 100644 --- a/core/connection.js +++ b/core/connection.js @@ -47,6 +47,7 @@ Blockly.Connection.REASON_TARGET_NULL = 3; Blockly.Connection.REASON_CHECKS_FAILED = 4; Blockly.Connection.REASON_DIFFERENT_WORKSPACES = 5; Blockly.Connection.REASON_SHADOW_PARENT = 6; +Blockly.Connection.REASON_DRAG_CHECKS_FAILED = 7; /** * Connection this connection connects to. Null if not connected. @@ -250,6 +251,7 @@ Blockly.Connection.prototype.isConnected = function() { * an error code otherwise. */ Blockly.Connection.prototype.canConnectWithReason = function(target) { + // TODO: deprecation warning with date, plus tests. return this.getConnectionTypeChecker().canConnectWithReason(this, target); }; @@ -261,12 +263,10 @@ Blockly.Connection.prototype.canConnectWithReason = function(target) { * @package */ Blockly.Connection.prototype.checkConnection = function(target) { - +// TODO: Add deprecation warning notices *and* add tests to make sure these +// still work (for any blocks that use them). var checker = this.getConnectionTypeChecker(); - var reason = checker.canConnectWithReason(this, target); - if (reason != Blockly.Connection.CAN_CONNECT) { - throw Error(checker.getErrorMessage(reason, this, target)); - } + checker.canConnect(this, target, false, true); }; Blockly.Connection.prototype.getConnectionTypeChecker = function() { @@ -315,7 +315,7 @@ Blockly.Connection.prototype.getConnectionTypeChecker = function() { * @return {boolean} True if the connection is allowed, false otherwise. */ Blockly.Connection.prototype.isConnectionAllowed = function(candidate) { - return this.getConnectionTypeChecker().isConnectionAllowed(this, candidate); + return this.getConnectionTypeChecker().canConnect(this, candidate, true, false); }; /** @@ -339,10 +339,7 @@ Blockly.Connection.prototype.connect = function(otherConnection) { } var checker = this.getConnectionTypeChecker(); - var reason = checker.canConnectWithReason(this, otherConnection); - if (reason != Blockly.Connection.CAN_CONNECT) { - throw Error(checker.getErrorMessage(reason, this, otherConnection)); - } + checker.canConnect(this, otherConnection, false, true); var eventGroup = Blockly.Events.getGroup(); if (!eventGroup) { diff --git a/core/connection_checks.js b/core/connection_checks.js index ade3ef949..7e51f62cb 100644 --- a/core/connection_checks.js +++ b/core/connection_checks.js @@ -9,7 +9,6 @@ goog.requireType('Blockly.Connection'); * @constructor */ Blockly.ConnectionTypeChecker = function() { - }; /** @@ -39,6 +38,8 @@ Blockly.ConnectionTypeChecker.prototype.getErrorMessage = function(errorCode, return msg; case Blockly.Connection.REASON_SHADOW_PARENT: return 'Connecting non-shadow to shadow block.'; + case Blockly.Connection.REASON_DRAG_CHECKS_FAILED: + return 'Drag checks failed.' default: return 'Unknown connection failure: this should never happen!'; } @@ -53,6 +54,17 @@ Blockly.ConnectionTypeChecker.prototype.getErrorMessage = function(errorCode, * an error code otherwise. */ Blockly.ConnectionTypeChecker.prototype.canConnectWithReason = function(one, two) { + var validity = this.doValidityChecks(one, two); + if (validity != Blockly.Connection.CAN_CONNECT) { + return validity; + } + if (!this.checkType(one, two)) { + return Blockly.Connection.REASON_CHECKS_FAILED; + } + return Blockly.Connection.CAN_CONNECT; +}; + +Blockly.ConnectionTypeChecker.prototype.doValidityChecks = function(one, two) { if (!one || !two) { return Blockly.Connection.REASON_TARGET_NULL; } @@ -65,20 +77,58 @@ Blockly.ConnectionTypeChecker.prototype.canConnectWithReason = function(one, two } // TODO (fenichel): The null checks seem like they're only for making tests // work better. - if (blockA && blockA == blockB) { + if (blockA == blockB) { return Blockly.Connection.REASON_SELF_CONNECTION; } else if (two.type != Blockly.OPPOSITE_TYPE[one.type]) { return Blockly.Connection.REASON_WRONG_TYPE; - } else if (blockA && blockB && blockA.workspace !== blockB.workspace) { + } else if (blockA.workspace !== blockB.workspace) { return Blockly.Connection.REASON_DIFFERENT_WORKSPACES; } else if (blockA.isShadow() && !blockB.isShadow()) { return Blockly.Connection.REASON_SHADOW_PARENT; - } else if (!this.checkType(one, two)) { - return Blockly.Connection.REASON_CHECKS_FAILED; } return Blockly.Connection.CAN_CONNECT; }; +/** + * Checks whether the current connection can connect with the target + * connection. + * @param {Blockly.Connection} one Connection to check compatibility with. + * @param {Blockly.Connection} two Connection to check compatibility with. + * @return {boolean} Whether the connection is legal. + */ +Blockly.ConnectionTypeChecker.prototype.canConnect = function(one, two, + isDragging, shouldThrow) { + var validity = this.doValidityChecks(one, two); + if (validity != Blockly.Connection.CAN_CONNECT) { + if (shouldThrow) { + throw Error(this.getErrorMessage(validity, one, two)); + } + return false; + } + + var passesTypeChecks = this.checkType(one, two); + if (!passesTypeChecks) { + + if (shouldThrow) { + throw Error(this.getErrorMessage( + Blockly.Connection.REASON_CHECKS_FAILED, one, two)); + } + return false; + } + + if (isDragging) { + var passesDragChecks = this.passesDragChecks(one, two); + if (!passesDragChecks) { + if (shouldThrow) { + throw Error(this.getErrorMessage( + Blockly.Connection.REASON_DRAG_CHECKS_FAILED, one, two)); + } + return false; + } + } + return true; +}; + /** * Is this connection compatible with another connection with respect to the * value type system. E.g. square_root("Hello") is not compatible. @@ -104,23 +154,11 @@ Blockly.ConnectionTypeChecker.prototype.checkType = function(one, two) { return false; }; - -/** - * Check if the two connections can be dragged to connect to each other. - * @param {!Blockly.Connection} one The moving connection to check. - * @param {!Blockly.Connection} two A stationary connection to check. - * @return {boolean} True if the connection is allowed, false otherwise. - */ -Blockly.ConnectionTypeChecker.prototype.isConnectionAllowed = function(one, two) { +Blockly.ConnectionTypeChecker.prototype.passesDragChecks = function(one, two) { // Don't consider insertion markers. if (two.sourceBlock_.isInsertionMarker()) { return false; } - // Type checking. - var canConnect = one.canConnectWithReason(two); - if (canConnect != Blockly.Connection.CAN_CONNECT) { - return false; - } switch (two.type) { case Blockly.PREVIOUS_STATEMENT: @@ -160,7 +198,7 @@ Blockly.ConnectionTypeChecker.prototype.isConnectionAllowed = function(one, two) break; } default: - throw Error('Unknown connection type in isConnectionAllowed'); + throw Error('Unknown connection type in passesDragChecks'); } // Don't let blocks try to connect to themselves or ones they nest. @@ -171,24 +209,6 @@ Blockly.ConnectionTypeChecker.prototype.isConnectionAllowed = function(one, two) return true; }; -/** - * Check if the two connections can be dragged to connect to each other. - * @param {!Blockly.RenderedConnection} dragging The connection to check on a - * block that is being dragged. - * @param {!Blockly.RenderedConnection} candidate The stationary connection to - * check. - * @param {number=} maxRadius The maximum radius allowed for connections, in - * workspace units. - * @return {boolean} True if the connection is allowed, false otherwise. - */ -Blockly.ConnectionTypeChecker.prototype.canConnectDuringDrag = function( - dragging, candidate, maxRadius) { - if (dragging.distanceFrom(candidate) > maxRadius) { - return false; - } - - return this.isConnectionAllowed(dragging, candidate); -}; /** * Check if the two connections can be dragged to connect to each other. * This is used by the connection database when searching for the closest diff --git a/core/connection_db.js b/core/connection_db.js index 5a8283f12..ff31704a2 100644 --- a/core/connection_db.js +++ b/core/connection_db.js @@ -238,15 +238,17 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, var bestConnection = null; var bestRadius = maxRadius; var temp; + var curDistance; // Walk forward and back on the y axis looking for the closest x,y point. var pointerMin = closestIndex - 1; while (pointerMin >= 0 && this.isInYRange_(pointerMin, conn.y, maxRadius)) { temp = this.connections_[pointerMin]; - if (this.typeChecker_.canConnectDuringDrag(conn, temp, bestRadius)) { - //conn.isConnectionAllowed(temp, bestRadius)) { + curDistance = temp.distanceFrom(conn); + if (curDistance <= bestRadius && + this.typeChecker_.canConnect(conn, temp, true, false)) { bestConnection = temp; - bestRadius = temp.distanceFrom(conn); + bestRadius = curDistance; } pointerMin--; } @@ -255,10 +257,11 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, while (pointerMax < this.connections_.length && this.isInYRange_(pointerMax, conn.y, maxRadius)) { temp = this.connections_[pointerMax]; - if (this.typeChecker_.canConnectDuringDrag(conn, temp, bestRadius)) { - //conn.isConnectionAllowed(temp, bestRadius)) { + curDistance = temp.distanceFrom(conn); + if (curDistance <= bestRadius && + this.typeChecker_.canConnect(conn, temp, true, false)) { bestConnection = temp; - bestRadius = temp.distanceFrom(conn); + bestRadius = curDistance; } pointerMax++; } diff --git a/core/keyboard_nav/navigation.js b/core/keyboard_nav/navigation.js index 9a59ee5c3..5a06690dc 100644 --- a/core/keyboard_nav/navigation.js +++ b/core/keyboard_nav/navigation.js @@ -411,9 +411,9 @@ Blockly.navigation.moveAndConnect_ = function(movingConnection, destConnection) } var movingBlock = movingConnection.getSourceBlock(); - if (destConnection.canConnectWithReason(movingConnection) == - Blockly.Connection.CAN_CONNECT) { + var checker = movingConnection.getConnectionTypeChecker(); + if (checker.canConnect(movingConnection, destConnection, false, false)) { Blockly.navigation.disconnectChild_(movingConnection, destConnection); if (!destConnection.isSuperior()) { @@ -502,10 +502,12 @@ Blockly.navigation.connect_ = function(movingConnection, destConnection) { return true; } else { try { - destConnection.checkConnection(movingConnection); + var checker = movingConnection.getConnectionTypeChecker(); + checker.canConnect(movingConnection, destConnection, false, true); } catch (e) { - // If nothing worked report the error from the original connections. + // If nothing worked there should be an error. + // Report the error from the original connections. Blockly.navigation.warn_('Connection failed with error: ' + e); } return false; diff --git a/tests/mocha/connection_db_test.js b/tests/mocha/connection_db_test.js index 34165831e..52f926391 100644 --- a/tests/mocha/connection_db_test.js +++ b/tests/mocha/connection_db_test.js @@ -194,26 +194,13 @@ suite('Connection Database', function() { suite('Search For Closest', function() { setup(function() { this.allowedStub = null; - this.database.typeChecker_.canConnectDuringDrag = function( - dragging, candidate, maxRadius) { - if (dragging.distanceFrom(candidate) > maxRadius) { - return false; - } - // Ignore non-distance parameters. - return true; - }; - + this.allowedStub = sinon.stub(this.database.typeChecker_, 'canConnect') + .callsFake(function(dragging, candidate) { + return true; + }); this.createCheckConnection = function(x, y) { var checkConnection = this.createConnection(x, y, Blockly.NEXT_STATEMENT, new Blockly.ConnectionDB()); - // this.allowedStub = sinon.stub(checkConnection, 'isConnectionAllowed') - // .callsFake(function(candidate, maxRadius) { - // if (this.distanceFrom(candidate) > maxRadius) { - // return false; - // } - // // Ignore non-distance parameters. - // return true; - // }); return checkConnection; }; }); diff --git a/tests/mocha/index.html b/tests/mocha/index.html index bc236ad93..9c81f3889 100644 --- a/tests/mocha/index.html +++ b/tests/mocha/index.html @@ -41,7 +41,6 @@ - From 450aed0aa2e5462ff68f9a93f4e2cf26acb2cb29 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Tue, 14 Jul 2020 13:49:58 -0600 Subject: [PATCH 04/18] More work on connection type checks --- core/block.js | 7 +- core/connection.js | 94 ++++++--------- core/connection_checks.js | 187 ++++++++++++++++++------------ core/rendered_connection.js | 4 +- core/renderers/common/renderer.js | 3 +- tests/mocha/connection_test.js | 7 +- 6 files changed, 163 insertions(+), 139 deletions(-) diff --git a/core/block.js b/core/block.js index 4d5612de2..56710b7a3 100644 --- a/core/block.js +++ b/core/block.js @@ -456,7 +456,8 @@ Blockly.Block.prototype.unplugFromRow_ = function(opt_healStack) { // Disconnect the child block. childConnection.disconnect(); // Connect child to the parent if possible, otherwise bump away. - if (childConnection.checkType(parentConnection)) { + if (this.workspace.connectionTypeChecker.canConnect( + childConnection, parentConnection, false, false)) { parentConnection.connect(childConnection); } else { childConnection.onFailedConnect(parentConnection); @@ -508,7 +509,9 @@ Blockly.Block.prototype.unplugFromStack_ = function(opt_healStack) { // Disconnect the next statement. var nextTarget = this.nextConnection.targetConnection; nextTarget.disconnect(); - if (previousTarget && previousTarget.checkType(nextTarget)) { + if (previousTarget && + this.workspace.connectionTypeChecker.canConnect( + previousTarget, nextTarget, false, false)) { // Attach the next statement to the previous statement. previousTarget.connect(nextTarget); } diff --git a/core/connection.js b/core/connection.js index a3597de03..9098f63a5 100644 --- a/core/connection.js +++ b/core/connection.js @@ -147,10 +147,10 @@ Blockly.Connection.prototype.connect_ = function(childConnection) { if (nextBlock && !nextBlock.isShadow()) { newBlock = nextBlock; } else { - if (orphanBlock.workspace.connectionTypeChecker.checkType( - orphanBlock.previousConnection, newBlock.nextConnection)) { - //orphanBlock.previousConnection.checkType( - //newBlock.nextConnection)) { + var typeChecker = orphanBlock.workspace.connectionTypeChecker; + if (typeChecker.canConnect( + orphanBlock.previousConnection, newBlock.nextConnection, + false, false)) { newBlock.nextConnection.connect(orphanBlock.previousConnection); orphanBlock = null; } @@ -249,6 +249,7 @@ Blockly.Connection.prototype.isConnected = function() { * @param {Blockly.Connection} target Connection to check compatibility with. * @return {number} Blockly.Connection.CAN_CONNECT if the connection is legal, * an error code otherwise. + * @deprecated July 2020 */ Blockly.Connection.prototype.canConnectWithReason = function(target) { // TODO: deprecation warning with date, plus tests. @@ -261,6 +262,7 @@ Blockly.Connection.prototype.canConnectWithReason = function(target) { * @param {Blockly.Connection} target The connection to check compatibility * with. * @package + * @deprecated July 2020 */ Blockly.Connection.prototype.checkConnection = function(target) { // TODO: Add deprecation warning notices *and* add tests to make sure these @@ -269,50 +271,21 @@ Blockly.Connection.prototype.checkConnection = function(target) { checker.canConnect(this, target, false, true); }; +/** + * Get the workspace's connection type checker object. + * @return {!Blockly.ConnectionTypeChecker} The connection type checker for the + * source block's workspace. + * @package + */ Blockly.Connection.prototype.getConnectionTypeChecker = function() { return this.sourceBlock_.workspace.connectionTypeChecker; }; -// /** -// * Check if the two connections can be dragged to connect to each other. -// * This is used by the connection database when searching for the closest -// * connection. -// * @param {!Blockly.Connection} candidate A nearby connection to check, which -// * must be a previous connection. -// * @return {boolean} True if the connection is allowed, false otherwise. -// * @private -// */ -// Blockly.Connection.prototype.canConnectToPrevious_ = function(candidate) { -// if (this.targetConnection) { -// // This connection is already occupied. -// // A next connection will never disconnect itself mid-drag. -// return false; -// } - -// // Don't let blocks try to connect to themselves or ones they nest. -// if (Blockly.draggingConnections.indexOf(candidate) != -1) { -// return false; -// } - -// if (!candidate.targetConnection) { -// return true; -// } - -// var targetBlock = candidate.targetBlock(); -// // If it is connected to a real block, game over. -// if (!targetBlock.isInsertionMarker()) { -// return false; -// } -// // If it's connected to an insertion marker but that insertion marker -// // is the first block in a stack, it's still fine. If that insertion -// // marker is in the middle of a stack, it won't work. -// return !targetBlock.getPreviousBlock(); -// }; - /** * Check if the two connections can be dragged to connect to each other. * @param {!Blockly.Connection} candidate A nearby connection to check. * @return {boolean} True if the connection is allowed, false otherwise. + * @deprecated July 2020 */ Blockly.Connection.prototype.isConnectionAllowed = function(candidate) { return this.getConnectionTypeChecker().canConnect(this, candidate, true, false); @@ -339,22 +312,22 @@ Blockly.Connection.prototype.connect = function(otherConnection) { } var checker = this.getConnectionTypeChecker(); - checker.canConnect(this, otherConnection, false, true); - - var eventGroup = Blockly.Events.getGroup(); - if (!eventGroup) { - Blockly.Events.setGroup(true); - } - // Determine which block is superior (higher in the source stack). - if (this.isSuperior()) { - // Superior block. - this.connect_(otherConnection); - } else { - // Inferior block. - otherConnection.connect_(this); - } - if (!eventGroup) { - Blockly.Events.setGroup(false); + if (checker.canConnect(this, otherConnection, false, true)) { + var eventGroup = Blockly.Events.getGroup(); + if (!eventGroup) { + Blockly.Events.setGroup(true); + } + // Determine which block is superior (higher in the source stack). + if (this.isSuperior()) { + // Superior block. + this.connect_(otherConnection); + } else { + // Inferior block. + otherConnection.connect_(this); + } + if (!eventGroup) { + Blockly.Events.setGroup(false); + } } }; @@ -383,10 +356,12 @@ Blockly.Connection.connectReciprocally_ = function(first, second) { */ Blockly.Connection.singleConnection_ = function(block, orphanBlock) { var connection = null; + var output = orphanBlock.outputConnection; for (var i = 0; i < block.inputList.length; i++) { var thisConnection = block.inputList[i].connection; + var typeChecker = output.getConnectionTypeChecker(); if (thisConnection && thisConnection.type == Blockly.INPUT_VALUE && - orphanBlock.outputConnection.checkType(thisConnection)) { + typeChecker.canConnect(output, thisConnection, false, false)) { if (connection) { return null; // More than one connection. } @@ -516,7 +491,7 @@ Blockly.Connection.prototype.targetBlock = function() { * @return {boolean} True if the connections share a type. */ Blockly.Connection.prototype.checkType = function(otherConnection) { - return this.getConnectionTypeChecker().checkType(this, otherConnection); + return this.getConnectionTypeChecker().canConnect(this, otherConnection, false, false); }; /** @@ -541,7 +516,8 @@ Blockly.Connection.prototype.checkType_ = function(otherConnection) { Blockly.Connection.prototype.onCheckChanged_ = function() { // The new value type may not be compatible with the existing connection. if (this.isConnected() && (!this.targetConnection || - !this.checkType(this.targetConnection))) { + !this.getConnectionTypeChecker().canConnect( + this, this.targetConnection, false, false))) { var child = this.isSuperior() ? this.targetBlock() : this.sourceBlock_; child.unplug(); } diff --git a/core/connection_checks.js b/core/connection_checks.js index 7e51f62cb..3d703b85a 100644 --- a/core/connection_checks.js +++ b/core/connection_checks.js @@ -11,6 +11,94 @@ goog.requireType('Blockly.Connection'); Blockly.ConnectionTypeChecker = function() { }; +/** + * Check whether the current connection can connect with the target + * connection. + * @param {Blockly.Connection} one Connection to check compatibility with. + * @param {Blockly.Connection} two Connection to check compatibility with. + * @param {boolean} isDragging True if the connection is being made by dragging + * a block. + * @param {boolean} shouldThrow Whether to throw an error when a connection is + * invalid. + * @return {boolean} Whether the connection is legal. + */ +Blockly.ConnectionTypeChecker.prototype.canConnect = function(one, two, + isDragging, shouldThrow) { + if (this.passesSafetyChecks(one, two, shouldThrow)) { + if (this.passesTypeChecks(one, two, shouldThrow)) { + if (!isDragging || this.passesDragChecks(one, two, shouldThrow)) { + return true; + } + } + } + return false; +}; + +/** + * Check that connecting the given connections is safe, meaning that it would + * not break any of Blockly's basic assumptions--no self connections, etc. + * @param {!Blockly.Connection} one The first of the connections to check. + * @param {!Blockly.Connection} two The second of the connections to check. + * @param {boolean} shouldThrow Whether to throw an error if the connection is + * unsafe. + * @return {boolean} Whether the connection is safe. + * @package + */ +Blockly.ConnectionTypeChecker.prototype.passesSafetyChecks = function(one, two, shouldThrow) { + var safety = this.doSafetyChecks_(one, two); + if (safety == Blockly.Connection.CAN_CONNECT) { + return true; + } + if (shouldThrow) { + throw Error(this.getErrorMessage_(safety, one, two)); + } + return false; +}; + + +Blockly.ConnectionTypeChecker.prototype.passesTypeChecks = function(one, two, + shouldThrow) { + if (this.doTypeChecks_(one, two)) { + return true; + } + if (shouldThrow) { + throw Error(this.getErrorMessage_( + Blockly.Connection.REASON_CHECKS_FAILED, one, two)); + } + return false; +}; + +Blockly.ConnectionTypeChecker.prototype.passesDragChecks = function(one, two, + shouldThrow) { + if (this.doDragChecks_(one, two)) { + return true; + } + if (shouldThrow) { + throw Error(this.getErrorMessage_( + Blockly.Connection.REASON_DRAG_CHECKS_FAILED, one, two)); + } + return false; +}; + +/** + * Checks whether the current connection can connect with the target + * connection. + * @param {Blockly.Connection} one Connection to check compatibility with. + * @param {Blockly.Connection} two Connection to check compatibility with. + * @return {number} Blockly.Connection.CAN_CONNECT if the connection is legal, + * an error code otherwise. + */ +Blockly.ConnectionTypeChecker.prototype.canConnectWithReason = function(one, two) { + var safety = this.doSafetyChecks_(one, two); + if (safety != Blockly.Connection.CAN_CONNECT) { + return safety; + } + if (!this.doTypeChecks_(one, two)) { + return Blockly.Connection.REASON_CHECKS_FAILED; + } + return Blockly.Connection.CAN_CONNECT; +}; + /** * Helper method that translates a connection error code into a string. * @param {number} errorCode The error code. @@ -18,9 +106,9 @@ Blockly.ConnectionTypeChecker = function() { * @param {!Blockly.Connection} two The second of the two connections being * checked. * @return {string} A developer-readable error string. - * @package + * @private */ -Blockly.ConnectionTypeChecker.prototype.getErrorMessage = function(errorCode, +Blockly.ConnectionTypeChecker.prototype.getErrorMessage_ = function(errorCode, one, two) { switch (errorCode) { case Blockly.Connection.REASON_SELF_CONNECTION: @@ -39,32 +127,21 @@ Blockly.ConnectionTypeChecker.prototype.getErrorMessage = function(errorCode, case Blockly.Connection.REASON_SHADOW_PARENT: return 'Connecting non-shadow to shadow block.'; case Blockly.Connection.REASON_DRAG_CHECKS_FAILED: - return 'Drag checks failed.' + return 'Drag checks failed.'; default: return 'Unknown connection failure: this should never happen!'; } }; /** - * Checks whether the current connection can connect with the target - * connection. - * @param {Blockly.Connection} one Connection to check compatibility with. - * @param {Blockly.Connection} two Connection to check compatibility with. - * @return {number} Blockly.Connection.CAN_CONNECT if the connection is legal, - * an error code otherwise. + * Check that connecting the given connections is safe, meaning that it would + * not break any of Blockly's basic assumptions--no self connections, etc. + * @param {!Blockly.Connection} one The first of the connections to check. + * @param {!Blockly.Connection} two The second of the connections to check. + * @return {boolean} True if making this connection is safe. + * @private */ -Blockly.ConnectionTypeChecker.prototype.canConnectWithReason = function(one, two) { - var validity = this.doValidityChecks(one, two); - if (validity != Blockly.Connection.CAN_CONNECT) { - return validity; - } - if (!this.checkType(one, two)) { - return Blockly.Connection.REASON_CHECKS_FAILED; - } - return Blockly.Connection.CAN_CONNECT; -}; - -Blockly.ConnectionTypeChecker.prototype.doValidityChecks = function(one, two) { +Blockly.ConnectionTypeChecker.prototype.doSafetyChecks_ = function(one, two) { if (!one || !two) { return Blockly.Connection.REASON_TARGET_NULL; } @@ -75,8 +152,6 @@ Blockly.ConnectionTypeChecker.prototype.doValidityChecks = function(one, two) { var blockB = one.getSourceBlock(); var blockA = two.getSourceBlock(); } - // TODO (fenichel): The null checks seem like they're only for making tests - // work better. if (blockA == blockB) { return Blockly.Connection.REASON_SELF_CONNECTION; } else if (two.type != Blockly.OPPOSITE_TYPE[one.type]) { @@ -90,53 +165,15 @@ Blockly.ConnectionTypeChecker.prototype.doValidityChecks = function(one, two) { }; /** - * Checks whether the current connection can connect with the target - * connection. - * @param {Blockly.Connection} one Connection to check compatibility with. - * @param {Blockly.Connection} two Connection to check compatibility with. - * @return {boolean} Whether the connection is legal. - */ -Blockly.ConnectionTypeChecker.prototype.canConnect = function(one, two, - isDragging, shouldThrow) { - var validity = this.doValidityChecks(one, two); - if (validity != Blockly.Connection.CAN_CONNECT) { - if (shouldThrow) { - throw Error(this.getErrorMessage(validity, one, two)); - } - return false; - } - - var passesTypeChecks = this.checkType(one, two); - if (!passesTypeChecks) { - - if (shouldThrow) { - throw Error(this.getErrorMessage( - Blockly.Connection.REASON_CHECKS_FAILED, one, two)); - } - return false; - } - - if (isDragging) { - var passesDragChecks = this.passesDragChecks(one, two); - if (!passesDragChecks) { - if (shouldThrow) { - throw Error(this.getErrorMessage( - Blockly.Connection.REASON_DRAG_CHECKS_FAILED, one, two)); - } - return false; - } - } - return true; -}; - -/** - * Is this connection compatible with another connection with respect to the - * value type system. E.g. square_root("Hello") is not compatible. + * Check whether this connection is compatible with another connection with + * respect to the value type system. E.g. square_root("Hello") is not + * compatible. * @param {!Blockly.Connection} one Connection to compare. * @param {!Blockly.Connection} two Connection to compare against. * @return {boolean} True if the connections share a type. + * @protected */ -Blockly.ConnectionTypeChecker.prototype.checkType = function(one, two) { +Blockly.ConnectionTypeChecker.prototype.doTypeChecks_ = function(one, two) { var checkArrayOne = one.getCheck(); var checkArrayTwo = two.getCheck(); @@ -154,7 +191,14 @@ Blockly.ConnectionTypeChecker.prototype.checkType = function(one, two) { return false; }; -Blockly.ConnectionTypeChecker.prototype.passesDragChecks = function(one, two) { +/** + * Check whether this connection can be made by dragging. + * @param {!Blockly.Connection} one Connection to compare. + * @param {!Blockly.Connection} two Connection to compare against. + * @return {boolean} True if the connections share a type. + * @protected + */ +Blockly.ConnectionTypeChecker.prototype.doDragChecks_ = function(one, two) { // Don't consider insertion markers. if (two.sourceBlock_.isInsertionMarker()) { return false; @@ -198,7 +242,8 @@ Blockly.ConnectionTypeChecker.prototype.passesDragChecks = function(one, two) { break; } default: - throw Error('Unknown connection type in passesDragChecks'); + // Unexpected connection type. + return false; } // Don't let blocks try to connect to themselves or ones they nest. @@ -210,15 +255,13 @@ Blockly.ConnectionTypeChecker.prototype.passesDragChecks = function(one, two) { }; /** - * Check if the two connections can be dragged to connect to each other. - * This is used by the connection database when searching for the closest - * connection. + * Helper function for drag checking * @param {!Blockly.Connection} one The connection to check, which must be a * statement input or next connection. * @param {!Blockly.Connection} two A nearby connection to check, which * must be a previous connection. * @return {boolean} True if the connection is allowed, false otherwise. - * @private + * @protected */ Blockly.ConnectionTypeChecker.prototype.canConnectToPrevious_ = function(one, two) { if (one.targetConnection) { diff --git a/core/rendered_connection.js b/core/rendered_connection.js index a2bd877e2..ea8e2868e 100644 --- a/core/rendered_connection.js +++ b/core/rendered_connection.js @@ -422,6 +422,7 @@ Blockly.RenderedConnection.prototype.startTrackingAll = function() { * @param {number=} maxRadius The maximum radius allowed for connections, in * workspace units. * @return {boolean} True if the connection is allowed, false otherwise. + * @deprecated July 2020 */ Blockly.RenderedConnection.prototype.isConnectionAllowed = function(candidate, maxRadius) { @@ -551,7 +552,8 @@ Blockly.RenderedConnection.prototype.connect_ = function(childConnection) { Blockly.RenderedConnection.prototype.onCheckChanged_ = function() { // The new value type may not be compatible with the existing connection. if (this.isConnected() && (!this.targetConnection || - !this.checkType(this.targetConnection))) { + !this.getConnectionTypeChecker().canConnect( + this, this.targetConnection, false, false))) { var child = this.isSuperior() ? this.targetBlock() : this.sourceBlock_; child.unplug(); // Bump away. diff --git a/core/renderers/common/renderer.js b/core/renderers/common/renderer.js index 05885d18c..f0b158c0e 100644 --- a/core/renderers/common/renderer.js +++ b/core/renderers/common/renderer.js @@ -254,7 +254,8 @@ Blockly.blockRendering.Renderer.prototype.orphanCanConnectAtEnd = if (!lastConnection) { return false; } - return orphanConnection.checkType(lastConnection); + return orphanConnection.getConnectionTypeChecker().canConnect( + lastConnection, orphanConnection, false, false); }; /** diff --git a/tests/mocha/connection_test.js b/tests/mocha/connection_test.js index a4ac148d7..dd8627943 100644 --- a/tests/mocha/connection_test.js +++ b/tests/mocha/connection_test.js @@ -229,13 +229,12 @@ suite('Connection type checker', function() { this.con2 = new Blockly.Connection({}, Blockly.NEXT_STATEMENT); }); function assertCheckTypes(checker, one, two) { - chai.assert.isTrue(checker.checkType(one, two)); + chai.assert.isTrue(checker.passesTypeChecks(one, two)); // Order should not matter. - chai.assert.isTrue(checker.checkType(one, two)); + chai.assert.isTrue(checker.passesTypeChecks(one, two)); } test('No Types', function() { assertCheckTypes(this.checker, this.con1, this.con2); - chai.assert.isTrue(this.checker.checkType(this.con1, this.con2)); }); test('Same Type', function() { this.con1.setCheck('type1'); @@ -259,7 +258,7 @@ suite('Connection type checker', function() { test('No Compatible Types', function() { this.con1.setCheck('type1'); this.con2.setCheck('type2'); - chai.assert.isFalse(this.checker.checkType(this.con1, this.con2)); + chai.assert.isFalse(this.checker.passesTypeChecks(this.con1, this.con2)); }); }); }); From 01d33ce2e71e41c0d7b017b27b068ae9bbd84971 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Tue, 14 Jul 2020 14:08:05 -0600 Subject: [PATCH 05/18] Fix type checks --- core/connection_checks.js | 33 +++++++++++++++++++-------------- core/workspace.js | 2 +- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/core/connection_checks.js b/core/connection_checks.js index 3d703b85a..1b456ddbd 100644 --- a/core/connection_checks.js +++ b/core/connection_checks.js @@ -25,8 +25,10 @@ Blockly.ConnectionTypeChecker = function() { Blockly.ConnectionTypeChecker.prototype.canConnect = function(one, two, isDragging, shouldThrow) { if (this.passesSafetyChecks(one, two, shouldThrow)) { - if (this.passesTypeChecks(one, two, shouldThrow)) { - if (!isDragging || this.passesDragChecks(one, two, shouldThrow)) { + var connOne = /** @type {!Blockly.Connection} **/ (one); + var connTwo = /** @type {!Blockly.Connection} **/ (two); + if (this.passesTypeChecks(connOne, connTwo, shouldThrow)) { + if (!isDragging || this.passesDragChecks(connOne, connTwo, shouldThrow)) { return true; } } @@ -37,8 +39,8 @@ Blockly.ConnectionTypeChecker.prototype.canConnect = function(one, two, /** * Check that connecting the given connections is safe, meaning that it would * not break any of Blockly's basic assumptions--no self connections, etc. - * @param {!Blockly.Connection} one The first of the connections to check. - * @param {!Blockly.Connection} two The second of the connections to check. + * @param {Blockly.Connection} one The first of the connections to check. + * @param {Blockly.Connection} two The second of the connections to check. * @param {boolean} shouldThrow Whether to throw an error if the connection is * unsafe. * @return {boolean} Whether the connection is safe. @@ -93,7 +95,10 @@ Blockly.ConnectionTypeChecker.prototype.canConnectWithReason = function(one, two if (safety != Blockly.Connection.CAN_CONNECT) { return safety; } - if (!this.doTypeChecks_(one, two)) { + + var connOne = /** @type {!Blockly.Connection} **/ (one); + var connTwo = /** @type {!Blockly.Connection} **/ (two); + if (!this.doTypeChecks_(connOne, connTwo)) { return Blockly.Connection.REASON_CHECKS_FAILED; } return Blockly.Connection.CAN_CONNECT; @@ -102,8 +107,8 @@ Blockly.ConnectionTypeChecker.prototype.canConnectWithReason = function(one, two /** * Helper method that translates a connection error code into a string. * @param {number} errorCode The error code. - * @param {!Blockly.Connection} one One of the two connections being checked. - * @param {!Blockly.Connection} two The second of the two connections being + * @param {Blockly.Connection} one One of the two connections being checked. + * @param {Blockly.Connection} two The second of the two connections being * checked. * @return {string} A developer-readable error string. * @private @@ -122,7 +127,7 @@ Blockly.ConnectionTypeChecker.prototype.getErrorMessage_ = function(errorCode, return 'Target connection is null.'; case Blockly.Connection.REASON_CHECKS_FAILED: var msg = 'Connection checks failed. '; - msg += one + ' expected ' + one.check_ + ', found ' + two.check_; + msg += one + ' expected ' + one.getCheck() + ', found ' + two.getCheck(); return msg; case Blockly.Connection.REASON_SHADOW_PARENT: return 'Connecting non-shadow to shadow block.'; @@ -136,9 +141,9 @@ Blockly.ConnectionTypeChecker.prototype.getErrorMessage_ = function(errorCode, /** * Check that connecting the given connections is safe, meaning that it would * not break any of Blockly's basic assumptions--no self connections, etc. - * @param {!Blockly.Connection} one The first of the connections to check. - * @param {!Blockly.Connection} two The second of the connections to check. - * @return {boolean} True if making this connection is safe. + * @param {Blockly.Connection} one The first of the connections to check. + * @param {Blockly.Connection} two The second of the connections to check. + * @return {number} An enum with the reason this connection is safe or unsafe. * @private */ Blockly.ConnectionTypeChecker.prototype.doSafetyChecks_ = function(one, two) { @@ -200,13 +205,13 @@ Blockly.ConnectionTypeChecker.prototype.doTypeChecks_ = function(one, two) { */ Blockly.ConnectionTypeChecker.prototype.doDragChecks_ = function(one, two) { // Don't consider insertion markers. - if (two.sourceBlock_.isInsertionMarker()) { + if (two.getSourceBlock().isInsertionMarker()) { return false; } switch (two.type) { case Blockly.PREVIOUS_STATEMENT: - return one.canConnectToPrevious_(two); + return this.canConnectToPrevious_(one, two); case Blockly.OUTPUT_VALUE: { // Don't offer to connect an already connected left (male) value plug to // an available right (female) value plug. @@ -234,7 +239,7 @@ Blockly.ConnectionTypeChecker.prototype.doDragChecks_ = function(one, two) { // fine. Similarly, replacing a terminal statement with another terminal // statement is allowed. if (two.isConnected() && - !one.sourceBlock_.nextConnection && + !one.getSourceBlock().nextConnection && !two.targetBlock().isShadow() && two.targetBlock().nextConnection) { return false; diff --git a/core/workspace.js b/core/workspace.js index b92750d3f..9275f7a32 100644 --- a/core/workspace.js +++ b/core/workspace.js @@ -104,7 +104,7 @@ Blockly.Workspace = function(opt_options) { * @private */ this.potentialVariableMap_ = null; - /** @type {Blockly.ConnectionTypeChecker} [description] */ + /** @type {!Blockly.ConnectionTypeChecker} [description] */ this.connectionTypeChecker = new Blockly.ConnectionTypeChecker(); }; From e3c8d834cb7af8b08154ca5ecb0772655edb295a Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Tue, 14 Jul 2020 14:56:34 -0600 Subject: [PATCH 06/18] Add todos and rebuild --- blockly_uncompressed.js | 3 ++- core/connection.js | 1 + core/keyboard_nav/navigation.js | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/blockly_uncompressed.js b/blockly_uncompressed.js index c37ffb66a..111ba6fd6 100644 --- a/blockly_uncompressed.js +++ b/blockly_uncompressed.js @@ -37,6 +37,7 @@ goog.addDependency('../../core/components/tree/basenode.js', ['Blockly.tree.Base goog.addDependency('../../core/components/tree/treecontrol.js', ['Blockly.tree.TreeControl'], ['Blockly.tree.BaseNode', 'Blockly.tree.TreeNode', 'Blockly.utils.aria', 'Blockly.utils.object', 'Blockly.utils.style'], {}); goog.addDependency('../../core/components/tree/treenode.js', ['Blockly.tree.TreeNode'], ['Blockly.tree.BaseNode', 'Blockly.utils.KeyCodes', 'Blockly.utils.object'], {}); goog.addDependency('../../core/connection.js', ['Blockly.Connection'], ['Blockly.Events', 'Blockly.Events.BlockMove', 'Blockly.Xml'], {}); +goog.addDependency('../../core/connection_checks.js', ['Blockly.ConnectionTypeChecker'], [], {}); goog.addDependency('../../core/connection_db.js', ['Blockly.ConnectionDB'], ['Blockly.RenderedConnection'], {}); goog.addDependency('../../core/constants.js', ['Blockly.constants'], [], {}); goog.addDependency('../../core/contextmenu.js', ['Blockly.ContextMenu'], ['Blockly.Events', 'Blockly.Events.BlockCreate', 'Blockly.Menu', 'Blockly.MenuItem', 'Blockly.Msg', 'Blockly.Xml', 'Blockly.utils', 'Blockly.utils.Coordinate', 'Blockly.utils.Rect', 'Blockly.utils.dom', 'Blockly.utils.userAgent'], {}); @@ -183,7 +184,7 @@ goog.addDependency('../../core/variables.js', ['Blockly.Variables'], ['Blockly.B goog.addDependency('../../core/variables_dynamic.js', ['Blockly.VariablesDynamic'], ['Blockly.Blocks', 'Blockly.Msg', 'Blockly.VariableModel', 'Blockly.Variables', 'Blockly.utils.xml'], {}); goog.addDependency('../../core/warning.js', ['Blockly.Warning'], ['Blockly.Bubble', 'Blockly.Events', 'Blockly.Events.Ui', 'Blockly.Icon', 'Blockly.utils.dom', 'Blockly.utils.object'], {}); goog.addDependency('../../core/widgetdiv.js', ['Blockly.WidgetDiv'], ['Blockly.utils.style'], {}); -goog.addDependency('../../core/workspace.js', ['Blockly.Workspace'], ['Blockly.Events', 'Blockly.Options', 'Blockly.VariableMap', 'Blockly.utils', 'Blockly.utils.math'], {}); +goog.addDependency('../../core/workspace.js', ['Blockly.Workspace'], ['Blockly.ConnectionTypeChecker', 'Blockly.Events', 'Blockly.Options', 'Blockly.VariableMap', 'Blockly.utils', 'Blockly.utils.math'], {}); goog.addDependency('../../core/workspace_audio.js', ['Blockly.WorkspaceAudio'], ['Blockly.utils', 'Blockly.utils.global', 'Blockly.utils.userAgent'], {'lang': 'es5'}); goog.addDependency('../../core/workspace_comment.js', ['Blockly.WorkspaceComment'], ['Blockly.Events', 'Blockly.Events.CommentChange', 'Blockly.Events.CommentCreate', 'Blockly.Events.CommentDelete', 'Blockly.Events.CommentMove', 'Blockly.utils', 'Blockly.utils.Coordinate', 'Blockly.utils.xml'], {}); goog.addDependency('../../core/workspace_comment_render_svg.js', ['Blockly.WorkspaceCommentSvg.render'], ['Blockly.utils', 'Blockly.utils.Coordinate', 'Blockly.utils.dom'], {}); diff --git a/core/connection.js b/core/connection.js index 9098f63a5..d989611ac 100644 --- a/core/connection.js +++ b/core/connection.js @@ -312,6 +312,7 @@ Blockly.Connection.prototype.connect = function(otherConnection) { } var checker = this.getConnectionTypeChecker(); + // TODO (fenichel): Try to get rid of the extra parameter (shouldThrow). if (checker.canConnect(this, otherConnection, false, true)) { var eventGroup = Blockly.Events.getGroup(); if (!eventGroup) { diff --git a/core/keyboard_nav/navigation.js b/core/keyboard_nav/navigation.js index 5a06690dc..985a854bf 100644 --- a/core/keyboard_nav/navigation.js +++ b/core/keyboard_nav/navigation.js @@ -502,6 +502,8 @@ Blockly.navigation.connect_ = function(movingConnection, destConnection) { return true; } else { try { + // TODO (fenichel): Don't rely on canConnect throwing an error. + // TODO (fenichel): Fix associated tests. var checker = movingConnection.getConnectionTypeChecker(); checker.canConnect(movingConnection, destConnection, false, true); } From e24f3cef9bcb89bad4fb3a0deb59118b558140cb Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Tue, 14 Jul 2020 16:57:29 -0600 Subject: [PATCH 07/18] Add isDragging to canConnectWithReason and delete newly unused code paths. --- blocks/logic.js | 10 ++-- core/block.js | 5 +- core/connection.js | 23 ++++---- core/connection_checks.js | 93 ++++++++----------------------- core/connection_db.js | 10 +++- core/keyboard_nav/navigation.js | 18 ++---- core/rendered_connection.js | 2 +- core/renderers/common/renderer.js | 2 +- tests/mocha/connection_test.js | 8 +-- 9 files changed, 63 insertions(+), 108 deletions(-) diff --git a/blocks/logic.js b/blocks/logic.js index 6cea9d1d4..bc46c82d6 100644 --- a/blocks/logic.js +++ b/blocks/logic.js @@ -543,9 +543,8 @@ Blockly.Constants.Logic.LOGIC_COMPARE_ONCHANGE_MIXIN = { var blockB = this.getInputTargetBlock('B'); // Disconnect blocks that existed prior to this change if they don't match. if (blockA && blockB && - //!this.workspace.connectionTypeChecker.checkType( - //blockA.outputConnection, blockB.outputConnection)) { - !blockA.outputConnection.checkType(blockB.outputConnection)) { + !this.workspace.connectionTypeChecker.doTypeChecks( + blockA.outputConnection, blockB.outputConnection)) { // Mismatch between two inputs. Revert the block connections, // bumping away the newly connected block(s). Blockly.Events.setGroup(e.group); @@ -613,9 +612,8 @@ Blockly.Constants.Logic.LOGIC_TERNARY_ONCHANGE_MIXIN = { for (var i = 0; i < 2; i++) { var block = (i == 1) ? blockA : blockB; if (block && - // !block.workspace.connectionTypeChecker.checkType( - // block.outputConnection, parentConnection)) { - !block.outputConnection.checkType(parentConnection)) { + !block.workspace.connectionTypeChecker.doTypeChecks( + block.outputConnection, parentConnection)) { // Ensure that any disconnections are grouped with the causing event. Blockly.Events.setGroup(e.group); if (parentConnection === this.prevParentConnection_) { diff --git a/core/block.js b/core/block.js index 56710b7a3..9aabe7edb 100644 --- a/core/block.js +++ b/core/block.js @@ -32,6 +32,7 @@ goog.require('Blockly.utils.string'); goog.require('Blockly.Workspace'); goog.requireType('Blockly.IASTNodeLocation'); +goog.requireType('Blockly.connectionTypeChecker'); /** @@ -457,7 +458,7 @@ Blockly.Block.prototype.unplugFromRow_ = function(opt_healStack) { childConnection.disconnect(); // Connect child to the parent if possible, otherwise bump away. if (this.workspace.connectionTypeChecker.canConnect( - childConnection, parentConnection, false, false)) { + childConnection, parentConnection, false)) { parentConnection.connect(childConnection); } else { childConnection.onFailedConnect(parentConnection); @@ -511,7 +512,7 @@ Blockly.Block.prototype.unplugFromStack_ = function(opt_healStack) { nextTarget.disconnect(); if (previousTarget && this.workspace.connectionTypeChecker.canConnect( - previousTarget, nextTarget, false, false)) { + previousTarget, nextTarget, false)) { // Attach the next statement to the previous statement. previousTarget.connect(nextTarget); } diff --git a/core/connection.js b/core/connection.js index d989611ac..92e578d11 100644 --- a/core/connection.js +++ b/core/connection.js @@ -149,8 +149,7 @@ Blockly.Connection.prototype.connect_ = function(childConnection) { } else { var typeChecker = orphanBlock.workspace.connectionTypeChecker; if (typeChecker.canConnect( - orphanBlock.previousConnection, newBlock.nextConnection, - false, false)) { + orphanBlock.previousConnection, newBlock.nextConnection, false)) { newBlock.nextConnection.connect(orphanBlock.previousConnection); orphanBlock = null; } @@ -253,7 +252,8 @@ Blockly.Connection.prototype.isConnected = function() { */ Blockly.Connection.prototype.canConnectWithReason = function(target) { // TODO: deprecation warning with date, plus tests. - return this.getConnectionTypeChecker().canConnectWithReason(this, target); + return this.getConnectionTypeChecker().canConnectWithReason( + this, target); }; /** @@ -268,7 +268,10 @@ Blockly.Connection.prototype.checkConnection = function(target) { // TODO: Add deprecation warning notices *and* add tests to make sure these // still work (for any blocks that use them). var checker = this.getConnectionTypeChecker(); - checker.canConnect(this, target, false, true); + var reason = !checker.canConnectWithReason(this, target, false); + if (reason != Blockly.Connection.CAN_CONNECT) { + throw new Error(checker.getErrorMessage(this, target, reason)); + } }; /** @@ -288,7 +291,7 @@ Blockly.Connection.prototype.getConnectionTypeChecker = function() { * @deprecated July 2020 */ Blockly.Connection.prototype.isConnectionAllowed = function(candidate) { - return this.getConnectionTypeChecker().canConnect(this, candidate, true, false); + return this.getConnectionTypeChecker().canConnect(this, candidate, true); }; /** @@ -312,8 +315,7 @@ Blockly.Connection.prototype.connect = function(otherConnection) { } var checker = this.getConnectionTypeChecker(); - // TODO (fenichel): Try to get rid of the extra parameter (shouldThrow). - if (checker.canConnect(this, otherConnection, false, true)) { + if (checker.canConnect(this, otherConnection, false)) { var eventGroup = Blockly.Events.getGroup(); if (!eventGroup) { Blockly.Events.setGroup(true); @@ -362,7 +364,7 @@ Blockly.Connection.singleConnection_ = function(block, orphanBlock) { var thisConnection = block.inputList[i].connection; var typeChecker = output.getConnectionTypeChecker(); if (thisConnection && thisConnection.type == Blockly.INPUT_VALUE && - typeChecker.canConnect(output, thisConnection, false, false)) { + typeChecker.canConnect(output, thisConnection, false)) { if (connection) { return null; // More than one connection. } @@ -492,7 +494,8 @@ Blockly.Connection.prototype.targetBlock = function() { * @return {boolean} True if the connections share a type. */ Blockly.Connection.prototype.checkType = function(otherConnection) { - return this.getConnectionTypeChecker().canConnect(this, otherConnection, false, false); + return this.getConnectionTypeChecker().canConnect(this, otherConnection, + false); }; /** @@ -518,7 +521,7 @@ Blockly.Connection.prototype.onCheckChanged_ = function() { // The new value type may not be compatible with the existing connection. if (this.isConnected() && (!this.targetConnection || !this.getConnectionTypeChecker().canConnect( - this, this.targetConnection, false, false))) { + this, this.targetConnection, false))) { var child = this.isSuperior() ? this.targetBlock() : this.sourceBlock_; child.unplug(); } diff --git a/core/connection_checks.js b/core/connection_checks.js index 1b456ddbd..8b52697da 100644 --- a/core/connection_checks.js +++ b/core/connection_checks.js @@ -18,68 +18,13 @@ Blockly.ConnectionTypeChecker = function() { * @param {Blockly.Connection} two Connection to check compatibility with. * @param {boolean} isDragging True if the connection is being made by dragging * a block. - * @param {boolean} shouldThrow Whether to throw an error when a connection is - * invalid. * @return {boolean} Whether the connection is legal. + * @public */ Blockly.ConnectionTypeChecker.prototype.canConnect = function(one, two, - isDragging, shouldThrow) { - if (this.passesSafetyChecks(one, two, shouldThrow)) { - var connOne = /** @type {!Blockly.Connection} **/ (one); - var connTwo = /** @type {!Blockly.Connection} **/ (two); - if (this.passesTypeChecks(connOne, connTwo, shouldThrow)) { - if (!isDragging || this.passesDragChecks(connOne, connTwo, shouldThrow)) { - return true; - } - } - } - return false; -}; - -/** - * Check that connecting the given connections is safe, meaning that it would - * not break any of Blockly's basic assumptions--no self connections, etc. - * @param {Blockly.Connection} one The first of the connections to check. - * @param {Blockly.Connection} two The second of the connections to check. - * @param {boolean} shouldThrow Whether to throw an error if the connection is - * unsafe. - * @return {boolean} Whether the connection is safe. - * @package - */ -Blockly.ConnectionTypeChecker.prototype.passesSafetyChecks = function(one, two, shouldThrow) { - var safety = this.doSafetyChecks_(one, two); - if (safety == Blockly.Connection.CAN_CONNECT) { - return true; - } - if (shouldThrow) { - throw Error(this.getErrorMessage_(safety, one, two)); - } - return false; -}; - - -Blockly.ConnectionTypeChecker.prototype.passesTypeChecks = function(one, two, - shouldThrow) { - if (this.doTypeChecks_(one, two)) { - return true; - } - if (shouldThrow) { - throw Error(this.getErrorMessage_( - Blockly.Connection.REASON_CHECKS_FAILED, one, two)); - } - return false; -}; - -Blockly.ConnectionTypeChecker.prototype.passesDragChecks = function(one, two, - shouldThrow) { - if (this.doDragChecks_(one, two)) { - return true; - } - if (shouldThrow) { - throw Error(this.getErrorMessage_( - Blockly.Connection.REASON_DRAG_CHECKS_FAILED, one, two)); - } - return false; + isDragging) { + return this.canConnectWithReason(one, two, isDragging) == + Blockly.Connection.CAN_CONNECT; }; /** @@ -87,20 +32,28 @@ Blockly.ConnectionTypeChecker.prototype.passesDragChecks = function(one, two, * connection. * @param {Blockly.Connection} one Connection to check compatibility with. * @param {Blockly.Connection} two Connection to check compatibility with. + * @param {boolean} isDragging [description] * @return {number} Blockly.Connection.CAN_CONNECT if the connection is legal, * an error code otherwise. + * @public */ -Blockly.ConnectionTypeChecker.prototype.canConnectWithReason = function(one, two) { - var safety = this.doSafetyChecks_(one, two); +Blockly.ConnectionTypeChecker.prototype.canConnectWithReason = function( + one, two, isDragging) { + var safety = this.doSafetyChecks(one, two); if (safety != Blockly.Connection.CAN_CONNECT) { return safety; } var connOne = /** @type {!Blockly.Connection} **/ (one); var connTwo = /** @type {!Blockly.Connection} **/ (two); - if (!this.doTypeChecks_(connOne, connTwo)) { + if (!this.doTypeChecks(connOne, connTwo)) { return Blockly.Connection.REASON_CHECKS_FAILED; } + + if (isDragging && this.doDragChecks(connOne, connTwo, false)) { + return Blockly.REASON_DRAG_CHECKS_FAILED; + } + return Blockly.Connection.CAN_CONNECT; }; @@ -111,9 +64,9 @@ Blockly.ConnectionTypeChecker.prototype.canConnectWithReason = function(one, two * @param {Blockly.Connection} two The second of the two connections being * checked. * @return {string} A developer-readable error string. - * @private + * @public */ -Blockly.ConnectionTypeChecker.prototype.getErrorMessage_ = function(errorCode, +Blockly.ConnectionTypeChecker.prototype.getErrorMessage = function(errorCode, one, two) { switch (errorCode) { case Blockly.Connection.REASON_SELF_CONNECTION: @@ -144,9 +97,9 @@ Blockly.ConnectionTypeChecker.prototype.getErrorMessage_ = function(errorCode, * @param {Blockly.Connection} one The first of the connections to check. * @param {Blockly.Connection} two The second of the connections to check. * @return {number} An enum with the reason this connection is safe or unsafe. - * @private + * @public */ -Blockly.ConnectionTypeChecker.prototype.doSafetyChecks_ = function(one, two) { +Blockly.ConnectionTypeChecker.prototype.doSafetyChecks = function(one, two) { if (!one || !two) { return Blockly.Connection.REASON_TARGET_NULL; } @@ -176,9 +129,9 @@ Blockly.ConnectionTypeChecker.prototype.doSafetyChecks_ = function(one, two) { * @param {!Blockly.Connection} one Connection to compare. * @param {!Blockly.Connection} two Connection to compare against. * @return {boolean} True if the connections share a type. - * @protected + * @public */ -Blockly.ConnectionTypeChecker.prototype.doTypeChecks_ = function(one, two) { +Blockly.ConnectionTypeChecker.prototype.doTypeChecks = function(one, two) { var checkArrayOne = one.getCheck(); var checkArrayTwo = two.getCheck(); @@ -201,9 +154,9 @@ Blockly.ConnectionTypeChecker.prototype.doTypeChecks_ = function(one, two) { * @param {!Blockly.Connection} one Connection to compare. * @param {!Blockly.Connection} two Connection to compare against. * @return {boolean} True if the connections share a type. - * @protected + * @public */ -Blockly.ConnectionTypeChecker.prototype.doDragChecks_ = function(one, two) { +Blockly.ConnectionTypeChecker.prototype.doDragChecks = function(one, two) { // Don't consider insertion markers. if (two.getSourceBlock().isInsertionMarker()) { return false; diff --git a/core/connection_db.js b/core/connection_db.js index ff31704a2..c45163509 100644 --- a/core/connection_db.js +++ b/core/connection_db.js @@ -23,6 +23,9 @@ goog.requireType('Blockly.ConnectionTypeChecker'); * Database of connections. * Connections are stored in order of their vertical component. This way * connections in an area may be looked up quickly using a binary search. + * @param {!Blockly.ConnectionTypeChecker} typeChecker The workspace's + * connection type checker, used to decide if connections are valid during a + * drag. * @constructor */ Blockly.ConnectionDB = function(typeChecker) { @@ -246,7 +249,7 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, temp = this.connections_[pointerMin]; curDistance = temp.distanceFrom(conn); if (curDistance <= bestRadius && - this.typeChecker_.canConnect(conn, temp, true, false)) { + this.typeChecker_.canConnect(conn, temp, true)) { bestConnection = temp; bestRadius = curDistance; } @@ -259,7 +262,7 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, temp = this.connections_[pointerMax]; curDistance = temp.distanceFrom(conn); if (curDistance <= bestRadius && - this.typeChecker_.canConnect(conn, temp, true, false)) { + this.typeChecker_.canConnect(conn, temp, true)) { bestConnection = temp; bestRadius = curDistance; } @@ -276,6 +279,9 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, /** * Initialize a set of connection DBs for a workspace. + * @param {!Blockly.ConnectionTypeChecker} typeChecker The workspace's + * connection type checker, used to decide if connections are valid during a + * drag. * @return {!Array.} Array of databases. */ Blockly.ConnectionDB.init = function(typeChecker) { diff --git a/core/keyboard_nav/navigation.js b/core/keyboard_nav/navigation.js index 985a854bf..33b68cfc4 100644 --- a/core/keyboard_nav/navigation.js +++ b/core/keyboard_nav/navigation.js @@ -413,7 +413,7 @@ Blockly.navigation.moveAndConnect_ = function(movingConnection, destConnection) var checker = movingConnection.getConnectionTypeChecker(); - if (checker.canConnect(movingConnection, destConnection, false, false)) { + if (checker.canConnect(movingConnection, destConnection, false)) { Blockly.navigation.disconnectChild_(movingConnection, destConnection); if (!destConnection.isSuperior()) { @@ -501,17 +501,11 @@ Blockly.navigation.connect_ = function(movingConnection, destConnection) { } else if (Blockly.navigation.moveAndConnect_(movingConnection, destConnection)){ return true; } else { - try { - // TODO (fenichel): Don't rely on canConnect throwing an error. - // TODO (fenichel): Fix associated tests. - var checker = movingConnection.getConnectionTypeChecker(); - checker.canConnect(movingConnection, destConnection, false, true); - } - catch (e) { - // If nothing worked there should be an error. - // Report the error from the original connections. - Blockly.navigation.warn_('Connection failed with error: ' + e); - } + var checker = movingConnection.getConnectionTypeChecker(); + var reason = checker.canConnectWithReason( + movingConnection, destConnection, false); + Blockly.navigation.warn_('Connection failed with error: ' + + checker.getErrorMessage(movingConnection, destConnection, reason)); return false; } }; diff --git a/core/rendered_connection.js b/core/rendered_connection.js index ea8e2868e..e450db3dd 100644 --- a/core/rendered_connection.js +++ b/core/rendered_connection.js @@ -553,7 +553,7 @@ Blockly.RenderedConnection.prototype.onCheckChanged_ = function() { // The new value type may not be compatible with the existing connection. if (this.isConnected() && (!this.targetConnection || !this.getConnectionTypeChecker().canConnect( - this, this.targetConnection, false, false))) { + this, this.targetConnection, false))) { var child = this.isSuperior() ? this.targetBlock() : this.sourceBlock_; child.unplug(); // Bump away. diff --git a/core/renderers/common/renderer.js b/core/renderers/common/renderer.js index f0b158c0e..2807ec8c5 100644 --- a/core/renderers/common/renderer.js +++ b/core/renderers/common/renderer.js @@ -255,7 +255,7 @@ Blockly.blockRendering.Renderer.prototype.orphanCanConnectAtEnd = return false; } return orphanConnection.getConnectionTypeChecker().canConnect( - lastConnection, orphanConnection, false, false); + lastConnection, orphanConnection, false); }; /** diff --git a/tests/mocha/connection_test.js b/tests/mocha/connection_test.js index dd8627943..f105bcbe2 100644 --- a/tests/mocha/connection_test.js +++ b/tests/mocha/connection_test.js @@ -8,7 +8,7 @@ suite('Connection type checker', function() { suiteSetup(function() { this.checker = new Blockly.ConnectionTypeChecker(); }); - suite('Can Connect With Reason', function() { + suite('Safety checks', function() { function assertReasonHelper(checker, one, two, reason) { chai.assert.equal(checker.canConnectWithReason(one, two), reason); // Order should not matter. @@ -229,9 +229,9 @@ suite('Connection type checker', function() { this.con2 = new Blockly.Connection({}, Blockly.NEXT_STATEMENT); }); function assertCheckTypes(checker, one, two) { - chai.assert.isTrue(checker.passesTypeChecks(one, two)); + chai.assert.isTrue(checker.doTypeChecks(one, two)); // Order should not matter. - chai.assert.isTrue(checker.passesTypeChecks(one, two)); + chai.assert.isTrue(checker.doTypeChecks(one, two)); } test('No Types', function() { assertCheckTypes(this.checker, this.con1, this.con2); @@ -258,7 +258,7 @@ suite('Connection type checker', function() { test('No Compatible Types', function() { this.con1.setCheck('type1'); this.con2.setCheck('type2'); - chai.assert.isFalse(this.checker.passesTypeChecks(this.con1, this.con2)); + chai.assert.isFalse(this.checker.doTypeChecks(this.con1, this.con2)); }); }); }); From 8ae3dc3d5e28d28247e88e3564d23b4e2c64f430 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Tue, 14 Jul 2020 17:02:28 -0600 Subject: [PATCH 08/18] Cleanup --- core/block.js | 1 - core/connection.js | 1 + core/connection_checks.js | 11 +++++++++++ core/connection_db.js | 2 -- 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/core/block.js b/core/block.js index 9aabe7edb..ff513cd8b 100644 --- a/core/block.js +++ b/core/block.js @@ -32,7 +32,6 @@ goog.require('Blockly.utils.string'); goog.require('Blockly.Workspace'); goog.requireType('Blockly.IASTNodeLocation'); -goog.requireType('Blockly.connectionTypeChecker'); /** diff --git a/core/connection.js b/core/connection.js index 92e578d11..ab4fec6bc 100644 --- a/core/connection.js +++ b/core/connection.js @@ -494,6 +494,7 @@ Blockly.Connection.prototype.targetBlock = function() { * @return {boolean} True if the connections share a type. */ Blockly.Connection.prototype.checkType = function(otherConnection) { + // TODO (fenichel): Add deprecation warnings. return this.getConnectionTypeChecker().canConnect(this, otherConnection, false); }; diff --git a/core/connection_checks.js b/core/connection_checks.js index 8b52697da..849f91979 100644 --- a/core/connection_checks.js +++ b/core/connection_checks.js @@ -1,3 +1,14 @@ +/** + * @license + * Copyright 2020 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * @fileoverview An object that encapsulates logic for checking whether a potential + * connection is safe and valid. + * @author fenichel@google.com (Rachel Fenichel) + */ 'use strict'; goog.provide('Blockly.ConnectionTypeChecker'); diff --git a/core/connection_db.js b/core/connection_db.js index c45163509..104ad46b0 100644 --- a/core/connection_db.js +++ b/core/connection_db.js @@ -16,8 +16,6 @@ goog.provide('Blockly.ConnectionDB'); goog.require('Blockly.RenderedConnection'); -goog.requireType('Blockly.ConnectionTypeChecker'); - /** * Database of connections. From 7288c66294e8c95e4ac96afa82021aaf168b1814 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 16 Jul 2020 10:56:32 -0600 Subject: [PATCH 09/18] Rename connectionTypeChecker->connectionChecker --- blockly_uncompressed.js | 4 +-- blocks/logic.js | 4 +-- core/block.js | 4 +-- core/connection.js | 26 ++++++++--------- ...ection_checks.js => connection_checker.js} | 29 ++++++++++--------- core/connection_db.js | 27 ++++++++--------- core/keyboard_nav/navigation.js | 5 ++-- core/rendered_connection.js | 4 +-- core/renderers/common/renderer.js | 2 +- core/workspace.js | 10 +++++-- core/workspace_svg.js | 3 +- tests/mocha/connection_db_test.js | 6 ++-- tests/mocha/connection_test.js | 2 +- 13 files changed, 66 insertions(+), 60 deletions(-) rename core/{connection_checks.js => connection_checker.js} (88%) diff --git a/blockly_uncompressed.js b/blockly_uncompressed.js index 111ba6fd6..5c69d5ac9 100644 --- a/blockly_uncompressed.js +++ b/blockly_uncompressed.js @@ -37,7 +37,7 @@ goog.addDependency('../../core/components/tree/basenode.js', ['Blockly.tree.Base goog.addDependency('../../core/components/tree/treecontrol.js', ['Blockly.tree.TreeControl'], ['Blockly.tree.BaseNode', 'Blockly.tree.TreeNode', 'Blockly.utils.aria', 'Blockly.utils.object', 'Blockly.utils.style'], {}); goog.addDependency('../../core/components/tree/treenode.js', ['Blockly.tree.TreeNode'], ['Blockly.tree.BaseNode', 'Blockly.utils.KeyCodes', 'Blockly.utils.object'], {}); goog.addDependency('../../core/connection.js', ['Blockly.Connection'], ['Blockly.Events', 'Blockly.Events.BlockMove', 'Blockly.Xml'], {}); -goog.addDependency('../../core/connection_checks.js', ['Blockly.ConnectionTypeChecker'], [], {}); +goog.addDependency('../../core/connection_checker.js', ['Blockly.ConnectionChecker'], [], {}); goog.addDependency('../../core/connection_db.js', ['Blockly.ConnectionDB'], ['Blockly.RenderedConnection'], {}); goog.addDependency('../../core/constants.js', ['Blockly.constants'], [], {}); goog.addDependency('../../core/contextmenu.js', ['Blockly.ContextMenu'], ['Blockly.Events', 'Blockly.Events.BlockCreate', 'Blockly.Menu', 'Blockly.MenuItem', 'Blockly.Msg', 'Blockly.Xml', 'Blockly.utils', 'Blockly.utils.Coordinate', 'Blockly.utils.Rect', 'Blockly.utils.dom', 'Blockly.utils.userAgent'], {}); @@ -184,7 +184,7 @@ goog.addDependency('../../core/variables.js', ['Blockly.Variables'], ['Blockly.B goog.addDependency('../../core/variables_dynamic.js', ['Blockly.VariablesDynamic'], ['Blockly.Blocks', 'Blockly.Msg', 'Blockly.VariableModel', 'Blockly.Variables', 'Blockly.utils.xml'], {}); goog.addDependency('../../core/warning.js', ['Blockly.Warning'], ['Blockly.Bubble', 'Blockly.Events', 'Blockly.Events.Ui', 'Blockly.Icon', 'Blockly.utils.dom', 'Blockly.utils.object'], {}); goog.addDependency('../../core/widgetdiv.js', ['Blockly.WidgetDiv'], ['Blockly.utils.style'], {}); -goog.addDependency('../../core/workspace.js', ['Blockly.Workspace'], ['Blockly.ConnectionTypeChecker', 'Blockly.Events', 'Blockly.Options', 'Blockly.VariableMap', 'Blockly.utils', 'Blockly.utils.math'], {}); +goog.addDependency('../../core/workspace.js', ['Blockly.Workspace'], ['Blockly.ConnectionChecker', 'Blockly.Events', 'Blockly.Options', 'Blockly.VariableMap', 'Blockly.utils', 'Blockly.utils.math'], {}); goog.addDependency('../../core/workspace_audio.js', ['Blockly.WorkspaceAudio'], ['Blockly.utils', 'Blockly.utils.global', 'Blockly.utils.userAgent'], {'lang': 'es5'}); goog.addDependency('../../core/workspace_comment.js', ['Blockly.WorkspaceComment'], ['Blockly.Events', 'Blockly.Events.CommentChange', 'Blockly.Events.CommentCreate', 'Blockly.Events.CommentDelete', 'Blockly.Events.CommentMove', 'Blockly.utils', 'Blockly.utils.Coordinate', 'Blockly.utils.xml'], {}); goog.addDependency('../../core/workspace_comment_render_svg.js', ['Blockly.WorkspaceCommentSvg.render'], ['Blockly.utils', 'Blockly.utils.Coordinate', 'Blockly.utils.dom'], {}); diff --git a/blocks/logic.js b/blocks/logic.js index bc46c82d6..afec94d3f 100644 --- a/blocks/logic.js +++ b/blocks/logic.js @@ -543,7 +543,7 @@ Blockly.Constants.Logic.LOGIC_COMPARE_ONCHANGE_MIXIN = { var blockB = this.getInputTargetBlock('B'); // Disconnect blocks that existed prior to this change if they don't match. if (blockA && blockB && - !this.workspace.connectionTypeChecker.doTypeChecks( + !this.workspace.connectionChecker.doTypeChecks( blockA.outputConnection, blockB.outputConnection)) { // Mismatch between two inputs. Revert the block connections, // bumping away the newly connected block(s). @@ -612,7 +612,7 @@ Blockly.Constants.Logic.LOGIC_TERNARY_ONCHANGE_MIXIN = { for (var i = 0; i < 2; i++) { var block = (i == 1) ? blockA : blockB; if (block && - !block.workspace.connectionTypeChecker.doTypeChecks( + !block.workspace.connectionChecker.doTypeChecks( block.outputConnection, parentConnection)) { // Ensure that any disconnections are grouped with the causing event. Blockly.Events.setGroup(e.group); diff --git a/core/block.js b/core/block.js index ff513cd8b..706ac2575 100644 --- a/core/block.js +++ b/core/block.js @@ -456,7 +456,7 @@ Blockly.Block.prototype.unplugFromRow_ = function(opt_healStack) { // Disconnect the child block. childConnection.disconnect(); // Connect child to the parent if possible, otherwise bump away. - if (this.workspace.connectionTypeChecker.canConnect( + if (this.workspace.connectionChecker.canConnect( childConnection, parentConnection, false)) { parentConnection.connect(childConnection); } else { @@ -510,7 +510,7 @@ Blockly.Block.prototype.unplugFromStack_ = function(opt_healStack) { var nextTarget = this.nextConnection.targetConnection; nextTarget.disconnect(); if (previousTarget && - this.workspace.connectionTypeChecker.canConnect( + this.workspace.connectionChecker.canConnect( previousTarget, nextTarget, false)) { // Attach the next statement to the previous statement. previousTarget.connect(nextTarget); diff --git a/core/connection.js b/core/connection.js index ab4fec6bc..b15b437ff 100644 --- a/core/connection.js +++ b/core/connection.js @@ -16,7 +16,7 @@ goog.require('Blockly.Events'); goog.require('Blockly.Events.BlockMove'); goog.require('Blockly.Xml'); -goog.requireType('Blockly.ConnectionTypeChecker'); +goog.requireType('Blockly.ConnectionChecker'); goog.requireType('Blockly.IASTNodeLocationWithBlock'); @@ -147,8 +147,8 @@ Blockly.Connection.prototype.connect_ = function(childConnection) { if (nextBlock && !nextBlock.isShadow()) { newBlock = nextBlock; } else { - var typeChecker = orphanBlock.workspace.connectionTypeChecker; - if (typeChecker.canConnect( + var checker = orphanBlock.workspace.connectionChecker; + if (checker.canConnect( orphanBlock.previousConnection, newBlock.nextConnection, false)) { newBlock.nextConnection.connect(orphanBlock.previousConnection); orphanBlock = null; @@ -252,7 +252,7 @@ Blockly.Connection.prototype.isConnected = function() { */ Blockly.Connection.prototype.canConnectWithReason = function(target) { // TODO: deprecation warning with date, plus tests. - return this.getConnectionTypeChecker().canConnectWithReason( + return this.getConnectionChecker().canConnectWithReason( this, target); }; @@ -267,7 +267,7 @@ Blockly.Connection.prototype.canConnectWithReason = function(target) { Blockly.Connection.prototype.checkConnection = function(target) { // TODO: Add deprecation warning notices *and* add tests to make sure these // still work (for any blocks that use them). - var checker = this.getConnectionTypeChecker(); + var checker = this.getConnectionChecker(); var reason = !checker.canConnectWithReason(this, target, false); if (reason != Blockly.Connection.CAN_CONNECT) { throw new Error(checker.getErrorMessage(this, target, reason)); @@ -276,12 +276,12 @@ Blockly.Connection.prototype.checkConnection = function(target) { /** * Get the workspace's connection type checker object. - * @return {!Blockly.ConnectionTypeChecker} The connection type checker for the + * @return {!Blockly.ConnectionChecker} The connection type checker for the * source block's workspace. * @package */ -Blockly.Connection.prototype.getConnectionTypeChecker = function() { - return this.sourceBlock_.workspace.connectionTypeChecker; +Blockly.Connection.prototype.getConnectionChecker = function() { + return this.sourceBlock_.workspace.connectionChecker; }; /** @@ -291,7 +291,7 @@ Blockly.Connection.prototype.getConnectionTypeChecker = function() { * @deprecated July 2020 */ Blockly.Connection.prototype.isConnectionAllowed = function(candidate) { - return this.getConnectionTypeChecker().canConnect(this, candidate, true); + return this.getConnectionChecker().canConnect(this, candidate, true); }; /** @@ -314,7 +314,7 @@ Blockly.Connection.prototype.connect = function(otherConnection) { return; } - var checker = this.getConnectionTypeChecker(); + var checker = this.getConnectionChecker(); if (checker.canConnect(this, otherConnection, false)) { var eventGroup = Blockly.Events.getGroup(); if (!eventGroup) { @@ -362,7 +362,7 @@ Blockly.Connection.singleConnection_ = function(block, orphanBlock) { var output = orphanBlock.outputConnection; for (var i = 0; i < block.inputList.length; i++) { var thisConnection = block.inputList[i].connection; - var typeChecker = output.getConnectionTypeChecker(); + var typeChecker = output.getConnectionChecker(); if (thisConnection && thisConnection.type == Blockly.INPUT_VALUE && typeChecker.canConnect(output, thisConnection, false)) { if (connection) { @@ -495,7 +495,7 @@ Blockly.Connection.prototype.targetBlock = function() { */ Blockly.Connection.prototype.checkType = function(otherConnection) { // TODO (fenichel): Add deprecation warnings. - return this.getConnectionTypeChecker().canConnect(this, otherConnection, + return this.getConnectionChecker().canConnect(this, otherConnection, false); }; @@ -521,7 +521,7 @@ Blockly.Connection.prototype.checkType_ = function(otherConnection) { Blockly.Connection.prototype.onCheckChanged_ = function() { // The new value type may not be compatible with the existing connection. if (this.isConnected() && (!this.targetConnection || - !this.getConnectionTypeChecker().canConnect( + !this.getConnectionChecker().canConnect( this, this.targetConnection, false))) { var child = this.isSuperior() ? this.targetBlock() : this.sourceBlock_; child.unplug(); diff --git a/core/connection_checks.js b/core/connection_checker.js similarity index 88% rename from core/connection_checks.js rename to core/connection_checker.js index 849f91979..a5fa84c65 100644 --- a/core/connection_checks.js +++ b/core/connection_checker.js @@ -11,7 +11,7 @@ */ 'use strict'; -goog.provide('Blockly.ConnectionTypeChecker'); +goog.provide('Blockly.ConnectionChecker'); goog.requireType('Blockly.Connection'); @@ -19,7 +19,7 @@ goog.requireType('Blockly.Connection'); * Class for connection type checking logic. * @constructor */ -Blockly.ConnectionTypeChecker = function() { +Blockly.ConnectionChecker = function() { }; /** @@ -32,7 +32,7 @@ Blockly.ConnectionTypeChecker = function() { * @return {boolean} Whether the connection is legal. * @public */ -Blockly.ConnectionTypeChecker.prototype.canConnect = function(one, two, +Blockly.ConnectionChecker.prototype.canConnect = function(one, two, isDragging) { return this.canConnectWithReason(one, two, isDragging) == Blockly.Connection.CAN_CONNECT; @@ -40,7 +40,7 @@ Blockly.ConnectionTypeChecker.prototype.canConnect = function(one, two, /** * Checks whether the current connection can connect with the target - * connection. + * connection, and return an error code if there are problems. * @param {Blockly.Connection} one Connection to check compatibility with. * @param {Blockly.Connection} two Connection to check compatibility with. * @param {boolean} isDragging [description] @@ -48,13 +48,14 @@ Blockly.ConnectionTypeChecker.prototype.canConnect = function(one, two, * an error code otherwise. * @public */ -Blockly.ConnectionTypeChecker.prototype.canConnectWithReason = function( +Blockly.ConnectionChecker.prototype.canConnectWithReason = function( one, two, isDragging) { var safety = this.doSafetyChecks(one, two); if (safety != Blockly.Connection.CAN_CONNECT) { return safety; } + // If the safety checks passed, both connections are non-null. var connOne = /** @type {!Blockly.Connection} **/ (one); var connTwo = /** @type {!Blockly.Connection} **/ (two); if (!this.doTypeChecks(connOne, connTwo)) { @@ -77,7 +78,7 @@ Blockly.ConnectionTypeChecker.prototype.canConnectWithReason = function( * @return {string} A developer-readable error string. * @public */ -Blockly.ConnectionTypeChecker.prototype.getErrorMessage = function(errorCode, +Blockly.ConnectionChecker.prototype.getErrorMessage = function(errorCode, one, two) { switch (errorCode) { case Blockly.Connection.REASON_SELF_CONNECTION: @@ -90,8 +91,10 @@ Blockly.ConnectionTypeChecker.prototype.getErrorMessage = function(errorCode, case Blockly.Connection.REASON_TARGET_NULL: return 'Target connection is null.'; case Blockly.Connection.REASON_CHECKS_FAILED: + var connOne = /** @type {!Blockly.Connection} **/ (one); + var connTwo = /** @type {!Blockly.Connection} **/ (two); var msg = 'Connection checks failed. '; - msg += one + ' expected ' + one.getCheck() + ', found ' + two.getCheck(); + msg += connOne + ' expected ' + connOne.getCheck() + ', found ' + connTwo.getCheck(); return msg; case Blockly.Connection.REASON_SHADOW_PARENT: return 'Connecting non-shadow to shadow block.'; @@ -104,13 +107,13 @@ Blockly.ConnectionTypeChecker.prototype.getErrorMessage = function(errorCode, /** * Check that connecting the given connections is safe, meaning that it would - * not break any of Blockly's basic assumptions--no self connections, etc. + * not break any of Blockly's basic assumptions (e.g. no self connections). * @param {Blockly.Connection} one The first of the connections to check. * @param {Blockly.Connection} two The second of the connections to check. * @return {number} An enum with the reason this connection is safe or unsafe. * @public */ -Blockly.ConnectionTypeChecker.prototype.doSafetyChecks = function(one, two) { +Blockly.ConnectionChecker.prototype.doSafetyChecks = function(one, two) { if (!one || !two) { return Blockly.Connection.REASON_TARGET_NULL; } @@ -142,7 +145,7 @@ Blockly.ConnectionTypeChecker.prototype.doSafetyChecks = function(one, two) { * @return {boolean} True if the connections share a type. * @public */ -Blockly.ConnectionTypeChecker.prototype.doTypeChecks = function(one, two) { +Blockly.ConnectionChecker.prototype.doTypeChecks = function(one, two) { var checkArrayOne = one.getCheck(); var checkArrayTwo = two.getCheck(); @@ -167,7 +170,7 @@ Blockly.ConnectionTypeChecker.prototype.doTypeChecks = function(one, two) { * @return {boolean} True if the connections share a type. * @public */ -Blockly.ConnectionTypeChecker.prototype.doDragChecks = function(one, two) { +Blockly.ConnectionChecker.prototype.doDragChecks = function(one, two) { // Don't consider insertion markers. if (two.getSourceBlock().isInsertionMarker()) { return false; @@ -224,7 +227,7 @@ Blockly.ConnectionTypeChecker.prototype.doDragChecks = function(one, two) { }; /** - * Helper function for drag checking + * Helper function for drag checking. * @param {!Blockly.Connection} one The connection to check, which must be a * statement input or next connection. * @param {!Blockly.Connection} two A nearby connection to check, which @@ -232,7 +235,7 @@ Blockly.ConnectionTypeChecker.prototype.doDragChecks = function(one, two) { * @return {boolean} True if the connection is allowed, false otherwise. * @protected */ -Blockly.ConnectionTypeChecker.prototype.canConnectToPrevious_ = function(one, two) { +Blockly.ConnectionChecker.prototype.canConnectToPrevious_ = function(one, two) { if (one.targetConnection) { // This connection is already occupied. // A next connection will never disconnect itself mid-drag. diff --git a/core/connection_db.js b/core/connection_db.js index 104ad46b0..4dca293bb 100644 --- a/core/connection_db.js +++ b/core/connection_db.js @@ -16,24 +16,26 @@ goog.provide('Blockly.ConnectionDB'); goog.require('Blockly.RenderedConnection'); +goog.requireType('Blockly.ConnectionChecker'); + /** * Database of connections. * Connections are stored in order of their vertical component. This way * connections in an area may be looked up quickly using a binary search. - * @param {!Blockly.ConnectionTypeChecker} typeChecker The workspace's + * @param {!Blockly.ConnectionChecker} checker The workspace's * connection type checker, used to decide if connections are valid during a * drag. * @constructor */ -Blockly.ConnectionDB = function(typeChecker) { +Blockly.ConnectionDB = function(checker) { /** * Array of connections sorted by y position in workspace units. * @type {!Array.} * @private */ this.connections_ = []; - this.typeChecker_ = typeChecker; + this.connectionChecker_ = checker; }; /** @@ -247,7 +249,7 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, temp = this.connections_[pointerMin]; curDistance = temp.distanceFrom(conn); if (curDistance <= bestRadius && - this.typeChecker_.canConnect(conn, temp, true)) { + this.connectionChecker_.canConnect(conn, temp, true)) { bestConnection = temp; bestRadius = curDistance; } @@ -260,7 +262,7 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, temp = this.connections_[pointerMax]; curDistance = temp.distanceFrom(conn); if (curDistance <= bestRadius && - this.typeChecker_.canConnect(conn, temp, true)) { + this.connectionChecker_.canConnect(conn, temp, true)) { bestConnection = temp; bestRadius = curDistance; } @@ -277,17 +279,16 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, /** * Initialize a set of connection DBs for a workspace. - * @param {!Blockly.ConnectionTypeChecker} typeChecker The workspace's - * connection type checker, used to decide if connections are valid during a - * drag. + * @param {!Blockly.ConnectionChecker} checker The workspace's + * connection checker, used to decide if connections are valid during a drag. * @return {!Array.} Array of databases. */ -Blockly.ConnectionDB.init = function(typeChecker) { +Blockly.ConnectionDB.init = function(checker) { // Create four databases, one for each connection type. var dbList = []; - dbList[Blockly.INPUT_VALUE] = new Blockly.ConnectionDB(typeChecker); - dbList[Blockly.OUTPUT_VALUE] = new Blockly.ConnectionDB(typeChecker); - dbList[Blockly.NEXT_STATEMENT] = new Blockly.ConnectionDB(typeChecker); - dbList[Blockly.PREVIOUS_STATEMENT] = new Blockly.ConnectionDB(typeChecker); + dbList[Blockly.INPUT_VALUE] = new Blockly.ConnectionDB(checker); + dbList[Blockly.OUTPUT_VALUE] = new Blockly.ConnectionDB(checker); + dbList[Blockly.NEXT_STATEMENT] = new Blockly.ConnectionDB(checker); + dbList[Blockly.PREVIOUS_STATEMENT] = new Blockly.ConnectionDB(checker); return dbList; }; diff --git a/core/keyboard_nav/navigation.js b/core/keyboard_nav/navigation.js index 33b68cfc4..fcf9fdfa8 100644 --- a/core/keyboard_nav/navigation.js +++ b/core/keyboard_nav/navigation.js @@ -18,7 +18,6 @@ goog.require('Blockly.ASTNode'); goog.require('Blockly.utils.Coordinate'); goog.require('Blockly.user.keyMap'); -goog.requireType('Blockly.ConnectionTypeChecker'); /** * A function to call to give feedback to the user about logs, warnings, and @@ -411,7 +410,7 @@ Blockly.navigation.moveAndConnect_ = function(movingConnection, destConnection) } var movingBlock = movingConnection.getSourceBlock(); - var checker = movingConnection.getConnectionTypeChecker(); + var checker = movingConnection.getConnectionChecker(); if (checker.canConnect(movingConnection, destConnection, false)) { Blockly.navigation.disconnectChild_(movingConnection, destConnection); @@ -501,7 +500,7 @@ Blockly.navigation.connect_ = function(movingConnection, destConnection) { } else if (Blockly.navigation.moveAndConnect_(movingConnection, destConnection)){ return true; } else { - var checker = movingConnection.getConnectionTypeChecker(); + var checker = movingConnection.getConnectionChecker(); var reason = checker.canConnectWithReason( movingConnection, destConnection, false); Blockly.navigation.warn_('Connection failed with error: ' + diff --git a/core/rendered_connection.js b/core/rendered_connection.js index e450db3dd..0046446b3 100644 --- a/core/rendered_connection.js +++ b/core/rendered_connection.js @@ -19,7 +19,7 @@ goog.require('Blockly.utils.Coordinate'); goog.require('Blockly.utils.dom'); goog.require('Blockly.utils.object'); -goog.requireType('Blockly.ConnectionTypeChecker'); +goog.requireType('Blockly.ConnectionChecker'); /** @@ -552,7 +552,7 @@ Blockly.RenderedConnection.prototype.connect_ = function(childConnection) { Blockly.RenderedConnection.prototype.onCheckChanged_ = function() { // The new value type may not be compatible with the existing connection. if (this.isConnected() && (!this.targetConnection || - !this.getConnectionTypeChecker().canConnect( + !this.getConnectionChecker().canConnect( this, this.targetConnection, false))) { var child = this.isSuperior() ? this.targetBlock() : this.sourceBlock_; child.unplug(); diff --git a/core/renderers/common/renderer.js b/core/renderers/common/renderer.js index 2807ec8c5..d0d8c1691 100644 --- a/core/renderers/common/renderer.js +++ b/core/renderers/common/renderer.js @@ -254,7 +254,7 @@ Blockly.blockRendering.Renderer.prototype.orphanCanConnectAtEnd = if (!lastConnection) { return false; } - return orphanConnection.getConnectionTypeChecker().canConnect( + return orphanConnection.getConnectionChecker().canConnect( lastConnection, orphanConnection, false); }; diff --git a/core/workspace.js b/core/workspace.js index 9275f7a32..b79ab340c 100644 --- a/core/workspace.js +++ b/core/workspace.js @@ -12,7 +12,7 @@ goog.provide('Blockly.Workspace'); -goog.require('Blockly.ConnectionTypeChecker'); +goog.require('Blockly.ConnectionChecker'); goog.require('Blockly.Events'); goog.require('Blockly.Options'); goog.require('Blockly.utils'); @@ -43,6 +43,12 @@ Blockly.Workspace = function(opt_options) { /** @type {number} */ this.toolboxPosition = this.options.toolboxPosition; + /** + * An object that encapsulates logic for safety, type, and dragging checks. + * @type {Blockly} + */ + this.connectionChecker = new Blockly.ConnectionChecker(); + /** * @type {!Array.} * @private @@ -104,8 +110,6 @@ Blockly.Workspace = function(opt_options) { * @private */ this.potentialVariableMap_ = null; - /** @type {!Blockly.ConnectionTypeChecker} [description] */ - this.connectionTypeChecker = new Blockly.ConnectionTypeChecker(); }; /** diff --git a/core/workspace_svg.js b/core/workspace_svg.js index e2fe81cd3..97739c44d 100644 --- a/core/workspace_svg.js +++ b/core/workspace_svg.js @@ -44,7 +44,6 @@ goog.require('Blockly.Xml'); goog.requireType('Blockly.blockRendering.Renderer'); goog.requireType('Blockly.IASTNodeLocationSvg'); goog.requireType('Blockly.IBoundedElement'); -goog.requireType('Blockly.ConnectionTypeChecker'); /** @@ -70,7 +69,7 @@ Blockly.WorkspaceSvg = function(options, options.setMetrics || Blockly.WorkspaceSvg.setTopLevelWorkspaceMetrics_; - this.connectionDBList = Blockly.ConnectionDB.init(this.connectionTypeChecker); + this.connectionDBList = Blockly.ConnectionDB.init(this.connectionChecker); if (opt_blockDragSurface) { this.blockDragSurface_ = opt_blockDragSurface; diff --git a/tests/mocha/connection_db_test.js b/tests/mocha/connection_db_test.js index 52f926391..f8dccd5d0 100644 --- a/tests/mocha/connection_db_test.js +++ b/tests/mocha/connection_db_test.js @@ -6,7 +6,7 @@ suite('Connection Database', function() { setup(function() { - this.database = new Blockly.ConnectionDB(new Blockly.ConnectionTypeChecker()); + this.database = new Blockly.ConnectionDB(new Blockly.ConnectionChecker()); this.assertOrder = function() { var length = this.database.connections_.length; @@ -194,8 +194,8 @@ suite('Connection Database', function() { suite('Search For Closest', function() { setup(function() { this.allowedStub = null; - this.allowedStub = sinon.stub(this.database.typeChecker_, 'canConnect') - .callsFake(function(dragging, candidate) { + this.allowedStub = sinon.stub(this.database.connectionChecker_, 'canConnect') + .callsFake(function(_dragging, _candidate) { return true; }); this.createCheckConnection = function(x, y) { diff --git a/tests/mocha/connection_test.js b/tests/mocha/connection_test.js index f105bcbe2..4f058126b 100644 --- a/tests/mocha/connection_test.js +++ b/tests/mocha/connection_test.js @@ -6,7 +6,7 @@ suite('Connection type checker', function() { suiteSetup(function() { - this.checker = new Blockly.ConnectionTypeChecker(); + this.checker = new Blockly.ConnectionChecker(); }); suite('Safety checks', function() { function assertReasonHelper(checker, one, two, reason) { From 809148b435ca957a9b44c8de3606771d518f761a Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 16 Jul 2020 11:36:48 -0600 Subject: [PATCH 10/18] Fix compiler errors --- blockly_uncompressed.js | 1 + core/connection.js | 10 +-- core/connection_checker.js | 6 +- core/connection_db.js | 12 +++- core/interfaces/i_connection_checker.js | 87 +++++++++++++++++++++++++ core/keyboard_nav/navigation.js | 2 +- core/rendered_connection.js | 2 - core/workspace.js | 3 +- 8 files changed, 109 insertions(+), 14 deletions(-) create mode 100644 core/interfaces/i_connection_checker.js diff --git a/blockly_uncompressed.js b/blockly_uncompressed.js index 5c69d5ac9..f8a07fd95 100644 --- a/blockly_uncompressed.js +++ b/blockly_uncompressed.js @@ -75,6 +75,7 @@ goog.addDependency('../../core/input.js', ['Blockly.Input'], ['Blockly.Connectio goog.addDependency('../../core/insertion_marker_manager.js', ['Blockly.InsertionMarkerManager'], ['Blockly.Events', 'Blockly.blockAnimations'], {'lang': 'es5'}); goog.addDependency('../../core/interfaces/i_accessibility.js', ['Blockly.IASTNodeLocation', 'Blockly.IASTNodeLocationSvg', 'Blockly.IASTNodeLocationWithBlock', 'Blockly.IBlocklyActionable'], [], {}); goog.addDependency('../../core/interfaces/i_bounded_element.js', ['Blockly.IBoundedElement'], [], {}); +goog.addDependency('../../core/interfaces/i_connection_checker.js', ['Blockly.IConnectionChecker'], [], {}); goog.addDependency('../../core/interfaces/i_copyable.js', ['Blockly.ICopyable'], [], {}); goog.addDependency('../../core/interfaces/i_deletable.js', ['Blockly.IDeletable'], [], {}); goog.addDependency('../../core/interfaces/i_deletearea.js', ['Blockly.IDeleteArea'], [], {}); diff --git a/core/connection.js b/core/connection.js index b15b437ff..dfbc0ac3a 100644 --- a/core/connection.js +++ b/core/connection.js @@ -16,7 +16,7 @@ goog.require('Blockly.Events'); goog.require('Blockly.Events.BlockMove'); goog.require('Blockly.Xml'); -goog.requireType('Blockly.ConnectionChecker'); +goog.requireType('Blockly.IConnectionChecker'); goog.requireType('Blockly.IASTNodeLocationWithBlock'); @@ -253,7 +253,7 @@ Blockly.Connection.prototype.isConnected = function() { Blockly.Connection.prototype.canConnectWithReason = function(target) { // TODO: deprecation warning with date, plus tests. return this.getConnectionChecker().canConnectWithReason( - this, target); + this, target, false); }; /** @@ -268,15 +268,15 @@ Blockly.Connection.prototype.checkConnection = function(target) { // TODO: Add deprecation warning notices *and* add tests to make sure these // still work (for any blocks that use them). var checker = this.getConnectionChecker(); - var reason = !checker.canConnectWithReason(this, target, false); + var reason = checker.canConnectWithReason(this, target, false); if (reason != Blockly.Connection.CAN_CONNECT) { - throw new Error(checker.getErrorMessage(this, target, reason)); + throw new Error(checker.getErrorMessage(reason, this, target)); } }; /** * Get the workspace's connection type checker object. - * @return {!Blockly.ConnectionChecker} The connection type checker for the + * @return {!Blockly.IConnectionChecker} The connection type checker for the * source block's workspace. * @package */ diff --git a/core/connection_checker.js b/core/connection_checker.js index a5fa84c65..cb04e1e40 100644 --- a/core/connection_checker.js +++ b/core/connection_checker.js @@ -13,10 +13,12 @@ goog.provide('Blockly.ConnectionChecker'); +goog.requireType('Blockly.IConnectionChecker'); goog.requireType('Blockly.Connection'); /** * Class for connection type checking logic. + * @implements {Blockly.IConnectionChecker} * @constructor */ Blockly.ConnectionChecker = function() { @@ -62,8 +64,8 @@ Blockly.ConnectionChecker.prototype.canConnectWithReason = function( return Blockly.Connection.REASON_CHECKS_FAILED; } - if (isDragging && this.doDragChecks(connOne, connTwo, false)) { - return Blockly.REASON_DRAG_CHECKS_FAILED; + if (isDragging && this.doDragChecks(connOne, connTwo)) { + return Blockly.Connection.REASON_DRAG_CHECKS_FAILED; } return Blockly.Connection.CAN_CONNECT; diff --git a/core/connection_db.js b/core/connection_db.js index 4dca293bb..2026678d2 100644 --- a/core/connection_db.js +++ b/core/connection_db.js @@ -16,14 +16,14 @@ goog.provide('Blockly.ConnectionDB'); goog.require('Blockly.RenderedConnection'); -goog.requireType('Blockly.ConnectionChecker'); +goog.requireType('Blockly.IConnectionChecker'); /** * Database of connections. * Connections are stored in order of their vertical component. This way * connections in an area may be looked up quickly using a binary search. - * @param {!Blockly.ConnectionChecker} checker The workspace's + * @param {!Blockly.IConnectionChecker} checker The workspace's * connection type checker, used to decide if connections are valid during a * drag. * @constructor @@ -35,6 +35,12 @@ Blockly.ConnectionDB = function(checker) { * @private */ this.connections_ = []; + /** + * The workspace's connection type checker, used to decide if connections are + * valid during a drag. + * @type {!Blockly.IConnectionChecker} + * @private + */ this.connectionChecker_ = checker; }; @@ -279,7 +285,7 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, /** * Initialize a set of connection DBs for a workspace. - * @param {!Blockly.ConnectionChecker} checker The workspace's + * @param {!Blockly.IConnectionChecker} checker The workspace's * connection checker, used to decide if connections are valid during a drag. * @return {!Array.} Array of databases. */ diff --git a/core/interfaces/i_connection_checker.js b/core/interfaces/i_connection_checker.js new file mode 100644 index 000000000..f2987f5aa --- /dev/null +++ b/core/interfaces/i_connection_checker.js @@ -0,0 +1,87 @@ +/** + * @license + * Copyright 2020 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * @fileoverview The interface for an object that encapsulates logic for + * checking whether a potential connection is safe and valid. + * @author fenichel@google.com (Rachel Fenichel) + */ +'use strict'; + +goog.provide('Blockly.IConnectionChecker'); + +goog.requireType('Blockly.Connection'); + +/** + * Class for connection type checking logic. + * @interface + */ +Blockly.IConnectionChecker = function() {}; + +/** + * Check whether the current connection can connect with the target + * connection. + * @param {Blockly.Connection} one Connection to check compatibility with. + * @param {Blockly.Connection} two Connection to check compatibility with. + * @param {boolean} isDragging True if the connection is being made by dragging + * a block. + * @return {boolean} Whether the connection is legal. + * @public + */ +Blockly.IConnectionChecker.prototype.canConnect; + +/** + * Checks whether the current connection can connect with the target + * connection, and return an error code if there are problems. + * @param {Blockly.Connection} one Connection to check compatibility with. + * @param {Blockly.Connection} two Connection to check compatibility with. + * @param {boolean} isDragging [description] + * @return {number} Blockly.Connection.CAN_CONNECT if the connection is legal, + * an error code otherwise. + * @public + */ +Blockly.IConnectionChecker.prototype.canConnectWithReason; + +/** + * Helper method that translates a connection error code into a string. + * @param {number} errorCode The error code. + * @param {Blockly.Connection} one One of the two connections being checked. + * @param {Blockly.Connection} two The second of the two connections being + * checked. + * @return {string} A developer-readable error string. + * @public + */ +Blockly.IConnectionChecker.prototype.getErrorMessage; + +/** + * Check that connecting the given connections is safe, meaning that it would + * not break any of Blockly's basic assumptions (e.g. no self connections). + * @param {Blockly.Connection} one The first of the connections to check. + * @param {Blockly.Connection} two The second of the connections to check. + * @return {number} An enum with the reason this connection is safe or unsafe. + * @public + */ +Blockly.IConnectionChecker.prototype.doSafetyChecks; + +/** + * Check whether this connection is compatible with another connection with + * respect to the value type system. E.g. square_root("Hello") is not + * compatible. + * @param {!Blockly.Connection} one Connection to compare. + * @param {!Blockly.Connection} two Connection to compare against. + * @return {boolean} True if the connections share a type. + * @public + */ +Blockly.IConnectionChecker.prototype.doTypeChecks; + +/** + * Check whether this connection can be made by dragging. + * @param {!Blockly.Connection} one Connection to compare. + * @param {!Blockly.Connection} two Connection to compare against. + * @return {boolean} True if the connections share a type. + * @public + */ +Blockly.IConnectionChecker.prototype.doDragChecks; diff --git a/core/keyboard_nav/navigation.js b/core/keyboard_nav/navigation.js index fcf9fdfa8..c162e282e 100644 --- a/core/keyboard_nav/navigation.js +++ b/core/keyboard_nav/navigation.js @@ -504,7 +504,7 @@ Blockly.navigation.connect_ = function(movingConnection, destConnection) { var reason = checker.canConnectWithReason( movingConnection, destConnection, false); Blockly.navigation.warn_('Connection failed with error: ' + - checker.getErrorMessage(movingConnection, destConnection, reason)); + checker.getErrorMessage(reason, movingConnection, destConnection)); return false; } }; diff --git a/core/rendered_connection.js b/core/rendered_connection.js index 0046446b3..5ceb347c4 100644 --- a/core/rendered_connection.js +++ b/core/rendered_connection.js @@ -19,8 +19,6 @@ goog.require('Blockly.utils.Coordinate'); goog.require('Blockly.utils.dom'); goog.require('Blockly.utils.object'); -goog.requireType('Blockly.ConnectionChecker'); - /** * Class for a connection between blocks that may be rendered on screen. diff --git a/core/workspace.js b/core/workspace.js index b79ab340c..3f3d0ceff 100644 --- a/core/workspace.js +++ b/core/workspace.js @@ -20,6 +20,7 @@ goog.require('Blockly.utils.math'); goog.require('Blockly.VariableMap'); goog.requireType('Blockly.IASTNodeLocation'); +goog.requireType('Blockly.IConnectionChecker'); /** @@ -45,7 +46,7 @@ Blockly.Workspace = function(opt_options) { /** * An object that encapsulates logic for safety, type, and dragging checks. - * @type {Blockly} + * @type {!Blockly.IConnectionChecker} */ this.connectionChecker = new Blockly.ConnectionChecker(); From e47d33148ef7a083809023c0c080c611da49718b Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 16 Jul 2020 14:36:39 -0600 Subject: [PATCH 11/18] Review feedback --- core/connection.js | 2 +- core/connection_checker.js | 6 ++++-- core/interfaces/i_connection_checker.js | 4 +++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/core/connection.js b/core/connection.js index dfbc0ac3a..8914c56f9 100644 --- a/core/connection.js +++ b/core/connection.js @@ -16,8 +16,8 @@ goog.require('Blockly.Events'); goog.require('Blockly.Events.BlockMove'); goog.require('Blockly.Xml'); -goog.requireType('Blockly.IConnectionChecker'); goog.requireType('Blockly.IASTNodeLocationWithBlock'); +goog.requireType('Blockly.IConnectionChecker'); /** diff --git a/core/connection_checker.js b/core/connection_checker.js index cb04e1e40..d880fc8a5 100644 --- a/core/connection_checker.js +++ b/core/connection_checker.js @@ -13,8 +13,9 @@ goog.provide('Blockly.ConnectionChecker'); -goog.requireType('Blockly.IConnectionChecker'); goog.requireType('Blockly.Connection'); +goog.requireType('Blockly.IConnectionChecker'); + /** * Class for connection type checking logic. @@ -45,7 +46,8 @@ Blockly.ConnectionChecker.prototype.canConnect = function(one, two, * connection, and return an error code if there are problems. * @param {Blockly.Connection} one Connection to check compatibility with. * @param {Blockly.Connection} two Connection to check compatibility with. - * @param {boolean} isDragging [description] + * @param {boolean} isDragging True if the connection is being made by dragging + * a block. * @return {number} Blockly.Connection.CAN_CONNECT if the connection is legal, * an error code otherwise. * @public diff --git a/core/interfaces/i_connection_checker.js b/core/interfaces/i_connection_checker.js index f2987f5aa..8bb5322bc 100644 --- a/core/interfaces/i_connection_checker.js +++ b/core/interfaces/i_connection_checker.js @@ -15,6 +15,7 @@ goog.provide('Blockly.IConnectionChecker'); goog.requireType('Blockly.Connection'); + /** * Class for connection type checking logic. * @interface @@ -38,7 +39,8 @@ Blockly.IConnectionChecker.prototype.canConnect; * connection, and return an error code if there are problems. * @param {Blockly.Connection} one Connection to check compatibility with. * @param {Blockly.Connection} two Connection to check compatibility with. - * @param {boolean} isDragging [description] + * @param {boolean} isDragging True if the connection is being made by dragging + * a block. * @return {number} Blockly.Connection.CAN_CONNECT if the connection is legal, * an error code otherwise. * @public From 745017f325c77e63640b63eac2a38605c5d499f6 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 16 Jul 2020 14:41:41 -0600 Subject: [PATCH 12/18] Move from one, two to a, b --- core/connection_checker.js | 108 ++++++++++++------------ core/interfaces/i_connection_checker.js | 24 +++--- 2 files changed, 66 insertions(+), 66 deletions(-) diff --git a/core/connection_checker.js b/core/connection_checker.js index d880fc8a5..15e044152 100644 --- a/core/connection_checker.js +++ b/core/connection_checker.js @@ -28,24 +28,24 @@ Blockly.ConnectionChecker = function() { /** * Check whether the current connection can connect with the target * connection. - * @param {Blockly.Connection} one Connection to check compatibility with. - * @param {Blockly.Connection} two Connection to check compatibility with. + * @param {Blockly.Connection} a Connection to check compatibility with. + * @param {Blockly.Connection} b Connection to check compatibility with. * @param {boolean} isDragging True if the connection is being made by dragging * a block. * @return {boolean} Whether the connection is legal. * @public */ -Blockly.ConnectionChecker.prototype.canConnect = function(one, two, +Blockly.ConnectionChecker.prototype.canConnect = function(a, b, isDragging) { - return this.canConnectWithReason(one, two, isDragging) == + return this.canConnectWithReason(a, b, isDragging) == Blockly.Connection.CAN_CONNECT; }; /** * Checks whether the current connection can connect with the target * connection, and return an error code if there are problems. - * @param {Blockly.Connection} one Connection to check compatibility with. - * @param {Blockly.Connection} two Connection to check compatibility with. + * @param {Blockly.Connection} a Connection to check compatibility with. + * @param {Blockly.Connection} b Connection to check compatibility with. * @param {boolean} isDragging True if the connection is being made by dragging * a block. * @return {number} Blockly.Connection.CAN_CONNECT if the connection is legal, @@ -53,15 +53,15 @@ Blockly.ConnectionChecker.prototype.canConnect = function(one, two, * @public */ Blockly.ConnectionChecker.prototype.canConnectWithReason = function( - one, two, isDragging) { - var safety = this.doSafetyChecks(one, two); + a, b, isDragging) { + var safety = this.doSafetyChecks(a, b); if (safety != Blockly.Connection.CAN_CONNECT) { return safety; } // If the safety checks passed, both connections are non-null. - var connOne = /** @type {!Blockly.Connection} **/ (one); - var connTwo = /** @type {!Blockly.Connection} **/ (two); + var connOne = /** @type {!Blockly.Connection} **/ (a); + var connTwo = /** @type {!Blockly.Connection} **/ (b); if (!this.doTypeChecks(connOne, connTwo)) { return Blockly.Connection.REASON_CHECKS_FAILED; } @@ -76,14 +76,14 @@ Blockly.ConnectionChecker.prototype.canConnectWithReason = function( /** * Helper method that translates a connection error code into a string. * @param {number} errorCode The error code. - * @param {Blockly.Connection} one One of the two connections being checked. - * @param {Blockly.Connection} two The second of the two connections being + * @param {Blockly.Connection} a One of the two connections being checked. + * @param {Blockly.Connection} b The second of the two connections being * checked. * @return {string} A developer-readable error string. * @public */ Blockly.ConnectionChecker.prototype.getErrorMessage = function(errorCode, - one, two) { + a, b) { switch (errorCode) { case Blockly.Connection.REASON_SELF_CONNECTION: return 'Attempted to connect a block to itself.'; @@ -95,8 +95,8 @@ Blockly.ConnectionChecker.prototype.getErrorMessage = function(errorCode, case Blockly.Connection.REASON_TARGET_NULL: return 'Target connection is null.'; case Blockly.Connection.REASON_CHECKS_FAILED: - var connOne = /** @type {!Blockly.Connection} **/ (one); - var connTwo = /** @type {!Blockly.Connection} **/ (two); + var connOne = /** @type {!Blockly.Connection} **/ (a); + var connTwo = /** @type {!Blockly.Connection} **/ (b); var msg = 'Connection checks failed. '; msg += connOne + ' expected ' + connOne.getCheck() + ', found ' + connTwo.getCheck(); return msg; @@ -112,25 +112,25 @@ Blockly.ConnectionChecker.prototype.getErrorMessage = function(errorCode, /** * Check that connecting the given connections is safe, meaning that it would * not break any of Blockly's basic assumptions (e.g. no self connections). - * @param {Blockly.Connection} one The first of the connections to check. - * @param {Blockly.Connection} two The second of the connections to check. + * @param {Blockly.Connection} a The first of the connections to check. + * @param {Blockly.Connection} b The second of the connections to check. * @return {number} An enum with the reason this connection is safe or unsafe. * @public */ -Blockly.ConnectionChecker.prototype.doSafetyChecks = function(one, two) { - if (!one || !two) { +Blockly.ConnectionChecker.prototype.doSafetyChecks = function(a, b) { + if (!a || !b) { return Blockly.Connection.REASON_TARGET_NULL; } - if (one.isSuperior()) { - var blockA = one.getSourceBlock(); - var blockB = two.getSourceBlock(); + if (a.isSuperior()) { + var blockA = a.getSourceBlock(); + var blockB = b.getSourceBlock(); } else { - var blockB = one.getSourceBlock(); - var blockA = two.getSourceBlock(); + var blockB = a.getSourceBlock(); + var blockA = b.getSourceBlock(); } if (blockA == blockB) { return Blockly.Connection.REASON_SELF_CONNECTION; - } else if (two.type != Blockly.OPPOSITE_TYPE[one.type]) { + } else if (b.type != Blockly.OPPOSITE_TYPE[a.type]) { return Blockly.Connection.REASON_WRONG_TYPE; } else if (blockA.workspace !== blockB.workspace) { return Blockly.Connection.REASON_DIFFERENT_WORKSPACES; @@ -144,14 +144,14 @@ Blockly.ConnectionChecker.prototype.doSafetyChecks = function(one, two) { * Check whether this connection is compatible with another connection with * respect to the value type system. E.g. square_root("Hello") is not * compatible. - * @param {!Blockly.Connection} one Connection to compare. - * @param {!Blockly.Connection} two Connection to compare against. + * @param {!Blockly.Connection} a Connection to compare. + * @param {!Blockly.Connection} b Connection to compare against. * @return {boolean} True if the connections share a type. * @public */ -Blockly.ConnectionChecker.prototype.doTypeChecks = function(one, two) { - var checkArrayOne = one.getCheck(); - var checkArrayTwo = two.getCheck(); +Blockly.ConnectionChecker.prototype.doTypeChecks = function(a, b) { + var checkArrayOne = a.getCheck(); + var checkArrayTwo = b.getCheck(); if (!checkArrayOne || !checkArrayTwo) { // One or both sides are promiscuous enough that anything will fit. @@ -169,26 +169,26 @@ Blockly.ConnectionChecker.prototype.doTypeChecks = function(one, two) { /** * Check whether this connection can be made by dragging. - * @param {!Blockly.Connection} one Connection to compare. - * @param {!Blockly.Connection} two Connection to compare against. + * @param {!Blockly.Connection} a Connection to compare. + * @param {!Blockly.Connection} b Connection to compare against. * @return {boolean} True if the connections share a type. * @public */ -Blockly.ConnectionChecker.prototype.doDragChecks = function(one, two) { +Blockly.ConnectionChecker.prototype.doDragChecks = function(a, b) { // Don't consider insertion markers. - if (two.getSourceBlock().isInsertionMarker()) { + if (b.getSourceBlock().isInsertionMarker()) { return false; } - switch (two.type) { + switch (b.type) { case Blockly.PREVIOUS_STATEMENT: - return this.canConnectToPrevious_(one, two); + return this.canConnectToPrevious_(a, b); case Blockly.OUTPUT_VALUE: { // Don't offer to connect an already connected left (male) value plug to // an available right (female) value plug. - if ((two.isConnected() && - !two.targetBlock().isInsertionMarker()) || - one.isConnected()) { + if ((b.isConnected() && + !b.targetBlock().isInsertionMarker()) || + a.isConnected()) { return false; } break; @@ -197,9 +197,9 @@ Blockly.ConnectionChecker.prototype.doDragChecks = function(one, two) { // Offering to connect the left (male) of a value block to an already // connected value pair is ok, we'll splice it in. // However, don't offer to splice into an immovable block. - if (two.isConnected() && - !two.targetBlock().isMovable() && - !two.targetBlock().isShadow()) { + if (b.isConnected() && + !b.targetBlock().isMovable() && + !b.targetBlock().isShadow()) { return false; } break; @@ -209,10 +209,10 @@ Blockly.ConnectionChecker.prototype.doDragChecks = function(one, two) { // stack. But covering up a shadow block or stack of shadow blocks is // fine. Similarly, replacing a terminal statement with another terminal // statement is allowed. - if (two.isConnected() && - !one.getSourceBlock().nextConnection && - !two.targetBlock().isShadow() && - two.targetBlock().nextConnection) { + if (b.isConnected() && + !a.getSourceBlock().nextConnection && + !b.targetBlock().isShadow() && + b.targetBlock().nextConnection) { return false; } break; @@ -223,7 +223,7 @@ Blockly.ConnectionChecker.prototype.doDragChecks = function(one, two) { } // Don't let blocks try to connect to themselves or ones they nest. - if (Blockly.draggingConnections.indexOf(two) != -1) { + if (Blockly.draggingConnections.indexOf(b) != -1) { return false; } @@ -232,30 +232,30 @@ Blockly.ConnectionChecker.prototype.doDragChecks = function(one, two) { /** * Helper function for drag checking. - * @param {!Blockly.Connection} one The connection to check, which must be a + * @param {!Blockly.Connection} a The connection to check, which must be a * statement input or next connection. - * @param {!Blockly.Connection} two A nearby connection to check, which + * @param {!Blockly.Connection} b A nearby connection to check, which * must be a previous connection. * @return {boolean} True if the connection is allowed, false otherwise. * @protected */ -Blockly.ConnectionChecker.prototype.canConnectToPrevious_ = function(one, two) { - if (one.targetConnection) { +Blockly.ConnectionChecker.prototype.canConnectToPrevious_ = function(a, b) { + if (a.targetConnection) { // This connection is already occupied. // A next connection will never disconnect itself mid-drag. return false; } // Don't let blocks try to connect to themselves or ones they nest. - if (Blockly.draggingConnections.indexOf(two) != -1) { + if (Blockly.draggingConnections.indexOf(b) != -1) { return false; } - if (!two.targetConnection) { + if (!b.targetConnection) { return true; } - var targetBlock = two.targetBlock(); + var targetBlock = b.targetBlock(); // If it is connected to a real block, game over. if (!targetBlock.isInsertionMarker()) { return false; diff --git a/core/interfaces/i_connection_checker.js b/core/interfaces/i_connection_checker.js index 8bb5322bc..6d386743c 100644 --- a/core/interfaces/i_connection_checker.js +++ b/core/interfaces/i_connection_checker.js @@ -25,8 +25,8 @@ Blockly.IConnectionChecker = function() {}; /** * Check whether the current connection can connect with the target * connection. - * @param {Blockly.Connection} one Connection to check compatibility with. - * @param {Blockly.Connection} two Connection to check compatibility with. + * @param {Blockly.Connection} a Connection to check compatibility with. + * @param {Blockly.Connection} b Connection to check compatibility with. * @param {boolean} isDragging True if the connection is being made by dragging * a block. * @return {boolean} Whether the connection is legal. @@ -37,8 +37,8 @@ Blockly.IConnectionChecker.prototype.canConnect; /** * Checks whether the current connection can connect with the target * connection, and return an error code if there are problems. - * @param {Blockly.Connection} one Connection to check compatibility with. - * @param {Blockly.Connection} two Connection to check compatibility with. + * @param {Blockly.Connection} a Connection to check compatibility with. + * @param {Blockly.Connection} b Connection to check compatibility with. * @param {boolean} isDragging True if the connection is being made by dragging * a block. * @return {number} Blockly.Connection.CAN_CONNECT if the connection is legal, @@ -50,8 +50,8 @@ Blockly.IConnectionChecker.prototype.canConnectWithReason; /** * Helper method that translates a connection error code into a string. * @param {number} errorCode The error code. - * @param {Blockly.Connection} one One of the two connections being checked. - * @param {Blockly.Connection} two The second of the two connections being + * @param {Blockly.Connection} a One of the two connections being checked. + * @param {Blockly.Connection} b The second of the two connections being * checked. * @return {string} A developer-readable error string. * @public @@ -61,8 +61,8 @@ Blockly.IConnectionChecker.prototype.getErrorMessage; /** * Check that connecting the given connections is safe, meaning that it would * not break any of Blockly's basic assumptions (e.g. no self connections). - * @param {Blockly.Connection} one The first of the connections to check. - * @param {Blockly.Connection} two The second of the connections to check. + * @param {Blockly.Connection} a The first of the connections to check. + * @param {Blockly.Connection} b The second of the connections to check. * @return {number} An enum with the reason this connection is safe or unsafe. * @public */ @@ -72,8 +72,8 @@ Blockly.IConnectionChecker.prototype.doSafetyChecks; * Check whether this connection is compatible with another connection with * respect to the value type system. E.g. square_root("Hello") is not * compatible. - * @param {!Blockly.Connection} one Connection to compare. - * @param {!Blockly.Connection} two Connection to compare against. + * @param {!Blockly.Connection} a Connection to compare. + * @param {!Blockly.Connection} b Connection to compare against. * @return {boolean} True if the connections share a type. * @public */ @@ -81,8 +81,8 @@ Blockly.IConnectionChecker.prototype.doTypeChecks; /** * Check whether this connection can be made by dragging. - * @param {!Blockly.Connection} one Connection to compare. - * @param {!Blockly.Connection} two Connection to compare against. + * @param {!Blockly.Connection} a Connection to compare. + * @param {!Blockly.Connection} b Connection to compare against. * @return {boolean} True if the connections share a type. * @public */ From 3322834a9b22822d2dbdbc3cc255e3d2105422b1 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 16 Jul 2020 15:04:59 -0600 Subject: [PATCH 13/18] Add deprecation warnings and tests --- core/connection.js | 15 +- tests/mocha/connection_checker_test.js | 264 +++++++++++++++++++++++ tests/mocha/connection_test.js | 281 +++---------------------- tests/mocha/index.html | 1 + 4 files changed, 302 insertions(+), 259 deletions(-) create mode 100644 tests/mocha/connection_checker_test.js diff --git a/core/connection.js b/core/connection.js index 8914c56f9..19aab3724 100644 --- a/core/connection.js +++ b/core/connection.js @@ -248,10 +248,11 @@ Blockly.Connection.prototype.isConnected = function() { * @param {Blockly.Connection} target Connection to check compatibility with. * @return {number} Blockly.Connection.CAN_CONNECT if the connection is legal, * an error code otherwise. - * @deprecated July 2020 + * @deprecated July 2020. Use the workspace's connectionChecker. */ Blockly.Connection.prototype.canConnectWithReason = function(target) { - // TODO: deprecation warning with date, plus tests. + console.warn('Connection.prototype.canConnectWithReason was deprecated in ' + + 'July 2020 and will be deleted in July 2021.'); return this.getConnectionChecker().canConnectWithReason( this, target, false); }; @@ -262,11 +263,11 @@ Blockly.Connection.prototype.canConnectWithReason = function(target) { * @param {Blockly.Connection} target The connection to check compatibility * with. * @package - * @deprecated July 2020 + * @deprecated July 2020. Use the workspace's connectionChecker. */ Blockly.Connection.prototype.checkConnection = function(target) { -// TODO: Add deprecation warning notices *and* add tests to make sure these -// still work (for any blocks that use them). + console.warn('Connection.prototype.checkConnection was deprecated in ' + + 'July 2020 and will be deleted in July 2021.'); var checker = this.getConnectionChecker(); var reason = checker.canConnectWithReason(this, target, false); if (reason != Blockly.Connection.CAN_CONNECT) { @@ -288,9 +289,11 @@ Blockly.Connection.prototype.getConnectionChecker = function() { * Check if the two connections can be dragged to connect to each other. * @param {!Blockly.Connection} candidate A nearby connection to check. * @return {boolean} True if the connection is allowed, false otherwise. - * @deprecated July 2020 + * @deprecated July 2020. Use the workspace's connectionChecker. */ Blockly.Connection.prototype.isConnectionAllowed = function(candidate) { + console.warn('Connection.prototype.isConnectionAllowed was deprecated in ' + + 'July 2020 and will be deleted in July 2021.'); return this.getConnectionChecker().canConnect(this, candidate, true); }; diff --git a/tests/mocha/connection_checker_test.js b/tests/mocha/connection_checker_test.js new file mode 100644 index 000000000..82528107d --- /dev/null +++ b/tests/mocha/connection_checker_test.js @@ -0,0 +1,264 @@ +/** + * @license + * Copyright 2020 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +suite('Connection checker', function() { + suiteSetup(function() { + this.checker = new Blockly.ConnectionChecker(); + }); + suite('Safety checks', function() { + function assertReasonHelper(checker, one, two, reason) { + chai.assert.equal(checker.canConnectWithReason(one, two), reason); + // Order should not matter. + chai.assert.equal(checker.canConnectWithReason(two, one), reason); + } + + test('Target Null', function() { + var connection = new Blockly.Connection({}, Blockly.INPUT_VALUE); + assertReasonHelper( + this.checker, + connection, + null, + Blockly.Connection.REASON_TARGET_NULL); + }); + test('Target Self', function() { + var block = {workspace: 1}; + var connection1 = new Blockly.Connection(block, Blockly.INPUT_VALUE); + var connection2 = new Blockly.Connection(block, Blockly.OUTPUT_VALUE); + + assertReasonHelper( + this.checker, + connection1, + connection2, + Blockly.Connection.REASON_SELF_CONNECTION); + }); + test('Different Workspaces', function() { + var connection1 = new Blockly.Connection( + {workspace: 1}, Blockly.INPUT_VALUE); + var connection2 = new Blockly.Connection( + {workspace: 2}, Blockly.OUTPUT_VALUE); + + assertReasonHelper( + this.checker, + connection1, + connection2, + Blockly.Connection.REASON_DIFFERENT_WORKSPACES); + }); + suite('Types', function() { + setup(function() { + // We have to declare each separately so that the connections belong + // on different blocks. + var prevBlock = { isShadow: function() {}}; + var nextBlock = { isShadow: function() {}}; + var outBlock = { isShadow: function() {}}; + var inBlock = { isShadow: function() {}}; + this.previous = new Blockly.Connection( + prevBlock, Blockly.PREVIOUS_STATEMENT); + this.next = new Blockly.Connection( + nextBlock, Blockly.NEXT_STATEMENT); + this.output = new Blockly.Connection( + outBlock, Blockly.OUTPUT_VALUE); + this.input = new Blockly.Connection( + inBlock, Blockly.INPUT_VALUE); + }); + test('Previous, Next', function() { + assertReasonHelper( + this.checker, + this.previous, + this.next, + Blockly.Connection.CAN_CONNECT); + }); + test('Previous, Output', function() { + assertReasonHelper( + this.checker, + this.previous, + this.output, + Blockly.Connection.REASON_WRONG_TYPE); + }); + test('Previous, Input', function() { + assertReasonHelper( + this.checker, + this.previous, + this.input, + Blockly.Connection.REASON_WRONG_TYPE); + }); + test('Next, Previous', function() { + assertReasonHelper( + this.checker, + this.next, + this.previous, + Blockly.Connection.CAN_CONNECT); + }); + test('Next, Output', function() { + assertReasonHelper( + this.checker, + this.next, + this.output, + Blockly.Connection.REASON_WRONG_TYPE); + }); + test('Next, Input', function() { + assertReasonHelper( + this.checker, + this.next, + this.input, + Blockly.Connection.REASON_WRONG_TYPE); + }); + test('Output, Previous', function() { + assertReasonHelper( + this.checker, + this.previous, + this.output, + Blockly.Connection.REASON_WRONG_TYPE); + }); + test('Output, Next', function() { + assertReasonHelper( + this.checker, + this.output, + this.next, + Blockly.Connection.REASON_WRONG_TYPE); + }); + test('Output, Input', function() { + assertReasonHelper( + this.checker, + this.output, + this.input, + Blockly.Connection.CAN_CONNECT); + }); + test('Input, Previous', function() { + assertReasonHelper( + this.checker, + this.previous, + this.input, + Blockly.Connection.REASON_WRONG_TYPE); + }); + test('Input, Next', function() { + assertReasonHelper( + this.checker, + this.input, + this.next, + Blockly.Connection.REASON_WRONG_TYPE); + }); + test('Input, Output', function() { + assertReasonHelper( + this.checker, + this.input, + this.output, + Blockly.Connection.CAN_CONNECT); + }); + }); + suite('Shadows', function() { + test('Previous Shadow', function() { + var prevBlock = { isShadow: function() { return true; }}; + var nextBlock = { isShadow: function() { return false; }}; + var prev = new Blockly.Connection(prevBlock, Blockly.PREVIOUS_STATEMENT); + var next = new Blockly.Connection(nextBlock, Blockly.NEXT_STATEMENT); + + assertReasonHelper( + this.checker, + prev, + next, + Blockly.Connection.CAN_CONNECT); + }); + test('Next Shadow', function() { + var prevBlock = { isShadow: function() { return false; }}; + var nextBlock = { isShadow: function() { return true; }}; + var prev = new Blockly.Connection(prevBlock, Blockly.PREVIOUS_STATEMENT); + var next = new Blockly.Connection(nextBlock, Blockly.NEXT_STATEMENT); + + assertReasonHelper( + this.checker, + prev, + next, + Blockly.Connection.REASON_SHADOW_PARENT); + }); + test('Prev and Next Shadow', function() { + var prevBlock = { isShadow: function() { return true; }}; + var nextBlock = { isShadow: function() { return true; }}; + var prev = new Blockly.Connection(prevBlock, Blockly.PREVIOUS_STATEMENT); + var next = new Blockly.Connection(nextBlock, Blockly.NEXT_STATEMENT); + + assertReasonHelper( + this.checker, + prev, + next, + Blockly.Connection.CAN_CONNECT); + }); + test('Output Shadow', function() { + var outBlock = { isShadow: function() { return true; }}; + var inBlock = { isShadow: function() { return false; }}; + var outCon = new Blockly.Connection(outBlock, Blockly.OUTPUT_VALUE); + var inCon = new Blockly.Connection(inBlock, Blockly.INPUT_VALUE); + + assertReasonHelper( + this.checker, + outCon, + inCon, + Blockly.Connection.CAN_CONNECT); + }); + test('Input Shadow', function() { + var outBlock = { isShadow: function() { return false; }}; + var inBlock = { isShadow: function() { return true; }}; + var outCon = new Blockly.Connection(outBlock, Blockly.OUTPUT_VALUE); + var inCon = new Blockly.Connection(inBlock, Blockly.INPUT_VALUE); + + assertReasonHelper( + this.checker, + outCon, + inCon, + Blockly.Connection.REASON_SHADOW_PARENT); + }); + test('Output and Input Shadow', function() { + var outBlock = { isShadow: function() { return true; }}; + var inBlock = { isShadow: function() { return true; }}; + var outCon = new Blockly.Connection(outBlock, Blockly.OUTPUT_VALUE); + var inCon = new Blockly.Connection(inBlock, Blockly.INPUT_VALUE); + + assertReasonHelper( + this.checker, + outCon, + inCon, + Blockly.Connection.CAN_CONNECT); + }); + }); + }); + suite('Check Types', function() { + setup(function() { + this.con1 = new Blockly.Connection({}, Blockly.PREVIOUS_STATEMENT); + this.con2 = new Blockly.Connection({}, Blockly.NEXT_STATEMENT); + }); + function assertCheckTypes(checker, one, two) { + chai.assert.isTrue(checker.doTypeChecks(one, two)); + // Order should not matter. + chai.assert.isTrue(checker.doTypeChecks(one, two)); + } + test('No Types', function() { + assertCheckTypes(this.checker, this.con1, this.con2); + }); + test('Same Type', function() { + this.con1.setCheck('type1'); + this.con2.setCheck('type1'); + assertCheckTypes(this.checker, this.con1, this.con2); + }); + test('Same Types', function() { + this.con1.setCheck(['type1', 'type2']); + this.con2.setCheck(['type1', 'type2']); + assertCheckTypes(this.checker, this.con1, this.con2); + }); + test('Single Same Type', function() { + this.con1.setCheck(['type1', 'type2']); + this.con2.setCheck(['type1', 'type3']); + assertCheckTypes(this.checker, this.con1, this.con2); + }); + test('One Typed, One Promiscuous', function() { + this.con1.setCheck('type1'); + assertCheckTypes(this.checker, this.con1, this.con2); + }); + test('No Compatible Types', function() { + this.con1.setCheck('type1'); + this.con2.setCheck('type2'); + chai.assert.isFalse(this.checker.doTypeChecks(this.con1, this.con2)); + }); + }); +}); diff --git a/tests/mocha/connection_test.js b/tests/mocha/connection_test.js index 4f058126b..a55fcc34e 100644 --- a/tests/mocha/connection_test.js +++ b/tests/mocha/connection_test.js @@ -4,261 +4,36 @@ * SPDX-License-Identifier: Apache-2.0 */ -suite('Connection type checker', function() { +suite('Connection', function() { suiteSetup(function() { - this.checker = new Blockly.ConnectionChecker(); + this.workspace = = { + connectionChecker: new Blockly.ConnectionChecker() + }; + this.createConnection = function(type) { + var connection = new Blockly.Connection({workspace: this.workspace}, type); + return connection; + }; }); - suite('Safety checks', function() { - function assertReasonHelper(checker, one, two, reason) { - chai.assert.equal(checker.canConnectWithReason(one, two), reason); - // Order should not matter. - chai.assert.equal(checker.canConnectWithReason(two, one), reason); - } - - test('Target Null', function() { - var connection = new Blockly.Connection({}, Blockly.INPUT_VALUE); - assertReasonHelper( - this.checker, - connection, - null, - Blockly.Connection.REASON_TARGET_NULL); - }); - test('Target Self', function() { - var block = {workspace: 1}; - var connection1 = new Blockly.Connection(block, Blockly.INPUT_VALUE); - var connection2 = new Blockly.Connection(block, Blockly.OUTPUT_VALUE); - - assertReasonHelper( - this.checker, - connection1, - connection2, - Blockly.Connection.REASON_SELF_CONNECTION); - }); - test('Different Workspaces', function() { - var connection1 = new Blockly.Connection( - {workspace: 1}, Blockly.INPUT_VALUE); - var connection2 = new Blockly.Connection( - {workspace: 2}, Blockly.OUTPUT_VALUE); - - assertReasonHelper( - this.checker, - connection1, - connection2, - Blockly.Connection.REASON_DIFFERENT_WORKSPACES); - }); - suite('Types', function() { - setup(function() { - // We have to declare each separately so that the connections belong - // on different blocks. - var prevBlock = { isShadow: function() {}}; - var nextBlock = { isShadow: function() {}}; - var outBlock = { isShadow: function() {}}; - var inBlock = { isShadow: function() {}}; - this.previous = new Blockly.Connection( - prevBlock, Blockly.PREVIOUS_STATEMENT); - this.next = new Blockly.Connection( - nextBlock, Blockly.NEXT_STATEMENT); - this.output = new Blockly.Connection( - outBlock, Blockly.OUTPUT_VALUE); - this.input = new Blockly.Connection( - inBlock, Blockly.INPUT_VALUE); - }); - test('Previous, Next', function() { - assertReasonHelper( - this.checker, - this.previous, - this.next, - Blockly.Connection.CAN_CONNECT); - }); - test('Previous, Output', function() { - assertReasonHelper( - this.checker, - this.previous, - this.output, - Blockly.Connection.REASON_WRONG_TYPE); - }); - test('Previous, Input', function() { - assertReasonHelper( - this.checker, - this.previous, - this.input, - Blockly.Connection.REASON_WRONG_TYPE); - }); - test('Next, Previous', function() { - assertReasonHelper( - this.checker, - this.next, - this.previous, - Blockly.Connection.CAN_CONNECT); - }); - test('Next, Output', function() { - assertReasonHelper( - this.checker, - this.next, - this.output, - Blockly.Connection.REASON_WRONG_TYPE); - }); - test('Next, Input', function() { - assertReasonHelper( - this.checker, - this.next, - this.input, - Blockly.Connection.REASON_WRONG_TYPE); - }); - test('Output, Previous', function() { - assertReasonHelper( - this.checker, - this.previous, - this.output, - Blockly.Connection.REASON_WRONG_TYPE); - }); - test('Output, Next', function() { - assertReasonHelper( - this.checker, - this.output, - this.next, - Blockly.Connection.REASON_WRONG_TYPE); - }); - test('Output, Input', function() { - assertReasonHelper( - this.checker, - this.output, - this.input, - Blockly.Connection.CAN_CONNECT); - }); - test('Input, Previous', function() { - assertReasonHelper( - this.checker, - this.previous, - this.input, - Blockly.Connection.REASON_WRONG_TYPE); - }); - test('Input, Next', function() { - assertReasonHelper( - this.checker, - this.input, - this.next, - Blockly.Connection.REASON_WRONG_TYPE); - }); - test('Input, Output', function() { - assertReasonHelper( - this.checker, - this.input, - this.output, - Blockly.Connection.CAN_CONNECT); - }); - }); - suite('Shadows', function() { - test('Previous Shadow', function() { - var prevBlock = { isShadow: function() { return true; }}; - var nextBlock = { isShadow: function() { return false; }}; - var prev = new Blockly.Connection(prevBlock, Blockly.PREVIOUS_STATEMENT); - var next = new Blockly.Connection(nextBlock, Blockly.NEXT_STATEMENT); - - assertReasonHelper( - this.checker, - prev, - next, - Blockly.Connection.CAN_CONNECT); - }); - test('Next Shadow', function() { - var prevBlock = { isShadow: function() { return false; }}; - var nextBlock = { isShadow: function() { return true; }}; - var prev = new Blockly.Connection(prevBlock, Blockly.PREVIOUS_STATEMENT); - var next = new Blockly.Connection(nextBlock, Blockly.NEXT_STATEMENT); - - assertReasonHelper( - this.checker, - prev, - next, - Blockly.Connection.REASON_SHADOW_PARENT); - }); - test('Prev and Next Shadow', function() { - var prevBlock = { isShadow: function() { return true; }}; - var nextBlock = { isShadow: function() { return true; }}; - var prev = new Blockly.Connection(prevBlock, Blockly.PREVIOUS_STATEMENT); - var next = new Blockly.Connection(nextBlock, Blockly.NEXT_STATEMENT); - - assertReasonHelper( - this.checker, - prev, - next, - Blockly.Connection.CAN_CONNECT); - }); - test('Output Shadow', function() { - var outBlock = { isShadow: function() { return true; }}; - var inBlock = { isShadow: function() { return false; }}; - var outCon = new Blockly.Connection(outBlock, Blockly.OUTPUT_VALUE); - var inCon = new Blockly.Connection(inBlock, Blockly.INPUT_VALUE); - - assertReasonHelper( - this.checker, - outCon, - inCon, - Blockly.Connection.CAN_CONNECT); - }); - test('Input Shadow', function() { - var outBlock = { isShadow: function() { return false; }}; - var inBlock = { isShadow: function() { return true; }}; - var outCon = new Blockly.Connection(outBlock, Blockly.OUTPUT_VALUE); - var inCon = new Blockly.Connection(inBlock, Blockly.INPUT_VALUE); - - assertReasonHelper( - this.checker, - outCon, - inCon, - Blockly.Connection.REASON_SHADOW_PARENT); - }); - test('Output and Input Shadow', function() { - var outBlock = { isShadow: function() { return true; }}; - var inBlock = { isShadow: function() { return true; }}; - var outCon = new Blockly.Connection(outBlock, Blockly.OUTPUT_VALUE); - var inCon = new Blockly.Connection(inBlock, Blockly.INPUT_VALUE); - - assertReasonHelper( - this.checker, - outCon, - inCon, - Blockly.Connection.CAN_CONNECT); - }); - }); + test('canConnectWithReason passes', function() { + var conn1 = this.createConnection(Blockly.PREVIOUS_STATEMENT); + var conn2 = this.createConnection(Blockly.NEXT_STATEMENT); + chai.assert.equal(conn1.canConnectWithReason(conn2), + Blockly.Connection.CAN_CONNECT); }); - suite('Check Types', function() { - setup(function() { - this.con1 = new Blockly.Connection({}, Blockly.PREVIOUS_STATEMENT); - this.con2 = new Blockly.Connection({}, Blockly.NEXT_STATEMENT); - }); - function assertCheckTypes(checker, one, two) { - chai.assert.isTrue(checker.doTypeChecks(one, two)); - // Order should not matter. - chai.assert.isTrue(checker.doTypeChecks(one, two)); - } - test('No Types', function() { - assertCheckTypes(this.checker, this.con1, this.con2); - }); - test('Same Type', function() { - this.con1.setCheck('type1'); - this.con2.setCheck('type1'); - assertCheckTypes(this.checker, this.con1, this.con2); - }); - test('Same Types', function() { - this.con1.setCheck(['type1', 'type2']); - this.con2.setCheck(['type1', 'type2']); - assertCheckTypes(this.checker, this.con1, this.con2); - }); - test('Single Same Type', function() { - this.con1.setCheck(['type1', 'type2']); - this.con2.setCheck(['type1', 'type3']); - assertCheckTypes(this.checker, this.con1, this.con2); - }); - test('One Typed, One Promiscuous', function() { - this.con1.setCheck('type1'); - assertCheckTypes(this.checker, this.con1, this.con2); - }); - test('No Compatible Types', function() { - this.con1.setCheck('type1'); - this.con2.setCheck('type2'); - chai.assert.isFalse(this.checker.doTypeChecks(this.con1, this.con2)); - }); + test('canConnectWithReason fails', function() { + var conn1 = this.createConnection(Blockly.PREVIOUS_STATEMENT); + var conn2 = this.createConnection(Blockly.OUTPUT_VALUE); + chai.assert.equal(conn1.canConnectWithReason(conn2), + Blockly.Connection.REASON_WRONG_TYPE); + }); + test('checkConnection passes', function() { + var conn1 = this.createConnection(Blockly.PREVIOUS_STATEMENT); + var conn2 = this.createConnection(Blockly.NEXT_STATEMENT); + chai.assert.doesNotThrow(conn1.checkConnection(conn2)); + }); + test('checkConnection fails', function() { + var conn1 = this.createConnection(Blockly.PREVIOUS_STATEMENT); + var conn2 = this.createConnection(Blockly.OUTPUT_VALUE); + chai.assert.throws(conn1.checkConnection(conn2)); }); }); diff --git a/tests/mocha/index.html b/tests/mocha/index.html index 9c81f3889..e4391e0c7 100644 --- a/tests/mocha/index.html +++ b/tests/mocha/index.html @@ -40,6 +40,7 @@ + From bc0f54b769563f688e35eb95bd95ed33d36cd44d Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 16 Jul 2020 16:49:56 -0600 Subject: [PATCH 14/18] Fix tests --- tests/mocha/connection_test.js | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/mocha/connection_test.js b/tests/mocha/connection_test.js index a55fcc34e..6472cd41b 100644 --- a/tests/mocha/connection_test.js +++ b/tests/mocha/connection_test.js @@ -6,11 +6,15 @@ suite('Connection', function() { suiteSetup(function() { - this.workspace = = { - connectionChecker: new Blockly.ConnectionChecker() - }; + this.workspace = { + connectionChecker: new Blockly.ConnectionChecker() + }; this.createConnection = function(type) { - var connection = new Blockly.Connection({workspace: this.workspace}, type); + var block = { + workspace: this.workspace, + isShadow: function() { return false; } + }; + var connection = new Blockly.Connection(block, type); return connection; }; }); @@ -29,11 +33,15 @@ suite('Connection', function() { test('checkConnection passes', function() { var conn1 = this.createConnection(Blockly.PREVIOUS_STATEMENT); var conn2 = this.createConnection(Blockly.NEXT_STATEMENT); - chai.assert.doesNotThrow(conn1.checkConnection(conn2)); + chai.assert.doesNotThrow(function() { + conn1.checkConnection(conn2); + }); }); test('checkConnection fails', function() { var conn1 = this.createConnection(Blockly.PREVIOUS_STATEMENT); var conn2 = this.createConnection(Blockly.OUTPUT_VALUE); - chai.assert.throws(conn1.checkConnection(conn2)); + chai.assert.throws(function() { + conn1.checkConnection(conn2); + }); }); }); From 1700efc77b1d2eaa71032e229b77761b9e0efd3a Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 16 Jul 2020 17:35:26 -0600 Subject: [PATCH 15/18] Add distance parameter to canConnect --- core/connection_checker.js | 29 ++++++++++++++++++------- core/connection_db.js | 13 ++++------- core/interfaces/i_connection_checker.js | 11 +++++++--- tests/mocha/connection_db_test.js | 26 +++++++++++++++------- 4 files changed, 51 insertions(+), 28 deletions(-) diff --git a/core/connection_checker.js b/core/connection_checker.js index 15e044152..d05de79e4 100644 --- a/core/connection_checker.js +++ b/core/connection_checker.js @@ -32,12 +32,14 @@ Blockly.ConnectionChecker = function() { * @param {Blockly.Connection} b Connection to check compatibility with. * @param {boolean} isDragging True if the connection is being made by dragging * a block. + * @param {number=} opt_distance The max allowable distance between the + * connections for drag checks. * @return {boolean} Whether the connection is legal. * @public */ Blockly.ConnectionChecker.prototype.canConnect = function(a, b, - isDragging) { - return this.canConnectWithReason(a, b, isDragging) == + isDragging, opt_distance) { + return this.canConnectWithReason(a, b, isDragging, opt_distance) == Blockly.Connection.CAN_CONNECT; }; @@ -48,12 +50,14 @@ Blockly.ConnectionChecker.prototype.canConnect = function(a, b, * @param {Blockly.Connection} b Connection to check compatibility with. * @param {boolean} isDragging True if the connection is being made by dragging * a block. + * @param {number=} opt_distance The max allowable distance between the + * connections for drag checks. * @return {number} Blockly.Connection.CAN_CONNECT if the connection is legal, * an error code otherwise. * @public */ Blockly.ConnectionChecker.prototype.canConnectWithReason = function( - a, b, isDragging) { + a, b, isDragging, opt_distance) { var safety = this.doSafetyChecks(a, b); if (safety != Blockly.Connection.CAN_CONNECT) { return safety; @@ -66,7 +70,11 @@ Blockly.ConnectionChecker.prototype.canConnectWithReason = function( return Blockly.Connection.REASON_CHECKS_FAILED; } - if (isDragging && this.doDragChecks(connOne, connTwo)) { + if (isDragging && + !this.doDragChecks( + /** @type {!Blockly.RenderedConnection} **/ (a), + /** @type {!Blockly.RenderedConnection} **/ (b), + opt_distance || 0)) { return Blockly.Connection.REASON_DRAG_CHECKS_FAILED; } @@ -169,12 +177,17 @@ Blockly.ConnectionChecker.prototype.doTypeChecks = function(a, b) { /** * Check whether this connection can be made by dragging. - * @param {!Blockly.Connection} a Connection to compare. - * @param {!Blockly.Connection} b Connection to compare against. - * @return {boolean} True if the connections share a type. + * @param {!Blockly.RenderedConnection} a Connection to compare. + * @param {!Blockly.RenderedConnection} b Connection to compare against. + * @param {number} distance The maximum allowable distance between connections. + * @return {boolean} True if the connection is allowed during a drag. * @public */ -Blockly.ConnectionChecker.prototype.doDragChecks = function(a, b) { +Blockly.ConnectionChecker.prototype.doDragChecks = function(a, b, distance) { + if (a.distanceFrom(b) > distance) { + return false; + } + // Don't consider insertion markers. if (b.getSourceBlock().isInsertionMarker()) { return false; diff --git a/core/connection_db.js b/core/connection_db.js index 2026678d2..d347ddbe0 100644 --- a/core/connection_db.js +++ b/core/connection_db.js @@ -247,17 +247,14 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, var bestConnection = null; var bestRadius = maxRadius; var temp; - var curDistance; // Walk forward and back on the y axis looking for the closest x,y point. var pointerMin = closestIndex - 1; while (pointerMin >= 0 && this.isInYRange_(pointerMin, conn.y, maxRadius)) { temp = this.connections_[pointerMin]; - curDistance = temp.distanceFrom(conn); - if (curDistance <= bestRadius && - this.connectionChecker_.canConnect(conn, temp, true)) { + if (this.connectionChecker_.canConnect(conn, temp, true, bestRadius)) { bestConnection = temp; - bestRadius = curDistance; + bestRadius = temp.distanceFrom(conn); } pointerMin--; } @@ -266,11 +263,9 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, while (pointerMax < this.connections_.length && this.isInYRange_(pointerMax, conn.y, maxRadius)) { temp = this.connections_[pointerMax]; - curDistance = temp.distanceFrom(conn); - if (curDistance <= bestRadius && - this.connectionChecker_.canConnect(conn, temp, true)) { + if (this.connectionChecker_.canConnect(conn, temp, true, bestRadius)) { bestConnection = temp; - bestRadius = curDistance; + bestRadius = temp.distanceFrom(conn); } pointerMax++; } diff --git a/core/interfaces/i_connection_checker.js b/core/interfaces/i_connection_checker.js index 6d386743c..505401666 100644 --- a/core/interfaces/i_connection_checker.js +++ b/core/interfaces/i_connection_checker.js @@ -29,6 +29,8 @@ Blockly.IConnectionChecker = function() {}; * @param {Blockly.Connection} b Connection to check compatibility with. * @param {boolean} isDragging True if the connection is being made by dragging * a block. + * @param {number=} opt_distance The max allowable distance between the + * connections for drag checks. * @return {boolean} Whether the connection is legal. * @public */ @@ -41,6 +43,8 @@ Blockly.IConnectionChecker.prototype.canConnect; * @param {Blockly.Connection} b Connection to check compatibility with. * @param {boolean} isDragging True if the connection is being made by dragging * a block. + * @param {number=} opt_distance The max allowable distance between the + * connections for drag checks. * @return {number} Blockly.Connection.CAN_CONNECT if the connection is legal, * an error code otherwise. * @public @@ -81,9 +85,10 @@ Blockly.IConnectionChecker.prototype.doTypeChecks; /** * Check whether this connection can be made by dragging. - * @param {!Blockly.Connection} a Connection to compare. - * @param {!Blockly.Connection} b Connection to compare against. - * @return {boolean} True if the connections share a type. + * @param {!Blockly.RenderedConnection} a Connection to compare. + * @param {!Blockly.RenderedConnection} b Connection to compare against. + * @param {number} distance The maximum allowable distance between connections. + * @return {boolean} True if the connection is allowed during a drag. * @public */ Blockly.IConnectionChecker.prototype.doDragChecks; diff --git a/tests/mocha/connection_db_test.js b/tests/mocha/connection_db_test.js index f8dccd5d0..2013be0f9 100644 --- a/tests/mocha/connection_db_test.js +++ b/tests/mocha/connection_db_test.js @@ -190,14 +190,26 @@ suite('Connection Database', function() { this.assertOrder(); }); }); - // Does not cover logic for isConnectionAllowed + suite('Search For Closest', function() { setup(function() { - this.allowedStub = null; - this.allowedStub = sinon.stub(this.database.connectionChecker_, 'canConnect') - .callsFake(function(_dragging, _candidate) { + this.allowedStubs = []; + // Ignore type checks. + this.allowedStubs.push(sinon.stub(this.database.connectionChecker_, 'doTypeChecks') + .callsFake(function(_a, _b) { return true; - }); + })); + // Ignore safety checks. + this.allowedStubs.push(sinon.stub(this.database.connectionChecker_, 'doSafetyChecks') + .callsFake(function(_a, _b) { + return Blockly.Connection.CAN_CONNECT; + })); + // Skip everything but the distance checks. + this.allowedStubs.push(sinon.stub(this.database.connectionChecker_, 'doDragChecks') + .callsFake(function(a, b, distance) { + return a.distanceFrom(b) <= distance; + })); + this.createCheckConnection = function(x, y) { var checkConnection = this.createConnection(x, y, Blockly.NEXT_STATEMENT, new Blockly.ConnectionDB()); @@ -205,9 +217,7 @@ suite('Connection Database', function() { }; }); teardown(function() { - if (this.allowedStub) { - this.allowedStub.restore(); - } + this.allowedStubs.forEach(stub => stub.restore()); }); test('Empty Database', function() { var checkConnection = this.createConnection(0, 0, Blockly.NEXT_STATEMENT, From 602d1b6d631e8a071e8b88fbe3effa23cc0d4aec Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 16 Jul 2020 17:46:49 -0600 Subject: [PATCH 16/18] Use new deprecation warning helper --- core/connection.js | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/core/connection.js b/core/connection.js index 19aab3724..e05dae5d8 100644 --- a/core/connection.js +++ b/core/connection.js @@ -14,6 +14,7 @@ goog.provide('Blockly.Connection'); goog.require('Blockly.Events'); goog.require('Blockly.Events.BlockMove'); +goog.require('Blockly.utils.deprecation'); goog.require('Blockly.Xml'); goog.requireType('Blockly.IASTNodeLocationWithBlock'); @@ -248,11 +249,15 @@ Blockly.Connection.prototype.isConnected = function() { * @param {Blockly.Connection} target Connection to check compatibility with. * @return {number} Blockly.Connection.CAN_CONNECT if the connection is legal, * an error code otherwise. - * @deprecated July 2020. Use the workspace's connectionChecker. + * @deprecated July 2020. Will be deleted July 2021. Use the workspace's + * connectionChecker instead. */ Blockly.Connection.prototype.canConnectWithReason = function(target) { - console.warn('Connection.prototype.canConnectWithReason was deprecated in ' + - 'July 2020 and will be deleted in July 2021.'); + Blockly.utils.deprecation.warn( + 'Connection.prototype.canConnectWithReason', + 'July 2020', + 'July 2021', + 'the workspace\'s connection checker'); return this.getConnectionChecker().canConnectWithReason( this, target, false); }; @@ -263,11 +268,15 @@ Blockly.Connection.prototype.canConnectWithReason = function(target) { * @param {Blockly.Connection} target The connection to check compatibility * with. * @package - * @deprecated July 2020. Use the workspace's connectionChecker. + * @deprecated July 2020. Will be deleted July 2021. Use the workspace's + * connectionChecker instead. */ Blockly.Connection.prototype.checkConnection = function(target) { - console.warn('Connection.prototype.checkConnection was deprecated in ' + - 'July 2020 and will be deleted in July 2021.'); + Blockly.utils.deprecation.warn( + 'Connection.prototype.checkConnection', + 'July 2020', + 'July 2021', + 'the workspace\'s connection checker'); var checker = this.getConnectionChecker(); var reason = checker.canConnectWithReason(this, target, false); if (reason != Blockly.Connection.CAN_CONNECT) { @@ -289,11 +298,15 @@ Blockly.Connection.prototype.getConnectionChecker = function() { * Check if the two connections can be dragged to connect to each other. * @param {!Blockly.Connection} candidate A nearby connection to check. * @return {boolean} True if the connection is allowed, false otherwise. - * @deprecated July 2020. Use the workspace's connectionChecker. + * @deprecated July 2020. Will be deleted July 2021. Use the workspace's + * connectionChecker instead. */ Blockly.Connection.prototype.isConnectionAllowed = function(candidate) { - console.warn('Connection.prototype.isConnectionAllowed was deprecated in ' + - 'July 2020 and will be deleted in July 2021.'); + Blockly.utils.deprecation.warn( + 'Connection.prototype.isConnectionAllowed', + 'July 2020', + 'July 2021', + 'the workspace\'s connection checker'); return this.getConnectionChecker().canConnect(this, candidate, true); }; @@ -497,7 +510,6 @@ Blockly.Connection.prototype.targetBlock = function() { * @return {boolean} True if the connections share a type. */ Blockly.Connection.prototype.checkType = function(otherConnection) { - // TODO (fenichel): Add deprecation warnings. return this.getConnectionChecker().canConnect(this, otherConnection, false); }; @@ -512,8 +524,11 @@ Blockly.Connection.prototype.checkType = function(otherConnection) { * @suppress {unusedPrivateMembers} */ Blockly.Connection.prototype.checkType_ = function(otherConnection) { - console.warn('Deprecated call to Blockly.Connection.prototype.checkType_, ' + - 'use Blockly.Connection.prototype.checkType instead.'); + Blockly.utils.deprecation.warn( + 'Connection.prototype.checkType_', + 'October 2019', + 'January 2021', + 'Connection.prototype.checkType'); return this.checkType(otherConnection); }; From 815c8b8919d421e4ce6c08c3db33d88422619a2c Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 16 Jul 2020 17:49:51 -0600 Subject: [PATCH 17/18] More deprecation --- core/connection.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/core/connection.js b/core/connection.js index e05dae5d8..8974ed292 100644 --- a/core/connection.js +++ b/core/connection.js @@ -508,8 +508,15 @@ Blockly.Connection.prototype.targetBlock = function() { * value type system. E.g. square_root("Hello") is not compatible. * @param {!Blockly.Connection} otherConnection Connection to compare against. * @return {boolean} True if the connections share a type. + * @deprecated July 2020. Will be deleted July 2021. Use the workspace's + * connectionChecker instead. */ Blockly.Connection.prototype.checkType = function(otherConnection) { + Blockly.utils.deprecation.warn( + 'Connection.prototype.checkType', + 'October 2019', + 'January 2021', + 'the workspace\'s connection checker'); return this.getConnectionChecker().canConnect(this, otherConnection, false); }; @@ -520,7 +527,8 @@ Blockly.Connection.prototype.checkType = function(otherConnection) { * @param {!Blockly.Connection} otherConnection Connection to compare against. * @return {boolean} True if the connections share a type. * @private - * @deprecated October 2019, use connection.checkType instead. + * @deprecated October 2019. Will be deleted January 2021. Use the workspace's + * connectionChecker instead. * @suppress {unusedPrivateMembers} */ Blockly.Connection.prototype.checkType_ = function(otherConnection) { @@ -528,7 +536,7 @@ Blockly.Connection.prototype.checkType_ = function(otherConnection) { 'Connection.prototype.checkType_', 'October 2019', 'January 2021', - 'Connection.prototype.checkType'); + 'the workspace\'s connection checker'); return this.checkType(otherConnection); }; From 7a515b2caf9f85df67b9d061931aeedbe5d16492 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Fri, 17 Jul 2020 09:49:37 -0600 Subject: [PATCH 18/18] Lint --- tests/mocha/connection_db_test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/mocha/connection_db_test.js b/tests/mocha/connection_db_test.js index 2013be0f9..9e43308f0 100644 --- a/tests/mocha/connection_db_test.js +++ b/tests/mocha/connection_db_test.js @@ -217,7 +217,9 @@ suite('Connection Database', function() { }; }); teardown(function() { - this.allowedStubs.forEach(stub => stub.restore()); + for (var i = 0; i < this.allowedStubs.length; i++) { + this.allowedStubs[i].restore(); + } }); test('Empty Database', function() { var checkConnection = this.createConnection(0, 0, Blockly.NEXT_STATEMENT,