Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-73526

assert() doesn't accept string arguments anymore (php80)

XMLWordPrintable

    • MOODLE_311_STABLE, MOODLE_400_STABLE
    • MOODLE_311_STABLE
    • Hide

      Sanity checks

      1. Verify that there isn't any reference to "diff_nwiki" in core.
      2. Verify that there isn't any assert() case in core (see the description for the regexp). All the cases are commented or a method within a class, no global PHP assertions.
      3. Put this into CiBoT hands, nothing should fail with the removal performed.

      Manual instructions / regression testing

      (if wanted to assign this to tester, though I'm 99.99% sure the removal is safe).

      1. Install Moodle
      2. Create a course
      3. As admin or teacher, edit the course and create a wiki activity.
      4. Repeat the following steps for every "Format" (HML, Creole, Nwiki):
        1. Create a new page with some content and formatting (say a header, some bold text...). Save it.
        2. Edit the just created page and perform some changes like removing some text, adding other text, changing the bolded text... Save it.
        3. In the dropdown... select "History"
        4. Verify that you see two versions listed.
        5. Select 1 & 2 in the diff column and press "Compare selected"
        6. Verify that you get a two columns display with:
          1. Version 1 on the left.
          2. Version 2 on the right.
          3. Removed text shows in red and has a yellow background and it is strikethrough.
          4. Added text shows in red and has a green background.
          5. The changes performed are rendered ok (bolds are bolds, heading levels too). Basically that both the left and right columns show the contents ok (apart from the details listed above).
      Show
      Sanity checks Verify that there isn't any reference to "diff_nwiki" in core. Verify that there isn't any assert() case in core (see the description for the regexp). All the cases are commented or a method within a class, no global PHP assertions. Put this into CiBoT hands, nothing should fail with the removal performed. Manual instructions / regression testing (if wanted to assign this to tester, though I'm 99.99% sure the removal is safe). Install Moodle Create a course As admin or teacher, edit the course and create a wiki activity. Repeat the following steps for every "Format" (HML, Creole, Nwiki): Create a new page with some content and formatting (say a header, some bold text...). Save it. Edit the just created page and perform some changes like removing some text, adding other text, changing the bolded text... Save it. In the dropdown... select "History" Verify that you see two versions listed. Select 1 & 2 in the diff column and press "Compare selected" Verify that you get a two columns display with: Version 1 on the left. Version 2 on the right. Removed text shows in red and has a yellow background and it is strikethrough. Added text shows in red and has a green background. The changes performed are rendered ok (bolds are bolds, heading levels too). Basically that both the left and right columns show the contents ok (apart from the details listed above).

      From php80 upgrade notes:

      assert() will no longer evaluate string arguments, instead they will be
      treated like any other argument. assert($a == $b) should be used instead of
      assert('$a == $b')

      So this is about to verify all the current assert() uses and ensure that no string arguments are passed to it.

      It seems that there are a few uses:

      $ ag '[^>_]assert\(' --php
      question/engine/tests/helpers.php
      482:    public function assert($expectation, $compare, $notused = '') {
       
      lib/htmlpurifier/HTMLPurifier/Arborize.php
      22:                //assert($r->name === $token->name);
      23:                //assert(empty($token->attr));
      35:        //assert(count($stack) == 1);
       
      lib/htmlpurifier/HTMLPurifier/Zipper.php
      134:     *      assert($old1 === $old2);
      135:     *      assert($arr1 === $arr2);
       
      lib/htmlpurifier/HTMLPurifier/ChildDef/Table.php
      206:                    //assert($node->is_whitespace);
       
      mod/wiki/diff/diff_nwiki.php
      164:            USE_ASSERTS_IN_WIKI && assert($yi < $n_to || $this->xchanged[$xi]);
      165:            USE_ASSERTS_IN_WIKI && assert($xi < $n_from || $this->ychanged[$yi]);
      253:            USE_ASSERTS_IN_WIKI && assert($k > 0);
      259:            USE_ASSERTS_IN_WIKI && assert($y < $this->seq[$k]);
      268:            USE_ASSERTS_IN_WIKI && assert($k > 0);
      304:    USE_ASSERTS_IN_WIKI && assert($ypos != $this->seq[$end]);
      384:    USE_ASSERTS_IN_WIKI && assert('sizeof($lines) == sizeof($changed)');
      404:        USE_ASSERTS_IN_WIKI && assert('$j < $other_len && ! $other_changed[$j]');
      436:            USE_ASSERTS_IN_WIKI && assert('$j > 0');
      439:            USE_ASSERTS_IN_WIKI && assert('$j >= 0 && !$other_changed[$j]');
      462:            USE_ASSERTS_IN_WIKI && assert('$j < $other_len && ! $other_changed[$j]');
      479:        USE_ASSERTS_IN_WIKI && assert('$j > 0');
      482:        USE_ASSERTS_IN_WIKI && assert('$j >= 0 && !$other_changed[$j]');
      666:        assert(sizeof($from_lines) == sizeof($mapped_from_lines));
      667:        assert(sizeof($to_lines) == sizeof($mapped_to_lines));
      888:            assert(!strstr($word, "\n"));
      

      So, in practice... from the list above, the only real cases to examine are the mod/wiki/diff/diff_nwiki.php ones. Everything else are "false positives" (comments and declarations).

            stronk7 Eloy Lafuente (stronk7)
            stronk7 Eloy Lafuente (stronk7)
            Tim Hunt Tim Hunt
            Andrew Lyons Andrew Lyons
            Andrew Lyons Andrew Lyons
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved:

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 2 hours, 55 minutes
                2h 55m

                  Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.