Moodle
  1. Moodle
  2. MDL-31711

invalid usages of '@property' found for class variables that are not really 'magic' variables.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: phpdoc
    • Labels:
    • Rank:
      38298

      Description

      i was working on MDL-25027 when i came across usages of @property - lib/externallib.php class external_value's phpdoc has :

          /** @property mixed $type value type PARAM_XX */
          public $type;
      

      see - http://manual.phpdoc.org/HTMLSmartyConverter/HandS/phpDocumentor/tutorial_tags.property.pkg.html

      looking it up i read this about @property - '....in the "Property Summary" listing of your class variables, but you will NOT see "$regular", "$foo", or "$bar" there...'
      clearly some usages of @property should actually be @var as we DO want them documented and shown in the class's property summary.

      @property is really only for magic variables where we want to HIDE them from normal users.

        Activity

        Hide
        Aparup Banerjee added a comment -

        I've put up a patch that will now have these variables show up in generated documentation properly.


        ps: i've ignored other @property usages where they are simply used as a supplement in a class's docblock as part of the doc block's description :

        • mod/lesson/locallib.php
        • calender/lib.php
          Those won't effectively hide any documentation as the variables are marked with @var still anyway.
        Show
        Aparup Banerjee added a comment - I've put up a patch that will now have these variables show up in generated documentation properly. ps: i've ignored other @property usages where they are simply used as a supplement in a class's docblock as part of the doc block's description : mod/lesson/locallib.php calender/lib.php Those won't effectively hide any documentation as the variables are marked with @var still anyway.
        Hide
        Ankit Agarwal added a comment -

        Hi Aparup,
        I also found some instances of @property in lib/externallib.php, which I don't think are "magic" variables either.
        Please have a look if these needs to be changed to @var or not.

        The patch looks great. +1 to integrate, once above mentioned instances are sorted.
        Thanks

        Show
        Ankit Agarwal added a comment - Hi Aparup, I also found some instances of @property in lib/externallib.php, which I don't think are "magic" variables either. Please have a look if these needs to be changed to @var or not. The patch looks great. +1 to integrate, once above mentioned instances are sorted. Thanks
        Hide
        Aparup Banerjee added a comment -

        ah some changes were done in MDL-30994 already now.

        Show
        Aparup Banerjee added a comment - ah some changes were done in MDL-30994 already now.
        Hide
        Aparup Banerjee added a comment -

        i've made changes to lib/externallib.php , please review and push up for integration

        Show
        Aparup Banerjee added a comment - i've made changes to lib/externallib.php , please review and push up for integration
        Hide
        Ankit Agarwal added a comment -

        Looks good, sending for integration.
        Thanks

        Show
        Ankit Agarwal added a comment - Looks good, sending for integration. Thanks
        Hide
        Eloy Lafuente (stronk7) added a comment -

        The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        TIA and ciao

        Show
        Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
        Hide
        Sam Hemelryk added a comment -

        Thanks Apu, this has been integrated now

        Show
        Sam Hemelryk added a comment - Thanks Apu, this has been integrated now
        Hide
        Sam Hemelryk added a comment -

        Passes fine

        Show
        Sam Hemelryk added a comment - Passes fine
        Hide
        Aparup Banerjee added a comment -

        The code here has been spread to upstream moodle repositories and mirrors for anyone to use .

        Closing, have a good weekend!

        Show
        Aparup Banerjee added a comment - The code here has been spread to upstream moodle repositories and mirrors for anyone to use . Closing, have a good weekend!

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: