From 43e95af14eaef1b1bf2c31c9c30891026f4ec6e4 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Sun, 25 Apr 2021 14:39:58 -0700 Subject: [PATCH 1/6] Fix getDefinition being too restrictive --- core/procedures.js | 6 +++-- tests/mocha/procedures_test.js | 47 +++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/core/procedures.js b/core/procedures.js index c720db195..59c364719 100644 --- a/core/procedures.js +++ b/core/procedures.js @@ -394,8 +394,10 @@ Blockly.Procedures.mutateCallers = function(defBlock) { * @return {Blockly.Block} The procedure definition block, or null not found. */ Blockly.Procedures.getDefinition = function(name, workspace) { - // Assume that a procedure definition is a top block. - var blocks = workspace.getTopBlocks(false); + // Do not assume procedure is a top block. Some languages allow nested + // procedures. Also do not assume it is one of the built-in blocks. Only + // rely on getProcedureDef. + var blocks = workspace.getAllBlocks(false); for (var i = 0; i < blocks.length; i++) { if (blocks[i].getProcedureDef) { var procedureBlock = /** @type {!Blockly.Procedures.ProcedureBlock} */ ( diff --git a/tests/mocha/procedures_test.js b/tests/mocha/procedures_test.js index 95791ef63..ad8ccabc0 100644 --- a/tests/mocha/procedures_test.js +++ b/tests/mocha/procedures_test.js @@ -18,7 +18,7 @@ suite('Procedures', function() { teardown(function() { sharedTestTeardown.call(this); }); - + suite('allProcedures', function() { test('Only Procedures', function() { var noReturnBlock = new Blockly.Block(this.workspace, 'procedures_defnoreturn'); @@ -367,6 +367,51 @@ suite('Procedures', function() { }); }); + suite('getDefinition - Modified cases', function() { + setup(function() { + Blockly.Blocks['new_proc'] = { + init: function() { }, + getProcedureDef: function() { + return [this.name, [], false]; + }, + name: 'test' + }; + + Blockly.Blocks['nested_proc'] = { + init: function() { + this.setPreviousStatement(true, null); + this.setNextStatement(true, null); + }, + getProcedureDef: function() { + return [this.name, [], false]; + }, + name: 'test', + }; + }); + + teardown(function() { + delete Blockly.Blocks['new_proc']; + delete Blockly.Blocks['nested_proc']; + }); + + test('New definition', function() { + // Do not require procedures to be the built-in procedures. + var defBlock = new Blockly.Block(this.workspace, 'new_proc'); + var def = Blockly.Procedures.getDefinition('test', this.workspace); + chai.assert.equal(def, defBlock); + }); + + test('Stacked procedures', function() { + var blockA = new Blockly.Block(this.workspace, 'nested_proc'); + var blockB = new Blockly.Block(this.workspace, 'nested_proc'); + blockA.name = 'a'; + blockB.name = 'b'; + blockA.nextConnection.connect(blockB.previousConnection); + var def = Blockly.Procedures.getDefinition('b', this.workspace); + chai.assert.equal(def, blockB); + }); + }); + const testSuites = [ {title: 'procedures_defreturn', hasReturn: true, defType: 'procedures_defreturn', callType: 'procedures_callreturn'}, From 9fbf06c5dfc19f0515608b69e767f70cf0feb026 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Tue, 27 Apr 2021 15:59:25 -0700 Subject: [PATCH 2/6] PR Comments --- tests/mocha/procedures_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mocha/procedures_test.js b/tests/mocha/procedures_test.js index ad8ccabc0..48803bcbd 100644 --- a/tests/mocha/procedures_test.js +++ b/tests/mocha/procedures_test.js @@ -394,7 +394,7 @@ suite('Procedures', function() { delete Blockly.Blocks['nested_proc']; }); - test('New definition', function() { + test('Custom procedure block', function() { // Do not require procedures to be the built-in procedures. var defBlock = new Blockly.Block(this.workspace, 'new_proc'); var def = Blockly.Procedures.getDefinition('test', this.workspace); From 918bdc23599443d69ba186448b14c7424ab3e8e3 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 6 May 2021 12:22:40 -0700 Subject: [PATCH 3/6] Move ProcedureBlock to interface --- core/interfaces/i_procedure_block.js | 43 ++++++++++++++++++++++++++++ core/procedures.js | 11 +------ 2 files changed, 44 insertions(+), 10 deletions(-) create mode 100644 core/interfaces/i_procedure_block.js diff --git a/core/interfaces/i_procedure_block.js b/core/interfaces/i_procedure_block.js new file mode 100644 index 000000000..17502c19a --- /dev/null +++ b/core/interfaces/i_procedure_block.js @@ -0,0 +1,43 @@ +/** + * @license + * Copyright 2021 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * @fileoverview The interface for a procedure block. + */ + +'use strict'; + +goog.provide('Blockly.Procedures.ProcedureBlock'); + +/** + * A procedure block interface. + * @interface + */ +Blockly.Procedures.ProcedureBlock = function() {}; + +/** + * Returns the name of the procedure the procedure block calls. + * @return {string} + */ +Blockly.Procedures.ProcedureBlock.prototype.getProcedureCall; + +/** + * Renames the procedure from the old name to the new name. If the procedure + * block receives this and the old name matches the block's current name, it + * should update itself to have the new name instead. + * @param {string} oldName The old name of the procedure. + * @param {string} newName The new name of hte procedure. + */ +Blockly.Procedures.ProcedureBlock.prototype.renameProcedure; + +/** + * Returns the signature of the procedure block's procedure definition. + * @return {!Array} Tuple containing three elements: + * - {string} the name of the defined procedure + * - {!Array} a list of all its arguments + * - {boolean} whether it has a return value or not + */ +Blockly.Procedures.ProcedureBlock.prototype.getProcedureDef; diff --git a/core/procedures.js b/core/procedures.js index 59c364719..03b0210b3 100644 --- a/core/procedures.js +++ b/core/procedures.js @@ -25,6 +25,7 @@ goog.require('Blockly.Events.BlockChange'); goog.require('Blockly.Field'); goog.require('Blockly.Msg'); goog.require('Blockly.Names'); +goog.require('Blockly.Procedures.ProcedureBlock'); goog.require('Blockly.utils.xml'); goog.require('Blockly.Workspace'); goog.require('Blockly.Xml'); @@ -47,16 +48,6 @@ Blockly.Procedures.NAME_TYPE = Blockly.PROCEDURE_CATEGORY_NAME; */ Blockly.Procedures.DEFAULT_ARG = 'x'; -/** - * Procedure block type. - * @typedef {{ - * getProcedureCall: function():string, - * renameProcedure: function(string,string), - * getProcedureDef: function():!Array - * }} - */ -Blockly.Procedures.ProcedureBlock; - /** * Find all user-created procedure definitions in a workspace. * @param {!Blockly.Workspace} root Root workspace. From c8a065fa1342ba64d7cc180ef32ba58230f735f5 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Sat, 8 May 2021 07:57:38 -0700 Subject: [PATCH 4/6] Rename to IProcedureBlock --- core/interfaces/i_procedure_block.js | 10 +++++----- core/procedures.js | 26 ++++++++++++++++++-------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/core/interfaces/i_procedure_block.js b/core/interfaces/i_procedure_block.js index 17502c19a..4e586e040 100644 --- a/core/interfaces/i_procedure_block.js +++ b/core/interfaces/i_procedure_block.js @@ -10,19 +10,19 @@ 'use strict'; -goog.provide('Blockly.Procedures.ProcedureBlock'); +goog.provide('Blockly.IProcedureBlock'); /** * A procedure block interface. * @interface */ -Blockly.Procedures.ProcedureBlock = function() {}; +Blockly.IProcedureBlock = function() {}; /** * Returns the name of the procedure the procedure block calls. * @return {string} */ -Blockly.Procedures.ProcedureBlock.prototype.getProcedureCall; +Blockly.IProcedureBlock.prototype.getProcedureCall; /** * Renames the procedure from the old name to the new name. If the procedure @@ -31,7 +31,7 @@ Blockly.Procedures.ProcedureBlock.prototype.getProcedureCall; * @param {string} oldName The old name of the procedure. * @param {string} newName The new name of hte procedure. */ -Blockly.Procedures.ProcedureBlock.prototype.renameProcedure; +Blockly.IProcedureBlock.prototype.renameProcedure; /** * Returns the signature of the procedure block's procedure definition. @@ -40,4 +40,4 @@ Blockly.Procedures.ProcedureBlock.prototype.renameProcedure; * - {!Array} a list of all its arguments * - {boolean} whether it has a return value or not */ -Blockly.Procedures.ProcedureBlock.prototype.getProcedureDef; +Blockly.IProcedureBlock.prototype.getProcedureDef; diff --git a/core/procedures.js b/core/procedures.js index 03b0210b3..e625b5eb2 100644 --- a/core/procedures.js +++ b/core/procedures.js @@ -25,13 +25,13 @@ goog.require('Blockly.Events.BlockChange'); goog.require('Blockly.Field'); goog.require('Blockly.Msg'); goog.require('Blockly.Names'); -goog.require('Blockly.Procedures.ProcedureBlock'); goog.require('Blockly.utils.xml'); goog.require('Blockly.Workspace'); goog.require('Blockly.Xml'); goog.requireType('Blockly.Block'); goog.requireType('Blockly.Events.Abstract'); +goog.requireType('Blockly.IProcedureBlock'); goog.requireType('Blockly.WorkspaceSvg'); @@ -48,6 +48,16 @@ Blockly.Procedures.NAME_TYPE = Blockly.PROCEDURE_CATEGORY_NAME; */ Blockly.Procedures.DEFAULT_ARG = 'x'; +/** + * A procedure block interface. Will be deprecated for Blockly.IProcedureBlock. + * @typedef {{ + * getProcedureCall: function():string, + * renameProcedure: function(string,string), + * getProcedureDef: function():!Array + * }} + */ +Blockly.Procedures.ProcedureBlock; + /** * Find all user-created procedure definitions in a workspace. * @param {!Blockly.Workspace} root Root workspace. @@ -59,10 +69,10 @@ Blockly.Procedures.DEFAULT_ARG = 'x'; Blockly.Procedures.allProcedures = function(root) { var proceduresNoReturn = root.getBlocksByType('procedures_defnoreturn', false) .map(function(block) { - return /** @type {!Blockly.Procedures.ProcedureBlock} */ (block).getProcedureDef(); + return /** @type {!Blockly.IProcedureBlock} */ (block).getProcedureDef(); }); var proceduresReturn = root.getBlocksByType('procedures_defreturn', false).map(function(block) { - return /** @type {!Blockly.Procedures.ProcedureBlock} */ (block).getProcedureDef(); + return /** @type {!Blockly.IProcedureBlock} */ (block).getProcedureDef(); }); proceduresNoReturn.sort(Blockly.Procedures.procTupleComparator_); proceduresReturn.sort(Blockly.Procedures.procTupleComparator_); @@ -137,7 +147,7 @@ Blockly.Procedures.isNameUsed = function(name, workspace, opt_exclude) { continue; } if (blocks[i].getProcedureDef) { - var procedureBlock = /** @type {!Blockly.Procedures.ProcedureBlock} */ ( + var procedureBlock = /** @type {!Blockly.IProcedureBlock} */ ( blocks[i]); var procName = procedureBlock.getProcedureDef(); if (Blockly.Names.equals(procName[0], name)) { @@ -166,7 +176,7 @@ Blockly.Procedures.rename = function(name) { var blocks = this.getSourceBlock().workspace.getAllBlocks(false); for (var i = 0; i < blocks.length; i++) { if (blocks[i].renameProcedure) { - var procedureBlock = /** @type {!Blockly.Procedures.ProcedureBlock} */ ( + var procedureBlock = /** @type {!Blockly.IProcedureBlock} */ ( blocks[i]); procedureBlock.renameProcedure( /** @type {string} */ (oldName), legalName); @@ -336,7 +346,7 @@ Blockly.Procedures.getCallers = function(name, workspace) { // Iterate through every block and check the name. for (var i = 0; i < blocks.length; i++) { if (blocks[i].getProcedureCall) { - var procedureBlock = /** @type {!Blockly.Procedures.ProcedureBlock} */ ( + var procedureBlock = /** @type {!Blockly.IProcedureBlock} */ ( blocks[i]); var procName = procedureBlock.getProcedureCall(); // Procedure name may be null if the block is only half-built. @@ -355,7 +365,7 @@ Blockly.Procedures.getCallers = function(name, workspace) { */ Blockly.Procedures.mutateCallers = function(defBlock) { var oldRecordUndo = Blockly.Events.recordUndo; - var procedureBlock = /** @type {!Blockly.Procedures.ProcedureBlock} */ ( + var procedureBlock = /** @type {!Blockly.IProcedureBlock} */ ( defBlock); var name = procedureBlock.getProcedureDef()[0]; var xmlElement = defBlock.mutationToDom(true); @@ -391,7 +401,7 @@ Blockly.Procedures.getDefinition = function(name, workspace) { var blocks = workspace.getAllBlocks(false); for (var i = 0; i < blocks.length; i++) { if (blocks[i].getProcedureDef) { - var procedureBlock = /** @type {!Blockly.Procedures.ProcedureBlock} */ ( + var procedureBlock = /** @type {!Blockly.IProcedureBlock} */ ( blocks[i]); var tuple = procedureBlock.getProcedureDef(); if (tuple && Blockly.Names.equals(tuple[0], name)) { From 8a00303aa704ca9a86925f99df25c55e79ecc7bc Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Mon, 10 May 2021 15:33:12 -0700 Subject: [PATCH 5/6] Revert "Rename to IProcedureBlock" This reverts commit c8a065fa1342ba64d7cc180ef32ba58230f735f5. --- core/interfaces/i_procedure_block.js | 10 +++++----- core/procedures.js | 26 ++++++++------------------ 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/core/interfaces/i_procedure_block.js b/core/interfaces/i_procedure_block.js index 4e586e040..17502c19a 100644 --- a/core/interfaces/i_procedure_block.js +++ b/core/interfaces/i_procedure_block.js @@ -10,19 +10,19 @@ 'use strict'; -goog.provide('Blockly.IProcedureBlock'); +goog.provide('Blockly.Procedures.ProcedureBlock'); /** * A procedure block interface. * @interface */ -Blockly.IProcedureBlock = function() {}; +Blockly.Procedures.ProcedureBlock = function() {}; /** * Returns the name of the procedure the procedure block calls. * @return {string} */ -Blockly.IProcedureBlock.prototype.getProcedureCall; +Blockly.Procedures.ProcedureBlock.prototype.getProcedureCall; /** * Renames the procedure from the old name to the new name. If the procedure @@ -31,7 +31,7 @@ Blockly.IProcedureBlock.prototype.getProcedureCall; * @param {string} oldName The old name of the procedure. * @param {string} newName The new name of hte procedure. */ -Blockly.IProcedureBlock.prototype.renameProcedure; +Blockly.Procedures.ProcedureBlock.prototype.renameProcedure; /** * Returns the signature of the procedure block's procedure definition. @@ -40,4 +40,4 @@ Blockly.IProcedureBlock.prototype.renameProcedure; * - {!Array} a list of all its arguments * - {boolean} whether it has a return value or not */ -Blockly.IProcedureBlock.prototype.getProcedureDef; +Blockly.Procedures.ProcedureBlock.prototype.getProcedureDef; diff --git a/core/procedures.js b/core/procedures.js index e625b5eb2..03b0210b3 100644 --- a/core/procedures.js +++ b/core/procedures.js @@ -25,13 +25,13 @@ goog.require('Blockly.Events.BlockChange'); goog.require('Blockly.Field'); goog.require('Blockly.Msg'); goog.require('Blockly.Names'); +goog.require('Blockly.Procedures.ProcedureBlock'); goog.require('Blockly.utils.xml'); goog.require('Blockly.Workspace'); goog.require('Blockly.Xml'); goog.requireType('Blockly.Block'); goog.requireType('Blockly.Events.Abstract'); -goog.requireType('Blockly.IProcedureBlock'); goog.requireType('Blockly.WorkspaceSvg'); @@ -48,16 +48,6 @@ Blockly.Procedures.NAME_TYPE = Blockly.PROCEDURE_CATEGORY_NAME; */ Blockly.Procedures.DEFAULT_ARG = 'x'; -/** - * A procedure block interface. Will be deprecated for Blockly.IProcedureBlock. - * @typedef {{ - * getProcedureCall: function():string, - * renameProcedure: function(string,string), - * getProcedureDef: function():!Array - * }} - */ -Blockly.Procedures.ProcedureBlock; - /** * Find all user-created procedure definitions in a workspace. * @param {!Blockly.Workspace} root Root workspace. @@ -69,10 +59,10 @@ Blockly.Procedures.ProcedureBlock; Blockly.Procedures.allProcedures = function(root) { var proceduresNoReturn = root.getBlocksByType('procedures_defnoreturn', false) .map(function(block) { - return /** @type {!Blockly.IProcedureBlock} */ (block).getProcedureDef(); + return /** @type {!Blockly.Procedures.ProcedureBlock} */ (block).getProcedureDef(); }); var proceduresReturn = root.getBlocksByType('procedures_defreturn', false).map(function(block) { - return /** @type {!Blockly.IProcedureBlock} */ (block).getProcedureDef(); + return /** @type {!Blockly.Procedures.ProcedureBlock} */ (block).getProcedureDef(); }); proceduresNoReturn.sort(Blockly.Procedures.procTupleComparator_); proceduresReturn.sort(Blockly.Procedures.procTupleComparator_); @@ -147,7 +137,7 @@ Blockly.Procedures.isNameUsed = function(name, workspace, opt_exclude) { continue; } if (blocks[i].getProcedureDef) { - var procedureBlock = /** @type {!Blockly.IProcedureBlock} */ ( + var procedureBlock = /** @type {!Blockly.Procedures.ProcedureBlock} */ ( blocks[i]); var procName = procedureBlock.getProcedureDef(); if (Blockly.Names.equals(procName[0], name)) { @@ -176,7 +166,7 @@ Blockly.Procedures.rename = function(name) { var blocks = this.getSourceBlock().workspace.getAllBlocks(false); for (var i = 0; i < blocks.length; i++) { if (blocks[i].renameProcedure) { - var procedureBlock = /** @type {!Blockly.IProcedureBlock} */ ( + var procedureBlock = /** @type {!Blockly.Procedures.ProcedureBlock} */ ( blocks[i]); procedureBlock.renameProcedure( /** @type {string} */ (oldName), legalName); @@ -346,7 +336,7 @@ Blockly.Procedures.getCallers = function(name, workspace) { // Iterate through every block and check the name. for (var i = 0; i < blocks.length; i++) { if (blocks[i].getProcedureCall) { - var procedureBlock = /** @type {!Blockly.IProcedureBlock} */ ( + var procedureBlock = /** @type {!Blockly.Procedures.ProcedureBlock} */ ( blocks[i]); var procName = procedureBlock.getProcedureCall(); // Procedure name may be null if the block is only half-built. @@ -365,7 +355,7 @@ Blockly.Procedures.getCallers = function(name, workspace) { */ Blockly.Procedures.mutateCallers = function(defBlock) { var oldRecordUndo = Blockly.Events.recordUndo; - var procedureBlock = /** @type {!Blockly.IProcedureBlock} */ ( + var procedureBlock = /** @type {!Blockly.Procedures.ProcedureBlock} */ ( defBlock); var name = procedureBlock.getProcedureDef()[0]; var xmlElement = defBlock.mutationToDom(true); @@ -401,7 +391,7 @@ Blockly.Procedures.getDefinition = function(name, workspace) { var blocks = workspace.getAllBlocks(false); for (var i = 0; i < blocks.length; i++) { if (blocks[i].getProcedureDef) { - var procedureBlock = /** @type {!Blockly.IProcedureBlock} */ ( + var procedureBlock = /** @type {!Blockly.Procedures.ProcedureBlock} */ ( blocks[i]); var tuple = procedureBlock.getProcedureDef(); if (tuple && Blockly.Names.equals(tuple[0], name)) { From 75319312c1a4534d8e8a68707f8c9920adbb6b30 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Mon, 10 May 2021 15:34:36 -0700 Subject: [PATCH 6/6] Revert "Move ProcedureBlock to interface" This reverts commit 918bdc23599443d69ba186448b14c7424ab3e8e3. --- core/interfaces/i_procedure_block.js | 43 ---------------------------- core/procedures.js | 11 ++++++- 2 files changed, 10 insertions(+), 44 deletions(-) delete mode 100644 core/interfaces/i_procedure_block.js diff --git a/core/interfaces/i_procedure_block.js b/core/interfaces/i_procedure_block.js deleted file mode 100644 index 17502c19a..000000000 --- a/core/interfaces/i_procedure_block.js +++ /dev/null @@ -1,43 +0,0 @@ -/** - * @license - * Copyright 2021 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -/** - * @fileoverview The interface for a procedure block. - */ - -'use strict'; - -goog.provide('Blockly.Procedures.ProcedureBlock'); - -/** - * A procedure block interface. - * @interface - */ -Blockly.Procedures.ProcedureBlock = function() {}; - -/** - * Returns the name of the procedure the procedure block calls. - * @return {string} - */ -Blockly.Procedures.ProcedureBlock.prototype.getProcedureCall; - -/** - * Renames the procedure from the old name to the new name. If the procedure - * block receives this and the old name matches the block's current name, it - * should update itself to have the new name instead. - * @param {string} oldName The old name of the procedure. - * @param {string} newName The new name of hte procedure. - */ -Blockly.Procedures.ProcedureBlock.prototype.renameProcedure; - -/** - * Returns the signature of the procedure block's procedure definition. - * @return {!Array} Tuple containing three elements: - * - {string} the name of the defined procedure - * - {!Array} a list of all its arguments - * - {boolean} whether it has a return value or not - */ -Blockly.Procedures.ProcedureBlock.prototype.getProcedureDef; diff --git a/core/procedures.js b/core/procedures.js index 03b0210b3..59c364719 100644 --- a/core/procedures.js +++ b/core/procedures.js @@ -25,7 +25,6 @@ goog.require('Blockly.Events.BlockChange'); goog.require('Blockly.Field'); goog.require('Blockly.Msg'); goog.require('Blockly.Names'); -goog.require('Blockly.Procedures.ProcedureBlock'); goog.require('Blockly.utils.xml'); goog.require('Blockly.Workspace'); goog.require('Blockly.Xml'); @@ -48,6 +47,16 @@ Blockly.Procedures.NAME_TYPE = Blockly.PROCEDURE_CATEGORY_NAME; */ Blockly.Procedures.DEFAULT_ARG = 'x'; +/** + * Procedure block type. + * @typedef {{ + * getProcedureCall: function():string, + * renameProcedure: function(string,string), + * getProcedureDef: function():!Array + * }} + */ +Blockly.Procedures.ProcedureBlock; + /** * Find all user-created procedure definitions in a workspace. * @param {!Blockly.Workspace} root Root workspace.