Skip to content

fix!: Fix invalid syntax and unexpected behavior in PHP generator #10661

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.

Sign up for GitHub

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

Merged
gonfunko merged 4 commits into from
Nov 11, 2026
Merged

fix!: Fix invalid syntax and unexpected behavior in PHP generator #10661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
70c3279
fix!: Fix invalid syntax and unexpected behavior in PHP generator
gonfunko Nov 6, 2026
08f1b07
feat: Make opening `<?php` tag configurable
gonfunko Nov 6, 2026
5e2714b
test: Fix tests
gonfunko Nov 6, 2026
a3104ff
chore: Fix docstring
gonfunko Nov 10, 2026
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
274 changes: 140 additions & 134 deletions packages/blockly/generators/php/lists.ts
View file
Open in desktop

Large diffs are not rendered by default.

78 changes: 45 additions & 33 deletions packages/blockly/generators/php/math.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,12 @@ export function math_number_property(
string,
[string, string, Order, Order] | [null, null, Order, Order]
> = {
'EVEN': ['', ' % 2 == 0', Order.MODULUS, Order.EQUALITY],
'ODD': ['', ' % 2 == 1', Order.MODULUS, Order.EQUALITY],
'WHOLE': ['is_int(', ')', Order.NONE, Order.FUNCTION_CALL],
'EVEN': ['fmod(', ', 2) == 0', Order.NONE, Order.EQUALITY],
'ODD': ['abs(fmod(', ', 2)) == 1', Order.NONE, Order.EQUALITY],
'WHOLE': ['fmod(', ', 1) == 0', Order.NONE, Order.EQUALITY],
'POSITIVE': ['', ' > 0', Order.RELATIONAL, Order.RELATIONAL],
'NEGATIVE': ['', ' < 0', Order.RELATIONAL, Order.RELATIONAL],
'DIVISIBLE_BY': [null, null, Order.MODULUS, Order.EQUALITY],
'DIVISIBLE_BY': [null, null, Order.NONE, Order.EQUALITY],
'PRIME': [null, null, Order.NONE, Order.FUNCTION_CALL],
};
type PropertyOption = keyof typeof PROPERTIES;
Expand All @@ -188,7 +188,7 @@ function ${generator.FUNCTION_NAME_PLACEHOLDER_}($n) {
}
// False if n is NaN, negative, is 1, or not whole.
// And false if n is divisible by 2 or 3.
if (!is_numeric($n) || $n <= 1 || $n % 1 != 0 || $n % 2 == 0 || $n % 3 == 0) {
if (!is_numeric($n) || $n <= 1 || fmod($n, 1) != 0 || fmod($n, 2) == 0 || fmod($n, 3) == 0) {
return false;
}
// Check all the numbers of form 6k +/- 1, up to sqrt(n).
Expand All @@ -203,12 +203,11 @@ function ${generator.FUNCTION_NAME_PLACEHOLDER_}($n) {
);
code = functionName + '(' + numberToCheck + ')';
} else if (dropdownProperty === 'DIVISIBLE_BY') {
const divisor =
generator.valueToCode(block, 'DIVISOR', Order.MODULUS) || '0';
const divisor = generator.valueToCode(block, 'DIVISOR', Order.NONE) || '0';
if (divisor === '0') {
return ['false', Order.ATOMIC];
}
code = numberToCheck + ' % ' + divisor + ' == 0';
code = 'fmod(' + numberToCheck + ', ' + divisor + ') == 0';
} else {
code = prefix + numberToCheck + suffix;
}
Expand All @@ -232,36 +231,43 @@ export function math_on_list(
block: Block,
generator: PhpGenerator,
): [string, Order] {
generator.provideListObject();

// Math functions for lists.
const func = block.getFieldValue('OP');
let list;
let code;
switch (func) {
case 'SUM':
list =
generator.valueToCode(block, 'LIST', Order.FUNCTION_CALL) || 'array()';
code = 'array_sum(' + list + ')';
generator.valueToCode(block, 'LIST', Order.FUNCTION_CALL) ||
'(new ListObject())';
code = 'array_sum(' + list + '->getArrayCopy())';
break;
case 'MIN':
list =
generator.valueToCode(block, 'LIST', Order.FUNCTION_CALL) || 'array()';
code = 'min(' + list + ')';
generator.valueToCode(block, 'LIST', Order.FUNCTION_CALL) ||
'(new ListObject())';
code = 'min(' + list + '->getArrayCopy())';
break;
case 'MAX':
list =
generator.valueToCode(block, 'LIST', Order.FUNCTION_CALL) || 'array()';
code = 'max(' + list + ')';
generator.valueToCode(block, 'LIST', Order.FUNCTION_CALL) ||
'(new ListObject())';
code = 'max(' + list + '->getArrayCopy())';
break;
case 'AVERAGE': {
const functionName = generator.provideFunction_(
'math_mean',
`
function ${generator.FUNCTION_NAME_PLACEHOLDER_}($myList) {
return array_sum($myList) / count($myList);
return array_sum($myList->getArrayCopy()) / count($myList);
}
`,
);
list = generator.valueToCode(block, 'LIST', Order.NONE) || 'array()';
list =
generator.valueToCode(block, 'LIST', Order.NONE) ||
'(new ListObject())';
code = functionName + '(' + list + ')';
break;
}
Expand All @@ -270,13 +276,16 @@ function ${generator.FUNCTION_NAME_PLACEHOLDER_}($myList) {
'math_median',
`
function ${generator.FUNCTION_NAME_PLACEHOLDER_}($arr) {
$arr = $arr->getArrayCopy();
sort($arr,SORT_NUMERIC);
return (count($arr) % 2) ? $arr[floor(count($arr) / 2)] :
($arr[floor(count($arr) / 2)] + $arr[floor(count($arr) / 2) - 1]) / 2;
}
`,
);
list = generator.valueToCode(block, 'LIST', Order.NONE) || '[]';
list =
generator.valueToCode(block, 'LIST', Order.NONE) ||
'(new ListObject())';
code = functionName + '(' + list + ')';
break;
}
Expand All @@ -288,15 +297,17 @@ function ${generator.FUNCTION_NAME_PLACEHOLDER_}($arr) {
'math_modes',
`
function ${generator.FUNCTION_NAME_PLACEHOLDER_}($values) {
if (empty($values)) return array();
$counts = array_count_values($values);
if (count($values) == 0) return new ListObject();
$counts = array_count_values($values->getArrayCopy());
arsort($counts); // Sort counts in descending order
$modes = array_keys($counts, current($counts), true);
return $modes;
return new ListObject($modes);
}
`,
);
list = generator.valueToCode(block, 'LIST', Order.NONE) || '[]';
list =
generator.valueToCode(block, 'LIST', Order.NONE) ||
'(new ListObject())';
code = functionName + '(' + list + ')';
break;
}
Expand All @@ -307,13 +318,15 @@ function ${generator.FUNCTION_NAME_PLACEHOLDER_}($values) {
function ${generator.FUNCTION_NAME_PLACEHOLDER_}($numbers) {
$n = count($numbers);
if (!$n) return null;
$mean = array_sum($numbers) / count($numbers);
$mean = array_sum($numbers->getArrayCopy()) / count($numbers);
foreach($numbers as $key => $num) $devs[$key] = pow($num - $mean, 2);
return sqrt(array_sum($devs) / (count($devs) - 1));
}
`,
);
list = generator.valueToCode(block, 'LIST', Order.NONE) || '[]';
list =
generator.valueToCode(block, 'LIST', Order.NONE) ||
'(new ListObject())';
code = functionName + '(' + list + ')';
break;
}
Expand All @@ -327,7 +340,9 @@ function ${generator.FUNCTION_NAME_PLACEHOLDER_}($list) {
}
`,
);
list = generator.valueToCode(block, 'LIST', Order.NONE) || '[]';
list =
generator.valueToCode(block, 'LIST', Order.NONE) ||
'(new ListObject())';
code = functionName + '(' + list + ')';
break;
}
Expand All @@ -342,12 +357,10 @@ export function math_modulo(
generator: PhpGenerator,
): [string, Order] {
// Remainder computation.
const argument0 =
generator.valueToCode(block, 'DIVIDEND', Order.MODULUS) || '0';
const argument1 =
generator.valueToCode(block, 'DIVISOR', Order.MODULUS) || '0';
const code = argument0 + ' % ' + argument1;
return [code, Order.MODULUS];
const argument0 = generator.valueToCode(block, 'DIVIDEND', Order.NONE) || '0';
const argument1 = generator.valueToCode(block, 'DIVISOR', Order.NONE) || '0';
const code = 'fmod(' + argument0 + ', ' + argument1 + ')';
return [code, Order.FUNCTION_CALL];
}

export function math_constrain(
Expand All @@ -357,8 +370,7 @@ export function math_constrain(
// Constrain a number between two limits.
const argument0 = generator.valueToCode(block, 'VALUE', Order.NONE) || '0';
const argument1 = generator.valueToCode(block, 'LOW', Order.NONE) || '0';
const argument2 =
generator.valueToCode(block, 'HIGH', Order.NONE) || 'Infinity';
const argument2 = generator.valueToCode(block, 'HIGH', Order.NONE) || 'INF';
const code =
'min(max(' + argument0 + ', ' + argument1 + '), ' + argument2 + ')';
return [code, Order.FUNCTION_CALL];
Expand Down Expand Up @@ -391,7 +403,7 @@ export function math_random_float(
generator: PhpGenerator,
): [string, Order] {
// Random fraction between 0 and 1.
return ['(float)rand()/(float)getrandmax()', Order.FUNCTION_CALL];
return ['(float)rand()/(float)getrandmax()', Order.DIVISION];
}

export function math_atan2(
Expand Down
95 changes: 89 additions & 6 deletions packages/blockly/generators/php/php_generator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ import type {Workspace} from '../../core/workspace.js';
// prettier-ignore
export enum Order {
ATOMIC = 0, // 0 "" ...
CLONE = 1, // clone
NEW = 1, // new
MEMBER = 2.1, // []
FUNCTION_CALL = 2.2, // ()
MEMBER = 1.1, // []
FUNCTION_CALL = 1.2, // ()
CLONE = 2, // clone
NEW = 2, // new
POWER = 3, // **
INCREMENT = 4, // ++
DECREMENT = 4, // --
Expand Down Expand Up @@ -66,6 +66,10 @@ export enum Order {
export class PhpGenerator extends CodeGenerator {
/** List of outer-inner pairings that do NOT require parentheses. */
ORDER_OVERRIDES: [Order, Order][] = [
// (foo())[0] -> foo()[0]
[Order.FUNCTION_CALL, Order.MEMBER],
// (foo())() -> foo()()
[Order.FUNCTION_CALL, Order.FUNCTION_CALL],
// (foo()).bar() -> foo().bar()
// (foo())[0] -> foo()[0]
[Order.MEMBER, Order.FUNCTION_CALL],
Expand All @@ -84,6 +88,9 @@ export class PhpGenerator extends CodeGenerator {
[Order.LOGICAL_OR, Order.LOGICAL_OR],
];

/** Whether to emit the opening <?php tag when generating code. */
protected outputOpeningTag = true;

/** @param name Name of the language the generator is for. */
constructor(name = 'PHP') {
super(name);
Expand Down Expand Up @@ -173,7 +180,12 @@ export class PhpGenerator extends CodeGenerator {
this.isInitialized = false;

this.nameDB_!.reset();
return definitions.join('\n\n') + '\n\n\n' + code;
return (
(this.outputOpeningTag ? '<?php\n\n' : '') +
definitions.join('\n\n') +
'\n\n\n' +
code
);
}

/**
Expand Down Expand Up @@ -312,9 +324,80 @@ export class PhpGenerator extends CodeGenerator {
if (negate) {
at = delta ? `-(${at})` : `-${at}`;
}
if (Math.floor(order) >= Math.floor(orderForInput)) {
if (Math.floor(order) <= Math.floor(orderForInput)) {
at = `(${at})`;
}
return at;
}

/**
* Sets whether or not this generator outputs the opening <?php tag when
* generating code. Defaults to true.
*
* @param outputTag True to include the opening PHP tag in generated code, or
* false to omit it.
*/
setOutputOpeningTag(outputTag: boolean) {
Comment thread
gonfunko marked this conversation as resolved.
Resolved
this.outputOpeningTag = outputTag;
}

/**
* Defines a class used by Blockly's built-in list blocks. PHP passes by
* value, including `array()`; using an object-based array wrapper gets us
* behavior that is more or less consistent with pass by reference semantics,
* consistent with the other languages we provide generators for. This is
* particularly important for variables that contain lists, where one would
* quite reasonably assume that mutating a list variable actually does so. See
* #1201 for more context.
* @internal
*/
provideListObject() {
// This isn't technically a function, but this gets us one-time-injection of
// a class definition just as well.
this.provideFunction_(
'ListObject',
`
class ListObject extends ArrayObject {

@jamesfudge jamesfudge Nov 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ListObject is defined in the global scope, so there could be a risk of collisions if the generated code is used alongside other code.

Would it be worth giving it a more specific name, such as BlocklyList?

Additionally, maybe we should only declare the class if it doesn't exist, in case the code is run alongside some other generated PHP code, e.g.

if (!class_exists('BlocklyList', false)) {
  class BlocklyList extends ArrayObject {
    // …
  }
}

@gonfunko gonfunko Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In general we more or less do have to assume that the generated code is run in isolation, because user-defined variable or procedure names, while kept unique within the generated code, have the same issue with conflicts if they were dropped into the middle of an external codebase. I did consider using something like BlocklyList, or a \Blockly namespace, but that felt like it leaked too much in the way of internal implementation details; if you're a student using a service built on Blockly, you very likely have never heard of Blockly, but may well actually be shown the generated code, and there's a potential for confusion there.

public function slice($offset, $length = null) {
$values = $this->getArrayCopy();
$result = array_slice($values, $offset, $length);
return new ListObject($result);
}

public function splice($offset, $length = null, $replacement = []) {
$values = $this->getArrayCopy();
$result = array_splice($values, $offset, $length, $replacement);
$this->exchangeArray($values);
return new ListObject($result);
}

public function shift() {
$values = $this->getArrayCopy();
$result = array_shift($values);
$this->exchangeArray($values);
return $result;
}

public function unshift(...$newValues) {
$values = $this->getArrayCopy();
$result = array_unshift($values, ...$newValues);
$this->exchangeArray($values);
return $result;
}

public function pop() {
$values = $this->getArrayCopy();
$result = array_pop($values);
$this->exchangeArray($values);
return $result;
}

public function __toString()
{
return implode(', ', $this->getArrayCopy());
}
}
`,
);
}
}
27 changes: 20 additions & 7 deletions packages/blockly/generators/php/text.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,24 +57,34 @@ export function text_append(block: Block, generator: PhpGenerator) {
return varName + ' .= ' + value + ';\n';
}

export function text_length(
block: Block,
generator: PhpGenerator,
): [string, Order] {
function provideLengthCheck(generator: PhpGenerator) {
// String or array length.
const functionName = generator.provideFunction_(
'length',
`
function ${generator.FUNCTION_NAME_PLACEHOLDER_}($value) {
if (is_string($value)) {
return strlen($value);
} else {
return count($value);
}
return count($value);
}
`,
);

return functionName;
}

export function text_length(
block: Block,
generator: PhpGenerator,
): [string, Order] {
// String or array length.
const text = generator.valueToCode(block, 'VALUE', Order.NONE) || "''";
return [functionName + '(' + text + ')', Order.FUNCTION_CALL];
return [
provideLengthCheck(generator) + '(' + text + ')',
Order.FUNCTION_CALL,
];
}

export function text_isEmpty(
Expand All @@ -83,7 +93,10 @@ export function text_isEmpty(
): [string, Order] {
// Is the string null or array empty?
const text = generator.valueToCode(block, 'VALUE', Order.NONE) || "''";
return ['empty(' + text + ')', Order.FUNCTION_CALL];
return [
provideLengthCheck(generator) + '(' + text + ') == 0',
Order.EQUALITY,
];
}

export function text_indexOf(
Expand Down
Loading
Loading