Skip to content

Commit 445eeb9

Browse files
PhilippImhofPhilippImhof
authored
also accept numeric strings when expecting numbers (#202)
1 parent 64a5cc4 commit 445eeb9

File tree

6 files changed

+76
-11
lines changed

6 files changed

+76
-11
lines changed

‎classes/local/evaluator.php

+4-2
Original file line numberDiff line numberDiff line change
@@ -1110,11 +1110,13 @@ private function abort_if_not_scalar(token $token, bool $enforcenumeric = true):
11101110
if ($token->type !== token::NUMBER) {
11111111
if ($token->type === token::SET) {
11121112
$found = '_algebraicvar';
1113-
$value = "algebraic variable";
11141113
} else if ($token->type === token::LIST) {
11151114
$found = '_list';
1116-
$value = "list";
11171115
} else if ($enforcenumeric) {
1116+
// Let's be lenient if the token is not a NUMBER, but its value is numeric.
1117+
if (is_numeric($token->value)) {
1118+
return;
1119+
}
11181120
$a->found = "'{$token->value}'";
11191121
} else if ($token->type === token::STRING) {
11201122
return;

‎classes/local/functions.php

+7-4
Original file line numberDiff line numberDiff line change
@@ -1274,7 +1274,7 @@ private static function abort_if_not_scalar($value, string $who = '', bool $enfo
12741274
if (!is_scalar($value)) {
12751275
self::die("error_{$variant}_{$expectation}", $a);
12761276
}
1277-
$isnumber = is_float($value) || is_int($value);
1277+
$isnumber = is_numeric($value);
12781278
if ($enforcenumeric && !$isnumber) {
12791279
$a->found = $value;
12801280
self::die("error_{$variant}_{$expectation}_found", $a);
@@ -1363,15 +1363,18 @@ public static function apply_binary_operator($op, $first, $second) {
13631363
break;
13641364
case '+':
13651365
// If at least one operand is a string, we use concatenation instead
1366-
// of addition.
1367-
if (is_string($first) || is_string($second)) {
1366+
// of addition, *UNLESS* both strings are numeric, in which case
1367+
// we follow PHP's type juggling and add those numbers. The user
1368+
// will have to use join() in such cases.
1369+
$bothnumeric = is_numeric($first) && is_numeric($second);
1370+
if ((is_string($first) || is_string($second)) && !$bothnumeric) {
13681371
self::abort_if_not_scalar($first, '+', false);
13691372
self::abort_if_not_scalar($second, '+', false);
13701373
$output = $first . $second;
13711374
break;
13721375
}
13731376
// In all other cases, addition must (currently) be numeric, so we abort
1374-
// if the arguments are not numbers.
1377+
// if the arguments are not numbers or numeric strings.
13751378
self::abort_if_not_scalar($first, '+');
13761379
self::abort_if_not_scalar($second, '+');
13771380
$output = $first + $second;

‎questiontype.php

+3-2
Original file line numberDiff line numberDiff line change
@@ -1117,13 +1117,14 @@ public function check_variables_and_expressions(object $data, array $parts): obj
11171117
}
11181118

11191119
// As $modelanswers is now an array, we can iterate over all answers. If the answer type is
1120-
// "algebraic formula", they must all be strings. Otherwise, they must all be numbers.
1120+
// "algebraic formula", they must all be strings. Otherwise, they must all be numbers or
1121+
// at least numeric strings.
11211122
foreach ($modelanswers as $answer) {
11221123
if ($isalgebraic && $answer->type !== token::STRING) {
11231124
$errors["answer[$i]"] = get_string('error_string_for_algebraic_formula', 'qtype_formulas');
11241125
continue;
11251126
}
1126-
if (!$isalgebraic && $answer->type !== token::NUMBER) {
1127+
if (!$isalgebraic && !($answer->type === token::NUMBER || is_numeric($answer->value))) {
11271128
$errors["answer[$i]"] = get_string('error_number_for_numeric_answertypes', 'qtype_formulas');
11281129
continue;
11291130
}

‎tests/evaluator_test.php

+39
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ public static function provide_invalid_diff(): array {
137137
* @dataProvider provide_ternary_expressions
138138
* @dataProvider provide_for_loops
139139
* @dataProvider provide_boolean_and_logical
140+
* @dataProvider provide_numeric_string_expressions
140141
*/
141142
public function test_expressions_with_numeric_result($expected, $input): void {
142143
$parser = new parser($input);
@@ -371,6 +372,32 @@ public static function provide_expressions_with_functions(): array {
371372
];
372373
}
373374

375+
/**
376+
* Provide various expressions where numbers are written as strings.
377+
*
378+
* @return array
379+
*/
380+
public static function provide_numeric_string_expressions(): array {
381+
return [
382+
[3, '"1" + "2"'],
383+
[3, '1 + "2"'],
384+
[3, '"1" + 2'],
385+
[-1, '"1" - "2"'],
386+
[-1, '1 - "2"'],
387+
[-1, '"1" - 2'],
388+
[8, '"4" * "2"'],
389+
[8, '"4" * 2'],
390+
[8, '4 * "2"'],
391+
[1 / 2, '"1" / "2"'],
392+
[1 / 2, '1 / "2"'],
393+
[1 / 2, '"1" / 2'],
394+
[16, '"4" ** "2"'],
395+
[16, '"4" ** 2'],
396+
[16, '4 ** "2"'],
397+
[5, '[1,5]["1"]'],
398+
];
399+
}
400+
374401
/**
375402
* Provide various simple expressions, i. e. expressions involving only operators but no functions.
376403
*
@@ -486,6 +513,18 @@ public static function provide_valid_assignments(): array {
486513
],
487514
's = "Hello" + " World!";',
488515
],
516+
'string concatenation, direct, string with number' => [
517+
[
518+
's' => new variable('s', 'a1', variable::STRING),
519+
],
520+
's = "a" + "1";',
521+
],
522+
'string concatenation, direct, number with string' => [
523+
[
524+
's' => new variable('s', '1a', variable::STRING),
525+
],
526+
's = "1" + "a";',
527+
],
489528
'string concatenation, from variables' => [
490529
[
491530
'a' => new variable('a', 'Hello', variable::STRING),

‎tests/functions_test.php

+19
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ public static function provide_normcdf(): array {
160160
[0.990184671371355, 'normcdf(135, 100, 15)'],
161161
[0.246921118121914, 'normcdf(10, 23, 19)'],
162162
[0.022750131948179, 'normcdf(-4, 2, 3)'],
163+
[0.022750131948179, 'normcdf("-4", "2", "3")'],
163164
];
164165
}
165166

@@ -181,6 +182,7 @@ public static function provide_stdnormcdf(): array {
181182
[0.9772498680518207927997, 'stdnormcdf(2)'],
182183
[0.99865010196836990547335, 'stdnormcdf(3)'],
183184
[0.9999997133484281208060883, 'stdnormcdf(5)'],
185+
[0.9999997133484281208060883, 'stdnormcdf("5")'],
184186
[0.9999999999999999999999999, 'stdnormcdf(10)'],
185187
[0.9999999999999999999999999, 'stdnormcdf(20)'],
186188
];
@@ -197,6 +199,7 @@ public static function provide_stdnormpdf(): array {
197199
[0.241970724519143349797830, 'stdnormpdf(1)'],
198200
[0.241970724519143349797830, 'stdnormpdf(-1)'],
199201
[0.0539909665131880519505642, 'stdnormpdf(2)'],
202+
[0.0539909665131880519505642, 'stdnormpdf("2")'],
200203
[0.0539909665131880519505642, 'stdnormpdf(-2)'],
201204
[0.3520653267642994777746804, 'stdnormpdf(0.5)'],
202205
[0.3520653267642994777746804, 'stdnormpdf(-0.5)'],
@@ -215,6 +218,7 @@ public static function provide_ncr(): array {
215218
[1, 'ncr(5, 0)'],
216219
[5, 'ncr(5, 1)'],
217220
[10, 'ncr(5, 2)'],
221+
[10, 'ncr("5", "2")'],
218222
[10, 'ncr(5, 3)'],
219223
[5, 'ncr(5, 4)'],
220224
[1, 'ncr(5, 5)'],
@@ -238,6 +242,7 @@ public static function provide_npr(): array {
238242
[0, 'npr(1, 5)'],
239243
[1, 'npr(5, 0)'],
240244
[5, 'npr(5, 1)'],
245+
[5, 'npr("5", "1")'],
241246
[20, 'npr(5, 2)'],
242247
[60, 'npr(5, 3)'],
243248
[120, 'npr(5, 4)'],
@@ -257,6 +262,7 @@ public static function provide_npr(): array {
257262
*/
258263
public static function provide_fact(): array {
259264
return [
265+
[6, 'fact("3")'],
260266
[1, 'fact(0)'],
261267
[1, 'fact(1)'],
262268
[2, 'fact(2)'],
@@ -282,6 +288,7 @@ public static function provide_binomialpdf(): array {
282288
[0, 'binomialpdf(10, 1, 20)'],
283289
[0.125, 'binomialpdf(3, 0.5, 0)'],
284290
[0.375, 'binomialpdf(3, 0.5, 1)'],
291+
[0.375, 'binomialpdf("3", "0.5", "1")'],
285292
[0.375, 'binomialpdf(3, 0.5, 2)'],
286293
[0.125, 'binomialpdf(3, 0.5, 3)'],
287294
['binomialpdf() expects the probability to be at least 0 and not more than 1.', 'binomialpdf(10, 3, 5)'],
@@ -302,6 +309,7 @@ public static function provide_binomialcdf(): array {
302309
[0, 'binomialcdf(1, 1, 0)'],
303310
[1, 'binomialcdf(10, 0.3, 10)'],
304311
[1, 'binomialcdf(10, 0.3, 20)'],
312+
[1, 'binomialcdf("10", "0.3", "20")'],
305313
[0.125, 'binomialcdf(3, 0.5, 0)'],
306314
[0.5, 'binomialcdf(3, 0.5, 1)'],
307315
[0.875, 'binomialcdf(3, 0.5, 2)'],
@@ -319,6 +327,7 @@ public static function provide_binomialcdf(): array {
319327
public static function provide_inv(): array {
320328
return [
321329
[[0, 1, 2, 3], 'a=inv([0, 1, 2, 3]);'],
330+
[[0, 1, 2, 3], 'a=inv(["0", "1", "2", "3"]);'],
322331
[[1, 2, 3, 4], 'a=inv([1, 2, 3, 4]);'],
323332
["Invalid number of arguments for function 'inv': 0 given.", 'a=inv();'],
324333
["Invalid number of arguments for function 'inv': 2 given.", 'a=inv([1, 2, 3], [4, 5, 6]);'],
@@ -451,6 +460,7 @@ public static function provide_gcd_inputs(): array {
451460
[10, 'gcd(10, 0)'],
452461
[10, 'gcd(0, 10)'],
453462
[1, 'gcd(3, 2)'],
463+
[1, 'gcd("3", "2")'],
454464
[3, 'gcd(6, 3)'],
455465
[3, 'gcd(12, 9)'],
456466
[1, 'gcd(2, 3)'],
@@ -478,6 +488,7 @@ public static function provide_lcm_inputs(): array {
478488
[0, 'lcm(0, 10)'],
479489
[6, 'lcm(3, 2)'],
480490
[6, 'lcm(6, 3)'],
491+
[6, 'lcm("6", "3")'],
481492
[36, 'lcm(12, 9)'],
482493
[6, 'lcm(2, 3)'],
483494
[6, 'lcm(3, 6)'],
@@ -521,6 +532,7 @@ public function test_gcd_lcm($expected, $input): void {
521532
public static function provide_pick_inputs(): array {
522533
return [
523534
[3, 'pick(3,[0,1,2,3,4,5])'],
535+
[3, 'pick("3",[0,1,2,3,4,5])'],
524536
[3, 'pick(3.9,[0,1,2,3,4,5])'],
525537
[0, 'pick(10,[0,1,2,3,4,5])'],
526538
[0, 'pick(10.9,[0,1,2,3,4,5])'],
@@ -594,6 +606,7 @@ public static function provide_sigfig_expressions(): array {
594606
['-20', 'sigfig(-17.1, 1)'],
595607

596608
['0.0123', 'sigfig(.012345, 3)'],
609+
['0.0123', 'sigfig(".012345", "3")'],
597610
['0.01235', 'sigfig(.012345, 4)'],
598611
['0.0123450', 'sigfig(.012345, 6)'],
599612
['-0.0123', 'sigfig(-.012345, 3)'],
@@ -935,6 +948,9 @@ public static function provide_algebraic_numerical_function_invocations(): array
935948
[false, 'a=log10();'],
936949
[false, 'a=log10(1, 2);'],
937950
[true, 'a=lg(0.5);'],
951+
[true, 'a=lg("0.5");'],
952+
[true, 'a=lb("0.5");'],
953+
[true, 'a=ln("0.5");'],
938954
[false, 'a=lg();'],
939955
[false, 'a=lg(1, 2);'],
940956
[true, 'a=log1p(0.5);'],
@@ -1310,6 +1326,7 @@ public static function provide_modular_function_calls(): array {
13101326
[15, 'fmod(35,20)'],
13111327
[5, 'fmod(-35, 20)'],
13121328
[-5, 'fmod(35, -20)'],
1329+
[-5, 'fmod("35", "-20")'],
13131330
[-15, 'fmod(-35, -20)'],
13141331
[0, 'fmod(12, 3)'],
13151332
[5, 'fmod(5, 8)'],
@@ -1331,6 +1348,7 @@ public static function provide_modular_function_calls(): array {
13311348
[5, 'modinv(3, 7)'],
13321349
[2, 'modinv(4, 7)'],
13331350
[3, 'modinv(5, 7)'],
1351+
[3, 'modinv("5", "7")'],
13341352
[6, 'modinv(6, 7)'],
13351353
[3, 'modinv(-3, 5)'],
13361354
['modinv() expects its second argument to be a positive integer.', 'modinv(8, -3)'],
@@ -1343,6 +1361,7 @@ public static function provide_modular_function_calls(): array {
13431361
[11, 'modpow(2, 7, 13)'],
13441362
[3, 'modpow(3, 7, 13)'],
13451363
[4, 'modpow(4, 7, 13)'],
1364+
[4, 'modpow("4", "7", "13")'],
13461365
[8, 'modpow(5, 7, 13)'],
13471366
[7, 'modpow(6, 7, 13)'],
13481367
[6, 'modpow(7, 7, 13)'],

‎tests/questiontype_test.php

+4-3
Original file line numberDiff line numberDiff line change
@@ -388,9 +388,10 @@ public static function provide_single_part_data_for_form_validation(): array {
388388
],
389389
[
390390
['answer[0]' => get_string('error_number_for_numeric_answertypes', 'qtype_formulas')],
391-
[
392-
'answer' => [0 => '"3"'],
393-
],
391+
['answer' => [0 => '"3x"']],
392+
],
393+
[
394+
[], ['answer' => [0 => 'sigfig(1.234, 2)']],
394395
],
395396
[
396397
['answer[0]' => get_string('error_string_for_algebraic_formula', 'qtype_formulas')],

0 commit comments

Comments
 (0)