Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: update language generators to const and let #5647

Merged
merged 4 commits into from
Oct 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 27 additions & 27 deletions generators/javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,17 +138,17 @@ Blockly.JavaScript.init = function(workspace) {
this.nameDB_.populateVariables(workspace);
this.nameDB_.populateProcedures(workspace);

var defvars = [];
const defvars = [];
// Add developer variables (not created or named by the user).
var devVarList = Blockly.Variables.allDeveloperVariables(workspace);
for (var i = 0; i < devVarList.length; i++) {
const devVarList = Blockly.Variables.allDeveloperVariables(workspace);
for (let i = 0; i < devVarList.length; i++) {
defvars.push(this.nameDB_.getName(devVarList[i],
Blockly.Names.DEVELOPER_VARIABLE_TYPE));
}

// Add user variables, but only ones that are being used.
var variables = Blockly.Variables.allUsedVarModels(workspace);
for (var i = 0; i < variables.length; i++) {
const variables = Blockly.Variables.allUsedVarModels(workspace);
for (let i = 0; i < variables.length; i++) {
defvars.push(this.nameDB_.getName(variables[i].getId(),
Blockly.VARIABLE_CATEGORY_NAME));
}
Expand All @@ -167,7 +167,7 @@ Blockly.JavaScript.init = function(workspace) {
*/
Blockly.JavaScript.finish = function(code) {
// Convert the definitions dictionary into a list.
var definitions = Blockly.utils.object.values(this.definitions_);
const definitions = Blockly.utils.object.values(this.definitions_);
// Call Blockly.Generator's finish.
code = Object.getPrototypeOf(this).finish.call(this, code);
this.isInitialized = false;
Expand Down Expand Up @@ -212,7 +212,7 @@ Blockly.JavaScript.quote_ = function(string) {
Blockly.JavaScript.multiline_quote_ = function(string) {
// Can't use goog.string.quote since Google's style guide recommends
// JS string literals use single quotes.
var lines = string.split(/\n/g).map(this.quote_);
const lines = string.split(/\n/g).map(this.quote_);
return lines.join(' + \'\\n\' +\n');
};

Expand All @@ -227,20 +227,20 @@ Blockly.JavaScript.multiline_quote_ = function(string) {
* @protected
*/
Blockly.JavaScript.scrub_ = function(block, code, opt_thisOnly) {
var commentCode = '';
let commentCode = '';
// Only collect comments for blocks that aren't inline.
if (!block.outputConnection || !block.outputConnection.targetConnection) {
// Collect comment for this block.
var comment = block.getCommentText();
let comment = block.getCommentText();
if (comment) {
comment = Blockly.utils.string.wrap(comment, this.COMMENT_WRAP - 3);
commentCode += this.prefixLines(comment + '\n', '// ');
}
// Collect comments for all value arguments.
// Don't collect comments for nested statements.
for (var i = 0; i < block.inputList.length; i++) {
for (let i = 0; i < block.inputList.length; i++) {
if (block.inputList[i].type === Blockly.inputTypes.VALUE) {
var childBlock = block.inputList[i].connection.targetBlock();
const childBlock = block.inputList[i].connection.targetBlock();
if (childBlock) {
comment = this.allNestedComments(childBlock);
if (comment) {
Expand All @@ -250,8 +250,8 @@ Blockly.JavaScript.scrub_ = function(block, code, opt_thisOnly) {
}
}
}
var nextBlock = block.nextConnection && block.nextConnection.targetBlock();
var nextCode = opt_thisOnly ? '' : this.blockToCode(nextBlock);
const nextBlock = block.nextConnection && block.nextConnection.targetBlock();
const nextCode = opt_thisOnly ? '' : this.blockToCode(nextBlock);
return commentCode + code + nextCode;
};

Expand All @@ -266,25 +266,28 @@ Blockly.JavaScript.scrub_ = function(block, code, opt_thisOnly) {
*/
Blockly.JavaScript.getAdjusted = function(block, atId, opt_delta, opt_negate,
opt_order) {
var delta = opt_delta || 0;
var order = opt_order || this.ORDER_NONE;
let delta = opt_delta || 0;
let order = opt_order || this.ORDER_NONE;
if (block.workspace.options.oneBasedIndex) {
delta--;
}
var defaultAtIndex = block.workspace.options.oneBasedIndex ? '1' : '0';
const defaultAtIndex = block.workspace.options.oneBasedIndex ? '1' : '0';

let innerOrder;
let outerOrder = order;
Comment on lines +276 to +277
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've read through this function about a hundred times and I've got some thoughts:

  • It's not easy to understand! Not the fault of the PR, obviously, but it makes it difficult to verify the correctness of the changes.
  • I'm not sure why both innerOrder and outerOrder are needed.
  • In the refactored code they will always have the same value unless delta === 0 && !opt_negate—which is probably going to be the most common case, but in that case innerOrder === undefined until it gets converted to NaN by Math.floor on line 311.
    • So the check if (innerOrder && order >= innerOrder) could (I think!) have more readably have been written if((delta !== 0 || opt_negate) && order >= outerOrder)
    • And thus, absent delta or opt_negate, at will never get parenthesised on lined 314.
  • In the original code if at is a number then innerOrder will remain undefined even if delta !== 0 or opt_negate (not true in the refactored code!)—but the result is the same in both original and refactor since in that case we skip over all the code that accesses innerOrder anyway.

Anyway: I think the refactoring is equivalent to the original, but there's no way that I'd pass it (or indeed the original) from a readability point of view.

if (delta > 0) {
var at = this.valueToCode(block, atId,
this.ORDER_ADDITION) || defaultAtIndex;
outerOrder = this.ORDER_ADDITION;
innerOrder = this.ORDER_ADDITION;
Comment on lines +279 to +280
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted, I'm not convinced we need two variables, but one could optionally have written this thus:

Suggested change
outerOrder = this.ORDER_ADDITION;
innerOrder = this.ORDER_ADDITION;
outerOrder = innerOrder = this.ORDER_ADDITION;

(And 2x below.)

} else if (delta < 0) {
var at = this.valueToCode(block, atId,
this.ORDER_SUBTRACTION) || defaultAtIndex;
outerOrder = this.ORDER_SUBTRACTION;
innerOrder = this.ORDER_SUBTRACTION;
} else if (opt_negate) {
var at = this.valueToCode(block, atId,
this.ORDER_UNARY_NEGATION) || defaultAtIndex;
} else {
var at = this.valueToCode(block, atId, order) || defaultAtIndex;
outerOrder = this.ORDER_UNARY_NEGATION;
innerOrder = this.ORDER_UNARY_NEGATION;
}

let at = this.valueToCode(block, atId, outerOrder) || defaultAtIndex;

if (Blockly.isNumber(at)) {
// If the index is a naked number, adjust it right now.
at = Number(at) + delta;
Expand All @@ -295,18 +298,15 @@ Blockly.JavaScript.getAdjusted = function(block, atId, opt_delta, opt_negate,
// If the index is dynamic, adjust it in code.
if (delta > 0) {
at = at + ' + ' + delta;
var innerOrder = this.ORDER_ADDITION;
} else if (delta < 0) {
at = at + ' - ' + -delta;
var innerOrder = this.ORDER_SUBTRACTION;
}
if (opt_negate) {
if (delta) {
at = '-(' + at + ')';
} else {
at = '-' + at;
}
var innerOrder = this.ORDER_UNARY_NEGATION;
}
innerOrder = Math.floor(innerOrder);
order = Math.floor(order);
Expand Down
16 changes: 8 additions & 8 deletions generators/lua.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ Blockly.Lua.init = function(workspace) {
*/
Blockly.Lua.finish = function(code) {
// Convert the definitions dictionary into a list.
var definitions = Blockly.utils.object.values(this.definitions_);
const definitions = Blockly.utils.object.values(this.definitions_);
// Call Blockly.Generator's finish.
code = Object.getPrototypeOf(this).finish.call(this, code);
this.isInitialized = false;
Expand Down Expand Up @@ -156,7 +156,7 @@ Blockly.Lua.quote_ = function(string) {
* @protected
*/
Blockly.Lua.multiline_quote_ = function(string) {
var lines = string.split(/\n/g).map(this.quote_);
const lines = string.split(/\n/g).map(this.quote_);
// Join with the following, plus a newline:
// .. '\n' ..
return lines.join(' .. \'\\n\' ..\n');
Expand All @@ -173,20 +173,20 @@ Blockly.Lua.multiline_quote_ = function(string) {
* @protected
*/
Blockly.Lua.scrub_ = function(block, code, opt_thisOnly) {
var commentCode = '';
let commentCode = '';
// Only collect comments for blocks that aren't inline.
if (!block.outputConnection || !block.outputConnection.targetConnection) {
// Collect comment for this block.
var comment = block.getCommentText();
let comment = block.getCommentText();
if (comment) {
comment = Blockly.utils.string.wrap(comment, this.COMMENT_WRAP - 3);
commentCode += this.prefixLines(comment, '-- ') + '\n';
}
// Collect comments for all value arguments.
// Don't collect comments for nested statements.
for (var i = 0; i < block.inputList.length; i++) {
for (let i = 0; i < block.inputList.length; i++) {
if (block.inputList[i].type === Blockly.inputTypes.VALUE) {
var childBlock = block.inputList[i].connection.targetBlock();
const childBlock = block.inputList[i].connection.targetBlock();
if (childBlock) {
comment = this.allNestedComments(childBlock);
if (comment) {
Expand All @@ -196,7 +196,7 @@ Blockly.Lua.scrub_ = function(block, code, opt_thisOnly) {
}
}
}
var nextBlock = block.nextConnection && block.nextConnection.targetBlock();
var nextCode = opt_thisOnly ? '' : this.blockToCode(nextBlock);
const nextBlock = block.nextConnection && block.nextConnection.targetBlock();
const nextCode = opt_thisOnly ? '' : this.blockToCode(nextBlock);
return commentCode + code + nextCode;
};
43 changes: 20 additions & 23 deletions generators/php.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ Blockly.PHP.init = function(workspace) {
*/
Blockly.PHP.finish = function(code) {
// Convert the definitions dictionary into a list.
var definitions = Blockly.utils.object.values(this.definitions_);
const definitions = Blockly.utils.object.values(this.definitions_);
// Call Blockly.Generator's finish.
code = Object.getPrototypeOf(this).finish.call(this, code);
this.isInitialized = false;
Expand Down Expand Up @@ -193,7 +193,7 @@ Blockly.PHP.quote_ = function(string) {
* @protected
*/
Blockly.PHP.multiline_quote_ = function (string) {
var lines = string.split(/\n/g).map(this.quote_);
const lines = string.split(/\n/g).map(this.quote_);
// Join with the following, plus a newline:
// . "\n" .
// Newline escaping only works in double-quoted strings.
Expand All @@ -211,20 +211,20 @@ Blockly.PHP.multiline_quote_ = function (string) {
* @protected
*/
Blockly.PHP.scrub_ = function(block, code, opt_thisOnly) {
var commentCode = '';
let commentCode = '';
// Only collect comments for blocks that aren't inline.
if (!block.outputConnection || !block.outputConnection.targetConnection) {
// Collect comment for this block.
var comment = block.getCommentText();
let comment = block.getCommentText();
if (comment) {
comment = Blockly.utils.string.wrap(comment, this.COMMENT_WRAP - 3);
commentCode += this.prefixLines(comment, '// ') + '\n';
}
// Collect comments for all value arguments.
// Don't collect comments for nested statements.
for (var i = 0; i < block.inputList.length; i++) {
for (let i = 0; i < block.inputList.length; i++) {
if (block.inputList[i].type === Blockly.inputTypes.VALUE) {
var childBlock = block.inputList[i].connection.targetBlock();
const childBlock = block.inputList[i].connection.targetBlock();
if (childBlock) {
comment = this.allNestedComments(childBlock);
if (comment) {
Expand All @@ -234,8 +234,8 @@ Blockly.PHP.scrub_ = function(block, code, opt_thisOnly) {
}
}
}
var nextBlock = block.nextConnection && block.nextConnection.targetBlock();
var nextCode = opt_thisOnly ? '' : this.blockToCode(nextBlock);
const nextBlock = block.nextConnection && block.nextConnection.targetBlock();
const nextCode = opt_thisOnly ? '' : this.blockToCode(nextBlock);
return commentCode + code + nextCode;
};

Expand All @@ -250,25 +250,25 @@ Blockly.PHP.scrub_ = function(block, code, opt_thisOnly) {
*/
Blockly.PHP.getAdjusted = function(block, atId, opt_delta, opt_negate,
opt_order) {
var delta = opt_delta || 0;
var order = opt_order || this.ORDER_NONE;
let delta = opt_delta || 0;
let order = opt_order || this.ORDER_NONE;
if (block.workspace.options.oneBasedIndex) {
delta--;
}
var defaultAtIndex = block.workspace.options.oneBasedIndex ? '1' : '0';
let defaultAtIndex = block.workspace.options.oneBasedIndex ? '1' : '0';
let outerOrder = order;
let innerOrder;
Comment on lines +259 to +260
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments above for Blockly.JavaScript.getAdjusted.

if (delta > 0) {
var at = this.valueToCode(block, atId,
this.ORDER_ADDITION) || defaultAtIndex;
outerOrder = this.ORDER_ADDITION;
innerOrder = this.ORDER_ADDITION;
} else if (delta < 0) {
var at = this.valueToCode(block, atId,
this.ORDER_SUBTRACTION) || defaultAtIndex;
outerOrder = this.ORDER_SUBTRACTION;
innerOrder = this.ORDER_SUBTRACTION;
} else if (opt_negate) {
var at = this.valueToCode(block, atId,
this.ORDER_UNARY_NEGATION) || defaultAtIndex;
} else {
var at = this.valueToCode(block, atId, order) ||
defaultAtIndex;
outerOrder = this.ORDER_UNARY_NEGATION;
innerOrder = this.ORDER_UNARY_NEGATION;
}
let at = this.valueToCode(block, atId, outerOrder) || defaultAtIndex;

if (Blockly.isNumber(at)) {
// If the index is a naked number, adjust it right now.
Expand All @@ -280,18 +280,15 @@ Blockly.PHP.getAdjusted = function(block, atId, opt_delta, opt_negate,
// If the index is dynamic, adjust it in code.
if (delta > 0) {
at = at + ' + ' + delta;
var innerOrder = this.ORDER_ADDITION;
} else if (delta < 0) {
at = at + ' - ' + -delta;
var innerOrder = this.ORDER_SUBTRACTION;
}
if (opt_negate) {
if (delta) {
at = '-(' + at + ')';
} else {
at = '-' + at;
}
var innerOrder = this.ORDER_UNARY_NEGATION;
}
innerOrder = Math.floor(innerOrder);
order = Math.floor(order);
Expand Down
Loading