From 25adb40e6697e0b8e493474b796e53e23f0979be Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Mon, 13 May 2019 15:25:52 -0700 Subject: [PATCH] Prefix and suffix edge cases for flow statements. Call suffix code on break/continue before executing the break/continue. Call prefix code for enclosing loop before executing continue. --- blocks/loops.js | 34 ++++++++++++++++++----------- blocks/procedures.js | 4 ++-- blocks/variables.js | 4 ++-- core/blockly.js | 2 +- core/extensions.js | 2 +- core/generator.js | 2 +- core/mutator.js | 2 +- core/toolbox.js | 4 ++-- core/touch.js | 4 ++-- generators/dart/loops.js | 22 ++++++++++++++++--- generators/javascript/loops.js | 24 +++++++++++++++++--- generators/javascript/procedures.js | 1 - generators/lua/loops.js | 23 ++++++++++++++++--- generators/php/loops.js | 22 ++++++++++++++++--- generators/php/procedures.js | 1 - generators/python/loops.js | 22 ++++++++++++++++--- 16 files changed, 131 insertions(+), 42 deletions(-) diff --git a/blocks/loops.js b/blocks/loops.js index fa8f9e5fe..eb245a808 100644 --- a/blocks/loops.js +++ b/blocks/loops.js @@ -258,7 +258,7 @@ Blockly.Constants.Loops.CUSTOM_CONTEXT_MENU_CREATE_VARIABLES_GET_MIXIN = { * @this Blockly.Block */ customContextMenu: function(options) { - if (this.isInFlyout){ + if (this.isInFlyout) { return; } var variable = this.getField('VAR').getVariable(); @@ -303,7 +303,24 @@ Blockly.Constants.Loops.CONTROL_FLOW_IN_LOOP_CHECK_MIXIN = { * Blockly.Constants.Loops.CONTROL_FLOW_IN_LOOP_CHECK_MIXIN.LOOP_TYPES.push('custom_loop'); */ LOOP_TYPES: ['controls_repeat', 'controls_repeat_ext', 'controls_forEach', - 'controls_for', 'controls_whileUntil'], + 'controls_for', 'controls_whileUntil'], + + /** + * Is the given block enclosed (at any level) by a loop? + * @param {!Blockly.Block} block Current block. + * @return {Blockly.Block} The nearest surrounding loop, or null if none. + */ + getSurroundLoop: function(block) { + // Is the block nested in a loop? + do { + if (Blockly.Constants.Loops.CONTROL_FLOW_IN_LOOP_CHECK_MIXIN.LOOP_TYPES + .indexOf(block.type) != -1) { + return block; + } + block = block.getSurroundParent(); + } while (block); + return null; + }, /** * Called whenever anything on the workspace changes. @@ -315,17 +332,8 @@ Blockly.Constants.Loops.CONTROL_FLOW_IN_LOOP_CHECK_MIXIN = { if (!this.workspace.isDragging || this.workspace.isDragging()) { return; // Don't change state at the start of a drag. } - var legal = false; - // Is the block nested in a loop? - var block = this; - do { - if (this.LOOP_TYPES.indexOf(block.type) != -1) { - legal = true; - break; - } - block = block.getSurroundParent(); - } while (block); - if (legal) { + if (Blockly.Constants.Loops.CONTROL_FLOW_IN_LOOP_CHECK_MIXIN + .getSurroundLoop(this)) { this.setWarningText(null); if (!this.isInFlyout) { this.setEnabled(true); diff --git a/blocks/procedures.js b/blocks/procedures.js index 934b8ee56..5188ac8b9 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -349,7 +349,7 @@ Blockly.Blocks['procedures_defnoreturn'] = { * @this Blockly.Block */ customContextMenu: function(options) { - if (this.isInFlyout){ + if (this.isInFlyout) { return; } // Add option to create caller. @@ -489,7 +489,7 @@ Blockly.Blocks['procedures_mutatorcontainer'] = { } return; } - + if (event.type != Blockly.Events.BLOCK_CREATE) { return; } diff --git a/blocks/variables.js b/blocks/variables.js index 4ea1d1b07..1cd2f36f8 100644 --- a/blocks/variables.js +++ b/blocks/variables.js @@ -100,7 +100,7 @@ Blockly.Constants.Variables.CUSTOM_CONTEXT_MENU_VARIABLE_GETTER_SETTER_MIXIN = { * @this Blockly.Block */ customContextMenu: function(options) { - if (!this.isInFlyout){ + if (!this.isInFlyout) { // Getter blocks have the option to create a setter block, and vice versa. if (this.type == 'variables_get') { var opposite_type = 'variables_set'; @@ -123,7 +123,7 @@ Blockly.Constants.Variables.CUSTOM_CONTEXT_MENU_VARIABLE_GETTER_SETTER_MIXIN = { options.push(option); // Getter blocks have the option to rename or delete that variable. } else { - if (this.type == 'variables_get' || this.type == 'variables_get_reporter'){ + if (this.type == 'variables_get' || this.type == 'variables_get_reporter') { var renameOption = { text: Blockly.Msg.RENAME_VARIABLE, enabled: true, diff --git a/core/blockly.js b/core/blockly.js index 562af5f29..500d41395 100644 --- a/core/blockly.js +++ b/core/blockly.js @@ -705,7 +705,7 @@ Blockly.setTheme = function(theme) { Blockly.refreshTheme_ = function(ws) { // Update all blocks in workspace that have a style name. this.updateBlockStyles_(ws.getAllBlocks().filter( - function(block){ + function(block) { return block.getStyleName() !== undefined; } )); diff --git a/core/extensions.js b/core/extensions.js index 7b0b3672d..583cdc910 100644 --- a/core/extensions.js +++ b/core/extensions.js @@ -73,7 +73,7 @@ Blockly.Extensions.register = function(name, initFn) { * registered. */ Blockly.Extensions.registerMixin = function(name, mixinObj) { - if (!mixinObj || typeof mixinObj != 'object'){ + if (!mixinObj || typeof mixinObj != 'object') { throw Error('Error: Mixin "' + name + '" must be a object'); } Blockly.Extensions.register(name, function() { diff --git a/core/generator.js b/core/generator.js index 81fdf1994..e9baf59a7 100644 --- a/core/generator.js +++ b/core/generator.js @@ -343,7 +343,7 @@ Blockly.Generator.prototype.addLoopTrap = function(branch, block) { Blockly.Generator.prototype.injectId = function(msg, block) { var id = block.id.replace(/\$/g, '$$$$'); // Issue 251. return msg.replace(/%1/g, '\'' + id + '\''); -} +}; /** * Comma-separated list of reserved words. diff --git a/core/mutator.js b/core/mutator.js index b7968e850..307c0bf8a 100644 --- a/core/mutator.js +++ b/core/mutator.js @@ -416,7 +416,7 @@ Blockly.Mutator.prototype.dispose = function() { Blockly.Mutator.prototype.updateBlockStyle = function() { var ws = this.workspace_; - if (ws && ws.getAllBlocks()){ + if (ws && ws.getAllBlocks()) { var workspaceBlocks = ws.getAllBlocks(); for (var i = 0; i < workspaceBlocks.length; i++) { var block = workspaceBlocks[i]; diff --git a/core/toolbox.js b/core/toolbox.js index c920d1f54..890b9d2e3 100644 --- a/core/toolbox.js +++ b/core/toolbox.js @@ -387,7 +387,7 @@ Blockly.Toolbox.prototype.syncTrees_ = function(treeIn, treeOut, pathToMedia) { * @param {string} categoryName Name of the toolbox category. * @private */ -Blockly.Toolbox.prototype.setColour_ = function(colourValue, childOut, categoryName){ +Blockly.Toolbox.prototype.setColour_ = function(colourValue, childOut, categoryName) { // Decode the colour for any potential message references // (eg. `%{BKY_MATH_HUE}`). var colour = Blockly.utils.replaceMessageReferences(colourValue); @@ -416,7 +416,7 @@ Blockly.Toolbox.prototype.setColour_ = function(colourValue, childOut, categoryN * @param {string} categoryName Name of the toolbox category. */ Blockly.Toolbox.prototype.setColourFromStyle_ = function( - styleName, childOut, categoryName){ + styleName, childOut, categoryName) { childOut.styleName = styleName; if (styleName && Blockly.getTheme()) { var style = Blockly.getTheme().getCategoryStyle(styleName); diff --git a/core/touch.js b/core/touch.js index 6b54bb8a7..041a4c487 100644 --- a/core/touch.js +++ b/core/touch.js @@ -244,8 +244,8 @@ Blockly.Touch.splitEventByTouches = function(e) { type: e.type, changedTouches: [e.changedTouches[i]], target: e.target, - stopPropagation: function(){ e.stopPropagation(); }, - preventDefault: function(){ e.preventDefault(); } + stopPropagation: function() { e.stopPropagation(); }, + preventDefault: function() { e.preventDefault(); } }; events[i] = newEvent; } diff --git a/generators/dart/loops.js b/generators/dart/loops.js index c0d3bf9aa..adab0f593 100644 --- a/generators/dart/loops.js +++ b/generators/dart/loops.js @@ -153,11 +153,27 @@ Blockly.Dart['controls_forEach'] = function(block) { Blockly.Dart['controls_flow_statements'] = function(block) { // Flow statements: continue, break. - switch (block.getFieldValue('FLOW')) { + var flowType = block.getFieldValue('FLOW'); + var xfix = ''; + if (Blockly.Dart.STATEMENT_SUFFIX) { + // Inject any statement suffix here since the regular one at the end + // will not get executed if the break/continue is triggered. + xfix += Blockly.Dart.injectId(Blockly.Dart.STATEMENT_SUFFIX, block); + } + if (Blockly.Dart.STATEMENT_PREFIX && flowType == 'CONTINUE') { + var loop = Blockly.Constants.Loops + .CONTROL_FLOW_IN_LOOP_CHECK_MIXIN.getSurroundLoop(block); + if (loop) { + // Inject loop's statement prefix here since the regular one at the end + // of the loop will not get executed if the continue is triggered. + xfix += Blockly.Dart.injectId(Blockly.Dart.STATEMENT_PREFIX, loop); + } + } + switch (flowType) { case 'BREAK': - return 'break;\n'; + return xfix + 'break;\n'; case 'CONTINUE': - return 'continue;\n'; + return xfix + 'continue;\n'; } throw Error('Unknown flow statement.'); }; diff --git a/generators/javascript/loops.js b/generators/javascript/loops.js index c1ad595eb..9f6c6001b 100644 --- a/generators/javascript/loops.js +++ b/generators/javascript/loops.js @@ -165,11 +165,29 @@ Blockly.JavaScript['controls_forEach'] = function(block) { Blockly.JavaScript['controls_flow_statements'] = function(block) { // Flow statements: continue, break. - switch (block.getFieldValue('FLOW')) { + var flowType = block.getFieldValue('FLOW'); + var xfix = ''; + if (Blockly.JavaScript.STATEMENT_SUFFIX) { + // Inject any statement suffix here since the regular one at the end + // will not get executed if the break/continue is triggered. + xfix += Blockly.JavaScript.injectId(Blockly.JavaScript.STATEMENT_SUFFIX, + block); + } + if (Blockly.JavaScript.STATEMENT_PREFIX && flowType == 'CONTINUE') { + var loop = Blockly.Constants.Loops + .CONTROL_FLOW_IN_LOOP_CHECK_MIXIN.getSurroundLoop(block); + if (loop) { + // Inject loop's statement prefix here since the regular one at the end + // of the loop will not get executed if the continue is triggered. + xfix += Blockly.JavaScript.injectId(Blockly.JavaScript.STATEMENT_PREFIX, + loop); + } + } + switch (flowType) { case 'BREAK': - return 'break;\n'; + return xfix + 'break;\n'; case 'CONTINUE': - return 'continue;\n'; + return xfix + 'continue;\n'; } throw Error('Unknown flow statement.'); }; diff --git a/generators/javascript/procedures.js b/generators/javascript/procedures.js index 2096ddef0..ad6083f30 100644 --- a/generators/javascript/procedures.js +++ b/generators/javascript/procedures.js @@ -106,7 +106,6 @@ Blockly.JavaScript['procedures_ifreturn'] = function(block) { if (Blockly.JavaScript.STATEMENT_SUFFIX) { // Inject any statement suffix here since the regular one at the end // will not get executed if the return is triggered. - var id = block.id.replace(/\$/g, '$$$$'); // Issue 251. code += Blockly.JavaScript.prefixLines( Blockly.JavaScript.injectId(Blockly.JavaScript.STATEMENT_SUFFIX, block), Blockly.JavaScript.INDENT); diff --git a/generators/lua/loops.js b/generators/lua/loops.js index 612f4ae78..4205af07d 100644 --- a/generators/lua/loops.js +++ b/generators/lua/loops.js @@ -49,6 +49,7 @@ Blockly.Lua.CONTINUE_STATEMENT = 'goto continue\n'; */ Blockly.Lua.addContinueLabel_ = function(branch) { if (branch.indexOf(Blockly.Lua.CONTINUE_STATEMENT) != -1) { + // False positives are possible (e.g. a string literal), but are harmless. return branch + Blockly.Lua.INDENT + '::continue::\n'; } else { return branch; @@ -156,11 +157,27 @@ Blockly.Lua['controls_forEach'] = function(block) { Blockly.Lua['controls_flow_statements'] = function(block) { // Flow statements: continue, break. - switch (block.getFieldValue('FLOW')) { + var flowType = block.getFieldValue('FLOW'); + var xfix = ''; + if (Blockly.Lua.STATEMENT_SUFFIX) { + // Inject any statement suffix here since the regular one at the end + // will not get executed if the break/continue is triggered. + xfix += Blockly.Lua.injectId(Blockly.Lua.STATEMENT_SUFFIX, block); + } + if (Blockly.Lua.STATEMENT_PREFIX && flowType == 'CONTINUE') { + var loop = Blockly.Constants.Loops + .CONTROL_FLOW_IN_LOOP_CHECK_MIXIN.getSurroundLoop(block); + if (loop) { + // Inject loop's statement prefix here since the regular one at the end + // of the loop will not get executed if the continue is triggered. + xfix += Blockly.Lua.injectId(Blockly.Lua.STATEMENT_PREFIX, loop); + } + } + switch (flowType) { case 'BREAK': - return 'break\n'; + return xfix + 'break\n'; case 'CONTINUE': - return Blockly.Lua.CONTINUE_STATEMENT; + return xfix + Blockly.Lua.CONTINUE_STATEMENT; } throw Error('Unknown flow statement.'); }; diff --git a/generators/php/loops.js b/generators/php/loops.js index f85ce0116..490f7e152 100644 --- a/generators/php/loops.js +++ b/generators/php/loops.js @@ -154,11 +154,27 @@ Blockly.PHP['controls_forEach'] = function(block) { Blockly.PHP['controls_flow_statements'] = function(block) { // Flow statements: continue, break. - switch (block.getFieldValue('FLOW')) { + var flowType = block.getFieldValue('FLOW'); + var xfix = ''; + if (Blockly.PHP.STATEMENT_SUFFIX) { + // Inject any statement suffix here since the regular one at the end + // will not get executed if the break/continue is triggered. + xfix += Blockly.PHP.injectId(Blockly.PHP.STATEMENT_SUFFIX, block); + } + if (Blockly.PHP.STATEMENT_PREFIX && flowType == 'CONTINUE') { + var loop = Blockly.Constants.Loops + .CONTROL_FLOW_IN_LOOP_CHECK_MIXIN.getSurroundLoop(block); + if (loop) { + // Inject loop's statement prefix here since the regular one at the end + // of the loop will not get executed if the continue is triggered. + xfix += Blockly.PHP.injectId(Blockly.PHP.STATEMENT_PREFIX, loop); + } + } + switch (flowType) { case 'BREAK': - return 'break;\n'; + return xfix + 'break;\n'; case 'CONTINUE': - return 'continue;\n'; + return xfix + 'continue;\n'; } throw Error('Unknown flow statement.'); }; diff --git a/generators/php/procedures.js b/generators/php/procedures.js index d24a1d6c5..56e042dcd 100644 --- a/generators/php/procedures.js +++ b/generators/php/procedures.js @@ -127,7 +127,6 @@ Blockly.PHP['procedures_ifreturn'] = function(block) { if (Blockly.PHP.STATEMENT_SUFFIX) { // Inject any statement suffix here since the regular one at the end // will not get executed if the return is triggered. - var id = block.id.replace(/\$/g, '$$$$'); // Issue 251. code += Blockly.PHP.prefixLines( Blockly.PHP.injectId(Blockly.PHP.STATEMENT_SUFFIX, block), Blockly.PHP.INDENT); diff --git a/generators/python/loops.js b/generators/python/loops.js index e7047dee1..a29b8d787 100644 --- a/generators/python/loops.js +++ b/generators/python/loops.js @@ -197,11 +197,27 @@ Blockly.Python['controls_forEach'] = function(block) { Blockly.Python['controls_flow_statements'] = function(block) { // Flow statements: continue, break. - switch (block.getFieldValue('FLOW')) { + var flowType = block.getFieldValue('FLOW'); + var xfix = ''; + if (Blockly.Python.STATEMENT_SUFFIX) { + // Inject any statement suffix here since the regular one at the end + // will not get executed if the break/continue is triggered. + xfix += Blockly.Python.injectId(Blockly.Python.STATEMENT_SUFFIX, block); + } + if (Blockly.Python.STATEMENT_PREFIX && flowType == 'CONTINUE') { + var loop = Blockly.Constants.Loops + .CONTROL_FLOW_IN_LOOP_CHECK_MIXIN.getSurroundLoop(block); + if (loop) { + // Inject loop's statement prefix here since the regular one at the end + // of the loop will not get executed if the continue is triggered. + xfix += Blockly.Python.injectId(Blockly.Python.STATEMENT_PREFIX, loop); + } + } + switch (flowType) { case 'BREAK': - return 'break\n'; + return xfix + 'break\n'; case 'CONTINUE': - return 'continue\n'; + return xfix + 'continue\n'; } throw Error('Unknown flow statement.'); };