Silently discard useless PREFIX

classes/local/answer_parser.php

@@ -251,7 +251,7 @@ private function is_acceptable_algebraic_formula(bool $fornumericalformula = fal
                 // formulas, students are allowed to use variables, so the expression is syntactically
                 // valid and will be interpreted as 'sin*30' which is most certainly wrong. The
                 // following check will make sure that students do not use function names as variables.
-                if (array_key_exists($token->value, functions::FUNCTIONS + evaluator::PHPFUNCTIONS)) {
+                if (self::is_valid_function_name($token->value)) {
                     return false;
                 }
                 /* TODO: maybe we should reject unknown variables, because that avoids mistakes

classes/local/parser.php

@@ -239,13 +239,15 @@ private function parse_general_expression(?int $stopat = null): expression {
             $nextvalue = $nexttoken->value;
 
             // If the current token is a PREFIX and the next one is an IDENTIFIER, we will consider
-            // that one as a FUNCTION. If the next token has already been classified as a function,
-            // there is nothing to do. Otherwise, this is a syntax error.
+            // that one as a FUNCTION, unless it is not a known function name. If the PREFIX is not
+            // followed by an identifier, we silently ignore it for maximum backwards compatibility, as
+            // legacy versions used to remove backslashes from variable definitions.
             if ($type === token::PREFIX) {
-                if ($nexttype === token::IDENTIFIER || $nexttype === token::FUNCTION) {
+                if ($nexttype === token::IDENTIFIER) {
+                    if (!self::is_valid_function_name($nextvalue)) {
+                        $this->die(get_string('error_prefix', 'qtype_formulas'));
+                    }
                     $nexttype = ($nexttoken->type = token::FUNCTION);
-                } else {
-                    $this->die(get_string('error_prefix', 'qtype_formulas'));
                 }
             }
 
@@ -266,7 +268,7 @@ private function parse_general_expression(?int $stopat = null): expression {
             // break existing questions.
             if ($type === token::IDENTIFIER) {
                 $isnotavariable = !$this->is_known_variable($currenttoken);
-                $isknownfunction = array_key_exists($value, functions::FUNCTIONS + evaluator::PHPFUNCTIONS);
+                $isknownfunction = self::is_valid_function_name($value);
                 $nextisparen = $nexttype === token::OPENING_PAREN;
                 if ($isnotavariable && $isknownfunction && $nextisparen) {
                     $type = ($currenttoken->type = token::FUNCTION);
@@ -371,6 +373,16 @@ private function parse_general_expression(?int $stopat = null): expression {
         return new expression(shunting_yard::infix_to_rpn($expression));
     }
 
+    /**
+     * Check if a given name is a valid function name.
+     *
+     * @param string $identifier the name to check
+     * @return bool
+     */
+    public static function is_valid_function_name(string $identifier): bool {
+        return array_key_exists($identifier, functions::FUNCTIONS + evaluator::PHPFUNCTIONS);
+    }
+
     /**
      * Retrieve the parsed statements.
      *

renderer.php

@@ -496,7 +496,18 @@ public function get_part_formulation(question_attempt $qa, question_display_opti
         }
 
         foreach ($inputs as $placeholder => $replacement) {
-            $subqreplaced = preg_replace('/'.$boxes[$placeholder]['placeholder'].'/', $replacement, $subqreplaced, 1);
+            // The replacement text *might* contain a backslash and in the worst case this might
+            // lead to an erroneous backreference, e. g. if the student's answer was \1. Thus,
+            // we better use preg_replace_callback() instead of just preg_replace(), as this allows
+            // us to ignore such unintentional backreferences.
+            $subqreplaced = preg_replace_callback(
+                '/' . $boxes[$placeholder]['placeholder'] . '/',
+                function ($matches) use ($replacement) {
+                    return $replacement;
+                },
+                $subqreplaced,
+                1
+            );
         }
         return $subqreplaced;
     }

tests/evaluator_test.php

@@ -454,6 +454,22 @@ public static function provide_simple_expressions(): array {
      */
     public static function provide_valid_assignments(): array {
         return [
+            'accepting useless prefix before number' => [
+                ['a' => new variable('a', 2, variable::NUMERIC)],
+                'a = \2',
+            ],
+            'accepting useless prefix' => [
+                ['a' => new variable('a', 4, variable::NUMERIC)],
+                'a = \ (3 + 1)',
+            ],
+            'accepting useless prefix, not swallowing parens' => [
+                ['a' => new variable('a', 20, variable::NUMERIC)],
+                'a = \ (3 + 1) * 5',
+            ],
+            'correctly reading a whole bunch of useless and useful PREFIXES' => [
+                ['a' => new variable('a', 20, variable::NUMERIC)],
+                'a \ = \ ( \ 3 \ + \ 1 \ ) \ * \ 5 * \cos(0)',
+            ],
             'one number' => [
                 ['a' => new variable('a', 1, variable::NUMERIC)],
                 'a = 1;',
@@ -1490,8 +1506,8 @@ public static function provide_invalid_assignments(): array {
                 'Individual chars of a string cannot be modified.',
                 's = "foo"; s[1] = "x"',
             ],
-            'assignment with invalid function' => [
-                "Unknown function: 'idontexist'",
+            'prefix with invalid function' => [
+                'Syntax error: invalid use of prefix character \.',
                 'a = \idontexist(5)',
             ],
             'assignment to constant' => [
@@ -1502,10 +1518,6 @@ public static function provide_invalid_assignments(): array {
                 'Left-hand side of assignment must be a variable.',
                 'π = 3',
             ],
-            'invalid use of prefix with number' => [
-                'Syntax error: invalid use of prefix character \.',
-                'a = \ 2',
-            ],
             'invalid argument for unary operator' => [
                 "Number expected, found 'foo'.",
                 'a = -"foo"',
@@ -1514,10 +1526,6 @@ public static function provide_invalid_assignments(): array {
                 "Number expected, found 'foo'.",
                 's = "foo"; a = -s',
             ],
-            'invalid use of prefix with paren' => [
-                'Syntax error: invalid use of prefix character \.',
-                'a = \ (3 + 1)',
-            ],
             'assignment to invalid variable' => [
                 '1:1:Invalid variable name: _a.',
                 '_a=3;',
@@ -2314,6 +2322,22 @@ public function test_impossible_stuff(): void {
         }
         self::assertNotNull($e);
 
+        // Trying to execute an unknown function should yield the appropriate error message.
+        $statement = new expression([
+            new token(token::VARIABLE, 'a'),
+            new token(token::NUMBER, 5),
+            new token(token::NUMBER, 1),
+            new token(token::FUNCTION, 'idontexist'),
+            new token(token::OPERATOR, '='),
+        ]);
+        $e = null;
+        try {
+            $evaluator->evaluate($statement);
+        } catch (Exception $e) {
+            self::assertStringEndsWith("Unknown function: 'idontexist'", $e->getMessage());
+        }
+        self::assertNotNull($e);
+
         // When executing the ternary operator, we must have enough stuff (and the right stuff) on the stack.
         $statement = new expression([
             new token(token::OPERATOR, '?'),
@@ -2342,6 +2366,8 @@ public function test_impossible_stuff(): void {
             self::assertStringEndsWith('Evaluation error: not enough arguments for ternary operator.', $e->getMessage());
         }
         self::assertNotNull($e);
+
+
     }
 
     /**

tests/questiontype_test.php

@@ -324,6 +324,10 @@ public static function provide_single_part_data_for_form_validation(): array {
                     'subqtext' => [0 => ['text' => '{_0} and {_0}']],
                 ],
             ],
+            [
+                ['answer[0]' => '1:1:Syntax error: invalid use of prefix character \.'],
+                ['answer' => [0 => '\idontexist(20)']],
+            ],
             [
                 ['answer[0]' => get_string('error_model_answer_no_content', 'qtype_formulas')],
                 ['answer' => [0 => '#']],

tests/renderer_test.php

@@ -103,6 +103,22 @@ public function test_answer_not_unique(): void {
         $this->check_output_contains_lang_string('uniquecorrectansweris', 'qtype_formulas', '5');
     }
 
+    public function test_show_response_with_backslash(): void {
+        $q = $this->get_test_formulas_question('testsinglenum');
+        $this->start_attempt_at_question($q, 'immediatefeedback', 1);
+
+        $this->render();
+        $this->check_current_state(question_state::$todo);
+        $this->check_output_contains_text_input('0_0', '', true);
+
+        // Submit wrong answer. The \1 must not be gobbled.
+        $this->process_submission(['0_0' => '\1', '-submit' => 1]);
+        $this->render();
+        $this->check_current_state(question_state::$gradedwrong);
+        $this->check_output_contains(qtype_formulas_test_helper::DEFAULT_INCORRECT_FEEDBACK);
+        $this->check_output_contains_text_input('0_0', '\1', false);
+    }
+
     public function test_render_question_with_combined_unit_field(): void {
         $q = $this->get_test_formulas_question('testsinglenumunit');
         $q->parts[0]->unitpenalty = 0.5;