Fix documentation for Revision::getComment and WikiPage::getComment
authorThiemo Kreuz <thiemo.kreuz@wikimedia.de>
Tue, 15 Jan 2019 13:30:45 +0000 (14:30 +0100)
committerThiemo Kreuz <thiemo.kreuz@wikimedia.de>
Tue, 15 Jan 2019 13:42:17 +0000 (14:42 +0100)
This mismatch between the documentation and how the code behaves was
introduced via Ia4c20a9 in December 2017. The getComment() methods have
never been documented to return null, but now do.

I tried to fix this both ways (see I32202df for the other approach), but
decided it's easier and cleaner to keep the null. Reasoning:

* The code behaves like this for more than a year already. Removing the
  null would possibly be as desruptive as when it was introduced.

* Removing the null breaks Wikibase.

* I think returning null is the right thing to do.

I did a brief check of all callers of these two methods. I found a few
that might misbehave a little bit when they are confronted with null, e.g.
in MobileFrontend or Echo. In the vast majority of cases the null will be
silently casted to an empty string and be fine. In a few cases a "the
comment is empty" message might disappear, and the null be shown in the UI
instead, possibly resulting in an empty, confusing HTML element. I would
argue this is an issue with the frontend code then. It should not expect
to have access to all comments. Some might be supressed.

Bug: T174025
Change-Id: I0e1ff9686d1d875812460631c29330c398e74bcf

includes/Revision.php
includes/page/WikiPage.php

index b0a3ba3..47b2ac4 100644 (file)
@@ -847,17 +847,15 @@ class Revision implements IDBAccessObject {
                return $user ? $user->getName() : '';
        }
        /**
-        * Fetch revision comment if it's available to the specified audience.
-        * If the specified audience does not have access to the comment, an
-        * empty string will be returned.
-        *
         * @param int $audience One of:
         *   Revision::FOR_PUBLIC       to be displayed to all users
         *   Revision::FOR_THIS_USER    to be displayed to the given user
         *   Revision::RAW              get the text regardless of permissions
         * @param User|null $user User object to check for, only if FOR_THIS_USER is passed
         *   to the $audience parameter
-        * @return string
+        *
+        * @return string|null Returns null if the specified audience does not have access to the
+        *  comment.
         */
        function getComment( $audience = self::FOR_PUBLIC, User $user = null ) {
                global $wgUser;
index b1bf7bd..b4fcc62 100644 (file)
@@ -895,7 +895,8 @@ class WikiPage implements Page, IDBAccessObject {
         *   Revision::RAW              get the text regardless of permissions
         * @param User|null $user User object to check for, only if FOR_THIS_USER is passed
         *   to the $audience parameter
-        * @return string Comment stored for the last article revision
+        * @return string|null Comment stored for the last article revision, or null if the specified
+        *  audience does not have access to the comment.
         */
        public function getComment( $audience = Revision::FOR_PUBLIC, User $user = null ) {
                $this->loadLastEdit();