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

Review nested ternary operators adding parenthesis where needed

    XMLWordPrintable

Details

    • Bug
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • 3.8, 3.9
    • 3.8.1
    • General
    • MOODLE_38_STABLE, MOODLE_39_STABLE
    • MOODLE_38_STABLE
    • Hide
      1. Verify that, with php74, there aren't more deprecated notices like this running PHPUnit:

        PHP Deprecated:  Unparenthesized `a ? b : c ? d : e` is deprecated. Use either `(a ? b : c) ? d : e` or `a ? b : (c ? d : e)` in ....
        

        (you can do that going to the "Console Output -> View as plain text" link of the following job that has been launched with the patch above applied and, then, looking to "Unparenthesized")
        Job Link: https://ci.moodle.org/view/Testing/job/DEV.02%20-%20Developer-requested%20PHPUnit/623/

      2. Verify that everything else in automate-land (travis, CI tests) continue passing ok.
      Show
      Verify that, with php74, there aren't more deprecated notices like this running PHPUnit: PHP Deprecated: Unparenthesized `a ? b : c ? d : e` is deprecated. Use either `(a ? b : c) ? d : e` or `a ? b : (c ? d : e)` in .... (you can do that going to the "Console Output -> View as plain text" link of the following job that has been launched with the patch above applied and, then, looking to "Unparenthesized") Job Link: https://ci.moodle.org/view/Testing/job/DEV.02%20-%20Developer-requested%20PHPUnit/623/ Verify that everything else in automate-land (travis, CI tests) continue passing ok.

    Description

      Nesting ternary operators without explicit parentheses is deprecated:

              // Code like
              $a ? $b : $c ? $d : $e
              // should be replaced by (current interpretation)
              ($a ? $b : $c) ? $d : $e
              // or (likely intended interpretation)
              $a ? $b : ($c ? $d : $e)
      

      So this is about to review all the "nested" ternary operators in core, adding the the "current interpretation" parenthesis if missing. We are not changing any behavior here, just making the interpretation explicit.

      And yes, the "current interpretation is the "crazy one" (left->right). That's the way PHP works.

      <?php
      $a = -1;
      echo $a < 0 ? "-1" : $a > 0 ? "+1" : "0";
       
      +1 (were you expecting a -1, nah!)
      

      Here there is the list of files having likely nested ternary operators, calculated with:

      $ ag '\?[^:;=]+:[^\?;=]+\?[^:;=]+:[^;]+;' --php -l

      As far as there are some false positives there, let's use the php semantic searcher to, accurately, look for nested ternaries. Those will be our target for this issue:

      ag '\?[^:;=]+:[^\?;=]+\?[^:;=]+:[^;]+;' --php -l | \
          xargs ../php-search/bin/php-search -e "ternary / ternary" | \
          grep "found." | sort | uniq

      46 matches. Here it's the list, let's review it: https://pastebin.com/e7U123MS

      Attachments

        Activity

          People

            stronk7 Eloy Lafuente (stronk7)
            stronk7 Eloy Lafuente (stronk7)
            Peter Dias Peter Dias
            Jake Dallimore Jake Dallimore
            Gladys Basiana Gladys Basiana
            Adrian Greeve, David Woloszyn, Huong Nguyen, Jake Dallimore, Meirza, Michael Hawkins, Raquel Ortega, Safat Shahin, Stevani Andolo
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:
              13/Jan/20

              Time Tracking

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