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

Review nested ternary operators adding parenthesis where needed

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.8, 3.9
    • Fix Version/s: 3.8.1
    • Component/s: General
    • Labels:
    • Testing Instructions:
      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.
    • Affected Branches:
      MOODLE_38_STABLE, MOODLE_39_STABLE
    • Fixed Branches:
      MOODLE_38_STABLE
    • Pull Master Branch:

      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

            Assignee:
            stronk7 Eloy Lafuente (stronk7)
            Reporter:
            stronk7 Eloy Lafuente (stronk7)
            Peer reviewer:
            Peter Dias
            Integrator:
            Jake Dallimore
            Tester:
            Gladys Basiana
            Participants:
            Component watchers:
            Adrian Greeve, Jake Dallimore, Mathew May, Mihail Geshoski, Peter Dias, Sujith Haridasan
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:
              Fix Release Date:
              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