type hints, further testing, some cleanup

classes/local/functions.php

@@ -725,7 +725,9 @@ public static function str($value): string {
      */
     public static function join($separator, ...$values): string {
         $result = [];
-        array_walk_recursive($values, function($val, $idx) use (&$result) {
+        // Using array_walk_recursive() makes it easy to accept a list of strings as the second
+        // argument instead of giving all strings individually.
+        array_walk_recursive($values, function($val) use (&$result) {
             $result[] = $val;
         });
         return implode($separator, $result);

question.php

@@ -825,82 +825,82 @@ class qtype_formulas_part {
     public array $evaluatedanswers = [];
 
     /** @var int the part's id */
-    public $id;
+    public int $id;
 
     /** @var int the parents question's id */
-    public $questionid;
+    public int $questionid;
 
     /** @var int the part's position among all parts of the question */
-    public $partindex;
+    public int $partindex;
 
     /** @var string the part's placeholder, e.g. #1 */
-    public $placeholder;
+    public string $placeholder;
 
     /** @var float the maximum grade for this part */
-    public $answermark;
+    public float $answermark;
 
     /** @var int answer type (number, numerical, numerical formula, algebraic) */
-    public $answertype;
+    public int $answertype;
 
     /** @var int number of answer boxes (not including a possible unit box) for this part */
-    public $numbox;
+    public int $numbox;
 
     /** @var string definition of local variables */
-    public $vars1;
+    public string $vars1;
 
     /** @var string definition of grading variables */
-    public $vars2;
+    public string $vars2;
 
     /** @var string definition of the model answer(s) */
-    public $answer;
+    public string $answer;
 
     /** @var int whether there are multiple possible answers */
-    public $answernotunique;
+    public int $answernotunique;
 
     /** @var string definition of the grading criterion */
-    public $correctness;
+    public string $correctness;
 
     /** @var float deduction for a wrong unit */
-    public $unitpenalty;
+    public float $unitpenalty;
 
     /** @var string unit */
-    public $postunit;
+    public string $postunit;
 
     /** @var int the set of basic unit conversion rules to be used */
-    public $ruleid;
+    public int $ruleid;
 
     /** @var string additional conversion rules for other accepted base units */
-    public $otherrule;
+    public string $otherrule;
 
     /** @var string the part's text */
-    public $subqtext;
+    public string $subqtext;
 
     /** @var int format constant (FORMAT_MOODLE, FORMAT_HTML, FORMAT_PLAIN or FORMAT_MARKDOWN) */
-    public $subqtextformat;
+    public int $subqtextformat;
 
     /** @var string general feedback for the part */
-    public $feedback;
+    public string $feedback;
 
     /** @var int format constant (FORMAT_MOODLE, FORMAT_HTML, FORMAT_PLAIN or FORMAT_MARKDOWN) */
-    public $feedbackformat;
+    public int $feedbackformat;
 
     /** @var string part's feedback for any correct response */
-    public $partcorrectfb;
+    public string $partcorrectfb;
 
     /** @var int format constant (FORMAT_MOODLE, FORMAT_HTML, FORMAT_PLAIN or FORMAT_MARKDOWN) */
-    public $partcorrectfbformat;
+    public int $partcorrectfbformat;
 
     /** @var string part's feedback for any partially correct response */
-    public $partpartiallycorrectfb;
+    public string $partpartiallycorrectfb;
 
     /** @var int format constant (FORMAT_MOODLE, FORMAT_HTML, FORMAT_PLAIN or FORMAT_MARKDOWN) */
-    public $partpartiallycorrectfbformat;
+    public int $partpartiallycorrectfbformat;
 
     /** @var string part's feedback for any incorrect response */
-    public $partincorrectfb;
+    public string $partincorrectfb;
 
     /** @var int format constant (FORMAT_MOODLE, FORMAT_HTML, FORMAT_PLAIN or FORMAT_MARKDOWN) */
-    public $partincorrectfbformat;
+    public int $partincorrectfbformat;
 
     /**
      * Constructor.

questiontype.php

@@ -88,12 +88,11 @@ public function extra_question_fields() {
 
     /**
      * Fetch the ID for every part of a given question.
-     * TODO: turn this into private method
      *
      * @param int $questionid
      * @return int[]
      */
-    public function fetch_part_ids_for_question(int $questionid): array {
+    protected function fetch_part_ids_for_question(int $questionid): array {
         global $DB;
 
         // Fetch the parts from the DB. The result will be an associative array with

tests/functions_test.php

@@ -38,8 +38,6 @@
 global $CFG;
 require_once($CFG->dirroot . '/question/engine/tests/helpers.php');
 
-// FIXME: add unit test for join() function
-
 /**
  * Unit tests for various functions
  *
@@ -1140,8 +1138,26 @@ public function provide_modular_function_calls(): array {
         ];
     }
 
+    public function provide_join_calls(): array {
+        return [
+            ["ab", 'join("", "a", "b")'],
+            ["a-b", 'join("-", "a", "b")'],
+            ["b", 'join("", "", "b")'],
+            ["a", 'join("", "a", "")'],
+            ["-b", 'join("-", "", "b")'],
+            ["a-", 'join("-", "a", "")'],
+            ["ab", 'join("", ["a", "b"])'],
+            ["a-b", 'join("-", ["a", "b"])'],
+            ["b", 'join("", ["", "b"])'],
+            ["a", 'join("", ["a", ""])'],
+            ["-b", 'join("-", ["", "b"])'],
+            ["a-", 'join("-", ["a", ""])'],
+        ];
+    }
+
     /**
      * @dataProvider provide_sigfig_expressions
+     * @dataProvider provide_join_calls
      */
     public function test_functions_returning_string($expected, $input): void {
         $parser = new parser($input);

tests/parser_test.php

@@ -28,8 +28,6 @@
 // parser features here, e.g. error detection, but conversion from input to token list is
 // purely an implementation thing; the user only cares about results
 
-// TODO/FIXME: maybe add explicit test for has_token_in_tokenlist() ?
-
 namespace qtype_formulas;
 
 use Exception;

tests/questiontype_test.php

@@ -539,11 +539,13 @@ public function test_calculation_of_numbox_numbertype($expected, $answer) {
     }
 
     public function test_fetch_part_ids_for_question(): void {
-        // TODO: remove this once method is private; it is indirectly tested
-        // with the tests to move/delete files
         $this->resetAfterTest();
         $this->setAdminUser();
 
+        // Prepare reflection and make protected method accessible.
+        $reflectedqtype = new \ReflectionClass($this->qtype);
+        $fetchermethod = $reflectedqtype->getMethod('fetch_part_ids_for_question');
+
         $generator = $this->getDataGenerator()->get_plugin_generator('core_question');
         $category = $generator->create_question_category([]);
 
@@ -552,14 +554,14 @@ public function test_fetch_part_ids_for_question(): void {
         $formdata->category = "{$category->id},{$category->contextid}";
         $formdata->id = 0;
         $saved = $this->qtype->save_question($questiondata, $formdata);
-        self::assertCount(1, $this->qtype->fetch_part_ids_for_question($saved->id));
+        self::assertCount(1, $fetchermethod->invokeArgs($this->qtype, [$saved->id]));
 
         $questiondata = test_question_maker::get_question_data('formulas', 'testmethodsinparts');
         $formdata = test_question_maker::get_question_form_data('formulas', 'testmethodsinparts');
         $formdata->category = "{$category->id},{$category->contextid}";
         $formdata->id = 0;
         $saved = $this->qtype->save_question($questiondata, $formdata);
-        self::assertCount(4, $this->qtype->fetch_part_ids_for_question($saved->id));
+        self::assertCount(4, $fetchermethod->invokeArgs($this->qtype, [$saved->id]));
     }
 
     /**